Skip to content

Normalize employeeid, unit, and category on write in compliance tables - #1191

Open
labkey-martyp wants to merge 1 commit into
release26.3-SNAPSHOTfrom
26.3_fb_final_onprc_scripts
Open

Normalize employeeid, unit, and category on write in compliance tables#1191
labkey-martyp wants to merge 1 commit into
release26.3-SNAPSHOTfrom
26.3_fb_final_onprc_scripts

Conversation

@labkey-martyp

Copy link
Copy Markdown
Contributor

Rationale

The ported PostgreSQL report procedures in ONPRC's compliance module wrapped every unit and category comparison in lower() to emulate the case-insensitive default collation the MS SQL originals relied on implicitly. All of those comparisons join employeeperunit against requirementspercategory, and the latter already canonicalizes unit and category on write through its trigger script — so employeeperunit, the only table in ehr_compliancedb without one, was the sole source of case drift. Normalizing on write instead removes the need for lower() downstream and keeps the joins indexable.

Related Pull Requests

Changes

  • Adds employeeperunit.js, normalizing employeeid, unit, and category against the lookups its schema metadata already declares.
  • Adds sopdates.js, normalizing employeeid. sopid is excluded deliberately — its fk to sops is commented out in ehr_compliancedb.xml, and getLookupValue returns null for a column with no fk, which would reject every row.

Tasks 📍

  • Claude Code Review
  • Code Review

employeeperunit was the only table in ehr_compliancedb with no trigger script, so its employeeid, unit, and category values were never canonicalized against their lookups - the reason the ONPRC report procedures had to wrap every unit and category comparison in lower(). Its schema metadata already declares all three columns as lookups, so the script is a drop-in copy of the ten siblings.

sopdates had no trigger script either. sopid is deliberately left out of its lookupFields: the fk to sops is commented out in ehr_compliancedb.xml, and getLookupValue returns null for a column with no fk, which would reject every row.

Pairs with the lower() removal in onprcEHRModules (PR 1859). Merge this first, or those reports will silently drop rows whose unit or category differs only by case. Existing rows carrying off-lookup values need a backfill before this deploys, since beforeUpdate revalidates the merged old row and will otherwise block edits that work today.
@bbimber

bbimber commented Aug 26, 2026

Copy link
Copy Markdown
Collaborator

@labkey-martyp and @labkey-jeckels: the pattern outlined here probably need to occur in more places. Are there ways to do this that are more efficiency and wont require repeating a lot of boilerplate code? Some thoughts:

  • If we need to do this in query-specific trigger scripts, can the code be generalized into a central method that accepts the row object, key, lookup target, etc.?
  • Could this jump into java, iterate ColumnInfos and infer what to do? For example, and string with a lookup to a table in EHR_Lookups probably should have enforcement.
  • Perhaps something in the EHR trigger init() layer could let modules register names of target tables or target tables/field pairs, and any column with a FK to that table undergoes case-normalization?
  • If we need JS files, could we use a JS schema customizer instead of per query?

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