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.

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

Feat/reinvite staff user #495

Open
wants to merge 10 commits into
base: development
Choose a base branch
from

Conversation

jusdino
Copy link
Collaborator

@jusdino jusdino commented Jan 24, 2025

Description List

  • Added a POST staff-user/:userId/reinvite endpoint that will resend an invite email, resetting the user's password if necessary

Testing List

  • Code review

Closes #
closes #476

@jusdino jusdino marked this pull request as draft January 24, 2025 15:14
@jusdino
Copy link
Collaborator Author

jusdino commented Jan 24, 2025

We'll need to merge #475 and rebase this branch, before we can merge this one.

Copy link
Collaborator

@landonshumway-ia landonshumway-ia left a comment

Choose a reason for hiding this comment

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

Just one small gap in checking the Cognito Status values

@landonshumway-ia landonshumway-ia self-requested a review January 24, 2025 22:07
@jusdino jusdino force-pushed the feat/reinvite-staff-user branch from d460d66 to 53eca9b Compare January 28, 2025 15:46
@jusdino jusdino force-pushed the feat/reinvite-staff-user branch from 53eca9b to 10419bb Compare January 28, 2025 15:51
@jusdino jusdino marked this pull request as ready for review January 28, 2025 15:51
@jusdino
Copy link
Collaborator Author

jusdino commented Jan 28, 2025

@jlkravitz , this one is ready for you!

@jusdino jusdino closed this Jan 28, 2025
@jusdino jusdino reopened this Jan 28, 2025
Copy link
Collaborator

@jlkravitz jlkravitz left a comment

Choose a reason for hiding this comment

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

Looks good! Few nits/questions.

Comment on lines +172 to +184
allowed_jurisdictions = get_allowed_jurisdictions(compact=compact, scopes=get_event_scopes(event))

# None means they are a compact admin - no jurisdiction restrictions at all
user = config.user_client.get_user_in_compact(compact=compact, user_id=user_id)
if allowed_jurisdictions is not None:
allowed_jurisdictions = set(allowed_jurisdictions)
user = config.user_client.get_user_in_compact(compact=compact, user_id=user_id)
user_jurisdictions = user['permissions']['jurisdictions'].keys()
common_jurisdictions = allowed_jurisdictions.intersection(user_jurisdictions)

# We won't show that the user even exists, if they have no common jurisdictions
if not common_jurisdictions:
raise CCNotFoundException('User not found')
Copy link
Collaborator

Choose a reason for hiding this comment

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

In what scenario will this fail, and why is this code necessary to reinvite a user?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This will return a 404 in the event that a user calls this endpoint with an id for a user they don't have permission to see (i.e. a board admin in Ohio tries to reinvite a user who has no permissions in Ohio).

backend/ruff.toml Outdated Show resolved Hide resolved
@jusdino
Copy link
Collaborator Author

jusdino commented Jan 31, 2025

@jlkravitz all resolved, I think

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Reinvite staff user
3 participants