Merge pull request #16879 from davelopez/23.1_fix_delete_collection

[23.1] Fix delete collection + elements

client/src/components/History/Content/ContentItem.vue

@@ -63,7 +63,7 @@
                     :state="state"
                     :item-urls="itemUrls"
                     :keyboard-selectable="expandDataset"
-                    @delete="$emit('delete')"
+                    @delete="onDelete"
                     @display="onDisplay"
                     @showCollectionInfo="onShowCollectionInfo"
                     @edit="onEdit"
@@ -234,6 +234,9 @@ export default {
                 this.$router.push(this.itemUrls.display, { title: this.name });
             }
         },
+        onDelete(recursive = false) {
+            this.$emit("delete", this.item, recursive);
+        },
         onDragStart(evt) {
             setDrag(evt, this.item);
         },

client/src/components/History/Content/ContentOptions.vue

@@ -1,5 +1,5 @@
 <template>
-    <span class="align-self-start btn-group">
+    <span class="align-self-start btn-group align-items-baseline">
         <!-- Special case for collections -->
         <b-button
             v-if="isCollection && canShowCollectionDetails"
@@ -43,8 +43,21 @@
             title="Delete"
             size="sm"
             variant="link"
-            @click.stop="$emit('delete')">
-            <icon icon="trash" />
+            @click.stop="onDelete($event)">
+            <icon v-if="isDataset" icon="trash" />
+            <b-dropdown v-else ref="deleteCollectionMenu" size="sm" variant="link" no-caret toggle-class="p-0 m-0">
+                <template v-slot:button-content>
+                    <icon icon="trash" />
+                </template>
+                <b-dropdown-item title="Delete collection only" @click.prevent.stop="onDeleteItem">
+                    <icon icon="file" />
+                    Collection only
+                </b-dropdown-item>
+                <b-dropdown-item title="Delete collection and elements" @click.prevent.stop="onDeleteItemRecursively">
+                    <icon icon="copy" />
+                    Collection and elements
+                </b-dropdown-item>
+            </b-dropdown>
         </b-button>
         <b-button
             v-if="writable && isHistoryItem && isDeleted"
@@ -130,6 +143,26 @@ export default {
                 this.$emit("display");
             }
         },
+        onDelete() {
+            if (this.isCollection) {
+                this.$refs.deleteCollectionMenu.show();
+            } else {
+                this.onDeleteItem();
+            }
+        },
+        onDeleteItem() {
+            this.$emit("delete");
+        },
+        onDeleteItemRecursively() {
+            const recursive = true;
+            this.$emit("delete", recursive);
+        },
     },
 };
 </script>
