From 9b43fcc336907bd2b06dd705244e3d7ed597f41c Mon Sep 17 00:00:00 2001
From: Olga Bulat <obulat@gmail.com>
Date: Wed, 4 May 2022 08:43:18 +0300
Subject: [PATCH] Await fetching related media in the single result store
 (#1349)

* Await fetching related media in the single result store

* Fix fetching related media both on the server and on the client

* Simplify typing hacks

* Stop showing 'Load more' button in the related images
---
 .../VImageDetails/VRelatedImages.vue          |   2 +-
 src/components/VImageGrid/VImageGrid.vue      |  12 +-
 src/locales/po-files/openverse.pot            |   7 +-
 src/pages/audio/_id.vue                       |  60 +++----
 src/pages/image/_id.vue                       | 149 ++++++++++--------
 src/stores/media/single-result.ts             |  78 ++++++---
 .../specs/stores/single-result-store.spec.js  |   2 +-
 7 files changed, 182 insertions(+), 128 deletions(-)

diff --git a/src/components/VImageDetails/VRelatedImages.vue b/src/components/VImageDetails/VRelatedImages.vue
index a713a002ad..d500675ff1 100644
--- a/src/components/VImageDetails/VRelatedImages.vue
+++ b/src/components/VImageDetails/VRelatedImages.vue
@@ -5,7 +5,7 @@
     </h3>
     <VImageGrid
       :images="media"
-      :can-load-more="false"
+      :show-load-more="false"
       :fetch-state="fetchState"
     />
   </aside>
diff --git a/src/components/VImageGrid/VImageGrid.vue b/src/components/VImageGrid/VImageGrid.vue
index 6d6226eb3b..0a1d7ec8dd 100644
--- a/src/components/VImageGrid/VImageGrid.vue
+++ b/src/components/VImageGrid/VImageGrid.vue
@@ -10,7 +10,7 @@
     <h5 v-if="isError && !fetchState.isFinished" class="py-4">
       {{ fetchState.fetchingError }}
     </h5>
-    <footer class="pt-4">
+    <footer v-if="showLoadMore" class="pt-4">
       <VLoadMore />
     </footer>
   </section>
@@ -34,7 +34,11 @@ export default {
     images: {
       default: () => [],
     },
-    canLoadMore: {
+    /**
+     * Whether to show the 'Load More' button.
+     * Is false for related images
+     */
+    showLoadMore: {
       type: Boolean,
       default: true,
     },
@@ -47,10 +51,6 @@ export default {
     isError() {
       return !!this.fetchState.fetchingError
     },
-    fetchingErrorHeading() {
-      const type = this.$t('browse-page.search-form.image')
-      return this.$t('browse-page.fetching-error', { type })
-    },
   },
   methods: {
     onLoadMore() {
diff --git a/src/locales/po-files/openverse.pot b/src/locales/po-files/openverse.pot
index 382cb2508f..9e2c87624c 100644
--- a/src/locales/po-files/openverse.pot
+++ b/src/locales/po-files/openverse.pot
@@ -4,7 +4,7 @@ msgid ""
 msgstr ""
 "Project-Id-Version: Openverse \n"
 "Report-Msgid-Bugs-To: https://github.com/wordpress/openverse/issues \n"
-"POT-Creation-Date: 2022-05-03T05:46:51+00:00\n"
+"POT-Creation-Date: 2022-05-03T16:18:32+00:00\n"
 "MIME-Version: 1.0\n"
 "Content-Type: text/plain; charset=UTF-8\n"
 "Content-Transfer-Encoding: 8bit\n"
@@ -797,7 +797,6 @@ msgctxt "browse-page.search-form.placeholder"
 msgid "Search all ###type###"
 msgstr ""
 
-#: src/components/VImageGrid/VImageGrid.vue:51
 msgctxt "browse-page.search-form.image"
 msgid "images"
 msgstr ""
@@ -1120,13 +1119,13 @@ msgid "An error occurred"
 msgstr ""
 
 #. Do not translate words between ### ###.
-#: src/pages/image/_id.vue:142
+#: src/pages/image/_id.vue:195
 msgctxt "error.image-not-found"
 msgid "Couldn't find image with id ###id###"
 msgstr ""
 
 #. Do not translate words between ### ###.
-#: src/pages/audio/_id.vue:71
+#: src/pages/audio/_id.vue:84
 msgctxt "error.media-not-found"
 msgid "Couldn't find ###mediaType### with id ###id###"
 msgstr ""
diff --git a/src/pages/audio/_id.vue b/src/pages/audio/_id.vue
index baf219c2ab..e6261cdbbc 100644
--- a/src/pages/audio/_id.vue
+++ b/src/pages/audio/_id.vue
@@ -18,11 +18,11 @@
   </main>
 </template>
 
-<script>
-import { computed } from '@nuxtjs/composition-api'
+<script lang="ts">
+import { computed, defineComponent } from '@nuxtjs/composition-api'
 
 import { AUDIO } from '~/constants/media'
-
+import type { AudioDetail } from '~/models/media'
 import { useRelatedMediaStore } from '~/stores/media/related-media'
 import { useSingleResultStore } from '~/stores/media/single-result'
 
@@ -32,7 +32,9 @@ import VBackToSearchResultsLink from '~/components/VBackToSearchResultsLink.vue'
 import VRelatedAudio from '~/components/VAudioDetails/VRelatedAudio.vue'
 import VMediaReuse from '~/components/VMediaInfo/VMediaReuse.vue'
 
-const AudioDetailPage = {
+import type { NuxtApp } from '@nuxt/types/app'
+
+export default defineComponent({
   name: 'AudioDetailPage',
   components: {
     VAudioDetails,
@@ -41,30 +43,41 @@ const AudioDetailPage = {
     VMediaReuse,
     VRelatedAudio,
   },
-  data() {
-    return {
-      backToSearchPath: '',
-    }
+  beforeRouteEnter(to, from, nextPage) {
+    nextPage((_this) => {
+      // `_this` is the component instance that also has nuxt app properties injected
+      const localeRoute = (_this as NuxtApp).localeRoute
+      if (
+        from.name === localeRoute({ path: '/search/' }).name ||
+        from.name === localeRoute({ path: '/search/audio' }).name
+      ) {
+        // I don't know how to type `_this` here
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
+        _this.backToSearchPath = from.fullPath
+      }
+    })
   },
   setup() {
+    const singleResultStore = useSingleResultStore()
     const relatedMediaStore = useRelatedMediaStore()
 
+    const audio = computed(() =>
+      singleResultStore.mediaType === AUDIO
+        ? (singleResultStore.mediaItem as AudioDetail)
+        : null
+    )
     const relatedMedia = computed(() => relatedMediaStore.media)
     const relatedFetchState = computed(() => relatedMediaStore.fetchState)
 
-    return { relatedMedia, relatedFetchState }
+    return { audio, relatedMedia, relatedFetchState }
   },
   async asyncData({ route, error, app, $pinia }) {
     const audioId = route.params.id
     const singleResultStore = useSingleResultStore($pinia)
 
     try {
-      await singleResultStore.fetchMediaItem(AUDIO, audioId)
-      const audio = singleResultStore.mediaItem
-
-      return {
-        audio,
-      }
+      await singleResultStore.fetch(AUDIO, audioId)
     } catch (err) {
       error({
         statusCode: 404,
@@ -75,15 +88,10 @@ const AudioDetailPage = {
       })
     }
   },
-  beforeRouteEnter(to, from, nextPage) {
-    nextPage((_this) => {
-      if (
-        from.name === _this.localeRoute({ path: '/search/' }).name ||
-        from.name === _this.localeRoute({ path: '/search/audio' }).name
-      ) {
-        _this.backToSearchPath = from.fullPath
-      }
-    })
+  data() {
+    return {
+      backToSearchPath: '',
+    }
   },
   head() {
     const title = this.audio.title
@@ -98,9 +106,7 @@ const AudioDetailPage = {
       ],
     }
   },
-}
-
-export default AudioDetailPage
+})
 </script>
 <style>
 .audio-page {
diff --git a/src/pages/image/_id.vue b/src/pages/image/_id.vue
index 7bc054074a..aa60376f47 100644
--- a/src/pages/image/_id.vue
+++ b/src/pages/image/_id.vue
@@ -73,14 +73,15 @@
   </div>
 </template>
 
-<script>
+<script lang="ts">
 import axios from 'axios'
 
-import { computed } from '@nuxtjs/composition-api'
+import { computed, defineComponent, ref } from '@nuxtjs/composition-api'
 
 import { IMAGE } from '~/constants/media'
 import { useSingleResultStore } from '~/stores/media/single-result'
 import { useRelatedMediaStore } from '~/stores/media/related-media'
+import type { ImageDetail } from '~/models/media'
 
 import VButton from '~/components/VButton.vue'
 import VLink from '~/components/VLink.vue'
@@ -90,7 +91,9 @@ import VRelatedImages from '~/components/VImageDetails/VRelatedImages.vue'
 import SketchFabViewer from '~/components/SketchFabViewer.vue'
 import VBackToSearchResultsLink from '~/components/VBackToSearchResultsLink.vue'
 
-const VImageDetailsPage = {
+import type { NuxtApp } from '@nuxt/types/app'
+
+export default defineComponent({
   name: 'VImageDetailsPage',
   components: {
     VButton,
@@ -101,85 +104,107 @@ const VImageDetailsPage = {
     SketchFabViewer,
     VBackToSearchResultsLink,
   },
-  data() {
-    return {
-      imageWidth: 0,
-      imageHeight: 0,
-      imageType: 'Unknown',
-      isLoadingFullImage: true,
-      backToSearchPath: '',
-      sketchFabfailure: false,
-    }
+  beforeRouteEnter(_to, from, nextPage) {
+    nextPage((_this) => {
+      // `_this` is the component instance that also has nuxt app properties injected
+      const localeRoute = (_this as NuxtApp).localeRoute
+      if (
+        from.name === localeRoute({ path: '/search/' })?.name ||
+        from.name === localeRoute({ path: '/search/image' })?.name
+      ) {
+        // I don't know how to type `this` here
+        // eslint-disable-next-line @typescript-eslint/ban-ts-comment
+        // @ts-ignore
+        _this.backToSearchPath = from.fullPath
+      }
+    })
   },
   setup() {
+    const singleResultStore = useSingleResultStore()
     const relatedMediaStore = useRelatedMediaStore()
+    const image = computed(() =>
+      singleResultStore.mediaType === IMAGE
+        ? (singleResultStore.mediaItem as ImageDetail)
+        : null
+    )
 
     const relatedMedia = computed(() => relatedMediaStore.media)
     const relatedFetchState = computed(() => relatedMediaStore.fetchState)
 
-    return { relatedMedia, relatedFetchState }
-  },
-  computed: {
-    sketchFabUid() {
-      if (this.image?.source !== 'sketchfab' || this.sketchFabfailure) {
+    const imageWidth = ref(0)
+    const imageHeight = ref(0)
+    const imageType = ref('Unknown')
+    const isLoadingFullImage = ref(true)
+    const sketchFabfailure = ref(false)
+
+    const sketchFabUid = computed(() => {
+      if (image.value?.source !== 'sketchfab' || sketchFabfailure.value) {
         return null
       }
-      return this.image.url
+      return image.value.url
         .split('https://media.sketchfab.com/models/')[1]
         .split('/')[0]
-    },
+    })
+
+    const onImageLoaded = (event: Event) => {
+      if (!(event.target instanceof HTMLImageElement)) {
+        return
+      }
+      imageWidth.value = image.value?.width || event.target.naturalWidth
+      imageHeight.value = image.value?.height || event.target.naturalHeight
+      if (image.value?.filetype) {
+        imageType.value = image.value.filetype
+      } else {
+        if (event.target) {
+          axios
+            .head(event.target.src)
+            .then((res) => {
+              imageType.value = res.headers['content-type']
+            })
+            .catch(() => {
+              /**
+               * Do nothing. This avoid the console warning "Uncaught (in promise) Error:
+               * Network Error" in Firefox in development mode.
+               */
+            })
+        }
+      }
+      isLoadingFullImage.value = false
+    }
+
+    return {
+      image,
+      relatedMedia,
+      relatedFetchState,
+      imageWidth,
+      imageHeight,
+      imageType,
+      isLoadingFullImage,
+      sketchFabfailure,
+      sketchFabUid,
+      onImageLoaded,
+    }
   },
   async asyncData({ app, error, route, $pinia }) {
     const imageId = route.params.id
     const singleResultStore = useSingleResultStore($pinia)
     try {
-      await singleResultStore.fetchMediaItem(IMAGE, imageId)
-      const image = singleResultStore.mediaItem
-      return {
-        image,
-      }
+      await singleResultStore.fetch(IMAGE, imageId)
     } catch (err) {
-      const errorMessage = app.i18n.t('error.image-not-found', {
-        id: imageId,
-      })
-      error({
+      const errorMessage = app.i18n
+        .t('error.image-not-found', {
+          id: imageId,
+        })
+        .toString()
+      return error({
         statusCode: 404,
         message: errorMessage,
       })
     }
   },
-  beforeRouteEnter(to, from, nextPage) {
-    nextPage((_this) => {
-      if (
-        from.name === _this.localeRoute({ path: '/search/' }).name ||
-        from.name === _this.localeRoute({ path: '/search/image' }).name
-      ) {
-        _this.backToSearchPath = from.fullPath
-      }
-    })
-  },
-  methods: {
-    onImageLoaded(event) {
-      this.imageWidth = this.image.width || event.target.naturalWidth
-      this.imageHeight = this.image.height || event.target.naturalHeight
-      if (this.image.filetype) {
-        this.imageType = this.image.filetype
-      } else {
-        axios
-          .head(event.target.src)
-          .then((res) => {
-            this.imageType = res.headers['content-type']
-          })
-          .catch(() => {
-            /**
-             * Do nothing. This avoids the console warning "Uncaught (in promise) Error:
-             * Network Error" in Firefox in development mode.
-             */
-          })
-      }
-      this.isLoadingFullImage = false
-    },
-  },
+  data: () => ({
+    backToSearchPath: '',
+  }),
   head() {
     const title = `${this.image.title} | Openverse`
 
@@ -204,9 +229,7 @@ const VImageDetailsPage = {
       ],
     }
   },
-}
-
-export default VImageDetailsPage
+})
 </script>
 
 <style scoped>
diff --git a/src/stores/media/single-result.ts b/src/stores/media/single-result.ts
index 3df6d388dc..1ade3cff8f 100644
--- a/src/stores/media/single-result.ts
+++ b/src/stores/media/single-result.ts
@@ -2,7 +2,7 @@ import { defineStore } from 'pinia'
 
 import axios from 'axios'
 
-import type { Media } from '~/models/media'
+import type { AudioDetail, ImageDetail, Media } from '~/models/media'
 import type { SupportedMediaType } from '~/constants/media'
 import {
   FetchState,
@@ -11,20 +11,30 @@ import {
 } from '~/composables/use-fetch-state'
 import { services } from '~/stores/media/services'
 import { useMediaStore } from '~/stores/media/index'
-import { IMAGE } from '~/constants/media'
 import { useRelatedMediaStore } from '~/stores/media/related-media'
 import { useProviderStore } from '~/stores/provider'
 
-export interface MediaItemState {
-  mediaItem: Media | null
-  mediaType: SupportedMediaType
-  fetchState: FetchState
-}
+export type MediaItemState =
+  | {
+      mediaType: 'audio'
+      mediaItem: AudioDetail
+      fetchState: FetchState
+    }
+  | {
+      mediaType: 'image'
+      mediaItem: ImageDetail
+      fetchState: FetchState
+    }
+  | {
+      mediaType: null
+      mediaItem: null
+      fetchState: FetchState
+    }
 
 export const useSingleResultStore = defineStore('single-result', {
   state: (): MediaItemState => ({
     mediaItem: null,
-    mediaType: IMAGE,
+    mediaType: null,
     fetchState: initialFetchState,
   }),
 
@@ -49,32 +59,48 @@ export const useSingleResultStore = defineStore('single-result', {
       return mediaItem
     },
 
-    async fetchMediaItem(type: SupportedMediaType, id: string) {
+    /**
+     * If the result is available in the media store (from search results or related media),
+     * we can re-use it. Otherwise, we fetch it from the API.
+     * The media objects from the search results currently don't have filetype or filesize properties.
+     *
+     * @param type - media type of the item to fetch.
+     * @param id - string id of the item to fetch.
+     */
+    async fetch(type: SupportedMediaType, id: string) {
       const mediaStore = useMediaStore()
       const existingItem = mediaStore.getItemById(type, id)
 
-      // Not awaiting to make this call non-blocking
-      useRelatedMediaStore().fetchMedia(type, id)
       if (existingItem) {
         this.mediaType = existingItem.frontendMediaType
         this.mediaItem = this._addProviderName(existingItem)
       } else {
-        try {
-          this._updateFetchState('start')
-          this.mediaItem = this._addProviderName(
-            await services[type].getMediaDetail(id)
-          )
-          this.mediaType = type
+        await this.fetchMediaItem(type, id)
+      }
+      await useRelatedMediaStore().fetchMedia(type, id)
+    },
+    /**
+     * Fetches a media item from the API.
+     *
+     * @param type - media type of the item to fetch.
+     * @param id - string id of the item to fetch.
+     */
+    async fetchMediaItem(type: SupportedMediaType, id: string) {
+      try {
+        this._updateFetchState('start')
+        this.mediaItem = this._addProviderName(
+          await services[type].getMediaDetail(id)
+        )
+        this.mediaType = type
 
-          this._updateFetchState('end')
-        } catch (error: unknown) {
-          this.mediaItem = null
-          this.mediaType = type
-          if (axios.isAxiosError(error) && error.response?.status === 404) {
-            throw new Error(`Media of type ${type} with id ${id} not found`)
-          } else {
-            this.handleMediaError(error)
-          }
+        this._updateFetchState('end')
+      } catch (error: unknown) {
+        this.mediaItem = null
+        this.mediaType = type
+        if (axios.isAxiosError(error) && error.response?.status === 404) {
+          throw new Error(`Media of type ${type} with id ${id} not found`)
+        } else {
+          this.handleMediaError(error)
         }
       }
     },
diff --git a/test/unit/specs/stores/single-result-store.spec.js b/test/unit/specs/stores/single-result-store.spec.js
index 1a63b0221c..1f66a084f9 100644
--- a/test/unit/specs/stores/single-result-store.spec.js
+++ b/test/unit/specs/stores/single-result-store.spec.js
@@ -36,7 +36,7 @@ describe('Media Item Store', () => {
       const singleResultStore = useSingleResultStore()
       expect(singleResultStore.fetchState).toEqual(initialFetchState)
       expect(singleResultStore.mediaItem).toEqual(null)
-      expect(singleResultStore.mediaType).toEqual(IMAGE)
+      expect(singleResultStore.mediaType).toEqual(null)
     })
   })