Skip to content

Add and populate core.Documents.ParentType column#7231

Merged
labkey-adam merged 13 commits intodevelopfrom
fb_parent_type
Dec 4, 2025
Merged

Add and populate core.Documents.ParentType column#7231
labkey-adam merged 13 commits intodevelopfrom
fb_parent_type

Conversation

@labkey-adam
Copy link
Contributor

@labkey-adam labkey-adam commented Nov 28, 2025

Rationale

Maintaining ParentType on core.Documents provides a fast way to associate attachments with their parents, which is useful for reporting, debugging, and eventually migrating to the global attachment manager

https://docs.google.com/document/d/19UOpB6nMBZ-DesYWUC4MTyN0heKZt5KDmHI-p3TBrc4/edit?tab=t.0#heading=h.4rg4h5vuek0

Changes

  • Rename AttachmentType -> AttachmentParentType for clarity.
  • Shorten all existing AttachmentParentType implementations to return short, human-readable names from getUniqueName(). No reason for long, verbose names in the DB and admin reports.
  • Add a ParentType column to core.Documents and populate via upgrade code that leverages existing AttachmentParentType.addWhereSql() method
  • Populate ParentType while inserting new attachments
  • Delete obsolete, orphaned auth logo attachments
  • Expose core.Documents table in core query schema. Add DocumentsGroupedByParentType query to replace old admin report.
  • Add new ${containerFilterName} substitution parameter option for URL templates
  • Add and populate ParentType column in all attachment audit events
  • Replace the old Attachments page with a link to the DocumentsGroupedByParentType query in the root, with AllFolders set as the container filter. ("Attachments - Old" is left pointed at the old page temporarily, to allow comparisons during testing.)
  • Support use of @DeferredUpgrade for core scripts. Two issues prevented the use of this annotation for core upgrade code:
    • We were clearing caches before (and after) invoking synchronous upgrade code, but not when queuing up deferred upgrade code. As a result, the core.UpgradeSteps table (where we queue the pending method names) was not recognized as in-the-database on bootstrap. Hoisting the cache clearing up a level causes it to be invoked in both cases.
    • When inserting rows into core.UpgradeSteps, we were passing in the upgrade user (which is the search user). This reference caused us to initialize roles like Site Administrator before site groups had been created, which ended up populating null groups into the excluded principals lists, which caused NPEs later. We can just pass in null to the insert. And I added a null check in addExcludedPrincipal() so we fail fast if this ever shows up again.

Note: Many admin reports are available to Troubleshooters. This one is not (it's available to Site/App admin) only because it's implemented as a standard query that's shown via query-executeQuery.view, that action requires read permission in the current folder, and the Troubleshooter role doesn't grant read permissions anywhere.

Tasks 📍

  • Manual Testing @labkey-tchad
    • Checkout the fb_parent_type feature branch in all five repos.
    • Upgrade a reasonably well-populated PostgreSQL database with signing module present
    • Verify clean upgrade. Note if upgrade time seems noticeable.
    • Insert a few attachments. Download some. Delete some. Verify that attachment audit log events now include ParentType for the new rows (not old ones)
    • Visit Admin Console -> Attachments, which displays the DocumentsGroupedByParentType query set to show AllFolders. Only Site/App admins should be able to see and invoked this link. Filter, sort, and navigate to the core.Documents query to see a detailed listing.
    • You should see no orphaned attachments (i.e., ParentType blank)
    • The "Attachments - Old" page shows the old (dynamically generated) attachment report; the new grouped query should match. (I'll remove this before merge.)
    • Schema browser -> core -> Documents is another entry point into browsing the individual document rows. Here also only Site/App admins should be able to see and execute this query. Note that the grouped query is hidden by default since most users won't be able to invoke it.
    • Test upgrade, etc. on SQL Server?
  • Needs Automation - 1f97d4b
  • Automated test review @labkey-tchad

import org.labkey.api.data.SQLFragment;

/**
* Tags {@link Attachment} objects based on what they're attached to. Does not indicate that they are a file of a
Copy link
Contributor

Choose a reason for hiding this comment

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

Line ending change to this while file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I honestly can't tell. IntelliJ doesn't think line endings changed. Opening before & after files in binary mode text editor shows no line ending changes. Maybe GitHub always shows renames this way?

Copy link
Contributor

Choose a reason for hiding this comment

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

It might look better if you add the new filename to .gitattributes

labkey-tchad and others added 4 commits December 2, 2025 14:48
@labkey-tchad
Copy link
Member

labkey-tchad commented Dec 3, 2025

New upgrade code doesn't work on SQL Server:

com.microsoft.sqlserver.jdbc.SQLServerException: Incorrect syntax near 'd'.
	[...]
	at org.labkey.api.data.dialect.StatementWrapper.execute(StatementWrapper.java:1620)
	at org.labkey.api.data.SqlExecutor$NormalStatementExecutor.execute(SqlExecutor.java:162)
	at org.labkey.api.data.SqlExecutor$NormalStatementExecutor.execute(SqlExecutor.java:136)
	at org.labkey.api.data.SqlExecutor.execute(SqlExecutor.java:114)
	at org.labkey.api.data.SqlExecutor.execute(SqlExecutor.java:75)
	at org.labkey.core.CoreUpgradeCode.populateAttachmentParentTypeColumn(CoreUpgradeCode.java:206)
	[...]
ERROR Table                    2025-12-03T13:10:41,104           Module Upgrade : SQL  [62]
    UPDATE "core"."Documents" d SET ParentType = ? WHERE d.Parent IN (SELECT EntityId FROM "exp"."ExperimentRun" er)
    ?[1] ExpRun

@labkey-adam labkey-adam merged commit a73f7b3 into develop Dec 4, 2025
11 of 12 checks passed
@labkey-adam labkey-adam deleted the fb_parent_type branch December 4, 2025 17:33
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