Skip to content

feat: server-side pagination and search for contract codes#3530

Merged
daniellefrappier18 merged 18 commits into
mainfrom
daniellef/update-api-pagination
Jul 1, 2026
Merged

feat: server-side pagination and search for contract codes#3530
daniellefrappier18 merged 18 commits into
mainfrom
daniellef/update-api-pagination

Conversation

@daniellefrappier18

@daniellefrappier18 daniellefrappier18 commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What are the relevant tickets?

Description (What does it do?)

Summary

  • Migrates contract codes list from client-side to server-side with pagination and debounced search
  • Adds a dedicated contractDetail API query for per-status seat counts (total, unassigned, assigned, redeemed) instead of deriving them by filtering the full codes list client-side
  • Implements the CSV export button
  • Removes the "Uninvite" action for redeemed codes — redeemed rows now show a disabled button with no menu

Screen Recording (if appropriate):

Screen.Recording.2026-06-26.at.2.03.14.PM.mov

Using NVDA

pagination_NVDA.mp4

How can this be tested?

  • Load the Contract Admin page — verify the four stat blocks Total purchased, Unassigned, Pending claim, Redeemed
  • Type in the search box — verify results filter server-side after a short debounce. Screen reader announcement reflects the correct total count after results load
  • Clear the search — verify the full list is restored
  • Tab between All / Pending claim / Redeemed — verify tab filtering works on the current page of results
  • Paginate through results — verify the correct page loads and the footer shows the right page info
  • Click "Export CSV" — verify a CSV downloads with all assigned/redeemed codes
  • Confirm redeemed rows show a disabled action button with no menu

Additional Context

@github-actions

github-actions Bot commented Jun 26, 2026

Copy link
Copy Markdown

OpenAPI Changes

3 changes: 0 error, 3 warning, 0 info

View full changelog

Unexpected changes? Ensure your branch is up-to-date with main (consider rebasing).

@daniellefrappier18 daniellefrappier18 marked this pull request as ready for review June 26, 2026 15:57
Copilot AI review requested due to automatic review settings June 26, 2026 15:57

This comment was marked as outdated.

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

Copilot reviewed 12 out of 13 changed files in this pull request and generated 6 comments.

Comment thread frontends/api/src/mitxonline/hooks/organizations/queries.ts Outdated
Comment thread frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.tsx Outdated
Comment thread frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.tsx Outdated
Comment thread frontends/main/src/app-pages/ContractAdminPage/AssignSeatsSection.tsx Outdated
Comment thread frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.tsx Outdated

@ChristopherChudzicki ChristopherChudzicki 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.

@daniellefrappier18 this worked well. I left some small suggestions—I think they are all worth doing [with caveat on backend CSV generation...could be followup, and maybe you and Dan discussed it already.]

