Skip to content

GitHub Issue 925, 949 & 1014 #7532

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
XingY merged 8 commits into from
Mar 31, 2026
Merged

GitHub Issue 925, 949 & 1014 #7532

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
e8fb43a
GitHub Issue 925: Not providing a MVTC column in an assay import thro…
XingY Mar 29, 2026
b93a7b9
GitHub Issue 949 & 1014
XingY Mar 30, 2026
392a75a
null check
XingY Mar 30, 2026
2ed47f4
Merge branch 'develop' into fb_gh925
XingY Mar 30, 2026
0d90b70
Merge branch 'develop' into fb_gh925
XingY Mar 30, 2026
d52152c
code review changes
XingY Mar 30, 2026
4417dcf
api tests
XingY Mar 30, 2026
94a0cf4
Merge branch 'develop' into fb_gh925
XingY Mar 31, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -867,6 +867,7 @@ else if (entry.getKey().equalsIgnoreCase(ProvenanceService.PROVENANCE_INPUT_PROP
if (PropertyType.MULTI_CHOICE == pd.getPropertyType())
{
o = MultiChoice.Converter.getInstance().convert(MultiChoice.Array.class, o);
map.put(pd.getName(), o);
}
else if (o instanceof String)
{
Expand Down
92 changes: 85 additions & 7 deletions api/src/org/labkey/api/exp/property/DomainUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
import org.labkey.api.collections.LongHashMap;
import org.labkey.api.data.ColumnInfo;
import org.labkey.api.data.ColumnRenderPropertiesImpl;
import org.labkey.api.data.CompareType;
import org.labkey.api.data.ConditionalFormat;
import org.labkey.api.data.Container;
import org.labkey.api.data.ContainerFilter;
Expand All @@ -39,8 +40,10 @@
import org.labkey.api.data.NameGenerator;
import org.labkey.api.data.PHI;
import org.labkey.api.data.PropertyStorageSpec;
import org.labkey.api.data.SQLFragment;
import org.labkey.api.data.SchemaTableInfo;
import org.labkey.api.data.SimpleFilter;
import org.labkey.api.data.SqlSelector;
import org.labkey.api.data.TableInfo;
import org.labkey.api.data.TableInfo.IndexDefinition;
import org.labkey.api.data.TableSelector;
Expand All @@ -58,6 +61,7 @@
import org.labkey.api.exp.api.ExperimentService;
import org.labkey.api.exp.api.SampleTypeDomainKind;
import org.labkey.api.exp.api.StorageProvisioner;
import org.labkey.api.exp.query.ExpSchema;
import org.labkey.api.gwt.client.AuditBehaviorType;
import org.labkey.api.gwt.client.DefaultScaleType;
import org.labkey.api.gwt.client.FacetingBehaviorType;
Expand Down Expand Up @@ -87,6 +91,7 @@
import org.labkey.api.util.JdbcUtil;
import org.labkey.api.util.JsonUtil;
import org.labkey.api.util.PageFlowUtil;
import org.labkey.api.util.Pair;
import org.labkey.api.util.StringExpression;
import org.labkey.api.util.StringUtilsLabKey;
import org.labkey.api.view.UnauthorizedException;
Expand Down Expand Up @@ -909,6 +914,8 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT
Map<DomainProperty, Object> defaultValues = new HashMap<>();
Map<DomainProperty, List<Map<String, Object>>> textChoiceValueUpdates = new HashMap<>();

TableInfo domainTable = null;

// and now update properties
for (GWTPropertyDescriptor pd : update.getFields())
{
Expand Down Expand Up @@ -936,7 +943,9 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT

if (old == null)
continue;
List<Map<String, Object>> propTextChoiceValueUpdates = updatePropertyValidators(p, old, pd);
var textChoiceUpdates = updatePropertyValidators(p, old, pd);
List<Map<String, Object>> propTextChoiceValueUpdates = textChoiceUpdates.first;
List<String> deletedValues = textChoiceUpdates.second;
if (propTextChoiceValueUpdates != null && !propTextChoiceValueUpdates.isEmpty())
{
if (PropertyType.MULTI_CHOICE.getTypeUri().equals(old.getRangeURI()) || PropertyType.MULTI_CHOICE.getTypeUri().equals(pd.getRangeURI()))
Expand All @@ -947,6 +956,52 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT
}
textChoiceValueUpdates.put(p, propTextChoiceValueUpdates);
}

// GitHub Issue 949: Text choice value can be deleted if usage is added after loading designer
if (!deletedValues.isEmpty())
{
// using ContainerFilter.EVERYTHING to account for /Shared domains
if (domainTable == null)
domainTable = d.getDomainKind().getTableInfo(user, d.getContainer(), d, ContainerFilter.getUnsafeEverythingFilter());

if (domainTable != null)
{
// if was regular text choice, then we just check the property value
if (TEXT_CHOICE_CONCEPT_URI.equals(old.getConceptURI()))
{
SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(p.getName()), deletedValues, CompareType.IN);
if (new TableSelector(domainTable, filter, null).exists())
{
validationException.addError(new SimpleValidationError("One or more values cannot be removed from the text choice list because they are in use: " + StringUtils.join(deletedValues, ", ")));
return validationException;
}
}
else if (PropertyType.MULTI_CHOICE.getTypeUri().equals(old.getRangeURI()))
{
var column = domainTable.getColumn(p.getName());

if (column != null)
{
var dialect = domainTable.getSchema().getSqlDialect();
SQLFragment deletedArray = new SQLFragment("CAST(? AS TEXT[])").add(deletedValues.toArray(new String[0]));
SQLFragment columnFrag = new SQLFragment().appendIdentifier(column.getAlias());

SQLFragment sql = new SQLFragment("SELECT 1 FROM ")
.append(domainTable)
.append(" WHERE ")
.append(dialect.array_some_in_array(columnFrag, deletedArray));

if (new SqlSelector(domainTable.getSchema().getScope(), sql).exists())
{
validationException.addError(new SimpleValidationError("One or more values cannot be removed from the multi-choice list because they are in use: " + StringUtils.join(deletedValues, ", ")));
return validationException;
}
}
}

}

}
if (old.equals(pd))
continue;

Expand Down Expand Up @@ -999,7 +1054,7 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT
for (Map.Entry<DomainProperty, List<Map<String, Object>>> entry : textChoiceValueUpdates.entrySet())
{
for (Map<String, Object> valueUpdate : entry.getValue())
updateTextChoiceValueRows(d, user, entry.getKey().getName(), valueUpdate, validationException);
updateTextChoiceValueRows(d, user, entry.getKey(), valueUpdate, validationException);
}

// update indices - add missing and drop those that aren't included in domain info
Expand Down Expand Up @@ -1279,10 +1334,12 @@ private static void _copyProperties(DomainProperty to, GWTPropertyDescriptor fro
to.setDerivationDataScope(from.getDerivationDataScope());
}

private static List<Map<String, Object>> updatePropertyValidators(DomainProperty dp, @Nullable GWTPropertyDescriptor oldPd, GWTPropertyDescriptor newPd)
// Returns list of value updates and list of deleted values for text choice validators. Only returns if we have an oldPd to compare to, otherwise we don't know what the deleted values are.
private static @NotNull Pair<List<Map<String, Object>>, List<String>> updatePropertyValidators(DomainProperty dp, @Nullable GWTPropertyDescriptor oldPd, GWTPropertyDescriptor newPd)
{
Map<Long, GWTPropertyValidator> newProps = new LongHashMap<>();
List<Map<String, Object>> valueUpdates = new ArrayList<>();
List<String> deletedValues = new ArrayList<>();

PropertyDescriptor oldPropertyDescriptor = dp.getPropertyDescriptor().clone();
boolean hasChange = false;
Expand Down Expand Up @@ -1325,7 +1382,24 @@ else if (prop == null)
// update any new or changed
for (IPropertyValidator pv : dp.getValidators())
{
boolean change = _copyValidator(pv, newProps.get(pv.getRowId()));
var gpv = newProps.get(pv.getRowId());
if (gpv == null)
continue;

boolean hasExpressionChange = !Objects.equals(pv.getExpressionValue(), gpv.getExpression());
if (hasExpressionChange && PropertyValidatorType.TextChoice.equals(gpv.getType()))
{
List<String> oldValidValues = PropertyService.get().getTextChoiceValidatorOptions(pv);

List<String> newValidValues = PageFlowUtil.splitStringToValues(gpv.getExpression(), '|');
deletedValues = new ArrayList<>(oldValidValues);
deletedValues.removeAll(newValidValues);
// Exclude renamed oldValidValues from deletedValues — their keys in valueUpdates are the old names
for (Map<String, Object> update : valueUpdates)
deletedValues.removeAll(update.keySet());
}
boolean change = _copyValidator(pv, gpv);

hasChange = hasChange || change;
}

Expand All @@ -1337,13 +1411,17 @@ else if (prop == null)
if (hasChange)
dp.setOldPropertyDescriptor(oldPropertyDescriptor); // mark dirty as needed

return oldPd != null ? valueUpdates : null;
return Pair.of(valueUpdates, deletedValues);
}

private static void updateTextChoiceValueRows(Domain domain, User user, String propName, Map<String, Object> valueUpdates, ValidationException errors)
private static void updateTextChoiceValueRows(Domain domain, User user, DomainProperty prop, Map<String, Object> valueUpdates, ValidationException errors)
{
if (domain != null && domain.getDomainKind() != null)
{
String propName = prop.getName();
// GitHub Issue 1014: Text choice value update doesn't update aliquot's aliquot-specific field values
boolean isParentOnlyField = StringUtils.isEmpty(prop.getDerivationDataScope())
|| ExpSchema.DerivationDataScopeType.ParentOnly.name().equalsIgnoreCase(prop.getDerivationDataScope());
// using ContainerFilter.EVERYTHING to account for /Shared domains
TableInfo domainTable = domain.getDomainKind().getTableInfo(user, domain.getContainer(), domain, ContainerFilter.getUnsafeEverythingFilter());
if (domainTable != null && domainTable.getUpdateService() != null)
Expand All @@ -1358,7 +1436,7 @@ private static void updateTextChoiceValueRows(Domain domain, User user, String p
// query for the row PKs of domain rows that have the original text choice value
SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(propName), entry.getKey());
// filter out aliquots for sample type domain
if (domain.getDomainKind() instanceof SampleTypeDomainKind)
if (domain.getDomainKind() instanceof SampleTypeDomainKind && isParentOnlyField)
filter.addCondition(FieldKey.fromParts("IsAliquot"), false);
List<ColumnInfo> columns = new ArrayList<>(domainTable.getPkColumns());
if (domainTable.getContainerFieldKey() != null)
Expand Down
Loading
Loading