From 212d2693cda4c2259c3934a18da0afa1809ef975 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Wed, 22 Jul 2026 11:04:29 -0600 Subject: [PATCH 1/2] Only wrap ehr_lookups tables whose pseudo-PK differs from the real PK The generic fallback in EHRLookupsUserSchema.createTable() wrapped any hard table with a single non-rowid key field and a container column in a ContainerScopedTable. For tables whose user-facing key is the true DB PK (clinpath_status, project_types, full_snomed, flag_values), the PK constraint already enforces uniqueness and the wrapper made the key column hidden and non-insertable in the UI. The heuristic now compares the promoted key against the real PK from JDBC metadata, so those tables get CustomPermissionsTable while all 26 rowid-plus-pseudo-PK tables keep container scoping. Claude-Session: https://claude.ai/code/session_013WM8RcTJDaaopwMTK4H9KX --- .../org/labkey/ehr/query/EHRLookupsUserSchema.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java b/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java index 415ec8c1f..23912e56a 100644 --- a/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java +++ b/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java @@ -212,10 +212,18 @@ else if (EHRSchema.TABLE_LOOKUP_SETS.equalsIgnoreCase(name)) // By default, any hard tables in the ehr_lookups schema not accounted for above will fall into one of the // two categories below. Both of these will add a check that makes sure the user has EHRDataAdminPermission - // in order to insert/update/delete on the table. The ContainerScopedTable case is for those tables that - // have a true DB PK or rowid but a User pseudoPK that should be accounted for at the container level. + // in order to insert/update/delete on the table. The ContainerScopedTable case is for tables whose + // user-facing key (promoted via isKeyField in ehr_lookups.xml) differs from the true DB PK: the PK + // constraint does not enforce uniqueness of the pseudo-PK, so the wrapper enforces it per container and + // resolves the pseudo-PK to the real PK on update. Tables whose user-facing key is the true PK + // (e.g. project_types) get CustomPermissionsTable instead: the PK constraint already enforces uniqueness, + // and container-scoping such a table made its key column non-insertable in the UI. String pkColName = getPkColName(ti); - if (pkColName != null && !"rowid".equalsIgnoreCase(pkColName) && ti.getColumn("container") != null) + List realPk = _dbSchema.getTable(name).getPkColumnNames(); + boolean singleColumnPks = pkColName != null && realPk.size() == 1; + boolean realPkMatchesPseudoPk = singleColumnPks && realPk.get(0).equalsIgnoreCase(pkColName); + + if (singleColumnPks && !realPkMatchesPseudoPk && ti.getColumn("container") != null) return getContainerScopedTable(name, cf, pkColName, EHRDataAdminPermission.class); else return getCustomPermissionTable(createSourceTable(name), cf, EHRDataAdminPermission.class); From f0a6b71904f6b72a8417be5520fb084fd26db9e5 Mon Sep 17 00:00:00 2001 From: Marty Pradere Date: Thu, 23 Jul 2026 06:07:00 -0600 Subject: [PATCH 2/2] Read the real PK from the wrapped table instead of re-querying DbSchema Addresses PR feedback: the wrapped SimpleTable already holds the source SchemaTableInfo, so read the real PK via getRealTable() (the same idiom ContainerScopedTable.init() uses) rather than resolving the table again through _dbSchema.getTable(). Claude-Session: https://claude.ai/code/session_01YJwkmACfqMmYiAX15YbU1D --- ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java b/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java index 23912e56a..1ee147f51 100644 --- a/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java +++ b/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java @@ -219,7 +219,9 @@ else if (EHRSchema.TABLE_LOOKUP_SETS.equalsIgnoreCase(name)) // (e.g. project_types) get CustomPermissionsTable instead: the PK constraint already enforces uniqueness, // and container-scoping such a table made its key column non-insertable in the UI. String pkColName = getPkColName(ti); - List realPk = _dbSchema.getTable(name).getPkColumnNames(); + List realPk = ti instanceof FilteredTable ft + ? ft.getRealTable().getPkColumnNames() + : _dbSchema.getTable(name).getPkColumnNames(); boolean singleColumnPks = pkColName != null && realPk.size() == 1; boolean realPkMatchesPseudoPk = singleColumnPks && realPk.get(0).equalsIgnoreCase(pkColName);