-
Notifications
You must be signed in to change notification settings - Fork 63
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.
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
add profileShowDirectDeposit release toggle #3291
Conversation
config/initializers/flipper.rb
Outdated
@@ -6,7 +6,7 @@ | |||
require 'flipper/adapters/active_support_cache_store' | |||
|
|||
# Add new Feature toggles here. They will be off until toggled int the UI | |||
FLIPPER_FEATURES = [].freeze | |||
FLIPPER_FEATURES = ['profileShowDirectDeposit'].freeze |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@annaswims should this be snake case?
@@ -43,7 +43,7 @@ | |||
# default feautures to enabled for development and test only | |||
Flipper.enable(feature) if Rails.env.development? || Rails.env.test? | |||
end | |||
rescue ActiveRecord::NoDatabaseError, ActiveRecord::StatementInvalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
catching the all of the db errors pre-migrations was a game of whack-a-mole
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a Raven.tags_context(feature: 'flipper')
so we can easily find all flipper related errors in sentry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All of the errors were when I tried to run a rake task like rake db:migrate
or rake db:setup
before the flipper before the flipper tables existed. So, I don't expect any of the errors to be in prod.
@@ -8,3 +8,6 @@ features: | |||
description: > | |||
On https://www.va.gov/find-locations/ enable veterans to search for Community care by showing that option in the "Search for" box. | |||
This toggle is owned by Rian | |||
profile_show_direct_deposit: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perfect thank you!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One thing I'm wondering about now is if we want to have a singular toggle for "enable the API endpoint for direct deposit" and "show it in the UI". If we had reason to shut it off we might want to make sure the backend also won't accept requests to update the banking info.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think separate toggles work for short lived toggles (release toggles) and that long term toggles (ops toggle) would cover it both systems.
We also have the option to poll the toggle values and live update ui.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one comment re: sentry
@@ -43,7 +43,7 @@ | |||
# default feautures to enabled for development and test only | |||
Flipper.enable(feature) if Rails.env.development? || Rails.env.test? | |||
end | |||
rescue ActiveRecord::NoDatabaseError, ActiveRecord::StatementInvalid |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe add a Raven.tags_context(feature: 'flipper')
so we can easily find all flipper related errors in sentry?
I think this is ready for final approval. |
Description of change
Testing done
Testing planned
Acceptance Criteria (Definition of Done)
Unique to this PR
Applies to all PRs