From 99006e29da244f9d215a93f38d7160d8d66b116b Mon Sep 17 00:00:00 2001 From: thomasgross Date: Wed, 11 Dec 2024 15:40:33 +0100 Subject: [PATCH 1/2] fix: issue where calls are made to the API with no identifier on useGetV1ProductsIdentifier + add tests + cleanup and refactor related logic Co-authored-by: Mikkel Jakobsen --- __tests__/helper.test.ts | 82 +++++++++++++++++++ .../pages/workPageLayout/WorkPageHeader.tsx | 16 ++-- components/pages/workPageLayout/helper.ts | 8 -- components/shared/workCard/WorkCard.tsx | 8 +- lib/graphql/generated/fbi/graphql.tsx | 12 +-- lib/helpers/ids.ts | 14 +++- lib/rest/publizon-api/generated/publizon.ts | 2 +- store/selectedManifestation.store.ts | 2 +- 8 files changed, 112 insertions(+), 32 deletions(-) create mode 100644 __tests__/helper.test.ts diff --git a/__tests__/helper.test.ts b/__tests__/helper.test.ts new file mode 100644 index 00000000..0231b43c --- /dev/null +++ b/__tests__/helper.test.ts @@ -0,0 +1,82 @@ +import { expect, test } from "vitest" + +import { + ManifestationIdentifiersFragment, + WorkTeaserSearchPageFragment, +} from "@/lib/graphql/generated/fbi/graphql" +import { getIsbnsFromManifestation, getIsbnsFromWork } from "@/lib/helpers/ids" + +test("test that we can get isbns from manifestation", async () => { + const manifestation = { + pid: "870970-basis:29142246", + identifiers: [ + { + type: "PUBLIZON", + value: "9788711402740", + }, + { + type: "ISBN", + value: "9788711402740", + }, + { + type: "ISBN", + value: "9788711402742", + }, + ], + } as ManifestationIdentifiersFragment + + const isbns = getIsbnsFromManifestation(manifestation) + + expect(isbns).toStrictEqual(["9788711402740", "9788711402742"]) +}) + +test("test that we get empty isbns when only having publizon identifiers", async () => { + const manifestation = { + pid: "870970-basis:29142246", + identifiers: [ + { + type: "PUBLIZON", + value: "9788711402740", + }, + ], + } as ManifestationIdentifiersFragment + + const isbns = getIsbnsFromManifestation(manifestation) + + expect(isbns).toStrictEqual([]) +}) + +test("test that we get an array of isbns string from work object", async () => { + const work = { + manifestations: { + all: [ + { + pid: "870970-basis:29142246", + identifiers: [ + { + type: "PUBLIZON", + value: "9788711402740", + }, + { + type: "ISBN", + value: "9788711402740", + }, + ], + materialTypes: [ + { + materialTypeGeneral: { + display: "e-bøger", + code: "EBOOKS", + }, + }, + ], + }, + ], + bestRepresentation: {}, + }, + } as Pick + + const isbns = getIsbnsFromWork(work as WorkTeaserSearchPageFragment) + + expect(isbns).toStrictEqual(["9788711402740"]) +}) diff --git a/components/pages/workPageLayout/WorkPageHeader.tsx b/components/pages/workPageLayout/WorkPageHeader.tsx index de9a1928..79758479 100644 --- a/components/pages/workPageLayout/WorkPageHeader.tsx +++ b/components/pages/workPageLayout/WorkPageHeader.tsx @@ -1,3 +1,5 @@ +"use client" + import { motion } from "framer-motion" import { useRouter, useSearchParams } from "next/navigation" import React, { useEffect, useState } from "react" @@ -8,6 +10,7 @@ import SlideSelect, { SlideSelectOption } from "@/components/shared/slideSelect/ import { displayCreators } from "@/components/shared/workCard/helper" import { WorkFullWorkPageFragment } from "@/lib/graphql/generated/fbi/graphql" import { getCoverUrls, getLowResCoverUrl } from "@/lib/helpers/covers" +import { getIsbnsFromManifestation } from "@/lib/helpers/ids" import { useGetCoverCollection } from "@/lib/rest/cover-service-api/generated/cover-service" import { GetCoverCollectionSizesItem } from "@/lib/rest/cover-service-api/generated/model" import { useGetV1ProductsIdentifier } from "@/lib/rest/publizon-api/generated/publizon" @@ -17,7 +20,6 @@ import WorkPageButtons from "./WorkPageButtons" import { addMaterialTypeIconToSelectOption, findInitialSliderValue, - getIsbnsFromManifestation, getManifestationByMaterialType, getManifestationLanguageIsoCode, getWorkMaterialTypes, @@ -32,10 +34,10 @@ const WorkPageHeader = ({ work }: WorkPageHeaderProps) => { const router = useRouter() const { selectedManifestation, setSelectedManifestation } = useSelectedManifestationStore() const [slideSelectOptions, setSlideSelectOptions] = useState(null) - const isbns = getIsbnsFromManifestation(selectedManifestation) + const isbns = selectedManifestation ? getIsbnsFromManifestation(selectedManifestation) : [] const languageIsoCode = getManifestationLanguageIsoCode(selectedManifestation) const titleSuffix = selectedManifestation?.titles?.identifyingAddition || "" - const [initialSliderValue, setinitialSliderValue] = useState( + const [initialSliderValue, setInitialSliderValue] = useState( undefined ) const workMaterialTypes = getWorkMaterialTypes(work).map(materialType => { @@ -54,11 +56,11 @@ const WorkPageHeader = ({ work }: WorkPageHeaderProps) => { { query: { enabled: !!selectedManifestation?.pid } } ) - const { data: dataPublizon } = useGetV1ProductsIdentifier(isbns[0]?.value || "", { + const { data: publizonData } = useGetV1ProductsIdentifier(isbns?.[0], { query: { // Publizon / useGetV1ProductsIdentifier is responsible for online // materials. It requires an ISBN to do lookups. - enabled: isbns && isbns.length > 0, + enabled: isbns.length > 0, }, }) @@ -92,7 +94,7 @@ const WorkPageHeader = ({ work }: WorkPageHeaderProps) => { useEffect(() => { // Initialize slideSelect initial value const searchParams = new URLSearchParams(window.location.search) - setinitialSliderValue( + setInitialSliderValue( findInitialSliderValue(slideSelectOptions, selectedManifestation, searchParams) ) }, [selectedManifestation, slideSelectOptions]) @@ -137,7 +139,7 @@ const WorkPageHeader = ({ work }: WorkPageHeaderProps) => { )}
- {!!dataPublizon?.product?.costFree && ( + {!!publizonData?.product?.costFree && ( BLÅ diff --git a/components/pages/workPageLayout/helper.ts b/components/pages/workPageLayout/helper.ts index 6726aa7e..d62d0fea 100644 --- a/components/pages/workPageLayout/helper.ts +++ b/components/pages/workPageLayout/helper.ts @@ -4,7 +4,6 @@ import { SlideSelectOption } from "@/components/shared/slideSelect/SlideSelect" import { materialTypeCategories } from "@/components/shared/workCard/helper" import { GeneralMaterialTypeCodeEnum, - IdentifierTypeEnum, ManifestationWorkPageFragment, WorkFullWorkPageFragment, WorkMaterialTypesFragment, @@ -77,13 +76,6 @@ export const isPodcast = (manifestation: ManifestationWorkPageFragment | undefin return isOfMaterialType(manifestation, GeneralMaterialTypeCodeEnum.Podcasts) } -export const getIsbnsFromManifestation = ( - manifestaion: ManifestationWorkPageFragment | undefined | null -) => { - if (!manifestaion) return [] - return manifestaion.identifiers.filter(identifier => identifier.type === IdentifierTypeEnum.Isbn) -} - export const getManifestationLanguageIsoCode = ( manifestation: ManifestationWorkPageFragment | undefined | null ) => { diff --git a/components/shared/workCard/WorkCard.tsx b/components/shared/workCard/WorkCard.tsx index 24204d63..7560df31 100644 --- a/components/shared/workCard/WorkCard.tsx +++ b/components/shared/workCard/WorkCard.tsx @@ -27,14 +27,12 @@ const WorkCard = ({ work }: WorkCardProps) => { ], }) const isbns = getIsbnsFromWork(work) - const shouldQueryBeEnabled = () => { - return isbns && isbns.length > 0 - } - const { data: dataPublizon } = useGetV1ProductsIdentifier(isbns[0] || "", { + + const { data: dataPublizon } = useGetV1ProductsIdentifier(isbns[0], { query: { // Publizon / useGetV1ProductsIdentifier is responsible for online // materials. It requires an ISBN to do lookups. - enabled: shouldQueryBeEnabled(), + enabled: isbns.length > 0, }, }) diff --git a/lib/graphql/generated/fbi/graphql.tsx b/lib/graphql/generated/fbi/graphql.tsx index 9841e287..6dcba078 100644 --- a/lib/graphql/generated/fbi/graphql.tsx +++ b/lib/graphql/generated/fbi/graphql.tsx @@ -2179,7 +2179,7 @@ export const useSearchWithPaginationQuery = < variables: SearchWithPaginationQueryVariables, options?: Omit, 'queryKey'> & { queryKey?: UseQueryOptions['queryKey'] } ) => { - + return useQuery( { queryKey: ['searchWithPagination', variables], @@ -2197,7 +2197,7 @@ export const useSuspenseSearchWithPaginationQuery = < variables: SearchWithPaginationQueryVariables, options?: Omit, 'queryKey'> & { queryKey?: UseSuspenseQueryOptions['queryKey'] } ) => { - + return useSuspenseQuery( { queryKey: ['searchWithPaginationSuspense', variables], @@ -2228,7 +2228,7 @@ export const useSearchFacetsQuery = < variables: SearchFacetsQueryVariables, options?: Omit, 'queryKey'> & { queryKey?: UseQueryOptions['queryKey'] } ) => { - + return useQuery( { queryKey: ['searchFacets', variables], @@ -2246,7 +2246,7 @@ export const useSuspenseSearchFacetsQuery = < variables: SearchFacetsQueryVariables, options?: Omit, 'queryKey'> & { queryKey?: UseSuspenseQueryOptions['queryKey'] } ) => { - + return useSuspenseQuery( { queryKey: ['searchFacetsSuspense', variables], @@ -2275,7 +2275,7 @@ export const useGetMaterialQuery = < variables: GetMaterialQueryVariables, options?: Omit, 'queryKey'> & { queryKey?: UseQueryOptions['queryKey'] } ) => { - + return useQuery( { queryKey: ['getMaterial', variables], @@ -2293,7 +2293,7 @@ export const useSuspenseGetMaterialQuery = < variables: GetMaterialQueryVariables, options?: Omit, 'queryKey'> & { queryKey?: UseSuspenseQueryOptions['queryKey'] } ) => { - + return useSuspenseQuery( { queryKey: ['getMaterialSuspense', variables], diff --git a/lib/helpers/ids.ts b/lib/helpers/ids.ts index cf33c287..d589556e 100644 --- a/lib/helpers/ids.ts +++ b/lib/helpers/ids.ts @@ -3,16 +3,22 @@ import { flatten } from "lodash" import { IdentifierTypeEnum, ManifestationIdentifiersFragment, + ManifestationWorkPageFragment, WorkTeaserSearchPageFragment, } from "../graphql/generated/fbi/graphql" import { filterFalsyValuesFromArray } from "./arrays" -export const getIsbnsFromManifestation = (manifestation: ManifestationIdentifiersFragment) => { - return manifestation.identifiers.map(identifier => { +export const getIsbnsFromManifestation = ( + manifestation: ManifestationIdentifiersFragment | ManifestationWorkPageFragment | null +) => { + if (!manifestation) return [] + + return manifestation.identifiers.reduce((acc, identifier) => { if (identifier.type === IdentifierTypeEnum.Isbn) { - return identifier.value + acc.push(identifier.value) } - }) + return acc + }, [] as string[]) } export const getIsbnsFromWork = (work: WorkTeaserSearchPageFragment) => { diff --git a/lib/rest/publizon-api/generated/publizon.ts b/lib/rest/publizon-api/generated/publizon.ts index 9c6a8033..5ff6c0bf 100644 --- a/lib/rest/publizon-api/generated/publizon.ts +++ b/lib/rest/publizon-api/generated/publizon.ts @@ -42,7 +42,7 @@ import type { /** * Sample **request**: - + GET /friendly { "name": "Some name" diff --git a/store/selectedManifestation.store.ts b/store/selectedManifestation.store.ts index 8d7f581b..b1e80817 100644 --- a/store/selectedManifestation.store.ts +++ b/store/selectedManifestation.store.ts @@ -21,7 +21,7 @@ const useSelectedManifestationStore = create()( })), }), { - name: "theme-storage", + name: "manifistation-storage", } ) ) From 6136702193c4cb47207f2a9f685073673f6940ea Mon Sep 17 00:00:00 2001 From: thomasgross Date: Thu, 12 Dec 2024 14:58:08 +0100 Subject: [PATCH 2/2] refactor: rename test to be more concise --- __tests__/{helper.test.ts => isbnsHelper.test.ts} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename __tests__/{helper.test.ts => isbnsHelper.test.ts} (100%) diff --git a/__tests__/helper.test.ts b/__tests__/isbnsHelper.test.ts similarity index 100% rename from __tests__/helper.test.ts rename to __tests__/isbnsHelper.test.ts