Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix fetching and types on single result pages #2585

Merged
merged 26 commits into from
Jul 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8892539
Fix fetching and types on single result pages
obulat Jul 7, 2023
0987208
On SSR, fetch the image in middleware
obulat Jul 7, 2023
4155abd
Fix unit test
obulat Jul 7, 2023
fd84ca8
Use default img props if available
obulat Jul 8, 2023
cbae5b2
Improve fetching
obulat Jul 9, 2023
8f2e6d3
Remove redundant Loading icon from Related images
obulat Jul 9, 2023
fdf4807
Fix report page
obulat Jul 8, 2023
f0b9215
Prevent Sentry from capturing the errors twice
obulat Jul 10, 2023
7798b23
Move Sentry capturing to the store
obulat Jul 11, 2023
928c357
Update the unit test
obulat Jul 11, 2023
01f83e1
After-rebase linting
obulat Jul 12, 2023
5edb13b
Refactor unsafe call to useContext
obulat Jul 12, 2023
9687d40
Add suggestion from code review
obulat Jul 12, 2023
164caec
Fix FetchState type
obulat Jul 12, 2023
23829f2
Add changes from code review
obulat Jul 12, 2023
da02843
Update frontend/src/types/media.ts
obulat Jul 12, 2023
e43868a
Set `fetchOnServer` to false
obulat Jul 13, 2023
9051722
Simplify `fetch` and move error handling to `fetchMediaItem`
obulat Jul 13, 2023
2a6a68a
Add tests
obulat Jul 13, 2023
0fee695
Add CORS header to a tape
obulat Jul 13, 2023
0a1571c
Do not fetch related media if main media is null
obulat Jul 13, 2023
24bc949
Update tapes
obulat Jul 13, 2023
b56490d
Move related fetching to the component
obulat Jul 13, 2023
a888b23
Update frontend/src/middleware/single-result.ts
obulat Jul 14, 2023
af26d35
Update CSR VR test
obulat Jul 14, 2023
2c707a9
Revert changes to sources tapes
obulat Jul 15, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
obulat marked this conversation as resolved.
Show resolved Hide resolved
}
},
})
</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
Loading