Issue 1324: Pass through audit comments to ExperimentListener on sample type and source type deletion

api/src/org/labkey/api/exp/api/ExperimentListener.java

@@ -15,6 +15,7 @@
  */
 package org.labkey.api.exp.api;
 
+import org.jetbrains.annotations.Nullable;
 import org.labkey.api.data.Container;
 import org.labkey.api.query.BatchValidationException;
 import org.labkey.api.security.User;
@@ -51,13 +52,13 @@ default void afterResultDataCreated(Container container, User user, ExpRun run,
     default void beforeRunDelete(ExpProtocol protocol, ExpRun run, User user) { }
 
     /** Called before deleting the datas. */
-    default void beforeDataDelete(Container c, User user, List<? extends ExpData> data) { }
+    default void beforeDataDelete(Container c, User user, List<? extends ExpData> data, @Nullable String auditUserComment) { }
 
     /** Called after deleting the datas. */
     default void afterDataDelete(Container c, User user, List<? extends ExpData> data) { }
 
     /** Called before deleting experiment materials (in-transaction). */
-    default void beforeMaterialDelete(List<? extends ExpMaterial> materials, Container container, User user) { }
+    default void beforeMaterialDelete(List<? extends ExpMaterial> materials, Container container, User user, @Nullable String auditUserComment) { }
 
     /** Called after a material has been created (and saved). NOTE: This is not currently implemented. */
     default void afterMaterialCreated(List<? extends ExpMaterial> materials, Container container, User user) { }

api/src/org/labkey/api/exp/property/DomainAuditProvider.java

@@ -20,6 +20,7 @@
 import org.labkey.api.audit.AuditTypeEvent;
 import org.labkey.api.audit.AuditTypeProvider;
 import org.labkey.api.audit.DetailedAuditTypeEvent;
+import org.labkey.api.audit.TransactionAuditProvider;
 import org.labkey.api.audit.query.AbstractAuditDomainKind;
 import org.labkey.api.audit.query.DefaultAuditTypeTable;
 import org.labkey.api.data.ColumnInfo;
@@ -104,6 +105,8 @@ protected void initColumn(MutableColumnInfo col)
                 }
                 else if (COLUMN_NAME_USER_COMMENT.equalsIgnoreCase(col.getName()))
                     col.setLabel("User Comment");
+                else if (COLUMN_NAME_TRANSACTION_ID.equalsIgnoreCase(col.getName()))
+                    col.setLabel("Transaction ID");
             }
         };
 
@@ -137,6 +140,7 @@ public DomainAuditEvent()
         public DomainAuditEvent(Container container, String comment)
         {
             super(EVENT_TYPE, container, comment);
+            setTransactionEvent(TransactionAuditProvider.getCurrentTransactionAuditEvent(), EVENT_TYPE);
         }
 
         public String getDomainUri()
@@ -166,6 +170,7 @@ public Map<String, Object> getAuditLogMessageElements()
             elements.put("domainUri", getDomainUri());
             elements.put("domainName", getDomainName());
             elements.putAll(super.getAuditLogMessageElements());
+            elements.put("transactionId", getTransactionId());
             return elements;
         }
     }
@@ -187,6 +192,7 @@ public DomainAuditDomainKind()
             fields.add(createOldDataMapPropertyDescriptor());
             fields.add(createNewDataMapPropertyDescriptor());
             fields.add(createPropertyDescriptor(COLUMN_NAME_USER_COMMENT, PropertyType.STRING));
+            fields.add(createPropertyDescriptor(COLUMN_NAME_TRANSACTION_ID, PropertyType.BIGINT));
             _fields = Collections.unmodifiableSet(fields);
         }
 

assay/src/org/labkey/assay/plate/PlateManager.java

@@ -5125,7 +5125,7 @@ private record HydratedResult(List<PlateData> plateData, @Nullable Integer plate
     }
 
     @Override
