Skip to content

Paginate monthly template usage stats#2858

Merged
whabanks merged 3 commits intomainfrom
task/paginate-template-stats
May 6, 2026
Merged

Paginate monthly template usage stats#2858
whabanks merged 3 commits intomainfrom
task/paginate-template-stats

Conversation

@whabanks
Copy link
Copy Markdown
Contributor

@whabanks whabanks commented May 5, 2026

Summary | Résumé

This PR aims to improve performance of the "Monthly Template Usage" page in admin with optimizations to the underlying query. For the average service load times should be similar, the real gain is for services with high sending volume across a large number of templates. The JSON payload from the API is bound by the page size, and the browser won't choke trying to render thousands of table rows.

Added fetch_monthly_template_usage_for_service_paginated dao method that performs pagination at the query level, splitting the query into two and running the more expensive par on the templates on the current "page" only.

  1. Get the page of template_ids, sorting them by count descending. We use a combination of offset and limit to construct the page.
  2. Fetch the detailed template stats using that subset of templates rather than fetching everything all at once.

Related Issues | Cartes liées

Test instructions | Instructions pour tester la modification

  • CI is passing
  • Release instructions test has been performed

Release Instructions | Instructions pour le déploiement

  1. Check out this branch and with your local Admin on main to ensure backwards compatibility (not the associated admin PR)
  • The dashboard loads without error
  • The /template-usage page loads without error (See all templates used link on the dashboard)

Reviewer checklist | Liste de vérification du réviseur

  • This PR does not break existing functionality.
  • This PR does not violate GCNotify's privacy policies.
  • This PR does not raise new security concerns. Refer to our GC Notify Risk Register document on our Google drive.
  • This PR does not significantly alter performance.
  • Additional required documentation resulting of these changes is covered (such as the README, setup instructions, a related ADR or the technical documentation).

⚠ If boxes cannot be checked off before merging the PR, they should be moved to the "Release Instructions" section with appropriate steps required to verify before release. For example, changes to celery code may require tests on staging to verify that performance has not been affected.

Added `fetch_monthly_template_usage_for_service_paginated` dao method
that performs pagination at the query level, and runs the expensive part
of the query only on templates on the current "page".
@whabanks whabanks marked this pull request as ready for review May 5, 2026 18:09
@whabanks whabanks requested a review from jimleroyer as a code owner May 5, 2026 18:09
Copilot AI review requested due to automatic review settings May 5, 2026 18:09
Copy link
Copy Markdown
Contributor

Copilot AI left a 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 introduces database-level pagination for the admin “Monthly Template Usage” endpoint to avoid loading usage stats for all templates at once, aiming to improve performance for large services.

Changes:

  • Added a new DAO method fetch_monthly_template_usage_for_service_paginated that pages by unique templates using a two-step query (page template_ids first, then fetch monthly rows for those templates).
  • Updated GET /service/<id>/notifications/templates_usage/monthly to return a paginated response shape when a page query param is provided.
  • Added DAO- and REST-level tests covering pagination shape, ordering, offsets, and basic validation.

Reviewed changes

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

File Description
app/dao/fact_notification_status_dao.py Adds the new paginated DAO query for monthly template usage.
app/service/rest.py Adds query-param parsing and paginated response mode to the monthly template usage endpoint.
tests/app/dao/test_fact_notification_status_dao.py Adds unit tests for the new paginated DAO behavior.
tests/app/service/test_statistics_rest.py Adds endpoint tests for paginated response shape, ordering, links, and invalid args.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread app/service/rest.py Outdated
Comment thread app/dao/fact_notification_status_dao.py
Comment thread app/dao/fact_notification_status_dao.py Outdated
whabanks added 2 commits May 5, 2026 14:49
- Added a request schema for `get_monthly_template_usage` as qParam
validation was getting out of hand
- Ensure that we also fetch from the `notifications` table when counting
  template stats and today's date falls in the start->end range
- Fix a rare edge case where two templates have the same notification
  count associated with them that could cause inconsistencies in paging
results
Copy link
Copy Markdown
Member

@andrewleith andrewleith left a comment

Choose a reason for hiding this comment

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

LGTM

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