LNK-4303: add event date - #1239
Conversation
* add EventDate field to PatientEvent * update PatientEventQueries for new field to simplify querying * update integration tests * update migration to set EventDate to payload value for existing records.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis PR adds an Changes
Sequence DiagramsequenceDiagram
participant Factory as PatientEventFactory
participant Payload as FHIR Payload
participant Event as PatientEvent
participant DB as Database
Factory->>Payload: Extract event type
alt Discharge Event
Factory->>Payload: Get DischargeDate
else Admit Event
Factory->>Payload: Get AdmitDate
else Other Event
Factory->>Factory: Use DateTime.UtcNow
end
Factory->>Event: Assign EventDate
Event->>DB: Persist with EventDate
DB->>DB: Index on EventDate
Note over Factory,DB: Query phase uses EventDate<br/>for filtering and sorting
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
DotNet/Census/Domain/Queries/PatientEncounterQueries.cs (1)
254-334: Process correlation groups in EventDate order.Inside the rebuild loop we still iterate
correlationGroup.OrderBy(e => e.ModifyDate). This defeats the earlier change to sort byEventDate: a discharge ingested later (ModifyDate newer) but representing an earlier clinical time will still execute before its admit event, so the discharge update is skipped and the encounter remains open. Please switch the per-correlation ordering toEventDateso the rebuild truly replays events chronologically.
🧹 Nitpick comments (1)
DotNet/ServiceTests/IntegrationTests/Census/QueryTests.cs (1)
291-294: Keep the discharge event and assert the new EventDate behaviorCommenting out the discharge event makes the dataset less realistic and stops us from exercising the branch where a patient’s latest event is a discharge. With the EventDate ordering now in play, we should leave that discharge record in place and update the expectations so the test proves we exclude patients once they’ve been discharged. Suggested change:
- // Within date range, discharge event, patient 1 (latest for patient 1) - //var dischargeCorrelationId1 = Guid.NewGuid().ToString(); - //var patient1DischargePayload = new FHIRListDischargePayload(patientId1, DateTime.UtcNow.AddDays(-1)); - //var patient1DischargeEvent = patient1DischargePayload.CreatePatientEvent(facilityId, dischargeCorrelationId1); - //events.Add(patient1DischargeEvent); + // Within date range, discharge event, patient 1 (latest for patient 1) + var dischargeCorrelationId1 = Guid.NewGuid().ToString(); + var patient1DischargePayload = new FHIRListDischargePayload(patientId1, DateTime.UtcNow.AddDays(-1)); + var patient1DischargeEvent = patient1DischargePayload.CreatePatientEvent(facilityId, dischargeCorrelationId1); + events.Add(patient1DischargeEvent); ... - // Should return 2 events (latest events for each patient within the date range) - Assert.Equal(2, results?.Count() ?? 0); - - // Verify both admitted patients are in the results - Assert.Contains(results, e => e.SourcePatientId == patientId1); - Assert.Contains(results, e => e.SourcePatientId == patientId2); + // Should return only the patient whose latest event in-range is still an admit + Assert.Single(results); + + Assert.DoesNotContain(results, e => e.SourcePatientId == patientId1); + Assert.Contains(results, e => e.SourcePatientId == patientId2);
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (10)
DotNet/Census/Application/Factories/PatientEventFactory.cs(2 hunks)DotNet/Census/Domain/Context/CensusContext.cs(2 hunks)DotNet/Census/Domain/Entities/POI/PatientEvent.cs(1 hunks)DotNet/Census/Domain/Managers/PatientEventManager.cs(0 hunks)DotNet/Census/Domain/Queries/PatientEncounterQueries.cs(3 hunks)DotNet/Census/Domain/Queries/PatientEventQueries.cs(5 hunks)DotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.Designer.cs(1 hunks)DotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.cs(1 hunks)DotNet/Census/Migrations/CensusContextModelSnapshot.cs(1 hunks)DotNet/ServiceTests/IntegrationTests/Census/QueryTests.cs(1 hunks)
💤 Files with no reviewable changes (1)
- DotNet/Census/Domain/Managers/PatientEventManager.cs
🧰 Additional context used
📓 Path-based instructions (2)
**/*.cs
⚙️ CodeRabbit configuration file
**/*.cs: TheHtmlInputSanitizerclass'sSanitize()andSanitizeAndRemove()methods should be used when dealing withstringquery parameters from REST requests.
Files:
DotNet/Census/Application/Factories/PatientEventFactory.csDotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.csDotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.Designer.csDotNet/Census/Domain/Context/CensusContext.csDotNet/ServiceTests/IntegrationTests/Census/QueryTests.csDotNet/Census/Domain/Entities/POI/PatientEvent.csDotNet/Census/Domain/Queries/PatientEventQueries.csDotNet/Census/Domain/Queries/PatientEncounterQueries.csDotNet/Census/Migrations/CensusContextModelSnapshot.cs
**
⚙️ CodeRabbit configuration file
**: Pull requests that have "TECH_DEBT" in the title should only contain changes related to typos, unused code, linter/IDE suggestions, swagger specification updates,
and logging improvements. These TECH_DEBT PRs must not affect core functionality. All PRs that are not considered technical debt must include information on what
testing was performed in the description of the PR. If it does not, ask the author to provide details on what testing was performed.
When reviewing code, suggest unit tests using XUnit in the following scenarios:
- If/Else or Switch/Case blocks are introduced or modified — ensure each branch has a corresponding unit test.
- Logic that depends on service or interface configuration — suggest tests to validate different implementations are correctly resolved.
- No network activity (HTTP calls, sockets, etc.) should appear in unit tests. Recommend using mocks (via Moq) for any external communication.
Large unit tests should be avoided; keeping unit tests small and focused on targeted business logic (i.e. string sanitization)
**: Pull requests that have DOCS in the title should only contain changes related to documentation within the /docs folder or in .md files through-out the code-base. The description
of the PR should specify what documentation was updated. Documentation updates should use EventCatalog.dev structure, where service-specific functionality should be described
in the service's index.mdx (i.e. /services/XXX/index.mdx or /domains/XXX/services/YYY/index.mdx). Configurations that are shared by multiple services should be
reflected in the /docs/docs/config files.
Files:
DotNet/Census/Application/Factories/PatientEventFactory.csDotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.csDotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.Designer.csDotNet/Census/Domain/Context/CensusContext.csDotNet/ServiceTests/IntegrationTests/Census/QueryTests.csDotNet/Census/Domain/Entities/POI/PatientEvent.csDotNet/Census/Domain/Queries/PatientEventQueries.csDotNet/Census/Domain/Queries/PatientEncounterQueries.csDotNet/Census/Migrations/CensusContextModelSnapshot.cs
🧠 Learnings (4)
📓 Common learnings
Learnt from: edward-miller-lcg
Repo: lantanagroup/link-cloud PR: 1095
File: Tests/BackendE2ETests/ApiRequests/SubmissionZipReader.cs:235-297
Timestamp: 2025-09-10T03:18:32.895Z
Learning: User edward-miller-lcg prefers minimal logging in test scenarios and considers frequent progress updates during polling loops to be "too noisy" for test output.
📚 Learning: 2025-01-22T17:06:33.481Z
Learnt from: edward-miller-lcg
Repo: lantanagroup/link-cloud PR: 615
File: DotNet/DataAcquisition.Domain/Migrations/20241204194556_Init.Designer.cs:202-247
Timestamp: 2025-01-22T17:06:33.481Z
Learning: Entity Framework Core migration designer files (*.Designer.cs) are auto-generated by the EF Core CLI and should never be manually edited. Changes to the database schema should be made by modifying the entity classes and creating new migrations.
Applied to files:
DotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.Designer.csDotNet/Census/Migrations/CensusContextModelSnapshot.cs
📚 Learning: 2025-01-22T17:07:51.015Z
Learnt from: edward-miller-lcg
Repo: lantanagroup/link-cloud PR: 615
File: DotNet/DataAcquisition.Domain/Migrations/20241204194556_Init.Designer.cs:59-88
Timestamp: 2025-01-22T17:07:51.015Z
Learning: Skip reviewing auto-generated Entity Framework Core migration files (*.Designer.cs) as they are generated automatically and should not be manually modified. Schema changes should be made in the entity model classes instead.
Applied to files:
DotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.Designer.csDotNet/Census/Migrations/CensusContextModelSnapshot.cs
📚 Learning: 2025-06-26T20:07:57.494Z
Learnt from: edward-miller-lcg
Repo: lantanagroup/link-cloud PR: 929
File: DotNet/DataAcquisitionTests/ServiceTests/PatientDataServiceTests.cs:337-454
Timestamp: 2025-06-26T20:07:57.494Z
Learning: In the `CreateLogEntries` method of `PatientDataService`, when `IQueryListProcessor.Process` throws a `ProduceException<string, ResourceAcquired>`, the exception is rethrown directly without being wrapped in a `TransientException`. The test should validate the original `ProduceException` being thrown.
Applied to files:
DotNet/ServiceTests/IntegrationTests/Census/QueryTests.cs
🧬 Code graph analysis (6)
DotNet/Census/Application/Factories/PatientEventFactory.cs (3)
DotNet/ServiceTests/IntegrationTests/Census/QueryTests.cs (1)
DateTime(345-354)DotNet/Census/Domain/Converters/PayloadJsonConverter.cs (1)
IPayload(24-88)DotNet/Census/Application/Models/Payloads/Fhir/List/FhirListAdmitPayload.cs (1)
FHIRListAdmitPayload(12-43)
DotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.cs (2)
DotNet/Census/Application/Factories/PatientEventFactory.cs (1)
DateTime(27-35)DotNet/ServiceTests/IntegrationTests/Census/QueryTests.cs (1)
DateTime(345-354)
DotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.Designer.cs (5)
DotNet/Census/Migrations/CensusContextModelSnapshot.cs (1)
DbContext(13-884)DotNet/Census/Domain/Context/CensusContext.cs (3)
CensusContext(11-153)CensusContext(19-21)CensusContext(23-25)DotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.cs (1)
AddEventDateToPatientEvent(9-71)DotNet/Census/Application/Factories/PatientEventFactory.cs (1)
DateTime(27-35)DotNet/ServiceTests/IntegrationTests/Census/QueryTests.cs (1)
DateTime(345-354)
DotNet/Census/Domain/Context/CensusContext.cs (1)
DotNet/Census/Application/Factories/PatientEventFactory.cs (1)
PatientEvent(10-25)
DotNet/Census/Domain/Entities/POI/PatientEvent.cs (2)
DotNet/Census/Application/Factories/PatientEventFactory.cs (1)
DateTime(27-35)DotNet/ServiceTests/IntegrationTests/Census/QueryTests.cs (1)
DateTime(345-354)
DotNet/Census/Migrations/CensusContextModelSnapshot.cs (2)
DotNet/Census/Migrations/20251106152644_AddEventDateToPatientEvent.Designer.cs (1)
DbContext(14-887)DotNet/Census/Domain/Context/CensusContext.cs (3)
CensusContext(11-153)CensusContext(19-21)CensusContext(23-25)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Unit Tests for Java
- GitHub Check: Unit Tests for DotNet
- GitHub Check: Smoke Test with Docker Compose
- GitHub Check: Integration Tests
- GitHub Check: Analyze (csharp)
- GitHub Check: Analyze (java-kotlin)
🔇 Additional comments (1)
DotNet/Census/Application/Factories/PatientEventFactory.cs (1)
27-34: Add targeted unit tests forGetEventDate.The new switch expression introduces multiple branches (admit, discharge, fallback). Per our test guidelines, please add focused XUnit coverage to prove each branch returns the expected date (including a non-FHIR payload/null case). This will guard against regressions if additional payload types appear later.
2 participants
🛠️ Description of Changes
Add EventDate column to PatientEvents to simplify querying when the need to evaluate an admit or discharge date. Update integration test for bad assertion. Update migration to backfill EventDates based on admit or discharge dates.
🧪 Testing Performed
Local testing and integration testing.
🧑🔬 Unit Testing
📓 Documentation Updated
n/a
Summary by CodeRabbit
New Features
Improvements
Tests