Skip to content

feat(clavis): pagination for list ca and cert#958

Merged
vlad-schur-external-sap merged 7 commits into
mainfrom
vlad-clavis-pagination
Jun 24, 2026
Merged

feat(clavis): pagination for list ca and cert#958
vlad-schur-external-sap merged 7 commits into
mainfrom
vlad-clavis-pagination

Conversation

@vlad-schur-external-sap

@vlad-schur-external-sap vlad-schur-external-sap commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Implement server-side pagination and client-side table pagination in the UI for list of CA and Certificates issued by CA.

Changes Made

  • Updated pca schemas with new pagination fields
  • Added new inputs for pcaRouters list/list-certs procedures
  • Implemented pagination for the list view with Pagination component

Testing Instructions

  1. pnpm i
  2. pnpm run test

Checklist

  • I have performed a self-review of my code.
  • I have commented my code, particularly in hard-to-understand areas.
  • I have added tests that prove my fix is effective or that my feature works.
  • New and existing unit tests pass locally with my changes.
  • I have made corresponding changes to the documentation (if applicable).
  • My changes generate no new warnings or errors.

Summary by CodeRabbit

  • New Features

    • Added server-backed pagination for PCA and certificate lists, loading results in batches and showing Next/Previous controls when more pages are available.
  • Bug Fixes

    • Updated list APIs to return full paginated responses (including pagination metadata) and to properly honor limit and cursor/marker navigation.
    • Improved validation and consistent handling of project-scoped request inputs.
  • Tests

    • Expanded pagination test coverage for both list containers and router/type validation.

Signed-off-by: Vladislav Schur <u.shchur@sap.com>
@coderabbitai

coderabbitai Bot commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds pagination support to PCA list and certificate list flows on both server and client sides. New and updated schemas carry pagination fields and response metadata, routers forward pagination query parameters, and both list containers render one page at a time with pagination controls.

Changes

PCA Pagination (Full Stack)

Layer / File(s) Summary
Pagination schemas and validation
packages/aurora/src/server/Services/types/pca.ts, packages/aurora/src/server/Services/types/pca.test.ts
Adds project-scoped list input schemas with limit and next_page_marker, extends list output schemas with links and next_page_marker, and updates validation coverage for project scoping, pagination bounds, and links shape.
Router pagination forwarding
packages/aurora/src/server/Services/routers/pcaRouter.ts, packages/aurora/src/server/Services/routers/pcaRouter.test.ts
Updates PCA list procedures to accept pagination input, forward query parameters to Clavis, and return full list response objects. Tests assert the forwarded URLs and updated response shapes.
PCA list pagination UI
packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.tsx, .../PcaListContainer.test.tsx
Adds paginated query state and a conditional Pagination control to the PCA list view, with tests covering empty state, page rendering, and next-page navigation.
Certificate list pagination UI
packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.tsx, .../PcaCertificatesListContainer.test.tsx
Adds the same paginated query flow and Pagination control to the certificates list view, with tests covering page boundaries, next-page navigation, and updated query arguments.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested labels

aurora-portal

Suggested reviewers

  • mark-karnaukh-extern-sap
  • andypf
  • KirylSAP
  • TilmanHaupt

Poem

🐇 I hopped through pages, one by one,
Fifty at a time, and then next one!
Markers, lists, and buttons too,
Rabbit-approved — a tidy view.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title is concise and accurately reflects the main change: adding pagination for CA and certificate lists.
Description check ✅ Passed The description covers the summary, changes made, testing instructions, and checklist, though Related Issues and Screenshots sections are not filled out.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch vlad-clavis-pagination

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@vlad-schur-external-sap vlad-schur-external-sap changed the title feat(clavis): pagination for list-ca feat(clavis): pagination for list ca and cert Jun 23, 2026

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

Adds pagination-related support for Private Certificate Authority (Clavis/PCA) CA listing, including server-side query param forwarding and client-side table pagination in the UI.

Changes:

  • Added a Zod input schema for listing CAs with optional limit and next_page_marker.
  • Updated pcaRouter.list to accept the new input and forward pagination query parameters to the OpenStack PCA service.
  • Implemented client-side pagination (50 items/page) for the PCA list view and added UI/tests.

Reviewed changes

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

Show a summary per file
File Description
packages/aurora/src/server/Services/types/pca.ts Adds CA list input schema with pagination fields.
packages/aurora/src/server/Services/types/pca.test.ts Adds validation tests for the new CA list input schema.
packages/aurora/src/server/Services/routers/pcaRouter.ts Forwards pagination query parameters when listing CAs.
packages/aurora/src/server/Services/routers/pcaRouter.test.ts Extends router tests to cover list URL with query params.
packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.tsx Adds client-side pagination + pagination controls for CA table.
packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.test.tsx Adds UI tests for pagination rendering and page navigation.

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

