#4769: Add standard users to fixtures - [ab]

[allly-b]

Ticket

Resolves #4769

Changes

• Added STANDARD_USERS list (feedback+1@get.gov through feedback+8@get.gov) to src/registrar/fixtures/fixtures_users.py
• Added src/registrar/fixtures/fixtures_standard_user_domains.py — a new StandardUserDomainFixture class that creates domain requests in every DomainRequestStatus and domains in every Domain.State for each standard user
• Added load_standard_user_permissions() to src/registrar/fixtures/fixtures_user_portfolio_permissions.py — assigns each standard user an ORGANIZATION_MEMBER portfolio role with EDIT_REQUESTS and VIEW_MEMBERS permissions
• Updated src/registrar/management/commands/load.py to call StandardUserDomainFixture.load()
• Updated docs/developer/README.md with a new "Adding Standard Users" section

Context for reviewers

Domain states are forced so no EPP commands are sent. However, there is a desire to have fixtures send epp commands for only these domains in the future. As of now these domains will show as "unknown" in EPP and may throw errors if any code attempts to interact with the registry for them. However, given the desire to add in epp down the road for these I opted to isolate this code into a separate file to not mix it's logic in with the existing fixture items.

Setup

On a sandbox (not advised approach but worth mentioning)

Standard users load automatically when ./manage.py load runs as part of reset-db.yml. This will reset everything in the database though.

1. go to Actions in github
2. select reset-db
3. in the drop down select this branch and your sandbox.
4. Once it finishes you should have all the standard users in your sandbox

On a sandbox (less annoying approach)

1. cf login -a api.fr.cloud.gov --sso
2. cf target -o cisa-dotgov-registry -s getgov-YOURSANDBOX
3. cf run-task getgov-YOURSANDBOX --wait --command 'python manage.py load' --name loaddata
4. Once it finishes you should have all the standard users in your sandbox
5. I suggest double checking the cloud.gov logs as well, keep an eye out for errors.

For running this locally (easiest):

1. go toload.py and comment out if not settings.IS_LOCAL:and replace it withif True:`
2. in terminal, run docker compose down && docker compose up (I'd suggest avoiding using the headless commands so you can see the fixtures output)

How to verify the right things were created

• log into your sandbox (or local env)
• navigate to /admin
• Go to users
• you should see all the standard users
• clicking each one should show the correct amount of domain requests and domains

For designers

• commands have already been run on ab, so you can check to see if the setup for the users looks right to you.

Code Review Verification Steps

As the original developer, I have

Satisfied acceptance criteria and met development standards

• Met the acceptance criteria, or will meet them in a subsequent PR
• Created/modified automated tests
• Update documentation in READMEs and/or onboarding guide

Ensured code standards are met (Original Developer)

• If any updated dependencies on Pipfile, also update dependencies in requirements.txt.
• Interactions with external systems are wrapped in try/except
• Error handling exists for unusual or missing values

Validated user-facing changes (if applicable)

• Follow the process for requesting a design review. If code is not user-facing, delete design reviewer checklist
• Verify new pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
• Checked keyboard navigability
• Reviewed accessibility checklist using screen reader (such as NVDA with Chrome or Voiceover with Safari), ANDI, or WAVE:
  • Tested general usability
  • Page header structure
  • Landmarks
  • Links and buttons
• Checked for errors or warnings with an a11y browser tool (such as ANDI or WAVE)

As a code reviewer, I have

Reviewed, tested, and left feedback about the changes

• Pulled this branch locally and tested it
• Verified code meets all checks above. Address any checks that are not satisfied
• Reviewed this code and left comments. Indicate if comments must be addressed before code is merged
• Checked that all code is adequately covered by tests
• Verify migrations are valid and do not conflict with existing migrations

Validated user-facing changes as a developer

Note: Multiple code reviewers can share the checklists above, a second reviewer should not make a duplicate checklist. All checks should be checked before approving, even those labeled N/A.

• New pages have been added to .pa11yci file so that they will be tested with our automated accessibility testing
• Meets all designs and user flows provided by design/product
• Checked keyboard navigability
• Reviewed accessibility checklist using screen reader (such as NVDA with Chrome or Voiceover with Safari), ANDI, or WAVE:
  • Tested general usability
  • Page header structure
  • Landmarks
  • Links and buttons
• Checked for errors or warnings with an a11y browser tool (such as ANDI or WAVE)
• (Rarely needed) Tested as both an analyst and applicant user

As a designer reviewer, I have

Verified that the changes match the design intention

• Checked that the design translated visually
• Checked behavior. Comment any found issues or broken flows.
• Checked different states (empty, one, some, error)
• Checked for landmarks, page heading structure, and links

Validated user-facing changes as a designer

• Checked keyboard navigability
• Reviewed accessibility checklist using screen reader (such as NVDA with Chrome or Voiceover with Safari), ANDI, or WAVE:
  • Tested general usability
  • Page header structure
  • Landmarks
  • Links and buttons
• Checked for errors or warnings with an a11y browser tool (such as ANDI or WAVE)
• Tested with multiple browsers (check off which ones were used)
  • Chrome
  • Microsoft Edge
  • FireFox
  • Safari
• (Rarely needed) Tested as both an analyst and applicant user

References

• Code review best practices

Screenshots

[abe-alam-ecs]

I'm having trouble following this. Why was this done?

[allly-b]

Good question! The "user" variable is used to set investigator on the next line. But, investigator has to a staff member as only staff members should be approving/investigating the validity of a request. Now that these new users are meant to be purely non staff users for UAT, the code hits an error if any of them are assigned as investigators. Hence, I'm checking if the user is not a staff member, then pick a random staff member as the investigator just so it doesn't error when it gets to approving the domain request.

[abe-alam-ecs]

Just one other question:

Would the team ever run ./manage.py load multiple times? Just wondering if we need an idempotency check here to prevent multiple domain requests and domain approvals (although I think we block those already).

Otherwise, I think this is pretty bullet-proof. Great work, Alysia.

[allly-b]

Just one other question:

Would the team ever run ./manage.py load multiple times? Just wondering if we need an idempotency check here to prevent multiple domain requests and domain approvals (although I think we block those already).

@abe-alam-ecs If the load command is run multiple times you do get multiple domains & requests which was intentional originally for when people wanted to test code with a lot of domains (such as filtering and pagination which is better when you have a decent amount domains & requests). However, it will not create duplicate users, luckily that logic was already in place like you thought. I don't think we need to worry about idempotency for this one just given if this is used for UAT testing and someone triggers it twice it's likely intentional and if not, it will just require deleting some of the requests & domains manually if for some reason it's too many.

[abe-alam-ecs]

This looks right to me! I tested by using the load command on my sandbox. I can see standard users 1-8 with domain requests and domains attached. Approved.

[witha-k]

LGTM!!