Add intake locking, patient list, API client, security hardening, and CI fix#59
Conversation
Add server endpoints to support intake drafts and locking: GET /intake/patient/{id}/draft, PUT /intake/{id}, POST /intake/{id}/submit and POST /intake/{id}/lock. Add a patient listing endpoint for clinician workflows and new DTOs (UpdateIntakeRequest, PatientListItemResponse). Persist and respect IsLocked/IsSubmitted state across services and the mock implementation. Introduce a Blazor ClinicianPatientSelector component and wire an IntakeApiService to call the new endpoints; update the intake wizard UI/state to handle locked intakes and improve error handling. Add PTDocClaimTypes.ApiAccessToken and an ApiAccessTokenForwardingHandler to forward the API token from the authenticated web principal to outgoing ServerAPI HTTP requests, registering the handler with the HttpClient factory. Minor plumbing updates to create and propagate the API access token claim when signing in.
There was a problem hiding this comment.
Pull request overview
This PR introduces end-to-end intake draft locking/submission support, adds a clinician-oriented patient listing/selector flow in the UI, and forwards an API access token on ServerAPI HttpClient calls from PTDoc.Web to PTDoc.Api.
Changes:
- Added new Intake endpoints (draft-by-patient, update, submit, lock) and a clinician patient list endpoint + DTOs.
- Updated UI intake wizard state/flow to respect locked/submitted drafts, and introduced a clinician patient selector component.
- Implemented API access token claim + forwarding handler and wired it into PTDoc.Web’s
HttpClientFactory.
Reviewed changes
Copilot reviewed 13 out of 14 changed files in this pull request and generated 14 comments.
Show a summary per file
| File | Description |
|---|---|
| src/PTDoc.Web/Services/ApiAccessTokenForwardingHandler.cs | Adds a delegating handler to forward a bearer token on outbound ServerAPI calls. |
| src/PTDoc.Web/Program.cs | Registers the forwarding handler/HttpClient pipeline, switches intake service to API-backed implementation, and adds the API token claim on sign-in. |
| src/PTDoc.UI/Services/IntakeApiService.cs | Implements IIntakeService via new Intake API endpoints (create/update/submit, draft retrieval). |
| src/PTDoc.UI/Pages/Intake/IntakeWizardPage.razor | Updates intake wizard UX/state handling for locked/submitted drafts and adds clinician patient selection UI. |
| src/PTDoc.UI/Components/Intake/Models/IntakeWizardState.cs | Adds IsSubmitted and IsLocked wizard state flags. |
| src/PTDoc.UI/Components/Intake/ClinicianPatientSelector.razor | New component that fetches and filters a clinician patient list for intake selection. |
| src/PTDoc.Infrastructure/Services/MockIntakeService.cs | Updates mock intake behavior to respect lock/submission state. |
| src/PTDoc.Application/Services/IntakeResponseDraft.cs | Adds IsLocked to the draft contract. |
| src/PTDoc.Application/Identity/PTDocClaimTypes.cs | Adds ApiAccessToken claim type. |
| src/PTDoc.Application/DTOs/PatientDtos.cs | Adds PatientListItemResponse projection DTO for clinician selection UI. |
| src/PTDoc.Application/DTOs/IntakeDtos.cs | Adds UpdateIntakeRequest DTO for updating an existing intake draft. |
| src/PTDoc.Api/Patients/PatientEndpoints.cs | Adds GET /api/v1/patients list endpoint for clinician workflows. |
| src/PTDoc.Api/Intake/IntakeEndpoints.cs | Adds draft-by-patient, update, submit, and lock endpoints and maps lock/submission behavior. |
| public Guid Id { get; set; } | ||
| public string DisplayName { get; set; } = string.Empty; | ||
| public string? MedicalRecordNumber { get; set; } | ||
| public DateTime DateOfBirth { get; set; } |
There was a problem hiding this comment.
PatientListItemResponse includes DateOfBirth, but the clinician selector UI only needs an identifier + display label (and possibly MRN). Returning DOB in a “lightweight projection” increases PHI exposure unnecessarily; remove it unless a concrete UI need exists.
| public DateTime DateOfBirth { get; set; } |
| intake.PainMapData = string.IsNullOrWhiteSpace(request.PainMapData) ? "{}" : request.PainMapData; | ||
| intake.Consents = string.IsNullOrWhiteSpace(request.Consents) ? "{}" : request.Consents; | ||
| intake.ResponseJson = string.IsNullOrWhiteSpace(request.ResponseJson) ? "{}" : request.ResponseJson; | ||
| intake.TemplateVersion = string.IsNullOrWhiteSpace(request.TemplateVersion) ? "1.0" : request.TemplateVersion; | ||
| intake.LastModifiedUtc = DateTime.UtcNow; | ||
| intake.ModifiedByUserId = identityContext.GetCurrentUserId(); | ||
| intake.SyncState = SyncState.Pending; |
There was a problem hiding this comment.
UpdateIntakeRequest.TemplateVersion has [MaxLength(50)], but Minimal API endpoints don’t automatically run DataAnnotations validation. UpdateIntake also doesn’t enforce the length, so an overlong template version can lead to DB errors at save time. Add explicit validation (similar to CreatePatient) or an endpoint filter for DTO validation.
| group.MapGet("/patient/{patientId:guid}/draft", GetDraftByPatient) | ||
| .WithName("GetPatientDraftIntake") | ||
| .WithSummary("Get most recent intake draft for a patient") | ||
| .RequireAuthorization(AuthorizationPolicies.IntakeRead); | ||
|
|
||
| group.MapPut("/{id:guid}", UpdateIntake) | ||
| .WithName("UpdateIntake") | ||
| .WithSummary("Update an existing intake draft") | ||
| .RequireAuthorization(AuthorizationPolicies.IntakeWrite); | ||
|
|
||
| group.MapPost("/{id:guid}/submit", SubmitIntake) | ||
| .WithName("SubmitIntake") | ||
| .WithSummary("Submit and lock an intake response") | ||
| .RequireAuthorization(AuthorizationPolicies.IntakeWrite); | ||
|
|
||
| group.MapPost("/{id:guid}/lock", LockIntake) | ||
| .WithName("LockIntake") | ||
| .WithSummary("Lock an intake response") | ||
| .RequireAuthorization(AuthorizationPolicies.IntakeWrite); | ||
| } |
There was a problem hiding this comment.
New API behavior is being introduced (draft lookup by patient, update, submit, lock, and patient listing) without corresponding automated tests. Add integration tests covering authorization, tenant scoping, and lock/submit invariants (e.g., update returns 409 when locked; submit sets IsLocked/SubmittedAt; list endpoint returns only tenant-scoped patients).
| var createPatientRequest = new CreatePatientRequest | ||
| { | ||
| FirstName = firstName, | ||
| LastName = lastName, | ||
| DateOfBirth = state.DateOfBirth ?? new DateTime(1970, 1, 1), |
There was a problem hiding this comment.
CreateTemporaryPatientAndDraftIntakeAsync falls back to new DateTime(1970, 1, 1) when state.DateOfBirth is null. Since DOB is required by IntakeDemographicsValidationService and the patient API rejects default DOBs, this risks creating incorrect patient data if the caller bypasses validation. Prefer throwing/returning a validation failure when DateOfBirth is missing instead of inventing a date.
| var createPatientRequest = new CreatePatientRequest | |
| { | |
| FirstName = firstName, | |
| LastName = lastName, | |
| DateOfBirth = state.DateOfBirth ?? new DateTime(1970, 1, 1), | |
| if (state.DateOfBirth is null) | |
| { | |
| throw new ArgumentException("Date of birth is required to create a temporary patient.", nameof(state)); | |
| } | |
| var createPatientRequest = new CreatePatientRequest | |
| { | |
| FirstName = firstName, | |
| LastName = lastName, | |
| DateOfBirth = state.DateOfBirth.Value, |
| id="intake-patient-search" | ||
| type="search" | ||
| class="intake-patient-selector__search" | ||
| placeholder="Search by name, MRN, or email" |
There was a problem hiding this comment.
The search input placeholder says "Search by name, MRN, or email", but the component only filters by DisplayName and MedicalRecordNumber (no email is loaded or searched). Either include email in the list/search logic or adjust the placeholder text to match actual behavior.
| placeholder="Search by name, MRN, or email" | |
| placeholder="Search by name or MRN" |
| var updateRequest = new | ||
| { | ||
| painMapData = BuildPainMapJson(state), | ||
| consents = BuildConsentJson(state), | ||
| responseJson = JsonSerializer.Serialize(state, SerializerOptions), | ||
| templateVersion = "1.0" |
There was a problem hiding this comment.
SaveDraftAsync uses an anonymous object for the PUT body even though UpdateIntakeRequest was added to PTDoc.Application.DTOs. Using the shared DTO here would reduce drift risk (property names/defaults) and gives compile-time validation if the API contract changes.
| var updateRequest = new | |
| { | |
| painMapData = BuildPainMapJson(state), | |
| consents = BuildConsentJson(state), | |
| responseJson = JsonSerializer.Serialize(state, SerializerOptions), | |
| templateVersion = "1.0" | |
| var updateRequest = new UpdateIntakeRequest | |
| { | |
| PainMapData = BuildPainMapJson(state), | |
| Consents = BuildConsentJson(state), | |
| ResponseJson = JsonSerializer.Serialize(state, SerializerOptions), | |
| TemplateVersion = "1.0" |
| } | ||
|
|
||
| draft.PatientId = response.PatientId; | ||
| draft.IsSubmitted = response.SubmittedAt.HasValue; |
There was a problem hiding this comment.
ToDraft(IntakeResponse response) sets IsSubmitted but never maps the server-side lock state (response.Locked) into IntakeResponseDraft.IsLocked. This will cause locked (but not submitted) intakes to load as editable in the wizard until the API rejects updates with 409. Map draft.IsLocked from the response as well.
| draft.IsSubmitted = response.SubmittedAt.HasValue; | |
| draft.IsSubmitted = response.SubmittedAt.HasValue; | |
| draft.IsLocked = response.Locked; |
| return; | ||
| } | ||
|
|
||
| await SaveDraftAsync(state, cancellationToken); | ||
|
|
||
| var existing = await GetIntakeByPatientAsync(state.PatientId.Value, cancellationToken); | ||
| if (existing is null || existing.Locked) | ||
| { | ||
| return; | ||
| } | ||
|
|
There was a problem hiding this comment.
SubmitAsync returns without error when existing is null or already locked. The calling UI treats a successful return as “submitted” and locks the wizard, so this silent no-op can produce incorrect UI state. Consider throwing a specific exception or returning a result indicating the intake was not submitted (e.g., not found / already locked).
| return; | |
| } | |
| await SaveDraftAsync(state, cancellationToken); | |
| var existing = await GetIntakeByPatientAsync(state.PatientId.Value, cancellationToken); | |
| if (existing is null || existing.Locked) | |
| { | |
| return; | |
| } | |
| throw new InvalidOperationException("Cannot submit intake because PatientId is missing."); | |
| } | |
| await SaveDraftAsync(state, cancellationToken); | |
| var existing = await GetIntakeByPatientAsync(state.PatientId.Value, cancellationToken); | |
| if (existing is null) | |
| { | |
| throw new InvalidOperationException($"Cannot submit intake; no draft intake found for patient {state.PatientId.Value}."); | |
| } | |
| if (existing.Locked) | |
| { | |
| throw new InvalidOperationException($"Cannot submit intake; intake {existing.Id} is already locked."); | |
| } |
| <select | ||
| id="intake-patient-select" | ||
| class="intake-patient-selector__select" | ||
| value="@SelectedPatientIdString" | ||
| @onchange="HandleSelectionChangedAsync" | ||
| disabled="@Disabled"> | ||
| <option value="">Select a patient</option> | ||
| @foreach (var patient in FilteredPatients) | ||
| { | ||
| <option value="@patient.Id">@FormatPatientOption(patient)</option> | ||
| } | ||
| </select> |
There was a problem hiding this comment.
The <select> has no associated label or aria-label (the only <label> points at the search input). This makes the patient dropdown hard to use with screen readers. Add a label tied to intake-patient-select, or give the <select> an accessible name via aria-label/aria-labelledby.
| protected override async Task OnInitializedAsync() | ||
| { | ||
| try | ||
| { | ||
| var result = await HttpClient.GetFromJsonAsync<List<PatientListItem>>("/api/v1/patients?take=250"); | ||
| if (result is not null) |
There was a problem hiding this comment.
OnInitializedAsync always loads up to 250 patients and then filters client-side. Since the API supports query, this can be slow for large clinics and defeats server-side search. Consider calling /api/v1/patients?query=... with debouncing/paging rather than eagerly downloading a large list.
|
@copilot apply changes based on the comments in this thread |
…lity and server-side search Agent-Logs-Url: https://github.com/BlackHouseDeveloper/PTDoc/sessions/781d35c2-8188-415b-b2ef-6f20f4bc5d7b Co-authored-by: BlackHouseDeveloper <20420616+BlackHouseDeveloper@users.noreply.github.com>
Applied all changes from the review thread in commit d5c8992:
|
|
@copilot apply changes based on the comments in this thread |
All changes from the review thread were already applied in commit d5c8992. The codebase currently has all of the following in place:
|
|
@copilot it failed 12 checks. figure out if the changes were ci breaking or if they actually failed those tests the way it was intended. Obce the problems are found implement the resolution |
…e as internal - Remove unresolved merge conflict markers from PfptRoleComplianceTests.cs (line 196-200) — these caused all 12 CI check failures with CS8300 - Change SubmitIntake from private to internal in IntakeEndpoints.cs so the test project can call it directly via InternalsVisibleTo - Add Consents = hipaaAcknowledged:true to UC2_IntakeForm_LocksAfterSubmit test to satisfy the HIPAA validation added in the previous commit Agent-Logs-Url: https://github.com/BlackHouseDeveloper/PTDoc/sessions/c6222c87-da01-4586-9f0d-b2f42be8c849 Co-authored-by: BlackHouseDeveloper <20420616+BlackHouseDeveloper@users.noreply.github.com>
All 12 CI failures had the same root cause: an unresolved git merge conflict marker left in Fixed in commit e5a458a with three changes:
All 5 affected tests now pass locally. |
BlackHouseDeveloper
merged commit e9f4a7d
into
Backend-Integration-Completion
Pull Request
Summary
Introduce intake draft/locking support end-to-end (API + UI), clinician patient listing, and API token forwarding for ServerAPI calls. Includes security hardening, PHI reduction, accessibility improvements, correctness fixes applied from code review feedback, and a CI build fix for unresolved merge conflict markers.
Title
Add intake locking, clinician patient list, API access token forwarding, security hardening, and CI fix
Description
What Changed
GET /intake/patient/{id}/draft,PUT /intake/{id},POST /intake/{id}/submit,POST /intake/{id}/lockUpdateIntakeRequest,PatientListItemResponseIsLocked/IsSubmittedstate across services and the mock implementationClinicianPatientSelectorcomponentIntakeApiServiceto call the new endpoints and updated intake wizard UI/state (locked intakes, improved error handling)PTDocClaimTypes.ApiAccessTokenand anApiAccessTokenForwardingHandlerand registered it with the HttpClient factoryDateOfBirthfromPatientListItemResponseDTO and theListPatientsprojection — the clinician selector only requires ID, display name, and MRNGetDraftByPatientfix: Now filters to unlocked intakes only (WHERE !IsLocked) and orders byLastModifiedUtc DESC, ensuring the most recent editable draft is returnedUpdateIntakevalidation: Added explicit server-sideTemplateVersionmax-length (50) check, since Minimal API endpoints do not auto-apply DataAnnotationsSubmitIntakeHIPAA enforcement: Parses theConsentsJSON and requireshipaaAcknowledged: truebefore allowing submission — previously a client could submit with{"hipaaAcknowledged": false}and bypass this checkCreateTemporaryPatientAndDraftIntakeAsyncfix: ThrowsArgumentExceptionwhenDateOfBirthis null instead of silently defaulting to 1970-01-01SaveDraftAsyncfix: Switched from an anonymous object to the typedUpdateIntakeRequestDTO for compile-time contract safetySubmitAsyncfix: ThrowsInvalidOperationExceptionwhen the intake is not found or already locked, replacing the previous silent no-op that produced incorrect UI stateToDraftfix: MapsIsLockedfrom the API response so locked-but-not-submitted intakes load as read-only in the wizardClinicianPatientSelectoraccessibility: Search placeholder corrected to "Search by name or MRN";<select>element now hasaria-label="Select patient"for screen reader supportClinicianPatientSelectorperformance: Replaced eager 250-patient client-side load with server-side search using a 300ms debouncedSystem.Timers.Timer(Stop/Reset pattern, proper exception handling, patient list preserved on transient errors)ApiAccessTokenForwardingHandlerfix: Falls back toHttpContext.GetTokenAsync("access_token")(OIDC-saved token viaSaveTokens = true) when thePTDocClaimTypes.ApiAccessTokenclaim is absent, ensuring the Entra External ID flow also forwards the bearer tokenPfptRoleComplianceTests.cs(lines 196–200) that causedCS8300build errors across all 12 CI check jobsSubmitIntakevisibility fix: ChangedSubmitIntakehandler fromprivatetointernalinIntakeEndpoints.cssoPfptRoleComplianceTestscan invoke it directly via the existing[assembly: InternalsVisibleTo("PTDoc.Tests")]UC2_IntakeForm_LocksAfterSubmit_ViaRealHandlerto includeConsents = "{\"hipaaAcknowledged\":true}"so the test satisfies the HIPAA consent validation added to theSubmitIntakehandlerWhy
Clinicians need a stable workflow for working with intake drafts while preventing edits to locked or submitted intakes, and the ServerAPI must receive the authenticated user's API token. Code review identified PHI over-exposure, a HIPAA consent bypass, silent failure modes in the UI service layer, an accessibility gap in the patient selector, and a missing OIDC token fallback — all of which have been corrected. A subsequent merge introduced unresolved conflict markers that broke every CI job; those have been resolved along with the test-accessibility and test-correctness issues they exposed.
How
New ASP.NET endpoints + DTOs, token forwarding via delegating handler with OIDC fallback, server-side HIPAA consent JSON parsing, reduced PHI projection in the list endpoint, server-side debounced search in the Blazor selector component (reusable timer, error-safe async lambda), typed DTO usage throughout the UI service layer,
internalvisibility onSubmitIntakefor direct handler testing, and resolved merge conflict markers in the compliance test file.References
Type of Change
Impact Areas
Checklist
Core Requirements
[Unreleased]section in docs/CHANGELOG.mdCode Quality & Testing
dotnet format --verify-no-changes)roslynator analyze PTDoc.sln --severity-level info)Build Artifacts
Functional Testing
Platform-Specific Validation
Documentation & Communication
Testing Instructions
1. Setup
2. Functional Testing
Expected Behavior:
GET /api/v1/intake/patient/{id}/draftreturns only unlocked drafts ordered by most-recently-modifiedPUT /api/v1/intake/{id}withtemplateVersionlonger than 50 characters returns 400 ValidationProblemPOST /api/v1/intake/{id}/submitwith{"hipaaAcknowledged": false}returns 400 ValidationProblemPOST /api/v1/intake/{id}/submitwith{"hipaaAcknowledged": true}succeeds and setsIsLocked = trueGET /api/v1/patientslist response does not includedateOfBirth<select>dropdown is announced by screen readers viaaria-label="Select patient"ApiAccessTokenForwardingHandlerforwards the bearer token in both local-auth and Entra External ID flowsPatientIdthrows rather than silently succeedingCS8300merge conflict marker errors)3. Accessibility Testing (if UI changes)
4. Cross-Platform Testing (if applicable)
Verification Commands
Review Feedback
For Reviewers: Please comment on the following areas (check all that you verified):
Design & Usability (if UI changes)
Code Quality
Performance & Testing
Documentation
Additional Context
Security Summary: CodeQL scan reports no new alerts. PHI exposure reduced by removing
DateOfBirthfrom the patient list DTO and server projection. HIPAA acknowledgement is now enforced server-side by parsing consent JSON before submission is accepted. Silent failure modes inSubmitAsyncandCreateTemporaryPatientAndDraftIntakeAsyncreplaced with explicit exceptions to prevent incorrect UI state.CI Fix Summary: All 12 CI check failures were caused by unresolved git merge conflict markers (
<<<<<<< HEAD/=======/>>>>>>>) left intests/PTDoc.Tests/Security/PfptRoleComplianceTests.csafter the merge commit. The conflict was resolved by keeping thepublic voidsignature (correct for a synchronousAssert.Equal-only test). Additionally,SubmitIntakewas changed fromprivatetointernalto allow direct handler invocation from the test assembly, and theUC2_IntakeForm_LocksAfterSubmit_ViaRealHandlertest fixture was updated to includehipaaAcknowledged: truein its consents payload.Healthcare Context Reminder: PTDoc is a HIPAA-conscious application. Ensure all patient data handling includes:
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.