Comment thread packages/aurora/src/server/Services/routers/pcaRouter.ts Outdated
Comment thread packages/aurora/src/server/Services/types/pca.ts Outdated
Signed-off-by: Vladislav Schur <u.shchur@sap.com>

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 8 out of 8 changed files in this pull request and generated 6 comments.

Comment thread packages/aurora/src/server/Services/routers/pcaRouter.ts Outdated
Comment thread packages/aurora/src/server/Services/routers/pcaRouter.ts Outdated
Comment thread packages/aurora/src/server/Services/types/pca.ts Outdated
Comment thread packages/aurora/src/server/Services/types/pca.ts
@vlad-schur-external-sap vlad-schur-external-sap marked this pull request as draft June 23, 2026 08:55
Signed-off-by: Vladislav Schur <u.shchur@sap.com>

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 8 out of 8 changed files in this pull request and generated 7 comments.

Comment thread packages/aurora/src/server/Services/types/pca.ts Outdated
Comment thread packages/aurora/src/server/Services/types/pca.test.ts
Comment thread packages/aurora/src/server/Services/types/pca.test.ts
Comment thread packages/aurora/src/server/Services/routers/pcaRouter.ts
Comment thread packages/aurora/src/server/Services/routers/pcaRouter.ts
@vlad-schur-external-sap vlad-schur-external-sap marked this pull request as ready for review June 24, 2026 07:51
Signed-off-by: Vladislav Schur <u.shchur@sap.com>

@coderabbitai coderabbitai Bot 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.

🧹 Nitpick comments (1)
packages/aurora/src/server/Services/types/pca.ts (1)

108-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Inconsistent next_page_marker validation across schemas.

In CertificateAuthoritiesListInputSchema (Line 84) next_page_marker is defined as z.string().trim().min(1).optional(), but here it is z.string().min(1).optional() without .trim(). Both fields feed the same upstream query parameter, so align them for consistent input handling.

Note: since next_page_marker is an opaque continuation token, consider whether trimming is appropriate at all (it could in theory mutate a token). If trimming is intentional for whitespace hygiene, apply it to both; otherwise drop it from both.

♻️ Align with the list schema (trim both)
 export const CertificateAuthorityCertificatesListInputSchema = CertificateAuthorityIdInputSchema.extend({
   limit: z.number().int().min(1).max(1000).optional(),
-  next_page_marker: z.string().min(1).optional(),
+  next_page_marker: z.string().trim().min(1).optional(),
 })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/aurora/src/server/Services/types/pca.ts` around lines 108 - 111, The
`next_page_marker` validation is inconsistent between
`CertificateAuthorityCertificatesListInputSchema` and
`CertificateAuthoritiesListInputSchema`, so align the handling in the `pca.ts`
schemas. Update the `CertificateAuthorityCertificatesListInputSchema` definition
to match the list schema’s `next_page_marker` behavior, or remove trimming from
both if the token should remain opaque; keep the validation consistent across
both schema constants.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/aurora/src/server/Services/types/pca.ts`:
- Around line 108-111: The `next_page_marker` validation is inconsistent between
`CertificateAuthorityCertificatesListInputSchema` and
`CertificateAuthoritiesListInputSchema`, so align the handling in the `pca.ts`
schemas. Update the `CertificateAuthorityCertificatesListInputSchema` definition
to match the list schema’s `next_page_marker` behavior, or remove trimming from
both if the token should remain opaque; keep the validation consistent across
both schema constants.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a83a3ee2-d576-42ef-8e88-778e80195744

📥 Commits

Reviewing files that changed from the base of the PR and between 04834f0 and aa2d2d9.

📒 Files selected for processing (8)
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.test.tsx
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.tsx
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.test.tsx
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.tsx
  • packages/aurora/src/server/Services/routers/pcaRouter.test.ts
  • packages/aurora/src/server/Services/routers/pcaRouter.ts
  • packages/aurora/src/server/Services/types/pca.test.ts
  • packages/aurora/src/server/Services/types/pca.ts

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 8 out of 8 changed files in this pull request and generated 12 comments.

Comment thread packages/aurora/src/server/Services/types/pca.ts
Comment thread packages/aurora/src/server/Services/types/pca.ts
Comment thread packages/aurora/src/server/Services/types/pca.ts
Comment thread packages/aurora/src/server/Services/types/pca.test.ts
Comment thread packages/aurora/src/server/Services/types/pca.test.ts

@coderabbitai coderabbitai Bot 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.

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
packages/aurora/src/server/Services/types/pca.test.ts (2)

668-685: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep these negative CA-list assertions isolated.

