From 07088d591dab91fed384156731c945e3c3f94922 Mon Sep 17 00:00:00 2001
From: Olga Bulat <obulat@gmail.com>
Date: Fri, 15 Mar 2024 22:36:17 +0300
Subject: [PATCH] Fix bugs discovered during code review

Signed-off-by: Olga Bulat <obulat@gmail.com>
---
 frontend/src/composables/use-collection.ts  | 66 ------------------
 frontend/src/composables/use-search-type.ts |  9 ++-
 frontend/src/pages/audio/collection.vue     | 74 +++++++++++++++------
 frontend/src/pages/image/collection.vue     | 68 ++++++++++++++-----
 frontend/src/pages/search.vue               | 14 +---
 frontend/src/stores/media/index.ts          |  3 -
 6 files changed, 115 insertions(+), 119 deletions(-)
 delete mode 100644 frontend/src/composables/use-collection.ts

diff --git a/frontend/src/composables/use-collection.ts b/frontend/src/composables/use-collection.ts
deleted file mode 100644
index 4c832626144..00000000000
--- a/frontend/src/composables/use-collection.ts
+++ /dev/null
@@ -1,66 +0,0 @@
-import { computed, ref } from "vue"
-
-import { useMediaStore } from "~/stores/media"
-import { useSearchStore } from "~/stores/search"
-import { useI18n } from "~/composables/use-i18n"
-import type { DetailFromMediaType } from "~/types/media"
-import type { SupportedMediaType } from "~/constants/media"
-
-export const useCollection = <T extends SupportedMediaType>({
-  mediaType,
-}: {
-  mediaType: T
-}) => {
-  const mediaStore = useMediaStore()
-  const searchStore = useSearchStore()
-
-  const collectionParams = computed(() => searchStore.collectionParams)
-  const isFetching = computed(() => mediaStore.fetchState.isFetching)
-
-  const media = ref<DetailFromMediaType<typeof mediaType>[]>([])
-  const creatorUrl = ref<string>()
-
-  const i18n = useI18n()
-
-  const collectionLabel = computed(() => {
-    if (!collectionParams.value) {
-      return ""
-    }
-    const { collection, ...params } = collectionParams.value
-    return i18n
-      .t(`collection.label.${collection}.image`, { ...params })
-      .toString()
-  })
-
-  const fetchMedia = async (
-    { shouldPersistMedia }: { shouldPersistMedia: boolean } = {
-      shouldPersistMedia: false,
-    }
-  ) => {
-    if (mediaStore._searchType !== mediaType) {
-      throw new Error(
-        `Search type is incorrectly set in the store to ${mediaStore._searchType} when it should be "${mediaType}"`
-      )
-    }
-    media.value = (await mediaStore.fetchMedia({
-      shouldPersistMedia,
-    })) as (typeof media)["value"]
-    creatorUrl.value =
-      media.value.length > 0 ? media.value[0].creator_url : undefined
-  }
-
-  const handleLoadMore = async () => {
-    await fetchMedia({ shouldPersistMedia: true })
-  }
-
-  const results = computed(() => ({ type: mediaType, items: media.value }))
-
-  return {
-    results,
-    creatorUrl,
-    fetchMedia,
-    handleLoadMore,
-    collectionLabel,
-    isFetching,
-  }
-}
diff --git a/frontend/src/composables/use-search-type.ts b/frontend/src/composables/use-search-type.ts
index 970b888a0c5..4e0d4f218e2 100644
--- a/frontend/src/composables/use-search-type.ts
+++ b/frontend/src/composables/use-search-type.ts
@@ -11,7 +11,6 @@ import {
   SearchType,
 } from "~/constants/media"
 
-import { useMediaStore } from "~/stores/media"
 import { useSearchStore } from "~/stores/search"
 import { useFeatureFlagStore } from "~/stores/feature-flag"
 
@@ -65,8 +64,14 @@ export default function useSearchType() {
       next: searchType,
       component: componentName,
     })
+
+    // `setActiveType` is called after the search middleware
+    // ran and updated the search store state
+    // TODO: Figure out why
+    if (activeType.value === searchType) {
+      return
+    }
     useSearchStore().setSearchType(searchType)
-    useMediaStore().clearMedia()
     previousSearchType.value = searchType
   }
 
