feat(ui): clavis pca certitificate get-by-id#844
Conversation
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>
|
Warning Review limit reached
More reviews will be available in 42 minutes and 9 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR adds a new certificate details page for PCA certificates, including backend service/schema updates, TanStack Router registration, a React route component with data fetching and UI rendering, table row navigation wiring, and localization strings for English and German languages. ChangesCertificate Details Page and Navigation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
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.
Actionable comments posted: 3
🧹 Nitpick comments (1)
apps/aurora-portal/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/-table/PcaCertificatesTableRow.test.tsx (1)
60-64: ⚡ Quick winStrengthen navigation assertion to verify
certificateIdmapping.Current check only validates that
paramsis a function. Assert the function result to catch regressions in route param construction.✅ Suggested test improvement
expect(mockNavigate).toHaveBeenCalledWith({ from: "/projects/$projectId/services/pca/$pcaId/", to: "$certificateId", params: expect.any(Function), }) + + const navArg = mockNavigate.mock.calls[0][0] + expect(navArg.params({ projectId: "project-1", pcaId: "ca-1" })).toEqual({ + projectId: "project-1", + pcaId: "ca-1", + certificateId: "cert-123", + }) })🤖 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 `@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/services/pca/$pcaId/-components/-table/PcaCertificatesTableRow.test.tsx around lines 60 - 64, The test currently asserts mockNavigate was called with params: expect.any(Function) but doesn't verify the function returns the correct route params; update the assertion to call the captured params function and assert its return maps the certificate id correctly. Locate the expectation around mockNavigate (the call using expect(mockNavigate).toHaveBeenCalledWith({...})) and replace the generic params check by extracting the params function (from the matched call) and invoking it with the same input used in the component, then assert the returned object contains the expected certificateId value (e.g., returnedParams.certificateId === "$certificateId") to ensure correct route param construction.
🤖 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
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/services/pca/$pcaId/-components/-table/PcaCertificatesTableRow.tsx:
- Around line 28-32: The DataGridRow currently only uses onClick (DataGridRow
with key={certificate.id} and onClick={navigateToCertificateDetailsPage}),
making navigation mouse-only; update the row to be keyboard-accessible by either
moving the navigation into a real interactive element inside a cell (preferred)
or adding tabIndex={0}, role="button", and an onKeyDown handler on DataGridRow
that calls navigateToCertificateDetailsPage when Enter or Space is pressed (use
certificate.id in the test-id/data attributes to keep references consistent).
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/services/pca/$pcaId/$certificateId.tsx:
- Line 40: RouteComponent is calling setPageTitle(...) directly during render
(both in the isLoading branch and after certificate is available); move those
calls into a useEffect hook so you don't cause render-phase side effects.
Specifically, remove direct calls to setPageTitle from the render body of
RouteComponent and add a useEffect that depends on [isLoading, certificate]
which sets the title to "Loading..." when isLoading is true and sets the
certificate title when certificate is non-null; reference the existing
setPageTitle function and the isLoading and certificate variables inside that
effect.
- Around line 49-53: Move the Route.useParams() call out of the click handler:
in the RouteComponent body call Route.useParams() once (e.g., const params =
Route.useParams()) and use params.projectId and params.pcaId when building the
navigate call inside handleBack (which currently calls navigate with to and
params). Update handleBack to reference the precomputed params instead of
calling Route.useParams() inline so params are reused and not re-evaluated on
each click.
---
Nitpick comments:
In
`@apps/aurora-portal/src/client/routes/_auth/projects/`$projectId/services/pca/$pcaId/-components/-table/PcaCertificatesTableRow.test.tsx:
- Around line 60-64: The test currently asserts mockNavigate was called with
params: expect.any(Function) but doesn't verify the function returns the correct
route params; update the assertion to call the captured params function and
assert its return maps the certificate id correctly. Locate the expectation
around mockNavigate (the call using
expect(mockNavigate).toHaveBeenCalledWith({...})) and replace the generic params
check by extracting the params function (from the matched call) and invoking it
with the same input used in the component, then assert the returned object
contains the expected certificateId value (e.g., returnedParams.certificateId
=== "$certificateId") to ensure correct route param construction.
🪄 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: b6c8dc7b-b8f3-4b3d-aba6-a5fde0e61859
📒 Files selected for processing (11)
apps/aurora-portal/src/client/routeTree.gen.tsapps/aurora-portal/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/$certificateId.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/$certificateId.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/-table/PcaCertificatesTableRow.test.tsxapps/aurora-portal/src/client/routes/_auth/projects/$projectId/services/pca/$pcaId/-components/-table/PcaCertificatesTableRow.tsxapps/aurora-portal/src/locales/de/messages.poapps/aurora-portal/src/locales/de/messages.tsapps/aurora-portal/src/locales/en/messages.poapps/aurora-portal/src/locales/en/messages.tsapps/aurora-portal/src/server/Services/routers/pcaRouter.test.tsapps/aurora-portal/src/server/Services/routers/pcaRouter.ts
Summary
Implemented details page with all states, connected bff endpoint, showing certificate-information(csr, project/pca/certificate ids, subject etc)
Changes Made
Related Issues
Testing Instructions
pnpm ipnpm run testChecklist
Summary by CodeRabbit