-    public void beforeMaterialDelete(List<? extends ExpMaterial> materials, Container container, User user)
+    public void beforeMaterialDelete(List<? extends ExpMaterial> materials, Container container, User user, @Nullable String auditUserComment)
     {
         if (materials == null || materials.isEmpty())
             return;

experiment/src/org/labkey/experiment/api/ExpDataClassDataTableImpl.java

@@ -32,7 +32,6 @@
 import org.labkey.api.data.BaseColumnInfo;
 import org.labkey.api.data.ColumnHeaderType;
 import org.labkey.api.data.ColumnInfo;
-import org.labkey.api.data.CompareType;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.ContainerFilter;
 import org.labkey.api.data.ContainerManager;
@@ -146,7 +145,6 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.function.Supplier;
-import java.util.function.UnaryOperator;
 import java.util.stream.Collectors;
 
 import static org.labkey.api.dataiterator.DataIteratorUtil.DUPLICATE_COLUMN_IN_DATA_ERROR;
@@ -1530,7 +1528,7 @@ protected void _delete(Container c, Map<String, Object> row) throws InvalidKeyEx
         @Override
         protected int truncateRows(User user, Container container)
         {
-            return ExperimentServiceImpl.get().truncateDataClass(_dataClass, user, container);
+            return ExperimentServiceImpl.get().truncateDataClass(_dataClass, user, container, null);
         }
 
         @Override

experiment/src/org/labkey/experiment/api/ExpDataClassDataTestCase.jsp

@@ -329,7 +329,7 @@ private void testTruncateRows(ExpDataClassImpl dataClass, TableInfo table, Strin
     {
         // TODO: truncate rows API doesn't support truncating from all containers
         //count = table.getUpdateService().truncateRows(user, c, null, null);
-        count = ExperimentServiceImpl.get().truncateDataClass(dataClass, _user, null);
+        count = ExperimentServiceImpl.get().truncateDataClass(dataClass, _user, null, null);
         tx.commit();
     }
 

experiment/src/org/labkey/experiment/api/ExpDataImpl.java

@@ -351,7 +351,7 @@ public void delete(User user)
     @Override
     public void delete(User user, boolean deleteRunsUsingData)
     {
-        ExperimentServiceImpl.get().deleteDataByRowIds(user, getContainer(), Collections.singleton(getRowId()), deleteRunsUsingData);
+        ExperimentServiceImpl.get().deleteDataByRowIds(user, getContainer(), Collections.singleton(getRowId()), deleteRunsUsingData, null);
     }
 
     public String getMimeType()

experiment/src/org/labkey/experiment/api/ExpRunImpl.java

@@ -548,7 +548,7 @@ public void deleteProtocolApplications(List<ExpDataImpl> datasToDelete, User use
 
         deleteProtocolApplicationProvenance();
 
-        svc.beforeDeleteData(user, getContainer(), datasToDelete);
+        svc.beforeDeleteData(user, getContainer(), datasToDelete, null);
 
         deleteInputObjects(svc, dialect);
 

experiment/src/org/labkey/experiment/api/ExperimentServiceImpl.java

@@ -4883,7 +4883,7 @@ public int deleteMaterialByRowIds(
     {
         SQLFragment rowIdSQL = new SQLFragment("RowId ");
         rowIdSQL.appendInClause(selectedMaterialIds, getSchema().getSqlDialect());
-        return deleteMaterialBySqlFilter(user, container, rowIdSQL, deleteRunsUsingMaterials, false, stDeleteFrom, ignoreStatus, truncateContainer);
+        return deleteMaterialBySqlFilter(user, container, rowIdSQL, deleteRunsUsingMaterials, false, stDeleteFrom, ignoreStatus, truncateContainer, null);
     }
 
     /**
@@ -4892,7 +4892,9 @@ public int deleteMaterialByRowIds(
      * null, the samples must have cpasType of {@link ExpMaterial#DEFAULT_CPAS_TYPE} unless
      * the <code>deleteFromAllSampleTypes</code> flag is true.
      * Deleting from multiple SampleTypes is only needed when cleaning an entire container.
+     *
      * @param truncateContainer delete all rows for this container. Not a real DB truncate because there may be rows in other containers.
+     * @param auditUserComment
      */
     public int deleteMaterialBySqlFilter(
         User user,
@@ -4902,8 +4904,8 @@ public int deleteMaterialBySqlFilter(
         boolean deleteFromAllSampleTypes,
         @Nullable ExpSampleType stDeleteFrom,
         boolean ignoreStatus,
-        boolean truncateContainer
-    )
+        boolean truncateContainer,
+        @Nullable String auditUserComment)
     {
         if (stDeleteFrom != null && deleteFromAllSampleTypes)
             throw new IllegalArgumentException("Can only delete from multiple sample types when no sample type is provided");
@@ -4983,7 +4985,7 @@ public int deleteMaterialBySqlFilter(
 
                 try (Timing ignored = MiniProfiler.step("beforeDelete"))
                 {
-                    beforeDeleteMaterials(user, container, materials);
+                    beforeDeleteMaterials(user, container, materials, auditUserComment);
                 }
 
                 try (Timing ignored = MiniProfiler.step("deleteRunsUsingInput"))
@@ -5393,12 +5395,12 @@ AND pa.cpastype IN ('ProtocolApplication', 'ExperimentRun')
 
     }
 
-    public void deleteDataByRowIds(User user, Container container, Collection<Long> selectedDataIds)
+    public void deleteDataByRowIds(User user, Container container, Collection<Long> selectedDataIds, @Nullable String auditUserComment)
     {
-        deleteDataByRowIds(user, container, selectedDataIds, true);
+        deleteDataByRowIds(user, container, selectedDataIds, true, auditUserComment);
     }
 
-    public void deleteDataByRowIds(User user, Container container, Collection<Long> selectedDataIds, boolean deleteRunsUsingData)
+    public void deleteDataByRowIds(User user, Container container, Collection<Long> selectedDataIds, boolean deleteRunsUsingData, @Nullable String auditUserComment)
     {
         if (selectedDataIds.isEmpty())
             return;
@@ -5424,7 +5426,7 @@ public void deleteDataByRowIds(User user, Container container, Collection<Long>
             }
 
             List<ExpDataImpl> expDatas = ExpDataImpl.fromDatas(datas);
-            beforeDeleteData(user, container, expDatas);
+            beforeDeleteData(user, container, expDatas, auditUserComment);
 
             // Delete any runs using the data if the ProtocolImplementation allows it
             if (deleteRunsUsingData)
@@ -5655,13 +5657,13 @@ public void deleteAllExpObjInContainer(Container c, User user) throws Experiment
             // now delete starting materials that were not associated with a MaterialSource upload.
             // we get this list now so that it doesn't include all the run-scoped Materials that were
             // deleted already
-            deleteMaterialBySqlFilter(user, c, new SQLFragment("Container = ?", c), true, true, null, true, true);
+            deleteMaterialBySqlFilter(user, c, new SQLFragment("Container = ?", c), true, true, null, true, true, null);
 
             // same drill for data objects
             sql = "SELECT RowId FROM exp.Data WHERE Container = ?";
             Collection<Long> dataIds = new SqlSelector(getExpSchema(), sql, c).getCollection(Long.class);
             LOG.debug("Deleting {} dataIds {} ", dataIds.size(), dataIds);
-            deleteDataByRowIds(user, c, dataIds);
+            deleteDataByRowIds(user, c, dataIds, null);
 
             LOG.debug("Deleting objects from container {}", c);
             OntologyManager.deleteAllObjects(c, user);
@@ -5728,7 +5730,7 @@ public void moveRuns(ViewBackgroundInfo info, Container sourceContainer, List<Ex
         }
     }
 
-    public void beforeDeleteData(User user, Container container, List<ExpDataImpl> datas)
+    public void beforeDeleteData(User user, Container container, List<ExpDataImpl> datas, @Nullable String auditUserComment)
     {
         try
         {
@@ -5751,7 +5753,7 @@ public void beforeDeleteData(User user, Container container, List<ExpDataImpl> d
 
         for (ExperimentListener listener : _listeners)
         {
-            listener.beforeDataDelete(container, user, datas);
+            listener.beforeDataDelete(container, user, datas, auditUserComment);
         }
     }
 
@@ -5763,12 +5765,12 @@ public void afterDeleteData(User user, Container container, List<ExpDataImpl> da
         }
     }
 
-    public void beforeDeleteMaterials(User user, Container container, List<? extends ExpMaterial> materials)
+    public void beforeDeleteMaterials(User user, Container container, List<? extends ExpMaterial> materials, @Nullable String auditUserComment)
     {
         // Notify that a deletion is about to happen
         for (ExperimentListener materialListener : _listeners)
         {
-            materialListener.beforeMaterialDelete(materials, container, user);
+            materialListener.beforeMaterialDelete(materials, container, user, auditUserComment);
         }
     }
 
@@ -6204,7 +6206,7 @@ void deleteDomainObjects(Container c, String lsid) throws ExperimentException
      * Delete all exp.Data from the DataClass.  If container is not provided,
      * all rows from the DataClass will be deleted regardless of container.
      */
-    public int truncateDataClass(ExpDataClassImpl dataClass, User user, @Nullable Container c)
+    public int truncateDataClass(ExpDataClassImpl dataClass, User user, @Nullable Container c, @Nullable String auditUserComment)
     {
         assert getExpSchema().getScope().isTransactionActive();
 
@@ -6221,7 +6223,7 @@ public int truncateDataClass(ExpDataClassImpl dataClass, User user, @Nullable Co
         for (Map.Entry<String, Collection<Long>> entry : byContainer.asMap().entrySet())
         {
             Container container = ContainerManager.getForId(entry.getKey());
-            deleteDataByRowIds(user, container, entry.getValue());
+            deleteDataByRowIds(user, container, entry.getValue(), auditUserComment);
             count += entry.getValue().size();
         }
         return count;
@@ -6249,7 +6251,7 @@ public void deleteDataClass(long rowId, Container c, User user, @Nullable final
 
         try (DbScope.Transaction transaction = ensureTransaction())
         {
-            truncateDataClass(dataClass, user, null);
+            truncateDataClass(dataClass, user, null, auditUserComment);
 
             d.delete(user, auditUserComment);
 

experiment/src/org/labkey/experiment/api/SampleTypeServiceImpl.java

@@ -625,7 +625,7 @@ public Pair<String, String> getSampleTypeSamplePrefixLsids(Container container)
      * Delete all exp.Material from the SampleType. If container is not provided,
      * all rows from the SampleType will be deleted regardless of container.
      */
-    public int truncateSampleType(ExpSampleTypeImpl source, User user, @Nullable Container c)
+    public int truncateSampleType(ExpSampleTypeImpl source, User user, @Nullable Container c, @Nullable String auditUserComment)
     {
         assert getExpSchema().getScope().isTransactionActive();
 
@@ -649,7 +649,7 @@ public int truncateSampleType(ExpSampleTypeImpl source, User user, @Nullable Con
             SQLFragment sqlFilter = new SQLFragment("CpasType = ? AND Container = ?");
             sqlFilter.add(source.getLSID());
             sqlFilter.add(toDelete);
-            count += ExperimentServiceImpl.get().deleteMaterialBySqlFilter(user, toDelete, sqlFilter, true, false, source, true, true);
+            count += ExperimentServiceImpl.get().deleteMaterialBySqlFilter(user, toDelete, sqlFilter, true, false, source, true, true, auditUserComment);
         }
         return count;
     }
@@ -670,7 +670,7 @@ public void deleteSampleType(long rowId, Container c, User user, @Nullable Strin
         {
             // TODO: option to skip deleting rows from the materialized table since we're about to delete it anyway
             // TODO do we need both truncateSampleType() and deleteDomainObjects()?
-            truncateSampleType(source, user, null);
+            truncateSampleType(source, user, null, auditUserComment);
 
             StudyService studyService = StudyService.get();
             if (studyService != null)

experiment/src/org/labkey/experiment/api/SampleTypeUpdateServiceDI.java

@@ -759,7 +759,7 @@ protected int truncateRows(User user, Container container)
         if (_sampleType == null)
             return 0;
 
-        int ret = SampleTypeServiceImpl.get().truncateSampleType(_sampleType, user, container);
+        int ret = SampleTypeServiceImpl.get().truncateSampleType(_sampleType, user, container, null);
         if (ret > 0)
         {
             // NOTE: Not necessary to call onSamplesChanged -- already called by truncateSampleSet

experiment/src/org/labkey/experiment/controllers/exp/ExperimentController.java

@@ -165,6 +165,7 @@
 import org.labkey.api.pipeline.PipelineValidationException;
 import org.labkey.api.qc.SampleStatusService;
 import org.labkey.api.query.AbstractQueryImportAction;
+import org.labkey.api.query.AbstractQueryUpdateService;
 import org.labkey.api.query.BatchValidationException;
 import org.labkey.api.query.DetailsURL;
 import org.labkey.api.query.DuplicateKeyException;
@@ -3589,6 +3590,11 @@ public boolean handlePost(DeleteForm deleteForm, BindException errors) throws Ex
             {
                 try (DbScope.Transaction tx = ExperimentService.get().ensureTransaction())
                 {
+                    if (tx.getAuditEvent() == null)
+                    {
+                        TransactionAuditProvider.TransactionAuditEvent auditEvent = AbstractQueryUpdateService.createTransactionAuditEvent(getContainer(), QueryService.AuditAction.DELETE, getTransactionAuditDetails());
+                        AbstractQueryUpdateService.addTransactionAuditEvent(tx, getUser(), auditEvent);
+                    }
                     tx.addCommitTask(deleteForm::clearSelected, POSTCOMMIT);
 
                     deleteObjects(deleteForm);

list/src/org/labkey/list/PicklistMaterialListener.java

@@ -15,6 +15,7 @@
  */
 package org.labkey.list;
 
+import org.jetbrains.annotations.Nullable;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.SimpleFilter;
 import org.labkey.api.data.Table;
@@ -35,7 +36,7 @@
 public class PicklistMaterialListener implements ExperimentListener
 {
     @Override
-    public void beforeMaterialDelete(List<? extends ExpMaterial> materials, Container container, User user)
+    public void beforeMaterialDelete(List<? extends ExpMaterial> materials, Container container, User user, @Nullable String auditUserComment)
     {
         Collection<ListDef> picklists = ListManager.get().getPicklists(container);
         List<Long> materialIds = materials.stream().map(ExpMaterial::getRowId).collect(Collectors.toList());

study/src/org/labkey/study/assay/ExperimentListenerImpl.java

@@ -15,6 +15,7 @@
  */
 package org.labkey.study.assay;
 
+import org.jetbrains.annotations.Nullable;
 import org.labkey.api.data.CompareType;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.SimpleFilter;
@@ -68,7 +69,7 @@ public void afterResultDataCreated(Container container, User user, ExpRun run, E
     }
 
     @Override
-    public void beforeMaterialDelete(List<? extends ExpMaterial> materials, Container container, User user)
+    public void beforeMaterialDelete(List<? extends ExpMaterial> materials, Container container, User user, @Nullable String auditUserComment)
     {
         // Check for datasets that need rows deleted due to a linked Sample Type row-level deletion