Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unescapes feature_name in FeatureNameFromRoute #377

Merged
merged 1 commit into from
Sep 30, 2018
Merged

Unescapes feature_name in FeatureNameFromRoute #377

merged 1 commit into from
Sep 30, 2018

Conversation

AlexWheeler
Copy link
Collaborator

@AlexWheeler AlexWheeler commented Sep 28, 2018 •
edited
Loading

  • Before we supported slashes we unescaped the feature name, but the
    change to get feature_name using FeatureNameFromRoute is not
    unescaping feature names.

This causes 2 issues:

  1. The redirect when creating the feature displays the feature name unescaped
  2. When you go to enable a gate for the feature the feature_key is the unescaped value

Reproduce:

  • Flipper, Flipper UI, Flipper ActiveRecord - (all 0.16.0)
  • Rails 4.2.4
  • browsers: ChromeVersion 69.0.3497.100 and SafariVersion 11.1
  1. Navigate to features/new and create my_team:cool_feature. This will redirect you to feature page:

create-feature-redirect

  1. Confirm database feature record looks correct

feature-key

  1. Enable the boolean gate for this feature by clicking "Enable"

  2. Confirm it is enabled for the wrong feature_key

gate-feature-key

Note: All is good if you link to the feature from the /features view

@AlexWheeler
Unescapes feature_name in FeatureNameFromRoute
910225d 
* Before we supported slashes we unescaped the feature name, but teh
change to pull feature_name using  FeatureNameFromRoute is not
unescaping feature names.  This is an issue because when you create a
new feature it redirects to the feature show page.  Then when you go to
enable a gate the feature_key the gate gets is not unescaped so the
feature is not enabled.
jnunemaker
jnunemaker approved these changes Sep 30, 2018
View reviewed changes
Copy link
Collaborator

@jnunemaker jnunemaker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool. Happy to get this out. Curious if there is any regression test that could be added, but not a blocker for this.

@jnunemaker jnunemaker merged commit 7151715 into master Sep 30, 2018
@jnunemaker jnunemaker deleted the unescape-feature-name branch September 30, 2018 20:28
@AlexWheeler AlexWheeler mentioned this pull request Oct 2, 2018
Merged
@AlexWheeler AlexWheeler mentioned this pull request Jan 31, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jnunemaker jnunemaker jnunemaker approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AlexWheeler @jnunemaker