Normalize employeeid, unit, and category on write in compliance tables - #1191
Open
labkey-martyp wants to merge 1 commit into
Open
Normalize employeeid, unit, and category on write in compliance tables#1191labkey-martyp wants to merge 1 commit into
labkey-martyp wants to merge 1 commit into
Conversation
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.
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:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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 joinemployeeperunitagainstrequirementspercategory, and the latter already canonicalizesunitandcategoryon write through its trigger script — soemployeeperunit, the only table inehr_compliancedbwithout one, was the sole source of case drift. Normalizing on write instead removes the need forlower()downstream and keeps the joins indexable.Related Pull Requests
lower()calls this change makes unnecessary. Merge this PR first, or those reports will silently drop rows whose unit or category differs only by case.Changes
employeeperunit.js, normalizingemployeeid,unit, andcategoryagainst the lookups its schema metadata already declares.sopdates.js, normalizingemployeeid.sopidis excluded deliberately — its fk tosopsis commented out inehr_compliancedb.xml, andgetLookupValuereturns null for a column with no fk, which would reject every row.Tasks 📍