From 11fc6cc669e7cd0a36d1cb0e6c9b44d7d8f84b77 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 27 Sep 2023 17:04:25 +0200 Subject: [PATCH 1/6] Add fetchCollectionAttributes to services --- client/src/stores/services/datasetCollection.service.ts | 9 ++++++++- client/src/stores/services/index.ts | 2 ++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/client/src/stores/services/datasetCollection.service.ts b/client/src/stores/services/datasetCollection.service.ts index 5eca0c4f861e..c52aabfc3350 100644 --- a/client/src/stores/services/datasetCollection.service.ts +++ b/client/src/stores/services/datasetCollection.service.ts @@ -1,6 +1,6 @@ import { fetcher } from "@/schema"; -import { CollectionEntry, DCESummary, HDCADetailed, isHDCA } from "."; +import { CollectionEntry, DatasetCollectionAttributes, DCESummary, HDCADetailed, isHDCA } from "."; const DEFAULT_LIMIT = 50; @@ -53,3 +53,10 @@ export async function fetchElementsFromCollection(params: { limit: params.limit ?? DEFAULT_LIMIT, }); } + +const getCollectionAttributes = fetcher.path("/api/dataset_collections/{id}/attributes").method("get").create(); + +export async function fetchCollectionAttributes(params: { hdcaId: string }): Promise { + const { data } = await getCollectionAttributes({ id: params.hdcaId, instance_type: "history" }); + return data; +} diff --git a/client/src/stores/services/index.ts b/client/src/stores/services/index.ts index 2efb3132897e..5cea4cd8e8da 100644 --- a/client/src/stores/services/index.ts +++ b/client/src/stores/services/index.ts @@ -58,6 +58,8 @@ export type HDCADetailed = components["schemas"]["HDCADetailed"]; */ export type DCObject = components["schemas"]["DCObject"]; +export type DatasetCollectionAttributes = components["schemas"]["DatasetCollectionAttributesResult"]; + /** * A SubCollection is a DatasetCollectionElement of type `dataset_collection` * with additional information to simplify its handling. From 4c4963a80a3f958465523ecc7d6c9a8aa75ac7e3 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 27 Sep 2023 17:05:17 +0200 Subject: [PATCH 2/6] Add collection attributes Pinia store --- .../src/stores/collectionAttributesStore.ts | 43 +++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 client/src/stores/collectionAttributesStore.ts diff --git a/client/src/stores/collectionAttributesStore.ts b/client/src/stores/collectionAttributesStore.ts new file mode 100644 index 000000000000..dd2c86265690 --- /dev/null +++ b/client/src/stores/collectionAttributesStore.ts @@ -0,0 +1,43 @@ +import { defineStore } from "pinia"; +import Vue, { computed, ref } from "vue"; + +import { DatasetCollectionAttributes } from "./services"; +import * as Service from "./services/datasetCollection.service"; + +export const useCollectionAttributesStore = defineStore("collectionAttributesStore", () => { + const storedAttributes = ref<{ [key: string]: DatasetCollectionAttributes }>({}); + const loadingAttributes = ref<{ [key: string]: boolean }>({}); + + const getAttributes = computed(() => { + return (hdcaId: string) => { + if (!storedAttributes.value[hdcaId]) { + Vue.set(storedAttributes.value, hdcaId, {}); + fetchAttributes({ hdcaId }); + } + return storedAttributes.value[hdcaId]; + }; + }); + + const isLoadingAttributes = computed(() => { + return (hdcaId: string) => { + return loadingAttributes.value[hdcaId] ?? false; + }; + }); + + async function fetchAttributes(params: { hdcaId: string }) { + Vue.set(loadingAttributes.value, params.hdcaId, true); + try { + const attributes = await Service.fetchCollectionAttributes(params); + Vue.set(storedAttributes.value, params.hdcaId, attributes); + return attributes; + } finally { + Vue.delete(loadingAttributes.value, params.hdcaId); + } + } + + return { + storedAttributes, + getAttributes, + isLoadingAttributes, + }; +}); From 767e1747bb971c2e23c8b8d50d8f8edc8d8a3a81 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 27 Sep 2023 17:37:05 +0200 Subject: [PATCH 3/6] Add unit tests for collectionAttributesStore --- .../stores/collectionAttributesStore.test.ts | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) create mode 100644 client/src/stores/collectionAttributesStore.test.ts diff --git a/client/src/stores/collectionAttributesStore.test.ts b/client/src/stores/collectionAttributesStore.test.ts new file mode 100644 index 000000000000..5afcfccbe812 --- /dev/null +++ b/client/src/stores/collectionAttributesStore.test.ts @@ -0,0 +1,55 @@ +import flushPromises from "flush-promises"; +import { createPinia, setActivePinia } from "pinia"; + +import { mockFetcher } from "@/schema/__mocks__"; +import { DatasetCollectionAttributes } from "@/stores/services"; + +import { useCollectionAttributesStore } from "./collectionAttributesStore"; + +jest.mock("@/schema"); + +const FAKE_HDCA_ID = "123"; + +const FAKE_ATTRIBUTES: DatasetCollectionAttributes = { + dbkey: "hg19", + extension: "bed", + model_class: "HistoryDatasetCollectionAssociation", + dbkeys: ["hg19", "hg38"], + extensions: ["bed", "vcf"], + tags: ["tag1", "tag2"], +}; + +const fetchCollectionAttributes = jest.fn().mockResolvedValue({ data: FAKE_ATTRIBUTES }); + +describe("collectionAttributesStore", () => { + beforeEach(() => { + setActivePinia(createPinia()); + mockFetcher.path("/api/dataset_collections/{id}/attributes").method("get").mock(fetchCollectionAttributes); + }); + + it("should fetch attributes and store them", async () => { + const store = useCollectionAttributesStore(); + expect(store.storedAttributes[FAKE_HDCA_ID]).toBeUndefined(); + expect(store.isLoadingAttributes(FAKE_HDCA_ID)).toBeFalsy(); + + store.getAttributes(FAKE_HDCA_ID); + // getAttributes will trigger a fetch if the attributes are not stored + expect(store.isLoadingAttributes(FAKE_HDCA_ID)).toBeTruthy(); + await flushPromises(); + expect(store.isLoadingAttributes(FAKE_HDCA_ID)).toBeFalsy(); + + expect(store.storedAttributes[FAKE_HDCA_ID]).toEqual(FAKE_ATTRIBUTES); + expect(fetchCollectionAttributes).toHaveBeenCalled(); + }); + + it("should not fetch attributes if already stored", async () => { + const store = useCollectionAttributesStore(); + + store.storedAttributes[FAKE_HDCA_ID] = FAKE_ATTRIBUTES; + + const result = store.getAttributes(FAKE_HDCA_ID); + + expect(result).toEqual(FAKE_ATTRIBUTES); + expect(fetchCollectionAttributes).not.toHaveBeenCalled(); + }); +}); From 5bbacf92acc5b883ae0c1dd5bc73c74129caad1c Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 27 Sep 2023 17:38:12 +0200 Subject: [PATCH 4/6] Use new Pinia store in CollectionEditView --- .../Collections/common/CollectionEditView.vue | 17 +++++------------ 1 file changed, 5 insertions(+), 12 deletions(-) diff --git a/client/src/components/Collections/common/CollectionEditView.vue b/client/src/components/Collections/common/CollectionEditView.vue index d48be17d7e83..3d43da30a30b 100644 --- a/client/src/components/Collections/common/CollectionEditView.vue +++ b/client/src/components/Collections/common/CollectionEditView.vue @@ -66,6 +66,7 @@ import { errorMessageAsString } from "utils/simple-error"; import Vue from "vue"; import { useConfig } from "@/composables/config"; +import { useCollectionAttributesStore } from "@/stores/collectionAttributesStore"; import { useHistoryStore } from "@/stores/historyStore"; import { DatatypesProvider, DbKeyProvider, SuitableConvertersProvider } from "../../providers"; @@ -101,7 +102,6 @@ export default { }, data: function () { return { - attributesData: {}, errorMessage: null, jobError: null, noQuotaIncrease: true, @@ -114,6 +114,10 @@ export default { }, computed: { ...mapState(useHistoryStore, ["currentHistoryId"]), + ...mapState(useCollectionAttributesStore, ["getAttributes"]), + attributesData() { + return this.getAttributes(this.collectionId); + }, databaseKeyFromElements: function () { return this.attributesData.dbkey; }, @@ -121,21 +125,10 @@ export default { return this.attributesData.extension; }, }, - created() { - this.getCollectionDataAndAttributes(); - }, methods: { updateInfoMessage: function (strMessage) { this.infoMessage = strMessage; }, - getCollectionDataAndAttributes: async function () { - let attributesGet = this.$store.getters.getCollectionAttributes(this.collectionId); - if (attributesGet == null) { - await this.$store.dispatch("fetchCollectionAttributes", this.collectionId); - attributesGet = this.$store.getters.getCollectionAttributes(this.collectionId); - } - this.attributesData = attributesGet; - }, clickedSave: function (attribute, newValue) { const url = prependPath(`/api/dataset_collections/${this.collectionId}/copy`); const data = {}; From cb0f62e614b0b4c384f1e1c79e09b5cafa943871 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Wed, 27 Sep 2023 17:46:10 +0200 Subject: [PATCH 5/6] Drop old unused Vuex store --- client/src/store/collectionAttributesStore.js | 33 ------------------- client/src/store/index.js | 2 -- 2 files changed, 35 deletions(-) delete mode 100644 client/src/store/collectionAttributesStore.js diff --git a/client/src/store/collectionAttributesStore.js b/client/src/store/collectionAttributesStore.js deleted file mode 100644 index e80c5c2bc632..000000000000 --- a/client/src/store/collectionAttributesStore.js +++ /dev/null @@ -1,33 +0,0 @@ -import axios from "axios"; -import { prependPath } from "utils/redirect"; -import Vue from "vue"; - -export const state = { - collectionAttributes: {}, -}; - -const getters = { - getCollectionAttributes: (state) => (collectionId) => { - return state.collectionAttributes[collectionId] || null; - }, -}; - -const actions = { - fetchCollectionAttributes: async ({ commit }, collectionId) => { - const { data } = await axios.get(prependPath("api/dataset_collections/" + collectionId + "/attributes")); - commit("saveCollectionAttributes", { collectionId, collectionAttributes: data }); - }, -}; - -const mutations = { - saveCollectionAttributes: (state, { collectionId, collectionAttributes }) => { - Vue.set(state.collectionAttributes, collectionId, collectionAttributes); - }, -}; - -export const collectionAttributesStore = { - state, - getters, - actions, - mutations, -}; diff --git a/client/src/store/index.js b/client/src/store/index.js index 19e2b1ed020e..6757f756d9d8 100644 --- a/client/src/store/index.js +++ b/client/src/store/index.js @@ -9,7 +9,6 @@ import Vuex from "vuex"; import createCache from "vuex-cache"; import VuexPersistence from "vuex-persist"; -import { collectionAttributesStore } from "./collectionAttributesStore"; import { datasetExtFilesStore } from "./datasetExtFilesStore"; import { datasetPathDestinationStore } from "./datasetPathDestinationStore"; import { gridSearchStore } from "./gridSearchStore"; @@ -45,7 +44,6 @@ export function createStore() { const storeConfig = { plugins: [createCache(), panelsPersistence.plugin], modules: { - collectionAttributesStore: collectionAttributesStore, destinationParameters: jobDestinationParametersStore, datasetExtFiles: datasetExtFilesStore, datasetPathDestination: datasetPathDestinationStore, From c4529f75dfc3bda7c50777ff06bf069f4c154568 Mon Sep 17 00:00:00 2001 From: davelopez <46503462+davelopez@users.noreply.github.com> Date: Thu, 28 Sep 2023 09:31:32 +0200 Subject: [PATCH 6/6] Improve imports from code review Co-authored-by: Laila Los <44241786+ElectronicBlueberry@users.noreply.github.com> --- client/src/stores/collectionAttributesStore.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/client/src/stores/collectionAttributesStore.ts b/client/src/stores/collectionAttributesStore.ts index dd2c86265690..96b31d0c03da 100644 --- a/client/src/stores/collectionAttributesStore.ts +++ b/client/src/stores/collectionAttributesStore.ts @@ -1,8 +1,8 @@ import { defineStore } from "pinia"; -import Vue, { computed, ref } from "vue"; +import { computed, del, ref, set } from "vue"; import { DatasetCollectionAttributes } from "./services"; -import * as Service from "./services/datasetCollection.service"; +import { fetchCollectionAttributes } from "./services/datasetCollection.service"; export const useCollectionAttributesStore = defineStore("collectionAttributesStore", () => { const storedAttributes = ref<{ [key: string]: DatasetCollectionAttributes }>({}); @@ -11,7 +11,7 @@ export const useCollectionAttributesStore = defineStore("collectionAttributesSto const getAttributes = computed(() => { return (hdcaId: string) => { if (!storedAttributes.value[hdcaId]) { - Vue.set(storedAttributes.value, hdcaId, {}); + set(storedAttributes.value, hdcaId, {}); fetchAttributes({ hdcaId }); } return storedAttributes.value[hdcaId]; @@ -25,13 +25,13 @@ export const useCollectionAttributesStore = defineStore("collectionAttributesSto }); async function fetchAttributes(params: { hdcaId: string }) { - Vue.set(loadingAttributes.value, params.hdcaId, true); + set(loadingAttributes.value, params.hdcaId, true); try { - const attributes = await Service.fetchCollectionAttributes(params); - Vue.set(storedAttributes.value, params.hdcaId, attributes); + const attributes = await fetchCollectionAttributes(params); + set(storedAttributes.value, params.hdcaId, attributes); return attributes; } finally { - Vue.delete(loadingAttributes.value, params.hdcaId); + del(loadingAttributes.value, params.hdcaId); } }