Skip to content

Commit c693a96

Browse files
XingYXingY
authored
GitHub Issue 925, 949 & 1014 (#7532)
1 parent d631e8e commit c693a96

5 files changed

Lines changed: 363 additions & 34 deletions

File tree

‎api/src/org/labkey/api/assay/AbstractAssayTsvDataHandler.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -867,6 +867,7 @@ else if (entry.getKey().equalsIgnoreCase(ProvenanceService.PROVENANCE_INPUT_PROP
867867
if (PropertyType.MULTI_CHOICE == pd.getPropertyType())
868868
{
869869
o = MultiChoice.Converter.getInstance().convert(MultiChoice.Array.class, o);
870+
map.put(pd.getName(), o);
870871
}
871872
else if (o instanceof String)
872873
{

‎api/src/org/labkey/api/exp/property/DomainUtil.java‎

Lines changed: 85 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.labkey.api.collections.LongHashMap;
3131
import org.labkey.api.data.ColumnInfo;
3232
import org.labkey.api.data.ColumnRenderPropertiesImpl;
33+
import org.labkey.api.data.CompareType;
3334
import org.labkey.api.data.ConditionalFormat;
3435
import org.labkey.api.data.Container;
3536
import org.labkey.api.data.ContainerFilter;
@@ -39,8 +40,10 @@
3940
import org.labkey.api.data.NameGenerator;
4041
import org.labkey.api.data.PHI;
4142
import org.labkey.api.data.PropertyStorageSpec;
43+
import org.labkey.api.data.SQLFragment;
4244
import org.labkey.api.data.SchemaTableInfo;
4345
import org.labkey.api.data.SimpleFilter;
46+
import org.labkey.api.data.SqlSelector;
4447
import org.labkey.api.data.TableInfo;
4548
import org.labkey.api.data.TableInfo.IndexDefinition;
4649
import org.labkey.api.data.TableSelector;
@@ -58,6 +61,7 @@
5861
import org.labkey.api.exp.api.ExperimentService;
5962
import org.labkey.api.exp.api.SampleTypeDomainKind;
6063
import org.labkey.api.exp.api.StorageProvisioner;
64+
import org.labkey.api.exp.query.ExpSchema;
6165
import org.labkey.api.gwt.client.AuditBehaviorType;
6266
import org.labkey.api.gwt.client.DefaultScaleType;
6367
import org.labkey.api.gwt.client.FacetingBehaviorType;
@@ -87,6 +91,7 @@
8791
import org.labkey.api.util.JdbcUtil;
8892
import org.labkey.api.util.JsonUtil;
8993
import org.labkey.api.util.PageFlowUtil;
94+
import org.labkey.api.util.Pair;
9095
import org.labkey.api.util.StringExpression;
9196
import org.labkey.api.util.StringUtilsLabKey;
9297
import org.labkey.api.view.UnauthorizedException;
@@ -909,6 +914,8 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT
909914
Map<DomainProperty, Object> defaultValues = new HashMap<>();
910915
Map<DomainProperty, List<Map<String, Object>>> textChoiceValueUpdates = new HashMap<>();
911916

917+
TableInfo domainTable = null;
918+
912919
// and now update properties
913920
for (GWTPropertyDescriptor pd : update.getFields())
914921
{
@@ -936,7 +943,9 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT
936943

937944
if (old == null)
938945
continue;
939-
List<Map<String, Object>> propTextChoiceValueUpdates = updatePropertyValidators(p, old, pd);
946+
var textChoiceUpdates = updatePropertyValidators(p, old, pd);
947+
List<Map<String, Object>> propTextChoiceValueUpdates = textChoiceUpdates.first;
948+
List<String> deletedValues = textChoiceUpdates.second;
940949
if (propTextChoiceValueUpdates != null && !propTextChoiceValueUpdates.isEmpty())
941950
{
942951
if (PropertyType.MULTI_CHOICE.getTypeUri().equals(old.getRangeURI()) || PropertyType.MULTI_CHOICE.getTypeUri().equals(pd.getRangeURI()))
@@ -947,6 +956,52 @@ public static ValidationException updateDomainDescriptor(GWTDomain<? extends GWT
947956
}
948957
textChoiceValueUpdates.put(p, propTextChoiceValueUpdates);
949958
}
959+
960+
// GitHub Issue 949: Text choice value can be deleted if usage is added after loading designer
961+
if (!deletedValues.isEmpty())
962+
{
963+
// using ContainerFilter.EVERYTHING to account for /Shared domains
964+
if (domainTable == null)
965+
domainTable = d.getDomainKind().getTableInfo(user, d.getContainer(), d, ContainerFilter.getUnsafeEverythingFilter());
966+
967+
if (domainTable != null)
968+
{
969+
// if was regular text choice, then we just check the property value
970+
if (TEXT_CHOICE_CONCEPT_URI.equals(old.getConceptURI()))
971+
{
972+
SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(p.getName()), deletedValues, CompareType.IN);
973+
if (new TableSelector(domainTable, filter, null).exists())
974+
{
975+
validationException.addError(new SimpleValidationError("One or more values cannot be removed from the text choice list because they are in use: " + StringUtils.join(deletedValues, ", ")));
976+
return validationException;
977+
}
978+
}
979+
else if (PropertyType.MULTI_CHOICE.getTypeUri().equals(old.getRangeURI()))
980+
{
981+
var column = domainTable.getColumn(p.getName());
982+
983+
if (column != null)
984+
{
985+
var dialect = domainTable.getSchema().getSqlDialect();
986+
SQLFragment deletedArray = new SQLFragment("CAST(? AS TEXT[])").add(deletedValues.toArray(new String[0]));
987+
SQLFragment columnFrag = new SQLFragment().appendIdentifier(column.getAlias());
988+
989+
SQLFragment sql = new SQLFragment("SELECT 1 FROM ")
990+
.append(domainTable)
991+
.append(" WHERE ")
992+
.append(dialect.array_some_in_array(columnFrag, deletedArray));
993+
994+
if (new SqlSelector(domainTable.getSchema().getScope(), sql).exists())
995+
{
996+
validationException.addError(new SimpleValidationError("One or more values cannot be removed from the multi-choice list because they are in use: " + StringUtils.join(deletedValues, ", ")));
997+
return validationException;
998+
}
999+
}
1000+
}
1001+
1002+
}
1003+
1004+
}
9501005
if (old.equals(pd))
9511006
continue;
9521007

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

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

1282-
private static List<Map<String, Object>> updatePropertyValidators(DomainProperty dp, @Nullable GWTPropertyDescriptor oldPd, GWTPropertyDescriptor newPd)
1337+
// 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.
1338+
private static @NotNull Pair<List<Map<String, Object>>, List<String>> updatePropertyValidators(DomainProperty dp, @Nullable GWTPropertyDescriptor oldPd, GWTPropertyDescriptor newPd)
12831339
{
12841340
Map<Long, GWTPropertyValidator> newProps = new LongHashMap<>();
12851341
List<Map<String, Object>> valueUpdates = new ArrayList<>();
1342+
List<String> deletedValues = new ArrayList<>();
12861343

12871344
PropertyDescriptor oldPropertyDescriptor = dp.getPropertyDescriptor().clone();
12881345
boolean hasChange = false;
@@ -1325,7 +1382,24 @@ else if (prop == null)
13251382
// update any new or changed
13261383
for (IPropertyValidator pv : dp.getValidators())
13271384
{
1328-
boolean change = _copyValidator(pv, newProps.get(pv.getRowId()));
1385+
var gpv = newProps.get(pv.getRowId());
1386+
if (gpv == null)
1387+
continue;
1388+
1389+
boolean hasExpressionChange = !Objects.equals(pv.getExpressionValue(), gpv.getExpression());
1390+
if (hasExpressionChange && PropertyValidatorType.TextChoice.equals(gpv.getType()))
1391+
{
1392+
List<String> oldValidValues = PropertyService.get().getTextChoiceValidatorOptions(pv);
1393+
1394+
List<String> newValidValues = PageFlowUtil.splitStringToValues(gpv.getExpression(), '|');
1395+
deletedValues = new ArrayList<>(oldValidValues);
1396+
deletedValues.removeAll(newValidValues);
1397+
// Exclude renamed oldValidValues from deletedValues — their keys in valueUpdates are the old names
1398+
for (Map<String, Object> update : valueUpdates)
1399+
deletedValues.removeAll(update.keySet());
1400+
}
1401+
boolean change = _copyValidator(pv, gpv);
1402+
13291403
hasChange = hasChange || change;
13301404
}
13311405

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

1340-
return oldPd != null ? valueUpdates : null;
1414+
return Pair.of(valueUpdates, deletedValues);
13411415
}
13421416

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

0 commit comments

Comments
 (0)