Skip to content

Conversation

@labkey-adam
Copy link
Contributor

@labkey-adam labkey-adam commented Jan 11, 2026

Rationale

Improve the core.Documents query by joining to each document's parent and providing Parent Description and Orphaned column. This makes it easier to find the parent document and highlights attachments that have been orphaned. https://github.com/LabKey/internal-issues/issues/711

Also, fix the extraction of entity IDs from data class LSIDs. At some point, the extraction method got out-of-sync with the LSIDs we produce, which resulted in data class attachment rows not getting correctly populated with their parent type. Once we've tested this PR, I'll add a new SQL script to re-run the upgrade code that populates the ParentType column.

Some notes and caveats:

  • All orphaned attachments have a null parent description, so filtering on ParentDescription IS BLANK is a reasonable way to find orphans. However, it may be possible for a parent to have a null description (e.g., a parent document with a null title); the hidden boolean field Orphaned provides a definitive check.
  • For now, "global" attachments all resolve to "Global Attachment Parent" instead of the specific Notebook or Notebook Entry. This will be reconciled once the global attachment approach is integrated into the standard attachment service.
  • Some attachments (e.g., look and feel resources, file system snapshots) use the container as their parent, so the parent description is simply the name of the container (or <Root>).

Changes

  • Add AttachmentParentType.getSelectEntityIdAndDescriptionSql() that returns SQL that selects the parents' EntityId and a Description that identifies that parent. Rework getSelectParentEntityIdsSql() to simply wrap the new method, selecting only EntityId (to allow use within an IN clause).
  • Implement getSelectEntityIdAndDescriptionSql() on every attachment parent. Stop overriding getSelectParentEntityIdsSql() and disallow return of null.
  • Fix Lsid.getSqlExpressionToExtractObjectId() so it correctly extracts EntityIds from data class LSIDs. Add a junit test to keep this working.
  • Add LabKeyCollectors.joining(SQLFragment) that joins a String<SQLFragment> into a single SQLFragment.
  • Simplify and correct SQLFragment.join() to correctly handle corner cases like CTEs and temp tokens.

Tasks 📍

  • Manual test AdjudicationAssayResultType @cnathe ?
  • Manual test PanoramaPublicLogoResourceType & CatalogImageAttachmentType @labkey-jeckels ?
  • Manual test ExpDataClassType @labkey-jeckels ?
  • Testing a specific attachment parent type typically entails:
    1. Add one or more attachments of the appropriate type
    2. Clear the core.Documents.ParentType column via direct DB tool (e.g., UPDATE core.Documents SET ParentType = NULL)
    3. View core.Documents to ensure all attachments of that type are tagged with the appropriate ParentType and show a reasonable ParentDescription
  • General manual testing @labkey-tchad
    • Ensure orphaned attachments are highlighted (likely requires direct database delete of parent rows)
    • Exercise Attachments action from admin console. This is also available as (hidden query) core.DocumentsGroupedByParentType, which allows querying specific projects and folders with a container filter applied.
    • Exercise core.Documents query and verify reasonable parent types and descriptions
    • Verify that hidden action admin-findAttachmentParents.view resolves attachments (except orphaned and global)
  • Needs Automation - junit test added for verifying getSqlExpressionToExtractObjectId(). Tests already exist for core.Documents.
  • Verify Fix @labkey-tchad

@labkey-adam labkey-adam self-assigned this Jan 12, 2026
@labkey-adam labkey-adam added this to the 25.03 milestone Jan 12, 2026
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