Skip to content

Conversation

@labkey-nicka
Copy link
Contributor

Rationale

Define LastIndexed expression column on exp.materials to give users insight into the last time a material was indexed. Follow up item for #786.

Changes

  • Define LastIndexed expression column on exp.materials
  • Provide consistent descriptions for materials and data

@labkey-nicka labkey-nicka requested a review from a team January 20, 2026 17:04
@labkey-nicka labkey-nicka self-assigned this Jan 20, 2026
Copy link
Contributor

@labkey-susanh labkey-susanh left a comment

Choose a reason for hiding this comment

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

Changes look good. I've not done any manual testing.

@labkey-jeckels
Copy link
Contributor

I manually tested. It worked fine in the normal case.

However, if a sample type already contains an admin-defined LastIndexed field, when asserts are enabled, the sample type is unusable:

java.lang.AssertionError: Column LastIndexed already exists for table Upgrader. Full set of existing columns: [RowId, MaterialSourceId, SourceProtocolApplication, SourceApplicationInput, RunApplication, RunApplicationOutput, Protocol, Name, Alias, Description, SampleSet, MaterialExpDate, Folder, Run, LSID, RootMaterialRowId, AliquotedFromLSID, IsAliquot, Created, CreatedBy, Modified, ModifiedBy, Flag, SampleState, genId, LastIndexed, AliquotCount, AliquotVolume, AliquotUnit, AvailableAliquotCount, AvailableAliquotVolume, StoredAmount, Units, RawAmount, RawUnits, IsPlated]
	at org.labkey.api.data.AbstractTableInfo.addColumn(AbstractTableInfo.java:789)
	at org.labkey.experiment.api.ExpTableImpl.addColumn(ExpTableImpl.java:223)
	at org.labkey.experiment.api.ExpTableImpl.addColumn(ExpTableImpl.java:203)
	at org.labkey.experiment.api.ExpMaterialTableImpl.populateColumns(ExpMaterialTableImpl.java:919)

Without asserts, I believe the admin-defined field would be masked by this calculated column.

This also applies to previously added fields. I had a sample type sitting around with an AliquotVolume that is in a similar state. Maybe we're comfortable with the risk of existing LastIndexed fields for this PR, but we need a better strategy for introducing new reserved field names.

@labkey-nicka
Copy link
Contributor Author

I manually tested. It worked fine in the normal case.

However, if a sample type already contains an admin-defined LastIndexed field, when asserts are enabled, the sample type is unusable

I've responded on the ticket.

@labkey-nicka labkey-nicka merged commit 9028e69 into release25.11-SNAPSHOT Jan 22, 2026
14 of 15 checks passed
@labkey-nicka labkey-nicka deleted the 25.11_fb_last_indexed_786 branch January 22, 2026 16:27
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.

4 participants