diff --git a/frontend/src/components/VAudioDetails/VRelatedAudio.vue b/frontend/src/components/VAudioDetails/VRelatedAudio.vue index d52420174d2..ff4e72878a4 100644 --- a/frontend/src/components/VAudioDetails/VRelatedAudio.vue +++ b/frontend/src/components/VAudioDetails/VRelatedAudio.vue @@ -1,5 +1,8 @@ diff --git a/frontend/src/components/VGlobalAudioSection/VGlobalAudioSection.vue b/frontend/src/components/VGlobalAudioSection/VGlobalAudioSection.vue index 6ccaa38ef3c..8c2e5d5fdaf 100644 --- a/frontend/src/components/VGlobalAudioSection/VGlobalAudioSection.vue +++ b/frontend/src/components/VGlobalAudioSection/VGlobalAudioSection.vue @@ -51,8 +51,8 @@ export default defineComponent({ return audioFromMediaStore as AudioDetail } const singleResultStore = useSingleResultStore() - if (singleResultStore.mediaItem?.id === trackId) { - return singleResultStore.mediaItem as AudioDetail + if (singleResultStore.mediaId === trackId) { + return singleResultStore.audio } return null } diff --git a/frontend/src/components/VImageDetails/VRelatedImages.vue b/frontend/src/components/VImageDetails/VRelatedImages.vue index 2d8d9d97833..d61913b3446 100644 --- a/frontend/src/components/VImageDetails/VRelatedImages.vue +++ b/frontend/src/components/VImageDetails/VRelatedImages.vue @@ -1,9 +1,8 @@ diff --git a/frontend/src/pages/image/_id/index.vue b/frontend/src/pages/image/_id/index.vue index b62fadd23c7..6f0a37d3c0a 100644 --- a/frontend/src/pages/image/_id/index.vue +++ b/frontend/src/pages/image/_id/index.vue @@ -7,13 +7,19 @@ /> -
+
+ - + @@ -91,7 +93,13 @@ import axios from "axios" import { computed, ref } from "vue" -import { defineComponent, useMeta } from "@nuxtjs/composition-api" +import { + defineComponent, + useContext, + useFetch, + useMeta, + useRoute, +} from "@nuxtjs/composition-api" import { IMAGE } from "~/constants/media" import { skipToContentTargetId } from "~/constants/window" @@ -99,12 +107,12 @@ import type { ImageDetail } from "~/types/media" import { useAnalytics } from "~/composables/use-analytics" import { useSingleResultStore } from "~/stores/media/single-result" -import { useRelatedMediaStore } from "~/stores/media/related-media" import { useSearchStore } from "~/stores/search" import { createDetailPageMeta } from "~/utils/og" import { singleResultMiddleware } from "~/middleware/single-result" import VBackToSearchResultsLink from "~/components/VBackToSearchResultsLink.vue" +import VBone from "~/components/VSkeleton/VBone.vue" import VButton from "~/components/VButton.vue" import VImageDetails from "~/components/VImageDetails/VImageDetails.vue" import VLink from "~/components/VLink.vue" @@ -117,6 +125,7 @@ import errorImage from "~/assets/image_not_available_placeholder.png" export default defineComponent({ name: "VImageDetailsPage", components: { + VBone, VBackToSearchResultsLink, VButton, VLink, @@ -127,32 +136,43 @@ export default defineComponent({ }, layout: "content-layout", middleware: singleResultMiddleware, + // Fetching on the server is disabled because it is + // handled by the `singleResultMiddleware`. + fetchOnServer: false, setup() { const singleResultStore = useSingleResultStore() - const relatedMediaStore = useRelatedMediaStore() const searchStore = useSearchStore() - const image = computed(() => - singleResultStore.mediaType === IMAGE - ? (singleResultStore.mediaItem as ImageDetail) - : null - ) + const route = useRoute() - const relatedMedia = computed( - () => relatedMediaStore.media as ImageDetail[] - ) - const backToSearchPath = computed(() => searchStore.backToSearchPath) - const hasRelatedMedia = computed(() => relatedMediaStore.media.length > 0) - const relatedFetchState = computed(() => relatedMediaStore.fetchState) + const image = ref(singleResultStore.image) - const imageWidth = ref(0) - const imageHeight = ref(0) - const imageType = ref("Unknown") /** * To make sure that image is loaded fast, we `src` to `image.thumbnail`, - * and then replace it with the provider image once it is loaded. + * and replace it with the provider image once the thumbnail is loaded. */ - const imageSrc = ref(image.value.thumbnail) + const imageSrc = ref(image.value?.thumbnail) + + const isLoadingThumbnail = ref(true) + + const { error: nuxtError } = useContext() + + useFetch(async () => { + const imageId = route.value.params.id + const fetchedImage = await singleResultStore.fetch(IMAGE, imageId) + if (!fetchedImage) { + nuxtError(singleResultStore.fetchState.fetchingError ?? {}) + } else { + image.value = fetchedImage + imageSrc.value = fetchedImage.thumbnail + } + }) + + const backToSearchPath = computed(() => searchStore.backToSearchPath) + + const imageWidth = ref(image.value?.width ?? 0) + const imageHeight = ref(image.value?.height ?? 0) + const imageType = ref(image.value?.filetype ?? "Unknown") const isLoadingMainImage = ref(true) const sketchFabfailure = ref(false) @@ -174,7 +194,7 @@ export default defineComponent({ return } imageSrc.value = - event.target.src === image.value.url + event.target.src === image.value?.url ? image.value.thumbnail : errorImage } @@ -185,14 +205,15 @@ export default defineComponent({ * @param event - the image load event. */ const onImageLoaded = (event: Event) => { - if (!(event.target instanceof HTMLImageElement)) { - return - } + if (!(event.target instanceof HTMLImageElement) || !image.value) return + + isLoadingThumbnail.value = false + if (isLoadingMainImage.value) { - imageWidth.value = image.value?.width || event.target.naturalWidth - imageHeight.value = image.value?.height || event.target.naturalHeight + imageWidth.value = image.value.width || event.target.naturalWidth + imageHeight.value = image.value.height || event.target.naturalHeight - if (image.value?.filetype) { + if (image.value.filetype) { imageType.value = image.value.filetype } else { axios @@ -237,7 +258,7 @@ export default defineComponent({ } sendCustomEvent("VISIT_CREATOR_LINK", { id: image.value.id, - url: image.value.creator_url, + url: image.value.creator_url ?? "", }) } @@ -245,15 +266,14 @@ export default defineComponent({ return { image, - hasRelatedMedia, - relatedMedia, - relatedFetchState, imageWidth, imageHeight, imageSrc, imageType, sketchFabfailure, sketchFabUid, + + isLoadingThumbnail, onImageLoaded, onImageError, handleRightClick, @@ -265,23 +285,6 @@ export default defineComponent({ sendVisitCreatorLinkEvent, } }, - async asyncData({ app, error, route, $pinia }) { - const imageId = route.params.id - const singleResultStore = useSingleResultStore($pinia) - try { - await singleResultStore.fetch(IMAGE, imageId) - } catch (err) { - const errorMessage = app.i18n - .t("error.imageNotFound", { - id: imageId, - }) - .toString() - return error({ - statusCode: 404, - message: errorMessage, - }) - } - }, head: {}, }) diff --git a/frontend/src/pages/image/_id/report.vue b/frontend/src/pages/image/_id/report.vue index f7acaf74119..dfb7bef5645 100644 --- a/frontend/src/pages/image/_id/report.vue +++ b/frontend/src/pages/image/_id/report.vue @@ -4,20 +4,20 @@ tabindex="-1" class="mx-auto mb-6 mt-8 max-w-none gap-x-10 px-4 md:grid md:max-w-4xl md:grid-cols-2 md:px-6 lg:mb-30 lg:px-0 xl:max-w-4xl" > -
+
diff --git a/frontend/src/stores/media/index.ts b/frontend/src/stores/media/index.ts index 1160aa40d39..c299ff91e72 100644 --- a/frontend/src/stores/media/index.ts +++ b/frontend/src/stores/media/index.ts @@ -5,7 +5,12 @@ import axios from "axios" import { warn } from "~/utils/console" import { hash, rand as prng } from "~/utils/prng" import prepareSearchQueryParams from "~/utils/prepare-search-query-params" -import type { DetailFromMediaType, Media } from "~/types/media" +import type { + AudioDetail, + DetailFromMediaType, + ImageDetail, + Media, +} from "~/types/media" import type { FetchState } from "~/types/fetch-state" import { ALL_MEDIA, @@ -190,7 +195,7 @@ export const useMediaStore = defineStore("media", { * TODO: Fix the algorithm. * This implementation can hide hits from media types with fewer hits. */ - allMedia(state): Media[] { + allMedia(state): (AudioDetail | ImageDetail)[] { const media = this.resultItems // Seed the random number generator with the ID of diff --git a/frontend/src/stores/media/related-media.ts b/frontend/src/stores/media/related-media.ts index 88be1ef7194..09fc54225dc 100644 --- a/frontend/src/stores/media/related-media.ts +++ b/frontend/src/stores/media/related-media.ts @@ -1,14 +1,17 @@ import { defineStore } from "pinia" +import axios from "axios" -import type { FetchState } from "~/types/fetch-state" import { initServices } from "~/stores/media/services" +import type { FetchState } from "~/types/fetch-state" import type { Media } from "~/types/media" import type { SupportedMediaType } from "~/constants/media" +import type { NuxtError } from "@nuxt/types" + interface RelatedMediaState { mainMediaId: null | string media: Media[] - fetchState: FetchState + fetchState: FetchState } export const useRelatedMediaStore = defineStore("related-media", { @@ -26,7 +29,7 @@ export const useRelatedMediaStore = defineStore("related-media", { }, actions: { - _endFetching(error?: string) { + _endFetching(error?: NuxtError) { this.fetchState.fetchingError = error || null this.fetchState.hasStarted = true this.fetchState.isFetching = false @@ -54,9 +57,17 @@ export const useRelatedMediaStore = defineStore("related-media", { await service.getRelatedMedia(id) ).results this._endFetching() + + return this.media.length } catch (error) { - this._endFetching(`Could not fetch related ${mediaType} for id ${id}`) - throw new Error(`Could not fetch related ${mediaType} for id ${id}`) + const message = `Could not fetch related ${mediaType} for id ${id}.` + const statusCode = + axios.isAxiosError(error) && error.response?.status + ? error.response.status + : 404 + this._endFetching({ message, statusCode }) + this.$nuxt.$sentry.captureException(error) + return null } }, }, diff --git a/frontend/src/stores/media/single-result.ts b/frontend/src/stores/media/single-result.ts index 1a909eeb066..9be4c32c9c9 100644 --- a/frontend/src/stores/media/single-result.ts +++ b/frontend/src/stores/media/single-result.ts @@ -2,54 +2,70 @@ import { defineStore } from "pinia" import axios from "axios" -import type { AudioDetail, ImageDetail, Media } from "~/types/media" -import type { SupportedMediaType } from "~/constants/media" import type { FetchState } from "~/types/fetch-state" +import type { + AudioDetail, + DetailFromMediaType, + ImageDetail, +} from "~/types/media" +import { isDetail, isMediaDetail } from "~/types/media" + +import type { SupportedMediaType } from "~/constants/media" + import { initServices } from "~/stores/media/services" import { useMediaStore } from "~/stores/media/index" -import { useRelatedMediaStore } from "~/stores/media/related-media" import { useProviderStore } from "~/stores/provider" +import { log } from "~/utils/console" -export type MediaItemState = - | { - mediaType: "audio" - mediaItem: AudioDetail - fetchState: FetchState - } - | { - mediaType: "image" - mediaItem: ImageDetail - fetchState: FetchState - } - | { - mediaType: null - mediaItem: null - fetchState: FetchState - } +import type { NuxtError } from "@nuxt/types" + +export type MediaItemState = { + mediaType: SupportedMediaType | null + mediaId: string | null + mediaItem: DetailFromMediaType | null + fetchState: FetchState +} export const useSingleResultStore = defineStore("single-result", { state: (): MediaItemState => ({ - mediaItem: null, mediaType: null, - fetchState: { isFetching: false, hasStarted: false, fetchingError: null }, + mediaId: null, + mediaItem: null, + fetchState: { isFetching: false, fetchingError: null }, }), + getters: { + audio(state): AudioDetail | null { + if (isDetail.audio(state.mediaItem)) { + return state.mediaItem + } + return null + }, + image(state): ImageDetail | null { + if (isDetail.image(state.mediaItem)) { + return state.mediaItem + } + return null + }, + }, + actions: { - _endFetching(error?: string) { + _endFetching(error?: NuxtError) { this.fetchState.isFetching = false this.fetchState.fetchingError = error || null }, _startFetching() { this.fetchState.isFetching = true - this.fetchState.hasStarted = true this.fetchState.fetchingError = null }, - _updateFetchState(action: "start" | "end", option?: string) { + _updateFetchState(action: "start" | "end", option?: NuxtError) { action === "start" ? this._startFetching() : this._endFetching(option) }, - _addProviderName(mediaItem: Media) { + _addProviderName( + mediaItem: DetailFromMediaType + ): DetailFromMediaType { const providerStore = useProviderStore() mediaItem.providerName = providerStore.getProviderName( @@ -64,93 +80,111 @@ export const useSingleResultStore = defineStore("single-result", { } return mediaItem }, + reset() { + this.mediaItem = null + this.mediaType = null + this.mediaId = null + this.fetchState.isFetching = false + this.fetchState.fetchingError = null + }, /** - * 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. + * Set the media item with the display name of its provider + * and its type. Reset the other media type item. + * If the media item is null, reset both media items. */ - async fetch(type: SupportedMediaType, id: string) { - const mediaStore = useMediaStore() - const existingItem = mediaStore.getItemById(type, id) + setMediaItem(mediaItem: AudioDetail | ImageDetail | null) { + if (mediaItem) { + this.mediaItem = this._addProviderName(mediaItem) + this.mediaType = mediaItem.frontendMediaType + this.mediaId = mediaItem.id + } else { + this.mediaItem = null + this.mediaType = null + this.mediaId = null + } + }, + /** + * If the item is already in the media store, reuse it. + * Otherwise, set the media type and id, fetch the media + * itself later. + */ + setMediaById(type: SupportedMediaType, id: string) { + if (this.mediaId === id && isMediaDetail(this.mediaItem, type)) return + const existingItem = useMediaStore().getItemById(type, id) if (existingItem) { - this.mediaType = existingItem.frontendMediaType - this.mediaItem = this._addProviderName(existingItem) - this.fetchMediaItem(type, id).then(() => { - /** noop to prevent the promise return ignored warning */ - }) + this.setMediaItem(existingItem) } else { - await this.fetchMediaItem(type, id) + this.mediaId = id + this.mediaType = type } - /** - * On the server, we await the related media to make sure it's rendered on the page. - * On the client, we don't await it to render the whole page while the related media are loading. - */ - if (process.server) { - try { - await useRelatedMediaStore().fetchMedia(type, id) - } catch (error) { - console.warn("Could not load related media: ", error) - } - } else { - useRelatedMediaStore() - .fetchMedia(type, id) - .catch((error) => - console.warn("Could not load related media: ", error) - ) + }, + + getExistingItem( + type: T, + id: string + ): DetailFromMediaType | null { + if (this.mediaId === id && isMediaDetail(this.mediaItem, type)) { + return this.mediaItem + } + const existingItem = useMediaStore().getItemById(type, id) + if (existingItem) { + return existingItem as DetailFromMediaType } + return null }, + /** - * Fetches a media item from the API. + * Check if the `id` matches the `mediaId` and the media item + * is already fetched. If middleware only set the `id` and + * did not set the media, fetch the media item. * - * @param type - media type of the item to fetch. - * @param id - string id of the item to fetch. + * Fetch the related media if necessary. + */ + async fetch(type: T, id: string) { + const existingItem = this.getExistingItem(type, id) + + return existingItem + ? existingItem + : ((await this.fetchMediaItem(type, id)) as DetailFromMediaType) + }, + + /** + * Fetch the media item from the API. + * On error, send the error to Sentry and return null. */ - async fetchMediaItem(type: SupportedMediaType, id: string) { + async fetchMediaItem( + type: MediaType, + id: string + ) { try { this._updateFetchState("start") const accessToken = this.$nuxt.$openverseApiToken const service = initServices[type](accessToken) const item = this._addProviderName(await service.getMediaDetail(id)) - this.mediaItem = item - this.mediaType = type - + this.setMediaItem(item) 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) - } - } - }, - /** - * Throws a new error with a new error message. - */ - handleMediaError(error: unknown) { - let errorMessage - if (axios.isAxiosError(error)) { - errorMessage = - error.response?.status === 500 - ? "There was a problem with our servers" - : `Request failed with status ${ - error.response?.status ?? "unknown" - }` - } else { - errorMessage = - error instanceof Error ? error.message : "Oops! Something went wrong" - } + return item as DetailFromMediaType + } catch (error) { + this.reset() + const statusCode = getErrorStatusCode(error) + const message = `Error fetching single ${type} item with id ${id}` + + this._updateFetchState("end", { statusCode, message }) - this._updateFetchState("end", errorMessage) - throw new Error(errorMessage) + log(`${message}. Sending error to Sentry: ${JSON.stringify(error)}`) + this.$nuxt.$sentry.captureException(error) + + return null + } }, }, }) + +const getErrorStatusCode = (error: unknown) => + axios.isAxiosError(error) && error.response?.status + ? error.response.status + : 404 diff --git a/frontend/src/types/fetch-state.ts b/frontend/src/types/fetch-state.ts index 3c655c6b166..c16579cf4da 100644 --- a/frontend/src/types/fetch-state.ts +++ b/frontend/src/types/fetch-state.ts @@ -1,6 +1,9 @@ -export interface FetchState { +export interface BaseFetchState { isFetching: boolean - fetchingError: null | string hasStarted?: boolean isFinished?: boolean } + +export interface FetchState extends BaseFetchState { + fetchingError: null | ErrorType +} diff --git a/frontend/src/types/media.ts b/frontend/src/types/media.ts index 824ae64c56b..02a7c6c96eb 100644 --- a/frontend/src/types/media.ts +++ b/frontend/src/types/media.ts @@ -1,6 +1,7 @@ import type { SupportedMediaType } from "~/constants/media" import type { License, LicenseVersion } from "~/constants/license" import type { Sensitivity } from "~/constants/content-safety" +import { AUDIO, IMAGE } from "~/constants/media" export interface Tag { name: string @@ -109,10 +110,15 @@ export interface ImageDimensions { export type AspectRatio = "square" | "intrinsic" export const isDetail = { - audio: (media: Media): media is AudioDetail => { - return media.frontendMediaType === "audio" - }, - image: (media: Media): media is ImageDetail => { - return media.frontendMediaType === "image" - }, + audio: (media: Media | null): media is AudioDetail => + isMediaDetail(media, AUDIO), + image: (media: Media | null): media is ImageDetail => + isMediaDetail(media, IMAGE), +} + +export const isMediaDetail = ( + media: Media | null, + mediaType: T +): media is DetailFromMediaType => { + return !!media && media.frontendMediaType === mediaType } diff --git a/frontend/test/playwright/visual-regression/pages/errors.spec.ts b/frontend/test/playwright/visual-regression/pages/errors.spec.ts index 5a76607435d..14a828b7ae9 100644 --- a/frontend/test/playwright/visual-regression/pages/errors.spec.ts +++ b/frontend/test/playwright/visual-regression/pages/errors.spec.ts @@ -31,35 +31,34 @@ for (const { errorStatus, imageId } of errorTapes) { }) test(`${errorStatus} on single-result page on CSR`, async ({ page }) => { - await page.route(new RegExp(`v1/images/${imageId}`), (route) => { - const requestUrl = route.request().url() - - if (requestUrl.includes("/thumb")) { - return route.continue() - } + await page.route(new RegExp(`v1/images/`), (route) => { + // const requestUrl = route.request().url() + // if (requestUrl.includes("/thumb")) { + // return route.continue() + // } return route.fulfill({ status: errorStatus, + headers: { "Access-Control-Allow-Origin": "*" }, body: JSON.stringify({}), }) }) await preparePageForTests(page, breakpoint) - await goToSearchTerm(page, "errorSearchPages", { - mode: "CSR", - searchType: "image", - }) - await page.locator(`a[href*='/image/${imageId}']`).click() + // If we navigate from the search results page, we will already have the + // image data in the store, and will not fetch it from the API. + // To simulate a client side error, we need to click on the home gallery: + // then we have to make a client-side request because we don't have any + // data for the images in the store. + await page.goto("/") + await page.locator("a.home-cell").first().click() + // We can't use `waitForURL` because it would be flaky: + // the URL loads a skeleton page before showing the error page. + // eslint-disable-next-line playwright/no-networkidle + await page.waitForLoadState("networkidle") - // TODO: remove `maxDiffPixelRatio` after single result pages fetching is fixed - // https://github.com/WordPress/openverse/pull/2585 - await expectSnapshot( - `single-result-error-CSR`, - page, - { fullPage: true }, - { maxDiffPixelRatio: 0.02 } - ) + await expectSnapshot("single-result-error-CSR", page, { fullPage: true }) }) }) } diff --git a/frontend/test/playwright/visual-regression/pages/errors.spec.ts-snapshots/single-result-error-CSR-xl-linux.png b/frontend/test/playwright/visual-regression/pages/errors.spec.ts-snapshots/single-result-error-CSR-xl-linux.png index 018a4c16c53..fb41a0b71b0 100644 Binary files a/frontend/test/playwright/visual-regression/pages/errors.spec.ts-snapshots/single-result-error-CSR-xl-linux.png and b/frontend/test/playwright/visual-regression/pages/errors.spec.ts-snapshots/single-result-error-CSR-xl-linux.png differ diff --git a/frontend/test/tapes/detail/images/e9d97a98-621b-4ec2-bf70-f47a74380452_close.json5 b/frontend/test/tapes/detail/images/e9d97a98-621b-4ec2-bf70-f47a74380452_close.json5 index b5c243db5a0..4992184d3b4 100644 --- a/frontend/test/tapes/detail/images/e9d97a98-621b-4ec2-bf70-f47a74380452_close.json5 +++ b/frontend/test/tapes/detail/images/e9d97a98-621b-4ec2-bf70-f47a74380452_close.json5 @@ -43,6 +43,9 @@ 'referrer-policy': [ 'same-origin', ], + 'access-control-allow-origin': [ + '*', + ], 'cross-origin-opener-policy': [ 'same-origin', ], @@ -105,4 +108,4 @@ related_url: 'http://localhost:49153/v1/images/e9d97a98-621b-4ec2-bf70-f47a74380452/related/', }, }, -} \ No newline at end of file +} diff --git a/frontend/test/tapes/search/audio/q=birds&peaks=true_close.json5 b/frontend/test/tapes/search/audio/q=birds&peaks=true_close.json5 index 556e0e2f00d..0d40157c62a 100644 --- a/frontend/test/tapes/search/audio/q=birds&peaks=true_close.json5 +++ b/frontend/test/tapes/search/audio/q=birds&peaks=true_close.json5 @@ -84,44 +84,78 @@ page: 1, results: [ { - id: '2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95', - title: 'Hello Black & White Bird', - foreign_landing_url: 'https://www.jamendo.com/track/101464', - creator: 'The Old Dada Weatherman', - creator_url: 'https://www.jamendo.com/artist/6079/cynicalsense', - url: 'https://mp3d.jamendo.com/download/track/101464/mp32', - license: 'by-nc-sa', - license_version: '3.0', - license_url: 'https://creativecommons.org/licenses/by-nc-sa/3.0/', - provider: 'jamendo', - source: 'jamendo', - category: 'music', - tags: [ + "id": "2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95", + "title": "Hello Black & White Bird", + "indexed_on": "2022-02-24T01:34:42.275707Z", + "foreign_landing_url": "https://www.jamendo.com/track/101464", + "url": "https://prod-1.storage.jamendo.com/?trackid=101464&format=mp32", + "creator": "The Old Dada Weatherman", + "creator_url": "https://www.jamendo.com/artist/6079/cynicalsense", + "license": "by-nc-sa", + "license_version": "3.0", + "license_url": "https://creativecommons.org/licenses/by-nc-sa/3.0/", + "provider": "jamendo", + "source": "jamendo", + "category": "music", + "genres": [ + "pop", + "singersongwriter" + ], + "filesize": null, + "filetype": "mp32", + "tags": [ { - name: 'vocal', + "name": "acousticguitar", + "accuracy": null }, { - name: 'male', + "name": "happy", + "accuracy": null }, { - name: 'speed_medium', + "name": "male", + "accuracy": null }, { - name: 'happy', + "name": "sentimental", + "accuracy": null }, + { + "name": "speed_medium", + "accuracy": null + }, + { + "name": "strings", + "accuracy": null + }, + { + "name": "vocal", + "accuracy": null + } ], - fields_matched: [ - 'title', - ], - genres: [ - 'pop', + "alt_files": null, + "attribution": "\"Hello Black & White Bird\" by The Old Dada Weatherman is licensed under CC BY-NC-SA 3.0. To view a copy of this license, visit https://creativecommons.org/licenses/by-nc-sa/3.0/.", + "fields_matched": [ + "title" ], - duration: 206000, - thumbnail: 'http://localhost:49153/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/thumb/', - waveform: 'http://localhost:49153/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/waveform/', - detail_url: 'http://localhost:49153/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/', - related_url: 'http://localhost:49153/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/related/', - peaks: [], + "mature": false, + "audio_set": { + "title": "The Dada Weatherman (In Airplane Trouble)", + "foreign_landing_url": "https://www.jamendo.com/album/11348/the-dada-weatherman-in-airplane-trouble", + "creator": "The Old Dada Weatherman", + "creator_url": "https://www.jamendo.com/artist/6079/cynicalsense", + "url": null, + "filesize": null, + "filetype": null + }, + "duration": 206000, + "bit_rate": null, + "sample_rate": null, + "thumbnail": "https://api.openverse.engineering/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/thumb/", + "detail_url": "https://api.openverse.engineering/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/", + "related_url": "https://api.openverse.engineering/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/related/", + "waveform": "https://api.openverse.engineering/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/waveform/", + "unstable__sensitivity": [] }, { id: '99103ce4-2772-4f10-876e-e708a106251e', diff --git a/frontend/test/tapes/search/audio/q=birds&peaks=true_keep-alive.json5 b/frontend/test/tapes/search/audio/q=birds&peaks=true_keep-alive.json5 index 8c61695a499..99939f012f2 100644 --- a/frontend/test/tapes/search/audio/q=birds&peaks=true_keep-alive.json5 +++ b/frontend/test/tapes/search/audio/q=birds&peaks=true_keep-alive.json5 @@ -87,44 +87,78 @@ page: 1, results: [ { - id: '2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95', - title: 'Hello Black & White Bird', - foreign_landing_url: 'https://www.jamendo.com/track/101464', - creator: 'The Old Dada Weatherman', - creator_url: 'https://www.jamendo.com/artist/6079/cynicalsense', - url: 'https://mp3d.jamendo.com/download/track/101464/mp32', - license: 'by-nc-sa', - license_version: '3.0', - license_url: 'https://creativecommons.org/licenses/by-nc-sa/3.0/', - provider: 'jamendo', - source: 'jamendo', - category: 'music', - tags: [ + "id": "2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95", + "title": "Hello Black & White Bird", + "indexed_on": "2022-02-24T01:34:42.275707Z", + "foreign_landing_url": "https://www.jamendo.com/track/101464", + "url": "https://prod-1.storage.jamendo.com/?trackid=101464&format=mp32", + "creator": "The Old Dada Weatherman", + "creator_url": "https://www.jamendo.com/artist/6079/cynicalsense", + "license": "by-nc-sa", + "license_version": "3.0", + "license_url": "https://creativecommons.org/licenses/by-nc-sa/3.0/", + "provider": "jamendo", + "source": "jamendo", + "category": "music", + "genres": [ + "pop", + "singersongwriter" + ], + "filesize": null, + "filetype": "mp32", + "tags": [ { - name: 'vocal', + "name": "acousticguitar", + "accuracy": null }, { - name: 'male', + "name": "happy", + "accuracy": null }, { - name: 'speed_medium', + "name": "male", + "accuracy": null }, { - name: 'happy', + "name": "sentimental", + "accuracy": null }, + { + "name": "speed_medium", + "accuracy": null + }, + { + "name": "strings", + "accuracy": null + }, + { + "name": "vocal", + "accuracy": null + } ], - fields_matched: [ - 'title', - ], - genres: [ - 'pop', + "alt_files": null, + "attribution": "\"Hello Black & White Bird\" by The Old Dada Weatherman is licensed under CC BY-NC-SA 3.0. To view a copy of this license, visit https://creativecommons.org/licenses/by-nc-sa/3.0/.", + "fields_matched": [ + "title" ], - duration: 206000, - thumbnail: 'http://localhost:49153/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/thumb/', - waveform: 'http://localhost:49153/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/waveform/', - detail_url: 'http://localhost:49153/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/', - related_url: 'http://localhost:49153/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/related/', - peaks: [], + "mature": false, + "audio_set": { + "title": "The Dada Weatherman (In Airplane Trouble)", + "foreign_landing_url": "https://www.jamendo.com/album/11348/the-dada-weatherman-in-airplane-trouble", + "creator": "The Old Dada Weatherman", + "creator_url": "https://www.jamendo.com/artist/6079/cynicalsense", + "url": null, + "filesize": null, + "filetype": null + }, + "duration": 206000, + "bit_rate": null, + "sample_rate": null, + "thumbnail": "https://api.openverse.engineering/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/thumb/", + "detail_url": "https://api.openverse.engineering/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/", + "related_url": "https://api.openverse.engineering/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/related/", + "waveform": "https://api.openverse.engineering/v1/audio/2e38ac1e-830c-4e9c-b13d-2c9a1ad53f95/waveform/", + "unstable__sensitivity": [] }, { id: '99103ce4-2772-4f10-876e-e708a106251e', diff --git a/frontend/test/unit/specs/components/v-related-audios.spec.js b/frontend/test/unit/specs/components/v-related-audios.spec.js deleted file mode 100644 index 680ebf76be3..00000000000 --- a/frontend/test/unit/specs/components/v-related-audios.spec.js +++ /dev/null @@ -1,73 +0,0 @@ -import { fireEvent, screen } from "@testing-library/vue" - -import { render } from "~~/test/unit/test-utils/render" -import { getAudioObj } from "~~/test/unit/fixtures/audio" - -import { AUDIO } from "~/constants/media" -import { useAnalytics } from "~/composables/use-analytics" -import { useRelatedMediaStore } from "~/stores/media/related-media" -import { useSearchStore } from "~/stores/search" - -import VRelatedAudio from "~/components/VAudioDetails/VRelatedAudio.vue" - -const audioResults = [getAudioObj(), getAudioObj()] - -jest.mock("~/composables/use-analytics", () => ({ - useAnalytics: jest.fn(), -})) -describe("RelatedAudios", () => { - let options = { - propsData: { - media: audioResults, - fetchState: { isFetching: false, isError: false }, - }, - stubs: { LoadingIcon: true, VAudioThumbnail: true }, - } - let sendCustomEventMock = null - - beforeEach(() => { - sendCustomEventMock = jest.fn() - useAnalytics.mockImplementation(() => ({ - sendCustomEvent: sendCustomEventMock, - })) - }) - - it("should render content when finished loading related audios", async () => { - render(VRelatedAudio, options) - - screen.getByText(/related audio/i) - - expect(screen.queryAllByLabelText("Play")).toHaveLength( - // Two for each as the "row" layout rendered by VRelatedAudio - // renders a "large" and "small" version that are visually hidden by a parent - // depending on the breakpoint (but critically still rendered in the - // DOM) - audioResults.length * 2 - ) - }) - - it("should send SELECT_SEARCH_RESULT event when clicked", async () => { - const mainMediaId = "123" - const query = "cat" - const { queryAllByRole } = render( - VRelatedAudio, - options, - (localVue, options) => { - const relatedMediaStore = useRelatedMediaStore(options.pinia) - relatedMediaStore.$patch({ mainMediaId }) - useSearchStore(localVue.pinia).$patch({ searchTerm: query }) - } - ) - const audioLink = queryAllByRole("application")[0] - - await fireEvent.click(audioLink) - - expect(sendCustomEventMock).toHaveBeenCalledWith("SELECT_SEARCH_RESULT", { - id: audioResults[0].id, - mediaType: AUDIO, - query, - provider: audioResults[0].provider, - relatedTo: mainMediaId, - }) - }) -}) diff --git a/frontend/test/unit/specs/components/v-related-images.spec.js b/frontend/test/unit/specs/components/v-related-images.spec.js deleted file mode 100644 index b0635a5b47b..00000000000 --- a/frontend/test/unit/specs/components/v-related-images.spec.js +++ /dev/null @@ -1,79 +0,0 @@ -import { fireEvent, screen } from "@testing-library/vue" - -import { render } from "~~/test/unit/test-utils/render" - -import { IMAGE } from "~/constants/media" -import { useSearchStore } from "~/stores/search" -import { useRelatedMediaStore } from "~/stores/media/related-media" -import { useAnalytics } from "~/composables/use-analytics" - -import VRelatedImages from "~/components/VImageDetails/VRelatedImages.vue" - -jest.mock("~/composables/use-analytics", () => ({ - useAnalytics: jest.fn(), -})) - -const media = [ - { id: "img1", url: "https://wp.org/img1.jpg" }, - { id: "img2", url: "https://wp.org/img2.jpg" }, -] - -describe("RelatedImage", () => { - let props - let options - let sendCustomEventMock = null - beforeEach(() => { - props = { media, fetchState: { isFetching: false } } - options = { - propsData: props, - stubs: ["VLicense"], - } - sendCustomEventMock = jest.fn() - useAnalytics.mockImplementation(() => ({ - sendCustomEvent: sendCustomEventMock, - })) - }) - it("should render an image grid", () => { - render(VRelatedImages, options) - - expect( - screen.getAllByRole("heading", { name: /related images/i }) - ).toHaveLength(1) - expect(screen.queryAllByRole("heading").length).toEqual(3) - expect(screen.queryAllByRole("img").length).toEqual(2) - expect(screen.queryAllByRole("figure").length).toEqual(2) - }) - - it("should not render data when media array is empty", () => { - options.propsData.media = [] - render(VRelatedImages, options) - expect(screen.getByRole("heading").textContent).toContain("Related images") - expect(screen.queryAllByRole("img").length).toEqual(0) - }) - - it("should send SELECT_SEARCH_RESULT event when clicked", async () => { - const mainMediaId = "123" - const query = "cat" - - const { queryAllByRole } = render( - VRelatedImages, - options, - (localVue, options) => { - const relatedMediaStore = useRelatedMediaStore(options.pinia) - relatedMediaStore.$patch({ mainMediaId }) - useSearchStore(options.pinia).$patch({ searchTerm: query }) - } - ) - const imageLink = queryAllByRole("link")[0] - - await fireEvent.click(imageLink) - - expect(sendCustomEventMock).toHaveBeenCalledWith("SELECT_SEARCH_RESULT", { - id: media[0].id, - mediaType: IMAGE, - query, - provider: media[0].provider, - relatedTo: mainMediaId, - }) - }) -}) diff --git a/frontend/test/unit/specs/stores/single-result-store.spec.js b/frontend/test/unit/specs/stores/single-result-store.spec.js index d87e2cbbf64..a573c9d4a5e 100644 --- a/frontend/test/unit/specs/stores/single-result-store.spec.js +++ b/frontend/test/unit/specs/stores/single-result-store.spec.js @@ -1,12 +1,26 @@ import { createPinia, setActivePinia } from "~~/test/unit/test-utils/pinia" +import { getAudioObj } from "~~/test/unit/fixtures/audio" + +import { image as imageObj } from "~~/test/unit/fixtures/image" + import { AUDIO, IMAGE, supportedMediaTypes } from "~/constants/media" import { useMediaStore } from "~/stores/media" import { useSingleResultStore } from "~/stores/media/single-result" const detailData = { - [AUDIO]: { title: "audioDetails", id: "audio1", frontendMediaType: AUDIO }, - [IMAGE]: { title: "imageDetails", id: "image1", frontendMediaType: IMAGE }, + [AUDIO]: { + ...getAudioObj(), + title: "audioDetails", + id: "audio1", + frontendMediaType: AUDIO, + }, + [IMAGE]: { + ...imageObj, + title: "imageDetails", + id: "image1", + frontendMediaType: IMAGE, + }, } jest.mock("axios", () => ({ ...jest.requireActual("axios"), @@ -39,12 +53,23 @@ jest.mock("~/stores/media/services", () => ({ })) describe("Media Item Store", () => { + let singleResultStore = null + let mediaStore = null + const originalEnv = process.env + + beforeEach(() => { + setActivePinia(createPinia()) + singleResultStore = useSingleResultStore() + mediaStore = useMediaStore() + }) + afterEach(() => { + mockGetMediaDetailAudio.mockClear() + mockGetMediaDetailImage.mockClear() + process.env = originalEnv + }) describe("state", () => { it("sets default state", () => { - setActivePinia(createPinia()) - const singleResultStore = useSingleResultStore() expect(singleResultStore.fetchState).toEqual({ - hasStarted: false, isFetching: false, fetchingError: null, }) @@ -53,21 +78,74 @@ describe("Media Item Store", () => { }) }) + describe("getters", () => { + it.each(supportedMediaTypes)( + "%s getter returns the item when current item type matches", + (mediaType) => { + singleResultStore.$patch({ + mediaItem: detailData[mediaType], + mediaType, + mediaId: detailData[mediaType].id, + }) + expect(singleResultStore[mediaType]).toEqual(detailData[mediaType]) + } + ) + + it.each(supportedMediaTypes)( + "`%s` returns `null` if the media type doesn't match", + (mediaType) => { + singleResultStore.$patch({ + mediaItem: detailData[mediaType], + mediaType: mediaType, + mediaId: detailData[mediaType].id, + }) + expect( + singleResultStore[mediaType === "image" ? "audio" : "image"] + ).toEqual(null) + } + ) + }) + describe("actions", () => { - beforeEach(() => { - setActivePinia(createPinia()) - useMediaStore() + it.each(supportedMediaTypes)( + "setMediaItem (%s) sets the media item and media type", + (type) => { + const mediaItem = detailData[type] + singleResultStore.setMediaItem(mediaItem) + expect(singleResultStore.mediaItem).toEqual(mediaItem) + expect(singleResultStore.mediaType).toEqual(type) + expect(singleResultStore.mediaId).toEqual(mediaItem.id) + } + ) + it("setMediaItem(null) sets the media item to null", () => { + singleResultStore.setMediaItem(null) + expect(singleResultStore.mediaItem).toEqual(null) + expect(singleResultStore.mediaType).toEqual(null) + expect(singleResultStore.mediaId).toEqual(null) + }) + + it("setMediaById sets the media if it exists in the media store", () => { + const mediaItem = detailData[AUDIO] + mediaStore.results.audio.items = { [mediaItem.id]: mediaItem } + singleResultStore.setMediaById(AUDIO, mediaItem.id) + + expect(singleResultStore.mediaItem).toEqual(mediaItem) + expect(singleResultStore.mediaType).toEqual(AUDIO) + expect(singleResultStore.mediaId).toEqual(mediaItem.id) }) - afterEach(() => { - mockGetMediaDetailAudio.mockClear() - mockGetMediaDetailImage.mockClear() + + it("setMediaById sets the media id and type if it doesn't exist media store", () => { + const mediaItem = detailData[AUDIO] + singleResultStore.setMediaById(AUDIO, mediaItem.id) + + expect(singleResultStore.mediaItem).toEqual(null) + expect(singleResultStore.mediaType).toEqual(AUDIO) + expect(singleResultStore.mediaId).toEqual(mediaItem.id) }) it.each(supportedMediaTypes)( "fetchMediaItem (%s) fetches a new media if none is found in the store", async (type) => { - const singleResultStore = useSingleResultStore() - await singleResultStore.fetchMediaItem(type, "foo") expect(singleResultStore.mediaItem).toEqual(detailData[type]) } @@ -75,8 +153,6 @@ describe("Media Item Store", () => { it.each(supportedMediaTypes)( "fetchMediaItem (%s) re-uses existing media from the store", async (type) => { - const singleResultStore = useSingleResultStore() - const mediaStore = useMediaStore() mediaStore.results[type].items = { [`${type}1`]: detailData[type], } @@ -88,32 +164,65 @@ describe("Media Item Store", () => { it.each(supportedMediaTypes)( "fetchMediaItem throws not found error on request error", async (type) => { - const expectedErrorMessage = "error" + const errorMessage = "error" mocks[type].mockImplementationOnce(() => - Promise.reject(new Error(expectedErrorMessage)) + Promise.reject(new Error(errorMessage)) ) + const expectedError = { + message: `Error fetching single ${type} item with id foo`, + statusCode: 404, + } - const singleResultStore = useSingleResultStore() + await singleResultStore.fetchMediaItem(type, "foo") - await expect(() => - singleResultStore.fetchMediaItem(type, "foo") - ).rejects.toThrow(expectedErrorMessage) + expect(singleResultStore.fetchState.fetchingError).toEqual( + expectedError + ) } ) it.each(supportedMediaTypes)( "fetchMediaItem on 404 sets fetchingError and throws a new error", async (type) => { - mocks[type].mockImplementationOnce(() => - Promise.reject({ response: { status: 404 } }) - ) - const singleResultStore = useSingleResultStore() + const errorResponse = { response: { status: 404 } } + + mocks[type].mockImplementationOnce(() => Promise.reject(errorResponse)) const id = "foo" - await expect(() => - singleResultStore.fetchMediaItem(type, id) - ).rejects.toThrow(`Media of type ${type} with id ${id} not found`) + + const expectedError = { + message: `Error fetching single ${type} item with id ${id}`, + statusCode: errorResponse.response.status, + } + expect(await singleResultStore.fetch(type, id)).toEqual(null) + expect(singleResultStore.fetchState.fetchingError).toEqual( + expectedError + ) } ) + + it("`fetch` returns current item if it matches", async () => { + const mediaItem = detailData[AUDIO] + singleResultStore.$patch({ + mediaItem: mediaItem, + mediaType: AUDIO, + mediaId: mediaItem.id, + }) + expect(await singleResultStore.fetch(AUDIO, mediaItem.id)).toEqual( + mediaItem + ) + expect(mockGetMediaDetailAudio).not.toHaveBeenCalled() + }) + + it("`fetch` gets an item from a media store and fetches related media", async () => { + const mediaType = /** @type {SupportedMediaType} */ (IMAGE) + const expectedMediaItem = detailData[mediaType] + + mediaStore.results[IMAGE].items = { image1: expectedMediaItem } + const actual = await singleResultStore.fetch(IMAGE, expectedMediaItem.id) + + expect(actual).toEqual(expectedMediaItem) + expect(mockGetMediaDetailImage).not.toHaveBeenCalled() + }) }) }) diff --git a/tsconfig.json b/tsconfig.json index 5d3f48a59d0..4446c3cfbce 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -42,6 +42,9 @@ "frontend/src/pages/about.vue", "frontend/src/pages/feedback.vue", "frontend/src/pages/search-help.vue", + "frontend/src/pages/audio/_id/index.vue", + "frontend/src/pages/image/_id/index.vue", + "frontend/src/pages/image/_id/report.vue", /** * Some files from this section cannot be converted to TypeScript because they are * used in non-TS contexts (like Tailwind configuration). Therefore they're left as