Skip to content

Create nbri_ehr module#1

Open
labkey-martyp wants to merge 1 commit into
release26.7-SNAPSHOTfrom
26.7_fb_create_nbri_ehr_module
Open

Create nbri_ehr module#1
labkey-martyp wants to merge 1 commit into
release26.7-SNAPSHOTfrom
26.7_fb_create_nbri_ehr_module

Conversation

@labkey-martyp

Copy link
Copy Markdown

Rationale

Seeds the new nbriEHRModules repository with a working EHR module so NBRI development can proceed from a functioning baseline.

Related Pull Requests

None.

Changes

  • Added the initial nbri_ehr module tree (~1,060 files: Java, JS, SQL, XML, HTML, TSV, gradle/properties, sample data), including the org.labkey.nbri_ehr Java package, NBRI_EHR* classes, and the NBRI_EHR folder type.
  • Updated the GitHub workflow files with permissions blocks required by the release automation.
  • Updated .gitignore to exclude referenceStudy ETL logs.

Seeds the new nbriEHRModules repository from the nircEHRModules module tree, with every occurrence of nirc replaced by nbri (casing-matched: NIRC→NBRI, Nirc→Nbri, nirc→nbri) in file contents, file names, and directory names.

Also carries over the nircEHRModules versions of the GitHub workflow files (adding permissions blocks) and .gitignore (ignoring referenceStudy ETL logs).

@labkey-bpatel labkey-bpatel left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Overall looks like a good starting point.

I am sure we'll trim and iterate as development progresses, but a few things stood out on this initial pass:

  1. Are we planning to ETL data into this module? If not, the ETL queries and definitions and go.
  2. Area/Building-specific .qview.xml files - there are quite a few of these with hardcoded location names (e.g. activeBehaviorCases/Building 10.qview.xml). Since these are site-specific and easy to recreate from the UI, I think we can drop them.
  3. Two nbri_ehr DB scripts - any reason not to combine these into a single script?
  4. nbri_ehr/resources/web/nbri_ehr/images/ull-logo-main-subsite.png - should be removed.
  5. Death/Necropsy workflow - the combined form is complex and quite center-specific. Would it be worth splitting into two separate forms (a simpler Death form and a separate Necropsy form) as a baseline?
  6. Copyright headers - Since this is the initial commit, normalize 'Copyright (c) 2024-2026 LabKey Corporation' (and other date-range variants) to 'Copyright (c) 2026 LabKey Corporation' across the new files.

Revised comments from claude code review:

  1. Remove the duplicate CagematesDemographicsProvider registration.
  2. Escape HL7 delimiters in the Orchard file writer.
  3. Null-check birth before formatting in the Orchard file writer.
  4. Guard isProcedureOrderEntered against empty result and null qcstate.
  5. Delete datasetBirth.tsv~.
  6. Non-blocker: Three workflows (branch_release.yml, merge_release.yml, validate_pr.yml) each add a top-level permissions: block that matches what the reusable actions require. actions/checkout@v3 is used in two of them - GitHub has been recommending v4 for a while; not blocking, but consider bumping to keep parity with the newer LabKey repos.

private static final NBRI_EHRSchema _instance = new NBRI_EHRSchema();
public static final String NAME = "nbri_ehr";

public static NBRI_EHRSchema get_instance()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

should this be getInstance()?

return ts.getRowCount();
}

public boolean canCloseCase(String category)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

param 'category' not used, should it be?

ehrService.registerDemographicsProvider(new ProtocolAssignmentDemographicsProvider(this));
ehrService.registerDemographicsProvider(new HousingDemographicsProvider(this));
ehrService.registerDemographicsProvider(new CagematesDemographicsProvider(this));
ehrService.registerDemographicsProvider(new CagematesDemographicsProvider(this));

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

CagematesDemographicsProvider - added twice

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