Skip to content

Allow inserts on ContainerScopedTable when pseudo-key is the real PK#305

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

Allow inserts on ContainerScopedTable when pseudo-key is the real PK#305
labkey-martyp wants to merge 1 commit into
release26.3-SNAPSHOTfrom
26.3_fb_container_scoped_natural_key

Conversation

@labkey-martyp

Copy link
Copy Markdown

Rationale

ContainerScopedTable.init() hid and disabled every real PK column, assuming the real PK is always a surrogate key (e.g. an auto-incrementing rowid). When the pseudo-key is also the real PK (a natural key, like ehr_lookups.project_types.type), this made the key column hidden and non-editable, so it could not be entered through the single-row insert UI (API-based inserts were unaffected).

Related Pull Requests

None.

Changes

  • Detect when the pseudo-key column is itself the real PK and leave it visible and editable so it can be entered in the single-row insert UI; other real PK columns are hidden and disabled as before.
  • Editing the column remains unavailable in the UI: it is hidden from the update form via setShownInUpdateView(false), so it is enterable on insert but not shown when editing.

ContainerScopedTable.init() hid and disabled every real PK column, assuming the real PK is always a surrogate key. When the pseudo-key is also the real PK (a natural key, like ehr_lookups.project_types.type), this made the key column hidden and non-editable, so inserts were impossible. Detect that case and leave the column visible and editable, hiding it only from the update form.
@bbimber

bbimber commented Jul 16, 2026

Copy link
Copy Markdown
Collaborator

@labkey-martyp and @labkey-bpatel: I dont think there is necessarily a problem with this; however, if the true PK is the pseudo PK, why is the upstream code using ContainerScopedTable? Is there anything that ContainerScopedTable actually adds?

I see that there are examples in EHR where a table is using ContainerScopedTable, where it probably should just use CustomPermissionsTable, such as EHRUserSchema line 79. I think these two would be more appropriately set as CustomPermissionsTable, although I doubt it would have created a problem until something like this (https://github.com/LabKey/ehrModules/blob/a53bc60337522fd1b10757b18aaa045fa5b6e828/ehr/src/org/labkey/ehr/query/EHRUserSchema.java#L80)

        else if (EHRSchema.TABLE_PROTOCOL.equalsIgnoreCase(name))
            return getCustomPermissionTable(createSourceTable(name), cf, EHRProtocolEditPermission.class);
        else if (EHRSchema.TABLE_PROJECT.equalsIgnoreCase(name))
            return getCustomPermissionTable(createSourceTable(name), cf, EHRProjectEditPermission.class);

Since this mentions project_type, I assume the source of that is here: https://github.com/LabKey/ehrModules/blob/a53bc60337522fd1b10757b18aaa045fa5b6e828/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java#L219. I cant think of any reason why that logic is right. Again, outside of editing a PK, it probably never mattered either. I bet this was introduced in ~2018 when container was added to a lot of these tables (one commit of many: LabKey/ehrModules@03e8338)

This code in EHRLookupUserSchema should be more like this: https://github.com/LabKey/ehrModules/blob/a53bc60337522fd1b10757b18aaa045fa5b6e828/ehr/src/org/labkey/ehr/query/EHRLookupsUserSchema.java#L217

            // 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.
            // This only applies if there is a column named rowid
            String pkColName = getPkColName(ti);
            if (pkColName != null && ti.getColumn("rowid") != null && "rowid".equalsIgnoreCase(pkColName) &&  ti.getColumn("container") != null)
                return getContainerScopedTable(name, cf, pkColName, EHRDataAdminPermission.class);
            else
                return getCustomPermissionTable(createSourceTable(name), cf, EHRDataAdminPermission.class);

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.

3 participants