Controls > OS settings > Certificates: View certificates - #48460
Conversation
|
@coderabbitai full review |
✅ Action performedFull review finished. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48460 +/- ##
==========================================
+ Coverage 67.39% 67.44% +0.04%
==========================================
Files 3676 3699 +23
Lines 233631 233853 +222
Branches 12298 12354 +56
==========================================
+ Hits 157463 157712 +249
+ Misses 62058 62030 -28
- Partials 14110 14111 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
WalkthroughThis change adds a "View certificate" modal to the Controls > OS settings > Certificates page, allowing admins to inspect certificate details including subject name and subject alternative name. It introduces an "Add certificate authority" empty-state card shown when no custom SCEP CA is configured. The certificates list now uses an actions dropdown (View/Delete) instead of a standalone delete button, displays "Updated" instead of "Added" timestamps, and reorders the add-certificate form so the CA field precedes the name field. The Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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
`@frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/Certificates.tsx`:
- Around line 200-201: The Certificates row label is misleading because the UI
still uses created_at but now says “Updated”. In Certificates.tsx, keep the
existing formatting around formatDistanceToNow(new Date(created_at)) and change
the label back to “Added” unless ICertificate and the API are extended to
provide a real updated_at field. Use the Certificates component and the
created_at display logic to locate the change.
- Around line 163-173: The Certificates page is incorrectly using the CA query
state to block the entire view, which hides already-fetched certificates in the
main flow. Update the logic in Certificates so the CA result only affects the
empty-state path: keep rendering the existing certificate list whenever
certs.length is non-zero, and only consult hasCustomScepCA when there are no
certificates to decide whether to show AddCertAuthorityCard. Preserve the
Spinner and DataError handling for the certificates fetch, but do not let
isLoadingCAs or isErrorCAs replace the full page once certificates are
available.
In
`@frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/_styles.scss`:
- Around line 13-15: The stylesheet for ViewCertificateModal currently uses a
quoted custom font family in the textarea rule, which violates Stylelint’s
font-family-name-quotes rule. Update the font-family declaration in _styles.scss
to reference SourceCodePro without quotes while keeping the monospace fallback,
and leave the textarea selector and related styling unchanged.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5b778472-fe4b-4cc6-8e30-a331530f6e20
📒 Files selected for processing (14)
frontend/__mocks__/certificatesMock.tsfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/Certificates.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertAuthorityCard/AddCertAuthorityCard.tests.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertAuthorityCard/AddCertAuthorityCard.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertAuthorityCard/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertAuthorityCard/index.tsfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateCard/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/AddCertificateModal.tests.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/AddCertificateModal.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/ViewCertificateModal.tests.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/ViewCertificateModal.tsxfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/_styles.scssfrontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/index.tsfrontend/services/entities/certificates.ts
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
There was a problem hiding this comment.
Pull request overview
Adds UI support in Controls → OS settings → Certificates to view an existing certificate’s details in a modal, introduces an empty state when no custom SCEP CA is configured, and updates the certificates list row actions and add-certificate form layout. This fits into the MDM/OS Settings controls UI by improving certificate inspection and related setup flows without changing APIs.
Changes:
- Added a View certificate modal (with tests) to inspect certificate details (CA, added time, SN, optional SAN).
- Added a “no custom SCEP CA configured” empty state card that routes admins to Certificate Authorities settings.
- Switched certificate row actions to an Actions dropdown (View/Delete) and reordered the Add Certificate form to show CA selection first.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/services/entities/certificates.ts | Extends ICertificate to include subject_name for display in the view modal. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/Certificates.tsx | Adds CA presence check + empty state, Actions dropdown (View/Delete), and view modal wiring. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/ViewCertificateModal.tsx | Implements the certificate details modal UI. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/ViewCertificateModal.tests.tsx | Adds unit tests for view modal rendering and exit behavior. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/index.ts | Barrel export for the view modal component. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/ViewCertificateModal/_styles.scss | Styles view modal layout and textarea font. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/AddCertificateModal.tsx | Reorders fields so CA selection appears above the Name field. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateModal/AddCertificateModal.tests.tsx | Adds test coverage to enforce the new CA-before-Name layout. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertificateCard/_styles.scss | Adjusts empty-state heading font size. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertAuthorityCard/AddCertAuthorityCard.tsx | Adds new empty-state card prompting admins to add a certificate authority. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertAuthorityCard/AddCertAuthorityCard.tests.tsx | Tests the empty-state copy and routing behavior. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertAuthorityCard/index.ts | Barrel export for the CA empty-state card. |
| frontend/pages/ManageControlsPage/OSSettings/cards/Certificates/components/AddCertAuthorityCard/_styles.scss | Styles the CA empty-state card layout. |
| frontend/mocks/certificatesMock.ts | Updates certificate mock data to include subject_name. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
The CA list query only decides which empty state to show; render the certificate list whenever certs exist, regardless of CA load/error. Claude-Session: https://claude.ai/code/session_01J77VQSSuPptxpoZDrFQxdj
|
@mostlikelee Changes file? |
|
The timestamp label should say "Added", right? |
|
While testing this I found two bugs unrelated to your changes but that should be easy fixes, up to you if you want to patch them here or open new tickets:
2. The Add Certificate modal is including non-SCEP CAs:
|
This was an explicit change in the Figma |
- Use $small font-size token in Add cert cards instead of px-to-rem(16) - Replace duplicated flex column rules with @include vertical-modal-layout Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Xm8MLdcjgXMdLuB29KNP3K
Good catches. These are both pre-exisiting issues so i filed new bugs for them |
|
@juan-fdz-hawa great catches. I made some updates and filed some bugs. Ready for another review |
Related issue: Resolves #39346
Adds a View certificate modal to Controls > OS settings > Certificates so admins can inspect (and copy) an existing certificate's details. Also adds a "no custom SCEP CA configured" empty state, switches each row to an Actions dropdown (View/Delete), and moves the CA field to the top of the Add form. Frontend-only; no API changes.
Checklist for submitter
changes/.SELECT *is avoided, SQL injection is prevented, JS inline code is prevented, and untrusted data interpolated into shell scripts/commands is validated.Testing
https://claude.ai/code/session_01J77VQSSuPptxpoZDrFQxdj
Summary by CodeRabbit
New Features
Improvements