Skip to content

refactor: better query for user counts#248

Merged
marius-mather merged 3 commits into
mainfrom
refactor/user-count-query
Apr 1, 2026
Merged

refactor: better query for user counts#248
marius-mather merged 3 commits into
mainfrom
refactor/user-count-query

Conversation

@marius-mather

@marius-mather marius-mather commented Apr 1, 2026

Copy link
Copy Markdown
Collaborator

Description

Rework functions that count users (based on query params) - use dedicated query logic that just counts user IDs instead of selecting the whole user table.

Changes

  • Rework query in UserQueryParams.get_count()
  • Add unit test for counts

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)

Run uv run pytest

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Refactors admin user-counting logic to run a dedicated COUNT() query instead of selecting full user rows, and adds a regression test to ensure the current admin user is excluded from pagination totals.

Changes:

  • Reworked UserQueryParams.get_count() to issue a direct SELECT COUNT(user_id) with existing permission/filter conditions.
  • Added an admin API test covering /admin/users/pages totals with exclusion of the current admin user across pagination and approval-status filters.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
routers/admin.py Updates UserQueryParams.get_count() to use a direct COUNT() statement with existing permission/filter predicates.
tests/admin_api/test_admin.py Adds regression coverage for /admin/users/pages to ensure the current admin user is excluded from counts.

Comment thread routers/admin.py
Comment thread routers/admin.py Outdated

@amandazhuyilan amandazhuyilan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice logic change! Hopes this makes the query faster

@marius-mather marius-mather merged commit ca3596f into main Apr 1, 2026
5 checks passed
@marius-mather marius-mather deleted the refactor/user-count-query branch April 1, 2026 03:47
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