diff --git a/frontend/src/pages/audio/collection.vue b/frontend/src/pages/audio/collection.vue
index e5d6aed02f7..9c01fd0a59c 100644
--- a/frontend/src/pages/audio/collection.vue
+++ b/frontend/src/pages/audio/collection.vue
@@ -4,22 +4,29 @@
       v-if="collectionParams"
       search-term=""
       :is-fetching="isFetching"
-      :results="results"
+      :results="{ type: 'audio', items: media }"
       :collection-label="collectionLabel"
       :collection-params="collectionParams"
-      @load-more="handleLoadMore"
+      @load-more="loadMore"
     />
   </div>
 </template>
 
 <script lang="ts">
-import { defineComponent, useFetch, useMeta } from "@nuxtjs/composition-api"
-import { computed } from "vue"
+import {
+  defineComponent,
+  useFetch,
+  useMeta,
+  useRoute,
+} from "@nuxtjs/composition-api"
+import { computed, ref, watch } from "vue"
 
 import { collectionMiddleware } from "~/middleware/collection"
 import { useSearchStore } from "~/stores/search"
-import { useCollection } from "~/composables/use-collection"
-import { AUDIO } from "~/constants/media"
+
+import { useMediaStore } from "~/stores/media"
+import type { AudioDetail } from "~/types/media"
+import { useI18n } from "~/composables/use-i18n"
 
 import VCollectionResults from "~/components/VSearchResultsGrid/VCollectionResults.vue"
 
