-
Notifications
You must be signed in to change notification settings - Fork 38
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
B 20034 int #12649
B 20034 int #12649
Conversation
Functions as described both with correct and incorrect codes. Testing instructions should probably include that the One test is failing due to spectral issues. Will approve once that is passing. |
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.
It seems to work. One issue I have is that we had to manually add a record to the table. What happens when a developer decides to turn the FF to true like what we did for this PR? I don't see a specific migration to do the INSERT.
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.
See a above comment.
20034
Review #1
Review #2
Summary
This PR is to fix some pre-test issues in the main review, that passed during the integration review(s). This review also includes a new UPDATE statement in the migrations. This is to automatically update the application_parameters table from supporting just validation_codes to supporting the new param_name and param_value structure. The INSERT for the validation_code values was initially done manually in testing and production environments.
How to test
Couple of setup things you need to do first:
make db_dev_migrate
(this will create theapplication_parameters
tableparameter_name
, 'parameter_value' andid
,updated_at
andcreated_at
will auto populate.envrc
, setexport FEATURE_FLAG_VALIDATION_CODE_REQUIRED=true
client
andserver
To test db update of validation_code to param_name and param_value (this is meant to automatically update real environments to support the new table structure):
Sample SQL statement for insert:
INSERT INTO application_parameters(id, validation_code)
VALUES ('30457d8e-b108-4173-a757-4acc17983904', 'test1234');
UI tests:
For the happiest path
application_parameters
tableFor the saddest path
Screenshots
Mobile
Mobile