From eccb0c8e9d59363d068e47f6b5938bd89bf20bb5 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 20 Apr 2022 13:07:48 +0200 Subject: [PATCH 1/6] Move `elements_datatypes` to summary view Since the new history doesn't get the detailed collection when retrieving contents I had to move this serializable property to the summary view --- lib/galaxy/managers/hdcas.py | 2 +- lib/galaxy/schema/schema.py | 7 +++---- lib/galaxy_test/api/test_history_contents.py | 4 +--- 3 files changed, 5 insertions(+), 8 deletions(-) diff --git a/lib/galaxy/managers/hdcas.py b/lib/galaxy/managers/hdcas.py index d30663009d2..7981f02bab8 100644 --- a/lib/galaxy/managers/hdcas.py +++ b/lib/galaxy/managers/hdcas.py @@ -281,6 +281,7 @@ class HDCASerializer(DCASerializer, taggable.TaggableSerializerMixin, annotatabl "update_time", "tags", "contents_url", + "elements_datatypes", ], ) self.add_view( @@ -288,7 +289,6 @@ class HDCASerializer(DCASerializer, taggable.TaggableSerializerMixin, annotatabl [ "populated", "elements", - "elements_datatypes", ], include_keys_from="summary", ) diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index c58d0b93569..51d4bbf70c4 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -9,7 +9,6 @@ from typing import ( Dict, List, Optional, - Set, Union, ) @@ -763,6 +762,9 @@ class HDCASummary(HistoryItemCommon): title="Collection ID", description="The encoded ID of the dataset collection associated with this HDCA.", ) + elements_datatypes: List[str] = Field( + ..., description="A set containing all the different element datatypes in the collection." + ) class HDCADetailed(HDCASummary): @@ -770,9 +772,6 @@ class HDCADetailed(HDCASummary): populated: bool = PopulatedField elements: List[DCESummary] = ElementsField - elements_datatypes: Set[str] = Field( - ..., description="A set containing all the different element datatypes in the collection." - ) class HistoryBase(BaseModel): diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index 511aa61acfa..135dc5dbb1a 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -835,9 +835,7 @@ class HistoryContentsApiTestCase(ApiTestCase): self._assert_status_code_is_ok(create_homogeneous_response) def _assert_collection_has_expected_elements_datatypes(self, history_id, collection_name, expected_datatypes): - contents_response = self._get( - f"histories/{history_id}/contents?v=dev&view=detailed&q=name-eq&qv={collection_name}" - ) + contents_response = self._get(f"histories/{history_id}/contents?v=dev&q=name-eq&qv={collection_name}") self._assert_status_code_is(contents_response, 200) collection = contents_response.json()[0] self.assertCountEqual(collection["elements_datatypes"], expected_datatypes) From 222764e5f5807c7b933684bd23475511666ce668 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 20 Apr 2022 13:09:01 +0200 Subject: [PATCH 2/6] Display homogeneous collection types on description --- .../Collection/CollectionDescription.test.js | 49 ++++++++++++++----- .../Collection/CollectionDescription.vue | 40 ++++++++++++--- .../History/Content/ContentItem.vue | 3 +- .../CurrentCollection/CollectionDetails.vue | 5 +- 4 files changed, 78 insertions(+), 19 deletions(-) diff --git a/client/src/components/History/Content/Collection/CollectionDescription.test.js b/client/src/components/History/Content/Collection/CollectionDescription.test.js index 50c9f7729b8..a55b5d4f04a 100644 --- a/client/src/components/History/Content/Collection/CollectionDescription.test.js +++ b/client/src/components/History/Content/Collection/CollectionDescription.test.js @@ -11,23 +11,50 @@ describe("CollectionDescription", () => { wrapper = mount(CollectionDescription, { propsData: { collectionType: "list", - elementCount: 10, }, localVue, }); }); - it("check basics", async () => { - const details = wrapper.findAll("span"); - expect(details.at(0).text()).toBe("a list"); - expect(details.at(1).text()).toBe("with 10 items"); - await wrapper.setProps({ elementCount: 1 }); - expect(details.at(1).text()).toBe("with 1 item"); - await wrapper.setProps({ collectionType: "paired" }); - expect(details.at(0).text()).toBe("a dataset pair"); + it("should display expected heterogeneous descriptions", async () => { + const HETEROGENEOUS_DATATYPES = ["txt", "csv", "tabular"]; + await wrapper.setProps({ elementCount: 1, elementsDatatypes: HETEROGENEOUS_DATATYPES }); + expect(wrapper.text()).toBe("a list with 1 dataset"); + + await wrapper.setProps({ elementCount: 2, collectionType: "paired" }); + expect(wrapper.text()).toBe("a pair with 2 datasets"); + + await wrapper.setProps({ elementCount: 10, collectionType: "list" }); + expect(wrapper.text()).toBe("a list with 10 datasets"); + await wrapper.setProps({ collectionType: "list:paired" }); - expect(details.at(0).text()).toBe("a list of pairs"); + expect(wrapper.text()).toBe("a list with 10 pairs"); + + await wrapper.setProps({ collectionType: "list:list" }); + expect(wrapper.text()).toBe("a list with 10 lists"); + await wrapper.setProps({ collectionType: "other" }); - expect(details.at(0).text()).toBe("a nested list"); + expect(wrapper.text()).toBe("a nested list with 10 datasets"); + }); + + it("should display expected homogeneous descriptions", async () => { + const EXPECTED_HOMOGENEOUS_DATATYPE = "tabular"; + await wrapper.setProps({ elementCount: 1, elementsDatatypes: [EXPECTED_HOMOGENEOUS_DATATYPE] }); + expect(wrapper.text()).toBe(`a list with 1 ${EXPECTED_HOMOGENEOUS_DATATYPE} dataset`); + + await wrapper.setProps({ elementCount: 2, collectionType: "paired" }); + expect(wrapper.text()).toBe(`a pair with 2 ${EXPECTED_HOMOGENEOUS_DATATYPE} datasets`); + + await wrapper.setProps({ elementCount: 10, collectionType: "list" }); + expect(wrapper.text()).toBe(`a list with 10 ${EXPECTED_HOMOGENEOUS_DATATYPE} datasets`); + + await wrapper.setProps({ collectionType: "list:paired" }); + expect(wrapper.text()).toBe(`a list with 10 ${EXPECTED_HOMOGENEOUS_DATATYPE} pairs`); + + await wrapper.setProps({ collectionType: "list:list" }); + expect(wrapper.text()).toBe(`a list with 10 ${EXPECTED_HOMOGENEOUS_DATATYPE} lists`); + + await wrapper.setProps({ collectionType: "other" }); + expect(wrapper.text()).toBe(`a nested list with 10 ${EXPECTED_HOMOGENEOUS_DATATYPE} datasets`); }); }); diff --git a/client/src/components/History/Content/Collection/CollectionDescription.vue b/client/src/components/History/Content/Collection/CollectionDescription.vue index b1cfcc3e732..fe328e34ea6 100644 --- a/client/src/components/History/Content/Collection/CollectionDescription.vue +++ b/client/src/components/History/Content/Collection/CollectionDescription.vue @@ -1,8 +1,6 @@ @@ -10,21 +8,51 @@ export default { props: { collectionType: { type: String, required: true }, - elementCount: { type: Number, required: true }, + elementCount: { type: Number, required: false, default: undefined }, + elementsDatatypes: { type: Array, required: false, default: () => [] }, }, data() { return { labels: { list: "list", - paired: "dataset pair", - "list:paired": "list of pairs", + "list:paired": "list", + "list:list": "list", + paired: "pair", }, }; }, computed: { + /**@return {String} */ collectionLabel() { return this.labels[this.collectionType] || "nested list"; }, + /**@return {Boolean} */ + hasSingleElement() { + return this.elementCount === 1; + }, + /**@return {Boolean} */ + isHomogeneous() { + return this.elementsDatatypes.length === 1; + }, + /**@return {String} */ + homogeneousDatatype() { + return this.isHomogeneous ? ` ${this.elementsDatatypes[0]}` : ""; + }, + /**@return {String} */ + pluralizedItem() { + if (this.collectionType === "list:list") { + return this.pluralize("list"); + } + if (this.collectionType === "list:paired") { + return this.pluralize("pair"); + } + return this.pluralize("dataset"); + }, + }, + methods: { + pluralize(word) { + return this.hasSingleElement ? word : `${word}s`; + }, }, }; diff --git a/client/src/components/History/Content/ContentItem.vue b/client/src/components/History/Content/ContentItem.vue index 52ef4928ffe..640637a45aa 100644 --- a/client/src/components/History/Content/ContentItem.vue +++ b/client/src/components/History/Content/ContentItem.vue @@ -32,7 +32,8 @@ + :element-count="item.element_count" + :elements-datatypes="item.elements_datatypes" />
diff --git a/client/src/components/History/CurrentCollection/CollectionDetails.vue b/client/src/components/History/CurrentCollection/CollectionDetails.vue index a8cf7df5363..f09ec9d7d3c 100644 --- a/client/src/components/History/CurrentCollection/CollectionDetails.vue +++ b/client/src/components/History/CurrentCollection/CollectionDetails.vue @@ -7,7 +7,10 @@ @save="$emit('update:dsc', $event)"> From 7e6b6d730a721f8e09a5ebf17a065af9df26ffb3 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 20 Apr 2022 13:10:41 +0200 Subject: [PATCH 3/6] Remove unused JSON test data --- .../json/DatasetCollection.heterogeneous.json | 40 ------------------- .../json/DatasetCollection.homogeneous.json | 40 ------------------- 2 files changed, 80 deletions(-) delete mode 100644 client/src/components/providers/test/json/DatasetCollection.heterogeneous.json delete mode 100644 client/src/components/providers/test/json/DatasetCollection.homogeneous.json diff --git a/client/src/components/providers/test/json/DatasetCollection.heterogeneous.json b/client/src/components/providers/test/json/DatasetCollection.heterogeneous.json deleted file mode 100644 index 0aad7ebd8af..00000000000 --- a/client/src/components/providers/test/json/DatasetCollection.heterogeneous.json +++ /dev/null @@ -1,40 +0,0 @@ -{ - "job_source_id": null, - "populated_state": "ok", - "type": "collection", - "hid": 9, - "name": "Heterogeneous list", - "url": "/api/histories/4ff6f47412c3e65e/contents/dataset_collections/5a1cff6882ddb5b2", - "element_count": 4, - "job_source_type": null, - "history_id": "4ff6f47412c3e65e", - "id": "5a1cff6882ddb5b2", - "type_id": "dataset_collection-5a1cff6882ddb5b2", - "create_time": "2020-06-26T14:22:58.435340", - "collection_type": "list", - "history_content_type": "dataset_collection", - "populated_state_message": null, - "job_state_summary": { - "error": 0, - "upload": 0, - "queued": 0, - "all_jobs": 0, - "paused": 0, - "ok": 0, - "new": 0, - "waiting": 0, - "failed": 0, - "deleted": 0, - "resubmitted": 0, - "running": 0, - "deleted_new": 0 - }, - "contents_url": "/api/dataset_collections/5a1cff6882ddb5b2/contents/f09437b8822035f7", - "tags": [], - "visible": true, - "deleted": false, - "populated": true, - "update_time": "2020-06-26T14:22:58.435348", - "collection_id": 23, - "elements_datatypes": ["txt", "csv", "tabular"] -} diff --git a/client/src/components/providers/test/json/DatasetCollection.homogeneous.json b/client/src/components/providers/test/json/DatasetCollection.homogeneous.json deleted file mode 100644 index fcb738124be..00000000000 --- a/client/src/components/providers/test/json/DatasetCollection.homogeneous.json +++ /dev/null @@ -1,40 +0,0 @@ -{ - "job_source_id": null, - "populated_state": "ok", - "type": "collection", - "hid": 9, - "name": "Homogeneous list", - "url": "/api/histories/4ff6f47412c3e65e/contents/dataset_collections/5a1cff6882ddb5b2", - "element_count": 4, - "job_source_type": null, - "history_id": "4ff6f47412c3e65e", - "id": "5a1cff6882ddb5b2", - "type_id": "dataset_collection-5a1cff6882ddb5b2", - "create_time": "2020-06-26T14:22:58.435340", - "collection_type": "list", - "history_content_type": "dataset_collection", - "populated_state_message": null, - "job_state_summary": { - "error": 0, - "upload": 0, - "queued": 0, - "all_jobs": 0, - "paused": 0, - "ok": 0, - "new": 0, - "waiting": 0, - "failed": 0, - "deleted": 0, - "resubmitted": 0, - "running": 0, - "deleted_new": 0 - }, - "contents_url": "/api/dataset_collections/5a1cff6882ddb5b2/contents/f09437b8822035f7", - "tags": [], - "visible": true, - "deleted": false, - "populated": true, - "update_time": "2020-06-26T14:22:58.435348", - "collection_id": 23, - "elements_datatypes": ["txt"] -} From 1f61c8d7a6a9a9f1be6096581c51e06486fd8d66 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 20 Apr 2022 16:30:46 +0200 Subject: [PATCH 4/6] Consider any other collection_type as nested collection In the collection description --- .../History/Content/Collection/CollectionDescription.test.js | 4 ++-- .../History/Content/Collection/CollectionDescription.vue | 4 ++++ 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/client/src/components/History/Content/Collection/CollectionDescription.test.js b/client/src/components/History/Content/Collection/CollectionDescription.test.js index a55b5d4f04a..9a485b7ea1e 100644 --- a/client/src/components/History/Content/Collection/CollectionDescription.test.js +++ b/client/src/components/History/Content/Collection/CollectionDescription.test.js @@ -34,7 +34,7 @@ describe("CollectionDescription", () => { expect(wrapper.text()).toBe("a list with 10 lists"); await wrapper.setProps({ collectionType: "other" }); - expect(wrapper.text()).toBe("a nested list with 10 datasets"); + expect(wrapper.text()).toBe("a nested list with 10 dataset collections"); }); it("should display expected homogeneous descriptions", async () => { @@ -55,6 +55,6 @@ describe("CollectionDescription", () => { expect(wrapper.text()).toBe(`a list with 10 ${EXPECTED_HOMOGENEOUS_DATATYPE} lists`); await wrapper.setProps({ collectionType: "other" }); - expect(wrapper.text()).toBe(`a nested list with 10 ${EXPECTED_HOMOGENEOUS_DATATYPE} datasets`); + expect(wrapper.text()).toBe(`a nested list with 10 ${EXPECTED_HOMOGENEOUS_DATATYPE} dataset collections`); }); }); diff --git a/client/src/components/History/Content/Collection/CollectionDescription.vue b/client/src/components/History/Content/Collection/CollectionDescription.vue index fe328e34ea6..faa2fd5947a 100644 --- a/client/src/components/History/Content/Collection/CollectionDescription.vue +++ b/client/src/components/History/Content/Collection/CollectionDescription.vue @@ -46,6 +46,10 @@ export default { if (this.collectionType === "list:paired") { return this.pluralize("pair"); } + if (!Object.keys(this.labels).includes(this.collectionType)) { + //Any other kind of nested collection + return this.pluralize("dataset collection"); + } return this.pluralize("dataset"); }, }, From d019bc9a8896f6d54ec5a0ae05e6b80234f1118d Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Apr 2022 12:13:29 +0200 Subject: [PATCH 5/6] Revert "Move `elements_datatypes` to summary view" This reverts commit eccb0c8e9d59363d068e47f6b5938bd89bf20bb5. --- lib/galaxy/managers/hdcas.py | 2 +- lib/galaxy/schema/schema.py | 7 ++++--- lib/galaxy_test/api/test_history_contents.py | 4 +++- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/lib/galaxy/managers/hdcas.py b/lib/galaxy/managers/hdcas.py index 7981f02bab8..d30663009d2 100644 --- a/lib/galaxy/managers/hdcas.py +++ b/lib/galaxy/managers/hdcas.py @@ -281,7 +281,6 @@ class HDCASerializer(DCASerializer, taggable.TaggableSerializerMixin, annotatabl "update_time", "tags", "contents_url", - "elements_datatypes", ], ) self.add_view( @@ -289,6 +288,7 @@ class HDCASerializer(DCASerializer, taggable.TaggableSerializerMixin, annotatabl [ "populated", "elements", + "elements_datatypes", ], include_keys_from="summary", ) diff --git a/lib/galaxy/schema/schema.py b/lib/galaxy/schema/schema.py index 51d4bbf70c4..c58d0b93569 100644 --- a/lib/galaxy/schema/schema.py +++ b/lib/galaxy/schema/schema.py @@ -9,6 +9,7 @@ from typing import ( Dict, List, Optional, + Set, Union, ) @@ -762,9 +763,6 @@ class HDCASummary(HistoryItemCommon): title="Collection ID", description="The encoded ID of the dataset collection associated with this HDCA.", ) - elements_datatypes: List[str] = Field( - ..., description="A set containing all the different element datatypes in the collection." - ) class HDCADetailed(HDCASummary): @@ -772,6 +770,9 @@ class HDCADetailed(HDCASummary): populated: bool = PopulatedField elements: List[DCESummary] = ElementsField + elements_datatypes: Set[str] = Field( + ..., description="A set containing all the different element datatypes in the collection." + ) class HistoryBase(BaseModel): diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index 135dc5dbb1a..511aa61acfa 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -835,7 +835,9 @@ class HistoryContentsApiTestCase(ApiTestCase): self._assert_status_code_is_ok(create_homogeneous_response) def _assert_collection_has_expected_elements_datatypes(self, history_id, collection_name, expected_datatypes): - contents_response = self._get(f"histories/{history_id}/contents?v=dev&q=name-eq&qv={collection_name}") + contents_response = self._get( + f"histories/{history_id}/contents?v=dev&view=detailed&q=name-eq&qv={collection_name}" + ) self._assert_status_code_is(contents_response, 200) collection = contents_response.json()[0] self.assertCountEqual(collection["elements_datatypes"], expected_datatypes) From 1beabe1b885ed1e4574290b08bd67007cebce10f Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Fri, 22 Apr 2022 13:13:58 +0200 Subject: [PATCH 6/6] Conditionally include `elements_datatypes` when serializing collections Only requests with a particular media type (used by the history UI) will include this additional field for dataset collections. --- .../webapps/galaxy/services/history_contents.py | 13 +++++++++++++ lib/galaxy_test/api/test_history_contents.py | 14 ++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/lib/galaxy/webapps/galaxy/services/history_contents.py b/lib/galaxy/webapps/galaxy/services/history_contents.py index b3b7c4ad7bf..fa528db14c8 100644 --- a/lib/galaxy/webapps/galaxy/services/history_contents.py +++ b/lib/galaxy/webapps/galaxy/services/history_contents.py @@ -971,6 +971,7 @@ class HistoriesContentsService(ServiceBase): f"Invalid filter found. When requesting stats, please avoid filtering by {non_orm_filter_keys}" ) + serialization_params = self._handle_extra_serialization_for_media_type(serialization_params, accept) filter_query_params.order = filter_query_params.order or "hid-asc" order_by = self.build_order_by(self.history_contents_manager, filter_query_params.order) contents = self.history_contents_manager.contents( @@ -999,6 +1000,18 @@ class HistoriesContentsService(ServiceBase): return HistoryContentsWithStatsResult.construct(contents=items, stats=stats) return HistoryContentsResult.construct(__root__=items) + def _handle_extra_serialization_for_media_type( + self, + serialization_params: SerializationParams, + request_media_type: str, + ) -> SerializationParams: + """According to the requested media type the response may include extra information.""" + if request_media_type == HistoryContentsWithStatsResult.__accept_type__: + if not serialization_params.keys: + serialization_params.keys = [] + serialization_params.keys.append("elements_datatypes") + return serialization_params + def _serialize_legacy_content_item( self, trans, diff --git a/lib/galaxy_test/api/test_history_contents.py b/lib/galaxy_test/api/test_history_contents.py index 511aa61acfa..848b046a061 100644 --- a/lib/galaxy_test/api/test_history_contents.py +++ b/lib/galaxy_test/api/test_history_contents.py @@ -1138,6 +1138,20 @@ class HistoryContentsApiBulkOperationTestCase(ApiTestCase): ) self._assert_status_code_is(response, 400) + def test_index_with_stats_has_extra_serialization(self): + expected_extra_keys_in_collections = ["elements_datatypes"] + with self.dataset_populator.test_history() as history_id: + self._create_collection_in_history(history_id) + response = self._get_contents_with_stats( + history_id, + search_query="&q=history_content_type-eq&qv=dataset_collection", + ) + self._assert_status_code_is(response, 200) + contents_with_stats = response.json() + assert contents_with_stats["contents"] + collection = contents_with_stats["contents"][0] + self._assert_has_keys(collection, *expected_extra_keys_in_collections) + def _get_contents_with_stats(self, history_id: str, search_query: str = ""): headers = {"accept": "application/vnd.galaxy.history.contents.stats+json"} search_response = self._get(f"histories/{history_id}/contents?v=dev{search_query}", headers=headers)