Skip to content

Commit

Permalink
Fix fetching and types on single result pages (#2585)
Browse files Browse the repository at this point in the history
* Fix fetching and types on single result pages

* On SSR, fetch the image in middleware

* Fix unit test

* Use default img props if available

* Improve fetching

* Remove redundant Loading icon from Related images

We show the loading skeleton when the images are loading

* Fix report page

* Prevent Sentry from capturing the errors twice

Capture Sentry errors in components/middleware

* Move Sentry capturing to the store

* Update the unit test

* After-rebase linting

* Refactor unsafe call to useContext

* Add suggestion from code review

* Fix FetchState type

* Add changes from code review

* Update frontend/src/types/media.ts

Co-authored-by: sarayourfriend <[email protected]>

* Set `fetchOnServer` to false

* Simplify `fetch` and move error handling to `fetchMediaItem`

* Add tests

* Add CORS header to a tape

* Do not fetch related media if main media is null

* Update tapes

* Move related fetching to the component

* Update frontend/src/middleware/single-result.ts

Co-authored-by: sarayourfriend <[email protected]>

* Update CSR VR test

* Revert changes to sources tapes

---------

Co-authored-by: sarayourfriend <[email protected]>
  • Loading branch information
obulat and sarayourfriend authored Jul 15, 2023
1 parent dd49aea commit a5abdd6
Show file tree
Hide file tree
Showing 22 changed files with 684 additions and 549 deletions.
51 changes: 37 additions & 14 deletions frontend/src/components/VAudioDetails/VRelatedAudio.vue
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
<template>
<aside :aria-label="$t('audioDetails.relatedAudios').toString()">
<aside
v-if="showRelated"
:aria-label="$t('audioDetails.relatedAudios').toString()"
>
<h2 class="heading-6 lg:heading-6 mb-6">
{{ $t("audioDetails.relatedAudios") }}
</h2>
Expand Down Expand Up @@ -30,7 +33,9 @@
</template>

<script lang="ts">
import { computed, defineComponent, PropType } from "vue"
import { computed, defineComponent, watch } from "vue"
import { useRoute } from "@nuxtjs/composition-api"
import { useUiStore } from "~/stores/ui"
import { useSearchStore } from "~/stores/search"
Expand All @@ -40,7 +45,6 @@ import { useAnalytics } from "~/composables/use-analytics"
import { AUDIO } from "~/constants/media"
import { defineEvent } from "~/types/emits"
import type { FetchState } from "~/types/fetch-state"
import type { AudioDetail } from "~/types/media"
import type { AudioInteractionData } from "~/types/analytics"
Expand All @@ -50,27 +54,38 @@ import VAudioTrack from "~/components/VAudioTrack/VAudioTrack.vue"
export default defineComponent({
name: "VRelatedAudio",
components: { VAudioTrack, LoadingIcon },
props: {
media: {
type: Array as PropType<AudioDetail[]>,
required: true,
},
fetchState: {
type: Object as PropType<FetchState>,
required: true,
},
},
emits: {
interacted: defineEvent<[Omit<AudioInteractionData, "component">]>(),
},
setup() {
const uiStore = useUiStore()
const relatedMediaStore = useRelatedMediaStore()
const route = useRoute()
const audioTrackSize = computed(() => {
return uiStore.isBreakpoint("md") ? "l" : "s"
})
const media = computed(
() => (relatedMediaStore.media ?? []) as AudioDetail[]
)
watch(
route,
async (newRoute) => {
if (newRoute.params.id !== relatedMediaStore.mainMediaId) {
await relatedMediaStore.fetchMedia("audio", newRoute.params.id)
}
},
{ immediate: true }
)
const showRelated = computed(
() => media.value.length > 0 || relatedMediaStore.fetchState.isFetching
)
const fetchState = computed(() => relatedMediaStore.fetchState)
const { sendCustomEvent } = useAnalytics()
const sendSelectSearchResultEvent = (audio: AudioDetail) => {
sendCustomEvent("SELECT_SEARCH_RESULT", {
Expand All @@ -82,7 +97,15 @@ export default defineComponent({
})
}
return { audioTrackSize, sendSelectSearchResultEvent }
return {
media,
showRelated,
fetchState,
audioTrackSize,
sendSelectSearchResultEvent,
}
},
})
</script>
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
52 changes: 37 additions & 15 deletions frontend/src/components/VImageDetails/VRelatedImages.vue
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
<template>
<aside>
<aside v-if="showRelated">
<h2 class="heading-6 md:heading-5 mb-6">
{{ $t("imageDetails.relatedImages") }}
</h2>
<VLoadingIcon v-if="fetchState.isFetching" />
<VImageGrid
:images="media"
:is-single-page="true"
Expand All @@ -14,26 +13,49 @@
</template>

<script lang="ts">
import { defineComponent, PropType } from "vue"
import { computed, defineComponent, watch } from "vue"
import { useRoute } from "@nuxtjs/composition-api"
import type { ImageDetail } from "~/types/media"
import type { FetchState } from "~/types/fetch-state"
import { useRelatedMediaStore } from "~/stores/media/related-media"
import VImageGrid from "~/components/VSearchResultsGrid/VImageGrid.vue"
import VLoadingIcon from "~/components/LoadingIcon.vue"
export default defineComponent({
name: "VRelatedImages",
components: { VImageGrid, VLoadingIcon },
props: {
media: {
type: Array as PropType<ImageDetail[]>,
required: true,
},
fetchState: {
type: Object as PropType<FetchState>,
required: true,
},
components: { VImageGrid },
setup() {
const relatedMediaStore = useRelatedMediaStore()
const route = useRoute()
watch(
route,
async (newRoute) => {
if (newRoute.params.id !== relatedMediaStore.mainMediaId) {
await relatedMediaStore.fetchMedia("image", newRoute.params.id)
}
},
{ immediate: true }
)
const showRelated = computed(
() => media.value.length > 0 || relatedMediaStore.fetchState.isFetching
)
const media = computed(
() => (relatedMediaStore.media ?? []) as ImageDetail[]
)
const fetchState = computed(() => relatedMediaStore.fetchState)
return {
showRelated,
media,
fetchState,
}
},
})
</script>
6 changes: 4 additions & 2 deletions frontend/src/components/VSearchResultsGrid/VImageGrid.vue
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import VGridSkeleton from "~/components/VSkeleton/VGridSkeleton.vue"
import VLoadMore from "~/components/VLoadMore.vue"
import VImageCell from "~/components/VSearchResultsGrid/VImageCell.vue"
import type { NuxtError } from "@nuxt/types"
export default defineComponent({
name: "ImageGrid",
components: { VGridSkeleton, VLoadMore, VImageCell },
Expand All @@ -62,7 +64,7 @@ export default defineComponent({
required: true,
},
fetchState: {
type: Object as PropType<FetchState>,
type: Object as PropType<FetchState<NuxtError>>,
required: true,
},
imageGridLabel: {
Expand All @@ -74,7 +76,7 @@ export default defineComponent({
const searchStore = useSearchStore()
const searchTerm = computed(() => searchStore.searchTerm)
const isError = computed(() => Boolean(props.fetchState.fetchingError))
const isError = computed(() => props.fetchState.fetchingError !== null)
const relatedTo = computed(() => {
return props.isSinglePage ? useRelatedMediaStore().mainMediaId : null
Expand Down
44 changes: 35 additions & 9 deletions frontend/src/middleware/single-result.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,42 @@
import { useSingleResultStore } from "~/stores/media/single-result"
import { useSearchStore } from "~/stores/search"
import { useRelatedMediaStore } from "~/stores/media/related-media"

import { AUDIO, IMAGE } from "~/constants/media"

import type { Middleware } from "@nuxt/types"

export const singleResultMiddleware: Middleware = ({ route, from, $pinia }) => {
if (from && from.path.includes("/search/")) {
const searchStore = useSearchStore($pinia)
searchStore.setBackToSearchPath(from.fullPath)
const searchTerm = Array.isArray(route.query.q)
? route.query.q[0]
: route.query.q
if (searchTerm) {
searchStore.setSearchTerm(searchTerm)
export const singleResultMiddleware: Middleware = async ({
route,
from,
error,
$pinia,
}) => {
const mediaType = route.fullPath.includes("/image/") ? IMAGE : AUDIO
const singleResultStore = useSingleResultStore($pinia)

if (process.server) {
const media = await singleResultStore.fetch(mediaType, route.params.id)
await useRelatedMediaStore($pinia).fetchMedia(mediaType, route.params.id)

if (!media) {
error(singleResultStore.fetchState.fetchingError ?? {})
}
} else {
// Client-side rendering
singleResultStore.setMediaById(mediaType, route.params.id)

if (from && from.path.includes("/search/")) {
const searchStore = useSearchStore($pinia)
searchStore.setBackToSearchPath(from.fullPath)

const searchTerm = Array.isArray(route.query.q)
? route.query.q[0]
: route.query.q

if (searchTerm) {
searchStore.setSearchTerm(searchTerm)
}
}
}
}
68 changes: 30 additions & 38 deletions frontend/src/pages/audio/_id/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,28 @@
>
<VMediaReuse data-testid="audio-attribution" :media="audio" />
<VAudioDetails data-testid="audio-info" :audio="audio" />
<VRelatedAudio
v-if="relatedMedia.length || relatedFetchState.isFetching"
:media="relatedMedia"
:fetch-state="relatedFetchState"
@interacted="sendAudioEvent($event, 'VRelatedAudio')"
/>
<VRelatedAudio @interacted="sendAudioEvent($event, 'VRelatedAudio')" />
</div>
</template>
</main>
</template>

<script lang="ts">
import { computed } from "vue"
import { defineComponent, useMeta } from "@nuxtjs/composition-api"
import { computed, ref } from "vue"
import {
defineComponent,
useContext,
useFetch,
useMeta,
useRoute,
} from "@nuxtjs/composition-api"
import { AUDIO } from "~/constants/media"
import { skipToContentTargetId } from "~/constants/window"
import type { AudioDetail } from "~/types/media"
import type { AudioInteractionData } from "~/types/analytics"
import { useAnalytics } from "~/composables/use-analytics"
import { singleResultMiddleware } from "~/middleware/single-result"
import { useRelatedMediaStore } from "~/stores/media/related-media"
import { useSingleResultStore } from "~/stores/media/single-result"
import { useSearchStore } from "~/stores/search"
import { createDetailPageMeta } from "~/utils/og"
Expand All @@ -62,24 +62,34 @@ 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 { sendCustomEvent } = useAnalytics()
const route = useRoute()
const audio = ref<AudioDetail | null>(singleResultStore.audio)
const { error: nuxtError } = useContext()
useFetch(async () => {
const audioId = route.value.params.id
await singleResultStore.fetch(AUDIO, audioId)
const fetchedAudio = singleResultStore.audio
if (!fetchedAudio) {
nuxtError(singleResultStore.fetchState.fetchingError ?? {})
} else {
audio.value = fetchedAudio
}
})
const audio = computed(() =>
singleResultStore.mediaType === AUDIO
? (singleResultStore.mediaItem as AudioDetail)
: null
)
const relatedMedia = computed(
() => relatedMediaStore.media as AudioDetail[]
)
const relatedFetchState = computed(() => relatedMediaStore.fetchState)
const backToSearchPath = computed(() => searchStore.backToSearchPath)
const { sendCustomEvent } = useAnalytics()
const sendAudioEvent = (
data: Omit<AudioInteractionData, "component">,
component: "AudioDetailPage" | "VRelatedAudio"
Expand All @@ -95,30 +105,12 @@ export default defineComponent({
return {
audio,
backToSearchPath,
relatedMedia,
relatedFetchState,
sendAudioEvent,
skipToContentTargetId,
}
},
async asyncData({ route, error, app, $pinia }) {
const audioId = route.params.id
const singleResultStore = useSingleResultStore($pinia)
try {
await singleResultStore.fetch(AUDIO, audioId)
} catch (err) {
error({
statusCode: 404,
message: app.i18n.t("error.media-not-found", {
mediaType: AUDIO,
id: route.params.id,
}),
})
}
},
head: {},
})
</script>
Expand Down
Loading

0 comments on commit a5abdd6

Please sign in to comment.