LEGLINK-620: AddSecretKeytoVendor-UI - #1778
Merged
Merged
Conversation
Captures the decisions behind associating a Key Vault secret with a vendor, so the reasoning survives outside a chat log: why the association is vendor-scoped rather than facility-scoped (Veradigm needs a different key generation algorithm than Epic and Cerner), and why the stored value is the Key Vault secret id rather than a JWKS kid, which LEGLINK-63's title conflates. Also records what this ticket deliberately cannot do. The update endpoint that persists a secret id is owned by neither LEGLINK-620 nor LEGLINK-743, so the UI is built against a single isolated service method. LEGLINK-63's audit-trail acceptance criterion is produced from backend managers onto Kafka and no UI change can satisfy it. Claude-Session: https://claude.ai/code/session_01STFKxmDaJenKVny6WUoWKo
Link signs the JWT that Data Acquisition presents to Epic and Cerner during client-credentials auth. LEGLINK-14 moved the PEM into Key Vault; this makes the association explicit and vendor-scoped, because Veradigm needs a different key generation algorithm and so cannot share a key. Add an edit path to Vendor Management. The dashboard grows a Secret ID column that reads "Not set" when empty and a per-row edit action; the form grows a JWT / Authentication panel holding the Key Vault Secret ID, expanded when a vendor already has one so existing configuration is visible without hunting. An emptied box travels as undefined rather than "", so clearing the association reads as absent rather than set-to-empty. No update endpoint exists yet. The Vendor model is moving out of Normalization into Tenant under LEGLINK-743, whose acceptance criteria cover list, add and delete but not update, so this operation is owned by neither ticket. VendorService.updateVendor is the single place that knows the route: when the contract lands, that method is the only edit required. A config-flagged dual path was considered and rejected as permanent complexity bought against a decision expected within days. Two defects in files this already touches: createVendor was typed as IVendorConfigModel while callers pass a name string, which interpolated "[object Object]" into the URL for anything else; and getVendors never cleared its loading flag on success. Deferred: the mocked Playwright spec the design calls for. That harness arrives with PR #1773, which is not yet merged into dev, so there is nowhere on this branch for the spec to live. LEGLINK-63's audit-trail criterion is also outstanding -- audit events are produced from backend managers onto Kafka, so it belongs with the update endpoint rather than here. 16 unit specs pass; the app builds clean. Claude-Session: https://claude.ai/code/session_01STFKxmDaJenKVny6WUoWKo
…itly Two defects found reviewing 4b894e1 against the design. ErrorHandlingService raises its own toastr before rethrowing, so once the save paths began emitting failure to the dialog -- which shows a snackbar and stays open so the admin's input survives -- one failed save reported itself twice, toastr bottom-full-width and snackbar top-right. Saves now route through handleSaveError, which suppresses the toastr and leaves the dialog as the single surface. List and delete keep theirs, having no dialog to carry the news. The rethrown error still carries the sanitized message either way. Clearing a key sent secretId as undefined, which JSON.stringify drops, so the field never reached the wire. An absent field reads as "leave unchanged" to any endpoint with partial-update semantics, which would have made clearing an association succeed visibly and do nothing. It now travels as an explicit null, and the design's open items record that the backend must honour null as "remove the association" when the contract is settled. Adds vendor.service.spec.ts, the service having had no direct coverage: the update route and body, a cleared key surviving serialization, name escaping in the create route, and the toastr suppressed for saves but kept for list and delete. Both new behaviours fail against the previous code -- args[1] was absent rather than false, and secretId was undefined rather than null. 21 unit specs pass; the app builds clean. Claude-Session: https://claude.ai/code/session_01CX9BzMrPhTzGSakXYDaAVa
Contributor
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe vendor UI now supports optional Key Vault secret IDs. Users can view IDs, edit them through an expandable form section, create or update vendor records, clear associations with ChangesVendor secret ID management
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VendorDashboardComponent
participant VendorConfigDialogComponent
participant VendorConfigFormComponent
participant VendorService
VendorDashboardComponent->>VendorConfigDialogComponent: open edit dialog with vendor
VendorConfigDialogComponent->>VendorConfigFormComponent: initialize edit form
VendorConfigFormComponent->>VendorService: submit updated vendor with secretId
VendorService-->>VendorConfigFormComponent: return save response or error
VendorConfigFormComponent-->>VendorConfigDialogComponent: emit save result
VendorConfigDialogComponent-->>VendorDashboardComponent: close after successful save
VendorDashboardComponent->>VendorDashboardComponent: refresh vendors and show notification
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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
`@Web/Admin.UI/src/app/components/vendor/vendor-config-form/vendor-config-form.component.spec.ts`:
- Around line 130-139: Add a focused unit test alongside the existing
Create-mode test that mocks vendorService.createVendor to return throwError with
a representative message, then submit the configured name and assert
submittedConfiguration emits { success: false, message }. Also assert
vendorService.updateVendor is not called, keeping the test fully mocked without
network activity.
In `@Web/Admin.UI/src/app/services/gateway/vendor/vendor.service.ts`:
- Around line 42-59: The updateVendor method currently targets a nonexistent
backend PUT route. Disable the vendor editing flow that calls updateVendor in
vendor-config-form.component.ts, or gate it behind the established feature-flag
mechanism, until the backend update contract—including secretId: null
clearing—is available; do not expose the current HTTP PUT operation.
🪄 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 Plus
Run ID: 8135fbec-b2ec-4993-a58a-852765fab572
📒 Files selected for processing (11)
Web/Admin.UI/src/app/components/vendor/vendor-config-form/vendor-config-form.component.htmlWeb/Admin.UI/src/app/components/vendor/vendor-config-form/vendor-config-form.component.spec.tsWeb/Admin.UI/src/app/components/vendor/vendor-config-form/vendor-config-form.component.tsWeb/Admin.UI/src/app/components/vendor/vendor-dashboard/vendor-dashboard.component.htmlWeb/Admin.UI/src/app/components/vendor/vendor-dashboard/vendor-dashboard.component.scssWeb/Admin.UI/src/app/components/vendor/vendor-dashboard/vendor-dashboard.component.spec.tsWeb/Admin.UI/src/app/components/vendor/vendor-dashboard/vendor-dashboard.component.tsWeb/Admin.UI/src/app/interfaces/vendor/vendor-config-model.interface.tsWeb/Admin.UI/src/app/services/gateway/vendor/vendor.service.spec.tsWeb/Admin.UI/src/app/services/gateway/vendor/vendor.service.tsdocs/superpowers/specs/2026-08-03-vendor-signing-key-secret-id-design.md
A failed createVendor had no test. Assert it emits a single failure with the error message and does not fall through into the update path. Claude-Session: https://claude.ai/code/session_01TDPYGwkQDCXConLTuyU4is
arianamihailescu
changed the title
VendorController exposes list, add and delete only -- no PUT -- so the edit dialog added on this branch would save into a 404. Put the edit button and onEdit behind a vendorEditEnabled config flag, shipped off, following the existing AppConfig boolean pattern. Flip it once the update contract, including clearing secretId with null, is confirmed. Claude-Session: https://claude.ai/code/session_01TDPYGwkQDCXConLTuyU4is
MikeAtPinnacle
approved these changes
Aug 4, 2026
This was referenced Aug 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🛠️ Description of Changes
Added secret-key to Vendor screen
🧪 Testing Performed
Tested locally
🧑🔬 Unit Testing
📓 Documentation Updated
Please update any relevant sections in the project documentation that were impacted by the changes in the PR.
Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation