Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Feat: automatically associate user to agency #2325

Merged
merged 7 commits into from
Aug 28, 2024
Merged

Feat: automatically associate user to agency #2325

merged 7 commits into from
Aug 28, 2024

Conversation

angela-tran
Copy link
Member

@angela-tran angela-tran commented Aug 27, 2024 •
edited
Loading

Closes #2243

This PR makes it so that users are added to their transit agency's staff_group upon login. This is done with the help of a field called sso_domain on the transit agency.

The user needs to be manually added to their transit agency's customer_service_group to see the part of the dashboard for "In-person enrollment".

Testing locally

  • Set your GOOGLE_SSO_STAFF_LIST environment variable to *
  • Make sure your GOOGLE_SSO_ALLOWABLE_DOMAINS environment variable is compiler.la

Those two steps make it so your Google account will be seen as a "transit agency staff user" - i.e. not put into the "Cal-ITP" group but still marked as is_staff.

  • Set SSO_SHOW_FORM_ON_ADMIN_PAGE to True temporarily in benefits/settings.py
  • Reset your database to use local fixtures - ./bin/reset_db.sh
  • Set up the SSO domain mapping
    • Launch the app and go to /admin and login as your superuser (based on your DJANGO_SUPERUSER_USERNAME and DJANGO_SUPERUSER_PASSWORD environment variables)
    • Edit the "CST" TransitAgency so that it has an sso_domain of compiler.la
    • Create a new group and set it as the staff_group for CST
    • Create another new group and set it as the customer_service_group for CST
  • Log out
  • Log back in using your compiler.la Google account

Expected result: your user has been added to the staff_group for CST upon login. When you go to the agency dashboard, you see the transit agency name, but no button or anything for in-person enrollment.

To elevate the user to having permission to do in-person enrollment:

  • Log in as the superuser
  • Add your Google user to the CST customer service group
  • Log back in as your Google user

Expected result: When you go to the agency dashboard, you see the the transit agency name as well as the in-person enrollment section.

Screenshots

User in CST's staff_group but not the customer_service_group

User in CST's staff_group and customer_service_group

@angela-tran angela-tran self-assigned this Aug 27, 2024
@github-actions github-actions bot added tests Related to automated testing (unit, UI, integration, etc.) migrations [auto] Review for potential model changes/needed data migrations updates back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev labels Aug 27, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Aug 27, 2024 •
edited
Loading

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)		Lines missing
  benefits
  admin.py
  benefits/core
  admin.py
  models.py
Project Total  

This report was generated by python-coverage-comment-action

@angela-tran
feat(model): add sso_domain field to TransitAgency
005799b
@angela-tran
feat: automatically associate user with TransitAgency using SSO domain
58f00be 
this lessens the chance that the user will reach the in_person views
without an associated TransitAgency.
@angela-tran angela-tran force-pushed the feat/link-user-and-agency branch from f0aeef2 to 3f6846c Compare August 27, 2024 20:42
@angela-tran
feat: separate concepts of association to a TransitAgency and permission
b6d61b8 
specifically, permission to do in-person enrollment.
now there are two separate groups representing those concepts instead of
one.

had to specify a `related_name` to avoid name collision when Django
follows relationships "backwards":
https://docs.djangoproject.com/en/5.1/ref/models/fields/#django.db.models.ForeignKey.related_name
https://docs.djangoproject.com/en/5.1/topics/db/queries/#following-relationships-backward

refactored `TransitAgency.for_user` method to loop over user's groups.
@angela-tran
feat: check customer service group when rendering agency dashboard
00c0de3
@angela-tran angela-tran force-pushed the feat/link-user-and-agency branch from 3f6846c to 00c0de3 Compare August 27, 2024 20:53
@angela-tran angela-tran marked this pull request as ready for review August 27, 2024 21:53
@angela-tran angela-tran requested a review from a team as a code owner August 27, 2024 21:53
@machikoyasuda
Copy link
Member

Starting to review now

@machikoyasuda
Copy link
Member

machikoyasuda commented Aug 28, 2024 •
edited
Loading

As a CST group member:
image

As a CST-CS group member:
image

image

machikoyasuda
machikoyasuda previously approved these changes Aug 28, 2024
View reviewed changes
Copy link
Member

@machikoyasuda machikoyasuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks for the instructions.

(When I first tested this with your instructions, I duh forgot to run source .env so it didn't work as expected. After I ran source and confirmed the env vars changed, it worked.)

thekaveman
thekaveman reviewed Aug 28, 2024
View reviewed changes
benefits/core/admin.py Outdated Show resolved Hide resolved
thekaveman
thekaveman reviewed Aug 28, 2024
View reviewed changes
benefits/core/admin.py Outdated Show resolved Hide resolved
@angela-tran
refactor: add user to group instead of group to user
04d3c19 
no functional change, just improving readability
@angela-tran angela-tran force-pushed the feat/link-user-and-agency branch from b69153f to 04d3c19 Compare August 28, 2024 18:56
thekaveman
thekaveman approved these changes Aug 28, 2024
View reviewed changes
Copy link
Member

@thekaveman thekaveman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

@angela-tran
Copy link
Member Author

Thank you both!

@angela-tran angela-tran merged commit 1e2ff89 into main Aug 28, 2024
23 checks passed
@angela-tran angela-tran deleted the feat/link-user-and-agency branch August 28, 2024 19:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thekaveman thekaveman thekaveman approved these changes

@machikoyasuda machikoyasuda Awaiting requested review from machikoyasuda

Assignees

@angela-tran angela-tran

Labels
back-end Django views, sessions, middleware, models, migrations etc. deployment-dev [auto] Changes that will trigger a deploy if merged to dev migrations [auto] Review for potential model changes/needed data migrations updates tests Related to automated testing (unit, UI, integration, etc.)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Ensure user is associated to a TransitAgency before they can access in-person enrollment views
3 participants
@angela-tran @machikoyasuda @thekaveman