Add detailed auditing of domain changes & comment ability

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<String> _excludedContainerIds;
+    private String _auditUserComment;
 
     public GWTProtocol()
     {
@@ -436,4 +439,39 @@ public void setExcludedContainerIds(List<String> excludedContainerIds)
     {
         _excludedContainerIds = excludedContainerIds;
     }
+
+    public String getAuditUserComment()
+    {
+        return _auditUserComment;
+    }
+
+    public void setAuditUserComment(String auditUserComment)
+    {
+        _auditUserComment = auditUserComment;
+    }
+
+    public Map<String, Object> getAuditRecordMap()
+    {
+        Map<String, Object> 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;
+    }
+
+
 }

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<String> toStringVals(List<GWTIndex> indices, Set<PropertyStorageSpec.Index> excludeBaseIndices)
+    {
+        if (indices == null || indices.isEmpty())
+            return null;
+
+        Set<String> 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();
+    }
+
 }

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<AnalysisScript> scripts) throws ExperimentException
+    public Pair<ValidationException, Pair<String, String>> setValidationAndAnalysisScripts(ExpProtocol protocol, @NotNull List<AnalysisScript> scripts) throws ExperimentException
     {
         Map<String, ObjectProperty> 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 */

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<AnalysisScript> scripts) throws ExperimentException;
+    Pair<ValidationException, Pair<String, String>> setValidationAndAnalysisScripts(ExpProtocol protocol, @NotNull List<AnalysisScript> scripts) throws ExperimentException;
 
     @NotNull
     List<AnalysisScript> getValidationAndAnalysisScripts(ExpProtocol protocol, Scope scope);

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);
     }

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<String, String> decodeFromDataMap(String properties)
         }
     }
 
-    public static String encodeForDataMap(Container c, Map<String, ?> properties)
+    public static String encodeForDataMap(Map<String, ?> properties)
     {
         if (properties == null) return null;
 

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);

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<? extends GWTPropertyDescriptor> original, GWTDomain<? extends GWTPropertyDescriptor> 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();
     }

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<? extends GWTConditionalFormat> formats, Co
 
         return success;
     }
+
+    public String toStringVal()
+    {
+        return getFilter() + ": " + getCssStyle();
+    }
+
+    public static @Nullable String toStringVal(List<? extends GWTConditionalFormat> formats)
+    {
+        if (formats == null || formats.isEmpty())
+            return null;
+
+        List<String> 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, ", ");
+    }
 }

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

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));
         }
     }