Comment thread frontends/main/src/app-pages/ContractAdminPage/AssignSeatsSection.test.tsx Outdated
Comment thread frontends/main/src/app-pages/ContractAdminPage/ContractAdminPage.tsx Outdated
Comment thread frontends/api/src/mitxonline/hooks/organizations/queries.ts Outdated
Comment thread frontends/api/src/mitxonline/hooks/organizations/queries.ts Outdated
setTimeout(() => {
setSearchAnnouncement(`${count} result${count !== 1 ? "s" : ""}`)
}, 0)
const handleExportCsv = async () => {

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.

Two comments:

  1. I had thought the plan was to build the CSV on the backend. Is that still the long term plan, or did we change that? IMO, backend generation is still the better long-term plan:
    • with the backend implementation, I'm imagining a single link <a href="/mitxonline/api/v0/b2b/manager/organizations/{org_id}/contracts/{contract_id}/codes/export.csv" download> Export CSV </a> ... very simple on the frontend side, and Python has good builtin tools for building the CSV
    • We discussed an org with several thousand members...9k was the number I remember. This would be 18 serial frontend queries. That's not fast, plus any one of them could fail for transient network issues.
  2. If you leave it here, I would lean toward doing queryClient.fetchQuery(managerOrganizationQueries.managerContractCodes(...params)) rather than using b2bApi client directly.
    • This is the only direct use of an axios API client outside the react-query layer in the entire codebase, so it's a bit of a pattern change
    • going through react query has some advantages, like retrying with exponential backoff (up to max 3 retries) if a query fails due to transient network errors

@dsubak what would you think of shifting the CSV generation to the backend? [Alternatively, I think I missed a meeting on the admin dashboard, so if you two decided to put it here, OK!]

@dsubak dsubak Jun 29, 2026

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.

@ChristopherChudzicki @daniellefrappier18

Danielle and I discussed it in person - I'm happy to give a bespoke export CSV projection if we'd like but I think it'd end up being pretty close to syntactic sugar over similar functionality to /codes w/ filtering and an arbitrarily large page_size.

  1. I believe we are assuming that export would want to respect filters. It might be weird if you had it filtered to a specific status and search term and you hit export and got a huge superset of records in your export. Please correct me if this is not the case.
  2. On the backend I deliberately don't enforce a maximum page size in the pagination configuration to allow for pulling every row in one request, so my thinking was that we could specify a page size == max_learners (as returned by the summary stats in the /contract/{id} request we use elsewhere on the page).

With those in mind, we were basically like "It's essentially just /codes with a different serialization format" when you look at the inputs, outputs and behaviors needed. I do think we ought to strike the pagination from this function - as far as I understand it there shouldn't be the need for more than one request - but otherwise I don't think a bespoke endpoint for csv export is hugely valuable at this time.

That all having been said, if either 1 or 2 does not hold anymore, that'd push me towards a bespoke endpoint. Absolutely let me know if my assumptions are now invalid!

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.

(Also, ALL of this assumes an export fits nicely in memory in a single web request - if we needed to do the 10 ton solve of an async job to export stuff to s3 and allow ya'll to retrieve it by subsequent API request, whole thing would need to look different. I think this is understood by everyone at this point, but wanted to be explicit)

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.

Discussed offline. Key takeaways are as follows!

  • For now, the exported CSV will provide an unfiltered output. We might revisit this in the future, so any backend APIs we stand up should consider retaining filtering, but for now we're just gonna hand em everything.
  • We're going to keep CSV export on the frontend for now. I'll file a ticket to stand up a tiny endpoint to move it to the backend; that endpoint should function really similarly to /codes (potentially including filtering), but we'll be able to drop the page_size arg and can use python csv builtins for actual payload construction. We can adopt that at any point in the future without introducing a breaking change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@ChristopherChudzicki @dsubak As discussed for now we are going to keep the CSV export on the frontend with the plan to move the CSV generation to the backend in the future. I have swapped using the b2bApi client directly in favor of queryClient.fetchQuery

Comment thread frontends/api/src/mitxonline/hooks/organizations/queries.ts Outdated
Comment thread frontends/main/public/sample-seat-assignments.csv Outdated

@dsubak dsubak 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.

Looks pretty good to me - I was able to test everything listed and didn't find any issues.

I think we ought to sync up and make sure we're on the same page w/r/t CSV export (per Chris' comment thread), but once we've chased that to ground, I'd be happy to run this back through its paces and provide an approval!

@ChristopherChudzicki ChristopherChudzicki 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.

Working well! Thanks for changing the query client

One tiny request... https://github.com/mitodl/mit-learn/pull/3530/changes#r3499608315 IMO, no need for a re-review after client bump + that change.

Comment thread frontends/api/package.json Outdated
@daniellefrappier18 daniellefrappier18 merged commit bedd1dc into main Jul 1, 2026
13 checks passed
@daniellefrappier18 daniellefrappier18 deleted the daniellef/update-api-pagination branch July 1, 2026 14:04
@odlbot odlbot mentioned this pull request Jul 1, 2026
3 tasks
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.

4 participants