Skip to content

Commit

Permalink
Merge pull request #76 from danskernesdigitalebibliotek/DDFBRA-263-vi…
Browse files Browse the repository at this point in the history
…-skaber-performance-problemer-hos-dbc

fix: issue where calls are made to the API with no identifier on useGetV1ProductsIdentifier + add tests + cleanup and refactor related logic
  • Loading branch information
ThomasGross authored Dec 12, 2024
2 parents 0f5fb0d + 6136702 commit 7a2526b
Show file tree
Hide file tree
Showing 8 changed files with 112 additions and 32 deletions.
82 changes: 82 additions & 0 deletions __tests__/isbnsHelper.test.ts
Original file line number Diff line number Diff line change
@@ -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<WorkTeaserSearchPageFragment, "manifestations">

const isbns = getIsbnsFromWork(work as WorkTeaserSearchPageFragment)

expect(isbns).toStrictEqual(["9788711402740"])
})
16 changes: 9 additions & 7 deletions components/pages/workPageLayout/WorkPageHeader.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
"use client"

import { motion } from "framer-motion"
import { useRouter, useSearchParams } from "next/navigation"
import React, { useEffect, useState } from "react"
Expand All @@ -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"
Expand All @@ -17,7 +20,6 @@ import WorkPageButtons from "./WorkPageButtons"
import {
addMaterialTypeIconToSelectOption,
findInitialSliderValue,
getIsbnsFromManifestation,
getManifestationByMaterialType,
getManifestationLanguageIsoCode,
getWorkMaterialTypes,
Expand All @@ -32,10 +34,10 @@ const WorkPageHeader = ({ work }: WorkPageHeaderProps) => {
const router = useRouter()
const { selectedManifestation, setSelectedManifestation } = useSelectedManifestationStore()
const [slideSelectOptions, setSlideSelectOptions] = useState<SlideSelectOption[] | null>(null)
const isbns = getIsbnsFromManifestation(selectedManifestation)
const isbns = selectedManifestation ? getIsbnsFromManifestation(selectedManifestation) : []
const languageIsoCode = getManifestationLanguageIsoCode(selectedManifestation)
const titleSuffix = selectedManifestation?.titles?.identifyingAddition || ""
const [initialSliderValue, setinitialSliderValue] = useState<SlideSelectOption | undefined>(
const [initialSliderValue, setInitialSliderValue] = useState<SlideSelectOption | undefined>(
undefined
)
const workMaterialTypes = getWorkMaterialTypes(work).map(materialType => {
Expand All @@ -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,
},
})

Expand Down Expand Up @@ -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])
Expand Down Expand Up @@ -137,7 +139,7 @@ const WorkPageHeader = ({ work }: WorkPageHeaderProps) => {
)}
</div>
<div className="col-span-4 flex flex-col items-start justify-end pt-grid-gap-3 lg:pt-0">
{!!dataPublizon?.product?.costFree && (
{!!publizonData?.product?.costFree && (
<Badge variant={"blue-title"} className="mb-1 lg:mb-2">
BLÅ
</Badge>
Expand Down
8 changes: 0 additions & 8 deletions components/pages/workPageLayout/helper.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { SlideSelectOption } from "@/components/shared/slideSelect/SlideSelect"
import { materialTypeCategories } from "@/components/shared/workCard/helper"
import {
GeneralMaterialTypeCodeEnum,
IdentifierTypeEnum,
ManifestationWorkPageFragment,
WorkFullWorkPageFragment,
WorkMaterialTypesFragment,
Expand Down Expand Up @@ -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
) => {
Expand Down
8 changes: 3 additions & 5 deletions components/shared/workCard/WorkCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
})

Expand Down
12 changes: 6 additions & 6 deletions lib/graphql/generated/fbi/graphql.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2179,7 +2179,7 @@ export const useSearchWithPaginationQuery = <
variables: SearchWithPaginationQueryVariables,
options?: Omit<UseQueryOptions<SearchWithPaginationQuery, TError, TData>, 'queryKey'> & { queryKey?: UseQueryOptions<SearchWithPaginationQuery, TError, TData>['queryKey'] }
) => {

return useQuery<SearchWithPaginationQuery, TError, TData>(
{
queryKey: ['searchWithPagination', variables],
Expand All @@ -2197,7 +2197,7 @@ export const useSuspenseSearchWithPaginationQuery = <
variables: SearchWithPaginationQueryVariables,
options?: Omit<UseSuspenseQueryOptions<SearchWithPaginationQuery, TError, TData>, 'queryKey'> & { queryKey?: UseSuspenseQueryOptions<SearchWithPaginationQuery, TError, TData>['queryKey'] }
) => {

return useSuspenseQuery<SearchWithPaginationQuery, TError, TData>(
{
queryKey: ['searchWithPaginationSuspense', variables],
Expand Down Expand Up @@ -2228,7 +2228,7 @@ export const useSearchFacetsQuery = <
variables: SearchFacetsQueryVariables,
options?: Omit<UseQueryOptions<SearchFacetsQuery, TError, TData>, 'queryKey'> & { queryKey?: UseQueryOptions<SearchFacetsQuery, TError, TData>['queryKey'] }
) => {

return useQuery<SearchFacetsQuery, TError, TData>(
{
queryKey: ['searchFacets', variables],
Expand All @@ -2246,7 +2246,7 @@ export const useSuspenseSearchFacetsQuery = <
variables: SearchFacetsQueryVariables,
options?: Omit<UseSuspenseQueryOptions<SearchFacetsQuery, TError, TData>, 'queryKey'> & { queryKey?: UseSuspenseQueryOptions<SearchFacetsQuery, TError, TData>['queryKey'] }
) => {

return useSuspenseQuery<SearchFacetsQuery, TError, TData>(
{
queryKey: ['searchFacetsSuspense', variables],
Expand Down Expand Up @@ -2275,7 +2275,7 @@ export const useGetMaterialQuery = <
variables: GetMaterialQueryVariables,
options?: Omit<UseQueryOptions<GetMaterialQuery, TError, TData>, 'queryKey'> & { queryKey?: UseQueryOptions<GetMaterialQuery, TError, TData>['queryKey'] }
) => {

return useQuery<GetMaterialQuery, TError, TData>(
{
queryKey: ['getMaterial', variables],
Expand All @@ -2293,7 +2293,7 @@ export const useSuspenseGetMaterialQuery = <
variables: GetMaterialQueryVariables,
options?: Omit<UseSuspenseQueryOptions<GetMaterialQuery, TError, TData>, 'queryKey'> & { queryKey?: UseSuspenseQueryOptions<GetMaterialQuery, TError, TData>['queryKey'] }
) => {

return useSuspenseQuery<GetMaterialQuery, TError, TData>(
{
queryKey: ['getMaterialSuspense', variables],
Expand Down
14 changes: 10 additions & 4 deletions lib/helpers/ids.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion lib/rest/publizon-api/generated/publizon.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ import type {

/**
* Sample **request**:
GET /friendly
{
"name": "Some name"
Expand Down
2 changes: 1 addition & 1 deletion store/selectedManifestation.store.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ const useSelectedManifestationStore = create<SelectedManifestationState>()(
})),
}),
{
name: "theme-storage",
name: "manifistation-storage",
}
)
)
Expand Down

0 comments on commit 7a2526b

Please sign in to comment.