These fixtures now omit the required links field, so they can pass even if the certificate_authorities validation regresses. Add links: [] to each case so the tests still prove the missing/null/invalid certificate_authorities behavior.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/aurora/src/server/Services/types/pca.test.ts` around lines 668 -
685, The negative CertificateAuthoritiesListSchema cases are missing the
required links field, so they may pass for the wrong reason. Update the three
tests in pca.test.ts to include links: [] in each fixture while keeping the
existing missing, null, and invalid certificate_authorities assertions. This
keeps the coverage focused on CertificateAuthoritiesListSchema and its
certificate_authorities validation instead of unrelated schema requirements.

1321-1338: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Keep these negative certificate-list assertions isolated.

Same problem here: the cases fail as soon as links is missing, so they no longer specifically cover the missing/null/invalid certificates paths. Adding links: [] preserves the intended coverage.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/aurora/src/server/Services/types/pca.test.ts` around lines 1321 -
1338, The certificate-list negative tests are being blocked by a missing
required links field, so they no longer isolate the intended certificates
validation paths. Update the affected cases in CertificatesListSchema safeParse
assertions to include links: [] while keeping the invalid certificates inputs
unchanged, so each test specifically exercises the missing/null/invalid
certificates behavior.
packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.test.tsx (1)

47-55: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Use a contract-valid default query payload.

Line 49 still defaults data to [], which no longer matches CertificateAuthoritiesList. That makes the helper less trustworthy and can let array-shaped regressions slip through. Default this to undefined or a real makeListResponse([]) payload instead.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/services/pca/-components/PcaListContainer.test.tsx
around lines 47 - 55, The test helper `createMockListQueryResult` is using an
invalid default for `data` by setting it to an empty array, which no longer
matches `CertificateAuthoritiesList`. Update the default payload in this helper
to use `undefined` or a contract-valid empty response from
`makeListResponse([])`, while keeping the override merge behavior intact. Make
sure the returned shape still satisfies `trpcReact.services.pca.list.useQuery`
so the `PcaListContainer` tests reflect the real query contract.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/services/pca/-components/PcaListContainer.tsx:
- Around line 29-55: Reset the pagination state in PcaListContainer when
projectId changes: currentPage and pageMarkers should be reinitialized so a new
project never reuses the previous project’s cursor. Update the PcaListContainer
component to watch projectId and clear pageMarkers back to the initial undefined
marker and set currentPage back to 1 before the
trpcReact.services.pca.list.useQuery call uses currentMarker.

---

Outside diff comments:
In
`@packages/aurora/src/client/routes/_auth/projects/`$projectId/services/pca/-components/PcaListContainer.test.tsx:
- Around line 47-55: The test helper `createMockListQueryResult` is using an
invalid default for `data` by setting it to an empty array, which no longer
matches `CertificateAuthoritiesList`. Update the default payload in this helper
to use `undefined` or a contract-valid empty response from
`makeListResponse([])`, while keeping the override merge behavior intact. Make
sure the returned shape still satisfies `trpcReact.services.pca.list.useQuery`
so the `PcaListContainer` tests reflect the real query contract.

In `@packages/aurora/src/server/Services/types/pca.test.ts`:
- Around line 668-685: The negative CertificateAuthoritiesListSchema cases are
missing the required links field, so they may pass for the wrong reason. Update
the three tests in pca.test.ts to include links: [] in each fixture while
keeping the existing missing, null, and invalid certificate_authorities
assertions. This keeps the coverage focused on CertificateAuthoritiesListSchema
and its certificate_authorities validation instead of unrelated schema
requirements.
- Around line 1321-1338: The certificate-list negative tests are being blocked
by a missing required links field, so they no longer isolate the intended
certificates validation paths. Update the affected cases in
CertificatesListSchema safeParse assertions to include links: [] while keeping
the invalid certificates inputs unchanged, so each test specifically exercises
the missing/null/invalid certificates behavior.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: bae8c24f-ac71-4676-8d13-c423445e20f7

📥 Commits

Reviewing files that changed from the base of the PR and between aa2d2d9 and b8a8781.

📒 Files selected for processing (8)
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.test.tsx
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.tsx
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.test.tsx
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.tsx
  • packages/aurora/src/server/Services/routers/pcaRouter.test.ts
  • packages/aurora/src/server/Services/routers/pcaRouter.ts
  • packages/aurora/src/server/Services/types/pca.test.ts
  • packages/aurora/src/server/Services/types/pca.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • packages/aurora/src/server/Services/types/pca.ts
  • packages/aurora/src/server/Services/routers/pcaRouter.ts
  • packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.tsx

@vlad-schur-external-sap vlad-schur-external-sap enabled auto-merge (squash) June 24, 2026 08:27
@vlad-schur-external-sap vlad-schur-external-sap merged commit ecbdba5 into main Jun 24, 2026
22 checks passed
@vlad-schur-external-sap vlad-schur-external-sap deleted the vlad-clavis-pagination branch June 24, 2026 08:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

clavis enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants