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.

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