diff --git a/api/gwtsrc/org/labkey/api/gwt/client/assay/model/GWTProtocol.java b/api/gwtsrc/org/labkey/api/gwt/client/assay/model/GWTProtocol.java index ff4cf8dc35b..2fed56970d3 100644 --- a/api/gwtsrc/org/labkey/api/gwt/client/assay/model/GWTProtocol.java +++ b/api/gwtsrc/org/labkey/api/gwt/client/assay/model/GWTProtocol.java @@ -17,12 +17,14 @@ package org.labkey.api.gwt.client.assay.model; import com.google.gwt.user.client.rpc.IsSerializable; +import org.apache.commons.lang3.StringUtils; import org.labkey.api.gwt.client.model.GWTContainer; import org.labkey.api.gwt.client.model.GWTDomain; import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -76,6 +78,7 @@ public class GWTProtocol implements IsSerializable private boolean _plateMetadata; private String _status; private List _excludedContainerIds; + private String _auditUserComment; public GWTProtocol() { @@ -436,4 +439,39 @@ public void setExcludedContainerIds(List excludedContainerIds) { _excludedContainerIds = excludedContainerIds; } + + public String getAuditUserComment() + { + return _auditUserComment; + } + + public void setAuditUserComment(String auditUserComment) + { + _auditUserComment = auditUserComment; + } + + public Map getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + map.put("Name", getName()); + if (!StringUtils.isEmpty(getDescription())) + map.put("Description", getDescription()); + if (!StringUtils.isEmpty(getStatus())) + map.put("Status", getStatus()); + String autoCopyTargetContainerId = getAutoCopyTargetContainer() != null ? getAutoCopyTargetContainer().getEntityId() : getAutoCopyTargetContainerId(); + if (!StringUtils.isEmpty(autoCopyTargetContainerId)) + map.put("AutoCopyTargetContainer", autoCopyTargetContainerId); + if (!StringUtils.isEmpty(getAutoLinkCategory())) + map.put("AutoLinkCategory", getAutoLinkCategory()); + map.put("SaveScriptFiles", isSaveScriptFiles()); + map.put("IsEditableResults", isEditableResults()); + map.put("IsEditableRuns", isEditableRuns()); + map.put("IsBackgroundUpload", isBackgroundUpload()); + map.put("IsQcEnabled", isQcEnabled()); + map.put("IsPlateMetadataEnabled", isPlateMetadata()); + + return map; + } + + } diff --git a/api/gwtsrc/org/labkey/api/gwt/client/model/GWTIndex.java b/api/gwtsrc/org/labkey/api/gwt/client/model/GWTIndex.java index 3b829d5a374..3edc2783fae 100644 --- a/api/gwtsrc/org/labkey/api/gwt/client/model/GWTIndex.java +++ b/api/gwtsrc/org/labkey/api/gwt/client/model/GWTIndex.java @@ -1,25 +1,30 @@ -/* - * Copyright (c) 2018-2019 LabKey Corporation - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ +/* + * Copyright (c) 2018-2019 LabKey Corporation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ package org.labkey.api.gwt.client.model; import com.google.gwt.user.client.rpc.IsSerializable; +import org.apache.commons.lang3.StringUtils; +import org.labkey.api.data.PropertyStorageSpec; import java.io.Serializable; import java.util.ArrayList; +import java.util.Collections; import java.util.List; +import java.util.Set; +import java.util.stream.Collectors; /** * User: kevink @@ -69,4 +74,22 @@ public void setUnique(boolean unique) { _unique = unique; } + + public String toStringVal() + { + if (_columnNames == null || _columnNames.isEmpty()) + return ""; + + return StringUtils.join(_columnNames, ", ") + ", unique: " + isUnique(); + } + + public static List toStringVals(List indices, Set excludeBaseIndices) + { + if (indices == null || indices.isEmpty()) + return null; + + Set excludeIndices = excludeBaseIndices == null ? Collections.emptySet() : excludeBaseIndices.stream().map(PropertyStorageSpec.Index::toStringVal).collect(Collectors.toSet()); + return indices.stream().map(GWTIndex::toStringVal).filter(v -> !excludeIndices.contains(v)).sorted().toList(); + } + } diff --git a/api/src/org/labkey/api/assay/AbstractAssayProvider.java b/api/src/org/labkey/api/assay/AbstractAssayProvider.java index 73943f8bd2b..a4bf95f51c7 100644 --- a/api/src/org/labkey/api/assay/AbstractAssayProvider.java +++ b/api/src/org/labkey/api/assay/AbstractAssayProvider.java @@ -74,6 +74,7 @@ import org.labkey.api.exp.api.IAssayDomainType; import org.labkey.api.exp.property.Domain; import org.labkey.api.exp.property.DomainProperty; +import org.labkey.api.exp.property.DomainUtil; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.exp.query.ExpRunTable; import org.labkey.api.files.FileContentService; @@ -1246,7 +1247,7 @@ public boolean hasUsefulDetailsPage() private static final String SCRIPT_PATH_DELIMITER = "|"; @Override - public ValidationException setValidationAndAnalysisScripts(ExpProtocol protocol, @NotNull List scripts) throws ExperimentException + public Pair> setValidationAndAnalysisScripts(ExpProtocol protocol, @NotNull List scripts) throws ExperimentException { Map props = new HashMap<>(protocol.getObjectProperties()); String propertyURI = ScriptType.TRANSFORM.getPropertyURI(protocol); @@ -1290,9 +1291,11 @@ public ValidationException setValidationAndAnalysisScripts(ExpProtocol protocol, // don't persist if any validation error has severity=ERROR if (validationErrors.getErrors().stream().anyMatch(e -> SEVERITY.ERROR == e.getSeverity())) - return validationErrors; + return new Pair<>(validationErrors, null); JSONArray json = AnalysisScript.toJson(scripts); + ObjectProperty oldProp = props.get(propertyURI); + String oldJson = oldProp == null ? null : oldProp.getStringValue(); if (json != null) { ObjectProperty prop = new ObjectProperty(protocol.getLSID(), protocol.getContainer(), @@ -1305,7 +1308,7 @@ public ValidationException setValidationAndAnalysisScripts(ExpProtocol protocol, } protocol.setObjectProperties(props); - return validationErrors; + return new Pair<>(validationErrors, new Pair<>(oldJson, json == null ? null : json.toString())); } /** For migrating legacy assay designs that have separate transform and validation script properties */ diff --git a/api/src/org/labkey/api/assay/AssayProvider.java b/api/src/org/labkey/api/assay/AssayProvider.java index b5e972f26b8..925d74281ca 100644 --- a/api/src/org/labkey/api/assay/AssayProvider.java +++ b/api/src/org/labkey/api/assay/AssayProvider.java @@ -239,8 +239,9 @@ enum Scope /** * File based QC and analysis scripts can be added to a protocol and invoked when the validate * method is called. Set to an empty list if no scripts exist. + * @return ValidationException, a pair of old and new string representation of the script description (for audit use) */ - ValidationException setValidationAndAnalysisScripts(ExpProtocol protocol, @NotNull List scripts) throws ExperimentException; + Pair> setValidationAndAnalysisScripts(ExpProtocol protocol, @NotNull List scripts) throws ExperimentException; @NotNull List getValidationAndAnalysisScripts(ExpProtocol protocol, Scope scope); diff --git a/api/src/org/labkey/api/audit/AbstractAuditHandler.java b/api/src/org/labkey/api/audit/AbstractAuditHandler.java index a6491624442..f21ff0ca9a0 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditHandler.java +++ b/api/src/org/labkey/api/audit/AbstractAuditHandler.java @@ -117,7 +117,7 @@ public void addAuditEvent(User user, Container c, TableInfo table, @Nullable Aud { case INSERT: { - String newRecord = AbstractAuditTypeProvider.encodeForDataMap(c, row); + String newRecord = AbstractAuditTypeProvider.encodeForDataMap(row); if (newRecord != null) event.setNewRecordMap(newRecord, c); break; @@ -126,7 +126,7 @@ public void addAuditEvent(User user, Container c, TableInfo table, @Nullable Aud { if (existingRow.isEmpty()) { - String newRecord = AbstractAuditTypeProvider.encodeForDataMap(c, row); + String newRecord = AbstractAuditTypeProvider.encodeForDataMap(row); if (newRecord != null) event.setNewRecordMap(newRecord, c); } @@ -138,7 +138,7 @@ public void addAuditEvent(User user, Container c, TableInfo table, @Nullable Aud } case DELETE: { - String oldRecord = AbstractAuditTypeProvider.encodeForDataMap(c, row); + String oldRecord = AbstractAuditTypeProvider.encodeForDataMap(row); if (oldRecord != null) event.setOldRecordMap(oldRecord, c); break; @@ -177,11 +177,11 @@ private void setOldAndNewMapsForUpdate(DetailedAuditTypeEvent event, Container c // allow for adding fields that may be present in the updated row but not represented in the original row addDetailedModifiedFields(existingRow, modifiedRow, row); - String oldRecord = AbstractAuditTypeProvider.encodeForDataMap(c, originalRow); + String oldRecord = AbstractAuditTypeProvider.encodeForDataMap(originalRow); if (oldRecord != null) event.setOldRecordMap(oldRecord, c); - String newRecord = AbstractAuditTypeProvider.encodeForDataMap(c, modifiedRow); + String newRecord = AbstractAuditTypeProvider.encodeForDataMap(modifiedRow); if (newRecord != null) event.setNewRecordMap(newRecord, c); } diff --git a/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java b/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java index b3a8e48c274..afc791fbf67 100644 --- a/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java +++ b/api/src/org/labkey/api/audit/AbstractAuditTypeProvider.java @@ -15,7 +15,9 @@ */ package org.labkey.api.audit; +import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.Nullable; import org.labkey.api.audit.data.DataMapColumn; import org.labkey.api.audit.data.DataMapDiffColumn; import org.labkey.api.audit.query.AbstractAuditDomainKind; @@ -48,6 +50,7 @@ import org.labkey.api.exp.property.PropertyService; import org.labkey.api.gwt.client.DefaultValueType; import org.labkey.api.query.AliasedColumn; +import org.labkey.api.query.DetailsURL; import org.labkey.api.query.FieldKey; import org.labkey.api.query.UserSchema; import org.labkey.api.security.User; @@ -345,6 +348,16 @@ public static Container getDomainContainer() } protected void appendValueMapColumns(AbstractTableInfo table) + { + appendValueMapColumns(table, null); + } + + protected void appendValueMapColumns(AbstractTableInfo table, String eventName) + { + appendValueMapColumns(table, eventName, false); + } + + protected void appendValueMapColumns(AbstractTableInfo table, String eventName, boolean noUrl) { MutableColumnInfo oldCol = table.getMutableColumn(FieldKey.fromString(OLD_RECORD_PROP_NAME)); MutableColumnInfo newCol = table.getMutableColumn(FieldKey.fromString(NEW_RECORD_PROP_NAME)); @@ -370,6 +383,17 @@ protected void appendValueMapColumns(AbstractTableInfo table) // add a column to show the differences between old and new values if (oldCol != null && newCol != null) table.addColumn(new DataMapDiffColumn(table, COLUMN_NAME_DATA_CHANGES, oldCol, newCol)); + + if (!noUrl) + { + String urlStr = "audit-detailedAuditChanges.view?auditRowId=${rowId}"; + if (StringUtils.isEmpty(eventName)) + urlStr = urlStr + "&auditEventType=" + eventName; + DetailsURL url = DetailsURL.fromString(urlStr); + url.setStrictContainerContextEval(true); + table.setDetailsURL(url); + } + } @Override @@ -394,7 +418,7 @@ public static Map decodeFromDataMap(String properties) } } - public static String encodeForDataMap(Container c, Map properties) + public static String encodeForDataMap(Map properties) { if (properties == null) return null; diff --git a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java index 0b629dfb1b5..5c072c8a414 100644 --- a/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java +++ b/api/src/org/labkey/api/audit/SampleTimelineAuditEvent.java @@ -208,7 +208,7 @@ public void setOldRecordMap(String oldRecordMap, Container container) if (label != null) { row.put("samplestatelabel", label); - oldRecordMap = AbstractAuditTypeProvider.encodeForDataMap(container, row); + oldRecordMap = AbstractAuditTypeProvider.encodeForDataMap(row); } } super.setOldRecordMap(oldRecordMap); @@ -228,7 +228,7 @@ public void setNewRecordMap(String newRecordMap, Container container) if (label != null) { row.put("samplestatelabel", label); - newRecordMap = AbstractAuditTypeProvider.encodeForDataMap(container, row); + newRecordMap = AbstractAuditTypeProvider.encodeForDataMap(row); } } super.setNewRecordMap(newRecordMap, container); diff --git a/api/src/org/labkey/api/audit/query/AbstractAuditDomainKind.java b/api/src/org/labkey/api/audit/query/AbstractAuditDomainKind.java index d21f0436ed3..7daba72dc54 100644 --- a/api/src/org/labkey/api/audit/query/AbstractAuditDomainKind.java +++ b/api/src/org/labkey/api/audit/query/AbstractAuditDomainKind.java @@ -248,13 +248,13 @@ public Domain createDomain(GWTDomain domain, JSONObject arguments, Container con @Override public ValidationException updateDomain(GWTDomain original, GWTDomain update, - JSONObject options, Container container, User user, boolean includeWarnings) + JSONObject options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { throw new UnsupportedOperationException(); } @Override - public void deleteDomain(User user, Domain domain) + public void deleteDomain(User user, Domain domain, @Nullable String auditUserComment) { throw new UnsupportedOperationException(); } diff --git a/api/src/org/labkey/api/data/ConditionalFormat.java b/api/src/org/labkey/api/data/ConditionalFormat.java index d838b7c13bc..4a0c3a5faab 100644 --- a/api/src/org/labkey/api/data/ConditionalFormat.java +++ b/api/src/org/labkey/api/data/ConditionalFormat.java @@ -15,6 +15,7 @@ */ package org.labkey.api.data; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; import org.fhcrc.cpas.exp.xml.PropertyDescriptorType; @@ -376,4 +377,27 @@ private static boolean addToXML(List formats, Co return success; } + + public String toStringVal() + { + return getFilter() + ": " + getCssStyle(); + } + + public static @Nullable String toStringVal(List formats) + { + if (formats == null || formats.isEmpty()) + return null; + + List strings = new ArrayList<>(); + formats.forEach(format -> { + if (format instanceof ConditionalFormat c) + strings.add(c.toStringVal()); + else + { + ConditionalFormat conditionalFormat = new ConditionalFormat(format); + strings.add(conditionalFormat.toStringVal()); + } + }); + return StringUtils.join(strings, ", "); + } } diff --git a/api/src/org/labkey/api/data/PropertyStorageSpec.java b/api/src/org/labkey/api/data/PropertyStorageSpec.java index 7ed40b552df..3ea16a85024 100644 --- a/api/src/org/labkey/api/data/PropertyStorageSpec.java +++ b/api/src/org/labkey/api/data/PropertyStorageSpec.java @@ -15,6 +15,7 @@ */ package org.labkey.api.data; +import org.apache.commons.lang3.StringUtils; import org.labkey.api.collections.CaseInsensitiveHashSet; import org.labkey.api.exp.MvColumn; import org.labkey.api.exp.PropertyDescriptor; @@ -500,6 +501,14 @@ public int hashCode() { return Objects.hash(Arrays.hashCode(columnNames), isClustered, isClustered); } + + public String toStringVal() + { + if (columnNames == null) + return ""; + + return StringUtils.join(columnNames, ", ") + ", unique: " + isUnique; + } } public enum Special diff --git a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java index 0ba41669dd0..cb481ad95df 100644 --- a/api/src/org/labkey/api/dataiterator/SimpleTranslator.java +++ b/api/src/org/labkey/api/dataiterator/SimpleTranslator.java @@ -455,7 +455,7 @@ private Object getSingleValue(Object k, Collection vs) if (vs.size() == 1) return vs.iterator().next(); - throw new ConversionException("Found " + vs.size() + " values matching: " + String.valueOf(k)); + throw new ConversionExceptionWithMessage("Found " + vs.size() + " values matching: " + String.valueOf(k)); } } diff --git a/api/src/org/labkey/api/exp/PropertyDescriptor.java b/api/src/org/labkey/api/exp/PropertyDescriptor.java index 20d55c932a9..f607e384250 100644 --- a/api/src/org/labkey/api/exp/PropertyDescriptor.java +++ b/api/src/org/labkey/api/exp/PropertyDescriptor.java @@ -15,6 +15,7 @@ */ package org.labkey.api.exp; +import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -49,6 +50,7 @@ import java.io.Serializable; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Objects; @@ -533,6 +535,62 @@ public void setDatabaseDefaultValue(@Nullable Object databaseDefaultValue) { _databaseDefaultValue = databaseDefaultValue; } + + public Map getAuditRecordMap(@Nullable String validatorStr, @Nullable String conditionalFormatStr) + { + Map map = new LinkedHashMap<>(); + if (!StringUtils.isEmpty(getName())) + map.put("Name", getName()); + if (!StringUtils.isEmpty(getLabel())) + map.put("Label", getLabel()); + if (null != getPropertyType()) + { + if (org.labkey.api.gwt.client.ui.PropertyType.expFlag.getURI().equals(getConceptURI())) + map.put("Type", "Flag"); + else + map.put("Type", getPropertyType().getXarName()); + } + if (getPropertyType().getJdbcType().isText()) + map.put("Scale", getScale()); + if (!StringUtils.isEmpty(getDescription())) + map.put("Description", getDescription()); + if (!StringUtils.isEmpty(getFormat())) + map.put("Format", getFormat()); + if (null !=getURL()) + map.put("URL", getURL().toString()); + if (null != getPHI()) + map.put("PHI", getPHI().getLabel()); + if (null !=getDefaultScale()) + map.put("DefaultScale", getDefaultScale().getLabel()); + map.put("Required", isRequired()); + map.put("Hidden", isHidden()); + map.put("MvEnabled", isMvEnabled()); + map.put("Measure", isMeasure()); + map.put("Dimension", isDimension()); + map.put("ShownInInsert", isShownInInsertView()); + map.put("ShownInDetails", isShownInDetailsView()); + map.put("ShownInUpdate", isShownInUpdateView()); + map.put("ShownInLookupView", isShownInLookupView()); + map.put("RecommendedVariable", isRecommendedVariable()); + map.put("ExcludedFromShifting", isExcludeFromShifting()); + map.put("Scannable", isScannable()); + if (!StringUtils.isEmpty(getDerivationDataScope())) + map.put("DerivationDataScope", getDerivationDataScope()); + String importAliasStr = StringUtils.join(getImportAliasSet(), ","); + if (!StringUtils.isEmpty(importAliasStr)) + map.put("ImportAliases", importAliasStr); + if (null != getDefaultValueTypeEnum()) + map.put("DefaultValueType", getDefaultValueTypeEnum().getLabel()); + if (getLookup() != null) + map.put("Lookup", getLookup().toJSONString()); + if (!StringUtils.isEmpty(validatorStr)) + map.put("Validator", validatorStr); + if (!StringUtils.isEmpty(conditionalFormatStr)) + map.put("ConditionalFormat", conditionalFormatStr); + + return map; + } + } diff --git a/api/src/org/labkey/api/exp/api/DataClassDomainKindProperties.java b/api/src/org/labkey/api/exp/api/DataClassDomainKindProperties.java index 545c99aad22..6fd087bf24d 100644 --- a/api/src/org/labkey/api/exp/api/DataClassDomainKindProperties.java +++ b/api/src/org/labkey/api/exp/api/DataClassDomainKindProperties.java @@ -10,6 +10,7 @@ import java.io.IOException; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -202,4 +203,23 @@ public void setExcludedContainerIds(List excludedContainerIds) { this.excludedContainerIds = excludedContainerIds; } + + public Map getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + // skip Name and Description since it's general domain property + if (!StringUtils.isEmpty(getNameExpression())) + map.put("NameExpression", getNameExpression()); + String importAliasStr = ExperimentJSONConverter.getImportAliasStringVal(getImportAliases()); + if (!StringUtils.isEmpty(importAliasStr)) + map.put("ImportAlias", importAliasStr); + if (!StringUtils.isEmpty(getCategory())) + map.put("Category", getCategory()); + if (getSampleType() != null) + map.put("SampleType", getSampleType()); + + return map; + } + + } diff --git a/api/src/org/labkey/api/exp/api/ExpDataClass.java b/api/src/org/labkey/api/exp/api/ExpDataClass.java index b195c37111e..5d794610259 100644 --- a/api/src/org/labkey/api/exp/api/ExpDataClass.java +++ b/api/src/org/labkey/api/exp/api/ExpDataClass.java @@ -118,5 +118,7 @@ public interface ExpDataClass extends ExpObject, ExpSearchable void setImportAliasMap(Map> aliasMap); + String getImportAliasJson(); + boolean hasData(); } diff --git a/api/src/org/labkey/api/exp/api/ExpObject.java b/api/src/org/labkey/api/exp/api/ExpObject.java index 646ef715877..a87c8353e3d 100644 --- a/api/src/org/labkey/api/exp/api/ExpObject.java +++ b/api/src/org/labkey/api/exp/api/ExpObject.java @@ -76,7 +76,7 @@ default void setComment(User user, String comment, boolean index) throws Validat void save(User user) throws BatchValidationException; void delete(User user); - default void delete(User user, String auditUserComment) + default void delete(User user, @Nullable String auditUserComment) { delete(user); } diff --git a/api/src/org/labkey/api/exp/api/ExpProtocol.java b/api/src/org/labkey/api/exp/api/ExpProtocol.java index d726d34e2c2..5754e4062d9 100644 --- a/api/src/org/labkey/api/exp/api/ExpProtocol.java +++ b/api/src/org/labkey/api/exp/api/ExpProtocol.java @@ -18,6 +18,7 @@ import org.jetbrains.annotations.Nullable; import org.labkey.api.assay.AbstractAssayProvider; +import org.labkey.api.assay.AssayProvider; import org.labkey.api.data.Container; import org.labkey.api.exp.ObjectProperty; import org.labkey.api.exp.ProtocolParameter; @@ -178,4 +179,6 @@ static boolean isSampleWorkflowProtocol(String lsid) { return isSampleWorkflowTaskProtocol(lsid) || isSampleWorkflowJobProtocol(lsid); } + + Map getAuditRecordMap(AssayProvider provider); } diff --git a/api/src/org/labkey/api/exp/api/ExperimentJSONConverter.java b/api/src/org/labkey/api/exp/api/ExperimentJSONConverter.java index 36ca5e86bd6..cf2a6450fba 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentJSONConverter.java +++ b/api/src/org/labkey/api/exp/api/ExperimentJSONConverter.java @@ -53,6 +53,7 @@ import java.io.File; import java.io.IOException; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -61,6 +62,7 @@ import java.util.Map; import java.util.Objects; import java.util.Set; +import java.util.TreeMap; import java.util.stream.Collectors; import static org.labkey.api.exp.api.ExpData.DATA_INPUTS_PREFIX; @@ -957,6 +959,21 @@ else if (value instanceof Map map) return aliases; } + public static @Nullable String getImportAliasStringVal(@Nullable Map> importAliases) + { + if (importAliases == null || importAliases.isEmpty()) + return null; + + List aliasStrings = new ArrayList<>(); + importAliases.forEach((key, value) -> { + String val = (String) value.get("inputType"); + String requiredStr = Objects.toString(value.get("required"), "false"); + aliasStrings.add(key + "(" + val + ",required=" + requiredStr + ")"); + }); + Collections.sort(aliasStrings); + return StringUtils.join(aliasStrings, "; "); + } + public static final class TestCase extends Assert { @Test diff --git a/api/src/org/labkey/api/exp/api/ExperimentService.java b/api/src/org/labkey/api/exp/api/ExperimentService.java index 0a03f3a7072..db2d42348bf 100644 --- a/api/src/org/labkey/api/exp/api/ExperimentService.java +++ b/api/src/org/labkey/api/exp/api/ExperimentService.java @@ -279,7 +279,8 @@ ValidationException updateDataClass( @NotNull ExpDataClass dataClass, @Nullable DataClassDomainKindProperties options, GWTDomain original, - GWTDomain update + GWTDomain update, + @Nullable String auditUserComment ); /** @@ -1054,7 +1055,10 @@ List getExpProtocolsWithParameterValue( void ensureContainerDataTypeExclusions(@NotNull DataTypeForExclusion dataType, @Nullable DataTypeForExclusion relatedDataType, @Nullable Collection excludedDataTypeRowIds, @NotNull String excludedContainerId, User user); - void ensureDataTypeContainerExclusions(@NotNull DataTypeForExclusion dataType, @Nullable Collection excludedContainerIds, @NotNull Integer dataTypeId, User user); + /** + * @return Details about what's changed, to be used in audit log (old -> new) + */ + @NotNull Pair, Collection> ensureDataTypeContainerExclusions(@NotNull DataTypeForExclusion dataType, @Nullable Collection excludedContainerIds, @NotNull Integer dataTypeId, User user); void ensureDataTypeContainerExclusionsNonAdmin(@NotNull DataTypeForExclusion dataType, @NotNull Integer dataTypeId, Container container, User user); diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java index c59da1b3c33..4eab9c0a44b 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKind.java @@ -394,9 +394,9 @@ public DefaultValueType[] getDefaultValueOptions(Domain domain) @Override @NotNull public ValidationException updateDomain(GWTDomain original, @NotNull GWTDomain update, - @Nullable SampleTypeDomainKindProperties options, Container container, User user, boolean includeWarnings) + @Nullable SampleTypeDomainKindProperties options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { - return SampleTypeService.get().updateSampleType(original, update, options, container, user, includeWarnings); + return SampleTypeService.get().updateSampleType(original, update, options, container, user, includeWarnings, auditUserComment); } @Override @@ -483,7 +483,7 @@ public void validateOptions(Container container, User user, SampleTypeDomainKind int aliquotNameExpMax = materialSourceTI.getColumn("AliquotNameExpression").getScale(); if (StringUtils.isNotBlank(options.getAliquotNameExpression()) && options.getAliquotNameExpression().length() > aliquotNameExpMax) - throw new IllegalArgumentException("Value for Aliquot Naming Patten field may not exceed " + nameExpMax + " characters."); + throw new IllegalArgumentException("Value for Aliquot Naming Patten field may not exceed " + aliquotNameExpMax + " characters."); int labelColorMax = materialSourceTI.getColumn("LabelColor").getScale(); if (StringUtils.isNotBlank(options.getLabelColor()) && options.getLabelColor().length() > labelColorMax) @@ -592,7 +592,7 @@ public Domain createDomain(GWTDomain domain, @Nullable SampleTypeDomainKindPrope 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); + templateInfo, aliases, labelColor, metricUnit, autoLinkTargetContainer, autoLinkCategory, category, domain.getDisabledSystemFields(), excludedContainerIds, excludedDashboardContainerIds, arguments != null ? arguments.getAuditRecordMap() : null); } catch (SQLException e) { @@ -606,13 +606,13 @@ public Domain createDomain(GWTDomain domain, @Nullable SampleTypeDomainKindPrope } @Override - public void deleteDomain(User user, Domain domain) + public void deleteDomain(User user, Domain domain, @Nullable String auditUserComment) { ExpSampleType st = SampleTypeService.get().getSampleType(domain.getTypeURI()); if (st == null) throw new NotFoundException("Sample Type not found: " + domain); - st.delete(user); + st.delete(user, auditUserComment); } @Override diff --git a/api/src/org/labkey/api/exp/api/SampleTypeDomainKindProperties.java b/api/src/org/labkey/api/exp/api/SampleTypeDomainKindProperties.java index 69eae7d7cf5..f99ba383926 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeDomainKindProperties.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeDomainKindProperties.java @@ -3,12 +3,14 @@ import com.fasterxml.jackson.annotation.JsonIgnore; import com.fasterxml.jackson.annotation.JsonTypeInfo; import com.fasterxml.jackson.databind.annotation.JsonDeserialize; +import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.Nullable; import org.json.JSONObject; import java.io.IOException; import java.util.Collections; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.stream.Collectors; @@ -272,4 +274,29 @@ public void setExcludedDashboardContainerIds(List excludedDashboardConta { this.excludedDashboardContainerIds = excludedDashboardContainerIds; } + + public Map getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + // skip Name and Description since it's general domain property + if (!StringUtils.isEmpty(getNameExpression())) + map.put("NameExpression", getNameExpression()); + if (!StringUtils.isEmpty(getAliquotNameExpression())) + map.put("AliquotNameExpression", getAliquotNameExpression()); + if (!StringUtils.isEmpty(getLabelColor())) + map.put("LabelColor", getLabelColor()); + if (!StringUtils.isEmpty(getMetricUnit())) + map.put("MetricUnit", getMetricUnit()); + String importAliasStr = ExperimentJSONConverter.getImportAliasStringVal(getImportAliases()); + if (!StringUtils.isEmpty(importAliasStr)) + map.put("ImportAlias", importAliasStr); + if (!StringUtils.isEmpty(getAutoLinkTargetContainerId())) + map.put("AutoLinkTargetContainerId", getAutoLinkTargetContainerId()); + if (!StringUtils.isEmpty(getAutoLinkCategory())) + map.put("AutoLinkCategory", getAutoLinkCategory()); + if (!StringUtils.isEmpty(getCategory())) + map.put("Category", getCategory()); + + return map; + } } diff --git a/api/src/org/labkey/api/exp/api/SampleTypeService.java b/api/src/org/labkey/api/exp/api/SampleTypeService.java index 2c15e0ab098..2d3bc26ecff 100644 --- a/api/src/org/labkey/api/exp/api/SampleTypeService.java +++ b/api/src/org/labkey/api/exp/api/SampleTypeService.java @@ -139,7 +139,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 List excludedContainerIds, @Nullable List excludedDashboardContainerIds, @Nullable Map changeDetails) throws ExperimentException, SQLException; @NotNull @@ -230,7 +230,7 @@ default Map incrementSampleCounts(@Nullable Date counterDate) long getProjectRootSampleCount(Container container); - ValidationException updateSampleType(GWTDomain original, GWTDomain update, SampleTypeDomainKindProperties options, Container container, User user, boolean includeWarnings); + ValidationException updateSampleType(GWTDomain original, GWTDomain update, SampleTypeDomainKindProperties options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment); void addAuditEvent(User user, Container container, String comment, String userComment, ExpMaterial sample, Map metadata); diff --git a/api/src/org/labkey/api/exp/list/ListDefinition.java b/api/src/org/labkey/api/exp/list/ListDefinition.java index 40e5a70b432..796c60cc34a 100644 --- a/api/src/org/labkey/api/exp/list/ListDefinition.java +++ b/api/src/org/labkey/api/exp/list/ListDefinition.java @@ -25,6 +25,7 @@ import org.labkey.api.exp.DomainNotFoundException; import org.labkey.api.exp.PropertyType; import org.labkey.api.exp.property.Domain; +import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.reader.DataLoader; @@ -39,6 +40,7 @@ import java.util.Collection; import java.util.Date; import java.util.List; +import java.util.Map; /** * Represents a single list definition, as captured by a domain and some list-specific configuration, and defined @@ -290,8 +292,9 @@ public static BodySetting getForValue(int value) void setKeyType(KeyType type); void save(User user) throws Exception; - void save(User user, boolean ensureKey) throws Exception; + void save(User user, boolean ensureKey, @Nullable Map newRecordMap, @Nullable List calculatedFields) throws Exception; void delete(User user) throws DomainNotFoundException; + void delete(User user, @Nullable String auditUserComment) throws DomainNotFoundException; ListItem createListItem(); ListItem getListItem(Object key, User user); diff --git a/api/src/org/labkey/api/exp/property/AbstractDomainKind.java b/api/src/org/labkey/api/exp/property/AbstractDomainKind.java index 5f7436bfb08..9b7178a7d56 100644 --- a/api/src/org/labkey/api/exp/property/AbstractDomainKind.java +++ b/api/src/org/labkey/api/exp/property/AbstractDomainKind.java @@ -123,13 +123,13 @@ public Domain createDomain(GWTDomain domain, T arguments, @Override @NotNull public ValidationException updateDomain(GWTDomain original, GWTDomain update, - @Nullable T options, Container container, User user, boolean includeWarnings) + @Nullable T options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { return DomainUtil.updateDomainDescriptor(original, update, container, user); } @Override - public void deleteDomain(User user, Domain domain) + public void deleteDomain(User user, Domain domain, @Nullable String auditUserComment) { } diff --git a/api/src/org/labkey/api/exp/property/Domain.java b/api/src/org/labkey/api/exp/property/Domain.java index 687bc171add..4c0e4da5580 100644 --- a/api/src/org/labkey/api/exp/property/Domain.java +++ b/api/src/org/labkey/api/exp/property/Domain.java @@ -28,6 +28,7 @@ import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.TemplateInfo; import org.labkey.api.gwt.client.model.GWTIndex; +import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; import org.labkey.api.security.User; import org.labkey.api.security.permissions.Permission; import org.labkey.api.view.ActionURL; @@ -88,8 +89,10 @@ default void delete(@Nullable User user, @Nullable String auditUserComment) thro delete(user); } void save(User user) throws ChangePropertyDescriptorException; - void save(User user, boolean auditComment) throws ChangePropertyDescriptorException; - void save(User user, @Nullable String allowAddBaseProperty) throws ChangePropertyDescriptorException; + void save(User user, @Nullable Map newRecordMap, @Nullable List calculatedFields) throws ChangePropertyDescriptorException; + void save(User user, @Nullable String auditComment, @Nullable String auditUserComment, + @Nullable Map oldRecordMap, @Nullable Map newRecordMap, + @Nullable List oldCalculatedFields, @Nullable List newCalculatedFields) throws ChangePropertyDescriptorException; /** Returns true if this domain has not yet been saved. */ boolean isNew(); diff --git a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java index efa4423e0fc..616c21bf64b 100644 --- a/api/src/org/labkey/api/exp/property/DomainAuditProvider.java +++ b/api/src/org/labkey/api/exp/property/DomainAuditProvider.java @@ -19,6 +19,7 @@ import org.labkey.api.audit.AbstractAuditTypeProvider; import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.audit.AuditTypeProvider; +import org.labkey.api.audit.DetailedAuditTypeEvent; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.data.ColumnInfo; @@ -31,6 +32,7 @@ import org.labkey.api.data.TableInfo; import org.labkey.api.exp.PropertyDescriptor; import org.labkey.api.exp.PropertyType; +import org.labkey.api.query.DetailsURL; import org.labkey.api.query.FieldKey; import org.labkey.api.query.UserSchema; import org.labkey.api.util.PageFlowUtil; @@ -98,7 +100,7 @@ public String getDescription() @Override public TableInfo createTableInfo(UserSchema userSchema, ContainerFilter cf) { - return new DefaultAuditTypeTable(this, createStorageTableInfo(), userSchema, cf, defaultVisibleColumns) + DefaultAuditTypeTable table = new DefaultAuditTypeTable(this, createStorageTableInfo(), userSchema, cf, defaultVisibleColumns) { @Override protected void initColumn(MutableColumnInfo col) @@ -112,6 +114,10 @@ else if (COLUMN_NAME_USER_COMMENT.equalsIgnoreCase(col.getName())) col.setLabel("User Comment"); } }; + + appendValueMapColumns(table, EVENT_TYPE); + + return table; } @Override @@ -135,7 +141,7 @@ public Class getEventClass() return (Class)DomainAuditEvent.class; } - public static class DomainAuditEvent extends AuditTypeEvent + public static class DomainAuditEvent extends DetailedAuditTypeEvent { private String _domainUri; private String _domainName; @@ -195,6 +201,8 @@ public DomainAuditDomainKind() Set fields = new LinkedHashSet<>(); fields.add(createPropertyDescriptor(COLUMN_NAME_DOMAIN_URI, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_DOMAIN_NAME, PropertyType.STRING)); + fields.add(createOldDataMapPropertyDescriptor()); + fields.add(createNewDataMapPropertyDescriptor()); fields.add(createPropertyDescriptor(COLUMN_NAME_USER_COMMENT, PropertyType.STRING)); _fields = Collections.unmodifiableSet(fields); } diff --git a/api/src/org/labkey/api/exp/property/DomainKind.java b/api/src/org/labkey/api/exp/property/DomainKind.java index 462cc0ebe30..8b96486c2d5 100644 --- a/api/src/org/labkey/api/exp/property/DomainKind.java +++ b/api/src/org/labkey/api/exp/property/DomainKind.java @@ -160,18 +160,19 @@ public Set getReservedPropertyNamePrefixes() * @param options Any domain kind specific properties/options. * @param container Container * @param user User + * @param includeWarnings + * @param auditUserComment * @return A list of errors collected during the update. */ abstract public ValidationException updateDomain(GWTDomain original, GWTDomain update, - @Nullable T options, Container container, User user, boolean includeWarnings); - + @Nullable T options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment); /** * Delete a Domain and its associated data. - * @param domain - * @param user * @param domain The domain to delete + * @param user + * @param auditUserComment */ - abstract public void deleteDomain(User user, Domain domain); + abstract public void deleteDomain(User user, Domain domain, @Nullable String auditUserComment); /** * Get base properties defined for that domainkind. The domain parameter is only when there may be a condition diff --git a/api/src/org/labkey/api/exp/property/DomainProperty.java b/api/src/org/labkey/api/exp/property/DomainProperty.java index 267683f99b7..3a44dd104bb 100644 --- a/api/src/org/labkey/api/exp/property/DomainProperty.java +++ b/api/src/org/labkey/api/exp/property/DomainProperty.java @@ -156,4 +156,6 @@ default boolean isUniqueIdField() boolean isScannable(); void setScannable(boolean scannable); + + void setOldPropertyDescriptor(PropertyDescriptor oldPropertyDescriptor); } diff --git a/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java b/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java index 2778047abf9..35d679d6782 100644 --- a/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java +++ b/api/src/org/labkey/api/exp/property/DomainPropertyAuditProvider.java @@ -17,6 +17,7 @@ import org.labkey.api.audit.AbstractAuditTypeProvider; import org.labkey.api.audit.AuditTypeEvent; +import org.labkey.api.audit.DetailedAuditTypeEvent; import org.labkey.api.audit.query.AbstractAuditDomainKind; import org.labkey.api.audit.query.DefaultAuditTypeTable; import org.labkey.api.data.Container; @@ -105,7 +106,7 @@ public List getDefaultVisibleColumns() @Override public TableInfo createTableInfo(UserSchema userSchema, ContainerFilter cf) { - return new DefaultAuditTypeTable(this, createStorageTableInfo(), userSchema, cf, DEFAULT_VISIBLE_COLUMNS) + DefaultAuditTypeTable table = new DefaultAuditTypeTable(this, createStorageTableInfo(), userSchema, cf, DEFAULT_VISIBLE_COLUMNS) { @Override protected void initColumn(MutableColumnInfo col) @@ -137,6 +138,10 @@ public TableInfo getLookupTableInfo() col.setLabel("Domain Name"); } }; + + appendValueMapColumns(table, EVENT_NAME); + + return table; } public static class DomainPropertyAuditDomainKind extends AbstractAuditDomainKind @@ -156,6 +161,8 @@ public DomainPropertyAuditDomainKind() fields.add(createPropertyDescriptor(COLUMN_NAME_ACTION, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_DOMAIN_NAME, PropertyType.STRING)); fields.add(createPropertyDescriptor(COLUMN_NAME_DOMAIN_EVENT_ID, PropertyType.BIGINT)); + fields.add(createOldDataMapPropertyDescriptor()); + fields.add(createNewDataMapPropertyDescriptor()); _fields = Collections.unmodifiableSet(fields); } @@ -178,7 +185,7 @@ public String getKindName() } } - public static class DomainPropertyAuditEvent extends AuditTypeEvent + public static class DomainPropertyAuditEvent extends DetailedAuditTypeEvent { private String _propertyUri; private String _propertyName; diff --git a/api/src/org/labkey/api/exp/property/DomainUtil.java b/api/src/org/labkey/api/exp/property/DomainUtil.java index ea78a9f7683..74dc6d8b26d 100644 --- a/api/src/org/labkey/api/exp/property/DomainUtil.java +++ b/api/src/org/labkey/api/exp/property/DomainUtil.java @@ -34,6 +34,7 @@ import org.labkey.api.data.ContainerService; import org.labkey.api.data.NameGenerator; import org.labkey.api.data.PHI; +import org.labkey.api.data.PropertyStorageSpec; import org.labkey.api.data.SchemaTableInfo; import org.labkey.api.data.SimpleFilter; import org.labkey.api.data.TableInfo; @@ -93,9 +94,11 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.ListIterator; import java.util.Map; +import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.regex.Matcher; @@ -744,9 +747,16 @@ public static ValidationException updateDomainDescriptor(GWTDomain orig, GWTDomain update, Container container, User user, boolean updateDomainName, @Nullable String auditComment) + { + return updateDomainDescriptor(orig, update, container, user, updateDomainName, auditComment, null, null, null); + } + + /** @return Errors encountered during the save attempt */ + @NotNull + public static ValidationException updateDomainDescriptor(GWTDomain orig, GWTDomain update, Container container, User user, + boolean updateDomainName, @Nullable String auditComment, @Nullable String auditUserComment, @Nullable Map oldProps, @Nullable Map newProps) { LOG.info("Updating domain descriptor for " + orig.getName()); assert orig.getDomainURI().equals(update.getDomainURI()); @@ -773,9 +783,16 @@ public static ValidationException updateDomainDescriptor(GWTDomain(); + if (oldProps == null) + oldProps = new LinkedHashMap<>(); + String updatedName = update.getName(); if (updateDomainName && !d.getName().equals(updatedName)) { + oldProps.put("Name", d.getName()); + newProps.put("Name", updatedName); updatedName = StringUtils.trimToEmpty(updatedName); String domainNameError = validateDomainName(updatedName, kind.getKindName(), kind.supportsNamingPattern()); if (!StringUtils.isEmpty(domainNameError)) @@ -787,7 +804,22 @@ public static ValidationException updateDomainDescriptor(GWTDomain oldDisabledSystemFields = d.getDisabledSystemFields(); + if (oldDisabledSystemFields != null && !oldDisabledSystemFields.isEmpty()) + oldProps.put("DisabledSystemFields", oldDisabledSystemFields); + List disabledSystemFieldsUpdate = kind.getDisabledSystemFields(update.getDisabledSystemFields()); + if (disabledSystemFieldsUpdate != null && !disabledSystemFieldsUpdate.isEmpty()) + newProps.put("DisabledSystemFields", disabledSystemFieldsUpdate); + + d.setDisabledSystemFields(disabledSystemFieldsUpdate); + + Set baseIndices = kind.getPropertyIndices(d); + List oldIndices = GWTIndex.toStringVals(orig.getIndices(), baseIndices); + if (oldIndices != null && !oldIndices.isEmpty()) + oldProps.put("Indices", oldIndices); + List newIndices = GWTIndex.toStringVals(update.getIndices(), baseIndices); + if (newIndices != null && !newIndices.isEmpty()) + newProps.put("Indices", newIndices); // NOTE that DomainImpl.save() does an optimistic concurrency check, but we still need to check here. // This code is diff'ing two GWTDomains and applying those changes to Domain d. We need to make sure we're @@ -908,7 +940,7 @@ public static ValidationException updateDomainDescriptor(GWTDomain(defaultValues); try @@ -1185,11 +1217,13 @@ private static void _copyProperties(DomainProperty to, GWTPropertyDescriptor fro to.setDerivationDataScope(from.getDerivationDataScope()); } - private static List> updatePropertyValidators(DomainProperty dp, GWTPropertyDescriptor oldPd, GWTPropertyDescriptor newPd) + private static List> updatePropertyValidators(DomainProperty dp, @Nullable GWTPropertyDescriptor oldPd, GWTPropertyDescriptor newPd) { Map newProps = new HashMap<>(); List> valueUpdates = new ArrayList<>(); + PropertyDescriptor oldPropertyDescriptor = dp.getPropertyDescriptor().clone(); + boolean hasChange = false; for (GWTPropertyValidator v : newPd.getPropertyValidators()) { if (v.getRowId() != 0) @@ -1201,6 +1235,7 @@ private static List> updatePropertyValidators(DomainProperty _copyValidator(pv, v); dp.addValidator(pv); + hasChange = true; } if (v.getExtraProperties() != null && v.getExtraProperties().containsKey("valueUpdates")) @@ -1219,21 +1254,28 @@ private static List> updatePropertyValidators(DomainProperty if (v.equals(prop)) newProps.remove(v.getRowId()); else if (prop == null) + { deleted.add(v); + hasChange = true; + } } // update any new or changed for (IPropertyValidator pv : dp.getValidators()) - _copyValidator(pv, newProps.get(pv.getRowId())); + { + boolean change = _copyValidator(pv, newProps.get(pv.getRowId())); + hasChange = hasChange || change; + } // deal with removed validators for (GWTPropertyValidator gpv : deleted) dp.removeValidator(gpv.getRowId()); - - return valueUpdates; } - return null; + if (hasChange) + dp.setOldPropertyDescriptor(oldPropertyDescriptor); // mark dirty as needed + + return oldPd != null ? valueUpdates : null; } private static void updateTextChoiceValueRows(Domain domain, User user, String propName, Map valueUpdates, ValidationException errors) @@ -1301,20 +1343,28 @@ private static void updateTextChoiceValueRows(Domain domain, User user, String p } } - private static void _copyValidator(IPropertyValidator pv, GWTPropertyValidator gpv) + private static boolean _copyValidator(IPropertyValidator pv, GWTPropertyValidator gpv) { + boolean hasChange = false; if (pv != null && gpv != null) { + hasChange = !Objects.equals(pv.getName(), gpv.getName()); pv.setName(gpv.getName()); + hasChange = hasChange || !Objects.equals(pv.getDescription(), gpv.getDescription()); pv.setDescription(gpv.getDescription()); + hasChange = hasChange || !Objects.equals(pv.getExpressionValue(), gpv.getExpression()); pv.setExpressionValue(gpv.getExpression()); + hasChange = hasChange || !Objects.equals(pv.getErrorMessage(), gpv.getErrorMessage()); pv.setErrorMessage(gpv.getErrorMessage()); + hasChange = hasChange || !Objects.equals(PageFlowUtil.toQueryString(pv.getProperties().entrySet()), PageFlowUtil.toQueryString(gpv.getProperties().entrySet())); for (Map.Entry entry : gpv.getProperties().entrySet()) { pv.setProperty(entry.getKey(), entry.getValue()); } } + + return hasChange; } private static String getDomainErrorMessage(@Nullable GWTDomain domain, String message) diff --git a/api/src/org/labkey/api/exp/property/TestDomainKind.java b/api/src/org/labkey/api/exp/property/TestDomainKind.java index 4a230d9be59..47fbf4aeacd 100644 --- a/api/src/org/labkey/api/exp/property/TestDomainKind.java +++ b/api/src/org/labkey/api/exp/property/TestDomainKind.java @@ -172,13 +172,13 @@ public Domain createDomain(GWTDomain domain, JSONObject arguments, Container con @Override public ValidationException updateDomain(GWTDomain original, GWTDomain update, - @Nullable JSONObject options, Container container, User user, boolean includeWarnings) + @Nullable JSONObject options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { throw new UnsupportedOperationException(); } @Override - public void deleteDomain(User user, Domain domain) + public void deleteDomain(User user, Domain domain, @Nullable String auditUserComment) { throw new UnsupportedOperationException(); } diff --git a/api/src/org/labkey/api/issues/AbstractIssuesListDefDomainKind.java b/api/src/org/labkey/api/issues/AbstractIssuesListDefDomainKind.java index e8453b11f6f..96ff421afb4 100644 --- a/api/src/org/labkey/api/issues/AbstractIssuesListDefDomainKind.java +++ b/api/src/org/labkey/api/issues/AbstractIssuesListDefDomainKind.java @@ -212,13 +212,13 @@ public static String getDomainURI(String schemaName, String tableName, String na public abstract void beforeDeleteDomain(User user, Domain domain); @Override - public final void deleteDomain(User user, Domain domain) + public final void deleteDomain(User user, Domain domain, @Nullable String auditUserComment) { try (DbScope.Transaction transaction = IssuesSchema.getInstance().getSchema().getScope().ensureTransaction()) { IssuesListDefService.get().deleteIssueDefsForDomain(user, domain); beforeDeleteDomain(user, domain); - domain.delete(user); + domain.delete(user, auditUserComment); transaction.commit(); } @@ -288,7 +288,7 @@ public Class getTypeClass() @Override public @NotNull ValidationException updateDomain(GWTDomain original, GWTDomain update, - @Nullable IssuesDomainKindProperties options, Container container, User user, boolean includeWarnings) + @Nullable IssuesDomainKindProperties options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { if (options != null && StringUtils.isBlank(options.getIssueDefName())) return new ValidationException("Issue name must not be null."); diff --git a/api/src/org/labkey/api/query/ExtendedTableDomainKind.java b/api/src/org/labkey/api/query/ExtendedTableDomainKind.java index d5f2d445b21..5b91f78925c 100644 --- a/api/src/org/labkey/api/query/ExtendedTableDomainKind.java +++ b/api/src/org/labkey/api/query/ExtendedTableDomainKind.java @@ -59,7 +59,7 @@ public Domain updateDomain(Container container, User user, GWTDomain getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + if (!StringUtils.isEmpty(getName())) + map.put("Name", getName()); + if (!StringUtils.isEmpty(getLabel())) + map.put("Label", getLabel()); + if (!StringUtils.isEmpty(getValueExpression())) + map.put("ValueExpression", getValueExpression()); + if (getScale() != null) + map.put("Scale", getScale()); + if (!StringUtils.isEmpty(getDescription())) + map.put("Description", getDescription()); + if (!StringUtils.isEmpty(getFormat())) + map.put("Format", getFormat()); + if (!StringUtils.isEmpty(getURL())) + map.put("URL", getURL()); + if (!StringUtils.isEmpty(getPHI())) + map.put("PHI", getPHI()); + if (!StringUtils.isEmpty(getDefaultScale())) + map.put("DefaultScale", getDefaultScale()); + map.put("Required", isRequired()); + map.put("Hidden", isHidden()); + map.put("Measure", isMeasure()); + map.put("Dimension", isDimension()); + map.put("ShownInInsert", isShownInInsertView()); + map.put("ShownInDetails", isShownInDetailsView()); + map.put("ShownInUpdate", isShownInUpdateView()); + map.put("RecommendedVariable", isRecommendedVariable()); + map.put("ExcludedFromShifting", isExcludeFromShifting()); + map.put("Scannable", isScannable()); + if (!StringUtils.isEmpty(getDerivationDataScope())) + map.put("DerivationDataScope", getDerivationDataScope()); + if (!StringUtils.isEmpty(getImportAliases())) + map.put("ImportAliases", getImportAliases()); + String conditionalFormatStr = ConditionalFormat.toStringVal(getConditionalFormats()); + if (!StringUtils.isEmpty(conditionalFormatStr)) + map.put("ConditionalFormat", conditionalFormatStr); + + return map; + } + } \ No newline at end of file diff --git a/api/src/org/labkey/api/study/Dataset.java b/api/src/org/labkey/api/study/Dataset.java index b44108ecaeb..cd13190e51d 100644 --- a/api/src/org/labkey/api/study/Dataset.java +++ b/api/src/org/labkey/api/study/Dataset.java @@ -364,6 +364,8 @@ public String getRecallFromStudyAuditMessage(String label, int recordCount) void delete(User user); + void delete(User user, @Nullable String auditUserComment); + void deleteAllRows(User user); /** diff --git a/assay/src/org/labkey/assay/AssayDomainServiceImpl.java b/assay/src/org/labkey/assay/AssayDomainServiceImpl.java index ebd0d26da75..64eb534d760 100644 --- a/assay/src/org/labkey/assay/AssayDomainServiceImpl.java +++ b/assay/src/org/labkey/assay/AssayDomainServiceImpl.java @@ -79,6 +79,7 @@ import java.io.File; import java.util.ArrayList; +import java.util.Collection; import java.util.HashMap; import java.util.HashSet; import java.util.LinkedHashMap; @@ -416,6 +417,10 @@ public GWTProtocol saveChanges(GWTProtocol assay, boolean replaceIfExisting) thr if (assay.getAutoLinkCategory() != null && assay.getAutoLinkCategory().length() > 200) throw new ValidationException("Linked Dataset Category name must be shorter than 200 characters."); + Map newProps = assay.getAuditRecordMap(); + Map oldProps = new LinkedHashMap<>(); + + StringBuilder changeDetails = new StringBuilder(); ExpProtocol protocol; boolean isNew = assay.getProtocolId() == null; boolean hasNameChange = false; @@ -451,7 +456,7 @@ public GWTProtocol saveChanges(GWTProtocol assay, boolean replaceIfExisting) thr else { GWTDomain previous = DomainUtil.getDomainDescriptor(getUser(), domain.getDomainURI(), protocol.getContainer()); - updateDomainDescriptor(assayProvider, protocol, previous, domain, false); + updateDomainDescriptor(assayProvider, protocol, previous, domain, false, null, assay.getAuditUserComment(), oldProps, newProps); domainURIs.add(domain.getDomainURI()); } } @@ -464,6 +469,8 @@ public GWTProtocol saveChanges(GWTProtocol assay, boolean replaceIfExisting) thr if (protocol == null) throw new ValidationException("Assay design has been deleted"); + oldProps = protocol.getAuditRecordMap(AssayService.get().getProvider(protocol)); + // ensure that the user has edit perms in this container if (!canUpdateProtocols()) throw new ValidationException("You do not have sufficient permissions to update this Assay"); @@ -475,6 +482,7 @@ public GWTProtocol saveChanges(GWTProtocol assay, boolean replaceIfExisting) thr hasNameChange = !assay.getName().equals(oldAssayName); if (hasNameChange) { + changeDetails.append("The name of the assay domain '" + oldAssayName + "' was changed to '" + assay.getName() + "'."); String nameError = DomainUtil.validateDomainName(assay.getName(), "Assay Design", false); if (nameError != null) throw new ValidationException(nameError); @@ -556,10 +564,27 @@ public GWTProtocol saveChanges(GWTProtocol assay, boolean replaceIfExisting) thr if (detectionMethod == null) throw new ValidationException("The selected detection method could not be found."); + if (!isNew) + { + String oldDetectionMethod = dmProvider.getSelectedDetectionMethod(getContainer(), protocol); + if (oldDetectionMethod != null && !StringUtils.isEmpty(oldDetectionMethod)) + oldProps.put("DetectionMethod", oldDetectionMethod); + } + if (!StringUtils.isEmpty(detectionMethod)) + newProps.put("DetectionMethod", detectionMethod); dmProvider.setSelectedDetectionMethod(getContainer(), protocol, detectionMethod); } - ValidationException scriptValidation = provider.setValidationAndAnalysisScripts(protocol, transformScripts); + Pair> scriptValidationResult = provider.setValidationAndAnalysisScripts(protocol, transformScripts); + ValidationException scriptValidation = scriptValidationResult.first; + Pair transformChanges = scriptValidationResult.second; + if (transformChanges != null) + { + if (!isNew && !StringUtils.isEmpty(transformChanges.first)) + oldProps.put("TransformScripts", transformChanges.first); + if (!StringUtils.isEmpty(transformChanges.second)) + newProps.put("TransformScripts", transformChanges.second); + } if (scriptValidation.hasErrors()) { for (var error : scriptValidation.getErrors()) @@ -605,21 +630,26 @@ public GWTProtocol saveChanges(GWTProtocol assay, boolean replaceIfExisting) thr protocol.save(getUser()); + if (assay.getExcludedContainerIds() != null && (!isNew || !assay.getExcludedContainerIds().isEmpty())) + { + Pair, Collection> exclusionChanges = ExperimentService.get().ensureDataTypeContainerExclusions(ExperimentService.DataTypeForExclusion.AssayDesign, assay.getExcludedContainerIds(), protocol.getRowId(), getUser()); + if (!isNew) + oldProps.put("ContainerExclusions", exclusionChanges.first); + newProps.put("ContainerExclusions", exclusionChanges.second); + } + else + ExperimentService.get().ensureDataTypeContainerExclusionsNonAdmin(ExperimentService.DataTypeForExclusion.AssayDesign, protocol.getRowId(), getContainer(), getUser()); + for (GWTDomain domain : assay.getDomains()) { GWTDomain previous = DomainUtil.getDomainDescriptor(getUser(), domain.getDomainURI(), protocol.getContainer()); - updateDomainDescriptor(provider, protocol, previous, domain, hasNameChange); + updateDomainDescriptor(provider, protocol, previous, domain, hasNameChange, changeDetails.toString(), assay.getAuditUserComment(), oldProps, newProps); boolean hasExistingCalcFields = previous != null && !previous.getCalculatedFields().isEmpty(); GWTDomain savedDomain = DomainUtil.getDomainDescriptor(getUser(), domain.getDomainURI(), protocol.getContainer()); QueryService.get().saveCalculatedFieldsMetadata(savedDomain.getSchemaName(), savedDomain.getQueryName(), null, domain.getCalculatedFields(), hasExistingCalcFields, getUser(), protocol.getContainer()); } - if (assay.getExcludedContainerIds() != null && (!isNew || !assay.getExcludedContainerIds().isEmpty())) - ExperimentService.get().ensureDataTypeContainerExclusions(ExperimentService.DataTypeForExclusion.AssayDesign, assay.getExcludedContainerIds(), protocol.getRowId(), getUser()); - else - ExperimentService.get().ensureDataTypeContainerExclusionsNonAdmin(ExperimentService.DataTypeForExclusion.AssayDesign, protocol.getRowId(), getContainer(), getUser()); - QueryService.get().updateLastModified(); transaction.commit(); AssayManager.get().clearProtocolCache(); @@ -642,7 +672,11 @@ private void updateDomainDescriptor( ExpProtocol protocol, GWTDomain original, GWTDomain update, - boolean hasNameChange + boolean hasNameChange, + String auditComment, + @Nullable String auditUserComment, + @Nullable Map oldRecordMap, + @Nullable Map newRecordMap ) throws ValidationException { for (GWTPropertyDescriptor prop : update.getFields()) @@ -651,15 +685,11 @@ private void updateDomainDescriptor( prop.setLookupQuery(prop.getLookupQuery().replace(AbstractAssayProvider.ASSAY_NAME_SUBSTITUTION, protocol.getName())); } - String auditComment = null; - if (hasNameChange) - auditComment = "The name of the assay domain '" + original.getName() + "' was changed to '" + update.getName() + "'."; - // Before update provider.beforeDomainChange(getUser(), protocol, original, update); // Update - ValidationException validationErrors = DomainUtil.updateDomainDescriptor(original, update, getContainer(), getUser(), hasNameChange, auditComment); + ValidationException validationErrors = DomainUtil.updateDomainDescriptor(original, update, getContainer(), getUser(), hasNameChange, auditComment, auditUserComment, oldRecordMap, newRecordMap); if (validationErrors.hasErrors()) throw validationErrors; diff --git a/assay/src/org/labkey/assay/PlateBasedAssaySampleTypeDomainKind.java b/assay/src/org/labkey/assay/PlateBasedAssaySampleTypeDomainKind.java index d6155b1dab3..74b22a0b62f 100644 --- a/assay/src/org/labkey/assay/PlateBasedAssaySampleTypeDomainKind.java +++ b/assay/src/org/labkey/assay/PlateBasedAssaySampleTypeDomainKind.java @@ -157,10 +157,10 @@ public Domain createDomain(GWTDomain domain, SampleTypeDomainKindProperties argu @NotNull @Override - public ValidationException updateDomain(GWTDomain original, GWTDomain update, @Nullable SampleTypeDomainKindProperties options, Container container, User user, boolean includeWarnings) + public ValidationException updateDomain(GWTDomain original, GWTDomain update, @Nullable SampleTypeDomainKindProperties options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { JSONObject args = options != null ? options.toJSONObject() : null; - return _assayDelegate.updateDomain(original, update, args, container, user, includeWarnings); + return _assayDelegate.updateDomain(original, update, args, container, user, includeWarnings, auditUserComment); } @Override diff --git a/core/package-lock.json b/core/package-lock.json index 2ec879018ad..82c15dada19 100644 --- a/core/package-lock.json +++ b/core/package-lock.json @@ -8,7 +8,7 @@ "name": "labkey-core", "version": "0.0.0", "dependencies": { - "@labkey/components": "6.43.1", + "@labkey/components": "6.43.2", "@labkey/themes": "1.4.2" }, "devDependencies": { @@ -2987,9 +2987,9 @@ } }, "node_modules/@labkey/api": { - "version": "1.41.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.41.0.tgz", - "integrity": "sha512-FWhELnNLkTVNNGXOnPHFoLb/CvPKVnibvVnINGETh1rBUmp8UdGrSV2OuhhPEkuGoi/SGp2zI3dkkSYjRR20Eg==" + "version": "1.41.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.41.1.tgz", + "integrity": "sha512-5CaGXA0Z2m/D7lDf8F8gZugQVxcWURpRsEdU+Xc/xn4Vz5ZdLjfgzyD87WsEtwNEi0Ed8Lw/BpsToFh80g+3IA==" }, "node_modules/@labkey/build": { "version": "8.5.0", @@ -3029,12 +3029,12 @@ } }, "node_modules/@labkey/components": { - "version": "6.43.1", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.43.1.tgz", - "integrity": "sha512-Rh/Pq8BwhwI0Iz8s9iR+ygucGrf0xGLDyria574ZgYLhih7DtKuN38isxQAkfBwzDf3/TBgF9Vxm4z7TmAn/yA==", + "version": "6.43.2", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.43.2.tgz", + "integrity": "sha512-f2rv0XIDCRW44CWM1wFik3PMLqBgmmpr50FIVH9FApL+CK5hDK+MNx5gkad3OjWMeOkFq9KD4bIOAPFJIrTMEw==", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.41.0", + "@labkey/api": "1.41.1", "@testing-library/dom": "~10.4.0", "@testing-library/jest-dom": "~6.6.3", "@testing-library/react": "~16.3.0", diff --git a/core/package.json b/core/package.json index 50ba53e7f20..141e42fd0e0 100644 --- a/core/package.json +++ b/core/package.json @@ -53,7 +53,7 @@ } }, "dependencies": { - "@labkey/components": "6.43.1", + "@labkey/components": "6.43.2", "@labkey/themes": "1.4.2" }, "devDependencies": { diff --git a/core/src/client/DataClassDesigner/DataClassDesigner.tsx b/core/src/client/DataClassDesigner/DataClassDesigner.tsx index 91f712d2d50..4a19ca02075 100644 --- a/core/src/client/DataClassDesigner/DataClassDesigner.tsx +++ b/core/src/client/DataClassDesigner/DataClassDesigner.tsx @@ -106,7 +106,7 @@ class DataClassDesignerWrapper extends React.Component { onCancel={this.onCancel} onComplete={this.onComplete} onChange={this.onChange} - showGenIdBanner={isUpdate} + isUpdate={isUpdate} /> ) diff --git a/core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx b/core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx index 6077961f732..e3b0e084b06 100644 --- a/core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx +++ b/core/src/client/SampleTypeDesigner/SampleTypeDesigner.tsx @@ -164,7 +164,7 @@ class SampleTypeDesignerWrapper extends React.PureComponent { onChange={this.onChange} includeDataClasses={true} showLinkToStudy={true} - showGenIdBanner={isUpdate} + isUpdate={isUpdate} /> ) diff --git a/experiment/package-lock.json b/experiment/package-lock.json index 0b6c9095532..7a0b65181e8 100644 --- a/experiment/package-lock.json +++ b/experiment/package-lock.json @@ -8,7 +8,7 @@ "name": "experiment", "version": "0.0.0", "dependencies": { - "@labkey/components": "6.43.1" + "@labkey/components": "6.43.2" }, "devDependencies": { "@labkey/build": "8.5.0", @@ -2800,9 +2800,9 @@ } }, "node_modules/@labkey/api": { - "version": "1.41.0", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.41.0.tgz", - "integrity": "sha512-FWhELnNLkTVNNGXOnPHFoLb/CvPKVnibvVnINGETh1rBUmp8UdGrSV2OuhhPEkuGoi/SGp2zI3dkkSYjRR20Eg==" + "version": "1.41.1", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/api/-/@labkey/api-1.41.1.tgz", + "integrity": "sha512-5CaGXA0Z2m/D7lDf8F8gZugQVxcWURpRsEdU+Xc/xn4Vz5ZdLjfgzyD87WsEtwNEi0Ed8Lw/BpsToFh80g+3IA==" }, "node_modules/@labkey/build": { "version": "8.5.0", @@ -2842,12 +2842,12 @@ } }, "node_modules/@labkey/components": { - "version": "6.43.1", - "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.43.1.tgz", - "integrity": "sha512-Rh/Pq8BwhwI0Iz8s9iR+ygucGrf0xGLDyria574ZgYLhih7DtKuN38isxQAkfBwzDf3/TBgF9Vxm4z7TmAn/yA==", + "version": "6.43.2", + "resolved": "https://labkey.jfrog.io/artifactory/api/npm/libs-client/@labkey/components/-/@labkey/components-6.43.2.tgz", + "integrity": "sha512-f2rv0XIDCRW44CWM1wFik3PMLqBgmmpr50FIVH9FApL+CK5hDK+MNx5gkad3OjWMeOkFq9KD4bIOAPFJIrTMEw==", "dependencies": { "@hello-pangea/dnd": "18.0.1", - "@labkey/api": "1.41.0", + "@labkey/api": "1.41.1", "@testing-library/dom": "~10.4.0", "@testing-library/jest-dom": "~6.6.3", "@testing-library/react": "~16.3.0", diff --git a/experiment/package.json b/experiment/package.json index 361dd049415..a46d2da1c0b 100644 --- a/experiment/package.json +++ b/experiment/package.json @@ -13,7 +13,7 @@ "test-integration": "cross-env NODE_ENV=test jest --ci --runInBand -c test/js/jest.config.integration.js" }, "dependencies": { - "@labkey/components": "6.43.1" + "@labkey/components": "6.43.2" }, "devDependencies": { "@labkey/build": "8.5.0", diff --git a/experiment/src/org/labkey/experiment/ExpDataFileListener.java b/experiment/src/org/labkey/experiment/ExpDataFileListener.java index 5853c078bb7..76f6374869a 100644 --- a/experiment/src/org/labkey/experiment/ExpDataFileListener.java +++ b/experiment/src/org/labkey/experiment/ExpDataFileListener.java @@ -76,7 +76,7 @@ public int fileMoved(@NotNull Path src, @NotNull Path dest, @Nullable User user, // if the data object moved containers, set that as well if (targetContainer != null && !targetContainer.equals(sourceContainer)) data.setContainer(targetContainer); - data.save(user); + data.save(user); // FIXME: Issue 53070: Moving files could result in duplicate exp.data records extra = 1; } diff --git a/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java b/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java index 3b2871877a3..05e50c23ba0 100644 --- a/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java +++ b/experiment/src/org/labkey/experiment/api/DataClassDomainKind.java @@ -402,23 +402,24 @@ public Domain createDomain(GWTDomain domain, DataClassDom @Nullable DataClassDomainKindProperties options, Container container, User user, - boolean includeWarnings + boolean includeWarnings, + String auditUserComment ) { ExpDataClass dc = ExperimentService.get().getDataClass(original.getDomainURI()); if (dc == null) return new ValidationException(String.format("Could not resolve data class from LSID \"%s\".", original.getDomainURI())); - return ExperimentService.get().updateDataClass(container, user, dc, options, original, update); + return ExperimentService.get().updateDataClass(container, user, dc, options, original, update, auditUserComment); } @Override - public void deleteDomain(User user, Domain domain) + public void deleteDomain(User user, Domain domain, @Nullable String auditUserComment) { ExpDataClass dc = getDataClass(domain); if (dc == null) throw new NotFoundException("DataClass not found: " + domain.getTypeURI()); - dc.delete(user); + dc.delete(user, auditUserComment); } @Override diff --git a/experiment/src/org/labkey/experiment/api/ExpDataClassImpl.java b/experiment/src/org/labkey/experiment/api/ExpDataClassImpl.java index 958789d8c7f..abea8c260a8 100644 --- a/experiment/src/org/labkey/experiment/api/ExpDataClassImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpDataClassImpl.java @@ -62,6 +62,7 @@ import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -494,6 +495,7 @@ public void ensureMinGenId(long newSeqValue, Container c) throws ExperimentExcep } } + @Override public String getImportAliasJson() { return _object.getDataParentImportAliasMap(); @@ -546,4 +548,27 @@ public void setImportAliasMapJson(String aliasJson) _object.setDataParentImportAliasMap(aliasJson); } + public Map getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + if (!StringUtils.isEmpty(getNameExpression())) + map.put("NameExpression", getNameExpression()); + String importAliasStr = null; + try + { + importAliasStr = ExperimentJSONConverter.getImportAliasStringVal(getImportAliasMap()); + } + catch (IOException ignore) + { + } + if (!StringUtils.isEmpty(importAliasStr)) + map.put("ImportAlias", importAliasStr); + if (!StringUtils.isEmpty(getCategory())) + map.put("Category", getCategory()); + if (getSampleType() != null) + map.put("SampleType", getSampleType().getRowId()); + + return map; + } + } diff --git a/experiment/src/org/labkey/experiment/api/ExpProtocolImpl.java b/experiment/src/org/labkey/experiment/api/ExpProtocolImpl.java index 9d8507397b9..a4747bbbf93 100644 --- a/experiment/src/org/labkey/experiment/api/ExpProtocolImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpProtocolImpl.java @@ -16,7 +16,9 @@ package org.labkey.experiment.api; +import org.apache.commons.lang3.StringUtils; import org.jetbrains.annotations.Nullable; +import org.labkey.api.assay.AssayProvider; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.Filter; @@ -44,12 +46,15 @@ import org.labkey.api.query.QueryRowReference; import org.labkey.api.query.RuntimeValidationException; import org.labkey.api.security.User; +import org.labkey.api.study.publish.StudyPublishService; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.view.ActionURL; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -475,4 +480,33 @@ public void setStatus(Status status) { _object.setStatus(status); } + + @Override + public Map getAuditRecordMap(AssayProvider provider) + { + Map map = new LinkedHashMap<>(); + map.put("Name", getName()); + if (!StringUtils.isEmpty(getProtocolDescription())) + map.put("Description", getProtocolDescription()); + if (null != getStatus()) + map.put("Status", getStatus().toString()); + Map props = new HashMap<>(getObjectProperties()); + ObjectProperty autoLinkTargetContainer = props.get(StudyPublishService.AUTO_LINK_TARGET_PROPERTY_URI); + String autoLinkTargetContainerId = autoLinkTargetContainer == null ? null : autoLinkTargetContainer.getStringValue(); + if (!StringUtils.isEmpty(autoLinkTargetContainerId)) + map.put("AutoCopyTargetContainer", autoLinkTargetContainerId); + ObjectProperty autoLinkCategory = props.get(StudyPublishService.AUTO_LINK_CATEGORY_PROPERTY_URI); + String autoLinkCategoryValue = autoLinkCategory == null ? null : autoLinkCategory.getStringValue(); + if (!StringUtils.isEmpty(autoLinkCategoryValue)) + map.put("AutoLinkCategory", autoLinkCategoryValue); + map.put("SaveScriptFiles", provider.isSaveScriptFiles(this)); + map.put("IsEditableResults", provider.isEditableResults(this)); + map.put("IsEditableRuns", provider.isEditableRuns(this)); + map.put("IsBackgroundUpload", provider.isBackgroundUpload(this)); + map.put("IsQcEnabled", provider.isQCEnabled(this)); + map.put("IsPlateMetadataEnabled", provider.isPlateMetadataEnabled(this)); + return map; + } + + } diff --git a/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java b/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java index 34cf8e31b4b..f3f31fc9205 100644 --- a/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExpSampleTypeImpl.java @@ -79,6 +79,7 @@ import java.util.Date; import java.util.HashMap; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; @@ -920,7 +921,7 @@ public void delete(User user) } @Override - public void delete(User user, String auditUserComment) + public void delete(User user, @Nullable String auditUserComment) { try { @@ -1096,4 +1097,37 @@ public boolean isMedia() { return ExpSchema.SampleTypeCategoryType.media.name().equalsIgnoreCase(getCategory()); } + + public Map getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + if (!StringUtils.isEmpty(getName())) + map.put("Name", getName()); + if (!StringUtils.isEmpty(getNameExpression())) + map.put("NameExpression", getNameExpression()); + if (!StringUtils.isEmpty(getAliquotNameExpression())) + map.put("AliquotNameExpression", getAliquotNameExpression()); + if (!StringUtils.isEmpty(getLabelColor())) + map.put("LabelColor", getLabelColor()); + if (!StringUtils.isEmpty(getMetricUnit())) + map.put("MetricUnit", getMetricUnit()); + String importAliasStr = null; + try + { + importAliasStr = ExperimentJSONConverter.getImportAliasStringVal(getImportAliasMap()); + } + catch (IOException ignore) + { + } + if (!StringUtils.isEmpty(importAliasStr)) + map.put("ImportAlias", importAliasStr); + if (getAutoLinkTargetContainer() != null) + map.put("AutoLinkTargetContainerId", getAutoLinkTargetContainer().getId()); + if (!StringUtils.isEmpty(getAutoLinkCategory())) + map.put("AutoLinkCategory", getAutoLinkCategory()); + if (!StringUtils.isEmpty(getCategory())) + map.put("Category", getCategory()); + + return map; + } } diff --git a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java index 92630371379..2e8a6b23471 100644 --- a/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java @@ -4504,7 +4504,7 @@ public List getExpRunsForProtocolIds(boolean includeRelated, @NotNul return ExpRunImpl.fromRuns(new SqlSelector(getExpSchema(), sb).getArrayList(ExperimentRun.class)); } - public void deleteProtocolByRowIds(Container c, User user, String auditUserComment, int... selectedProtocolIds) throws ExperimentException + public void deleteProtocolByRowIds(Container c, User user, @Nullable String auditUserComment, int... selectedProtocolIds) throws ExperimentException { if (selectedProtocolIds.length == 0) return; @@ -4546,7 +4546,7 @@ public void deleteProtocolByRowIds(Container c, User user, String auditUserComme { for (Dataset dataset : StudyPublishService.get().getDatasetsForPublishSource(protocolToDelete.getRowId(), Dataset.PublishSource.Assay)) { - dataset.delete(user); + dataset.delete(user, auditUserComment); } } else @@ -7933,7 +7933,7 @@ else if (domain.getPropertyByName(propertyName) != null) // issue 25275 if (kind != null) domain.setPropertyForeignKeys(kind.getPropertyForeignKeys(c)); - domain.save(u); + domain.save(u, options == null ? null : options.getAuditRecordMap(), calculatedFields); impl.save(u); SchemaKey schemaKey = SchemaKey.fromParts(ExpSchema.SCHEMA_NAME, DataClassUserSchema.NAME); @@ -7960,28 +7960,44 @@ else if (domain.getPropertyByName(propertyName) != null) // issue 25275 } @Override - public ValidationException updateDataClass(@NotNull Container c, @NotNull User u, @NotNull ExpDataClass dataClass, + public ValidationException updateDataClass(@NotNull Container c, @NotNull User u, @NotNull ExpDataClass dc, @Nullable DataClassDomainKindProperties properties, GWTDomain original, - GWTDomain update) + GWTDomain update, + @Nullable String auditUserComment) { + ExpDataClassImpl dataClass = (ExpDataClassImpl) dc; + + Map oldProps = dataClass.getAuditRecordMap(); + Map newProps = properties != null ? properties.getAuditRecordMap() : dataClass.getAuditRecordMap() /* no update */; + ValidationException errors; + StringBuilder changeDetails = new StringBuilder(); // if options doesn't have a rowId value, then it is just coming from the property-editDomain action only only updating domain fields DataClassDomainKindProperties options = properties != null && properties.getRowId() == dataClass.getRowId() ? properties : null; boolean hasNameChange = false; String oldDataClassName = dataClass.getName(); String newName = null; + + oldProps.put("Name", oldDataClassName); + newProps.put("Name", oldDataClassName); // to be updated by options + oldProps.put("Description", dataClass.getDescription()); + newProps.put("Description", dataClass.getDescription()); // to be updated by options + if (options != null) { validateDataClassOptions(c, u, options); newName = StringUtils.trimToNull(options.getName()); + newProps.put("Name", newName); if (!oldDataClassName.equals(newName)) { validateDataClassName(c, u, newName, oldDataClassName.equalsIgnoreCase(newName)); hasNameChange = true; dataClass.setName(newName); + changeDetails.append("The name of the data class '" + oldDataClassName + "' was changed to '" + newName + "'."); } + newProps.put("Description", options.getDescription()); dataClass.setDescription(options.getDescription()); dataClass.setNameExpression(options.getNameExpression()); dataClass.setSampleType(options.getSampleType()); @@ -8012,24 +8028,24 @@ public ValidationException updateDataClass(@NotNull Container c, @NotNull User u LOG.debug("Saving data class " + dataClass.getName()); dataClass.save(u); - String auditComment = null; SchemaKey schemaKey = SchemaKey.fromParts(ExpSchema.SCHEMA_NAME, DataClassUserSchema.NAME); if (hasNameChange) - { QueryChangeListener.QueryPropertyChange.handleQueryNameChange(oldDataClassName, newName, schemaKey, u, c); - auditComment = "The name of the data class '" + oldDataClassName + "' was changed to '" + newName + "'."; + + if (options != null && options.getExcludedContainerIds() != null) + { + Pair, Collection> exclusionChanges = ExperimentService.get().ensureDataTypeContainerExclusions(DataTypeForExclusion.DataClass, options.getExcludedContainerIds(), dataClass.getRowId(), u); + oldProps.put("ContainerExclusions", exclusionChanges.first); + newProps.put("ContainerExclusions", exclusionChanges.second); } - errors = DomainUtil.updateDomainDescriptor(original, update, c, u, hasNameChange, auditComment); + errors = DomainUtil.updateDomainDescriptor(original, update, c, u, hasNameChange, changeDetails.toString(), auditUserComment, oldProps, newProps); QueryService.get().saveCalculatedFieldsMetadata(schemaKey.toString(), update.getQueryName(), hasNameChange ? newName : null, update.getCalculatedFields(), !original.getCalculatedFields().isEmpty(), u, c); if (hasNameChange) addObjectLegacyName(dataClass.getObjectId(), ExperimentServiceImpl.getNamespacePrefix(ExpDataClass.class), oldDataClassName, u); - if (options != null && options.getExcludedContainerIds() != null) - ExperimentService.get().ensureDataTypeContainerExclusions(DataTypeForExclusion.DataClass, options.getExcludedContainerIds(), dataClass.getRowId(), u); - if (!errors.hasErrors()) { transaction.addCommitTask(() -> clearDataClassCache(c), DbScope.CommitTaskOption.IMMEDIATE, POSTCOMMIT, POSTROLLBACK); @@ -8854,12 +8870,14 @@ public void ensureContainerDataTypeExclusions(@NotNull DataTypeForExclusion data } @Override - public void ensureDataTypeContainerExclusions(@NotNull DataTypeForExclusion dataType, @Nullable Collection excludedContainerIds, @NotNull Integer dataTypeId, User user) + @NotNull + public Pair, Collection> ensureDataTypeContainerExclusions(@NotNull DataTypeForExclusion dataType, @Nullable Collection excludedContainerIds, @NotNull Integer dataTypeId, User user) { + Set previousExclusions = getDataTypeContainerExclusions(dataType, dataTypeId); + if (excludedContainerIds == null) - return; + return new Pair<>(previousExclusions, null); - Set previousExclusions = getDataTypeContainerExclusions(dataType, dataTypeId); Set updatedExclusions = new HashSet<>(excludedContainerIds); Set toAdd = new HashSet<>(updatedExclusions); @@ -8885,6 +8903,8 @@ public void ensureDataTypeContainerExclusions(@NotNull DataTypeForExclusion data addAuditEventForDataTypeContainerUpdate(dataType, remove, user); } } + + return new Pair<>(new TreeSet<>(previousExclusions), new TreeSet<>(updatedExclusions)); } @Override @@ -9586,7 +9606,7 @@ public Map moveDataClassObjects(Collection d return updateCounts; } - private void addDataClassSummaryAuditEvent(User user, Container container, TableInfo dataClassTable, int rowCount, String auditUserComment) + private void addDataClassSummaryAuditEvent(User user, Container container, TableInfo dataClassTable, int rowCount, @Nullable String auditUserComment) { QueryService queryService = QueryService.get(); queryService.getDefaultAuditHandler().addSummaryAuditEvent(user, container, dataClassTable, QueryService.AuditAction.UPDATE, rowCount, AuditBehaviorType.SUMMARY, auditUserComment); diff --git a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java index ccc47bcedf6..b9b43484560 100644 --- a/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java +++ b/experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java @@ -593,7 +593,7 @@ public void deleteSampleType(int rowId, Container c, User user, @Nullable String { for (Dataset dataset : StudyPublishService.get().getDatasetsForPublishSource(rowId, Dataset.PublishSource.SampleType)) { - dataset.delete(user); + dataset.delete(user, auditUserComment); } } else @@ -602,7 +602,7 @@ public void deleteSampleType(int rowId, Container c, User user, @Nullable String } Domain d = source.getDomain(); - d.delete(user); + d.delete(user, auditUserComment); ExperimentServiceImpl.get().deleteDomainObjects(source.getContainer(), source.getLSID()); @@ -643,12 +643,12 @@ public void deleteSampleType(int rowId, Container c, User user, @Nullable String LOG.info("Deleted SampleType '" + source.getName() + "' from '" + c.getPath() + "' in " + timer.getDuration()); } - private void addSampleTypeDeletedAuditEvent(User user, Container c, ExpSampleType sampleType, Long txAuditId, String auditUserComment) + private void addSampleTypeDeletedAuditEvent(User user, Container c, ExpSampleType sampleType, Long txAuditId, @Nullable String auditUserComment) { addSampleTypeAuditEvent(user, c, sampleType, txAuditId, String.format("Sample Type deleted: %1$s", sampleType.getName()),auditUserComment, "delete type"); } - private void addSampleTypeAuditEvent(User user, Container c, ExpSampleType sampleType, Long txAuditId, String comment, String auditUserComment, String insertUpdateChoice) + private void addSampleTypeAuditEvent(User user, Container c, ExpSampleType sampleType, Long txAuditId, String comment, @Nullable String auditUserComment, String insertUpdateChoice) { SampleTypeAuditProvider.SampleTypeAuditEvent event = new SampleTypeAuditProvider.SampleTypeAuditEvent(c, comment); event.setUserComment(auditUserComment); @@ -696,7 +696,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); + 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); } @NotNull @@ -704,7 +704,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 List excludedContainerIds, @Nullable List excludedDashboardContainerIds, @Nullable Map changeDetails) throws ExperimentException { validateSampleTypeName(c, u, name, false); @@ -877,7 +877,7 @@ public ExpSampleTypeImpl createSampleType(Container c, User u, String name, Stri { try { - domain.save(u); + domain.save(u, changeDetails, calculatedFields); st.save(u); QueryService.get().saveCalculatedFieldsMetadata(SamplesSchema.SCHEMA_NAME, name, null, calculatedFields, false, u, c); DefaultValueService.get().setDefaultValues(domain.getContainer(), defaultValues); @@ -996,28 +996,44 @@ private void validateSampleTypeName(Container container, User user, String name, } @Override - public ValidationException updateSampleType(GWTDomain original, GWTDomain update, SampleTypeDomainKindProperties options, Container container, User user, boolean includeWarnings) + public ValidationException updateSampleType(GWTDomain original, GWTDomain update, SampleTypeDomainKindProperties options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { ValidationException errors; ExpSampleTypeImpl st = new ExpSampleTypeImpl(getMaterialSource(update.getDomainURI())); + StringBuilder changeDetails = new StringBuilder(); + + Map oldProps = new LinkedHashMap<>(); + Map newProps = new LinkedHashMap<>(); + String newName = StringUtils.trimToNull(update.getName()); String oldSampleTypeName = st.getName(); + oldProps.put("Name", oldSampleTypeName); + newProps.put("Name", newName); + boolean hasNameChange = false; if (!oldSampleTypeName.equals(newName)) { validateSampleTypeName(container, user, newName, oldSampleTypeName.equalsIgnoreCase(newName)); hasNameChange = true; st.setName(newName); + changeDetails.append("The name of the sample type '" + oldSampleTypeName + "' was changed to '" + newName + "'."); } String newDescription = StringUtils.trimToNull(update.getDescription()); String description = st.getDescription(); + if (StringUtils.isNotBlank(description)) + oldProps.put("Description", description); + if (StringUtils.isNotBlank(newDescription)) + newProps.put("Description", newDescription); if (description == null || !description.equals(newDescription)) - { st.setDescription(newDescription); - } + + Map oldProps_ = st.getAuditRecordMap(); + Map newProps_ = options != null ? options.getAuditRecordMap() : st.getAuditRecordMap() /* no update */; + newProps.putAll(newProps_); + oldProps.putAll(oldProps_); boolean hasMetricUnitChanged = false; @@ -1035,14 +1051,11 @@ public ValidationException updateSampleType(GWTDomain, Collection> exclusionChanges = ExperimentService.get().ensureDataTypeContainerExclusions(ExperimentService.DataTypeForExclusion.SampleType, options.getExcludedContainerIds(), st.getRowId(), user); + oldProps.put("ContainerExclusions", exclusionChanges.first); + newProps.put("ContainerExclusions", exclusionChanges.second); + } + if (options != null && options.getExcludedDashboardContainerIds() != null) + { + Pair, Collection> exclusionChanges = ExperimentService.get().ensureDataTypeContainerExclusions(ExperimentService.DataTypeForExclusion.DashboardSampleType, options.getExcludedDashboardContainerIds(), st.getRowId(), user); + oldProps.put("DashboardContainerExclusions", exclusionChanges.first); + newProps.put("DashboardContainerExclusions", exclusionChanges.second); } - errors = DomainUtil.updateDomainDescriptor(original, update, container, user, hasNameChange, auditComment); + errors = DomainUtil.updateDomainDescriptor(original, update, container, user, hasNameChange, changeDetails.toString(), auditUserComment, oldProps, newProps); if (!errors.hasErrors()) { @@ -1092,10 +1114,6 @@ public ValidationException updateSampleType(GWTDomain { @@ -1228,7 +1246,7 @@ else if (event.getSampleLsid() != null) SampleTimelineAuditEvent.SampleTimelineEventType timelineEventType = SampleTimelineAuditEvent.SampleTimelineEventType.getTypeFromAction(action); if (timelineEventType != null) eventMetadata.put(SAMPLE_TIMELINE_EVENT_TYPE, action); - event.setMetadata(AbstractAuditTypeProvider.encodeForDataMap(c, eventMetadata)); + event.setMetadata(AbstractAuditTypeProvider.encodeForDataMap(eventMetadata)); } return event; @@ -1249,7 +1267,7 @@ private SampleTimelineAuditEvent createAuditRecord(Container container, String c event.setSampleTypeId(type.getRowId()); } event.setUserComment(userComment); - event.setMetadata(AbstractAuditTypeProvider.encodeForDataMap(container, metadata)); + event.setMetadata(AbstractAuditTypeProvider.encodeForDataMap(metadata)); return event; } @@ -1896,8 +1914,8 @@ public Map moveSamples(Collection sample newRecordMap.put(fileUpdateData.fieldName, fileUpdateData.targetFile.getAbsolutePath()); }); } - event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(targetContainer, oldRecordMap)); - event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(targetContainer, newRecordMap)); + event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(oldRecordMap)); + event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(newRecordMap)); AuditLogService.get().addEvent(user, event); } } diff --git a/experiment/src/org/labkey/experiment/api/VocabularyDomainKind.java b/experiment/src/org/labkey/experiment/api/VocabularyDomainKind.java index 7aab012c22d..a1654378403 100644 --- a/experiment/src/org/labkey/experiment/api/VocabularyDomainKind.java +++ b/experiment/src/org/labkey/experiment/api/VocabularyDomainKind.java @@ -107,12 +107,12 @@ public boolean canDeleteDefinition(User user, Domain domain) } @Override - public void deleteDomain(User user, Domain domain) + public void deleteDomain(User user, Domain domain, @Nullable String auditUserComment) { try { if (domain.getContainer().hasPermission(user, DesignVocabularyPermission.class)) - domain.delete(user); + domain.delete(user, auditUserComment); } catch (DomainNotFoundException e) { diff --git a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java index e4801f6316e..3d264d3a62b 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainImpl.java @@ -24,6 +24,7 @@ import org.jetbrains.annotations.Nullable; import org.json.JSONArray; import org.json.JSONObject; +import org.labkey.api.audit.AbstractAuditTypeProvider; import org.labkey.api.audit.AuditLogService; import org.labkey.api.audit.AuditTypeEvent; import org.labkey.api.collections.CaseInsensitiveHashMap; @@ -57,12 +58,11 @@ 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.property.Lookup; -import org.labkey.api.exp.property.PropertyService; -import org.labkey.api.gwt.client.DefaultValueType; import org.labkey.api.gwt.client.model.GWTIndex; +import org.labkey.api.gwt.client.model.GWTPropertyDescriptor; import org.labkey.api.query.BatchValidationException; import org.labkey.api.query.FieldKey; +import org.labkey.api.query.MetadataColumnJSON; import org.labkey.api.query.QueryService; import org.labkey.api.query.QueryUpdateService; import org.labkey.api.query.QueryUpdateServiceException; @@ -89,6 +89,7 @@ import java.util.LinkedHashSet; import java.util.List; import java.util.Map; +import java.util.Objects; import java.util.Random; import java.util.Set; import java.util.concurrent.locks.Lock; @@ -365,7 +366,7 @@ public void delete(@Nullable User user, @Nullable String auditUserComment) throw DefaultValueService.get().clearDefaultValues(getContainer(), this); OntologyManager.deleteDomain(getTypeURI(), getContainer()); StorageProvisioner.get().drop(this); - addAuditEvent(user, String.format("The domain %s was deleted", _dd.getName()), auditUserComment); + addAuditEvent(user, String.format("The domain %s was deleted", _dd.getName()), auditUserComment, getContainer(), null, null); DomainPropertyManager.clearCaches(); transaction.commit(); } @@ -380,7 +381,7 @@ public boolean isNew() @Override public void save(User user) throws ChangePropertyDescriptorException { - save(user, false); + save(user, null, null); } @Override @@ -526,22 +527,26 @@ private void validatePropertyLookup(User user, DomainProperty dp) throws ChangeP public void saveIfNotExists(User user) throws ChangePropertyDescriptorException { - save(user, false, true, null); + save(user, false, true, null, null, null, null, null, null); } @Override - public void save(User user, boolean allowAddBaseProperty) throws ChangePropertyDescriptorException + public void save(User user, @Nullable Map newRecordMap, @Nullable List calculatedFields) throws ChangePropertyDescriptorException { - save(user, false, false, null); + save(user, false, false, null, null, null, newRecordMap, null, calculatedFields); } @Override - public void save(User user, @Nullable String auditComment) throws ChangePropertyDescriptorException + public void save(User user, @Nullable String auditComment, @Nullable String auditUserComment, + @Nullable Map oldRecordMap, @Nullable Map newRecordMap, + @Nullable List oldCalculatedFields, @Nullable List newCalculatedFields) throws ChangePropertyDescriptorException { - save(user, false, false, auditComment); + save(user, false, false, auditComment, auditUserComment, oldRecordMap, newRecordMap, oldCalculatedFields, newCalculatedFields); } - public void save(User user, boolean allowAddBaseProperty, boolean saveOnlyIfNotExists, @Nullable String auditComment) throws ChangePropertyDescriptorException + public void save(User user, boolean allowAddBaseProperty, boolean saveOnlyIfNotExists, @Nullable String auditComment, @Nullable String auditUserComment, + @Nullable Map oldRecordMap, @Nullable Map newRecordMap, + @Nullable List oldCalculatedFields, @Nullable List newCalculatedFields) throws ChangePropertyDescriptorException { ExperimentService exp = ExperimentService.get(); @@ -654,11 +659,12 @@ public void save(User user, boolean allowAddBaseProperty, boolean saveOnlyIfNotE { if (!impl._deleted) { + String newPropName = impl._pd.getName(); // remember the actual name before being substituted with tmpName // make sure all properties have storageColumnName if (null == impl._pd.getStorageColumnName()) { - if (!allowAddBaseProperty && baseProperties.contains(impl._pd.getName())) - impl._pd.setStorageColumnName(impl._pd.getName()); // Issue 29047: if we allow base property (like "date"), we're later going to use the base property name for storage + if (!allowAddBaseProperty && baseProperties.contains(newPropName)) + impl._pd.setStorageColumnName(newPropName); // Issue 29047: if we allow base property (like "date"), we're later going to use the base property name for storage else generateStorageColumnName(impl._pd); } @@ -700,7 +706,7 @@ else if (impl.isNew()) if (impl.isDirty()) { - if (null != impl._pdOld && !impl._pdOld.getName().equalsIgnoreCase(impl._pd.getName())) + if (null != impl._pdOld && !impl._pdOld.getName().equalsIgnoreCase(newPropName)) { finalNames.put(impl, new Pair<>(impl.getName(), sortOrder)); // Issue 17020: Save any fields whose name changed with a temp, guaranteed unique name. @@ -735,7 +741,7 @@ else if (impl.isNew()) boolean isImplNew = impl.isNew(); PropertyDescriptor pdOld = impl._pdOld; String oldValidators = null != pdOld ? PropertyChangeAuditInfo.renderValidators(pdOld) : null; - String oldFormats = null != pdOld ? PropertyChangeAuditInfo.renderConditionalFormats(pdOld) : null; + String oldConditionalFormats = null != pdOld ? PropertyChangeAuditInfo.renderConditionalFormats(pdOld) : null; impl.save(user, _dd, sortOrder++); // Automatically preserve order String defaultValue = impl.getDefaultValue(); @@ -746,7 +752,7 @@ else if (impl.isNew()) propertyAuditInfo.add(new PropertyChangeAuditInfo(impl, true)); else if (null != pdOld) { - PropertyChangeAuditInfo auditInfo = new PropertyChangeAuditInfo(impl, pdOld, oldValidators, oldFormats); + PropertyChangeAuditInfo auditInfo = new PropertyChangeAuditInfo(impl, newPropName, pdOld, oldValidators, oldConditionalFormats); if (auditInfo.isChanged()) propertyAuditInfo.add(auditInfo); } @@ -814,23 +820,33 @@ else if (null != pdOld) } } + CalculatedFieldsUpdate calculatedFieldsUpdate = determineCalculatedFieldsUpdates(oldCalculatedFields, newCalculatedFields); + if (calculatedFieldsUpdate.hasChange()) + propChanged = true; + final boolean finalPropChanged = propChanged; - final String extraAuditComment = auditComment == null ? "" : auditComment + ' '; + final String extraAuditComment = StringUtils.isEmpty(auditComment) ? "" : auditComment + ' '; // Move audit event creation to outside the transaction to avoid deadlocks involving audit storage table creation Runnable afterDomainCommit = () -> { + Long newDomainEventId = null; + String columnModifiedMsg = String.format("The column(s) of domain %s were modified.", _dd.getName()); if (isDomainNew) - addAuditEvent(user, extraAuditComment + String.format("The domain %s was created", _dd.getName()), null); + { + String columnMsg_ = finalPropChanged ? " " + columnModifiedMsg : ""; + newDomainEventId = addAuditEvent(user, extraAuditComment + String.format("The domain %s was created.", _dd.getName()) + columnMsg_, auditUserComment, getContainer(), oldRecordMap, newRecordMap); + } if (finalPropChanged) { - final Long domainEventId = addAuditEvent(user, extraAuditComment + String.format("The column(s) of domain %s were modified", _dd.getName()), null); - propertyAuditInfo.forEach(auditInfo -> addPropertyAuditEvent(user, auditInfo.getProp(), auditInfo.getAction(), domainEventId, getName(), auditInfo.getDetails())); + final Long domainEventId = newDomainEventId != null ? newDomainEventId : addAuditEvent(user, extraAuditComment + columnModifiedMsg, auditUserComment, getContainer(), oldRecordMap, newRecordMap); + propertyAuditInfo.forEach(auditInfo -> addPropertyAuditEvent(user, getContainer(), auditInfo.getProp(), auditInfo.getAction(), domainEventId, getName(), auditInfo.getDetails())); + calculatedFieldsUpdate.addAuditEvent(user, getContainer(), domainEventId, getName()); } else if (!isDomainNew) { - addAuditEvent(user, extraAuditComment + String.format("The descriptor of domain %s was updated", _dd.getName()), null); + addAuditEvent(user, extraAuditComment + String.format("The descriptor of domain %s was updated.", _dd.getName()), auditUserComment, getContainer(), oldRecordMap, newRecordMap); } }; transaction.addCommitTask(afterDomainCommit, DbScope.CommitTaskOption.POSTCOMMIT); @@ -850,6 +866,123 @@ else if (!isDomainNew) } } + record CalculatedFieldsUpdate(@Nullable List added, @Nullable List removed, @Nullable List> updated) + { + public boolean hasChange() + { + return (added != null && !added.isEmpty()) || (removed != null && !removed.isEmpty()) || (updated != null && !updated.isEmpty()); + } + + public void addAuditEvent(@Nullable User user, Container container, Long domainEventId, String domainName) + { + if (added != null) + { + for (MetadataColumnJSON field : added) + { + DomainPropertyAuditProvider.DomainPropertyAuditEvent event = + new DomainPropertyAuditProvider.DomainPropertyAuditEvent(container, null, field.getName(), + "Created", domainEventId, domainName, "Calculated field created."); + event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(field.getAuditRecordMap())); + AuditLogService.get().addEvent(user, event); + } + } + + if (removed != null) + { + for (MetadataColumnJSON field : removed) + { + DomainPropertyAuditProvider.DomainPropertyAuditEvent event = + new DomainPropertyAuditProvider.DomainPropertyAuditEvent(container, null, field.getName(), + "Deleted", domainEventId, domainName, "Calculated field removed."); + AuditLogService.get().addEvent(user, event); + } + } + + if (updated != null) + { + for (Pair oldNew : updated) + { + DomainPropertyAuditProvider.DomainPropertyAuditEvent event = + new DomainPropertyAuditProvider.DomainPropertyAuditEvent(container, null, oldNew.first.getName(), + "Modified", domainEventId, domainName, "Calculated field updated."); + event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(oldNew.first.getAuditRecordMap())); + event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(oldNew.second.getAuditRecordMap())); + AuditLogService.get().addEvent(user, event); + } + } + } + } + + private CalculatedFieldsUpdate determineCalculatedFieldsUpdates(@Nullable List oldCalculatedFields, @Nullable List newCalculatedFields) + { + Map oldFieldsMap = new HashMap<>(); + if (oldCalculatedFields != null) + oldCalculatedFields.forEach(field -> oldFieldsMap.put(field.getName(), field)); + Map newFieldsMap = new HashMap<>(); + if (newCalculatedFields != null) + newCalculatedFields.forEach(field -> newFieldsMap.put(field.getName(), field)); + + Set oldFields = oldFieldsMap.keySet(); + Set newFields = newFieldsMap.keySet(); + + List added = new ArrayList<>(newFields); + added.removeAll(oldFields); + List addedFields = new ArrayList<>(); + for (String field : added) + { + GWTPropertyDescriptor descriptor = newFieldsMap.get(field); + MetadataColumnJSON metadataColumnJSON = null; + if (descriptor instanceof MetadataColumnJSON) + metadataColumnJSON = (MetadataColumnJSON) descriptor; + else + metadataColumnJSON = new MetadataColumnJSON(descriptor); + addedFields.add(metadataColumnJSON); + } + + List deleted = new ArrayList<>(oldFields); + deleted.removeAll(newFields); + List removedFields = new ArrayList<>(); + for (String field : deleted) + { + GWTPropertyDescriptor descriptor = oldFieldsMap.get(field); + MetadataColumnJSON metadataColumnJSON = null; + if (descriptor instanceof MetadataColumnJSON) + metadataColumnJSON = (MetadataColumnJSON) descriptor; + else + metadataColumnJSON = new MetadataColumnJSON(descriptor); + removedFields.add(metadataColumnJSON); + } + + List retained = new ArrayList<>(oldFields); + retained.retainAll(newFields); + List> updatedFields = new ArrayList<>(); + for (String field : retained) + { + GWTPropertyDescriptor oldDescriptor = oldFieldsMap.get(field); + MetadataColumnJSON metadataColumnJSONOld = null; + if (oldDescriptor instanceof MetadataColumnJSON) + metadataColumnJSONOld = (MetadataColumnJSON) oldDescriptor; + else + metadataColumnJSONOld = new MetadataColumnJSON(oldDescriptor); + + GWTPropertyDescriptor newDescriptor = newFieldsMap.get(field); + MetadataColumnJSON metadataColumnJSONNew = null; + if (newDescriptor instanceof MetadataColumnJSON) + metadataColumnJSONNew = (MetadataColumnJSON) newDescriptor; + else + metadataColumnJSONNew = new MetadataColumnJSON(newDescriptor); + + Map oldDetails = metadataColumnJSONOld.getAuditRecordMap(); + Map newDetails = metadataColumnJSONNew.getAuditRecordMap(); + if (oldDetails.equals(newDetails)) + continue; + + updatedFields.add(new Pair<>(metadataColumnJSONOld, metadataColumnJSONNew)); + } + + return new CalculatedFieldsUpdate(addedFields, removedFields, updatedFields); + } + private void ensureUniqueIdValues(List propsAdded) throws SQLException, BatchValidationException { SchemaTableInfo table = StorageProvisioner.get().getSchemaTableInfo(this); @@ -941,11 +1074,14 @@ private void checkAndThrowSizeConstraints(DomainKind kind, DomainProperty pro } } - private Long addAuditEvent(@Nullable User user, String comment, @Nullable String auditUserComment) + private Long addAuditEvent(@Nullable User user, String comment, @Nullable String auditUserComment, @Nullable Container container, + @Nullable Map oldProps, @Nullable Map newProps) { if (user != null) { DomainAuditProvider.DomainAuditEvent event = new DomainAuditProvider.DomainAuditEvent(getContainer(), comment); + event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(oldProps)); + event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(newProps)); event.setUserComment(auditUserComment); event.setDomainUri(getTypeURI()); @@ -957,33 +1093,42 @@ private Long addAuditEvent(@Nullable User user, String comment, @Nullable String return null; } - private void addPropertyAuditEvent(@Nullable User user, DomainProperty prop, String action, Long domainEventId, String domainName, String comment) + private void addPropertyAuditEvent(@Nullable User user, Container container, DomainProperty prop, String action, Long domainEventId, String domainName, PropertyChangeAuditInfoDetail changeDetail) { + String changeSummary = changeDetail == null ? null : changeDetail.changeSummary; + Map oldProps = changeDetail == null ? null : changeDetail.oldRecordMap; + Map newProps = changeDetail == null ? null : changeDetail.newRecordMap; DomainPropertyAuditProvider.DomainPropertyAuditEvent event = new DomainPropertyAuditProvider.DomainPropertyAuditEvent(getContainer(), prop.getPropertyURI(), prop.getName(), - action, domainEventId, domainName, comment); + action, domainEventId, domainName, changeSummary); + event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(oldProps)); + event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(newProps)); AuditLogService.get().addEvent(user, event); } + record PropertyChangeAuditInfoDetail(String changeSummary, Map oldRecordMap, Map newRecordMap) + { + } + private static class PropertyChangeAuditInfo { private final DomainProperty _prop; private final String _action; - private final String _details; // to go in comments + private final PropertyChangeAuditInfoDetail _details; // to go in comments public PropertyChangeAuditInfo(DomainPropertyImpl prop, boolean isCreated) { _prop = prop; _action = isCreated ? "Created" : "Deleted"; - _details = isCreated ? makeNewPropAuditComment(prop) : ""; + _details = isCreated ? makeNewPropAuditComment(prop) : null; } - public PropertyChangeAuditInfo(DomainPropertyImpl prop, PropertyDescriptor pdOld, - String oldValidators, String oldFormats) + public PropertyChangeAuditInfo(DomainPropertyImpl prop, String newPropName, PropertyDescriptor pdOld, + String oldValidators, String oldConditionalFormats) { _prop = prop; _action = "Modified"; - _details = makeModifiedPropAuditComment(prop, pdOld, oldValidators, oldFormats); + _details = makeModifiedPropAuditComment(prop, newPropName, pdOld, oldValidators, oldConditionalFormats); } public DomainProperty getProp() @@ -996,225 +1141,72 @@ public String getAction() return _action; } - public String getDetails() + public PropertyChangeAuditInfoDetail getDetails() { return _details; } - public boolean isChanged() { return !_details.isEmpty(); } + public boolean isChanged() + { + return !StringUtils.isEmpty(_details.changeSummary); + } - private String makeNewPropAuditComment(DomainProperty prop) + private PropertyChangeAuditInfoDetail makeNewPropAuditComment(DomainPropertyImpl prop) { - StringBuilder str = new StringBuilder(); - str.append("Name: ").append(prop.getName()).append("; "); - str.append("Label: ").append(renderCheckingBlank(prop.getLabel())).append("; "); - str.append("Type: ").append(prop.getPropertyType().getXarName()).append("; "); - if (prop.getPropertyType().getJdbcType().isText()) - str.append("Scale: ").append(prop.getScale()).append("; "); - - Lookup lookup = prop.getLookup(); - if (null != lookup) - { - str.append("Lookup: ["); - if (null != lookup.getContainer()) - str.append("Container: ").append(lookup.getContainer().getName()).append(", "); - str.append("Schema: ").append(lookup.getSchemaKey()).append(", ") - .append("Query: ").append(lookup.getQueryName()).append("]; "); - } + String newValidators = PropertyChangeAuditInfo.renderValidators(prop.getPropertyDescriptor()); + String newConditionalFormats = PropertyChangeAuditInfo.renderConditionalFormats(prop.getPropertyDescriptor()); + Map newProps = prop.getAuditRecordMap(newValidators, newConditionalFormats); - str.append("Description: ").append(renderCheckingBlank(prop.getDescription())).append("; "); - str.append("Format: ").append(renderCheckingBlank(prop.getFormat())).append("; "); - str.append("URL: ").append(renderCheckingBlank(prop.getURL())).append("; "); - str.append("PHI: ").append(prop.getPHI().toString()).append("; "); - str.append("ImportAliases: ").append(renderImportAliases(prop.getPropertyDescriptor())).append("; "); - str.append("Validators: ").append(renderValidators(prop.getPropertyDescriptor())).append("; "); - str.append("ConditionalFormats: ").append(renderConditionalFormats(prop.getPropertyDescriptor())).append("; "); - str.append("DefaultValueType: ").append(renderDefaultValueType(prop.getPropertyDescriptor())).append("; "); - str.append("DefaultScale: ").append(prop.getDefaultScale().getLabel()).append("; "); - str.append("Required: ").append(renderBool(prop.isRequired())).append("; "); - str.append("Hidden: ").append(renderBool(prop.isHidden())).append("; "); - str.append("MvEnabled: ").append(renderBool(prop.isMvEnabled())).append("; "); - str.append("Measure: ").append(renderBool(prop.isMeasure())).append("; "); - str.append("Dimension: ").append(renderBool(prop.isDimension())).append("; "); - str.append("ShownInInsert: ").append(renderBool(prop.isShownInInsertView())).append("; "); - str.append("ShownInDetails: ").append(renderBool(prop.isShownInDetailsView())).append("; "); - str.append("ShownInUpdate: ").append(renderBool(prop.isShownInUpdateView())).append("; "); - str.append("RecommendedVariable: ").append(renderBool(prop.isRecommendedVariable())).append("; "); - str.append("ExcludedFromShifting: ").append(renderBool(prop.isExcludeFromShifting())).append("; "); - str.append("Scannable: ").append(renderBool(prop.isScannable())).append("; "); - return str.toString(); + return new PropertyChangeAuditInfoDetail(null, null, newProps); } - private String makeModifiedPropAuditComment(DomainPropertyImpl prop, PropertyDescriptor pdOld, String oldValidators, String oldFormats) + private PropertyChangeAuditInfoDetail makeModifiedPropAuditComment(DomainPropertyImpl prop, String newPropName, PropertyDescriptor pdOld, String oldValidators, String oldConditionalFormats) { - StringBuilder str = new StringBuilder(); - if (!pdOld.getName().equals(prop.getName())) - str.append("Name: ").append(renderOldVsNew(pdOld.getName(), prop.getName())).append("; "); - if (!StringUtils.equals(pdOld.getLabel(), prop.getLabel())) - str.append("Label: ").append(renderOldVsNew(renderCheckingBlank(pdOld.getLabel()), renderCheckingBlank(prop.getLabel()))).append("; "); - if (null != pdOld.getPropertyType() && !pdOld.getPropertyType().equals(prop.getPropertyType())) - str.append("Type: ").append(renderOldVsNew(pdOld.getPropertyType().getXarName(), prop.getPropertyType().getXarName())).append("; "); - if (prop.getPropertyType().getJdbcType().isText()) - if (pdOld.getScale() != prop.getScale()) - str.append("Scale: ").append(renderOldVsNew(Integer.toString(pdOld.getScale()), Integer.toString(prop.getScale()))).append("; "); - - if (!StringUtils.equals(pdOld.getLookupSchema(), prop.getPropertyDescriptor().getLookupSchema()) || - !StringUtils.equals(pdOld.getLookupQuery(), prop.getPropertyDescriptor().getLookupQuery()) || - !StringUtils.equals(pdOld.getLookupContainer(), prop.getPropertyDescriptor().getLookupContainer())) + Map oldProps = pdOld.getAuditRecordMap(oldValidators, oldConditionalFormats); + + String newValidators = PropertyChangeAuditInfo.renderValidators(prop.getPropertyDescriptor()); + String newConditionalFormats = PropertyChangeAuditInfo.renderConditionalFormats(prop.getPropertyDescriptor()); + Map newProps = prop.getAuditRecordMap(newValidators, newConditionalFormats); + newProps.put("Name", newPropName); + + List changed = new ArrayList<>(); + for (String oldKey : oldProps.keySet()) { - renderLookupDiff(prop.getPropertyDescriptor(), pdOld, str); + if (!Objects.equals(oldProps.get(oldKey), newProps.get(oldKey))) + changed.add(oldKey); + } + for (String newKey : newProps.keySet()) + { + if (!Objects.equals(oldProps.get(newKey), newProps.get(newKey)) && !changed.contains(newKey)) + changed.add(newKey); } - if (!StringUtils.equals(pdOld.getDescription(), prop.getDescription())) - str.append("Description: ").append(renderOldVsNew(renderCheckingBlank(pdOld.getDescription()), renderCheckingBlank(prop.getDescription()))).append("; "); - if (!StringUtils.equals(prop.getFormat(), prop.getFormat())) - str.append("Format: ").append(renderOldVsNew(renderCheckingBlank(pdOld.getFormat()), renderCheckingBlank(prop.getFormat()))).append("; "); - if (!StringUtils.equals((null != pdOld.getURL() ? pdOld.getURL().toString() : null), prop.getURL())) - str.append("URL: ").append(renderOldVsNew(renderCheckingBlank(null != pdOld.getURL() ? pdOld.getURL().toString() : null), renderCheckingBlank(prop.getURL()))).append("; "); - if (!pdOld.getPHI().equals(prop.getPHI())) - str.append("PHI: ").append(renderOldVsNew(pdOld.getPHI().getLabel(), prop.getPHI().getLabel())).append("; "); - - renderImportAliasesDiff(prop, pdOld, str); - renderValidatorsDiff(prop, oldValidators, str); - renderConditionalFormatsDiff(prop, oldFormats, str); - renderDefaultValueTypeDiff(prop, pdOld, str); - - if (!pdOld.getDefaultScale().getLabel().equals(prop.getDefaultScale().getLabel())) - str.append("DefaultScale: ").append(renderOldVsNew(pdOld.getDefaultScale().getLabel(), prop.getDefaultScale().getLabel())).append("; "); - if (pdOld.isRequired() != prop.isRequired()) - str.append("Required: ").append(renderOldVsNew(renderBool(pdOld.isRequired()), renderBool(prop.isRequired()))).append("; "); - if (pdOld.isHidden() != prop.isHidden()) - str.append("Hidden: ").append(renderOldVsNew(renderBool(pdOld.isHidden()), renderBool(prop.isHidden()))).append("; "); - if (pdOld.isMvEnabled() != prop.isMvEnabled()) - str.append("MvEnabled: ").append(renderOldVsNew(renderBool(pdOld.isMvEnabled()), renderBool(prop.isMvEnabled()))).append("; "); - if (pdOld.isMeasure() != prop.isMeasure()) - str.append("Measure: ").append(renderOldVsNew(renderBool(pdOld.isMeasure()), renderBool(prop.isMeasure()))).append("; "); - if (pdOld.isDimension() != prop.isDimension()) - str.append("Dimension: ").append(renderOldVsNew(renderBool(pdOld.isDimension()), renderBool(prop.isDimension()))).append("; "); - if (pdOld.isShownInInsertView() != prop.isShownInInsertView()) - str.append("ShownInInsert: ").append(renderOldVsNew(renderBool(pdOld.isShownInInsertView()), renderBool(prop.isShownInInsertView()))).append("; "); - if (pdOld.isShownInDetailsView() != prop.isShownInDetailsView()) - str.append("ShownInDetails: ").append(renderOldVsNew(renderBool(pdOld.isShownInDetailsView()), renderBool(prop.isShownInDetailsView()))).append("; "); - if (pdOld.isShownInUpdateView() != prop.isShownInUpdateView()) - str.append("ShownInUpdate: ").append(renderOldVsNew(renderBool(pdOld.isShownInUpdateView()), renderBool(prop.isShownInUpdateView()))).append("; "); - if (pdOld.isShownInLookupView() != prop.isShownInLookupView()) - str.append("ShownInLookupView: ").append(renderOldVsNew(renderBool(pdOld.isShownInLookupView()), renderBool(prop.isShownInLookupView()))).append("; "); - if (pdOld.isRecommendedVariable() != prop.isRecommendedVariable()) - str.append("RecommendedVariable: ").append(renderOldVsNew(renderBool(pdOld.isRecommendedVariable()), renderBool(prop.isRecommendedVariable()))).append("; "); - if (pdOld.isExcludeFromShifting() != prop.isExcludeFromShifting()) - str.append("ExcludedFromShifting: ").append(renderOldVsNew(renderBool(pdOld.isExcludeFromShifting()), renderBool(prop.isExcludeFromShifting()))).append("; "); - if (pdOld.isScannable() != prop.isScannable()) - str.append("Scannable: ").append(renderOldVsNew(renderBool(pdOld.isScannable()), renderBool(prop.isScannable()))).append("; "); - if (!StringUtils.equals(pdOld.getDerivationDataScope(), prop.getDerivationDataScope())) - str.append("DerivationDataScope: ").append(renderOldVsNew(renderCheckingBlank(pdOld.getDerivationDataScope()), renderCheckingBlank(prop.getDerivationDataScope()))).append("; "); - return str.toString(); - } - - private String renderCheckingBlank(String value) - { - return StringUtils.isNotBlank(value) ? value : ""; + String changeSummary = changed.isEmpty() ? null : + "The following " + (changed.size() > 1 ? "properties were" : "property was") + " updated: " + StringUtils.join(changed, ", "); + return new PropertyChangeAuditInfoDetail(changeSummary, oldProps, newProps); } private static String renderValidators(PropertyDescriptor prop) { Collection validators = DomainPropertyManager.get().getValidators(prop); - if (validators.isEmpty()) - return ""; - List strings = new ArrayList<>(); - validators.forEach(validator -> strings.add(validator.getName() + " [" + - StringUtils.replace(PropertyService.get().getValidatorKind(validator.getTypeURI()).getName(), " Property Validator", "") + "]") - ); - return StringUtils.join(strings, ", "); + return getPropertyValidatorStringVal(validators); } private static String renderConditionalFormats(PropertyDescriptor prop) { List formats = DomainPropertyManager.get().getConditionalFormats(prop); - if (formats.isEmpty()) - return ""; - return Integer.toString(formats.size()); - } - - private void renderValidatorsDiff(DomainProperty prop, String oldValidators, StringBuilder str) - { - String validators = renderValidators(prop.getPropertyDescriptor()); - if (!StringUtils.equals(oldValidators, validators)) - str.append("Validators: ").append("old: ").append(oldValidators).append(", new: ").append(validators).append("; "); - } - - private void renderConditionalFormatsDiff(DomainProperty prop, String oldFormats, StringBuilder str) - { - String formats = renderConditionalFormats(prop.getPropertyDescriptor()); - if (!StringUtils.equals(oldFormats, formats)) - str.append("ConditionalFormats: ").append("old: ").append(oldFormats).append(", new: ").append(formats).append("; "); - } - - private String renderImportAliases(PropertyDescriptor prop) - { - Set aliases = prop.getImportAliasSet(); - if (aliases.isEmpty()) - return ""; - return StringUtils.join(aliases, ","); - } - - private void renderImportAliasesDiff(DomainProperty prop, PropertyDescriptor pdOld, StringBuilder str) - { - String oldAliases = renderImportAliases(pdOld); - String aliases = renderImportAliases(prop.getPropertyDescriptor()); - if (!StringUtils.equals(oldAliases, aliases)) - str.append("ImportAliases: ").append("old: ").append(oldAliases).append(", new: ").append(aliases).append("; "); - } - - private String renderDefaultValueType(PropertyDescriptor prop) - { - DefaultValueType type = prop.getDefaultValueTypeEnum(); - if (null == type) - return ""; - return type.getLabel(); + return ConditionalFormat.toStringVal(formats); } - private void renderDefaultValueTypeDiff(DomainProperty prop, PropertyDescriptor pdOld, StringBuilder str) - { - if (pdOld.getDefaultValueTypeEnum() != prop.getDefaultValueTypeEnum()) - str.append("DefaultValueType: ").append("old: ").append(renderDefaultValueType(pdOld)).append(", new: ") - .append(renderDefaultValueType(prop.getPropertyDescriptor())).append("; "); - } - - private String renderBool(boolean value) - { - return value ? "true" : "false"; - } - - private String renderOldVsNew(String oldVal, String newVal) - { - return oldVal + " -> " + newVal; - } - - private void renderLookupDiff(PropertyDescriptor pdNew, PropertyDescriptor pdOld, StringBuilder str) - { - str.append("Lookup: ["); - if (!StringUtils.equals(pdOld.getLookupContainer(), pdNew.getLookupContainer())) - str.append("Container: ").append("old: ").append(getContainerName(pdOld.getLookupContainer())).append(", new: ") - .append(getContainerName(pdNew.getLookupContainer())).append(", "); - if (!StringUtils.equals(pdOld.getLookupSchema(), pdNew.getLookupSchema())) - str.append("Schema: ").append("old: ").append(pdOld.getLookupSchema()).append(", new: ") - .append(pdNew.getLookupSchema()).append(", "); - if (!StringUtils.equals(pdOld.getLookupQuery(), pdNew.getLookupQuery())) - str.append("Query: ").append("old: ").append(pdOld.getLookupQuery()).append(", new: ") - .append(pdNew.getLookupQuery()); - str.append("]; "); - } + } - private String getContainerName(String containerId) - { - if (null != containerId) - { - Container container = ContainerManager.getForId(containerId); - if (null != container) - return container.getName(); - } + public static String getPropertyValidatorStringVal(Collection validators) + { + if (validators == null || validators.isEmpty()) return null; - } + List strings = new ArrayList<>(); + validators.forEach(validator -> strings.add(validator.getStringVal())); + return StringUtils.join(strings, ", "); } @Override diff --git a/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java b/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java index 1911be81c43..51bafa688eb 100644 --- a/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/DomainPropertyImpl.java @@ -56,7 +56,9 @@ import java.util.ArrayList; import java.util.Collections; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.Set; @@ -576,6 +578,9 @@ public void setDefaultValueType(String defaultValueTypeName) if (getDefaultValueType() != null && getDefaultValueType().equals(defaultValueTypeName)) return; + if (getDefaultValueType() == null && defaultValueTypeName == null) + return; // if both are null, don't call edit(), with marks property as dirty + edit().setDefaultValueType(defaultValueTypeName); } @@ -639,6 +644,15 @@ public void setScannable(boolean scannable) edit().setScannable(scannable); } + @Override + public void setOldPropertyDescriptor(PropertyDescriptor oldPropertyDescriptor) + { + if (isEdited()) + return; + + _pdOld = oldPropertyDescriptor.clone(); + } + @Override public boolean isScannable() { @@ -1056,6 +1070,12 @@ else if (lookup.getContainer().equals(targetContainer)) @Override public void setConditionalFormats(List formats) { + String newVal = ConditionalFormat.toStringVal(formats); + String oldVal = ConditionalFormat.toStringVal(getConditionalFormats()); + + if (!Objects.equals(newVal, oldVal)) + edit(); + _formats = formats; } @@ -1127,6 +1147,62 @@ public String toString() return super.toString() + _pd.getPropertyURI(); } + public Map getAuditRecordMap(@Nullable String validatorStr, @Nullable String conditionalFormatStr) + { + Map map = new LinkedHashMap<>(); + if (!StringUtils.isEmpty(getName())) + map.put("Name", getName()); + if (!StringUtils.isEmpty(getLabel())) + map.put("Label", getLabel()); + if (null != getPropertyType()) + { + if (org.labkey.api.gwt.client.ui.PropertyType.expFlag.getURI().equals(getConceptURI())) + map.put("Type", "Flag"); + else + map.put("Type", getPropertyType().getXarName()); + } + if (getPropertyType().getJdbcType().isText()) + map.put("Scale", getScale()); + if (!StringUtils.isEmpty(getDescription())) + map.put("Description", getDescription()); + if (!StringUtils.isEmpty(getFormat())) + map.put("Format", getFormat()); + if (!StringUtils.isEmpty(getURL())) + map.put("URL", getURL()); + if (getPHI() != null) + map.put("PHI", getPHI().getLabel()); + if (getDefaultScale() != null) + map.put("DefaultScale", getDefaultScale().getLabel()); + map.put("Required", isRequired()); + map.put("Hidden", isHidden()); + map.put("MvEnabled", isMvEnabled()); + map.put("Measure", isMeasure()); + map.put("Dimension", isDimension()); + map.put("ShownInInsert", isShownInInsertView()); + map.put("ShownInDetails", isShownInDetailsView()); + map.put("ShownInUpdate", isShownInUpdateView()); + map.put("ShownInLookupView", isShownInLookupView()); + map.put("RecommendedVariable", isRecommendedVariable()); + map.put("ExcludedFromShifting", isExcludeFromShifting()); + map.put("Scannable", isScannable()); + if (!StringUtils.isEmpty(getDerivationDataScope())) + map.put("DerivationDataScope", getDerivationDataScope()); + String importAliasStr = StringUtils.join(getImportAliasSet(), ","); + if (!StringUtils.isEmpty(importAliasStr)) + map.put("ImportAliases", importAliasStr); + if (getDefaultValueTypeEnum() != null) + map.put("DefaultValueType", getDefaultValueTypeEnum().getLabel()); + if (getLookup() != null) + map.put("Lookup", getLookup().toJSONString()); + + if (!StringUtils.isEmpty(validatorStr)) + map.put("Validator", validatorStr); + if (!StringUtils.isEmpty(conditionalFormatStr)) + map.put("ConditionalFormat", conditionalFormatStr); + + return map; + } + public static class TestCase extends Assert { private PropertyDescriptor _pd; diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyValidator.java b/experiment/src/org/labkey/experiment/api/property/PropertyValidator.java index 28e31a644b8..8980e7e4700 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyValidator.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyValidator.java @@ -15,11 +15,16 @@ */ package org.labkey.experiment.api.property; +import org.apache.commons.lang3.StringUtils; import org.labkey.api.exp.property.IPropertyValidator; +import org.labkey.api.exp.property.PropertyService; import org.labkey.api.util.PageFlowUtil; import org.labkey.api.util.UnexpectedException; import java.io.Serializable; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; public class PropertyValidator implements Serializable, Cloneable { @@ -149,4 +154,24 @@ public final PropertyValidator clone() throw UnexpectedException.wrap(cnse); } } + + public String getPropertyAuditStr() + { + String propertyStr = getProperties(); + if ("failOnMatch=false".equals(propertyStr)) // ignore NULL vs false + return null; + return propertyStr; + } + + public String getStringVal() + { + List parts = new ArrayList<>(); + parts.add(getName()); + parts.add(getDescription()); + parts.add(getExpression()); + parts.add(getPropertyAuditStr()); + parts.add(getErrorMessage()); + parts.add(StringUtils.replace(PropertyService.get().getValidatorKind(getTypeURI()).getName(), " Property Validator", "")); + return parts.stream().filter(StringUtils::isNotBlank).collect(Collectors.joining(", ")); + } } \ No newline at end of file diff --git a/experiment/src/org/labkey/experiment/api/property/PropertyValidatorImpl.java b/experiment/src/org/labkey/experiment/api/property/PropertyValidatorImpl.java index 86f6ded7c72..7de34645c51 100644 --- a/experiment/src/org/labkey/experiment/api/property/PropertyValidatorImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/PropertyValidatorImpl.java @@ -15,6 +15,7 @@ */ package org.labkey.experiment.api.property; +import org.apache.commons.lang3.StringUtils; import org.labkey.api.data.Container; import org.labkey.api.data.ContainerManager; import org.labkey.api.data.Table; @@ -65,6 +66,9 @@ public String getName() @Override public void setName(String name) { + if (StringUtils.equals(name, getName())) + return; + edit().setName(name); } @@ -77,6 +81,8 @@ public String getDescription() @Override public void setDescription(String description) { + if (StringUtils.equals(description, getDescription())) + return; edit().setDescription(description); } @@ -100,6 +106,9 @@ public String getExpressionValue() @Override public void setExpressionValue(String expression) { + if (StringUtils.equals(getExpressionValue(), expression)) + return; + edit().setExpression(expression); } @@ -140,6 +149,9 @@ public Map getProperties() @Override public void setErrorMessage(String message) { + if (StringUtils.equals(getErrorMessage(), message)) + return; + edit().setErrorMessage(message); } @@ -147,6 +159,9 @@ public void setErrorMessage(String message) public void setProperty(String key, String value) { Map props = getProperties(); + if (StringUtils.equals(props.get(key), value)) + return; + props.put(key, value); edit().setProperties(PageFlowUtil.toQueryString(props.entrySet())); } diff --git a/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java b/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java index 43fda173531..71062667db2 100644 --- a/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java +++ b/experiment/src/org/labkey/experiment/api/property/StorageProvisionerImpl.java @@ -1814,7 +1814,7 @@ public DomainKind getDomainKind() return k; } }; - d.save(user, false); + d.save(user); StorageProvisioner.get().ensureStorageTable(d, k, k.getScope()); // check that prop exists diff --git a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java index eb2217ed5d8..fc570799d85 100644 --- a/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java +++ b/experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java @@ -132,6 +132,7 @@ import org.labkey.api.exp.api.SampleTypeService; import org.labkey.api.exp.list.ListService; import org.labkey.api.exp.property.Domain; +import org.labkey.api.exp.property.DomainAuditProvider; import org.labkey.api.exp.property.DomainKind; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.DomainTemplate; @@ -8013,6 +8014,7 @@ public Object execute(EntitySequenceForm form, BindException errors) try { + Domain domain = null; if (SampleTypeDomainKind.NAME.equalsIgnoreCase(form.getKindName())) { if (form.getSeqType() == NameGenerator.EntityCounter.genId) @@ -8022,7 +8024,10 @@ public Object execute(EntitySequenceForm form, BindException errors) ExpSampleType sampleType = SampleTypeService.get().getSampleType(form.getRowId()); if (sampleType != null) + { sampleType.ensureMinGenId(form.getNewValue()); + domain = sampleType.getDomain(); + } else { resp.put("success", false); @@ -8044,13 +8049,24 @@ else if (DataClassDomainKind.NAME.equalsIgnoreCase(form.getKindName())) ExpDataClass dataClass = ExperimentService.get().getDataClass(form.getRowId()); if (dataClass != null) + { dataClass.ensureMinGenId(form.getNewValue(), getContainer()); + domain = dataClass.getDomain(); + } else { resp.put("success", false); resp.put("error", "DataClass does not exist."); } } + + if (domain != null) + { + DomainAuditProvider.DomainAuditEvent event = new DomainAuditProvider.DomainAuditEvent(getContainer(), "The genId for domain " + domain.getName() + " has been updated to " + form.getNewValue() + "."); + event.setDomainUri(domain.getTypeURI()); + event.setDomainName(domain.getName()); + AuditLogService.get().addEvent(getUser(), event); + } } catch (ExperimentException e) { diff --git a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java index 4727b009386..45a7f993041 100644 --- a/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java +++ b/experiment/src/org/labkey/experiment/controllers/property/PropertyController.java @@ -543,7 +543,7 @@ public Object execute(DomainApiForm form, BindException errors) boolean includeWarnings = form.includeWarnings(); boolean hasErrors = false; - ValidationException updateErrors = updateDomain(originalDomain, newDomain, form.getOptions(), getContainer(), getUser(), includeWarnings); + ValidationException updateErrors = updateDomain(originalDomain, newDomain, form.getOptions(), getContainer(), getUser(), includeWarnings, form.getAuditUserComment()); for (ValidationError ve : updateErrors.getErrors()) { @@ -760,7 +760,7 @@ public Object execute(UpdateDomainApiForm form, BindException errors) throws Exc boolean includeWarnings = form.isIncludeWarnings(); boolean hasErrors = false; - ValidationException updateErrors = updateDomain(originalDomain, newDomain, null, getContainer(), getUser(), includeWarnings); + ValidationException updateErrors = updateDomain(originalDomain, newDomain, null, getContainer(), getUser(), includeWarnings, null); for (ValidationError ve : updateErrors.getErrors()) { if (ve.getSeverity().equals(ValidationException.SEVERITY.ERROR)) @@ -809,7 +809,7 @@ public Object execute(DomainApiForm form, BindException errors) String queryName = form.getQueryName(); String schemaName = form.getSchemaName(); - deleteDomain(schemaName, queryName, getContainer(), getUser()); + deleteDomain(schemaName, queryName, getContainer(), getUser(), form.getAuditUserComment()); return success("Domain deleted"); } } @@ -832,6 +832,7 @@ public static class DomainApiForm private String queryName; private Integer domainId; private boolean includeWarnings; + private String auditUserComment; public Integer getDomainId() { @@ -1001,6 +1002,16 @@ public void setIncludeWarnings(boolean includeWarnings) @JsonIgnore private Map optionsProperties; + public String getAuditUserComment() + { + return auditUserComment; + } + + public void setAuditUserComment(String auditUserComment) + { + this.auditUserComment = auditUserComment; + } + /** * Convenience method to cache options map */ @@ -1477,7 +1488,7 @@ public void setTsvText(String tsvText) /** @return Errors encountered during the save attempt */ @NotNull private static ValidationException updateDomain(GWTDomain original, GWTDomain update, - @Nullable JSONObject options, Container container, User user, boolean includeWarnings) + @Nullable JSONObject options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { DomainKind kind = PropertyService.get().getDomainKind(original.getDomainURI()); if (kind == null) @@ -1498,7 +1509,7 @@ private static ValidationException updateDomain(GWTDomain newRecordMap, @Nullable List calculatedFields) throws Exception { if (ensureKey) { @@ -396,7 +397,7 @@ public void save(User user, boolean ensureKey) throws Exception // The domain kind cannot lookup the list definition if the domain has not been saved ((ListDomainKind) domain.getDomainKind()).setListDefinition(this); - domain.save(user); + domain.save(user, newRecordMap, calculatedFields); _def.setDomainId(domain.getTypeId()); ListDef inserted = ListManager.get().insert(user, _def, _preferredListIds); @@ -525,6 +526,12 @@ private boolean hasListItem(SimpleFilter filter, User user, Container c) @Override public void delete(User user) throws DomainNotFoundException + { + delete(user, null); + } + + @Override + public void delete(User user, @Nullable String auditUserComment) throws DomainNotFoundException { TableInfo table = getTable(user); QueryUpdateService qus = null; @@ -545,7 +552,7 @@ public void delete(User user) throws DomainNotFoundException // then delete the list itself ListManager.get().deleteListDef(getContainer(), getListId()); Domain domain = getDomain(); - domain.delete(user); + domain.delete(user, auditUserComment); ListManager.get().addAuditEvent(this, user, String.format("The list %s was deleted", _def.getName())); diff --git a/list/src/org/labkey/list/model/ListDomainKind.java b/list/src/org/labkey/list/model/ListDomainKind.java index 7b462691aeb..55e67e2ec2d 100644 --- a/list/src/org/labkey/list/model/ListDomainKind.java +++ b/list/src/org/labkey/list/model/ListDomainKind.java @@ -436,7 +436,7 @@ public Domain createDomain(GWTDomain domain, ListDomainKindProperties listProper // TODO: This looks like the wrong order to me -- we should updateListProperties() (persist the indexing // settings and handle potential transitions) before calling save() (which indexes the list). Since this is // the create case there's no data to index, but there is meta data... - list.save(user); + list.save(user, true, listProperties.getAuditRecordMap(), domain.getCalculatedFields()); updateListProperties(container, user, list.getListId(), listProperties); QueryService.get().saveCalculatedFieldsMetadata(ListQuerySchema.NAME, name, null, domain.getCalculatedFields(), false, user, container); @@ -455,13 +455,14 @@ public Domain createDomain(GWTDomain domain, ListDomainKindProperties listProper @Override public @NotNull ValidationException updateDomain(GWTDomain original, GWTDomain update, - ListDomainKindProperties listProperties, Container container, User user, boolean includeWarnings) + ListDomainKindProperties listProperties, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { ValidationException exception; try (DbScope.Transaction transaction = ListManager.get().getListMetadataSchema().getScope().ensureTransaction()) { exception = new ValidationException(); + StringBuilder changeDetails = new StringBuilder(); Domain domain = PropertyService.get().getDomain(container, original.getDomainURI()); if (null == domain) @@ -496,7 +497,6 @@ else if (!key.getName().equalsIgnoreCase(newKey.getName())) //handle name change String updatedName = StringUtils.trim(update.getName()); boolean hasNameChange = !original.getName().equals(updatedName); - String auditComment = null; if (hasNameChange) { if (updatedName.length() > MAX_NAME_LENGTH) @@ -507,7 +507,7 @@ else if (ListService.get().getList(container, updatedName, false) != null) { return exception.addGlobalError("The name '" + updatedName + "' is already in use."); } - auditComment = "The name of the list domain '" + original.getName() + "' was changed to '" + updatedName + "'."; + changeDetails.append("The name of the list domain '" + original.getName() + "' was changed to '" + updatedName + "'."); } //return if there are errors before moving forward with the save @@ -517,6 +517,8 @@ else if (ListService.get().getList(container, updatedName, false) != null) } //update list properties + Map oldProps = null; + Map newProps = null; if (null != listProperties) { if (listProperties.getDomainId() != original.getDomainId() || listProperties.getDomainId() != update.getDomainId()) @@ -524,14 +526,18 @@ else if (ListService.get().getList(container, updatedName, false) != null) if (!original.getDomainURI().equals(update.getDomainURI())) return exception.addGlobalError("domainURI mismatch between old and new domain"); - updateListProperties(container, user, listDefinition.getListId(), listProperties); + Pair, Map> updatedProps = updateListProperties(container, user, listDefinition.getListId(), listProperties); + oldProps = updatedProps.first; + newProps = updatedProps.second; } // Issue 45042: Allow for the list description to be set via the save domain API calls else if (update.getDescription() != null) { listProperties = getListProperties(container, user, listDefinition.getListId()); listProperties.setDescription(update.getDescription()); - updateListProperties(container, user, listDefinition.getListId(), listProperties); + Pair, Map> updatedProps = updateListProperties(container, user, listDefinition.getListId(), listProperties); + oldProps = updatedProps.first; + newProps = updatedProps.second; } //update domain design properties @@ -576,7 +582,7 @@ else if (update.getDescription() != null) } //update domain properties - exception.addErrors(DomainUtil.updateDomainDescriptor(original, update, container, user, hasNameChange, auditComment)); + exception.addErrors(DomainUtil.updateDomainDescriptor(original, update, container, user, hasNameChange, changeDetails.toString(), auditUserComment, oldProps, newProps)); QueryService.get().saveCalculatedFieldsMetadata(ListQuerySchema.NAME, update.getQueryName(), hasNameChange ? update.getName() : null, update.getCalculatedFields(), !original.getCalculatedFields().isEmpty(), user, container); } @@ -611,21 +617,25 @@ private ListDomainKindProperties getListProperties(Container container, User use return new TableSelector(ListManager.get().getListMetadataTable(), filter, null).getObject(ListDomainKindProperties.class); } - private void updateListProperties(Container container, User user, int listId, ListDomainKindProperties listProperties) + private Pair, Map> updateListProperties(Container container, User user, int listId, ListDomainKindProperties listProperties) { ListDomainKindProperties existingListProps = getListProperties(container, user, listId); + Map oldProps = existingListProps == null ? null : existingListProps.getAuditRecordMap(); + Map newProps = listProperties == null ? oldProps : listProperties.getAuditRecordMap(); //merge existing and new properties ListDomainKindProperties updatedListProps = updateListProperties(existingListProps, listProperties); ListManager.get().update(user, container, updatedListProps); + + return new Pair<>(oldProps, newProps); } //updates list properties except listId, domainId, keyName, keyType, and lastIndexed private ListDomainKindProperties updateListProperties(ListDomainKindProperties existingListProps, ListDomainKindProperties newListProps) { - ListDomainKindProperties updatedListProps = new ListDomainKindProperties(existingListProps); + ListDomainKindProperties updatedListProps = new ListDomainKindProperties(existingListProps); if (null != newListProps.getName()) updatedListProps.setName(newListProps.getName().trim()); @@ -681,7 +691,7 @@ private GWTPropertyDescriptor findField(int id, List getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + map.put("Name", getName()); + if (!StringUtils.isEmpty(getTitleColumn())) + map.put("TitleColumn", getTitleColumn()); + if (!StringUtils.isEmpty(getDescription())) + map.put("Description", getDescription()); + map.put("AllowDelete", isAllowDelete()); + map.put("AllowUpload", isAllowUpload()); + map.put("AllowExport", isAllowExport()); + map.put("DiscussionSetting", getDiscussionSetting()); + if (!StringUtils.isEmpty(getCategory())) + map.put("Category", getCategory()); + if (!StringUtils.isEmpty(getEntireListTitleTemplate())) + map.put("EntireListTitleTemplate", getEntireListTitleTemplate()); + if (!StringUtils.isEmpty(getEntireListBodyTemplate())) + map.put("EntireListBodyTemplate", getEntireListBodyTemplate()); + if (!StringUtils.isEmpty(getEachItemTitleTemplate())) + map.put("EachItemTitleTemplate", getEachItemTitleTemplate()); + if (!StringUtils.isEmpty(getEachItemBodyTemplate())) + map.put("EachItemBodyTemplate", getEachItemBodyTemplate()); + map.put("EntireListIndexSetting", getEntireListIndexSetting()); + map.put("EntireListBodySetting", getEntireListBodySetting()); + map.put("EachItemBodySetting", getEachItemBodySetting()); + map.put("EntireListIndex", isEntireListIndex()); + map.put("EachItemIndex", isEachItemIndex()); + map.put("FileAttachmentIndex", isFileAttachmentIndex()); + return map; + } } diff --git a/list/src/org/labkey/list/model/ListManager.java b/list/src/org/labkey/list/model/ListManager.java index f2000484fab..613b2c562b2 100644 --- a/list/src/org/labkey/list/model/ListManager.java +++ b/list/src/org/labkey/list/model/ListManager.java @@ -1237,7 +1237,7 @@ String formatAuditItem(ListDefinitionImpl list, User user, Map p } if (!recordChangedMap.isEmpty()) - itemRecord = ListAuditProvider.encodeForDataMap(list.getContainer(), recordChangedMap); + itemRecord = ListAuditProvider.encodeForDataMap(recordChangedMap); } return itemRecord; diff --git a/query/src/org/labkey/query/QueryServiceImplTestCase.jsp b/query/src/org/labkey/query/QueryServiceImplTestCase.jsp index 54a728e7834..352e4097c4d 100644 --- a/query/src/org/labkey/query/QueryServiceImplTestCase.jsp +++ b/query/src/org/labkey/query/QueryServiceImplTestCase.jsp @@ -142,7 +142,7 @@ d.addProperty(new PropertyStorageSpec("A", JdbcType.INTEGER)); d.addProperty(new PropertyStorageSpec("B", JdbcType.INTEGER)); d.addProperty(new PropertyStorageSpec("C", JdbcType.INTEGER)); - list.save(user,true); + list.save(user,true, null, null); } diff --git a/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java b/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java index 93e26112da3..9adc5421140 100644 --- a/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java +++ b/query/src/org/labkey/query/audit/QueryUpdateAuditProvider.java @@ -140,11 +140,7 @@ else if (COLUMN_NAME_USER_COMMENT.equalsIgnoreCase(col.getName())) } } }; - appendValueMapColumns(table); - - DetailsURL url = DetailsURL.fromString("audit-detailedAuditChanges.view?auditRowId=${rowId}&auditEventType=" + QUERY_UPDATE_AUDIT_EVENT); - url.setStrictContainerContextEval(true); - table.setDetailsURL(url); + appendValueMapColumns(table, QUERY_UPDATE_AUDIT_EVENT); return table; } diff --git a/query/src/org/labkey/query/controllers/QueryController.java b/query/src/org/labkey/query/controllers/QueryController.java index 94dfefd0cbc..85ad426fefe 100644 --- a/query/src/org/labkey/query/controllers/QueryController.java +++ b/query/src/org/labkey/query/controllers/QueryController.java @@ -8140,6 +8140,7 @@ public static class QueryImportTemplateForm { private String schemaName; private String queryName; + private String auditUserComment; private List templateLabels; private List templateUrls; private Long _lastKnownModified; @@ -8195,6 +8196,16 @@ public void setLastKnownModified(Long lastKnownModified) _lastKnownModified = lastKnownModified; } + public String getAuditUserComment() + { + return auditUserComment; + } + + public void setAuditUserComment(String auditUserComment) + { + this.auditUserComment = auditUserComment; + } + } @Marshal(Marshaller.Jackson) @@ -8429,6 +8440,7 @@ public Object execute(QueryImportTemplateForm form, BindException errors) throws } DomainAuditProvider.DomainAuditEvent event = new DomainAuditProvider.DomainAuditEvent(getContainer(), "Import templates updated."); + event.setUserComment(form.getAuditUserComment()); event.setDomainUri(_domain.getTypeURI()); event.setDomainName(_domain.getName()); AuditLogService.get().addEvent(user, event); diff --git a/study/api-src/org/labkey/api/specimen/model/AbstractSpecimenDomainKind.java b/study/api-src/org/labkey/api/specimen/model/AbstractSpecimenDomainKind.java index 963a568de44..a20e2ed1e1a 100644 --- a/study/api-src/org/labkey/api/specimen/model/AbstractSpecimenDomainKind.java +++ b/study/api-src/org/labkey/api/specimen/model/AbstractSpecimenDomainKind.java @@ -88,11 +88,11 @@ public Domain createDomain(GWTDomain domain, JSONObject a } @Override - public @NotNull ValidationException updateDomain(GWTDomain original, GWTDomain update, @Nullable JSONObject options, Container container, User user, boolean includeWarnings) + public @NotNull ValidationException updateDomain(GWTDomain original, GWTDomain update, @Nullable JSONObject options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { ValidationException validation = checkFieldNameLength(update); if (validation != null) return validation; - return super.updateDomain(original, update, options, container, user, includeWarnings); + return super.updateDomain(original, update, options, container, user, includeWarnings, auditUserComment); } // Issue 52666: Don't allow property names that might cause problems due to a storage vs user-facing name diff --git a/study/api-src/org/labkey/api/specimen/model/SpecimenDomainKind.java b/study/api-src/org/labkey/api/specimen/model/SpecimenDomainKind.java index b09617077f2..b54a91b3ce4 100644 --- a/study/api-src/org/labkey/api/specimen/model/SpecimenDomainKind.java +++ b/study/api-src/org/labkey/api/specimen/model/SpecimenDomainKind.java @@ -168,7 +168,7 @@ public ActionURL urlEditDefinition(Domain domain, ContainerUser containerUser) @Override public @NotNull ValidationException updateDomain(GWTDomain original, GWTDomain update, - @Nullable JSONObject options, Container container, User user, boolean includeWarnings) + @Nullable JSONObject options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { ValidationException exception; try (var transaction = SpecimenSchema.get().getScope().ensureTransaction()) @@ -213,7 +213,7 @@ public ActionURL urlEditDefinition(Domain domain, ContainerUser containerUser) } exception = checkRollups(null, optionalSpecimenProps, container, user, exception, includeWarnings); - exception.addErrors(super.updateDomain(original, update, options, container, user, includeWarnings)); + exception.addErrors(super.updateDomain(original, update, options, container, user, includeWarnings, auditUserComment)); if (!exception.hasErrors()) { diff --git a/study/api-src/org/labkey/api/specimen/model/SpecimenEventDomainKind.java b/study/api-src/org/labkey/api/specimen/model/SpecimenEventDomainKind.java index d6245e5e7f7..f2e341d06ae 100644 --- a/study/api-src/org/labkey/api/specimen/model/SpecimenEventDomainKind.java +++ b/study/api-src/org/labkey/api/specimen/model/SpecimenEventDomainKind.java @@ -233,7 +233,7 @@ public ActionURL urlEditDefinition(Domain domain, ContainerUser containerUser) @Override public @NotNull ValidationException updateDomain(GWTDomain original, GWTDomain update, - @Nullable JSONObject options, Container container, User user, boolean includeWarnings) + @Nullable JSONObject options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) { ValidationException validationException; try (var transaction = SpecimenSchema.get().getScope().ensureTransaction()) @@ -253,7 +253,7 @@ public ActionURL urlEditDefinition(Domain domain, ContainerUser containerUser) } } - validationException.addErrors(super.updateDomain(original, update, options, container, user, includeWarnings)); + validationException.addErrors(super.updateDomain(original, update, options, container, user, includeWarnings, auditUserComment)); if (!validationException.hasErrors()) { diff --git a/study/api-src/org/labkey/api/specimen/model/VialDomainKind.java b/study/api-src/org/labkey/api/specimen/model/VialDomainKind.java index 9bfbe03b7ec..39e311136b1 100644 --- a/study/api-src/org/labkey/api/specimen/model/VialDomainKind.java +++ b/study/api-src/org/labkey/api/specimen/model/VialDomainKind.java @@ -171,7 +171,8 @@ public ActionURL urlEditDefinition(Domain domain, ContainerUser containerUser) @Override public @NotNull ValidationException updateDomain(GWTDomain original, GWTDomain update, - @Nullable JSONObject options, Container container, User user, boolean includeWarnings){ + @Nullable JSONObject options, Container container, User user, boolean includeWarnings, @Nullable String auditUserComment) + { ValidationException exception; try (var transaction = SpecimenSchema.get().getScope().ensureTransaction()) { @@ -216,7 +217,7 @@ public ActionURL urlEditDefinition(Domain domain, ContainerUser containerUser) } exception = checkRollups(optionalVialFields, null, container, user, exception, includeWarnings); - exception.addErrors(super.updateDomain(original, update, options, container, user, includeWarnings)); + exception.addErrors(super.updateDomain(original, update, options, container, user, includeWarnings, auditUserComment)); if (!exception.hasErrors()) { diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 694d2e80881..3e14f6c7534 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -642,7 +642,7 @@ private void logPublishEvent(Dataset.PublishSource publishSource, ExpObject sour var timelineEventType = SampleTimelineAuditEvent.SampleTimelineEventType.PUBLISH; Map eventMetadata = new HashMap<>(); eventMetadata.put(SAMPLE_TIMELINE_EVENT_TYPE, timelineEventType.name()); - String metadata = AbstractAuditTypeProvider.encodeForDataMap(sourceContainer, eventMetadata); + String metadata = AbstractAuditTypeProvider.encodeForDataMap(eventMetadata); List sampleIds = rows.stream().map(m -> (Integer) m.get(StudyPublishService.ROWID_PROPERTY_NAME)).collect(toList()); List samples = ExperimentService.get().getExpMaterials(sampleIds); @@ -1597,7 +1597,7 @@ public void addRecallAuditEvent(Container sourceContainer, User user, Dataset de var timelineEventType = SampleTimelineAuditEvent.SampleTimelineEventType.RECALL; Map eventMetadata = new HashMap<>(); eventMetadata.put(SAMPLE_TIMELINE_EVENT_TYPE, timelineEventType.name()); - String metadata = AbstractAuditTypeProvider.encodeForDataMap(sourceContainer, eventMetadata); + String metadata = AbstractAuditTypeProvider.encodeForDataMap(eventMetadata); List sampleIds = pairs.stream().map(Pair::getValue).collect(toList()); List samples = ExperimentService.get().getExpMaterials(sampleIds); diff --git a/study/src/org/labkey/study/audit/ParticipantGroupAuditProvider.java b/study/src/org/labkey/study/audit/ParticipantGroupAuditProvider.java index 66bf4ff6083..d22a41c7c4d 100644 --- a/study/src/org/labkey/study/audit/ParticipantGroupAuditProvider.java +++ b/study/src/org/labkey/study/audit/ParticipantGroupAuditProvider.java @@ -105,7 +105,7 @@ else if (PARTICIPANT_GROUP_ID_COLUMN_NAME.equalsIgnoreCase(col.getName())) } } }; - appendValueMapColumns(table); + appendValueMapColumns(table, null, true); return table; } @@ -220,8 +220,8 @@ public static ParticipantGroupAuditEvent categoryChange( var event = new ParticipantGroupAuditProvider.ParticipantGroupAuditEvent(c, comment, newCategory.getRowId()); if (prevCategory != null) - event.setOldRecordMap(createEncodedRecordMap(c, factory.toMap(prevCategory, null))); - event.setNewRecordMap(createEncodedRecordMap(c, factory.toMap(newCategory, null))); + event.setOldRecordMap(createEncodedRecordMap(factory.toMap(prevCategory, null))); + event.setNewRecordMap(createEncodedRecordMap(factory.toMap(newCategory, null))); return event; } @@ -248,10 +248,10 @@ public static ParticipantGroupAuditEvent groupChange( var event = new ParticipantGroupAuditProvider.ParticipantGroupAuditEvent(c, comment, newGroup.getCategoryId()); event.setParticipantGroup(newGroup.getRowId()); - event.setNewRecordMap(createEncodedRecordMap(c, factory.toMap(newGroup, null))); + event.setNewRecordMap(createEncodedRecordMap(factory.toMap(newGroup, null))); if (prevGroup != null) { - event.setOldRecordMap(createEncodedRecordMap(c, factory.toMap(prevGroup, null))); + event.setOldRecordMap(createEncodedRecordMap(factory.toMap(prevGroup, null))); if (event.getNewRecordMap().equals(event.getOldRecordMap())) return null; } @@ -282,7 +282,7 @@ public static ParticipantGroupAuditEvent participantDeleted( "The participant '" + participantId + "' was deleted", groupId); } - private static String createEncodedRecordMap(Container c, Map bean) + private static String createEncodedRecordMap(Map bean) { if (bean.containsKey("ownerId")) { @@ -299,7 +299,7 @@ private static String createEncodedRecordMap(Container c, Map be .filter(e -> _allowedFields.contains(e.getKey())) .collect(Collectors.toMap(Map.Entry::getKey, e -> e.getValue() != null ? e.getValue() : "")); - return AbstractAuditTypeProvider.encodeForDataMap(c, filteredMap); + return AbstractAuditTypeProvider.encodeForDataMap(filteredMap); } } } diff --git a/study/src/org/labkey/study/controllers/DatasetController.java b/study/src/org/labkey/study/controllers/DatasetController.java index 2b901dcfd52..394cdf23fc8 100644 --- a/study/src/org/labkey/study/controllers/DatasetController.java +++ b/study/src/org/labkey/study/controllers/DatasetController.java @@ -225,7 +225,7 @@ public boolean handlePost(DatasetDeleteForm form, BindException errors) { if (!def.canDeleteDefinition(getUser())) continue; - StudyManager.getInstance().deleteDataset(study, getUser(), def, false); + StudyManager.getInstance().deleteDataset(study, getUser(), def, false, null); transaction.commit(); countDeleted++; } diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index 9c19979a564..1791c03670a 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/study/src/org/labkey/study/controllers/StudyController.java @@ -4803,7 +4803,7 @@ public boolean handlePost(IdForm form, BindException errors) throws Exception try (DbScope.Transaction transaction = scope.ensureTransaction()) { // performStudyResync==false so we can do this out of the transaction - StudyManager.getInstance().deleteDataset(getStudyRedirectIfNull(), getUser(), ds, false); + StudyManager.getInstance().deleteDataset(getStudyRedirectIfNull(), getUser(), ds, false, null); transaction.commit(); } @@ -5117,7 +5117,7 @@ private void deletePreviousDatasetDefinition(StudySnapshotForm form) DatasetDefinition dsDef = StudyManager.getInstance().getDatasetDefinition(study, form.getSnapshotDatasetId()); if (dsDef != null) { - StudyManager.getInstance().deleteDataset(study, getUser(), dsDef, true); + StudyManager.getInstance().deleteDataset(study, getUser(), dsDef, true, null); form.setSnapshotDatasetId(-1); } } diff --git a/study/src/org/labkey/study/dataset/DatasetAuditProvider.java b/study/src/org/labkey/study/dataset/DatasetAuditProvider.java index 5d70107450d..9865ffd556c 100644 --- a/study/src/org/labkey/study/dataset/DatasetAuditProvider.java +++ b/study/src/org/labkey/study/dataset/DatasetAuditProvider.java @@ -122,7 +122,7 @@ public TableInfo getLookupTableInfo() } } }; - appendValueMapColumns(table); + appendValueMapColumns(table, null, true); DetailsURL url = DetailsURL.fromString("dataset/datasetAuditHistory.view?auditRowId=${rowId}"); url.setStrictContainerContextEval(true); diff --git a/study/src/org/labkey/study/model/DatasetDefinition.java b/study/src/org/labkey/study/model/DatasetDefinition.java index 6d3a77e0aa2..bab733391ba 100644 --- a/study/src/org/labkey/study/model/DatasetDefinition.java +++ b/study/src/org/labkey/study/model/DatasetDefinition.java @@ -1152,15 +1152,20 @@ public boolean hasMatchingExtraKey(Dataset other) } @Override - public void delete(User user) + public void delete(User user, @Nullable String auditUserComment) { if (!canDeleteDefinition(user)) { throw new UnauthorizedException("No permission to delete dataset " + getName() + " for study in " + getContainer().getPath()); } - StudyManager.getInstance().deleteDataset(getStudy(), user, this, true); + StudyManager.getInstance().deleteDataset(getStudy(), user, this, true, auditUserComment); } + @Override + public void delete(User user) + { + delete(user, null); + } @Override public void deleteAllRows(User user) @@ -1800,28 +1805,28 @@ protected DatasetAuditProvider.DatasetAuditEvent createDetailedAuditRecord(User if (action==DELETE || action==TRUNCATE) { - oldRecordString = DatasetAuditProvider.encodeForDataMap(c, record); + oldRecordString = DatasetAuditProvider.encodeForDataMap(record); } else if (existingRecord != null && existingRecord.size() > 0) { Pair, Map> rowPair = AuditHandler.getOldAndNewRecordForMerge(record, existingRecord, Collections.emptySet(), tInfo == null? TableInfo.defaultExcludedDetailedUpdateAuditFields : tInfo.getExcludedDetailedUpdateAuditFields(), tInfo); - oldRecordString = DatasetAuditProvider.encodeForDataMap(c, rowPair.first); + oldRecordString = DatasetAuditProvider.encodeForDataMap(rowPair.first); // Check if no fields changed, if so adjust messaging if (rowPair.second.size() == 0 ) { auditComment = "Dataset row was processed, but no changes detected"; // Record values that were processed - newRecordString = DatasetAuditProvider.encodeForDataMap(c, record); + newRecordString = DatasetAuditProvider.encodeForDataMap(record); } else { - newRecordString = DatasetAuditProvider.encodeForDataMap(c, rowPair.second); + newRecordString = DatasetAuditProvider.encodeForDataMap(rowPair.second); } } else { - newRecordString = DatasetAuditProvider.encodeForDataMap(c, record); + newRecordString = DatasetAuditProvider.encodeForDataMap(record); } DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(c, auditComment, _dataset.getDatasetId()); @@ -2819,6 +2824,30 @@ public static void cleanupOrphanedDatasetDomains() } } + public Map getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + map.put("Name", getName()); + if (!StringUtils.isEmpty(getDescription())) + map.put("Description", getDescription()); + if (!StringUtils.isEmpty(getCategory())) + map.put("Category", getCategory()); + if (!StringUtils.isEmpty(getLabel())) + map.put("Label", getLabel()); + if (!StringUtils.isEmpty(getKeyPropertyName())) + map.put("KeyPropertyName", getKeyPropertyName()); + if (!StringUtils.isEmpty(getVisitDatePropertyName())) + map.put("VisitDateColumnName", getVisitDatePropertyName()); + if (!StringUtils.isEmpty(getTag())) + map.put("Tag", getTag()); + map.put("KeyPropertyManaged", getKeyManagementType() != Dataset.KeyManagementType.None); + map.put("IsDemographicData", isDemographicData()); + map.put("IsUseTimeKeyField", getUseTimeKeyField()); + if (getCohortId() != null) + map.put("CohortId", getCohortId()); + + return map; + } public static class Builder implements org.labkey.api.data.Builder { private final String _name; diff --git a/study/src/org/labkey/study/model/DatasetDomainKind.java b/study/src/org/labkey/study/model/DatasetDomainKind.java index ee63af411c6..ba213b65f06 100644 --- a/study/src/org/labkey/study/model/DatasetDomainKind.java +++ b/study/src/org/labkey/study/model/DatasetDomainKind.java @@ -514,7 +514,7 @@ else if (!timepointType.isVisitBased() && getKindName().equals(VisitDatasetDomai DomainUtil.addProperty(newDomain, pd, defaultValues, propertyUris, null); } - newDomain.save(user); + newDomain.save(user, arguments.getAuditRecordMap(), domain.getCalculatedFields()); List indices = (List)domain.getIndices(); newDomain.setPropertyIndices(indices, lowerReservedNames); @@ -646,11 +646,27 @@ private void checkCanUpdate(DatasetDefinition def, Container container, User use } } - private ValidationException updateDomainDescriptor(GWTDomain original, GWTDomain update, - Container container, User user) + private @NotNull ValidationException updateDomainDescriptor(GWTDomain original, GWTDomain update, + @Nullable DatasetDefinition oldDef, @Nullable DatasetDomainKindProperties datasetPropertiesUpdate, Container container, User user, String userComment) { + StringBuilder changeDetails = new StringBuilder(); + boolean hasNameChange = false; + Map oldProps = null; + Map newProps = datasetPropertiesUpdate != null ? datasetPropertiesUpdate.getAuditRecordMap() : null; + + if (oldDef != null) + { + oldProps = oldDef.getAuditRecordMap(); + if (newProps == null) + newProps = oldProps; // no update + + hasNameChange = !datasetPropertiesUpdate.getName().equals(oldDef.getName()); + if (hasNameChange) + changeDetails.append("The name of the dataset '" + oldDef.getName() + "' was changed to '" + datasetPropertiesUpdate.getName() + "'."); + } + ValidationException exception = new ValidationException(); - exception.addErrors(DomainUtil.updateDomainDescriptor(original, update, container, user)); + exception.addErrors(DomainUtil.updateDomainDescriptor(original, update, container, user, hasNameChange, changeDetails.toString(), userComment, oldProps, newProps)); return exception; } @@ -715,7 +731,7 @@ private ValidationException updateDataset(DatasetDomainKindProperties datasetPro @Override public @NotNull ValidationException updateDomain(GWTDomain original, GWTDomain update, - DatasetDomainKindProperties datasetProperties, Container container, User user, boolean includeWarnings) + @Nullable DatasetDomainKindProperties datasetProperties, Container container, User user, boolean includeWarnings, String userComment) { assert original.getDomainURI().equals(update.getDomainURI()); StudyImpl study = StudyManager.getInstance().getStudy(container); @@ -736,7 +752,7 @@ private ValidationException updateDataset(DatasetDomainKindProperties datasetPro Lock[] locks = def == null ? new Lock[0] : new Lock[] { def.getDomainLoadingLock() }; try (DbScope.Transaction transaction = StudySchema.getInstance().getScope().ensureTransaction(locks)) { - ValidationException exception = updateDomainDescriptor(original, update, container, user); + ValidationException exception = updateDomainDescriptor(original, update, def, datasetProperties, container, user, userComment); QueryService.get().saveCalculatedFieldsMetadata("study", update.getQueryName(), hasNameChange ? datasetProperties.getName() : null, update.getCalculatedFields(), !original.getCalculatedFields().isEmpty(), user, container); @@ -760,7 +776,7 @@ private ValidationException updateDataset(DatasetDomainKindProperties datasetPro } @Override - public void deleteDomain(User user, Domain domain) + public void deleteDomain(User user, Domain domain, @Nullable String auditUserComment) { DatasetDefinition def = StudyManager.getInstance().getDatasetDefinition(domain.getTypeURI()); if (def == null) @@ -772,7 +788,7 @@ public void deleteDomain(User user, Domain domain) try (DbScope.Transaction transaction = StudySchema.getInstance().getSchema().getScope().ensureTransaction()) { - StudyManager.getInstance().deleteDataset(study, user, def, false); + StudyManager.getInstance().deleteDataset(study, user, def, false, auditUserComment); transaction.commit(); } } diff --git a/study/src/org/labkey/study/model/DatasetDomainKindProperties.java b/study/src/org/labkey/study/model/DatasetDomainKindProperties.java index 402a7f0c6d8..32cef7cf4fa 100644 --- a/study/src/org/labkey/study/model/DatasetDomainKindProperties.java +++ b/study/src/org/labkey/study/model/DatasetDomainKindProperties.java @@ -1,11 +1,15 @@ package org.labkey.study.model; +import org.apache.commons.lang3.StringUtils; import org.labkey.api.data.Container; import org.labkey.api.exp.api.ExpObject; import org.labkey.api.study.Dataset; import org.labkey.api.study.Study; import org.labkey.api.study.StudyService; +import java.util.LinkedHashMap; +import java.util.Map; + public class DatasetDomainKindProperties implements Cloneable { private String _entityId; @@ -330,4 +334,29 @@ public void setStrictFieldValidation(boolean strictFieldValidation) { _strictFieldValidation = strictFieldValidation; } + + public Map getAuditRecordMap() + { + Map map = new LinkedHashMap<>(); + map.put("Name", getName()); + if (!StringUtils.isEmpty(getDescription())) + map.put("Description", getDescription()); + if (!StringUtils.isEmpty(getCategory())) + map.put("Category", getCategory()); + if (!StringUtils.isEmpty(getLabel())) + map.put("Label", getLabel()); + if (!StringUtils.isEmpty(getKeyPropertyName())) + map.put("KeyPropertyName", getKeyPropertyName()); + if (!StringUtils.isEmpty(getVisitDatePropertyName())) + map.put("VisitDateColumnName", getVisitDatePropertyName()); + if (!StringUtils.isEmpty(getTag())) + map.put("Tag", getTag()); + map.put("KeyPropertyManaged", isKeyPropertyManaged()); + map.put("IsDemographicData", isDemographicData()); + map.put("IsUseTimeKeyField", isUseTimeKeyField()); + if (getCohortId() != null) + map.put("CohortId", getCohortId()); + + return map; + } } diff --git a/study/src/org/labkey/study/model/StudyManager.java b/study/src/org/labkey/study/model/StudyManager.java index ec3ab03b145..4fd01f05100 100644 --- a/study/src/org/labkey/study/model/StudyManager.java +++ b/study/src/org/labkey/study/model/StudyManager.java @@ -97,6 +97,7 @@ import org.labkey.api.exp.api.ProvenanceService; import org.labkey.api.exp.api.StorageProvisioner; import org.labkey.api.exp.property.Domain; +import org.labkey.api.exp.property.DomainAuditProvider; import org.labkey.api.exp.property.DomainProperty; import org.labkey.api.exp.property.PropertyService; import org.labkey.api.exp.property.SystemProperty; @@ -1935,8 +1936,8 @@ public void updateDataQCState(Container container, User user, int datasetId, Col DatasetAuditProvider.DatasetAuditEvent event = new DatasetAuditProvider.DatasetAuditEvent(container, auditComment, datasetId); event.setHasDetails(true); - event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(container, oldQCStates)); - event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(container, newQCStates)); + event.setOldRecordMap(AbstractAuditTypeProvider.encodeForDataMap(oldQCStates)); + event.setNewRecordMap(AbstractAuditTypeProvider.encodeForDataMap(newQCStates)); AuditLogService.get().addEvent(user, event); clearCaches(container, false); @@ -2481,7 +2482,7 @@ public int purgeDataset(DatasetDefinition dataset, @Nullable Date cutoff) * @param performStudyResync whether to kick off our normal bookkeeping. If the whole study is being deleted, * we don't need to bother doing this, for example. */ - public void deleteDataset(StudyImpl study, User user, DatasetDefinition ds, boolean performStudyResync) + public void deleteDataset(StudyImpl study, User user, DatasetDefinition ds, boolean performStudyResync, @Nullable String auditUserComment) { try (Transaction transaction = StudySchema.getInstance().getScope().ensureTransaction()) { @@ -2512,10 +2513,10 @@ public void deleteDataset(StudyImpl study, User user, DatasetDefinition ds, bool } }); - - deleteDatasetType(study, user, ds); try { + deleteDatasetType(study, user, ds, auditUserComment); + QuerySnapshotDefinition def = QueryService.get().getSnapshotDef(study.getContainer(), StudySchema.getInstance().getSchemaName(), ds.getName()); if (def != null) def.delete(user); @@ -2563,7 +2564,7 @@ public void deleteDataset(StudyImpl study, User user, DatasetDefinition ds, bool /** delete a dataset type and data * does not clear typeURI as we're about to delete the dataset */ - private void deleteDatasetType(Study study, User user, DatasetDefinition ds) + private void deleteDatasetType(Study study, User user, DatasetDefinition ds, @Nullable String auditUserComment) { assert StudySchema.getInstance().getSchema().getScope().isTransactionActive(); @@ -2573,7 +2574,17 @@ private void deleteDatasetType(Study study, User user, DatasetDefinition ds) if (!ds.canDeleteDefinition(user)) throw new IllegalStateException("Can't delete dataset: " + ds.getName()); - StorageProvisioner.get().drop(ds.getDomain()); + Domain domain = ds.getDomain(); + if (domain == null) + return; + + DomainAuditProvider.DomainAuditEvent event = new DomainAuditProvider.DomainAuditEvent(study.getContainer(), String.format("The domain %s was deleted", domain.getName())); + event.setUserComment(auditUserComment); + event.setDomainUri(domain.getTypeURI()); + event.setDomainName(domain.getName()); + AuditLogService.get().addEvent(user, event); + + StorageProvisioner.get().drop(domain); if (ds.getTypeURI() != null) { @@ -2634,7 +2645,7 @@ public void deleteAllStudyData(Container c, User user) for (DatasetDefinition dsd : dsds) { if (dsd.getContainer().equals(dsd.getDefinitionContainer())) - deleteDataset(study, user, dsd, false); + deleteDataset(study, user, dsd, false, null); else dsd.deleteAllRows(user); } @@ -4549,7 +4560,7 @@ public DatasetDefinition linkPlaceHolderDataset(StudyImpl study, User user, Data String label = expectationDataset.getLabel(); // no need to resync the study, as there should be no data in the expectation dataset - deleteDataset(study, user, expectationDataset, false); + deleteDataset(study, user, expectationDataset, false, null); targetDataset = targetDataset.createMutable(); targetDataset.setName(name); diff --git a/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java b/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java index 657963ad8f4..3d8f6b3f6a0 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyDatasetsTest.java @@ -33,6 +33,7 @@ import org.labkey.test.pages.ImportDataPage; import org.labkey.test.pages.TimeChartWizard; import org.labkey.test.pages.study.DatasetDesignerPage; +import org.labkey.test.util.AuditLogHelper; import org.labkey.test.util.DataRegionTable; import org.labkey.test.util.LogMethod; import org.labkey.test.util.LoggedParam; @@ -42,6 +43,7 @@ import java.util.ArrayList; import java.util.Arrays; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -82,6 +84,7 @@ public class StudyDatasetsTest extends BaseWebDriverTest a6\t6\tx6\ty6\tz6 """; private static final String DATASET_B_MERGE = "a4\t4\tx4_merged\ty4_merged\tz4_merged\n"; + private final AuditLogHelper _auditLogHelper = new AuditLogHelper(this); @Override protected BrowserType bestBrowser() @@ -250,7 +253,16 @@ protected void renameDataset(@Nullable String error, String orgName, String newN } if (error == null) + { editDatasetPage.clickSave(); + String changeDetails = "Name: " + orgName + " > " + newName.trim(); + changeDetails += "\nLabel: " + orgLabel + " > " + newLabel.trim(); + AuditLogHelper.DetailedAuditEventRow expectedDomainEvent = new AuditLogHelper.DetailedAuditEventRow(null, orgName, null, + "The name of the dataset '" + orgName + "' was changed to '" + newName.trim() + "'. The descriptor of domain " + orgName + " was updated.", + "", null, null, changeDetails); + boolean pass = _auditLogHelper.validateLastDomainAuditEvents(orgName, getProjectName(), expectedDomainEvent, Collections.emptyMap()); + checker().verifyTrue("The comment logged for the dataset renaming was not as expected", pass); + } else { editDatasetPage.saveExpectFail(error); @@ -309,6 +321,15 @@ protected void deleteFields(String name) assertEquals(Arrays.asList("XTest"), remainingFields); editDatasetPage.clickSave(); + + AuditLogHelper.DetailedAuditEventRow expectedDomainEvent = new AuditLogHelper.DetailedAuditEventRow(null, name, null, + "The column(s) of domain " + name + " were modified.", + "", null, null, null); + boolean pass = _auditLogHelper.validateLastDomainAuditEvents(name, getProjectName(), expectedDomainEvent, + Map.of("YTest", new AuditLogHelper.DetailedAuditEventRow(null, "YTest", "Deleted",null,null, null, null, null), + "ZTest", new AuditLogHelper.DetailedAuditEventRow(null, "ZTest", "Deleted",null,null, null, null, null) + )); + checker().verifyTrue("Domain audit log not as expected after removing fields", pass); } @LogMethod diff --git a/study/test/src/org/labkey/test/tests/study/StudyTest.java b/study/test/src/org/labkey/test/tests/study/StudyTest.java index 9393eaa6b46..ae70ad9912d 100644 --- a/study/test/src/org/labkey/test/tests/study/StudyTest.java +++ b/study/test/src/org/labkey/test/tests/study/StudyTest.java @@ -43,6 +43,7 @@ import org.labkey.test.pages.study.ManageStudyPage; import org.labkey.test.params.FieldDefinition; import org.labkey.test.tests.StudyBaseTest; +import org.labkey.test.util.AuditLogHelper; import org.labkey.test.util.ChartHelper; import org.labkey.test.util.DataRegionExportHelper; import org.labkey.test.util.DataRegionTable; @@ -78,6 +79,7 @@ public class StudyTest extends StudyBaseTest protected String datasetLink = datasetCount + " datasets"; protected static final String DEMOGRAPHICS_DESCRIPTION = "This is the demographics dataset, dammit. Here are some \u2018special symbols\u2019 - they help test that we're roundtripping in UTF-8."; protected static final String DEMOGRAPHICS_TITLE = "DEM-1: Demographics"; + protected static final String DEMOGRAPHICS_DOMAIN_NAME = "DEM-1"; protected String _tsv = "participantid\tsequencenum\tvisitdate\tSampleId\tDateField\tNumberField\tTextField\treplace\taliasedColumn\n" + "999321234\t1\t1/1/2006\t1234_A\t2/1/2006\t1.2\ttext\t\taliasedData\n" + @@ -1134,6 +1136,13 @@ protected void verifyManageDatasetsPage() setDemographicsBit(DEMOGRAPHICS_TITLE, false) .clickViewData(); + String changeDetails = "IsDemographicData: true > false" ; + AuditLogHelper.DetailedAuditEventRow expectedDomainEvent = new AuditLogHelper.DetailedAuditEventRow(null, DEMOGRAPHICS_DOMAIN_NAME, null, + "The descriptor of domain DEM-1 was updated", + "", null, null, changeDetails); + boolean pass = _auditLogHelper.validateLastDomainAuditEvents(DEMOGRAPHICS_DOMAIN_NAME, getProjectName(), expectedDomainEvent, Collections.emptyMap()); + checker().verifyTrue("Domain audit comment not as expected after changing demographic bit", pass); + log("verify "); _customizeViewsHelper.openCustomizeViewPanel(); _customizeViewsHelper.showHiddenItems(); @@ -1292,6 +1301,14 @@ protected void verifyParticipantVisitDay() .selectVisitDateColumn("DEMdt") .clickApply() .clickSave(); + + AuditLogHelper.DetailedAuditEventRow expectedDomainEvent = new AuditLogHelper.DetailedAuditEventRow(null, DEMOGRAPHICS_DOMAIN_NAME, null, + "The column(s) of domain DEM-1 were modified", + "", null, null, ""); + boolean pass = _auditLogHelper.validateLastDomainAuditEvents(DEMOGRAPHICS_DOMAIN_NAME, getProjectName(), expectedDomainEvent, + Map.of("VisitDay", new AuditLogHelper.DetailedAuditEventRow(null, "VisitDay", null, null, null, null, null, "VisitDateColumnName: > DEMdt"))); + checker().verifyTrue("Domain audit comment not as expected after changing visit date column", pass); + new DatasetPropertiesPage(getDriver()) .clickViewData();