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..88408679f75 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; @@ -72,41 +75,55 @@ 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)) + Map> containerSamples = entry.getValue(); + + for (Map.Entry> containerEntry : containerSamples.entrySet()) { - throw new UnauthorizedException("Cannot delete rows from dataset " + dataset); - } + Container sampleContainer = containerEntry.getKey(); + List 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 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 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); } } } diff --git a/study/src/org/labkey/study/assay/StudyPublishManager.java b/study/src/org/labkey/study/assay/StudyPublishManager.java index 8a30d5c44e7..4e912b4ee61 100644 --- a/study/src/org/labkey/study/assay/StudyPublishManager.java +++ b/study/src/org/labkey/study/assay/StudyPublishManager.java @@ -1512,7 +1512,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) @@ -1533,15 +1533,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); } }