Skip to content

Add pagination to credentials and project credentials page#4672

Open
sharleenawinja wants to merge 5 commits into
OpenFn:mainfrom
sharleenawinja:credentials-pagination
Open

Add pagination to credentials and project credentials page#4672
sharleenawinja wants to merge 5 commits into
OpenFn:mainfrom
sharleenawinja:credentials-pagination

Conversation

@sharleenawinja
Copy link
Copy Markdown

Description

This PR adds pagination to the credentials, keychain credentials, and OAuth clients tables on the credentials list pages (10 items per page, with Previous/Next control). OAuth clients are now in a collapsible section, collapsed by default. Tables are ordered: credentials first, keychain credentials second, OAuth clients last.

Closes #4301

Validation steps

  1. Go to /credentials with more than 10 credentials — confirm the table paginates

  2. Confirm the OAuth Clients section is collapsed by default and the toggle shows/hides it.

  3. If there are more than 10 OAuth clients, expand the section and confirm pagination works.

  4. Verify the same behaviour on Project Settings → Credentials.

Additional notes for the reviewer

  1. (Is there anything else the reviewer should know or look out for?)

AI Usage

Please disclose whether you've used AI anywhere in this PR (it's cool, we just
want to know!):

  • I have used Claude Code
  • I have used another model
  • I have not used AI

You can read more details in our
Responsible AI Policy

Pre-submission checklist

  • I have performed an AI review of my code (we recommend using /review
    with Claude Code)
  • I have implemented and tested all related authorization policies.
    (e.g., :owner, :admin, :editor, :viewer)
  • I have updated the changelog.
  • I have ticked a box in "AI usage" in this PR

@sharleenawinja sharleenawinja marked this pull request as draft April 28, 2026 10:37
@github-project-automation github-project-automation Bot moved this to New Issues in Core Apr 28, 2026
@sharleenawinja sharleenawinja marked this pull request as ready for review April 28, 2026 10:39
@sharleenawinja
Copy link
Copy Markdown
Author

@midigofrank could you please review this PR when you have a moment? Thanks!

@midigofrank
Copy link
Copy Markdown
Collaborator

Hey @sharleenawinja , thank you. I will review this today

@josephjclark
Copy link
Copy Markdown
Collaborator

Hi @sharleenawinja, thank you for this! There are two tests failing in CI, I think just because some of the HTML structures have changed. Would you mind taking a look before Frank reviews this properly? You can see the errors here: https://app.circleci.com/pipelines/github/OpenFn/lightning/17326/workflows/2b9ed83e-0bac-4f14-9180-2958451d29ac/jobs/93782/tests

Copy link
Copy Markdown
Collaborator

@midigofrank midigofrank left a comment

Choose a reason for hiding this comment

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

Hey @sharleenawinja , thank you for picking this up, you've picked a tough one, pagination in a page with multiple lists is an unchartered territory in lightning.

The credentials page is gnarly, it has multiple lists and the lists are shared in both the credentials page and the project settings page.

I see that you've chosen to do the pagination in the UI, I think that's understandable given the complexity of the page. I'm just not sure if that's the direction we want to take given that every other place does pagination using db queries. @elias-ba what do you think about this approach?

I also noticed that you created a new pagination component, could you try and reuse the one in https://github.com/OpenFn/lightning/blob/main/lib/lightning_web/pagination.ex ?

Also, test this in both pages (project settings and credentials page)

@github-project-automation github-project-automation Bot moved this from New Issues to In review in Core May 4, 2026
@midigofrank midigofrank requested a review from elias-ba May 4, 2026 08:49
@sharleenawinja
Copy link
Copy Markdown
Author

@midigofrank Thanks for the context. I'll rework the pagination to use DB queries to stay consistent with the rest of the app, and I'll reuse the existing pagination component instead of the one I added. I'll update the PR once that's done.

@sharleenawinja sharleenawinja force-pushed the credentials-pagination branch from d0a4068 to a4ab2f0 Compare May 6, 2026 02:08
@sharleenawinja
Copy link
Copy Markdown
Author

@josephjclark @midigofrank @elias-ba I have switched to DB-level pagination and reused the existing pagination_bar component. Each table now paginates via URL params. For the pipeline, the elixir tests job is failing due to pre-existing flaky tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: In review

Development

Successfully merging this pull request may close these issues.

Add pagination to credentials and project credentials page

3 participants