-
Notifications
You must be signed in to change notification settings - Fork 35
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 20010 int Add FF for HQ role #12984
Conversation
…e hq role request
|
@@ -62,10 +62,10 @@ export const RequestAccount = ({ setFlashMessage }) => { | |||
roleType: 'qae_csr', | |||
}); | |||
} | |||
if (values.headquartersCheckBox) { | |||
if (values.customerSupportRepresentativeCheckBox) { |
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.
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.
Ha, figures. It didn't even flag it as a conflict. I'll go fix that.
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.
Fixed it. Only did that on this INT branch. Thanks git *sigh
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.
Functions as described. Tests are passing.
Just needs that duplicate if statement removed but that shouldn't affect functionality.
When I create a new HQ user from http://officelocal:3000/devlocal-auth/login with the FF enabled it only has the TOO role. |
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.
Never mind, with the comment above. No need to hold up this any longer. I'm adding a fix for it in my PR, it could reasonably be part of either.
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.
Looks good. Just one minor CONSTANTS usage comment.
B-20010
Summary
The changes here are to FF off the Headquarters Role in the Admin App Office Account creation page and remove the HQ role checkbox from the Office App Request Account page. Since FF can't currently be read without being signed in the Office App will get the HQ role checkbox back once we determine all HQ role options are functioning properly. For now the only way to create a HQ user role is via the Admin App Office Creation page with the FEATURE_FLAG_HEADQUARTERS_ROLE feature flag turned on.
This backlog adds a new Headquarters Office user role. Further downstream tasks will add privileges meant for the HQ role which will limit pages to read only.
Is there anything you would like reviewers to give additional scrutiny?
Turn the FEATURE_FLAG_HEADQUARTERS_ROLE on/off and test.
Verification Steps for the Author
These are to be checked by the author.
Verification Steps for Reviewers
These are to be checked by a reviewer.
Setup to Run the Code
How to test
Go to the Office app and sign in as the newly created user
Click the change user role and verify you can login with the HQ role
NOTE: At this point you should be able to do what a TOO can do but this story was to just allow a new login role
Turn off the FEATURE_FLAG_HEADQUARTERS_ROLE by changing it to false in the .envrc file
Also turn off headquarters_role in the development.features.yaml file by setting enabled and value to false for the headquarters_role key.
Kill your server and client in the terminal
Run direnv allow
Relaunch server and client in terminal
Login to the Admin app and go to create a new Office User
Fill out the form then attempt to select the Headquarters role checkbox you should NOT be able to
Frontend
officeApp
class or custommin-width
styling is used to hide any states the would not be visible to the user.Backend
Database
Any new migrations/schema changes:
Screenshots