Skip to content

Conversation

@amandazhuyilan
Copy link
Contributor

@amandazhuyilan amandazhuyilan commented Nov 25, 2025

Description

AAI-530: Add endpoint to caluculate users

Changes

  • Added UserCountsResponse, count helper, and GET admin/users/counts in routers/admin.py
  • Implemented the pending/revoked list logic to include group (bundle) memberships
  • Updated tests to verify counts and permissions.

Checklist

  • I have commented my code, particularly in hard-to-understand areas
  • I have added unit / integration tests that prove my fix is effective or that my feature works
  • I have run all tests locally and they pass
  • I have updated the documentation (if applicable)
  • For any new secrets, I have updated the shared spreadsheet and the GitHub Secrets.

How to Test Manually (if necessary)

<Describe how reviewers can manually, if necessary, test the changes>

@amandazhuyilan amandazhuyilan changed the title feat: add user calculation endpoint feat: add user calculation endpoint and updating pending/revoked logic Nov 25, 2025
Copy link
Collaborator

@marius-mather marius-mather left a comment

Choose a reason for hiding this comment

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

I was going through this and trying to think about how to do this while keeping the query logic inside UserQueryParams where possible - would it help if we added a general approval_status filter that return results for both platforms and groups with the status? We could make it so that you can't specify both approval_status and the more specific platform_approval_status etc. filters. That way we could probably do most of what we need within the UserQueryParams logic

@amandazhuyilan
Copy link
Contributor Author

amandazhuyilan commented Nov 25, 2025

Thanks for the review and the suggestion @marius-mather - UserQueryParams now includes an approval_status filter that builds a combined condition over both platform and group memberships. Pending/revoked endpoints and the /admin/users/counts counts use this general status filter to include both platforms and bundles.

Copy link
Collaborator

@marius-mather marius-mather left a comment

Choose a reason for hiding this comment

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

A couple of minor changes to suggest but otherwise this is looking great!

Copy link
Collaborator

@marius-mather marius-mather left a comment

Choose a reason for hiding this comment

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

Looks great, just needs a comment on how the approval status logic works because it was confusing (to me at least)

Copy link
Collaborator

@marius-mather marius-mather left a comment

Choose a reason for hiding this comment

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

looking good, thanks!

@amandazhuyilan amandazhuyilan merged commit 538503b into main Nov 25, 2025
4 checks passed
@amandazhuyilan amandazhuyilan deleted the AAI-530-calculate-users branch November 25, 2025 23:13
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.

3 participants