GitHub Issue 1028: Can't delete a sample when any sample in the sample type has been linked to study

api/src/org/labkey/api/study/publish/StudyPublishService.java

@@ -156,7 +156,7 @@ ActionURL publishData(User user, Container sourceContainer, @Nullable Container
 
     String checkForLockedLinks(Dataset def, @Nullable List<Long> rowIds);
 
-    void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection<Pair<String,Long>> datasetRowLsidAndSourceRowIds);
+    void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection<Long> rowIds);
 
     /**
      * Adds columns to an assay data table, providing a link to any datasets that have

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

@@ -15,6 +15,7 @@
  */
 package org.labkey.study.assay;
 
+import org.labkey.api.data.CompareType;
 import org.labkey.api.data.Container;
 import org.labkey.api.data.SimpleFilter;
 import org.labkey.api.data.TableInfo;
@@ -30,8 +31,10 @@
 import org.labkey.api.query.QueryService;
 import org.labkey.api.query.UserSchema;
 import org.labkey.api.query.ValidationException;
+import org.labkey.api.security.ElevatedUser;
 import org.labkey.api.security.User;
 import org.labkey.api.security.permissions.DeletePermission;
+import org.labkey.api.security.roles.ReaderRole;
 import org.labkey.api.study.Dataset;
 import org.labkey.api.study.publish.StudyPublishService;
 import org.labkey.api.view.UnauthorizedException;
@@ -72,41 +75,55 @@ public void beforeMaterialDelete(List<? extends ExpMaterial> materials, Containe
 
         // It's likely that we'll have multiple materials from the same sample type, so group them for efficient processing
 
-        Map<ExpSampleType, List<ExpMaterial>> typeToMaterials = new HashMap<>();
+        Map<ExpSampleType, Map<Container, List<ExpMaterial>>> typeToMaterials = new HashMap<>();
 
         for (ExpMaterial material: materials)
         {
             ExpSampleType sampleType = material.getSampleType();
             if (sampleType != null)
             {
-                typeToMaterials.computeIfAbsent(sampleType, x -> new ArrayList<>()).add(material);
+                Container materialContainer = material.getContainer();
+                typeToMaterials.
+                        computeIfAbsent(sampleType, k -> new HashMap<>()).
+                        computeIfAbsent(materialContainer, k -> new ArrayList<>()).
+                        add(material);
             }
         }
 
-        for (Map.Entry<ExpSampleType, List<ExpMaterial>> entry : typeToMaterials.entrySet())
+        for (Map.Entry<ExpSampleType, Map<Container, List<ExpMaterial>>> entry : typeToMaterials.entrySet())
         {
             for (Dataset dataset: StudyPublishService.get().getDatasetsForPublishSource(entry.getKey().getRowId(), Dataset.PublishSource.SampleType))
             {
-                TableInfo t = dataset.getTableInfo(user);
-                if (null == t || !t.hasPermission(user, DeletePermission.class))
+                Map<Container, List<ExpMaterial>> containerSamples = entry.getValue();
+
+                for (Map.Entry<Container, List<ExpMaterial>> containerEntry : containerSamples.entrySet())
                 {
-                    throw new UnauthorizedException("Cannot delete rows from dataset " + dataset);
-                }
+                    Container sampleContainer = containerEntry.getKey();
+                    List<ExpMaterial> samples = containerEntry.getValue();
 
-                UserSchema schema = QueryService.get().getUserSchema(user, dataset.getContainer(), "study");
-                TableInfo tableInfo = schema.getTable(dataset.getName());
+                    // Need Read permission to check for linked samples
+                    User userWithReadPerm = ElevatedUser.getElevatedUser(user, ReaderRole.class);
 
-                // Future optimization - query for all the materials at once
-                for (ExpMaterial material : entry.getValue())
-                {
-                    SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.RowId.toString()), material.getRowId());
-                    String lsid = new TableSelector(tableInfo, singleton("LSID"), filter, null).getObject(String.class);
+                    UserSchema schemaWithReadPerm = QueryService.get().getUserSchema(userWithReadPerm, dataset.getContainer(), "study");
+                    TableInfo tableInfoForRead = schemaWithReadPerm.getTable(dataset.getName());
 
-                    if (lsid != null)
+                    // GitHub Issue 1028: Can't delete a sample when any sample in the sample type has been linked to study
+                    // check if samples are linked to the dataset, if not, skip the permission check for DeletePermission since we won't be deleting any rows
+                    SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.RowId.toString()), samples.stream().map(ExpMaterial::getRowId).toList(), CompareType.IN);
+                    List<String> linkedLsids = new TableSelector(tableInfoForRead, singleton("LSID"), filter, null).getArrayList(String.class);
+
+                    if (linkedLsids.isEmpty())
+                        continue;
+
+                    TableInfo tableInfo = dataset.getTableInfo(user);
+                    if (null == tableInfo || !tableInfo.hasPermission(user, DeletePermission.class))
                     {
-                        StudyPublishService.get().addRecallAuditEvent(material.getContainer(), user, dataset, 1, null);
-                        dataset.deleteDatasetRows(user, Arrays.asList(lsid));
+                        throw new UnauthorizedException("Cannot delete rows from dataset " + dataset);
                     }
