-
Notifications
You must be signed in to change notification settings - Fork 18
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
1890: Add documentation for making migrations in user_group #1985
Conversation
🥳 Successfully deployed to developer sandbox es. |
🥳 Successfully deployed to developer sandbox es. |
1 similar comment
🥳 Successfully deployed to developer sandbox es. |
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
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.
Hey Erin can we actually move some of these instructions to developer readme? Or even duplicating it in the developer readme given that this is such a weird thing. Another place this could go instead is the migration troubleshooting document or the document on databases. I will leave it to you on which one feels like a doc you may see it in first as a new developer.
Also sorry on the confusion on this ticket. You don't need to update old migration files as migration files should be left untouched when possible after they are applied.
Good point, thank you @abroddrick! I just duplicated the docs to the user-permissions docs but let me know if we should remove these docs from user_group to remove redundancy. Also reverted the docs changes to our migration files |
🥳 Successfully deployed to developer sandbox es. |
🥳 Successfully deployed to developer sandbox es. |
docs/developer/user-permissions.md
Outdated
file for user permission changes. | ||
To update analyst permissions do the following: | ||
1. Make desired changes to analyst group permissions in user_group.py. | ||
2. Follow the steps in 0037_create_groups_v01.py to create a duplicate |
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.
non blocking suggestion
2. Follow the steps in 0037_create_groups_v01.py to create a duplicate | |
2. Follow the steps in the migration file "0037_create_groups_v01.py" to create a duplicate |
or alternatively, you could make this a link
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.
Just added the suggestion - thank you!
🥳 Successfully deployed to developer sandbox es. |
Addressed these changes as discussed in sprint planning meeting
Ticket
Resolves #1890
Changes
Add documentation directing people updating user group permissions to custom migration instructions documented in
0037_create_groups_v01.py
.Fixes some errors in dependency comments on user_group migrations. Am open to removing these comments on dependencies if we don't find them necessary to comment in user group migrations.
Context for reviewers
Setup
Code Review Verification Steps
As the original developer, I have
Satisfied acceptance criteria and met development standards
Ensured code standards are met (Original Developer)
Validated user-facing changes (if applicable)
As a code reviewer, I have
Reviewed, tested, and left feedback about the changes
Ensured code standards are met (Code reviewer)
Validated user-facing changes as a developer
New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
Checked keyboard navigability
Meets all designs and user flows provided by design/product
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Tested with multiple browsers, the suggestion is to use ones that the developer didn't (check off which ones were used)
(Rarely needed) Tested as both an analyst and applicant user
Note: Multiple code reviewers can share the checklists above, a second reviewers should not make a duplicate checklist
As a designer reviewer, I have
Verified that the changes match the design intention
Validated user-facing changes as a designer
Checked keyboard navigability
Tested general usability, landmarks, page header structure, and links with a screen reader (such as Voiceover or ANDI)
Tested with multiple browsers (check off which ones were used)
(Rarely needed) Tested as both an analyst and applicant user
Screenshots