diff --git a/api/src/org/labkey/api/study/publish/StudyPublishService.java b/api/src/org/labkey/api/study/publish/StudyPublishService.java index 027a78aabf9..45b554d9159 100644 --- a/api/src/org/labkey/api/study/publish/StudyPublishService.java +++ b/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 rowIds); - void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection> datasetRowLsidAndSourceRowIds); + void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection rowIds); /** * Adds columns to an assay data table, providing a link to any datasets that have diff --git a/study/src/org/labkey/study/assay/ExperimentListenerImpl.java b/study/src/org/labkey/study/assay/ExperimentListenerImpl.java index ece944cc309..ec8c6e027b9 100644 --- a/study/src/org/labkey/study/assay/ExperimentListenerImpl.java +++ b/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; @@ -41,6 +44,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Set; import static java.util.Collections.singleton; @@ -72,41 +76,58 @@ public void beforeMaterialDelete(List materials, Containe // It's likely that we'll have multiple materials from the same sample type, so group them for efficient processing - Map> typeToMaterials = new HashMap<>(); + Map>> 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> entry : typeToMaterials.entrySet()) + for (Map.Entry>> 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)) - { - throw new UnauthorizedException("Cannot delete rows from dataset " + dataset); - } + Map> containerSamples = entry.getValue(); + + // Need Read permission to check for linked samples + User userWithReadPerm = ElevatedUser.getElevatedUser(user, ReaderRole.class); - UserSchema schema = QueryService.get().getUserSchema(user, dataset.getContainer(), "study"); - TableInfo tableInfo = schema.getTable(dataset.getName()); + UserSchema schemaWithReadPerm = QueryService.get().getUserSchema(userWithReadPerm, dataset.getContainer(), "study"); + TableInfo tableInfoForRead = schemaWithReadPerm.getTable(dataset.getName()); + if (null == tableInfoForRead) + throw new UnauthorizedException("Cannot delete rows from dataset " + dataset); - // Future optimization - query for all the materials at once - for (ExpMaterial material : entry.getValue()) + for (Map.Entry> containerEntry : containerSamples.entrySet()) { - SimpleFilter filter = new SimpleFilter(FieldKey.fromParts(ExpMaterialTable.Column.RowId.toString()), material.getRowId()); - String lsid = new TableSelector(tableInfo, singleton("LSID"), filter, null).getObject(String.class); + Container sampleContainer = containerEntry.getKey(); + List samples = containerEntry.getValue(); + + // 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); + Map[] linkedLsidRowIds = new TableSelector(tableInfoForRead, Set.of("LSID", "RowId"), filter, null).getMapArray(); - if (lsid != null) + Set linkedLsids = Arrays.stream(linkedLsidRowIds).map(m -> (String)m.get("LSID")).collect(java.util.stream.Collectors.toSet()); + Set linkedRowIds = Arrays.stream(linkedLsidRowIds).map(m -> ((Integer)m.get("RowId")).longValue()).collect(java.util.stream.Collectors.toSet()); + 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); } + + StudyPublishService.get().addRecallAuditEvent(sampleContainer, user, dataset, linkedLsids.size(), linkedRowIds); + dataset.deleteDatasetRows(user, linkedLsids); } } } diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 8aaf36c2fe8..841d722430c 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -1520,7 +1520,7 @@ public String checkForLockedLinks(Dataset def, @Nullable List rowIds) } @Override - public void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection> pairs) + public void addRecallAuditEvent(Container sourceContainer, User user, Dataset def, int rowCount, @Nullable Collection 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 eventMetadata = new HashMap<>(); eventMetadata.put(SAMPLE_TIMELINE_EVENT_TYPE, timelineEventType.name()); String metadata = AbstractAuditTypeProvider.encodeForDataMap(eventMetadata); - List sampleIds = pairs.stream().map(Pair::getValue).collect(toList()); - List samples = ExperimentService.get().getExpMaterials(sampleIds); + List samples = ExperimentService.get().getExpMaterials(rowIds); List events = new ArrayList<>(samples.size()); for (ExpMaterial sample : samples) { diff --git a/study/src/org/labkey/study/controllers/StudyController.java b/study/src/org/labkey/study/controllers/StudyController.java index 83b02c438fe..a4674a3a571 100644 --- a/study/src/org/labkey/study/controllers/StudyController.java +++ b/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> pairs = entry.getValue(); + Collection 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); } }