+
+                    List<Long> linkedIds = samples.stream().filter(s -> linkedLsids.contains(s.getLSID())).map(ExpMaterial::getRowId).toList();
+                    StudyPublishService.get().addRecallAuditEvent(sampleContainer, user, dataset, linkedLsids.size(), linkedIds);
+                    dataset.deleteDatasetRows(user, linkedLsids);
                 }
             }
         }

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

@@ -1520,7 +1520,7 @@ public String checkForLockedLinks(Dataset def, @Nullable List<Long> rowIds)
     }
 
     @Override
-    public void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection<Pair<String,Long>> pairs)
+    public void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection<Long> rowIds)
     {
         Dataset.PublishSource sourceType = def.getPublishSource();
         if (sourceType != null)
@@ -1541,15 +1541,14 @@ public void addRecallAuditEvent(Container sourceContainer, User user, Dataset de
             AuditLogService.get().addEvent(user, event);
 
             // Create sample timeline event for each of the samples
-            if (sourceType == Dataset.PublishSource.SampleType && pairs != null)
+            if (sourceType == Dataset.PublishSource.SampleType && rowIds != null && rowIds.isEmpty())
             {
                 var timelineEventType = SampleTimelineAuditEvent.SampleTimelineEventType.RECALL;
                 Map<String, Object> eventMetadata = new HashMap<>();
                 eventMetadata.put(SAMPLE_TIMELINE_EVENT_TYPE, timelineEventType.name());
                 String metadata = AbstractAuditTypeProvider.encodeForDataMap(eventMetadata);
 
-                List<Long> sampleIds = pairs.stream().map(Pair::getValue).collect(toList());
-                List<? extends ExpMaterial> samples = ExperimentService.get().getExpMaterials(sampleIds);
+                List<? extends ExpMaterial> samples = ExperimentService.get().getExpMaterials(rowIds);
                 List<AuditTypeEvent> events = new ArrayList<>(samples.size());
                 for (ExpMaterial sample : samples)
                 {

study/src/org/labkey/study/controllers/StudyController.java

@@ -313,6 +313,7 @@
 import java.util.concurrent.TimeUnit;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
+import java.util.stream.Collectors;
 
 import static org.labkey.api.util.IntegerUtils.asInteger;
 import static org.labkey.study.model.QCStateSet.PUBLIC_STATES_LABEL;
@@ -3106,9 +3107,10 @@ public boolean handlePost(DeleteDatasetRowsForm form, BindException errors)
                 {
                     String sourceLsid = entry.getKey();
                     Collection<Pair<String, Long>> pairs = entry.getValue();
+                    Collection<Long> rowIds = pairs.stream().map(Pair::getValue).collect(Collectors.toList());
                     Container sourceContainer = publishSource.resolveSourceLsidContainer(sourceLsid, _sourceRowId);
                     if (sourceContainer != null)
-                        StudyPublishService.get().addRecallAuditEvent(sourceContainer, getUser(), _def, pairs.size(), pairs);
+                        StudyPublishService.get().addRecallAuditEvent(sourceContainer, getUser(), _def, pairs.size(), rowIds);
                 }
             }