diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java index 2724d303d99..8a7007c1e4e 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,13 +624,18 @@ public boolean hasNullValues(Domain domain, DomainProperty prop) if (getTotalAndNonBlankSql(domain, prop, allRowsSQL, nonBlankRowsSQL)) { + String scope = prop.getDerivationDataScope(); // 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"); + // GH Issue 1472: Include aliquot rows when checking for a property that is editable for both aliquots and samples + 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; } diff --git a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java index c17a1bebd45..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; @@ -123,6 +124,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 { @@ -756,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); } @@ -874,6 +878,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) { @@ -926,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()