From cc3a95fa53c560c3c0825d74d1d3e2f364fe371f Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Tue, 18 Aug 2026 17:05:24 -0700 Subject: [PATCH 1/4] Issue 1472: Update check for allowing a field to be required to exclude or include aliquots depending on field derivation scope --- .../org/labkey/api/exp/api/SampleTypeDomainKind.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java index 2724d303d99..5a246c328d9 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java @@ -47,6 +47,7 @@ import org.labkey.api.exp.property.DomainUtil; import org.labkey.api.exp.query.ExpMaterialTable; import org.labkey.api.exp.query.ExpSampleTypeTable; +import org.labkey.api.exp.query.ExpSchema; import org.labkey.api.exp.query.SamplesSchema; import org.labkey.api.gwt.client.DefaultValueType; import org.labkey.api.gwt.client.model.GWTDomain; @@ -623,11 +624,15 @@ public boolean hasNullValues(Domain domain, DomainProperty prop) if (getTotalAndNonBlankSql(domain, prop, allRowsSQL, nonBlankRowsSQL)) { - // Issue 43754: Don't include aliquot rows in the null value check (see ExpMaterialTableImpl.createColumn for IsAliquot) + String table = domain.getStorageTableName(); SQLFragment nonAliquotRowsSQL = new SQLFragment("SELECT * FROM exp.material WHERE RowId IN (") - .append("SELECT RowId FROM " + getStorageSchemaName() + "." + table) - .append(") AND RootMaterialRowId = RowId"); + .append("SELECT RowId FROM ").append(getStorageSchemaName()).append(".").append(table) + .append(")"); + // Issue 43754: Don't include aliquot rows in the null value check (see ExpMaterialTableImpl.createColumn for IsAliquot) + // GH Issue 1472: Include aliquot rows when checking for a property that is editable for both aliquots and samples + if (ExpSchema.DerivationDataScopeType.ParentOnly.name().equalsIgnoreCase(prop.getDerivationDataScope())) + nonAliquotRowsSQL.append(" AND RootMaterialRowId = RowId"); long totalRows = new SqlSelector(ExperimentService.get().getSchema(), nonAliquotRowsSQL).getRowCount(); long nonBlankRows = new SqlSelector(ExperimentService.get().getSchema(), nonBlankRowsSQL).getRowCount(); From 0d905e0c96deadee160e60fa0df90cf60b8cefbf Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 19 Aug 2026 15:58:12 -0700 Subject: [PATCH 2/4] Handle blank scope == ParentOnly --- .../api/exp/api/SampleTypeDomainKind.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java index 5a246c328d9..8a7007c1e4e 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java @@ -624,17 +624,18 @@ public boolean hasNullValues(Domain domain, DomainProperty prop) if (getTotalAndNonBlankSql(domain, prop, allRowsSQL, nonBlankRowsSQL)) { - - String table = domain.getStorageTableName(); - SQLFragment nonAliquotRowsSQL = new SQLFragment("SELECT * FROM exp.material WHERE RowId IN (") - .append("SELECT RowId FROM ").append(getStorageSchemaName()).append(".").append(table) - .append(")"); + String scope = prop.getDerivationDataScope(); // Issue 43754: Don't include aliquot rows in the null value check (see ExpMaterialTableImpl.createColumn for IsAliquot) // GH Issue 1472: Include aliquot rows when checking for a property that is editable for both aliquots and samples - if (ExpSchema.DerivationDataScopeType.ParentOnly.name().equalsIgnoreCase(prop.getDerivationDataScope())) - nonAliquotRowsSQL.append(" AND RootMaterialRowId = RowId"); + if (StringUtils.isEmpty(scope) || ExpSchema.DerivationDataScopeType.ParentOnly.name().equalsIgnoreCase(scope)) + { + String table = domain.getStorageTableName(); + allRowsSQL = new SQLFragment("SELECT * FROM exp.material WHERE RowId IN (") + .append("SELECT RowId FROM ").append(getStorageSchemaName()).append(".").append(table) + .append(") AND RootMaterialRowId = RowId"); + } - long totalRows = new SqlSelector(ExperimentService.get().getSchema(), nonAliquotRowsSQL).getRowCount(); + long totalRows = new SqlSelector(ExperimentService.get().getSchema(), allRowsSQL).getRowCount(); long nonBlankRows = new SqlSelector(ExperimentService.get().getSchema(), nonBlankRowsSQL).getRowCount(); return totalRows != nonBlankRows; } From d56563fbf6e8d1714690b76b4bf25c7080bdb050 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Wed, 19 Aug 2026 15:58:31 -0700 Subject: [PATCH 3/4] Add server-side check for trying to make a child-only field required --- .../org/labkey/experiment/api/property/DomainImpl.java | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java index c17a1bebd45..518347da8b2 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java @@ -123,6 +123,7 @@ import java.util.concurrent.locks.Lock; import static org.labkey.api.data.ColumnRenderPropertiesImpl.STORAGE_UNIQUE_ID_SEQUENCE_PREFIX; +import static org.labkey.api.exp.query.ExpSchema.DerivationDataScopeType.ChildOnly; public class DomainImpl implements Domain { @@ -874,6 +875,14 @@ else if (null != pdOld) { for (DomainProperty prop : checkRequiredStatus) { + // Issue 46733: A field that is editable for aliquots only is never populated for a sample, so it can + // never be required. Reject it before the blank value check below, which would otherwise report every + // sample row as blank and give a misleading reason. + if (ChildOnly.name().equalsIgnoreCase(prop.getDerivationDataScope())) + { + throw new ChangePropertyDescriptorException("The property \"" + prop.getName() + "\" cannot be required when it is editable for aliquots only."); + } + boolean hasRows = kind.hasNullValues(this, prop); if (hasRows) { From 52a40d7d5cbf087207ebfa5a886312593cb67761 Mon Sep 17 00:00:00 2001 From: labkey-susanh Date: Thu, 20 Aug 2026 14:33:50 -0700 Subject: [PATCH 4/4] Check for changing to a required field from a parent-only field --- .../labkey/experiment/api/property/DomainImpl.java | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java index 518347da8b2..b07d0391a77 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java @@ -83,6 +83,7 @@ import org.labkey.api.exp.property.DomainPropertyAuditProvider; import org.labkey.api.exp.property.DomainTemplate; import org.labkey.api.exp.property.DomainUtil; +import org.labkey.api.exp.query.ExpSchema; import org.labkey.api.gwt.client.model.GWTIndex; import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; import org.labkey.api.query.BatchValidationException; @@ -757,7 +758,9 @@ else if (impl.isNew()) // If this field is newly required, or it's required and we're disabling MV indicators on // it, make sure that all the rows have values for it if ((!impl._pdOld.isRequired() && impl._pd.isRequired()) || - (impl._pd.isRequired() && !impl._pd.isMvEnabled() && impl._pdOld.isMvEnabled())) + (impl._pd.isRequired() && !impl._pd.isMvEnabled() && impl._pdOld.isMvEnabled()) || + (impl._pdOld.isRequired() && derivationDataScopeChanged(impl._pdOld.getDerivationDataScope(), impl._pd.getDerivationDataScope())) + ) { checkRequiredStatus.add(impl); } @@ -935,6 +938,13 @@ else if (!isDomainNew) } } + private boolean derivationDataScopeChanged(String oldScope, String newScope) + { + ExpSchema.DerivationDataScopeType oldType = oldScope == null ? ExpSchema.DerivationDataScopeType.ParentOnly : ExpSchema.DerivationDataScopeType.valueOf(oldScope); + ExpSchema.DerivationDataScopeType newType = newScope == null ? ExpSchema.DerivationDataScopeType.ParentOnly : ExpSchema.DerivationDataScopeType.valueOf(newScope); + return oldType != newType; + } + record CalculatedFieldsUpdate(@Nullable List added, @Nullable List removed, @Nullable List> updated) { public boolean hasChange()