From 06f52969d9ca0cbff71c18a688e8776ca955283e Mon Sep 17 00:00:00 2001 From: Mikkel Jakobsen Date: Fri, 26 Jul 2024 10:25:53 +0200 Subject: [PATCH 1/6] Increase readability of getBlacklistedQueryArgs functionality --- src/apps/material/helper.ts | 61 ++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 17 deletions(-) diff --git a/src/apps/material/helper.ts b/src/apps/material/helper.ts index 5512349ea4..0295f21e06 100644 --- a/src/apps/material/helper.ts +++ b/src/apps/material/helper.ts @@ -475,32 +475,59 @@ export const isParallelReservation = (manifestations: Manifestation[]) => hasCorrectAccessType(AccessTypeCode.Physical, manifestations) && !isArticle(manifestations); +type BlacklistType = "availability" | "pickup" | "both"; + +const formatBranches = (branches: string[][]) => { + return branches.flat().length ? { exclude: branches.flat() } : {}; +}; + +const branchesFromConfig = ( + blacklist: BlacklistType, + config: UseConfigFunction +) => { + const configMap: Record, string> = { + availability: "blacklistedAvailabilityBranchesConfig", + pickup: "blacklistedPickupBranchesConfig" + }; + + type ConfigMapKey = keyof typeof configMap; + + if (!configMap[blacklist as ConfigMapKey]) { + return []; + } + + return config(configMap[blacklist as ConfigMapKey], { + transformer: "stringToArray" + }).filter((branch) => branch); +}; + // Because we need to exclude the branches that are blacklisted, we need to // use a custom hook to prevent duplicate code export const getBlacklistedQueryArgs = ( faustIds: FaustId[], config: UseConfigFunction, - blacklist: "availability" | "pickup" | "both" + blacklistType: BlacklistType ) => { - const configKey = { - availability: "blacklistedAvailabilityBranchesConfig", - pickup: "blacklistedPickupBranchesConfig", - both: "blacklistedAvailabilityBranchesConfig" + // Fixed arguments for the query. + const args = { + recordid: faustIds }; - let blacklistBranches = config(configKey[blacklist], { - transformer: "stringToArray" - }); - // If we want to blacklist both availability and pickup branches we now add the - // complimentary blacklist - if (blacklist === "both") { - const additionalBlacklistBranches = config(configKey.pickup, { - transformer: "stringToArray" - }); - blacklistBranches = blacklistBranches.concat(additionalBlacklistBranches); + // Return query args with the either availability or pickup branches excluded. + if (blacklistType !== "both") { + return { + ...args, + ...formatBranches([branchesFromConfig(blacklistType, config)]) + }; } + + // If we want to blacklist both availability and pickup branches + // return query args with both blacklist types excluded. return { - recordid: faustIds, - ...(blacklistBranches ? { exclude: blacklistBranches } : {}) + ...args, + ...formatBranches([ + branchesFromConfig("availability", config), + branchesFromConfig("pickup", config) + ]) }; }; From 428078727952094ef1735d3b5feee154343038cf Mon Sep 17 00:00:00 2001 From: Mikkel Jakobsen Date: Fri, 26 Jul 2024 15:30:44 +0200 Subject: [PATCH 2/6] Split availability hook into two separate hooks Two major factors to be aware of here: * Three hooks are introduced instead of one: One main hook and one for physical and one for online materials, to get a better overview of the rules and the logic. * Apparently we cannot make use of `onSuccess` handlers on queries if the request have alredy been performed and the data is delivered the second time by the `QueryClient` cache. And that is the case of the availability labels on a material page, where they are used both in the header and in the "udagver" section later on the page. --- .../availability-label/availability-label.tsx | 3 +- src/components/availability-label/helper.ts | 142 +----------------- .../availability-label/useAvailabilityData.ts | 44 ++++++ .../useOnlineAvailabilityData.ts | 99 ++++++++++++ .../usePhysicalAvailabilityData.ts | 65 ++++++++ .../material/MaterialMainfestationItem.tsx | 1 + src/core/utils/useGetAvailability.ts | 4 +- 7 files changed, 217 insertions(+), 141 deletions(-) create mode 100644 src/components/availability-label/useAvailabilityData.ts create mode 100644 src/components/availability-label/useOnlineAvailabilityData.ts create mode 100644 src/components/availability-label/usePhysicalAvailabilityData.ts diff --git a/src/components/availability-label/availability-label.tsx b/src/components/availability-label/availability-label.tsx index c2ddf48c4f..578c31a0f6 100644 --- a/src/components/availability-label/availability-label.tsx +++ b/src/components/availability-label/availability-label.tsx @@ -4,13 +4,14 @@ import { useText } from "../../core/utils/text"; import LinkNoStyle from "../atoms/links/LinkNoStyle"; import { useStatistics } from "../../core/statistics/useStatistics"; import { statistics } from "../../core/statistics/statistics"; -import { getParentAvailabilityLabelClass, useAvailabilityData } from "./helper"; +import { getParentAvailabilityLabelClass } from "./helper"; import { Access, AccessTypeCode } from "../../core/dbc-gateway/generated/graphql"; import AvailabilityLabelInside from "./availability-label-inside"; import { FaustId } from "../../core/utils/types/ids"; +import useAvailabilityData from "./useAvailabilityData"; export interface AvailabilityLabelProps { manifestText: string; diff --git a/src/components/availability-label/helper.ts b/src/components/availability-label/helper.ts index e98efc2c55..e9e3cb06ce 100644 --- a/src/components/availability-label/helper.ts +++ b/src/components/availability-label/helper.ts @@ -1,148 +1,14 @@ -import { useEffect, useState } from "react"; import clsx from "clsx"; -import { - useGetV1LoanstatusIdentifier, - useGetV1ProductsIdentifier -} from "../../core/publizon/publizon"; -import { useConfig } from "../../core/utils/config"; -import { - Access, - AccessTypeCode -} from "../../core/dbc-gateway/generated/graphql"; -import { FaustId } from "../../core/utils/types/ids"; -import useGetAvailability from "../../core/utils/useGetAvailability"; -import { publizonProductStatuses } from "./types"; -import { ManifestationMaterialType } from "../../core/utils/types/material-type"; - -export const useAvailabilityData = ({ - accessTypes, - access, - faustIds, - isbn, - manifestText -}: { - accessTypes: AccessTypeCode[]; - access: Access["__typename"][]; - faustIds: FaustId[] | null; - isbn: string | null; - manifestText: string; -}) => { - const [isAvailable, setIsAvailable] = useState(null); - const config = useConfig(); - const isOnline = accessTypes?.includes(AccessTypeCode.Online) ?? false; - const [isCostFree, setIsCostFree] = useState(null); - const [isLoading, setIsLoading] = useState(null); - - useEffect(() => { - // An online material is by default always available. - if (isOnline) { - setIsAvailable(true); - } - }, [isOnline]); - - useEffect(() => { - // Articles are always available. - if ( - manifestText === ManifestationMaterialType.article && - isAvailable === null - ) { - setIsAvailable(true); - } - }, [manifestText, isAvailable]); - - const { isLoading: isLoadingIdentifier } = useGetV1ProductsIdentifier( - isbn ?? "", - { - query: { - // Publizon / useGetV1ProductsIdentifier is responsible for online - // materials. It requires an ISBN to do lookups. - enabled: - isOnline && - isbn !== null && - manifestText !== ManifestationMaterialType.article, - onSuccess: (res) => { - // If an online material isn't cost-free we need to check whether there is a queue - // to reserve it (via useGetV1LoanstatusIdentifier below in the code) - if (res?.product?.costFree === false) { - setIsCostFree(false); - return; - } - setIsCostFree(true); - } - } - } - ); - - const { isLoading: isLoadingProductInfo } = useGetV1LoanstatusIdentifier( - isbn || "", - { - // Publizon / useGetV1LoanstatusIdentifier shows loan status per material. - // This status is only available for products found on Ereol. Other online - // materials are always supposed to be shown as "available" - enabled: - isOnline && - !!isbn && - isCostFree === false && - access.some((acc) => acc === "Ereol") && - manifestText !== ManifestationMaterialType.article, - onSuccess: (res) => { - if (res && res.loanStatus) { - setIsAvailable(publizonProductStatuses[res.loanStatus].isAvailable); - return; - } - // In case the load status data doesn't exist we assume it isn't available - setIsAvailable(false); - } - } - ); - - const { isLoading: isLoadingAvailability } = useGetAvailability({ - faustIds: faustIds ?? [], - config, - options: { - query: { - // FBS / useGetAvailabilityV3 is responsible for handling availability - // for physical items. This will be the majority of all materials so we - // use this for everything except materials that are explicitly online. - enabled: - !isOnline && - faustIds !== null && - manifestText !== ManifestationMaterialType.article, - onSuccess: (data) => { - if (data?.some((item) => item.available)) { - setIsAvailable(true); - } - } - } - } - }); - - useEffect(() => { - // Articles are always available, thus no need to load. - if (manifestText !== ManifestationMaterialType.article) { - setIsLoading( - (isLoadingAvailability || - isLoadingIdentifier || - isLoadingProductInfo) && - isAvailable === null - ); - } - }, [ - isLoadingAvailability, - isLoadingIdentifier, - isLoadingProductInfo, - isAvailable, - manifestText - ]); - - return { isLoading, isAvailable }; -}; +import { AccessTypeCode } from "../../core/dbc-gateway/generated/graphql"; type GetParrentAvailabilityLabelClassProps = { selected?: boolean; cursorPointer?: boolean; }; +export const isOnline = (accessTypes: AccessTypeCode[]): boolean => + accessTypes?.includes(AccessTypeCode.Online) ?? false; + export const getParentAvailabilityLabelClass = ({ selected, cursorPointer diff --git a/src/components/availability-label/useAvailabilityData.ts b/src/components/availability-label/useAvailabilityData.ts new file mode 100644 index 0000000000..f96f015737 --- /dev/null +++ b/src/components/availability-label/useAvailabilityData.ts @@ -0,0 +1,44 @@ +import { + Access, + AccessTypeCode +} from "../../core/dbc-gateway/generated/graphql"; +import { FaustId } from "../../core/utils/types/ids"; +import { isOnline } from "./helper"; +import useOnlineAvailabilityData from "./useOnlineAvailabilityData"; +import usePhysicalAvailabilityData from "./usePhysicalAvailabilityData"; + +const useAvailabilityData = ({ + accessTypes, + access, + faustIds, + manifestText, + isbn +}: { + accessTypes: AccessTypeCode[]; + access: Access["__typename"][]; + faustIds: FaustId[]; + manifestText: string; + isbn: string | null; +}) => { + const availabilityOnline = useOnlineAvailabilityData({ + enabled: isOnline(accessTypes), + access, + faustIds, + isbn, + manifestText + }); + + const availabilityPhysical = usePhysicalAvailabilityData({ + enabled: !isOnline(accessTypes), + faustIds, + manifestText + }); + + if (isOnline(accessTypes)) { + return availabilityOnline; + } + + return availabilityPhysical; +}; + +export default useAvailabilityData; diff --git a/src/components/availability-label/useOnlineAvailabilityData.ts b/src/components/availability-label/useOnlineAvailabilityData.ts new file mode 100644 index 0000000000..5e9b4bfa13 --- /dev/null +++ b/src/components/availability-label/useOnlineAvailabilityData.ts @@ -0,0 +1,99 @@ +import { useEffect, useState } from "react"; +import { + useGetV1LoanstatusIdentifier, + useGetV1ProductsIdentifier +} from "../../core/publizon/publizon"; +import { Access } from "../../core/dbc-gateway/generated/graphql"; +import { FaustId } from "../../core/utils/types/ids"; +import { publizonProductStatuses } from "./types"; + +const useOnlineAvailabilityData = ({ + enabled, + access, + faustIds, + isbn, + manifestText +}: { + enabled: boolean; + access: Access["__typename"][]; + faustIds: FaustId[] | null; + isbn: string | null; + manifestText: string; +}) => { + const [isAvailable, setIsAvailable] = useState(null); + + // Find out if the material is cost free. + const { isLoading: isLoadingIdentifier, data: dataIdentifier } = + useGetV1ProductsIdentifier(isbn ?? "", { + query: { + // Publizon / useGetV1ProductsIdentifier is responsible for online + // materials. It requires an ISBN to do lookups. + enabled: enabled && isAvailable === null && isbn !== null + } + }); + + // Ereol request. + const { isLoading: isLoadingEreolData, data: dataEreol } = + useGetV1LoanstatusIdentifier(isbn || "", { + // Publizon / useGetV1LoanstatusIdentifier shows loan status per material. + // This status is only available for products found on Ereol. Other online + // materials are always supposed to be shown as "available" + enabled: + enabled && + isAvailable === null && + !!isbn && + // If the material is free (I think it is called blue material btw.) + // we should not load the loan status because then we know that it is available. + // So If the material is not free and we know it is an "Ereol" material we should load the loan status. + dataIdentifier?.product?.costFree === false && + access.some((acc) => acc === "Ereol") + }); + + useEffect(() => { + if ( + !enabled || + isAvailable !== null || + isLoadingIdentifier !== false || + isLoadingEreolData !== false + ) { + return; + } + + // If we have ereol data, we can use that to determine the availability. + if (dataEreol && dataEreol.loanStatus) { + setIsAvailable(publizonProductStatuses[dataEreol.loanStatus].isAvailable); + } + }, [ + isLoadingIdentifier, + isAvailable, + manifestText, + faustIds, + enabled, + dataEreol, + isLoadingEreolData + ]); + + // If hook is not enabled make it clear that the loading and availability status is unknown. + if (!enabled) { + return { + isLoading: null, + isAvailable: null + }; + } + + // An online material is by default always available if the availability status has not been set yet. + if (isAvailable === null) { + return { + isLoading: false, + isAvailable: true + }; + } + + // Return the availability status. + return { + isLoading: isLoadingIdentifier && isLoadingEreolData, + isAvailable + }; +}; + +export default useOnlineAvailabilityData; diff --git a/src/components/availability-label/usePhysicalAvailabilityData.ts b/src/components/availability-label/usePhysicalAvailabilityData.ts new file mode 100644 index 0000000000..7c597e82ea --- /dev/null +++ b/src/components/availability-label/usePhysicalAvailabilityData.ts @@ -0,0 +1,65 @@ +import { useConfig } from "../../core/utils/config"; +import { FaustId } from "../../core/utils/types/ids"; +import useGetAvailability from "../../core/utils/useGetAvailability"; +import { ManifestationMaterialType } from "../../core/utils/types/material-type"; + +const usePhysicalAvailabilityData = ({ + enabled, + faustIds, + manifestText +}: { + enabled: boolean; + faustIds: FaustId[] | null; + manifestText: string; +}) => { + const config = useConfig(); + + const response = useGetAvailability({ + faustIds: faustIds ?? [], + config, + options: { + query: { + // FBS / useGetAvailabilityV3 is responsible for handling availability + // for physical items. This will be the majority of all materials so we + // use this for everything except materials that are explicitly online. + enabled: + enabled && + faustIds !== null && + manifestText !== ManifestationMaterialType.article + } + } + }); + + const { isLoading, data } = response; + + // Articles are always available. + if (manifestText === ManifestationMaterialType.article) { + return { + isLoading: false, + isAvailable: true + }; + } + + // If we do not have data yet, return loading state. + if (!data) { + return { + isLoading, + isAvailable: null + }; + } + + // If we have data, check if any of the items are available. + if (data?.some((item) => item.available)) { + return { + isLoading: false, + isAvailable: true + }; + } + // Otherwise the manifestation is not available. + return { + isLoading: false, + isAvailable: false + }; +}; + +export default usePhysicalAvailabilityData; diff --git a/src/components/material/MaterialMainfestationItem.tsx b/src/components/material/MaterialMainfestationItem.tsx index 35d3fd8cee..23e93c64a5 100644 --- a/src/components/material/MaterialMainfestationItem.tsx +++ b/src/components/material/MaterialMainfestationItem.tsx @@ -115,6 +115,7 @@ const MaterialMainfestationItem: FC = ({
identifier.value)} diff --git a/src/core/utils/useGetAvailability.ts b/src/core/utils/useGetAvailability.ts index bea2af9543..43ca765088 100644 --- a/src/core/utils/useGetAvailability.ts +++ b/src/core/utils/useGetAvailability.ts @@ -15,11 +15,11 @@ const useGetAvailability = ({ query?: UseQueryOptions>>; }; }) => { - const { data, isLoading, isError } = useGetAvailabilityV3( + const response = useGetAvailabilityV3( getBlacklistedQueryArgs(faustIds, config, "availability"), options ); - return { data, isLoading, isError }; + return response; }; export default useGetAvailability; From b21447d93d625e65fd6e9c667611dcc4a4e1cb08 Mon Sep 17 00:00:00 2001 From: Mikkel Jakobsen Date: Thu, 1 Aug 2024 13:15:09 +0200 Subject: [PATCH 3/6] Remove unneeded manifestText from useOnlineAvailabilityData --- src/components/availability-label/useAvailabilityData.ts | 3 +-- .../availability-label/useOnlineAvailabilityData.ts | 5 +---- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/components/availability-label/useAvailabilityData.ts b/src/components/availability-label/useAvailabilityData.ts index f96f015737..fe54973d6c 100644 --- a/src/components/availability-label/useAvailabilityData.ts +++ b/src/components/availability-label/useAvailabilityData.ts @@ -24,8 +24,7 @@ const useAvailabilityData = ({ enabled: isOnline(accessTypes), access, faustIds, - isbn, - manifestText + isbn }); const availabilityPhysical = usePhysicalAvailabilityData({ diff --git a/src/components/availability-label/useOnlineAvailabilityData.ts b/src/components/availability-label/useOnlineAvailabilityData.ts index 5e9b4bfa13..ea15bc42bd 100644 --- a/src/components/availability-label/useOnlineAvailabilityData.ts +++ b/src/components/availability-label/useOnlineAvailabilityData.ts @@ -11,14 +11,12 @@ const useOnlineAvailabilityData = ({ enabled, access, faustIds, - isbn, - manifestText + isbn }: { enabled: boolean; access: Access["__typename"][]; faustIds: FaustId[] | null; isbn: string | null; - manifestText: string; }) => { const [isAvailable, setIsAvailable] = useState(null); @@ -66,7 +64,6 @@ const useOnlineAvailabilityData = ({ }, [ isLoadingIdentifier, isAvailable, - manifestText, faustIds, enabled, dataEreol, From 812b5ad032af721e4ca5011f6f5a1f91bc1a22f8 Mon Sep 17 00:00:00 2001 From: Mikkel Jakobsen Date: Thu, 1 Aug 2024 13:15:55 +0200 Subject: [PATCH 4/6] Make sure that the usePhysicalAvailabilityData returns null if it is not enabled. --- .../availability-label/usePhysicalAvailabilityData.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/components/availability-label/usePhysicalAvailabilityData.ts b/src/components/availability-label/usePhysicalAvailabilityData.ts index 7c597e82ea..abc9e7d98b 100644 --- a/src/components/availability-label/usePhysicalAvailabilityData.ts +++ b/src/components/availability-label/usePhysicalAvailabilityData.ts @@ -32,6 +32,14 @@ const usePhysicalAvailabilityData = ({ const { isLoading, data } = response; + // If hook is not enabled make it clear that the loading and availability status is unknown. + if (!enabled) { + return { + isLoading: null, + isAvailable: null + }; + } + // Articles are always available. if (manifestText === ManifestationMaterialType.article) { return { @@ -55,6 +63,7 @@ const usePhysicalAvailabilityData = ({ isAvailable: true }; } + // Otherwise the manifestation is not available. return { isLoading: false, From 5c39ba1836af290cc9e0992d2031dd15b5d0cf82 Mon Sep 17 00:00:00 2001 From: Mikkel Jakobsen Date: Thu, 1 Aug 2024 13:16:47 +0200 Subject: [PATCH 5/6] Add tests for the availability hooks --- src/tests/unit/availability.test.tsx | 451 +++++++++++++++++++++++++++ 1 file changed, 451 insertions(+) create mode 100644 src/tests/unit/availability.test.tsx diff --git a/src/tests/unit/availability.test.tsx b/src/tests/unit/availability.test.tsx new file mode 100644 index 0000000000..5d1053350f --- /dev/null +++ b/src/tests/unit/availability.test.tsx @@ -0,0 +1,451 @@ +// eslint-disable-next-line import/no-extraneous-dependencies +import { renderHook } from "@testing-library/react-hooks"; +import { + afterEach, + beforeEach, + beforeAll, + describe, + expect, + it, + vi +} from "vitest"; +import { act } from "react"; +import usePhysicalAvailabilityData from "../../components/availability-label/usePhysicalAvailabilityData"; +import { useGetAvailabilityV3 } from "../../core/fbs/fbs"; +import { ManifestationMaterialType } from "../../core/utils/types/material-type"; +import { useConfig } from "../../core/utils/config"; +import { + useGetV1LoanstatusIdentifier, + useGetV1ProductsIdentifier +} from "../../core/publizon/publizon"; +import useOnlineAvailabilityData from "../../components/availability-label/useOnlineAvailabilityData"; + +describe("usePhysicalAvailability tests", () => { + beforeAll(() => { + vi.mock("../../core/fbs/fbs", () => ({ + useGetAvailabilityV3: vi.fn() + })); + vi.mock("../../core/utils/config", () => ({ + useConfig: vi.fn() + })); + + // Make sure that the config hook returns an array with an empty string. + // In that way we do not have any blacklisted branches (they are not needed for the test). + // Typescript does not understand our mocked hook. + // So we gracefully ignore the error :). + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useConfig.mockReturnValue(() => [""]); + }); + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("Test that if one material is a book and is available, the hook returns that the material is available", () => { + // Typescript does not understand our mocked hook. + // So we gracefully ignore the error :). + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetAvailabilityV3.mockReturnValue({ + data: [ + { + recordId: "24859451", + reservable: true, + available: true, + reservations: 0 + }, + { + recordId: "24859450", + reservable: true, + available: false, + reservations: 0 + } + ], + isLoading: false, + isError: false + }); + + const { result } = renderHook(() => + usePhysicalAvailabilityData({ + enabled: true, + faustIds: ["24859452"], + manifestText: "bog" + }) + ); + + act(() => { + expect(result.current).toEqual({ + isLoading: false, + isAvailable: true + }); + }); + }); + + it("Test that if the material is a book amd no material is available the hook returns that it is unavailable", async () => { + // Typescript does not understand our mocked hook. + // So we gracefully ignore the error :). + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetAvailabilityV3.mockReturnValue({ + data: [ + { + recordId: "24859451", + reservable: true, + available: false, + reservations: 0 + }, + { + recordId: "24859450", + reservable: true, + available: false, + reservations: 0 + } + ], + isLoading: false, + isError: false + }); + + const { result } = renderHook(() => + usePhysicalAvailabilityData({ + enabled: true, + faustIds: ["24859452"], + manifestText: "bog" + }) + ); + + act(() => { + expect(result.current).toEqual({ + isLoading: false, + isAvailable: false + }); + }); + }); + + it("Test that if the material is an article it will always be available even though the remote service tells otherwise", () => { + // Typescript does not understand our mocked hook. + // So we gracefully ignore the error :). + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetAvailabilityV3.mockReturnValue({ + data: [ + { + recordId: "24859451", + reservable: true, + available: false, + reservations: 0 + }, + { + recordId: "24859450", + reservable: true, + available: false, + reservations: 0 + } + ], + isLoading: false, + isError: false + }); + + const { result } = renderHook(() => + usePhysicalAvailabilityData({ + enabled: true, + faustIds: ["24859452"], + manifestText: ManifestationMaterialType.article + }) + ); + + act(() => { + expect(result.current).toEqual({ + isLoading: false, + isAvailable: true + }); + }); + }); + + it("Test that if the material is an article it will always be available even though the remote service tells otherwise", () => { + // Typescript does not understand our mocked hook. + // So we gracefully ignore the error :). + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetAvailabilityV3.mockReturnValue({ + data: [ + { + recordId: "24859451", + reservable: true, + available: false, + reservations: 0 + }, + { + recordId: "24859450", + reservable: true, + available: false, + reservations: 0 + } + ], + isLoading: false, + isError: false + }); + + const { result } = renderHook(() => + usePhysicalAvailabilityData({ + enabled: true, + faustIds: ["24859452"], + manifestText: ManifestationMaterialType.article + }) + ); + + act(() => { + expect(result.current).toEqual({ + isLoading: false, + isAvailable: true + }); + }); + }); + + it("Test that if the hook is not enabled it should return null statuses", () => { + // Typescript does not understand our mocked hook. + // So we gracefully ignore the error :). + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetAvailabilityV3.mockReturnValue({ + data: undefined, + isLoading: false, + isError: false + }); + + const { result } = renderHook(() => + usePhysicalAvailabilityData({ + enabled: false, + faustIds: ["24859452"], + manifestText: ManifestationMaterialType.article + }) + ); + + act(() => { + expect(result.current).toEqual({ + isLoading: null, + isAvailable: null + }); + }); + }); +}); + +describe("useOnlineAvailabilityData tests", () => { + beforeAll(() => { + vi.mock("../../core/publizon/publizon", async () => { + const actual = + (await vi.importActual("../../core/publizon/publizon")) ?? {}; + return { + // No need for the ts check here. + // We want to partially mock the module and this is the way to do it. + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + ...actual, + useGetV1ProductsIdentifier: vi.fn(), + useGetV1LoanstatusIdentifier: vi.fn() + }; + }); + + // Make sure that the config hook returns an array with an empty string. + // In that way we do not have any blacklisted branches (they are not needed for the test). + // Typescript does not understand our mocked hook. + // So we gracefully ignore the error :). + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + // useConfig.mockReturnValue(() => [""]); + }); + + beforeEach(() => { + vi.useFakeTimers(); + }); + + afterEach(() => { + vi.useRealTimers(); + }); + + it("If the useGetV1ProductsIdentifier service tells us that the material is NOT `costFree` (not a blue title) and the material belongs to 'Ereol' (the access param) the Publizon product status should dictate the availability ", () => { + // The only Publizon product status that is NOT available is 5. + + // Typescript does not understand our mocked hooks. + // So we gracefully ignore the errors:). + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetV1ProductsIdentifier.mockReturnValue({ + isLoading: false, + data: { + product: { + costFree: false + } + } + }); + + /** + * First test: + * Publizon product status: 4 + */ + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetV1LoanstatusIdentifier.mockReturnValue({ + isLoading: false, + data: { + loanStatus: 4 + } + }); + + const { result: firstResult } = renderHook(() => + useOnlineAvailabilityData({ + enabled: true, + access: ["Ereol"], + faustIds: ["138625958"], + isbn: "9788794564076" + }) + ); + + act(() => { + expect(firstResult.current).toEqual({ + isLoading: false, + isAvailable: true + }); + }); + /** + * End first test. + */ + + /** + * Second test: + * Publizon product status: 5 + */ + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetV1LoanstatusIdentifier.mockReturnValue({ + isLoading: false, + data: { + loanStatus: 5 + } + }); + const { result: secondResult } = renderHook(() => + useOnlineAvailabilityData({ + enabled: true, + access: ["Ereol"], + faustIds: ["138625958"], + isbn: "9788794564076" + }) + ); + + act(() => { + expect(secondResult.current).toEqual({ + isLoading: false, + isAvailable: false + }); + }); + /** + * End second test. + */ + + /** + * Third test: + * Publizon product status: 2 + */ + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetV1LoanstatusIdentifier.mockReturnValue({ + isLoading: false, + data: { + loanStatus: 2 + } + }); + const { result: thirdResult } = renderHook(() => + useOnlineAvailabilityData({ + enabled: true, + access: ["Ereol"], + faustIds: ["138625958"], + isbn: "9788794564076" + }) + ); + + act(() => { + expect(thirdResult.current).toEqual({ + isLoading: false, + isAvailable: true + }); + }); + /** + * End third test. + */ + }); + + it("Test that if the material is cost free nothing else matters", () => { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetV1ProductsIdentifier.mockReturnValue({ + isLoading: false, + data: { + product: { + costFree: true + } + } + }); + + // If the material is cost free no data is being loaded from this service. + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetV1LoanstatusIdentifier.mockReturnValue({ + isLoading: false, + data: undefined + }); + + const { result } = renderHook(() => + useOnlineAvailabilityData({ + enabled: true, + access: ["Ereol"], + faustIds: ["138625958"], + isbn: "9788794564076" + }) + ); + + act(() => { + expect(result.current).toEqual({ + isLoading: false, + isAvailable: true + }); + }); + }); + + it("Test that if the hook is not enabled it should return null statuses", () => { + // If the hook is not enabled no data is being loaded from this service. + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetV1ProductsIdentifier.mockReturnValue({ + isLoading: false, + data: undefined + }); + + // If the hook is not enabled no data is being loaded from this service. + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore-next-line + useGetV1LoanstatusIdentifier.mockReturnValue({ + isLoading: false, + data: undefined + }); + + const { result } = renderHook(() => + useOnlineAvailabilityData({ + enabled: false, + access: ["Ereol"], + faustIds: ["138625958"], + isbn: "9788794564076" + }) + ); + + act(() => { + expect(result.current).toEqual({ + isLoading: null, + isAvailable: null + }); + }); + }); +}); From db378dfd15d76f432de9792fba42b5b6bc0b0580 Mon Sep 17 00:00:00 2001 From: Mikkel Jakobsen Date: Fri, 2 Aug 2024 15:01:16 +0200 Subject: [PATCH 6/6] Expand isArticle functionality globally with "new" types Since we have multiple article types we should consider those in the various places where we have functionality relying on those types. --- src/components/availability-label/helper.ts | 4 ++++ .../availability-label/usePhysicalAvailabilityData.ts | 9 +++------ src/components/material/material-buttons/helper.ts | 7 +++---- src/core/utils/types/article-types.ts | 10 ++++++++++ 4 files changed, 20 insertions(+), 10 deletions(-) create mode 100644 src/core/utils/types/article-types.ts diff --git a/src/components/availability-label/helper.ts b/src/components/availability-label/helper.ts index e9e3cb06ce..420dca62d3 100644 --- a/src/components/availability-label/helper.ts +++ b/src/components/availability-label/helper.ts @@ -1,5 +1,6 @@ import clsx from "clsx"; import { AccessTypeCode } from "../../core/dbc-gateway/generated/graphql"; +import articleTypes from "../../core/utils/types/article-types"; type GetParrentAvailabilityLabelClassProps = { selected?: boolean; @@ -9,6 +10,9 @@ type GetParrentAvailabilityLabelClassProps = { export const isOnline = (accessTypes: AccessTypeCode[]): boolean => accessTypes?.includes(AccessTypeCode.Online) ?? false; +export const isArticle = (manifestText: string): boolean => + articleTypes.some((type) => manifestText.toLowerCase() === type); + export const getParentAvailabilityLabelClass = ({ selected, cursorPointer diff --git a/src/components/availability-label/usePhysicalAvailabilityData.ts b/src/components/availability-label/usePhysicalAvailabilityData.ts index abc9e7d98b..4244945e8d 100644 --- a/src/components/availability-label/usePhysicalAvailabilityData.ts +++ b/src/components/availability-label/usePhysicalAvailabilityData.ts @@ -1,7 +1,7 @@ import { useConfig } from "../../core/utils/config"; import { FaustId } from "../../core/utils/types/ids"; import useGetAvailability from "../../core/utils/useGetAvailability"; -import { ManifestationMaterialType } from "../../core/utils/types/material-type"; +import { isArticle } from "./helper"; const usePhysicalAvailabilityData = ({ enabled, @@ -22,10 +22,7 @@ const usePhysicalAvailabilityData = ({ // FBS / useGetAvailabilityV3 is responsible for handling availability // for physical items. This will be the majority of all materials so we // use this for everything except materials that are explicitly online. - enabled: - enabled && - faustIds !== null && - manifestText !== ManifestationMaterialType.article + enabled: enabled && faustIds !== null && !isArticle(manifestText) } } }); @@ -41,7 +38,7 @@ const usePhysicalAvailabilityData = ({ } // Articles are always available. - if (manifestText === ManifestationMaterialType.article) { + if (isArticle(manifestText)) { return { isLoading: false, isAvailable: true diff --git a/src/components/material/material-buttons/helper.ts b/src/components/material/material-buttons/helper.ts index a13d8b2a0a..5c916d682b 100644 --- a/src/components/material/material-buttons/helper.ts +++ b/src/components/material/material-buttons/helper.ts @@ -4,7 +4,7 @@ import { AccessTypeCode } from "../../../core/dbc-gateway/generated/graphql"; import { Manifestation } from "../../../core/utils/types/entities"; -import { ManifestationMaterialType } from "../../../core/utils/types/material-type"; +import articleTypes from "../../../core/utils/types/article-types"; export const hasCorrectAccess = ( desiredAccess: NonNullable, @@ -57,9 +57,8 @@ export const hasCorrectPartialMaterialType = ( }; export const isArticle = (manifestations: Manifestation[]) => { - return hasCorrectPartialMaterialType( - ManifestationMaterialType.article, - manifestations + return articleTypes.some((type) => + hasCorrectMaterialType(type, manifestations) ); }; diff --git a/src/core/utils/types/article-types.ts b/src/core/utils/types/article-types.ts new file mode 100644 index 0000000000..bd679466c5 --- /dev/null +++ b/src/core/utils/types/article-types.ts @@ -0,0 +1,10 @@ +import { ManifestationMaterialType } from "./material-type"; + +const articleTypes = [ + ManifestationMaterialType.article, + ManifestationMaterialType.paperArticle, + ManifestationMaterialType.onlineArticle, + ManifestationMaterialType.earticle +] as const; + +export default articleTypes;