feat(clavis): pagination for list ca and cert#958
Conversation
Signed-off-by: Vladislav Schur <u.shchur@sap.com>
📝 WalkthroughWalkthroughAdds 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. ChangesPCA Pagination (Full Stack)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
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
limitandnext_page_marker. - Updated
pcaRouter.listto 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.
Signed-off-by: Vladislav Schur <u.shchur@sap.com>
Signed-off-by: Vladislav Schur <u.shchur@sap.com>
Signed-off-by: Vladislav Schur <u.shchur@sap.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/aurora/src/server/Services/types/pca.ts (1)
108-111: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winInconsistent
next_page_markervalidation across schemas.In
CertificateAuthoritiesListInputSchema(Line 84)next_page_markeris defined asz.string().trim().min(1).optional(), but here it isz.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_markeris 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
📒 Files selected for processing (8)
packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.tsxpackages/aurora/src/server/Services/routers/pcaRouter.test.tspackages/aurora/src/server/Services/routers/pcaRouter.tspackages/aurora/src/server/Services/types/pca.test.tspackages/aurora/src/server/Services/types/pca.ts
There was a problem hiding this comment.
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 winKeep these negative CA-list assertions isolated.
These fixtures now omit the required
linksfield, so they can pass even if thecertificate_authoritiesvalidation regresses. Addlinks: []to each case so the tests still prove the missing/null/invalidcertificate_authoritiesbehavior.🤖 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 winKeep these negative certificate-list assertions isolated.
Same problem here: the cases fail as soon as
linksis missing, so they no longer specifically cover the missing/null/invalidcertificatespaths. Addinglinks: []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 winUse a contract-valid default query payload.
Line 49 still defaults
datato[], which no longer matchesCertificateAuthoritiesList. That makes the helper less trustworthy and can let array-shaped regressions slip through. Default this toundefinedor a realmakeListResponse([])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
📒 Files selected for processing (8)
packages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/PcaCertificatesListContainer.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.test.tsxpackages/aurora/src/client/routes/_auth/projects/$projectId/services/pca/-components/PcaListContainer.tsxpackages/aurora/src/server/Services/routers/pcaRouter.test.tspackages/aurora/src/server/Services/routers/pcaRouter.tspackages/aurora/src/server/Services/types/pca.test.tspackages/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
Summary
Implement server-side pagination and client-side table pagination in the UI for list of CA and Certificates issued by CA.
Changes Made
Testing Instructions
pnpm ipnpm run testChecklist
Summary by CodeRabbit
New Features
Bug Fixes
limitand cursor/marker navigation.Tests