+<style lang="css">
+.dropdown-menu .dropdown-item {
+    font-weight: normal;
+}
+</style>
+```

client/src/components/History/Content/GenericItem.vue

@@ -12,7 +12,7 @@
                 :is-dataset="item.history_content_type == 'dataset' || item.element_type == 'hda'"
                 @update:expand-dataset="expandDataset = $event"
                 @view-collection="viewCollection = !viewCollection"
-                @delete="onDelete(item)"
+                @delete="onDelete"
                 @toggleHighlights="onHighlight(item)"
                 @undelete="onUndelete(item)"
                 @unhide="onUnhide(item)" />
@@ -74,8 +74,8 @@ export default {
     },
     methods: {
         ...mapActions(useHistoryStore, ["applyFilters"]),
-        onDelete(item) {
-            deleteContent(item);
+        onDelete(item, recursive = false) {
+            deleteContent(item, { recursive: recursive });
         },
         onUndelete(item) {
             updateContentFields(item, { deleted: false });

client/src/components/History/CurrentHistory/HistoryPanel.vue

@@ -127,7 +127,7 @@
                                     @update:expand-dataset="setExpanded(item, $event)"
                                     @update:selected="setSelected(item, $event)"
                                     @view-collection="$emit('view-collection', item, currentOffset)"
-                                    @delete="onDelete(item)"
+                                    @delete="onDelete"
                                     @undelete="onUndelete(item)"
                                     @unhide="onUnhide(item)" />
                             </template>
@@ -357,9 +357,9 @@ export default {
                 this.loading = false;
             }
         },
-        onDelete(item) {
+        onDelete(item, recursive = false) {
             this.setInvisible(item);
-            deleteContent(item);
+            deleteContent(item, { recursive: recursive });
         },
         onHideSelection(selectedItems) {
             selectedItems.forEach((item) => {

lib/galaxy/webapps/galaxy/services/history_contents.py

@@ -1384,7 +1384,7 @@ def __init__(
         self._operation_map: Dict[HistoryContentItemOperation, ItemOperation] = {
             HistoryContentItemOperation.hide: lambda item, params, trans: self._hide(item),
             HistoryContentItemOperation.unhide: lambda item, params, trans: self._unhide(item),
-            HistoryContentItemOperation.delete: lambda item, params, trans: self._delete(item),
+            HistoryContentItemOperation.delete: lambda item, params, trans: self._delete(item, trans),
             HistoryContentItemOperation.undelete: lambda item, params, trans: self._undelete(item),
             HistoryContentItemOperation.purge: lambda item, params, trans: self._purge(item, trans),
             HistoryContentItemOperation.change_datatype: lambda item, params, trans: self._change_datatype(
@@ -1415,9 +1415,10 @@ def _hide(self, item: HistoryItemModel):
     def _unhide(self, item: HistoryItemModel):
         item.visible = True
 
-    def _delete(self, item: HistoryItemModel):
-        manager = self._get_item_manager(item)
-        manager.delete(item, flush=self.flush)
+    def _delete(self, item: HistoryItemModel, trans: ProvidesHistoryContext):
+        if isinstance(item, HistoryDatasetCollectionAssociation):
+            return self.dataset_collection_manager.delete(trans, "history", item.id, recursive=True, purge=False)
+        return self.hda_manager.delete(item, flush=self.flush)
 
     def _undelete(self, item: HistoryItemModel):
         if getattr(item, "purged", False):

lib/galaxy_test/api/test_history_contents.py

@@ -1026,6 +1026,41 @@ def test_purging_collection_should_purge_contents(self):
                 if item["history_content_type"] == "dataset":
                     self.dataset_populator.wait_for_purge(history_id=history_id, content_id=item["id"])
 
+    def test_deleting_collection_should_delete_contents(self):
+        with self.dataset_populator.test_history() as history_id:
+            num_expected_datasets = 2
+            # Create collection and datasets
+            collection_ids = self._create_collection_in_history(history_id, num_collections=1)
+            original_collection_id = collection_ids[0]
+            # Check datasets are hidden and not deleted
+            history_contents = self._get_history_contents(history_id)
+            datasets = list(filter(lambda item: item["history_content_type"] == "dataset", history_contents))
+            assert len(datasets) == num_expected_datasets
+            for dataset in datasets:
+                assert dataset["deleted"] is False
+                assert dataset["visible"] is False
+
+            # Delete the collection
+            payload = {
+                "operation": "delete",
+                "items": [
+                    {
+                        "id": original_collection_id,
+                        "history_content_type": "dataset_collection",
+                    },
+                ],
+            }
+            bulk_operation_result = self._apply_bulk_operation(history_id, payload)
+            self._assert_bulk_success(bulk_operation_result, 1)
+
+            # We expect the original collection and the datasets to be deleted
+            num_expected_history_contents = num_expected_datasets + 1
+
+            history_contents = self._get_history_contents(history_id)
+            assert len(history_contents) == num_expected_history_contents
+            for item in history_contents:
+                assert item["deleted"] is True
+
     @requires_new_user
     def test_only_owner_can_apply_bulk_operations(self):
         with self.dataset_populator.test_history() as history_id: