-
Notifications
You must be signed in to change notification settings - Fork 0
feat: add rejected approval status (AAI-526) #141
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
Conversation
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.
Pull request overview
This PR adds rejection functionality for group membership requests, aligns state transitions to be more explicit, and implements automatic linking of Auth0 admin roles to platforms and groups based on naming conventions.
Key Changes
- Introduced a new
REJECTEDstatus toApprovalStatusEnumwith arejection_reasonfield for group memberships - Implemented state transition validation that prevents invalid state changes (e.g., approving already-approved memberships now returns 400 instead of 200)
- Added
link_admin_roles()function to automatically associate admin roles with platforms/groups based on the patternbiocommons/role/{id}/admin
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| migrations/versions/c4c7a8e9b2d3_group_membership_rejection.py | Adds database migration for the REJECTED enum value and rejection_reason column |
| db/types.py | Adds REJECTED status to ApprovalStatusEnum and rejection_reason to GroupMembershipData |
| db/models.py | Implements reject() method, updates save_history() to handle rejection reasons, adds Auth0Role.get_all() |
| routers/admin.py | Adds reject endpoint for group memberships, adds state validation to approval endpoints |
| routers/biocommons_groups.py | Allows users to re-request group access after rejection, validates state transitions in approval |
| scheduled_tasks/tasks.py | Implements link_admin_roles() to automatically associate admin roles with platforms/groups |
| tests/test_admin.py | Adds comprehensive tests for rejection workflow and state transition validation |
| tests/biocommons/test_api.py | Tests re-requesting after rejection and preventing approval of rejected memberships |
| tests/db/test_models.py | Updates existing test to ensure approval status is set correctly |
| tests/scheduled_tasks/test_tasks.py | Tests link_admin_roles() functionality including case-insensitive matching |
marius-mather
left a comment
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 but I don't know if we need a separate _reason field for each type of change, seems cleaner to just have an all-purpose reason field.
marius-mather
left a comment
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.
good to go
Description
AAI-526: add rejection status
Changes
Checklist
How to Test Manually (if necessary)
register a user for a bundle
log in as bundle admin
find the user click reject, record a reason