Skip to content

LNK-4455: Fhir List Configuration Changes for POI - #1245

Merged
arianamihailescu merged 2 commits into
devfrom
LNK-4455-FhirListChangesForPOI
Nov 7, 2025
Merged

LNK-4455: Fhir List Configuration Changes for POI#1245
arianamihailescu merged 2 commits into
devfrom
LNK-4455-FhirListChangesForPOI

Conversation

@arianamihailescu

@arianamihailescu arianamihailescu commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

🛠️ Description of Changes

Fhir List Configuration Changes for POI

🧪 Testing Performed

Tested locally

🧑‍🔬 Unit Testing

  • I have written or updated unit tests to cover my changes

📓 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

    • Added FHIR ID field to patient list configuration with read-only mode support and error validation.
  • Refactor

    • Streamlined patient list form with compact status and time frame display.
    • Updated data structure to consolidate patient information using status, time frame, and FHIR ID.
    • Adjusted dialog size for improved visibility and usability.

@coderabbitai

coderabbitai Bot commented Nov 7, 2025

Copy link
Copy Markdown
Contributor

Walkthrough

The PR refactors the FHIR List configuration form to support a new POI data structure, replacing measure ID and list ID fields with status, time frame, and FHIR ID. The component logic, template, styles, and interface are updated to populate and manage a predefined matrix of status and time frame combinations with associated FHIR identifiers.

Changes

Cohort / File(s) Summary
Template and Styling
data-acquisition-fhir-list-config-form.component.html, data-acquisition-fhir-list-config-form.component.scss
Replaced per-patient form block with a table-like layout featuring a static header row (Status, Time Frame, FhirId) and compact value rows. Added FhirId input field with readonly binding and inline error handling. Removed Measure IDs and List IDs controls. New CSS classes and styling for patient container, rows, and columns.
Component Logic
data-acquisition-fhir-list-config-form.component.ts
Removed MeasureDefinitionService dependency. Refactored form initialization to use new createPatientFormGroup() and populateAllCombinations() methods. Added statuses and timeCategories properties. Replaced patient list management methods (addPatientList, removePatientList, loadPatientLists). Updated submitConfiguration to extract status, timeFrame, and fhirId from form controls.
Data Model
data-acquisition-fhir-list-config-model.interface.ts
Updated IEhrPatientListModel to replace required listIds and measureIds arrays with optional status, timeFrame, and fhirId string fields.
Dialog Configuration
facility-edit.component.ts
Adjusted DataAcquisitionFhirListConfigDialog width from 75% to 50vw with 50vw maxWidth.

Sequence Diagram

sequenceDiagram
    participant User
    participant Component
    participant Form as FormArray

    User->>Component: Open configuration (ngOnInit/ngOnChanges)
    Component->>Component: populateAllCombinations()
    loop For each status × timeFrame combination
        Component->>Form: createPatientFormGroup()
        Form-->>Component: FormGroup {status, timeFrame, fhirId}
    end
    Component->>Component: Initialize patientListControl
    
    User->>Component: Enter FHIR IDs and Submit
    Component->>Form: Iterate form controls
    loop For each patient form
        Form->>Component: Extract {status, timeFrame, fhirId}
    end
    Component->>Component: submitConfiguration()
    Component-->>User: Send ehrPatientLists
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Component refactoring: Complete overhaul of form initialization and patient list management logic; verify new populateAllCombinations() correctly generates all status/timeFrame combinations and properly maps existing data
  • Interface breaking change: IEhrPatientListModel now has fundamentally different fields (status/timeFrame/fhirId vs. listIds/measureIds); confirm all consumers are updated
  • Service removal: MeasureDefinitionService dependency eliminated; verify no lingering references or side effects
  • Form data flow: Trace form construction, submission, and data extraction to ensure status, timeFrame, and fhirId are correctly wired end-to-end

Possibly related PRs

Suggested reviewers

  • amphillipsLGC
  • dvargaslantana

Poem

