From 7f58d13f98680117d0fc6777ce24d88260447c94 Mon Sep 17 00:00:00 2001 From: XingY Date: Fri, 17 Jul 2026 18:52:29 -0700 Subject: [PATCH 01/11] Sample type color field and color configuration --- .../org/labkey/api/data/ContainerManager.java | 3 + .../org/labkey/api/exp/api/ExpMaterial.java | 4 + .../labkey/api/exp/api/ExperimentService.java | 14 ++ .../api/SampleTypeDomainKindProperties.java | 11 ++ .../api/exp/query/ExpMaterialTable.java | 1 + .../org/labkey/api/exp/query/ExpSchema.java | 8 + .../postgresql/exp-26.007-26.008.sql | 39 +++++ experiment/resources/schemas/exp.xml | 28 ++++ .../labkey/experiment/ExperimentModule.java | 5 +- .../org/labkey/experiment/api/DataColor.java | 62 +++++++ .../experiment/api/DataColorManager.java | 120 ++++++++++++++ .../labkey/experiment/api/DataColorTable.java | 155 ++++++++++++++++++ .../experiment/api/ExpMaterialImpl.java | 12 ++ .../experiment/api/ExpMaterialTableImpl.java | 27 +++ .../experiment/api/ExperimentServiceImpl.java | 117 ++++++++++++- .../org/labkey/experiment/api/Material.java | 11 ++ .../experiment/api/SampleTypeServiceImpl.java | 34 +++- .../api/SampleTypeUpdateServiceDI.java | 2 + 18 files changed, 650 insertions(+), 3 deletions(-) create mode 100644 experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql create mode 100644 experiment/src/org/labkey/experiment/api/DataColor.java create mode 100644 experiment/src/org/labkey/experiment/api/DataColorManager.java create mode 100644 experiment/src/org/labkey/experiment/api/DataColorTable.java diff --git a/api/src/org/labkey/api/data/ContainerManager.java b/api/src/org/labkey/api/data/ContainerManager.java index 7d3c22a9729..12252b68af9 100644 --- a/api/src/org/labkey/api/data/ContainerManager.java +++ b/api/src/org/labkey/api/data/ContainerManager.java @@ -1952,7 +1952,10 @@ private static boolean delete(final Container c, User user, @Nullable String com ExperimentService experimentService = ExperimentService.get(); if (experimentService != null) + { experimentService.removeContainerDataTypeExclusions(c.getId()); + experimentService.removeContainerDataColorExclusions(c.getId()); + } // Issue 17015: After we've committed the transaction, be sure that we remove this container from the cache tx.addCommitTask(() -> diff --git a/api/src/org/labkey/api/exp/api/ExpMaterial.java b/api/src/org/labkey/api/exp/api/ExpMaterial.java index 26ac0c41810..76a61354fc7 100644 --- a/api/src/org/labkey/api/exp/api/ExpMaterial.java +++ b/api/src/org/labkey/api/exp/api/ExpMaterial.java @@ -85,6 +85,10 @@ public interface ExpMaterial extends ExpRunItem void setSampleStateId(Long stateId); + Long getSampleColorId(); + + void setSampleColorId(Long colorId); + Date getMaterialExpDate(); ActionURL detailsURL(Container container, boolean checkForOverride); diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index df052bbe503..8d616d713b9 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -710,6 +710,8 @@ static void validateParentAlias(Map aliasMap, Set reserv SampleStatusTable createSampleStatusTable(ExpSchema expSchema, ContainerFilter cf); + TableInfo createDataColorTable(ExpSchema expSchema, ContainerFilter cf); + ExpUnreferencedSampleFilesTable createUnreferencedSampleFilesTable(ExpSchema expSchema, ContainerFilter cf); FilteredTable createFieldsTable(ExpSchema expSchema, ContainerFilter cf); @@ -1139,6 +1141,18 @@ List getExpProtocolsWithParameterValue( String getDisabledDataTypeAuditMsg(DataTypeForExclusion type, List ids, boolean isUpdate); + @NotNull Set getDataTypeExcludedColors(DataTypeForExclusion dataType, long dataTypeId); + + @NotNull Set getActiveDataTypeColors(@NotNull Container container, DataTypeForExclusion dataType, long dataTypeId); + + boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion dataType, @Nullable Collection disabledColorRowIds, @NotNull Container container, User user); + + void removeDataColorExclusionsForColor(long colorRowId); + + void removeDataColorExclusionsForDataType(long dataTypeId, DataTypeForExclusion dataType); + + void removeContainerDataColorExclusions(String containerId); + void registerRunInputsViewProvider(QueryViewProvider provider); void registerRunOutputsViewProvider(QueryViewProvider providers); diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKindProperties.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKindProperties.java index 5ebe037728e..d80ab1c8a06 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKindProperties.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKindProperties.java @@ -87,6 +87,7 @@ public SampleTypeDomainKindProperties(ExpSampleType st) private String category; private List excludedContainerIds; private List excludedDashboardContainerIds; + private List disabledSampleColorRowIds; //Ignored on import/save, use Domain.name & Domain.description instead private String name; @@ -285,6 +286,16 @@ public List getExcludedDashboardContainerIds() return excludedDashboardContainerIds; } + public List getDisabledSampleColorRowIds() + { + return disabledSampleColorRowIds; + } + + public void setDisabledSampleColorRowIds(List disabledSampleColorRowIds) + { + this.disabledSampleColorRowIds = disabledSampleColorRowIds; + } + public void setExcludedDashboardContainerIds(List excludedDashboardContainerIds) { this.excludedDashboardContainerIds = excludedDashboardContainerIds; diff --git a/api/src/org/labkey/api/exp/query/ExpMaterialTable.java b/api/src/org/labkey/api/exp/query/ExpMaterialTable.java index 9d00482ab25..36a535f0523 100644 --- a/api/src/org/labkey/api/exp/query/ExpMaterialTable.java +++ b/api/src/org/labkey/api/exp/query/ExpMaterialTable.java @@ -66,6 +66,7 @@ enum Column RunId, // database table only RunApplication, RunApplicationOutput, + SampleColor, SampleSet, SampleState, SourceApplicationId, // database table only diff --git a/api/src/org/labkey/api/exp/query/ExpSchema.java b/api/src/org/labkey/api/exp/query/ExpSchema.java index 65b4a4c8805..e88cc46fe4d 100644 --- a/api/src/org/labkey/api/exp/query/ExpSchema.java +++ b/api/src/org/labkey/api/exp/query/ExpSchema.java @@ -245,6 +245,14 @@ public TableInfo createTable(ExpSchema expSchema, String queryName, ContainerFil return ExperimentService.get().createSampleStatusTable(expSchema, cf); } }, + DataColors + { + @Override + public TableInfo createTable(ExpSchema expSchema, String queryName, ContainerFilter cf) + { + return ExperimentService.get().createDataColorTable(expSchema, cf); + } + }, Fields { @Override diff --git a/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql b/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql new file mode 100644 index 00000000000..933872d668c --- /dev/null +++ b/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql @@ -0,0 +1,39 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0: http://www.apache.org/licenses/LICENSE-2.0 + */ + +CREATE TABLE exp.DataColors +( + RowId SERIAL NOT NULL, + Container ENTITYID NOT NULL, + Label VARCHAR(64) NOT NULL, + Color VARCHAR(7) NOT NULL, + Archived BOOLEAN NOT NULL DEFAULT FALSE, + Created TIMESTAMP, + CreatedBy INT, + Modified TIMESTAMP, + ModifiedBy INT, + + CONSTRAINT PK_DataColors PRIMARY KEY (RowId), + CONSTRAINT UQ_DataColors_Label UNIQUE (Container, Label) +); + +ALTER TABLE exp.Material ADD COLUMN SampleColor INT; +ALTER TABLE exp.Material ADD CONSTRAINT FK_Material_SampleColor FOREIGN KEY (SampleColor) REFERENCES exp.DataColors (RowId); + +CREATE TABLE exp.DataTypeColorExclusion +( + RowId SERIAL NOT NULL, + DataTypeRowId INT NOT NULL, + DataType VARCHAR(20) NOT NULL, + ColorRowId INT NOT NULL, + Created TIMESTAMP, + CreatedBy INT, + Modified TIMESTAMP, + ModifiedBy INT, + + CONSTRAINT PK_DataTypeColorExclusion PRIMARY KEY (RowId), + CONSTRAINT UQ_DataTypeColorExclusion UNIQUE (DataTypeRowId, DataType, ColorRowId) +); diff --git a/experiment/resources/schemas/exp.xml b/experiment/resources/schemas/exp.xml index 0aa6faa310e..524566f9484 100644 --- a/experiment/resources/schemas/exp.xml +++ b/experiment/resources/schemas/exp.xml @@ -295,6 +295,9 @@ Represents the status of the sample + + The color assigned to this individual sample + Amount @@ -1255,4 +1258,29 @@ + + + + + + + + + + + + +
+ + + + + + + + + + + +
diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index b81ced637c0..2ffea5a2155 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -118,6 +118,7 @@ import org.labkey.api.webdav.WebdavService; import org.labkey.api.writer.ContainerUser; import org.labkey.experiment.api.DataClassDomainKind; +import org.labkey.experiment.api.DataColorManager; import org.labkey.experiment.api.EdgeDiagnosticsTestCase; import org.labkey.experiment.api.ExpDataClassImpl; import org.labkey.experiment.api.ExpDataClassTableImpl; @@ -209,7 +210,7 @@ public String getName() @Override public Double getSchemaVersion() { - return 26.007; + return 26.008; } @Nullable @@ -553,6 +554,8 @@ protected void startupAfterSpringConfig(ModuleContext moduleContext) AuditLogService.get().registerAuditType(new SampleTypeAuditProvider()); AuditLogService.get().registerAuditType(new SampleTimelineAuditProvider()); + DataColorManager.getInstance().registerHandler(SampleTypeServiceImpl.get()); + FileContentService fileContentService = FileContentService.get(); if (null != fileContentService) { diff --git a/experiment/src/org/labkey/experiment/api/DataColor.java b/experiment/src/org/labkey/experiment/api/DataColor.java new file mode 100644 index 00000000000..02edbd45f17 --- /dev/null +++ b/experiment/src/org/labkey/experiment/api/DataColor.java @@ -0,0 +1,62 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0: http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.labkey.experiment.api; + +import org.labkey.api.data.Container; +import org.labkey.api.data.Entity; + +public class DataColor extends Entity +{ + private int _rowId; + private String _label; + private String _color; // hex value + private boolean _archived; + + public int getRowId() + { + return _rowId; + } + + public void setRowId(int rowId) + { + _rowId = rowId; + } + + public String getLabel() + { + return _label; + } + + public void setLabel(String label) + { + _label = label; + } + + public String getColor() + { + return _color; + } + + public void setColor(String color) + { + _color = color; + } + + public boolean isArchived() + { + return _archived; + } + + public void setArchived(boolean archived) + { + _archived = archived; + } + + public Container getContainer() + { + return lookupContainer(); + } +} diff --git a/experiment/src/org/labkey/experiment/api/DataColorManager.java b/experiment/src/org/labkey/experiment/api/DataColorManager.java new file mode 100644 index 00000000000..86c393cb79d --- /dev/null +++ b/experiment/src/org/labkey/experiment/api/DataColorManager.java @@ -0,0 +1,120 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0: http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.labkey.experiment.api; + +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.cache.Cache; +import org.labkey.api.cache.CacheManager; +import org.labkey.api.collections.LongHashMap; +import org.labkey.api.collections.StringHashMap; +import org.labkey.api.data.Container; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.Sort; +import org.labkey.api.data.TableSelector; + +import java.util.ArrayList; +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +public class DataColorManager +{ + /** Maximum number of data colors (active + archived) allowed per container. */ + public static final int MAX_DATA_COLORS = 200; + + private static final DataColorManager _instance = new DataColorManager(); + private static final Cache CACHE = CacheManager.getBlockingCache( + CacheManager.UNLIMITED, CacheManager.DAY, "Data colors", (c, argument) -> new DataColorCollections(c)); + private static final Map _handlers = new HashMap<>(); + + private static class DataColorCollections + { + private final List _colors; + private final Map _byRowId; + private final Map _byLabel; + + private DataColorCollections(Container c) + { + List colors = new ArrayList<>(); + Map byRowId = new LongHashMap<>(); + Map byLabel = new StringHashMap<>(); + + new TableSelector(ExperimentServiceImpl.get().getTinfoDataColors(), SimpleFilter.createContainerFilter(c), new Sort("Label")) + .forEach(DataColor.class, color -> { + colors.add(color); + byRowId.put((long) color.getRowId(), color); + byLabel.put(color.getLabel(), color); + }); + + _colors = Collections.unmodifiableList(colors); + _byRowId = Collections.unmodifiableMap(byRowId); + _byLabel = Collections.unmodifiableMap(byLabel); + } + } + + private DataColorManager() {} + + public static DataColorManager getInstance() + { + return _instance; + } + + public interface DataColorHandler + { + String getHandlerType(); + + boolean isColorInUse(Container container, long colorRowId); + } + + public void registerHandler(DataColorHandler handler) + { + String type = handler.getHandlerType(); + if (_handlers.containsKey(type)) + throw new IllegalArgumentException("DataColorHandler '" + type + "' is already registered."); + _handlers.put(type, handler); + } + + public boolean isInUse(Container container, long colorRowId) + { + for (DataColorHandler handler : _handlers.values()) + { + if (handler.isColorInUse(container, colorRowId)) + return true; + } + return false; + } + + @NotNull + public List getColors(Container container) + { + return CACHE.get(container)._colors; + } + + @NotNull + public List getActiveColors(Container container) + { + return getColors(container).stream().filter(c -> !c.isArchived()).toList(); + } + + @Nullable + public DataColor getColorForRowId(Container container, Long rowId) + { + return rowId == null ? null : CACHE.get(container)._byRowId.get(rowId); + } + + @Nullable + public DataColor getColorForLabel(Container container, String label) + { + return label == null ? null : CACHE.get(container)._byLabel.get(label); + } + + public void clearCache(Container c) + { + CACHE.remove(c); + } +} diff --git a/experiment/src/org/labkey/experiment/api/DataColorTable.java b/experiment/src/org/labkey/experiment/api/DataColorTable.java new file mode 100644 index 00000000000..076897a90ab --- /dev/null +++ b/experiment/src/org/labkey/experiment/api/DataColorTable.java @@ -0,0 +1,155 @@ +/* + * Copyright (c) 2026 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0: http://www.apache.org/licenses/LICENSE-2.0 + */ +package org.labkey.experiment.api; + +import org.apache.commons.lang3.StringUtils; +import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; +import org.labkey.api.data.ColumnInfo; +import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.DbScope; +import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.TableSelector; +import org.labkey.api.exp.query.ExpSchema; +import org.labkey.api.query.DefaultQueryUpdateService; +import org.labkey.api.query.DuplicateKeyException; +import org.labkey.api.query.FilteredTable; +import org.labkey.api.query.InvalidKeyException; +import org.labkey.api.query.QueryUpdateService; +import org.labkey.api.query.QueryUpdateServiceException; +import org.labkey.api.query.ValidationException; +import org.labkey.api.security.User; +import org.labkey.api.security.UserPrincipal; +import org.labkey.api.security.permissions.AdminPermission; +import org.labkey.api.security.permissions.Permission; +import org.labkey.api.security.permissions.ReadPermission; + +import java.sql.SQLException; +import java.util.Map; + +import static org.labkey.api.util.IntegerUtils.asLong; + +public class DataColorTable extends FilteredTable +{ + public DataColorTable(ExpSchema schema, ContainerFilter cf) + { + super(ExperimentServiceImpl.get().getTinfoDataColors(), schema, cf); + setName(ExpSchema.TableType.DataColors.name()); + for (ColumnInfo baseColumn : _rootTable.getColumns()) + { + String name = baseColumn.getName(); + if ("Container".equalsIgnoreCase(name)) + continue; + var col = addWrapColumn(baseColumn); + if ("RowId".equalsIgnoreCase(name)) + col.setHidden(true); + } + } + + @Override + public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class perm) + { + return getContainer().hasPermission(user, perm == ReadPermission.class ? perm : AdminPermission.class); + } + + @Override + public @Nullable QueryUpdateService getUpdateService() + { + return new DataColorUpdateService(this); + } + + private static class DataColorUpdateService extends DefaultQueryUpdateService + { + public DataColorUpdateService(FilteredTable table) + { + super(table, table.getRealTable()); + } + + private boolean isBlankLabel(Map row, boolean allowMissing) + { + if (allowMissing && !row.containsKey("label")) + return false; + return StringUtils.isBlank((String) row.get("label")); + } + + private boolean isDuplicateLabel(String label, Container container, int currentRowId) + { + for (DataColor color : DataColorManager.getInstance().getColors(container)) + { + if (color.getRowId() != currentRowId && color.getLabel().equalsIgnoreCase(label)) + return true; + } + return false; + } + + private long getColorCount(Container container) + { + SimpleFilter filter = SimpleFilter.createContainerFilter(container); + return new TableSelector(ExperimentServiceImpl.get().getTinfoDataColors(), filter, null).getRowCount(); + } + + @Override + protected Map insertRow(User user, Container container, Map row) throws DuplicateKeyException, ValidationException, QueryUpdateServiceException, SQLException + { + // TODO: audit insert/update/delete for this table + + if (isBlankLabel(row, false)) + throw new QueryUpdateServiceException("Label cannot be blank."); + if (isDuplicateLabel(String.valueOf(row.get("label")), container, -1)) + throw new QueryUpdateServiceException("Label '" + row.get("label") + "' is already in use."); + if (getColorCount(container) >= DataColorManager.MAX_DATA_COLORS) + throw new QueryUpdateServiceException("Cannot add more than " + DataColorManager.MAX_DATA_COLORS + " colors."); + + Map inserted; + try (DbScope.Transaction tx = ExperimentServiceImpl.getExpSchema().getScope().ensureTransaction()) + { + inserted = super.insertRow(user, container, row); + tx.addCommitTask(() -> DataColorManager.getInstance().clearCache(container), DbScope.CommitTaskOption.IMMEDIATE, DbScope.CommitTaskOption.POSTCOMMIT); + tx.commit(); + } + return inserted; + } + + @Override + protected Map updateRow(User user, Container container, Map row, @NotNull Map oldRow, boolean allowOwner, boolean retainCreation) throws InvalidKeyException, ValidationException, QueryUpdateServiceException, SQLException + { + if (isBlankLabel(row, true)) + throw new QueryUpdateServiceException("Label cannot be blank."); + int rowId = (int) row.get("rowId"); + if (row.containsKey("label") && isDuplicateLabel(String.valueOf(row.get("label")), container, rowId)) + throw new QueryUpdateServiceException("Label '" + row.get("label") + "' is already in use."); + + Map updated; + try (DbScope.Transaction tx = ExperimentServiceImpl.getExpSchema().getScope().ensureTransaction()) + { + updated = super.updateRow(user, container, row, oldRow, allowOwner, retainCreation); + tx.addCommitTask(() -> DataColorManager.getInstance().clearCache(container), DbScope.CommitTaskOption.IMMEDIATE, DbScope.CommitTaskOption.POSTCOMMIT); + tx.commit(); + } + return updated; + } + + @Override + protected Map deleteRow(User user, Container container, Map oldRowMap) throws InvalidKeyException, QueryUpdateServiceException, SQLException + { + long rowId = asLong(oldRowMap.get("rowId")); + if (DataColorManager.getInstance().isInUse(container, rowId)) + throw new QueryUpdateServiceException("This color can't be deleted because it is in use."); + + Map deleted; + try (DbScope.Transaction tx = ExperimentServiceImpl.getExpSchema().getScope().ensureTransaction()) + { + deleted = super.deleteRow(user, container, oldRowMap); + // Also drop any per-data-type exclusion rows that reference this color. + ExperimentServiceImpl.get().removeDataColorExclusionsForColor(rowId); + tx.addCommitTask(() -> DataColorManager.getInstance().clearCache(container), DbScope.CommitTaskOption.IMMEDIATE, DbScope.CommitTaskOption.POSTCOMMIT); + tx.commit(); + } + return deleted; + } + } +} diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java index 3799d5c8713..c2db27a6757 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java @@ -191,6 +191,18 @@ public void setSampleStateId(Long stateId) _object.setSampleState(stateId); } + @Override + public Long getSampleColorId() + { + return _object.getSampleColor(); + } + + @Override + public void setSampleColorId(Long colorId) + { + _object.setSampleColor(colorId); + } + @Override public DataState getSampleState() { diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 38f71e66222..f2a628375e3 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -597,6 +597,20 @@ protected ContainerFilter getLookupContainerFilter() { return createPropertiesColumn(alias); } + case SampleColor -> + { + boolean colorsEnabled = colorsEnabled(getContainer()); + var ret = wrapColumn(alias, _rootTable.getColumn(column.name())); + ret.setLabel("Sample Color"); + ret.setHidden(!colorsEnabled); + ret.setShownInDetailsView(colorsEnabled); + ret.setShownInInsertView(colorsEnabled); + ret.setShownInUpdateView(colorsEnabled); + ret.setRemapMissingBehavior(SimpleTranslator.RemapMissingBehavior.Error); + ret.setFk(new QueryForeignKey.Builder(getUserSchema(), getSampleStatusLookupContainerFilter()) + .schema(getExpSchema()).table(ExpSchema.TableType.DataColors).display("Label")); + return ret; + } case SampleState -> { boolean statusEnabled = isStatusEnabled(getContainer()); @@ -757,6 +771,16 @@ private static boolean isStatusEnabled(Container c) return SampleStatusService.get().supportsSampleStatus() && !SampleStatusService.get().getAllProjectStates(c).isEmpty(); } + // The SampleColor system field is shown only when at least one sample color is enabled for the sample type + // (or, for the non-type-specific "All Samples" table, when any active color is defined in the container). + private boolean colorsEnabled(Container c) + { + // TODO: check SampleManagement module + if (_ss != null) + return !ExperimentService.get().getActiveDataTypeColors(c, ExperimentService.DataTypeForExclusion.SampleType, _ss.getRowId()).isEmpty(); + return !DataColorManager.getInstance().getActiveColors(c).isEmpty(); + } + private Unit getSampleTypeUnit() { Unit typeUnit = null; @@ -862,6 +886,9 @@ protected void populateColumns() addColumn(SampleState); if (isStatusEnabled(getContainer())) defaultCols.add(SampleState.fieldKey()); + addColumn(SampleColor); + if (colorsEnabled(getContainer())) + defaultCols.add(SampleColor.fieldKey()); // TODO is this a real Domain??? if (st != null && !"urn:lsid:labkey.com:SampleSource:Default".equals(st.getDomain().getTypeURI())) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 5e97c39ce6e..d26cf649ad4 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -291,7 +291,6 @@ import java.util.concurrent.locks.ReentrantLock; import java.util.function.Predicate; import java.util.function.Supplier; -import java.util.function.UnaryOperator; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -1665,6 +1664,12 @@ public SampleStatusTable createSampleStatusTable(ExpSchema expSchema, ContainerF return new SampleStatusTable(expSchema, containerFilter); } + @Override + public TableInfo createDataColorTable(ExpSchema expSchema, ContainerFilter containerFilter) + { + return new DataColorTable(expSchema, containerFilter); + } + @Override public ExpUnreferencedSampleFilesTable createUnreferencedSampleFilesTable(ExpSchema expSchema, ContainerFilter cf) { @@ -3979,6 +3984,16 @@ public TableInfo getTinfoDataTypeExclusion() return getExpSchema().getTable("DataTypeExclusion"); } + public TableInfo getTinfoDataColors() + { + return getExpSchema().getTable("DataColors"); + } + + public TableInfo getTinfoDataTypeColorExclusion() + { + return getExpSchema().getTable("DataTypeColorExclusion"); + } + /** * return the object of any known experiment type that is identified with the LSID * @@ -9108,6 +9123,106 @@ public String getDisabledDataTypeAuditMsg(DataTypeForExclusion type, List return builder.toString(); } + @Override + public @NotNull Set getDataTypeExcludedColors(DataTypeForExclusion dataType, long dataTypeId) + { + SQLFragment sql = new SQLFragment("SELECT ColorRowId FROM ") + .append(getTinfoDataTypeColorExclusion()) + .append(" WHERE DataTypeRowId = ? AND DataType = ?") + .add(dataTypeId).add(dataType.name()); + return new HashSet<>(new SqlSelector(getExpSchema(), sql).getArrayList(Long.class)); + } + + @Override + public @NotNull Set getActiveDataTypeColors(@NotNull Container container, DataTypeForExclusion dataType, long dataTypeId) + { + Set disabled = getDataTypeExcludedColors(dataType, dataTypeId); + return DataColorManager.getInstance().getActiveColors(container).stream() + .map(c -> (long) c.getRowId()) + .filter(rowId -> !disabled.contains(rowId)) + .collect(toSet()); + } + + @Override + public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion dataType, @Nullable Collection disabledColorRowIds, @NotNull Container container, User user) + { + if (disabledColorRowIds == null) + return false; + + Set previous = getDataTypeExcludedColors(dataType, dataTypeId); + Set updated = new HashSet<>(disabledColorRowIds); + + Set toAdd = new HashSet<>(updated); + toAdd.removeAll(previous); + + Set toRemove = new HashSet<>(previous); + toRemove.removeAll(updated); + + if (toAdd.isEmpty() && toRemove.isEmpty()) + return false; + + try (DbScope.Transaction tx = getExpSchema().getScope().ensureTransaction()) + { + for (Long colorRowId : toAdd) + { + Map fields = new HashMap<>(); + fields.put("DataTypeRowId", dataTypeId); + fields.put("DataType", dataType.name()); + fields.put("ColorRowId", colorRowId); + Table.insert(user, getTinfoDataTypeColorExclusion(), fields); + } + if (!toRemove.isEmpty()) + { + SQLFragment sql = new SQLFragment("DELETE FROM ") + .append(getTinfoDataTypeColorExclusion()) + .append(" WHERE DataTypeRowId = ? AND DataType = ?") + .add(dataTypeId).add(dataType.name()) + .append(" AND ColorRowId "); + sql.appendInClause(toRemove, getExpSchema().getSqlDialect()); + new SqlExecutor(getExpSchema()).execute(sql); + } + tx.commit(); + } + + return true; + } + + @Override + public void removeDataColorExclusionsForColor(long colorRowId) + { + SQLFragment sql = new SQLFragment("DELETE FROM ") + .append(getTinfoDataTypeColorExclusion()) + .append(" WHERE ColorRowId = ?").add(colorRowId); + new SqlExecutor(getExpSchema()).execute(sql); + } + + @Override + public void removeDataColorExclusionsForDataType(long dataTypeId, DataTypeForExclusion dataType) + { + SQLFragment sql = new SQLFragment("DELETE FROM ") + .append(getTinfoDataTypeColorExclusion()) + .append(" WHERE DataTypeRowId = ? AND DataType = ?") + .add(dataTypeId).add(dataType.name()); + new SqlExecutor(getExpSchema()).execute(sql); + } + + @Override + public void removeContainerDataColorExclusions(String containerId) + { + SqlExecutor executor = new SqlExecutor(getExpSchema()); + SQLFragment delExclusions = new SQLFragment("DELETE FROM ") + .append(getTinfoDataTypeColorExclusion()) + .append(" WHERE ColorRowId IN (SELECT RowId FROM ") + .append(getTinfoDataColors()) + .append(" WHERE Container = ?)").add(containerId); + executor.execute(delExclusions); + + SQLFragment delColors = new SQLFragment("DELETE FROM ") + .append(getTinfoDataColors()) + .append(" WHERE Container = ?").add(containerId); + executor.execute(delColors); + } + @Override public void addObjectLegacyName(long objectId, String objectType, String legacyName, User user) { diff --git a/experiment/src/org/labkey/experiment/api/Material.java b/experiment/src/org/labkey/experiment/api/Material.java index 1d0be05427d..ca342e17891 100644 --- a/experiment/src/org/labkey/experiment/api/Material.java +++ b/experiment/src/org/labkey/experiment/api/Material.java @@ -35,6 +35,7 @@ public class Material extends RunItem private Long rootMaterialRowId; private String aliquotedFromLSID; private Long sampleState; + private Long sampleColor; private Date materialExpDate; private Double storedAmount; @@ -112,6 +113,16 @@ public void setSampleState(Long sampleState) this.sampleState = sampleState; } + public Long getSampleColor() + { + return sampleColor; + } + + public void setSampleColor(Long sampleColor) + { + this.sampleColor = sampleColor; + } + public Integer getAliquotCount() { return aliquotCount; diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index f69d0948abe..d90626fc60a 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -175,7 +175,7 @@ import static org.labkey.api.exp.query.ExpMaterialTable.Column.Units; -public class SampleTypeServiceImpl extends AbstractAuditHandler implements SampleTypeService +public class SampleTypeServiceImpl extends AbstractAuditHandler implements SampleTypeService, DataColorManager.DataColorHandler { public static final String SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:sampleCount"; public static final String ROOT_SAMPLE_COUNT_SEQ_NAME = "org.labkey.api.exp.api.ExpMaterial:rootSampleCount"; @@ -203,6 +203,20 @@ public static SampleTypeServiceImpl get() return (SampleTypeServiceImpl) SampleTypeService.get(); } + @Override + public String getHandlerType() + { + return "SampleColorMaterial"; + } + + @Override + public boolean isColorInUse(Container container, long colorRowId) + { + // Colors are referenced by samples across folders, so this is intentionally not container-scoped. + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.SampleColor.name()), colorRowId); + return new TableSelector(ExperimentServiceImpl.get().getTinfoMaterial(), filter, null).exists(); + } + private static final Logger LOG = LogHelper.getLogger(SampleTypeServiceImpl.class, "Info about sample type operations"); /** SampleType LSID -> Container cache */ @@ -686,6 +700,7 @@ public void deleteSampleType(long rowId, Container c, User user, @Nullable Strin ExperimentService.get().removeDataTypeExclusion(Collections.singleton(rowId), ExperimentService.DataTypeForExclusion.SampleType); ExperimentService.get().removeDataTypeExclusion(Collections.singleton(rowId), ExperimentService.DataTypeForExclusion.DashboardSampleType); + ExperimentService.get().removeDataColorExclusionsForDataType(rowId, ExperimentService.DataTypeForExclusion.SampleType); transaction.addCommitTask(() -> clearMaterialSourceCache(c), DbScope.CommitTaskOption.IMMEDIATE, POSTCOMMIT, POSTROLLBACK); transaction.commit(); @@ -1200,6 +1215,13 @@ public ValidationException updateSampleType(GWTDomain disabledColorRowIds = options.getDisabledSampleColorRowIds().stream().map(Integer::longValue).toList(); + boolean hasChange = ExperimentService.get().ensureDataColorExclusions(st.getRowId(), ExperimentService.DataTypeForExclusion.SampleType, disabledColorRowIds, container, user); + if (hasChange) + addAuditEventForSampleColorExclusion(container, st.getRowId(), user); + } errors = DomainUtil.updateDomainDescriptor(original, update, container, user, hasNameChange, changeDetails.toString(), auditUserComment, oldProps, newProps); @@ -1224,6 +1246,16 @@ public ValidationException updateSampleType(GWTDomain disabled = ExperimentService.get().getDataTypeExcludedColors(ExperimentService.DataTypeForExclusion.SampleType, materialSourceId); + String msg = "Sample color exclusion was updated for sample type (rowId " + materialSourceId + "). " + + (disabled.isEmpty() ? "All colors enabled." : "Excluded color rowIds: " + StringUtils.join(disabled, ", ") + "."); + AuditTypeEvent event = new AuditTypeEvent(SampleTypeAuditProvider.EVENT_TYPE, container, msg); + AuditLogService.get().addEvent(user, event); + } + + public String getCommentDetailed(QueryService.AuditAction action, boolean isUpdate) { String comment = SampleTimelineAuditEvent.SampleTimelineEventType.getActionCommentDetailed(action, isUpdate); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 39b5dabde21..968af990932 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -1309,6 +1309,8 @@ public DataIterator getDataIterator(DataIteratorContext context) continue; if (isExpMaterialColumn(SampleState, name)) continue; + if (isExpMaterialColumn(SampleColor, name)) + continue; if (isExpMaterialColumn(MaterialExpDate, name)) continue; if (isExpMaterialColumn(StoredAmount, name)) From 042d8832b53a0127237aa342bfa49583bd79e1d3 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 20 Jul 2026 10:01:28 -0700 Subject: [PATCH 02/11] Sample type color field and color configuration - handle exclusion --- .../labkey/api/exp/api/ExperimentService.java | 4 ++ .../labkey/api/exp/api/SampleTypeService.java | 2 + .../org/labkey/api/exp/query/ExpSchema.java | 8 +++ .../experiment/api/ExperimentServiceImpl.java | 60 +++++++++++++++++++ .../experiment/api/SampleTypeServiceImpl.java | 5 +- 5 files changed, 77 insertions(+), 2 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 8d616d713b9..8b4628c63c6 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -712,6 +712,8 @@ static void validateParentAlias(Map aliasMap, Set reserv TableInfo createDataColorTable(ExpSchema expSchema, ContainerFilter cf); + TableInfo createDataTypeColorExclusionTable(ExpSchema expSchema, ContainerFilter cf); + ExpUnreferencedSampleFilesTable createUnreferencedSampleFilesTable(ExpSchema expSchema, ContainerFilter cf); FilteredTable createFieldsTable(ExpSchema expSchema, ContainerFilter cf); @@ -1147,6 +1149,8 @@ List getExpProtocolsWithParameterValue( boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion dataType, @Nullable Collection disabledColorRowIds, @NotNull Container container, User user); + @NotNull Set updateColorDataTypeExclusions(long colorRowId, DataTypeForExclusion dataType, @Nullable Collection newlyDisabledDataTypeIds, @Nullable Collection newlyEnabledDataTypeIds, User user); + void removeDataColorExclusionsForColor(long colorRowId); void removeDataColorExclusionsForDataType(long dataTypeId, DataTypeForExclusion dataType); diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index e4a0f7c992f..24b0e50404a 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -123,6 +123,8 @@ static void setInstance(SampleTypeService impl) Map getSampleTypesForRoles(Container container, ContainerFilter filter, ExpProtocol.ApplicationType type); + void auditSampleColorExclusion(Container container, long sampleTypeRowId, User user); + /** * Create a new SampleType with the provided properties. * If a 'Name' property exists in the list, it will be used as the 'id' property of the SampleType. diff --git a/api/src/org/labkey/api/exp/query/ExpSchema.java b/api/src/org/labkey/api/exp/query/ExpSchema.java index e88cc46fe4d..954c95cf4ed 100644 --- a/api/src/org/labkey/api/exp/query/ExpSchema.java +++ b/api/src/org/labkey/api/exp/query/ExpSchema.java @@ -253,6 +253,14 @@ public TableInfo createTable(ExpSchema expSchema, String queryName, ContainerFil return ExperimentService.get().createDataColorTable(expSchema, cf); } }, + DataTypeColorExclusion + { + @Override + public TableInfo createTable(ExpSchema expSchema, String queryName, ContainerFilter cf) + { + return ExperimentService.get().createDataTypeColorExclusionTable(expSchema, cf); + } + }, Fields { @Override diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index d26cf649ad4..337176c4467 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -1670,6 +1670,12 @@ public TableInfo createDataColorTable(ExpSchema expSchema, ContainerFilter conta return new DataColorTable(expSchema, containerFilter); } + @Override + public TableInfo createDataTypeColorExclusionTable(ExpSchema expSchema, ContainerFilter containerFilter) + { + return new DataTypeColorExclusionTable(expSchema, containerFilter); + } + @Override public ExpUnreferencedSampleFilesTable createUnreferencedSampleFilesTable(ExpSchema expSchema, ContainerFilter cf) { @@ -9187,6 +9193,60 @@ public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion d return true; } + private Set getDataTypesExcludingColor(DataTypeForExclusion dataType, long colorRowId) + { + SQLFragment sql = new SQLFragment("SELECT DataTypeRowId FROM ") + .append(getTinfoDataTypeColorExclusion()) + .append(" WHERE ColorRowId = ? AND DataType = ?") + .add(colorRowId).add(dataType.name()); + return new HashSet<>(new SqlSelector(getExpSchema(), sql).getArrayList(Long.class)); + } + + @Override + public @NotNull Set updateColorDataTypeExclusions(long colorRowId, DataTypeForExclusion dataType, @Nullable Collection newlyDisabledDataTypeIds, @Nullable Collection newlyEnabledDataTypeIds, User user) + { + Set toAdd = newlyDisabledDataTypeIds == null ? new HashSet<>() : new HashSet<>(newlyDisabledDataTypeIds); + Set toRemove = newlyEnabledDataTypeIds == null ? new HashSet<>() : new HashSet<>(newlyEnabledDataTypeIds); + toRemove.removeAll(toAdd); + + if (toAdd.isEmpty() && toRemove.isEmpty()) + return Set.of(); + + Set existing = getDataTypesExcludingColor(dataType, colorRowId); + toAdd.removeAll(existing); + toRemove.retainAll(existing); + + if (toAdd.isEmpty() && toRemove.isEmpty()) + return Set.of(); + + try (DbScope.Transaction tx = getExpSchema().getScope().ensureTransaction()) + { + for (Long dataTypeId : toAdd) + { + Map fields = new HashMap<>(); + fields.put("DataTypeRowId", dataTypeId); + fields.put("DataType", dataType.name()); + fields.put("ColorRowId", colorRowId); + Table.insert(user, getTinfoDataTypeColorExclusion(), fields); + } + if (!toRemove.isEmpty()) + { + SQLFragment sql = new SQLFragment("DELETE FROM ") + .append(getTinfoDataTypeColorExclusion()) + .append(" WHERE ColorRowId = ? AND DataType = ?") + .add(colorRowId).add(dataType.name()) + .append(" AND DataTypeRowId "); + sql.appendInClause(toRemove, getExpSchema().getSqlDialect()); + new SqlExecutor(getExpSchema()).execute(sql); + } + tx.commit(); + } + + Set affected = new HashSet<>(toAdd); + affected.addAll(toRemove); + return affected; + } + @Override public void removeDataColorExclusionsForColor(long colorRowId) { diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index d90626fc60a..10e14e645f9 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -1220,7 +1220,7 @@ public ValidationException updateSampleType(GWTDomain disabledColorRowIds = options.getDisabledSampleColorRowIds().stream().map(Integer::longValue).toList(); boolean hasChange = ExperimentService.get().ensureDataColorExclusions(st.getRowId(), ExperimentService.DataTypeForExclusion.SampleType, disabledColorRowIds, container, user); if (hasChange) - addAuditEventForSampleColorExclusion(container, st.getRowId(), user); + auditSampleColorExclusion(container, st.getRowId(), user); } errors = DomainUtil.updateDomainDescriptor(original, update, container, user, hasNameChange, changeDetails.toString(), auditUserComment, oldProps, newProps); @@ -1246,7 +1246,8 @@ public ValidationException updateSampleType(GWTDomain disabled = ExperimentService.get().getDataTypeExcludedColors(ExperimentService.DataTypeForExclusion.SampleType, materialSourceId); String msg = "Sample color exclusion was updated for sample type (rowId " + materialSourceId + "). " From 746da0a486c273419d8bb2c857d716cff2f76550 Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 20 Jul 2026 11:00:58 -0700 Subject: [PATCH 03/11] metrics --- experiment/resources/schemas/exp.xml | 1 + .../labkey/experiment/ExperimentModule.java | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/experiment/resources/schemas/exp.xml b/experiment/resources/schemas/exp.xml index 524566f9484..fddc45f11b3 100644 --- a/experiment/resources/schemas/exp.xml +++ b/experiment/resources/schemas/exp.xml @@ -1050,6 +1050,7 @@ + diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 2ffea5a2155..7044cb2f35c 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -796,6 +796,24 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM results.put("sampleTypesWithVolumeTypeUnit", new SqlSelector(schema, "SELECT COUNT(*) from exp.materialSource WHERE category IS NULL AND metricunit IN ('L', 'mL', 'uL')").getObject(Long.class)); results.put("sampleTypesWithCountTypeUnit", new SqlSelector(schema, "SELECT COUNT(*) from exp.materialSource WHERE category IS NULL AND metricunit = ?", "unit").getObject(Long.class)); + Map sampleColorMetrics = new HashMap<>(); + Long colorCount = new SqlSelector(schema, "SELECT COUNT(*) FROM exp.datacolors").getObject(Long.class); + results.put("colorCount", colorCount); + if (colorCount > 0) + { + Long archivedColorCount = new SqlSelector(schema, new SQLFragment("SELECT COUNT(*) FROM exp.datacolors WHERE archived = " + schema.getSqlDialect().getBooleanTRUE())).getObject(Long.class); + results.put("archivedColorCount", archivedColorCount); + results.put("samplesWithColorCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.material WHERE samplecolor IS NOT NULL").getObject(Long.class)); + results.put("samplesWithArchivedColorCount", new SqlSelector(schema, new SQLFragment("SELECT COUNT(*) FROM exp.material m JOIN exp.datacolors dc ON m.samplecolor = dc.rowid WHERE dc.archived = " + schema.getSqlDialect().getBooleanTRUE())).getObject(Long.class)); + + results.put("sampleTypesWithColorsEnabledCount", new SqlSelector(schema, new SQLFragment( + "SELECT COUNT(*) FROM exp.materialsource ms WHERE EXISTS (" + + "SELECT 1 FROM exp.datacolors dc WHERE dc.container = ms.container AND dc.archived = " + schema.getSqlDialect().getBooleanFALSE() + " AND NOT EXISTS (" + + "SELECT 1 FROM exp.datatypecolorexclusion e WHERE e.datatype = ? AND e.datatyperowid = ms.rowid AND e.colorrowid = dc.rowid))") + .add(ExperimentService.DataTypeForExclusion.SampleType.name())).getObject(Long.class)); + } + results.put("sampleColors", sampleColorMetrics); + results.put("duplicateSampleMaterialNameCount", new SqlSelector(schema, "SELECT COUNT(*) as duplicateCount FROM " + "(SELECT name, cpastype FROM exp.material WHERE cpastype <> 'Material' GROUP BY name, cpastype HAVING COUNT(*) > 1) d").getObject(Long.class)); results.put("duplicateSpecimenMaterialNameCount", new SqlSelector(schema, "SELECT COUNT(*) as duplicateCount FROM " + From 5ae256d86ad7b553530364ff293a9de88a0f8ada Mon Sep 17 00:00:00 2001 From: XingY Date: Mon, 20 Jul 2026 14:21:55 -0700 Subject: [PATCH 04/11] Fix audit --- api/src/org/labkey/api/exp/api/ExperimentService.java | 5 +++-- api/src/org/labkey/api/exp/query/ExpSchema.java | 8 -------- .../src/org/labkey/experiment/ExperimentModule.java | 10 +++++----- .../src/org/labkey/experiment/api/DataColorTable.java | 2 -- .../labkey/experiment/api/ExperimentServiceImpl.java | 9 ++------- 5 files changed, 10 insertions(+), 24 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 8b4628c63c6..86155933550 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -712,8 +712,6 @@ static void validateParentAlias(Map aliasMap, Set reserv TableInfo createDataColorTable(ExpSchema expSchema, ContainerFilter cf); - TableInfo createDataTypeColorExclusionTable(ExpSchema expSchema, ContainerFilter cf); - ExpUnreferencedSampleFilesTable createUnreferencedSampleFilesTable(ExpSchema expSchema, ContainerFilter cf); FilteredTable createFieldsTable(ExpSchema expSchema, ContainerFilter cf); @@ -1145,6 +1143,9 @@ List getExpProtocolsWithParameterValue( @NotNull Set getDataTypeExcludedColors(DataTypeForExclusion dataType, long dataTypeId); + /** The data type rowIds (e.g. sample type rowIds) that currently exclude the given color. Inverse of {@link #getDataTypeExcludedColors}. */ + @NotNull Set getDataTypesExcludingColor(DataTypeForExclusion dataType, long colorRowId); + @NotNull Set getActiveDataTypeColors(@NotNull Container container, DataTypeForExclusion dataType, long dataTypeId); boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion dataType, @Nullable Collection disabledColorRowIds, @NotNull Container container, User user); diff --git a/api/src/org/labkey/api/exp/query/ExpSchema.java b/api/src/org/labkey/api/exp/query/ExpSchema.java index 954c95cf4ed..e88cc46fe4d 100644 --- a/api/src/org/labkey/api/exp/query/ExpSchema.java +++ b/api/src/org/labkey/api/exp/query/ExpSchema.java @@ -253,14 +253,6 @@ public TableInfo createTable(ExpSchema expSchema, String queryName, ContainerFil return ExperimentService.get().createDataColorTable(expSchema, cf); } }, - DataTypeColorExclusion - { - @Override - public TableInfo createTable(ExpSchema expSchema, String queryName, ContainerFilter cf) - { - return ExperimentService.get().createDataTypeColorExclusionTable(expSchema, cf); - } - }, Fields { @Override diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 7044cb2f35c..9c9958442f5 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -798,15 +798,15 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM Map sampleColorMetrics = new HashMap<>(); Long colorCount = new SqlSelector(schema, "SELECT COUNT(*) FROM exp.datacolors").getObject(Long.class); - results.put("colorCount", colorCount); + sampleColorMetrics.put("colorCount", colorCount); if (colorCount > 0) { Long archivedColorCount = new SqlSelector(schema, new SQLFragment("SELECT COUNT(*) FROM exp.datacolors WHERE archived = " + schema.getSqlDialect().getBooleanTRUE())).getObject(Long.class); - results.put("archivedColorCount", archivedColorCount); - results.put("samplesWithColorCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.material WHERE samplecolor IS NOT NULL").getObject(Long.class)); - results.put("samplesWithArchivedColorCount", new SqlSelector(schema, new SQLFragment("SELECT COUNT(*) FROM exp.material m JOIN exp.datacolors dc ON m.samplecolor = dc.rowid WHERE dc.archived = " + schema.getSqlDialect().getBooleanTRUE())).getObject(Long.class)); + sampleColorMetrics.put("archivedColorCount", archivedColorCount); + sampleColorMetrics.put("samplesWithColorCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.material WHERE samplecolor IS NOT NULL").getObject(Long.class)); + sampleColorMetrics.put("samplesWithArchivedColorCount", new SqlSelector(schema, new SQLFragment("SELECT COUNT(*) FROM exp.material m JOIN exp.datacolors dc ON m.samplecolor = dc.rowid WHERE dc.archived = " + schema.getSqlDialect().getBooleanTRUE())).getObject(Long.class)); - results.put("sampleTypesWithColorsEnabledCount", new SqlSelector(schema, new SQLFragment( + sampleColorMetrics.put("sampleTypesWithColorsEnabledCount", new SqlSelector(schema, new SQLFragment( "SELECT COUNT(*) FROM exp.materialsource ms WHERE EXISTS (" + "SELECT 1 FROM exp.datacolors dc WHERE dc.container = ms.container AND dc.archived = " + schema.getSqlDialect().getBooleanFALSE() + " AND NOT EXISTS (" + "SELECT 1 FROM exp.datatypecolorexclusion e WHERE e.datatype = ? AND e.datatyperowid = ms.rowid AND e.colorrowid = dc.rowid))") diff --git a/experiment/src/org/labkey/experiment/api/DataColorTable.java b/experiment/src/org/labkey/experiment/api/DataColorTable.java index 076897a90ab..674dee5dd32 100644 --- a/experiment/src/org/labkey/experiment/api/DataColorTable.java +++ b/experiment/src/org/labkey/experiment/api/DataColorTable.java @@ -95,8 +95,6 @@ private long getColorCount(Container container) @Override protected Map insertRow(User user, Container container, Map row) throws DuplicateKeyException, ValidationException, QueryUpdateServiceException, SQLException { - // TODO: audit insert/update/delete for this table - if (isBlankLabel(row, false)) throw new QueryUpdateServiceException("Label cannot be blank."); if (isDuplicateLabel(String.valueOf(row.get("label")), container, -1)) diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 337176c4467..b6c55364f62 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -1670,12 +1670,6 @@ public TableInfo createDataColorTable(ExpSchema expSchema, ContainerFilter conta return new DataColorTable(expSchema, containerFilter); } - @Override - public TableInfo createDataTypeColorExclusionTable(ExpSchema expSchema, ContainerFilter containerFilter) - { - return new DataTypeColorExclusionTable(expSchema, containerFilter); - } - @Override public ExpUnreferencedSampleFilesTable createUnreferencedSampleFilesTable(ExpSchema expSchema, ContainerFilter cf) { @@ -9193,7 +9187,8 @@ public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion d return true; } - private Set getDataTypesExcludingColor(DataTypeForExclusion dataType, long colorRowId) + @Override + public @NotNull Set getDataTypesExcludingColor(DataTypeForExclusion dataType, long colorRowId) { SQLFragment sql = new SQLFragment("SELECT DataTypeRowId FROM ") .append(getTinfoDataTypeColorExclusion()) From 50751851460abbf7dbe191f8887882197fac89d4 Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 21 Jul 2026 10:11:12 -0700 Subject: [PATCH 05/11] CC code review --- .../org/labkey/experiment/api/DataColorManager.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/experiment/src/org/labkey/experiment/api/DataColorManager.java b/experiment/src/org/labkey/experiment/api/DataColorManager.java index 86c393cb79d..47180d914ee 100644 --- a/experiment/src/org/labkey/experiment/api/DataColorManager.java +++ b/experiment/src/org/labkey/experiment/api/DataColorManager.java @@ -10,7 +10,6 @@ import org.labkey.api.cache.Cache; import org.labkey.api.cache.CacheManager; import org.labkey.api.collections.LongHashMap; -import org.labkey.api.collections.StringHashMap; import org.labkey.api.data.Container; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; @@ -36,24 +35,20 @@ private static class DataColorCollections { private final List _colors; private final Map _byRowId; - private final Map _byLabel; private DataColorCollections(Container c) { List colors = new ArrayList<>(); Map byRowId = new LongHashMap<>(); - Map byLabel = new StringHashMap<>(); new TableSelector(ExperimentServiceImpl.get().getTinfoDataColors(), SimpleFilter.createContainerFilter(c), new Sort("Label")) .forEach(DataColor.class, color -> { colors.add(color); byRowId.put((long) color.getRowId(), color); - byLabel.put(color.getLabel(), color); }); _colors = Collections.unmodifiableList(colors); _byRowId = Collections.unmodifiableMap(byRowId); - _byLabel = Collections.unmodifiableMap(byLabel); } } @@ -107,12 +102,6 @@ public DataColor getColorForRowId(Container container, Long rowId) return rowId == null ? null : CACHE.get(container)._byRowId.get(rowId); } - @Nullable - public DataColor getColorForLabel(Container container, String label) - { - return label == null ? null : CACHE.get(container)._byLabel.get(label); - } - public void clearCache(Container c) { CACHE.remove(c); From 6d73e9b3e4f5407d0b10785e1158ff5b9d962adc Mon Sep 17 00:00:00 2001 From: XingY Date: Tue, 21 Jul 2026 20:08:26 -0700 Subject: [PATCH 06/11] Use ExpMaterialColor as field name. Exclude color from dropdown. --- api/src/org/labkey/api/exp/query/ExpMaterialTable.java | 2 +- .../schemas/dbscripts/postgresql/exp-26.007-26.008.sql | 4 ++-- experiment/resources/schemas/exp.xml | 4 ++-- .../src/org/labkey/experiment/ExperimentModule.java | 4 ++-- .../src/org/labkey/experiment/api/ExpMaterialImpl.java | 4 ++-- .../labkey/experiment/api/ExpMaterialTableImpl.java | 6 +++--- experiment/src/org/labkey/experiment/api/Material.java | 10 +++++----- .../labkey/experiment/api/SampleTypeServiceImpl.java | 3 +-- .../experiment/api/SampleTypeUpdateServiceDI.java | 2 +- 9 files changed, 19 insertions(+), 20 deletions(-) diff --git a/api/src/org/labkey/api/exp/query/ExpMaterialTable.java b/api/src/org/labkey/api/exp/query/ExpMaterialTable.java index 36a535f0523..66f1e1b3d68 100644 --- a/api/src/org/labkey/api/exp/query/ExpMaterialTable.java +++ b/api/src/org/labkey/api/exp/query/ExpMaterialTable.java @@ -66,7 +66,7 @@ enum Column RunId, // database table only RunApplication, RunApplicationOutput, - SampleColor, + ExpMaterialColor, SampleSet, SampleState, SourceApplicationId, // database table only diff --git a/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql b/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql index 933872d668c..631d28f2ac8 100644 --- a/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql +++ b/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql @@ -20,8 +20,8 @@ CREATE TABLE exp.DataColors CONSTRAINT UQ_DataColors_Label UNIQUE (Container, Label) ); -ALTER TABLE exp.Material ADD COLUMN SampleColor INT; -ALTER TABLE exp.Material ADD CONSTRAINT FK_Material_SampleColor FOREIGN KEY (SampleColor) REFERENCES exp.DataColors (RowId); +ALTER TABLE exp.Material ADD COLUMN ExpMaterialColor INT; +ALTER TABLE exp.Material ADD CONSTRAINT FK_Material_ExpMaterialColor FOREIGN KEY (ExpMaterialColor) REFERENCES exp.DataColors (RowId); CREATE TABLE exp.DataTypeColorExclusion ( diff --git a/experiment/resources/schemas/exp.xml b/experiment/resources/schemas/exp.xml index fddc45f11b3..dc4a51480e9 100644 --- a/experiment/resources/schemas/exp.xml +++ b/experiment/resources/schemas/exp.xml @@ -295,7 +295,7 @@ Represents the status of the sample - + The color assigned to this individual sample @@ -1050,7 +1050,7 @@ - + diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 9c9958442f5..4a47bb8e3c0 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -803,8 +803,8 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM { Long archivedColorCount = new SqlSelector(schema, new SQLFragment("SELECT COUNT(*) FROM exp.datacolors WHERE archived = " + schema.getSqlDialect().getBooleanTRUE())).getObject(Long.class); sampleColorMetrics.put("archivedColorCount", archivedColorCount); - sampleColorMetrics.put("samplesWithColorCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.material WHERE samplecolor IS NOT NULL").getObject(Long.class)); - sampleColorMetrics.put("samplesWithArchivedColorCount", new SqlSelector(schema, new SQLFragment("SELECT COUNT(*) FROM exp.material m JOIN exp.datacolors dc ON m.samplecolor = dc.rowid WHERE dc.archived = " + schema.getSqlDialect().getBooleanTRUE())).getObject(Long.class)); + sampleColorMetrics.put("samplesWithColorCount", new SqlSelector(schema, "SELECT COUNT(*) FROM exp.material WHERE expmaterialcolor IS NOT NULL").getObject(Long.class)); + sampleColorMetrics.put("samplesWithArchivedColorCount", new SqlSelector(schema, new SQLFragment("SELECT COUNT(*) FROM exp.material m JOIN exp.datacolors dc ON m.expmaterialcolor = dc.rowid WHERE dc.archived = " + schema.getSqlDialect().getBooleanTRUE())).getObject(Long.class)); sampleColorMetrics.put("sampleTypesWithColorsEnabledCount", new SqlSelector(schema, new SQLFragment( "SELECT COUNT(*) FROM exp.materialsource ms WHERE EXISTS (" + diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java index c2db27a6757..152dd004add 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialImpl.java @@ -194,13 +194,13 @@ public void setSampleStateId(Long stateId) @Override public Long getSampleColorId() { - return _object.getSampleColor(); + return _object.getExpMaterialColor(); } @Override public void setSampleColorId(Long colorId) { - _object.setSampleColor(colorId); + _object.setExpMaterialColor(colorId); } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index f2a628375e3..56bc2c481c3 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -597,7 +597,7 @@ protected ContainerFilter getLookupContainerFilter() { return createPropertiesColumn(alias); } - case SampleColor -> + case ExpMaterialColor -> { boolean colorsEnabled = colorsEnabled(getContainer()); var ret = wrapColumn(alias, _rootTable.getColumn(column.name())); @@ -886,9 +886,9 @@ protected void populateColumns() addColumn(SampleState); if (isStatusEnabled(getContainer())) defaultCols.add(SampleState.fieldKey()); - addColumn(SampleColor); + addColumn(ExpMaterialColor); if (colorsEnabled(getContainer())) - defaultCols.add(SampleColor.fieldKey()); + defaultCols.add(ExpMaterialColor.fieldKey()); // TODO is this a real Domain??? if (st != null && !"urn:lsid:labkey.com:SampleSource:Default".equals(st.getDomain().getTypeURI())) diff --git a/experiment/src/org/labkey/experiment/api/Material.java b/experiment/src/org/labkey/experiment/api/Material.java index ca342e17891..8a4f1a15af0 100644 --- a/experiment/src/org/labkey/experiment/api/Material.java +++ b/experiment/src/org/labkey/experiment/api/Material.java @@ -35,7 +35,7 @@ public class Material extends RunItem private Long rootMaterialRowId; private String aliquotedFromLSID; private Long sampleState; - private Long sampleColor; + private Long expMaterialColor; private Date materialExpDate; private Double storedAmount; @@ -113,14 +113,14 @@ public void setSampleState(Long sampleState) this.sampleState = sampleState; } - public Long getSampleColor() + public Long getExpMaterialColor() { - return sampleColor; + return expMaterialColor; } - public void setSampleColor(Long sampleColor) + public void setExpMaterialColor(Long expMaterialColor) { - this.sampleColor = sampleColor; + this.expMaterialColor = expMaterialColor; } public Integer getAliquotCount() diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index 10e14e645f9..fa8cba6f9c8 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -212,8 +212,7 @@ public String getHandlerType() @Override public boolean isColorInUse(Container container, long colorRowId) { - // Colors are referenced by samples across folders, so this is intentionally not container-scoped. - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.SampleColor.name()), colorRowId); + SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("ExpMaterialColor"), colorRowId); return new TableSelector(ExperimentServiceImpl.get().getTinfoMaterial(), filter, null).exists(); } diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index 968af990932..ddecc483e2a 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -1309,7 +1309,7 @@ public DataIterator getDataIterator(DataIteratorContext context) continue; if (isExpMaterialColumn(SampleState, name)) continue; - if (isExpMaterialColumn(SampleColor, name)) + if (isExpMaterialColumn(ExpMaterialColor, name)) continue; if (isExpMaterialColumn(MaterialExpDate, name)) continue; From 791be6b463d3216f37c768a601fe5740116449b6 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 22 Jul 2026 09:52:47 -0700 Subject: [PATCH 07/11] add experimental flag --- api/src/org/labkey/api/exp/api/ExperimentService.java | 4 +++- .../schemas/dbscripts/postgresql/exp-26.007-26.008.sql | 1 + experiment/resources/schemas/exp.xml | 1 + .../src/org/labkey/experiment/ExperimentModule.java | 3 +++ .../org/labkey/experiment/api/ExpMaterialTableImpl.java | 6 +++--- .../org/labkey/experiment/api/ExperimentServiceImpl.java | 8 ++++---- 6 files changed, 15 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 86155933550..3157a60fab3 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -134,6 +134,8 @@ public interface ExperimentService extends ExperimentRunTypeSource String EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE = "org.labkey.experiment.api.SampleTypeUpdateServiceDI#ALLOW_ROW_ID_SAMPLE_MERGE"; + String EXPERIMENTAL_SAMPLE_COLORS = "org.labkey.api.exp.api.ExperimentService#SAMPLE_COLORS"; + int SIMPLE_PROTOCOL_FIRST_STEP_SEQUENCE = 1; int SIMPLE_PROTOCOL_CORE_STEP_SEQUENCE = 10; int SIMPLE_PROTOCOL_EXTRA_STEP_SEQUENCE = 15; @@ -1150,7 +1152,7 @@ List getExpProtocolsWithParameterValue( boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion dataType, @Nullable Collection disabledColorRowIds, @NotNull Container container, User user); - @NotNull Set updateColorDataTypeExclusions(long colorRowId, DataTypeForExclusion dataType, @Nullable Collection newlyDisabledDataTypeIds, @Nullable Collection newlyEnabledDataTypeIds, User user); + @NotNull Set updateColorDataTypeExclusions(long colorRowId, DataTypeForExclusion dataType, @Nullable Collection newlyDisabledDataTypeIds, @Nullable Collection newlyEnabledDataTypeIds, @NotNull Container container, User user); void removeDataColorExclusionsForColor(long colorRowId); diff --git a/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql b/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql index 631d28f2ac8..6d501eef379 100644 --- a/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql +++ b/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql @@ -26,6 +26,7 @@ ALTER TABLE exp.Material ADD CONSTRAINT FK_Material_ExpMaterialColor FOREIGN KEY CREATE TABLE exp.DataTypeColorExclusion ( RowId SERIAL NOT NULL, + Container ENTITYID NOT NULL, DataTypeRowId INT NOT NULL, DataType VARCHAR(20) NOT NULL, ColorRowId INT NOT NULL, diff --git a/experiment/resources/schemas/exp.xml b/experiment/resources/schemas/exp.xml index dc4a51480e9..e0ca8b900b3 100644 --- a/experiment/resources/schemas/exp.xml +++ b/experiment/resources/schemas/exp.xml @@ -1275,6 +1275,7 @@ + diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 4a47bb8e3c0..5cbce75ec7d 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -296,6 +296,8 @@ protected void init() "Support for querying lineage of experiment objects", false, true); OptionalFeatureService.get().addExperimentalFeatureFlag(ExperimentService.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE, "Allow RowId to be accepted when merging samples or data class data", "If the incoming data includes a RowId column we will allow the column but ignore it's values.", false, true); + OptionalFeatureService.get().addExperimentalFeatureFlag(ExperimentService.EXPERIMENTAL_SAMPLE_COLORS, "Sample Colors", + "Enable assigning custom colors to individual samples, with an app-level color palette configurable per sample type.", false); RoleManager.registerPermission(new DesignVocabularyPermission(), true); RoleManager.registerRole(new SampleTypeDesignerRole()); @@ -1218,6 +1220,7 @@ public JSONObject getPageContextJson(ContainerUser context) { JSONObject json = super.getPageContextJson(context); json.put(SAMPLE_FILES_TABLE, OptionalFeatureService.get().isFeatureEnabled(SAMPLE_FILES_TABLE)); + json.put("SampleColors", OptionalFeatureService.get().isFeatureEnabled(ExperimentService.EXPERIMENTAL_SAMPLE_COLORS)); return json; } } diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 56bc2c481c3..87f2e99301e 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -118,6 +118,7 @@ import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.ReadPermission; import org.labkey.api.security.permissions.UpdatePermission; +import org.labkey.api.settings.OptionalFeatureService; import org.labkey.api.test.TestWhen; import org.labkey.api.util.ContextListener; import org.labkey.api.util.GUID; @@ -771,11 +772,10 @@ private static boolean isStatusEnabled(Container c) return SampleStatusService.get().supportsSampleStatus() && !SampleStatusService.get().getAllProjectStates(c).isEmpty(); } - // The SampleColor system field is shown only when at least one sample color is enabled for the sample type - // (or, for the non-type-specific "All Samples" table, when any active color is defined in the container). private boolean colorsEnabled(Container c) { - // TODO: check SampleManagement module + if (!OptionalFeatureService.get().isFeatureEnabled(ExperimentService.EXPERIMENTAL_SAMPLE_COLORS)) + return false; if (_ss != null) return !ExperimentService.get().getActiveDataTypeColors(c, ExperimentService.DataTypeForExclusion.SampleType, _ss.getRowId()).isEmpty(); return !DataColorManager.getInstance().getActiveColors(c).isEmpty(); diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index b6c55364f62..2b19b02d2f2 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9166,6 +9166,7 @@ public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion d for (Long colorRowId : toAdd) { Map fields = new HashMap<>(); + fields.put("Container", container.getId()); fields.put("DataTypeRowId", dataTypeId); fields.put("DataType", dataType.name()); fields.put("ColorRowId", colorRowId); @@ -9198,7 +9199,7 @@ public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion d } @Override - public @NotNull Set updateColorDataTypeExclusions(long colorRowId, DataTypeForExclusion dataType, @Nullable Collection newlyDisabledDataTypeIds, @Nullable Collection newlyEnabledDataTypeIds, User user) + public @NotNull Set updateColorDataTypeExclusions(long colorRowId, DataTypeForExclusion dataType, @Nullable Collection newlyDisabledDataTypeIds, @Nullable Collection newlyEnabledDataTypeIds, @NotNull Container container, User user) { Set toAdd = newlyDisabledDataTypeIds == null ? new HashSet<>() : new HashSet<>(newlyDisabledDataTypeIds); Set toRemove = newlyEnabledDataTypeIds == null ? new HashSet<>() : new HashSet<>(newlyEnabledDataTypeIds); @@ -9219,6 +9220,7 @@ public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion d for (Long dataTypeId : toAdd) { Map fields = new HashMap<>(); + fields.put("Container", container.getId()); fields.put("DataTypeRowId", dataTypeId); fields.put("DataType", dataType.name()); fields.put("ColorRowId", colorRowId); @@ -9267,9 +9269,7 @@ public void removeContainerDataColorExclusions(String containerId) SqlExecutor executor = new SqlExecutor(getExpSchema()); SQLFragment delExclusions = new SQLFragment("DELETE FROM ") .append(getTinfoDataTypeColorExclusion()) - .append(" WHERE ColorRowId IN (SELECT RowId FROM ") - .append(getTinfoDataColors()) - .append(" WHERE Container = ?)").add(containerId); + .append(" WHERE Container = ?").add(containerId); executor.execute(delExclusions); SQLFragment delColors = new SQLFragment("DELETE FROM ") From 8ddb5f9a30fb4f8411d39f34ec26a7600160de49 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 22 Jul 2026 10:11:20 -0700 Subject: [PATCH 08/11] fix build --- experiment/src/org/labkey/experiment/ExperimentModule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 5cbce75ec7d..20695ea73e9 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -297,7 +297,7 @@ protected void init() OptionalFeatureService.get().addExperimentalFeatureFlag(ExperimentService.EXPERIMENTAL_FEATURE_ALLOW_ROW_ID_MERGE, "Allow RowId to be accepted when merging samples or data class data", "If the incoming data includes a RowId column we will allow the column but ignore it's values.", false, true); OptionalFeatureService.get().addExperimentalFeatureFlag(ExperimentService.EXPERIMENTAL_SAMPLE_COLORS, "Sample Colors", - "Enable assigning custom colors to individual samples, with an app-level color palette configurable per sample type.", false); + "Enable assigning custom colors to individual samples, with an app-level color palette configurable per sample type.", false, true); RoleManager.registerPermission(new DesignVocabularyPermission(), true); RoleManager.registerRole(new SampleTypeDesignerRole()); From 73031e1ef3dafbe907a5fc1e81e6eb929753c429 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 22 Jul 2026 12:42:18 -0700 Subject: [PATCH 09/11] fix audit --- .../api/exp/api/SampleTypeDomainKind.java | 5 +++- .../labkey/api/exp/api/SampleTypeService.java | 4 +-- .../api/query/AbstractQueryUpdateService.java | 7 ++--- .../experiment/api/SampleTypeServiceImpl.java | 27 +++++++++++++------ 4 files changed, 29 insertions(+), 14 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java index f6e3124ed11..2724d303d99 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java @@ -535,6 +535,8 @@ public Domain createDomain(GWTDomain domain, @Nullable Sa Map> aliases = null; List excludedContainerIds = null; List excludedDashboardContainerIds = null; + List excludedSampleColorIds = null; + if (arguments != null) { @@ -556,12 +558,13 @@ public Domain createDomain(GWTDomain domain, @Nullable Sa aliases = arguments.getImportAliases(); excludedContainerIds = arguments.getExcludedContainerIds(); excludedDashboardContainerIds = arguments.getExcludedDashboardContainerIds(); + excludedSampleColorIds = arguments.getDisabledSampleColorRowIds(); } ExpSampleType st; try { st = SampleTypeService.get().createSampleType(container, user, name, description, properties, indices, idCol1, idCol2, idCol3, parentCol, nameExpression, aliquotNameExpression, - templateInfo, aliases, labelColor, metricUnit, autoLinkTargetContainer, autoLinkCategory, category, domain.getDisabledSystemFields(), excludedContainerIds, excludedDashboardContainerIds, arguments != null ? arguments.getAuditRecordMap() : null); + templateInfo, aliases, labelColor, metricUnit, autoLinkTargetContainer, autoLinkCategory, category, domain.getDisabledSystemFields(), excludedContainerIds, excludedDashboardContainerIds, excludedSampleColorIds, arguments != null ? arguments.getAuditRecordMap() : null); } catch (ExperimentException e) { diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index 24b0e50404a..83f1190cddc 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -123,7 +123,7 @@ static void setInstance(SampleTypeService impl) Map getSampleTypesForRoles(Container container, ContainerFilter filter, ExpProtocol.ApplicationType type); - void auditSampleColorExclusion(Container container, long sampleTypeRowId, User user); + void auditSampleColorExclusion(Container container, long sampleTypeRowId, @Nullable String auditUserComment, User user); /** * Create a new SampleType with the provided properties. @@ -152,7 +152,7 @@ ExpSampleType createSampleType(Container container, User user, String name, Stri ExpSampleType createSampleType(Container c, User u, String name, String description, List properties, List indices, int idCol1, int idCol2, int idCol3, int parentCol, String nameExpression, String aliquotNameExpression, @Nullable TemplateInfo templateInfo, @Nullable Map> importAliases, @Nullable String labelColor, @Nullable String metricUnit, @Nullable Container autoLinkTargetContainer, @Nullable String autoLinkCategory, @Nullable String category, @Nullable List disabledSystemField, - @Nullable List excludedContainerIds, @Nullable List excludedDashboardContainerIds, @Nullable Map changeDetails) + @Nullable List excludedContainerIds, @Nullable List excludedDashboardContainerIds, @Nullable List excludedSampleColorIds, @Nullable Map changeDetails) throws ExperimentException; @NotNull diff --git a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java index eef0416412b..31eaf3efbc3 100644 --- a/api/src/org/labkey/api/query/AbstractQueryUpdateService.java +++ b/api/src/org/labkey/api/query/AbstractQueryUpdateService.java @@ -596,7 +596,8 @@ protected DataIteratorBuilder _toDataIteratorBuilder(String debugName, List> _insertRowsUsingInsertRow(User user, Container container, List> rows, BatchValidationException errors, Map extraScriptContext) + protected List> _insertRowsUsingInsertRow(User user, Container container, List> rows, BatchValidationException errors, + @Nullable Map configParameters, Map extraScriptContext) throws DuplicateKeyException, BatchValidationException, QueryUpdateServiceException, SQLException { if (!hasInsertRowsPermission(user)) @@ -664,7 +665,7 @@ else if (SqlDialect.isTransactionException(sqlx) && errors.hasErrors()) if (hasTableScript) getQueryTable().fireBatchTrigger(container, user, TableInfo.TriggerType.INSERT, null, false, errors, extraScriptContext); - addAuditEvent(user, container, QueryService.AuditAction.INSERT, null, result, null, providedValues); + addAuditEvent(user, container, QueryService.AuditAction.INSERT, configParameters, result, null, providedValues); return result; } @@ -715,7 +716,7 @@ public List> insertRows(User user, Container container, List { try { - List> ret = _insertRowsUsingInsertRow(user, container, rows, errors, extraScriptContext); + List> ret = _insertRowsUsingInsertRow(user, container, rows, errors, configParameters, extraScriptContext); afterInsertUpdate(null==ret?0:ret.size(), errors); if (errors.hasErrors()) return null; diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index fa8cba6f9c8..c13d2185b50 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -771,7 +771,7 @@ public ExpSampleTypeImpl createSampleType(Container c, User u, String name, Stri public ExpSampleTypeImpl createSampleType(Container c, User u, String name, String description, List properties, List indices, int idCol1, int idCol2, int idCol3, int parentCol, String nameExpression, String aliquotNameExpression, @Nullable TemplateInfo templateInfo, @Nullable Map> importAliases, @Nullable String labelColor, @Nullable String metricUnit) throws ExperimentException { - return createSampleType(c, u, name, description, properties, indices, idCol1, idCol2, idCol3, parentCol, nameExpression, aliquotNameExpression, templateInfo, importAliases, labelColor, metricUnit, null, null, null, null, null, null, null); + return createSampleType(c, u, name, description, properties, indices, idCol1, idCol2, idCol3, parentCol, nameExpression, aliquotNameExpression, templateInfo, importAliases, labelColor, metricUnit, null, null, null, null, null, null, null, null); } @NotNull @@ -779,7 +779,7 @@ public ExpSampleTypeImpl createSampleType(Container c, User u, String name, Stri public ExpSampleTypeImpl createSampleType(Container c, User u, String name, String description, List properties, List indices, int idCol1, int idCol2, int idCol3, int parentCol, String nameExpression, String aliquotNameExpression, @Nullable TemplateInfo templateInfo, @Nullable Map> importAliases, @Nullable String labelColor, @Nullable String metricUnit, @Nullable Container autoLinkTargetContainer, @Nullable String autoLinkCategory, @Nullable String category, @Nullable List disabledSystemField, - @Nullable List excludedContainerIds, @Nullable List excludedDashboardContainerIds, @Nullable Map changeDetails) + @Nullable List excludedContainerIds, @Nullable List excludedDashboardContainerIds, @Nullable List excludedSampleColorIds, @Nullable Map changeDetails) throws ExperimentException { validateSampleTypeName(c, u, name, false); @@ -968,6 +968,13 @@ public ExpSampleTypeImpl createSampleType(Container c, User u, String name, Stri ExperimentService.get().ensureDataTypeContainerExclusions(ExperimentService.DataTypeForExclusion.DashboardSampleType, excludedDashboardContainerIds, st.getRowId(), u); else ExperimentService.get().ensureDataTypeContainerExclusionsNonAdmin(ExperimentService.DataTypeForExclusion.DashboardSampleType, st.getRowId(), c, u); + if (excludedSampleColorIds != null && !excludedSampleColorIds.isEmpty()) + { + List disabledColorRowIds = excludedSampleColorIds.stream().map(Integer::longValue).toList(); + boolean hasColorChange = ExperimentService.get().ensureDataColorExclusions(st.getRowId(), ExperimentService.DataTypeForExclusion.SampleType, disabledColorRowIds, c, u); + if (hasColorChange) + auditSampleColorExclusion(c, st, null, u); + } transaction.addCommitTask(() -> clearMaterialSourceCache(c), DbScope.CommitTaskOption.IMMEDIATE, POSTCOMMIT, POSTROLLBACK); transaction.addCommitTask(() -> indexSampleType(SampleTypeService.get().getSampleType(domain.getTypeURI()), SearchService.get().defaultTask().getQueue(c, SearchService.PRIORITY.modified)), POSTCOMMIT); @@ -1219,7 +1226,7 @@ public ValidationException updateSampleType(GWTDomain disabledColorRowIds = options.getDisabledSampleColorRowIds().stream().map(Integer::longValue).toList(); boolean hasChange = ExperimentService.get().ensureDataColorExclusions(st.getRowId(), ExperimentService.DataTypeForExclusion.SampleType, disabledColorRowIds, container, user); if (hasChange) - auditSampleColorExclusion(container, st.getRowId(), user); + auditSampleColorExclusion(container, st, auditUserComment, user); } errors = DomainUtil.updateDomainDescriptor(original, update, container, user, hasNameChange, changeDetails.toString(), auditUserComment, oldProps, newProps); @@ -1246,13 +1253,17 @@ public ValidationException updateSampleType(GWTDomain disabled = ExperimentService.get().getDataTypeExcludedColors(ExperimentService.DataTypeForExclusion.SampleType, materialSourceId); - String msg = "Sample color exclusion was updated for sample type (rowId " + materialSourceId + "). " + Set disabled = ExperimentService.get().getDataTypeExcludedColors(ExperimentService.DataTypeForExclusion.SampleType, sampleType.getRowId()); + String msg = "Sample color exclusion was updated for sample type (rowId " + sampleType.getRowId() + "). " + (disabled.isEmpty() ? "All colors enabled." : "Excluded color rowIds: " + StringUtils.join(disabled, ", ") + "."); - AuditTypeEvent event = new AuditTypeEvent(SampleTypeAuditProvider.EVENT_TYPE, container, msg); - AuditLogService.get().addEvent(user, event); + addSampleTypeAuditEvent(user, container, sampleType, msg, auditUserComment, "update colors"); } From 95f568b7d6e153e1c897b7d3e3c3ab3c36e3e881 Mon Sep 17 00:00:00 2001 From: XingY Date: Wed, 22 Jul 2026 14:09:49 -0700 Subject: [PATCH 10/11] add validations --- .../org/labkey/experiment/api/DataColorTable.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/experiment/src/org/labkey/experiment/api/DataColorTable.java b/experiment/src/org/labkey/experiment/api/DataColorTable.java index 674dee5dd32..cac310e1881 100644 --- a/experiment/src/org/labkey/experiment/api/DataColorTable.java +++ b/experiment/src/org/labkey/experiment/api/DataColorTable.java @@ -30,6 +30,7 @@ import java.sql.SQLException; import java.util.Map; +import java.util.regex.Pattern; import static org.labkey.api.util.IntegerUtils.asLong; @@ -64,6 +65,8 @@ public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class row, boolean allowMissing) return StringUtils.isBlank((String) row.get("label")); } + private boolean isInvalidColor(Map row, boolean allowMissing) + { + if (allowMissing && !row.containsKey("color")) + return false; + String color = (String) row.get("color"); + return color == null || !COLOR_PATTERN.matcher(color).matches(); + } + private boolean isDuplicateLabel(String label, Container container, int currentRowId) { for (DataColor color : DataColorManager.getInstance().getColors(container)) @@ -97,6 +108,8 @@ protected Map insertRow(User user, Container container, Map= DataColorManager.MAX_DATA_COLORS) @@ -117,6 +130,8 @@ protected Map updateRow(User user, Container container, Map Date: Thu, 23 Jul 2026 19:57:38 -0700 Subject: [PATCH 11/11] code review changes, bug fixes, add junit tests --- .../postgresql/exp-26.007-26.008.sql | 9 +- .../labkey/experiment/ExperimentModule.java | 5 + .../experiment/api/DataColorManager.java | 24 +- .../labkey/experiment/api/DataColorTable.java | 378 +++++++++++++++++- .../experiment/api/ExpMaterialTableImpl.java | 8 +- .../experiment/api/ExperimentServiceImpl.java | 82 ++-- .../experiment/api/SampleTypeServiceImpl.java | 2 +- .../api/SampleTypeUpdateServiceDI.java | 2 + 8 files changed, 437 insertions(+), 73 deletions(-) diff --git a/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql b/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql index 6d501eef379..c1de3664b4e 100644 --- a/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql +++ b/experiment/resources/schemas/dbscripts/postgresql/exp-26.007-26.008.sql @@ -17,11 +17,14 @@ CREATE TABLE exp.DataColors ModifiedBy INT, CONSTRAINT PK_DataColors PRIMARY KEY (RowId), - CONSTRAINT UQ_DataColors_Label UNIQUE (Container, Label) + CONSTRAINT FK_DataColors_Container FOREIGN KEY (Container) REFERENCES core.Containers (EntityId) ); +CREATE UNIQUE INDEX UQ_DataColors_Label ON exp.DataColors (Container, LOWER(Label)); + ALTER TABLE exp.Material ADD COLUMN ExpMaterialColor INT; ALTER TABLE exp.Material ADD CONSTRAINT FK_Material_ExpMaterialColor FOREIGN KEY (ExpMaterialColor) REFERENCES exp.DataColors (RowId); +CREATE INDEX IX_Material_ExpMaterialColor ON exp.Material (ExpMaterialColor) WHERE ExpMaterialColor IS NOT NULL; CREATE TABLE exp.DataTypeColorExclusion ( @@ -36,5 +39,7 @@ CREATE TABLE exp.DataTypeColorExclusion ModifiedBy INT, CONSTRAINT PK_DataTypeColorExclusion PRIMARY KEY (RowId), - CONSTRAINT UQ_DataTypeColorExclusion UNIQUE (DataTypeRowId, DataType, ColorRowId) + CONSTRAINT UQ_DataTypeColorExclusion UNIQUE (DataTypeRowId, DataType, ColorRowId), + CONSTRAINT FK_DataTypeColorExclusion_Container FOREIGN KEY (Container) REFERENCES core.Containers (EntityId), + CONSTRAINT FK_DataTypeColorExclusion_Color FOREIGN KEY (ColorRowId) REFERENCES exp.DataColors (RowId) ); diff --git a/experiment/src/org/labkey/experiment/ExperimentModule.java b/experiment/src/org/labkey/experiment/ExperimentModule.java index 20695ea73e9..0226497abab 100644 --- a/experiment/src/org/labkey/experiment/ExperimentModule.java +++ b/experiment/src/org/labkey/experiment/ExperimentModule.java @@ -119,6 +119,7 @@ import org.labkey.api.writer.ContainerUser; import org.labkey.experiment.api.DataClassDomainKind; import org.labkey.experiment.api.DataColorManager; +import org.labkey.experiment.api.DataColorTable; import org.labkey.experiment.api.EdgeDiagnosticsTestCase; import org.labkey.experiment.api.ExpDataClassImpl; import org.labkey.experiment.api.ExpDataClassTableImpl; @@ -813,6 +814,9 @@ SELECT COUNT(DISTINCT DD.DomainURI) FROM "SELECT 1 FROM exp.datacolors dc WHERE dc.container = ms.container AND dc.archived = " + schema.getSqlDialect().getBooleanFALSE() + " AND NOT EXISTS (" + "SELECT 1 FROM exp.datatypecolorexclusion e WHERE e.datatype = ? AND e.datatyperowid = ms.rowid AND e.colorrowid = dc.rowid))") .add(ExperimentService.DataTypeForExclusion.SampleType.name())).getObject(Long.class)); + sampleColorMetrics.put("sampleTypesWithColorsDisabledCount", new SqlSelector(schema, new SQLFragment( + "SELECT COUNT(DISTINCT datatyperowid) FROM exp.datatypecolorexclusion WHERE datatype = ?") + .add(ExperimentService.DataTypeForExclusion.SampleType.name())).getObject(Long.class)); } results.put("sampleColors", sampleColorMetrics); @@ -1152,6 +1156,7 @@ public Collection getSummary(Container c) return Set.of( EdgeDiagnosticsTestCase.class, ExperimentController.ContainerScopingTestCase.class, + DataColorTable.TestCase.class, DomainImpl.TestCase.class, DomainPropertyImpl.TestCase.class, ExpDataTableImpl.TestCase.class, diff --git a/experiment/src/org/labkey/experiment/api/DataColorManager.java b/experiment/src/org/labkey/experiment/api/DataColorManager.java index 47180d914ee..cc90c1cca4d 100644 --- a/experiment/src/org/labkey/experiment/api/DataColorManager.java +++ b/experiment/src/org/labkey/experiment/api/DataColorManager.java @@ -11,6 +11,7 @@ import org.labkey.api.cache.CacheManager; import org.labkey.api.collections.LongHashMap; import org.labkey.api.data.Container; +import org.labkey.api.data.ContainerManager; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.Sort; import org.labkey.api.data.TableSelector; @@ -63,7 +64,7 @@ public interface DataColorHandler { String getHandlerType(); - boolean isColorInUse(Container container, long colorRowId); + boolean isColorInUse(long colorRowId); } public void registerHandler(DataColorHandler handler) @@ -74,11 +75,11 @@ public void registerHandler(DataColorHandler handler) _handlers.put(type, handler); } - public boolean isInUse(Container container, long colorRowId) + public boolean isInUse(long colorRowId) { for (DataColorHandler handler : _handlers.values()) { - if (handler.isColorInUse(container, colorRowId)) + if (handler.isColorInUse(colorRowId)) return true; } return false; @@ -96,6 +97,23 @@ public List getActiveColors(Container container) return getColors(container).stream().filter(c -> !c.isArchived()).toList(); } + @NotNull + public List getAllProjectColors(Container container) + { + List colors = new ArrayList<>(getColors(container)); + if (!container.isProject() && container.getProject() != null) + colors.addAll(getColors(container.getProject())); + if (container != ContainerManager.getSharedContainer()) + colors.addAll(getColors(ContainerManager.getSharedContainer())); + return colors; + } + + @NotNull + public List getActiveProjectColors(Container container) + { + return getAllProjectColors(container).stream().filter(c -> !c.isArchived()).toList(); + } + @Nullable public DataColor getColorForRowId(Container container, Long rowId) { diff --git a/experiment/src/org/labkey/experiment/api/DataColorTable.java b/experiment/src/org/labkey/experiment/api/DataColorTable.java index cac310e1881..e989817562f 100644 --- a/experiment/src/org/labkey/experiment/api/DataColorTable.java +++ b/experiment/src/org/labkey/experiment/api/DataColorTable.java @@ -8,28 +8,57 @@ import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.junit.After; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.labkey.api.audit.AuditLogService; +import org.labkey.api.audit.AuditTypeEvent; +import org.labkey.api.collections.CaseInsensitiveHashMap; import org.labkey.api.data.ColumnInfo; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerFilter; +import org.labkey.api.data.ContainerManager; import org.labkey.api.data.DbScope; +import org.labkey.api.data.SQLFragment; import org.labkey.api.data.SimpleFilter; +import org.labkey.api.data.Sort; +import org.labkey.api.data.SqlSelector; +import org.labkey.api.data.TableInfo; import org.labkey.api.data.TableSelector; +import org.labkey.api.exp.api.ExpMaterial; +import org.labkey.api.exp.api.ExpSampleType; +import org.labkey.api.exp.api.ExperimentService; +import org.labkey.api.exp.api.ExperimentService.DataTypeForExclusion; +import org.labkey.api.exp.api.SampleTypeService; import org.labkey.api.exp.query.ExpSchema; +import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; +import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.DefaultQueryUpdateService; import org.labkey.api.query.DuplicateKeyException; import org.labkey.api.query.FilteredTable; import org.labkey.api.query.InvalidKeyException; +import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.query.QueryUpdateServiceException; +import org.labkey.api.query.SchemaKey; +import org.labkey.api.query.UserSchema; import org.labkey.api.query.ValidationException; import org.labkey.api.security.User; import org.labkey.api.security.UserPrincipal; import org.labkey.api.security.permissions.AdminPermission; import org.labkey.api.security.permissions.Permission; import org.labkey.api.security.permissions.ReadPermission; +import org.labkey.api.util.JunitUtil; +import org.labkey.api.util.TestContext; +import org.labkey.experiment.SampleTypeAuditProvider; import java.sql.SQLException; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Map; +import java.util.Set; import java.util.regex.Pattern; import static org.labkey.api.util.IntegerUtils.asLong; @@ -66,6 +95,7 @@ public boolean hasPermission(@NotNull UserPrincipal user, @NotNull Class insertRow(User user, Container container, Map row) throws DuplicateKeyException, ValidationException, QueryUpdateServiceException, SQLException + // Shared insert/update validation. On insert the label and color are required and a duplicate label is always + // rejected; on update only the provided fields are validated, and the duplicate check runs (excluding the row + // itself) only when the label is being changed. The 200-color cap is insert-only and stays in insertRow. + private void validateColor(Map row, Container container, boolean isInsert) throws QueryUpdateServiceException { - if (isBlankLabel(row, false)) + boolean allowMissing = !isInsert; + if (isBlankLabel(row, allowMissing)) throw new QueryUpdateServiceException("Label cannot be blank."); - if (isInvalidColor(row, false)) + String label = (String) row.get("label"); + if (label != null && label.length() > MAX_LABEL_LENGTH) + throw new QueryUpdateServiceException("Label may not exceed " + MAX_LABEL_LENGTH + " characters."); + if (isInvalidColor(row, allowMissing)) throw new QueryUpdateServiceException("Color must be a 6-digit hex value (e.g. #1a2b3c)."); - if (isDuplicateLabel(String.valueOf(row.get("label")), container, -1)) - throw new QueryUpdateServiceException("Label '" + row.get("label") + "' is already in use."); + if (isInsert || row.containsKey("label")) + { + int currentRowId = isInsert ? -1 : (int) row.get("rowId"); + if (isDuplicateLabel(String.valueOf(row.get("label")), container, currentRowId)) + throw new QueryUpdateServiceException("Label '" + row.get("label") + "' is already in use."); + } + } + + @Override + protected Map insertRow(User user, Container container, Map row) throws DuplicateKeyException, ValidationException, QueryUpdateServiceException, SQLException + { + validateColor(row, container, true); if (getColorCount(container) >= DataColorManager.MAX_DATA_COLORS) throw new QueryUpdateServiceException("Cannot add more than " + DataColorManager.MAX_DATA_COLORS + " colors."); @@ -128,13 +174,7 @@ protected Map insertRow(User user, Container container, Map updateRow(User user, Container container, Map row, @NotNull Map oldRow, boolean allowOwner, boolean retainCreation) throws InvalidKeyException, ValidationException, QueryUpdateServiceException, SQLException { - if (isBlankLabel(row, true)) - throw new QueryUpdateServiceException("Label cannot be blank."); - if (isInvalidColor(row, true)) - throw new QueryUpdateServiceException("Color must be a 6-digit hex value (e.g. #1a2b3c)."); - int rowId = (int) row.get("rowId"); - if (row.containsKey("label") && isDuplicateLabel(String.valueOf(row.get("label")), container, rowId)) - throw new QueryUpdateServiceException("Label '" + row.get("label") + "' is already in use."); + validateColor(row, container, false); Map updated; try (DbScope.Transaction tx = ExperimentServiceImpl.getExpSchema().getScope().ensureTransaction()) @@ -150,19 +190,325 @@ protected Map updateRow(User user, Container container, Map deleteRow(User user, Container container, Map oldRowMap) throws InvalidKeyException, QueryUpdateServiceException, SQLException { long rowId = asLong(oldRowMap.get("rowId")); - if (DataColorManager.getInstance().isInUse(container, rowId)) + if (DataColorManager.getInstance().isInUse(rowId)) throw new QueryUpdateServiceException("This color can't be deleted because it is in use."); Map deleted; try (DbScope.Transaction tx = ExperimentServiceImpl.getExpSchema().getScope().ensureTransaction()) { - deleted = super.deleteRow(user, container, oldRowMap); - // Also drop any per-data-type exclusion rows that reference this color. + // Drop the per-data-type exclusion rows that reference this color BEFORE deleting the color itself: + // exp.DataTypeColorExclusion.ColorRowId has a (RESTRICT) FK to exp.DataColors, so the color row can't + // be removed while exclusion rows still point at it. ExperimentServiceImpl.get().removeDataColorExclusionsForColor(rowId); + deleted = super.deleteRow(user, container, oldRowMap); tx.addCommitTask(() -> DataColorManager.getInstance().clearCache(container), DbScope.CommitTaskOption.IMMEDIATE, DbScope.CommitTaskOption.POSTCOMMIT); tx.commit(); } return deleted; } } + + /** + * Integration tests for the Custom Sample Colors server logic: the {@code exp.DataColors} QueryUpdateService + * validation (above), the {@code exp.DataTypeColorExclusion} service methods, create-path exclusion persistence + + * audit, and orphan cleanup on color / sample-type / container deletion. Registered in + * {@code ExperimentModule.getIntegrationTests()}. + */ + @SuppressWarnings("JUnitMalformedDeclaration") + public static class TestCase extends Assert + { + private Container _c; + private User _user; + + @Before + public void setUp() + { + JunitUtil.deleteTestContainer(); + _c = JunitUtil.getTestContainer(); + _user = TestContext.get().getUser(); + } + + @After + public void tearDown() + { + JunitUtil.deleteTestContainer(); + } + + // ---- helpers -------------------------------------------------------- + + private QueryUpdateService colorQus(Container c) + { + UserSchema schema = QueryService.get().getUserSchema(_user, c, ExpSchema.SCHEMA_NAME); + return schema.getTable(ExpSchema.TableType.DataColors.name()).getUpdateService(); + } + + private static Map colorRow(String label, String color, boolean archived) + { + Map row = new CaseInsensitiveHashMap<>(); + row.put("Label", label); + row.put("Color", color); + row.put("Archived", archived); + return row; + } + + private long insertColor(Container c, String label, String color, boolean archived) throws Exception + { + BatchValidationException errors = new BatchValidationException(); + List> inserted = colorQus(c).insertRows(_user, c, List.of(colorRow(label, color, archived)), errors, null, null); + if (errors.hasErrors()) + throw errors; + return ((Number) new CaseInsensitiveHashMap<>(inserted.get(0)).get("RowId")).longValue(); + } + + private long insertColor(String label, String color, boolean archived) throws Exception + { + return insertColor(_c, label, color, archived); + } + + private void deleteColor(long rowId) throws Exception + { + colorQus(_c).deleteRows(_user, _c, List.of(CaseInsensitiveHashMap.of("RowId", rowId)), null, null); + } + + private void assertColorInsertFails(Map row, String expectedFragment) + { + try + { + BatchValidationException errors = new BatchValidationException(); + colorQus(_c).insertRows(_user, _c, List.of(row), errors, null, null); + if (errors.hasErrors()) + throw errors; + fail("Expected color insert to be rejected: " + row); + } + catch (Exception e) + { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue("Unexpected error message: " + e.getMessage(), msg.contains(expectedFragment.toLowerCase())); + } + } + + private ExpSampleType createSampleType(String name) throws Exception + { + List props = new ArrayList<>(); + props.add(new GWTPropertyDescriptor("Name", "string")); + return SampleTypeService.get().createSampleType(_c, _user, name, null, props, Collections.emptyList(), -1, -1, -1, -1, null); + } + + private void insertSample(ExpSampleType st, String name) throws Exception + { + UserSchema schema = QueryService.get().getUserSchema(_user, _c, SchemaKey.fromParts("Samples")); + QueryUpdateService qus = schema.getTable(st.getName()).getUpdateService(); + BatchValidationException errors = new BatchValidationException(); + qus.insertRows(_user, _c, List.of(CaseInsensitiveHashMap.of("Name", name)), errors, null, null); + if (errors.hasErrors()) + throw errors; + } + + private long countInContainer(TableInfo table, String containerId) + { + SQLFragment sql = new SQLFragment("SELECT COUNT(*) FROM ").append(table).append(" WHERE Container = ?").add(containerId); + return new SqlSelector(ExperimentServiceImpl.getExpSchema(), sql).getObject(Long.class); + } + + // ---- DataColorTable QUS validation ---------------------------------- + + @Test + public void testColorValidation() throws Exception + { + long red = insertColor("Red", "#ff0000", false); + assertTrue(red > 0); + + assertColorInsertFails(colorRow("", "#00ff00", false), "blank"); // blank label + assertColorInsertFails(colorRow("BadHex", "red", false), "hex"); // not hex + assertColorInsertFails(colorRow("BadHex2", "#ff00", false), "hex"); // too short + assertColorInsertFails(colorRow("BadHex3", "#GGGGGG", false), "hex"); // non-hex chars + assertColorInsertFails(colorRow("RED", "#0000ff", false), "already in use"); // case-insensitive dup + } + + @Test + public void testColorCapEnforced() throws Exception + { + List> rows = new ArrayList<>(); + for (int i = 0; i <= DataColorManager.MAX_DATA_COLORS; i++) // MAX + 1 rows + rows.add(colorRow("Color" + i, String.format("#0000%02x", i % 256), false)); + + try + { + BatchValidationException errors = new BatchValidationException(); + colorQus(_c).insertRows(_user, _c, rows, errors, null, null); + if (errors.hasErrors()) + throw errors; + fail("Expected the " + DataColorManager.MAX_DATA_COLORS + "-color cap to be enforced"); + } + catch (Exception e) + { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue("Unexpected error message: " + e.getMessage(), msg.contains("more than")); + } + } + + @Test + public void testInUseDeleteGuard() throws Exception + { + long red = insertColor("InUse", "#123456", false); + ExpSampleType st = createSampleType("ColorInUseST"); + insertSample(st, "s1"); + + ExpMaterial m = st.getSample(_c, "s1"); + m.setSampleColorId(red); + m.save(_user); + assertTrue("color should be reported in use", DataColorManager.getInstance().isInUse(red)); + + try + { + deleteColor(red); + fail("Expected in-use color delete to be rejected"); + } + catch (Exception e) + { + String msg = e.getMessage() == null ? "" : e.getMessage().toLowerCase(); + assertTrue("Unexpected error message: " + e.getMessage(), msg.contains("in use")); + } + + // clear the reference and the delete should now succeed + m.setSampleColorId(null); + m.save(_user); + assertFalse(DataColorManager.getInstance().isInUse(red)); + deleteColor(red); + assertNull(DataColorManager.getInstance().getColorForRowId(_c, red)); + } + + // ---- exclusion service methods -------------------------------------- + + @Test + public void testExclusionDeltaAndReads() throws Exception + { + ExperimentService svc = ExperimentService.get(); + ExpSampleType stA = createSampleType("ExclA"); + ExpSampleType stB = createSampleType("ExclB"); + long red = insertColor("Red", "#ff0000", false); + long blue = insertColor("Blue", "#0000ff", false); + long typeA = stA.getRowId(); + long typeB = stB.getRowId(); + + assertTrue(svc.getDataTypeExcludedColors(DataTypeForExclusion.SampleType, typeA).isEmpty()); + + // exclude red for stA + Set affected = svc.updateColorDataTypeExclusions(red, DataTypeForExclusion.SampleType, List.of(typeA), null, _c, _user); + assertEquals(Set.of(typeA), affected); + assertEquals(Set.of(red), svc.getDataTypeExcludedColors(DataTypeForExclusion.SampleType, typeA)); + assertEquals(Set.of(typeA), svc.getDataTypesExcludingColor(DataTypeForExclusion.SampleType, red)); + + // the exclusion row is stamped with the caller's container + String stampedContainer = new SqlSelector(ExperimentServiceImpl.getExpSchema(), + new SQLFragment("SELECT Container FROM ").append(ExperimentServiceImpl.get().getTinfoDataTypeColorExclusion()) + .append(" WHERE ColorRowId = ? AND DataTypeRowId = ?").add(red).add(typeA)).getObject(String.class); + assertEquals(_c.getId(), stampedContainer); + + // idempotent re-add is a no-op + assertTrue(svc.updateColorDataTypeExclusions(red, DataTypeForExclusion.SampleType, List.of(typeA), null, _c, _user).isEmpty()); + + // getActiveDataTypeColors reflects the exclusion for stA only + assertFalse(svc.getActiveDataTypeColors(_c, DataTypeForExclusion.SampleType, typeA).contains(red)); + assertTrue(svc.getActiveDataTypeColors(_c, DataTypeForExclusion.SampleType, typeB).contains(red)); + + // re-enable (delta remove) + affected = svc.updateColorDataTypeExclusions(red, DataTypeForExclusion.SampleType, null, List.of(typeA), _c, _user); + assertEquals(Set.of(typeA), affected); + assertTrue(svc.getDataTypeExcludedColors(DataTypeForExclusion.SampleType, typeA).isEmpty()); + + // ensureDataColorExclusions sets the full disabled set (create-time path) + assertTrue(svc.ensureDataColorExclusions(typeB, DataTypeForExclusion.SampleType, List.of(blue), _c, _user)); + assertEquals(Set.of(blue), svc.getDataTypeExcludedColors(DataTypeForExclusion.SampleType, typeB)); + // ...and is idempotent + assertFalse(svc.ensureDataColorExclusions(typeB, DataTypeForExclusion.SampleType, List.of(blue), _c, _user)); + } + + // ---- create-path persistence + audit -------------------------------- + + @Test + public void testCreateSampleTypeWithExclusionsAndAudit() throws Exception + { + long red = insertColor("Red", "#ff0000", false); + + List props = new ArrayList<>(); + props.add(new GWTPropertyDescriptor("Name", "string")); + ExpSampleType st = SampleTypeService.get().createSampleType(_c, _user, "CreatedWithColors", null, props, + Collections.emptyList(), -1, -1, -1, -1, + null, null, null, null, null, null, // nameExpression..metricUnit + null, null, null, null, // autoLink..disabledSystemField + null, null, List.of((int) red), null); // excludedContainerIds, excludedDashboardContainerIds, excludedSampleColorIds, changeDetails + + assertEquals(Set.of(red), ExperimentService.get().getDataTypeExcludedColors(DataTypeForExclusion.SampleType, st.getRowId())); + assertColorAuditEventWritten(); + } + + @Test + public void testAuditSampleColorExclusion() throws Exception + { + ExpSampleType st = createSampleType("AuditST"); + long red = insertColor("Red", "#ff0000", false); + ExperimentService.get().updateColorDataTypeExclusions(red, DataTypeForExclusion.SampleType, List.of((long) st.getRowId()), null, _c, _user); + SampleTypeService.get().auditSampleColorExclusion(_c, st.getRowId(), "junit comment", _user); + assertColorAuditEventWritten(); + } + + private void assertColorAuditEventWritten() + { + List events = AuditLogService.get().getAuditEvents(_c, _user, SampleTypeAuditProvider.EVENT_TYPE, null, new Sort("-RowId")); + assertTrue("expected a sample-type audit event mentioning color exclusion", + events.stream().anyMatch(e -> e.getComment() != null && e.getComment().contains("Sample color exclusion"))); + } + + // ---- orphan cleanup ------------------------------------------------- + + @Test + public void testNoOrphanExclusionsOnColorDelete() throws Exception + { + ExpSampleType st = createSampleType("OrphanColorST"); + long red = insertColor("Red", "#ff0000", false); + ExperimentService.get().updateColorDataTypeExclusions(red, DataTypeForExclusion.SampleType, List.of((long) st.getRowId()), null, _c, _user); + assertFalse(ExperimentService.get().getDataTypesExcludingColor(DataTypeForExclusion.SampleType, red).isEmpty()); + + deleteColor(red); + assertTrue("exclusion rows for a deleted color must be removed", + ExperimentService.get().getDataTypesExcludingColor(DataTypeForExclusion.SampleType, red).isEmpty()); + } + + @Test + public void testNoOrphanExclusionsOnSampleTypeDelete() throws Exception + { + ExpSampleType st = createSampleType("OrphanTypeST"); + long red = insertColor("Red", "#ff0000", false); + long typeRowId = st.getRowId(); + ExperimentService.get().updateColorDataTypeExclusions(red, DataTypeForExclusion.SampleType, List.of(typeRowId), null, _c, _user); + assertFalse(ExperimentService.get().getDataTypeExcludedColors(DataTypeForExclusion.SampleType, typeRowId).isEmpty()); + + st.delete(_user, null); + assertTrue("exclusion rows for a deleted sample type must be removed", + ExperimentService.get().getDataTypeExcludedColors(DataTypeForExclusion.SampleType, typeRowId).isEmpty()); + } + + @Test + public void testNoOrphanColorsOrExclusionsOnContainerDelete() throws Exception + { + Container child = ContainerManager.createContainer(_c, "SampleColorsChild", _user); + String childId = child.getId(); + + long red = insertColor(child, "ChildRed", "#ff0000", false); + + ExpSampleType st = SampleTypeService.get().createSampleType(child, _user, "ChildST", null, + List.of(new GWTPropertyDescriptor("Name", "string")), Collections.emptyList(), -1, -1, -1, -1, null); + ExperimentService.get().updateColorDataTypeExclusions(red, DataTypeForExclusion.SampleType, List.of((long) st.getRowId()), null, child, _user); + + assertEquals(1L, countInContainer(ExperimentServiceImpl.get().getTinfoDataColors(), childId)); + assertEquals(1L, countInContainer(ExperimentServiceImpl.get().getTinfoDataTypeColorExclusion(), childId)); + + ContainerManager.delete(child, _user); + + assertEquals("no orphaned colors after container delete", 0L, countInContainer(ExperimentServiceImpl.get().getTinfoDataColors(), childId)); + assertEquals("no orphaned exclusions after container delete", 0L, countInContainer(ExperimentServiceImpl.get().getTinfoDataTypeColorExclusion(), childId)); + } + } + } diff --git a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java index 87f2e99301e..176aec9b24e 100644 --- a/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpMaterialTableImpl.java @@ -608,7 +608,7 @@ protected ContainerFilter getLookupContainerFilter() ret.setShownInInsertView(colorsEnabled); ret.setShownInUpdateView(colorsEnabled); ret.setRemapMissingBehavior(SimpleTranslator.RemapMissingBehavior.Error); - ret.setFk(new QueryForeignKey.Builder(getUserSchema(), getSampleStatusLookupContainerFilter()) + ret.setFk(new QueryForeignKey.Builder(getUserSchema(), getMaterialBaseFieldLookupContainerFilter()) .schema(getExpSchema()).table(ExpSchema.TableType.DataColors).display("Label")); return ret; } @@ -622,7 +622,7 @@ protected ContainerFilter getLookupContainerFilter() ret.setShownInInsertView(statusEnabled); ret.setShownInUpdateView(statusEnabled); ret.setRemapMissingBehavior(SimpleTranslator.RemapMissingBehavior.Error); - ret.setFk(new QueryForeignKey.Builder(getUserSchema(), getSampleStatusLookupContainerFilter()) + ret.setFk(new QueryForeignKey.Builder(getUserSchema(), getMaterialBaseFieldLookupContainerFilter()) .schema(getExpSchema()).table(ExpSchema.TableType.SampleStatus).display("Label")); return ret; } @@ -778,7 +778,7 @@ private boolean colorsEnabled(Container c) return false; if (_ss != null) return !ExperimentService.get().getActiveDataTypeColors(c, ExperimentService.DataTypeForExclusion.SampleType, _ss.getRowId()).isEmpty(); - return !DataColorManager.getInstance().getActiveColors(c).isEmpty(); + return !DataColorManager.getInstance().getActiveProjectColors(c).isEmpty(); } private Unit getSampleTypeUnit() @@ -1012,7 +1012,7 @@ protected void populateColumns() addColumn(lineageLookup); } - private ContainerFilter getSampleStatusLookupContainerFilter() + private ContainerFilter getMaterialBaseFieldLookupContainerFilter() { // The default lookup container filter is Current. However, we want to have the default be CurrentPlusProjectAndShared // for the sample status lookup since in the app project context we want to share status definitions across diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 2b19b02d2f2..da6bf7d1a9a 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -9137,54 +9137,63 @@ public String getDisabledDataTypeAuditMsg(DataTypeForExclusion type, List public @NotNull Set getActiveDataTypeColors(@NotNull Container container, DataTypeForExclusion dataType, long dataTypeId) { Set disabled = getDataTypeExcludedColors(dataType, dataTypeId); - return DataColorManager.getInstance().getActiveColors(container).stream() + return DataColorManager.getInstance().getActiveProjectColors(container).stream() .map(c -> (long) c.getRowId()) .filter(rowId -> !disabled.contains(rowId)) .collect(toSet()); } - @Override - public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion dataType, @Nullable Collection disabledColorRowIds, @NotNull Container container, User user) + // Applies a reconciled set of exclusion changes to exp.DataTypeColorExclusion in one transaction: one key column is + // held fixed (fixedColumn = fixedValue), the other varies. Rows in toAdd are inserted; rows in toRemove are deleted. + // Shared by ensureDataColorExclusions (fixes DataTypeRowId, varies ColorRowId) and updateColorDataTypeExclusions + // (fixes ColorRowId, varies DataTypeRowId). The column names are code constants, not caller input. + private void applyExclusionChanges(String fixedColumn, long fixedValue, String varyingColumn, Set toAdd, Set toRemove, DataTypeForExclusion dataType, Container container, User user) { - if (disabledColorRowIds == null) - return false; - - Set previous = getDataTypeExcludedColors(dataType, dataTypeId); - Set updated = new HashSet<>(disabledColorRowIds); - - Set toAdd = new HashSet<>(updated); - toAdd.removeAll(previous); - - Set toRemove = new HashSet<>(previous); - toRemove.removeAll(updated); - - if (toAdd.isEmpty() && toRemove.isEmpty()) - return false; - try (DbScope.Transaction tx = getExpSchema().getScope().ensureTransaction()) { - for (Long colorRowId : toAdd) + for (Long id : toAdd) { Map fields = new HashMap<>(); fields.put("Container", container.getId()); - fields.put("DataTypeRowId", dataTypeId); fields.put("DataType", dataType.name()); - fields.put("ColorRowId", colorRowId); + fields.put(fixedColumn, fixedValue); + fields.put(varyingColumn, id); Table.insert(user, getTinfoDataTypeColorExclusion(), fields); } if (!toRemove.isEmpty()) { SQLFragment sql = new SQLFragment("DELETE FROM ") .append(getTinfoDataTypeColorExclusion()) - .append(" WHERE DataTypeRowId = ? AND DataType = ?") - .add(dataTypeId).add(dataType.name()) - .append(" AND ColorRowId "); + .append(" WHERE ").append(fixedColumn).append(" = ? AND DataType = ?") + .add(fixedValue).add(dataType.name()) + .append(" AND ").append(varyingColumn).append(" "); sql.appendInClause(toRemove, getExpSchema().getSqlDialect()); new SqlExecutor(getExpSchema()).execute(sql); } tx.commit(); } + } + + @Override + public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion dataType, @Nullable Collection disabledColorRowIds, @NotNull Container container, User user) + { + if (disabledColorRowIds == null) + return false; + + Set previous = getDataTypeExcludedColors(dataType, dataTypeId); + Set updated = new HashSet<>(disabledColorRowIds); + Set toAdd = new HashSet<>(updated); + toAdd.removeAll(previous); + + Set toRemove = new HashSet<>(previous); + toRemove.removeAll(updated); + + if (toAdd.isEmpty() && toRemove.isEmpty()) + return false; + + // Fix the sample type; vary the colors being disabled/re-enabled for it. + applyExclusionChanges("DataTypeRowId", dataTypeId, "ColorRowId", toAdd, toRemove, dataType, container, user); return true; } @@ -9215,29 +9224,8 @@ public boolean ensureDataColorExclusions(long dataTypeId, DataTypeForExclusion d if (toAdd.isEmpty() && toRemove.isEmpty()) return Set.of(); - try (DbScope.Transaction tx = getExpSchema().getScope().ensureTransaction()) - { - for (Long dataTypeId : toAdd) - { - Map fields = new HashMap<>(); - fields.put("Container", container.getId()); - fields.put("DataTypeRowId", dataTypeId); - fields.put("DataType", dataType.name()); - fields.put("ColorRowId", colorRowId); - Table.insert(user, getTinfoDataTypeColorExclusion(), fields); - } - if (!toRemove.isEmpty()) - { - SQLFragment sql = new SQLFragment("DELETE FROM ") - .append(getTinfoDataTypeColorExclusion()) - .append(" WHERE ColorRowId = ? AND DataType = ?") - .add(colorRowId).add(dataType.name()) - .append(" AND DataTypeRowId "); - sql.appendInClause(toRemove, getExpSchema().getSqlDialect()); - new SqlExecutor(getExpSchema()).execute(sql); - } - tx.commit(); - } + // Fix the color; vary the sample types it's disabled/re-enabled for. + applyExclusionChanges("ColorRowId", colorRowId, "DataTypeRowId", toAdd, toRemove, dataType, container, user); Set affected = new HashSet<>(toAdd); affected.addAll(toRemove); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index c13d2185b50..1ae70f5b04c 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -210,7 +210,7 @@ public String getHandlerType() } @Override - public boolean isColorInUse(Container container, long colorRowId) + public boolean isColorInUse(long colorRowId) { SimpleFilter filter = new SimpleFilter(FieldKey.fromParts("ExpMaterialColor"), colorRowId); return new TableSelector(ExperimentServiceImpl.get().getTinfoMaterial(), filter, null).exists(); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java index ddecc483e2a..d9e5026d898 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java @@ -185,6 +185,8 @@ public class SampleTypeUpdateServiceDI extends DefaultQueryUpdateService SAMPLE_ALT_IMPORT_NAME_COLS.put("Expiration Date", "MaterialExpDate"); SAMPLE_ALT_IMPORT_NAME_COLS.put("Entered Storage", "Stored"); SAMPLE_ALT_IMPORT_NAME_COLS.put("EnteredStorage", "Stored"); + SAMPLE_ALT_IMPORT_NAME_COLS.put("SampleColor", "ExpSampleColor"); + SAMPLE_ALT_IMPORT_NAME_COLS.put("Sample Color", "ExpSampleColor"); } public enum Options