fix(phi): open the patient record when access justification is confirmed - #223
Merged
Merged
Conversation
Confirming the PHI access justification bounced the user back to the page they came from without ever opening the record. Radix's AlertDialogAction closes the dialog itself, so the resulting onOpenChange(false) ran the cancel path while the authorization IPC was still in flight. handleCancel settled the request as cancelled first, and because a promise resolves once, the grant that arrived moments later was discarded. Every route into a patient record was affected, including the Risk Intel aHHQ, Lab Currency, and Readiness Barriers tabs. The dialog now records which button caused the close so a confirm is not read as a cancellation, and the hook ignores a cancel while a confirm owns the outcome. Cancelling also emits exactly one onCancel instead of two, which had made PatientDetails call window.history.back() twice and skip a page. A denied request (insufficient role, IPC failure) now shows the reason with a way back rather than leaving the page waiting on a dialog that has already closed. The existing PatientDetails suite mocks both the dialog and the hook, so their interaction was never exercised and this passed CI. The new suite uses the real implementations and fails on the pre-fix code. No Epic/FHIR/HL7 or main-process code is touched. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here. |
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
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.
The bug
In the Risk Intel dashboard, clicking View next to a patient (aHHQ, Lab Currency, Readiness Barriers tabs) opens the access-justification prompt. Selecting a reason and clicking Confirm Access returned the user to the dashboard without ever opening the record.
Root cause
Radix's
AlertDialogActioncloses the dialog itself, so pressing Confirm Access firedonOpenChange(false)in addition to the submit handler. The dialog treated any close as a cancellation:<AlertDialog open={open} onOpenChange={(isOpen) => { if (!isOpen) onCancel(); }}>That ran the cancel path while the
access:authorizePhiAccessIPC was still in flight.handleCancelsettled the request as cancelled first, and because a promise resolves only once, the real grant arriving moments later was discarded — soaccessGrantedstayed false andPatientDetails'sonCancelhad already calledwindow.history.back().This affected every route into a patient record, not just the two tabs reported.
A second defect: Cancel invoked
onCanceltwice (its ownonClickplus the close event), sowindow.history.back()ran twice and skipped a page.Changes
JustificationDialogrecords which button caused the close, so a confirm is no longer read as a cancellation. All non-confirm closes (Cancel, Escape, overlay) route through a single cancel path, emitting exactly oneonCancel.useJustifiedAccessignores a cancellation while a confirm owns the outcome — defence in depth against the same race.PatientDetailsdistinguishes denied from cancelled and shows the reason with a way back, instead of waiting forever on a dialog that already closed. This is what aviewer/user/regulatorrole (nopatient:view_phi) would previously have hit.Why CI missed it
tests/components/PatientDetails.test.jsxmocks bothuseJustifiedAccessandJustificationDialog, so their interaction was never exercised.Testing
New
tests/components/JustificationAccessFlow.test.jsxdrives the real dialog and real hook through actual user events: confirm, slow IPC, custom text, empty justification, denial, thrown IPC error, Cancel, and Escape.Verified the suite genuinely reproduces the bug — against the pre-fix code 7 of 10 fail, including the double-cancel; all 10 pass after.
Epic Connection Hub
Not touched. The change is confined to three renderer files; no main-process, FHIR, HL7, or EHR code is modified.
Made with Cursor