@@ -29,36 +36,63 @@ export default defineComponent({
   layout: "content-layout",
   middleware: collectionMiddleware,
   setup() {
+    const mediaStore = useMediaStore()
     const searchStore = useSearchStore()
 
     const collectionParams = computed(() => searchStore.collectionParams)
+    const isFetching = computed(() => mediaStore.fetchState.isFetching)
 
-    const {
-      results,
-      creatorUrl,
-      fetchMedia,
-      handleLoadMore,
-      collectionLabel,
-      isFetching,
-    } = useCollection({
-      mediaType: AUDIO,
+    const media = ref<AudioDetail[]>([])
+    const creatorUrl = ref<string>()
+
+    const i18n = useI18n()
+
+    const collectionLabel = computed(() => {
+      if (!collectionParams.value) {
+        return ""
+      }
+      const { collection, ...params } = collectionParams.value
+      return i18n
+        .t(`collection.label.${collection}.audio`, { ...params })
+        .toString()
+    })
+
+    const fetchMedia = async (
+      { shouldPersistMedia }: { shouldPersistMedia: boolean } = {
+        shouldPersistMedia: false,
+      }
+    ) => {
+      media.value = (await mediaStore.fetchMedia({
+        shouldPersistMedia,
+      })) as (typeof media)["value"]
+      creatorUrl.value =
+        media.value.length > 0 ? media.value[0].creator_url : undefined
+    }
+    const loadMore = () => {
+      fetchMedia({ shouldPersistMedia: true })
+    }
+
+    useMeta({
+      meta: [{ hid: "robots", name: "robots", content: "all" }],
     })
 
     useFetch(async () => {
       await fetchMedia()
     })
 
-    useMeta({
-      meta: [{ hid: "robots", name: "robots", content: "all" }],
+    // Fetch media when the route changes, e.g. when the user navigates from
+    // a creator collection page to a source collection page.
+    const route = useRoute()
+    watch(route, () => {
+      fetchMedia()
     })
 
     return {
-      results,
       collectionParams,
       isFetching,
-      creatorUrl,
+      media,
       collectionLabel,
-      handleLoadMore,
+      loadMore,
     }
   },
   head: {},
diff --git a/frontend/src/pages/image/collection.vue b/frontend/src/pages/image/collection.vue
index 133dba9257d..4cbb5d9a8d8 100644
--- a/frontend/src/pages/image/collection.vue
+++ b/frontend/src/pages/image/collection.vue
@@ -4,22 +4,28 @@
       v-if="collectionParams"
       search-term=""
       :is-fetching="isFetching"
-      :results="results"
+      :results="{ type: 'image', items: media }"
       :collection-label="collectionLabel"
       :collection-params="collectionParams"
-      @load-more="handleLoadMore"
+      @load-more="loadMore"
     />
   </div>
 </template>
 
 <script lang="ts">
-import { defineComponent, useFetch, useMeta } from "@nuxtjs/composition-api"
-import { computed } from "vue"
+import {
+  defineComponent,
+  useFetch,
+  useMeta,
+  useRoute,
+} from "@nuxtjs/composition-api"
+import { computed, ref, watch } from "vue"
 
 import { collectionMiddleware } from "~/middleware/collection"
-import { useCollection } from "~/composables/use-collection"
+import { useMediaStore } from "~/stores/media"
 import { useSearchStore } from "~/stores/search"
-import { IMAGE } from "~/constants/media"
+import type { ImageDetail } from "~/types/media"
+import { useI18n } from "~/composables/use-i18n"
 
 import VCollectionResults from "~/components/VSearchResultsGrid/VCollectionResults.vue"
 
@@ -29,21 +35,42 @@ export default defineComponent({
   layout: "content-layout",
   middleware: collectionMiddleware,
   setup() {
+    const mediaStore = useMediaStore()
     const searchStore = useSearchStore()
 
     const collectionParams = computed(() => searchStore.collectionParams)
+    const isFetching = computed(() => mediaStore.fetchState.isFetching)
 
-    const {
-      results,
-      creatorUrl,
-      fetchMedia,
-      handleLoadMore,
-      collectionLabel,
-      isFetching,
-    } = useCollection({
-      mediaType: IMAGE,
+    const media = ref<ImageDetail[]>([])
+    const creatorUrl = ref<string>()
+
+    const i18n = useI18n()
+
+    const collectionLabel = computed(() => {
+      if (!collectionParams.value) {
+        return ""
+      }
+      const { collection, ...params } = collectionParams.value
+      return i18n
+        .t(`collection.label.${collection}.image`, { ...params })
+        .toString()
     })
 
+    const fetchMedia = async (
+      { shouldPersistMedia }: { shouldPersistMedia: boolean } = {
+        shouldPersistMedia: false,
+      }
+    ) => {
+      media.value = (await mediaStore.fetchMedia({
+        shouldPersistMedia,
+      })) as (typeof media)["value"]
+      creatorUrl.value =
+        media.value.length > 0 ? media.value[0].creator_url : undefined
+    }
+    const loadMore = () => {
+      fetchMedia({ shouldPersistMedia: true })
+    }
+
     useMeta({
       meta: [{ hid: "robots", name: "robots", content: "all" }],
     })
@@ -52,13 +79,20 @@ export default defineComponent({
       await fetchMedia()
     })
 
+    // Fetch media when the route changes, e.g. when the user navigates from
+    // a creator collection page to a source collection page.
+    const route = useRoute()
+    watch(route, () => {
+      fetchMedia()
+    })
+
     return {
       collectionParams,
-      results,
       isFetching,
       creatorUrl,
       collectionLabel,
-      handleLoadMore,
+      loadMore,
+      media,
     }
   },
   head: {},
diff --git a/frontend/src/pages/search.vue b/frontend/src/pages/search.vue
index 51672e4b1f1..29134ae2e18 100644
--- a/frontend/src/pages/search.vue
+++ b/frontend/src/pages/search.vue
@@ -147,23 +147,15 @@ export default defineComponent({
     const isFetching = computed(() => fetchState.value.isFetching)
 
     /**
-     * Search middleware runs when the path changes. This watcher
-     * is necessary to handle the query changes.
-     *
-     * It updates the search store state from the URL query,
-     * fetches media, and scrolls to top if necessary.
+     * The search middleware updates the search store on every path or query
+     * change.
+     * This watcher fetches media, and scrolls to top if necessary.
      */
     watch(route, async (newRoute, oldRoute) => {
       if (
         newRoute.path !== oldRoute.path ||
         !isShallowEqualObjects(newRoute.query, oldRoute.query)
       ) {
-        const { query: urlQuery, path } = newRoute
-        searchStore.setSearchStateFromUrl({ urlQuery, path })
-
-        const mediaStore = useMediaStore()
-        mediaStore.clearMedia()
-
         /**
          * By default, Nuxt only scrolls to top when the path changes.
          * This is a workaround to scroll to top when the query changes.
diff --git a/frontend/src/stores/media/index.ts b/frontend/src/stores/media/index.ts
index 959e14fe45b..b87c55e0a4f 100644
--- a/frontend/src/stores/media/index.ts
+++ b/frontend/src/stores/media/index.ts
@@ -416,9 +416,6 @@ export const useMediaStore = defineStore("media", {
     async fetchMedia(payload: { shouldPersistMedia?: boolean } = {}) {
       const mediaType = this._searchType
       const shouldPersistMedia = Boolean(payload.shouldPersistMedia)
-      if (!shouldPersistMedia) {
-        this.clearMedia()
-      }
 
       const mediaToFetch = this._fetchableMediaTypes