🐰 Status, timeFrame, and fhirId so bright,
Old measure lists fade from sight,
A table of combinations, neat and clean,
The most elegant POI form I've seen!
thump thump goes my feet with delight ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 inconclusive)
Check name Status Explanation Resolution
Out of Scope Changes check ❓ Inconclusive The dialog width adjustment in facility-edit.component.ts (50vw instead of 75%) appears to be a presentational adjustment related to the UI reconfiguration but is not explicitly mentioned in objectives. Clarify whether the dialog width change from 75% to 50vw is intentional for the POI configuration UI redesign or an unrelated adjustment.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'LNK-4455: Fhir List Configuration Changes for POI' directly references the linked issue and accurately summarizes the primary change of updating FHIR list configuration for POI.
Description check ✅ Passed The description follows the template structure with sections for Changes, Testing, and Unit Testing. However, the description lacks specific technical details about what was changed and why.
Linked Issues check ✅ Passed Changes implement the POI-based configuration with Status, TimeFrame, and FhirId fields as required LNK-4455, replacing previous ListIds/MeasureIds approach.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch LNK-4455-FhirListChangesForPOI

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
Web/Admin.UI/src/app/components/tenant/facility-edit/facility-edit.component.ts (1)

348-349: Verify dialog dimension consistency across the component.

The FHIR List config dialog now uses 50vw dimensions, deviating from the 75% width pattern used in showFacilityDialog(), showCensusDialog(), showQueryDispatchDialog(), and showDataAcqFhirQueryDialog(). Only showOperationDialog() (line 521) currently matches this pattern.

Consider whether this narrower width is intentional for the FHIR List form, or if all similar dialogs should be standardized for consistency. If this is a deliberate change to accommodate the form's layout, document the reasoning or ensure visual testing validates the responsiveness across viewport sizes.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between c9c65e3 and 9c88220.

📒 Files selected for processing (5)
  • Web/Admin.UI/src/app/components/data-acquisition/data-acquisition-fhir-list-config-form/data-acquisition-fhir-list-config-form.component.html (2 hunks)
  • Web/Admin.UI/src/app/components/data-acquisition/data-acquisition-fhir-list-config-form/data-acquisition-fhir-list-config-form.component.scss (1 hunks)
  • Web/Admin.UI/src/app/components/data-acquisition/data-acquisition-fhir-list-config-form/data-acquisition-fhir-list-config-form.component.ts (6 hunks)
  • Web/Admin.UI/src/app/components/tenant/facility-edit/facility-edit.component.ts (1 hunks)
  • Web/Admin.UI/src/app/interfaces/data-acquisition/data-acquisition-fhir-list-config-model.interface.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**

⚙️ 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:

  • Web/Admin.UI/src/app/components/tenant/facility-edit/facility-edit.component.ts
  • Web/Admin.UI/src/app/components/data-acquisition/data-acquisition-fhir-list-config-form/data-acquisition-fhir-list-config-form.component.scss
  • Web/Admin.UI/src/app/components/data-acquisition/data-acquisition-fhir-list-config-form/data-acquisition-fhir-list-config-form.component.ts
  • Web/Admin.UI/src/app/components/data-acquisition/data-acquisition-fhir-list-config-form/data-acquisition-fhir-list-config-form.component.html
  • Web/Admin.UI/src/app/interfaces/data-acquisition/data-acquisition-fhir-list-config-model.interface.ts
🧬 Code graph analysis (1)
Web/Admin.UI/src/app/components/data-acquisition/data-acquisition-fhir-list-config-form/data-acquisition-fhir-list-config-form.component.ts (1)
Web/Admin.UI/src/app/interfaces/data-acquisition/data-acquisition-fhir-list-config-model.interface.ts (1)
  • IEhrPatientListModel (11-15)
⏰ 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). (1)
  • GitHub Check: Analyze (csharp)

@arianamihailescu
arianamihailescu merged commit 39b5bdf into dev Nov 7, 2025
18 checks passed
@arianamihailescu
arianamihailescu deleted the LNK-4455-FhirListChangesForPOI branch November 7, 2025 18:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants