diff --git a/CHANGELOG b/CHANGELOG index f9bfe434a..d2c91d4b0 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -5,6 +5,15 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.3.8] 2024-12-9 + +Note this release was not merged to main before deploying. + +### Updated + +- add call number and standard numbers to advanced search ([SCC-4326](https://newyorkpubliclibrary.atlassian.net/browse/SCC-4326)) +- Pull in Owning institution from items to populate bib details ([SCC-4334](https://newyorkpubliclibrary.atlassian.net/browse/SCC-4334)) + ### [1.3.7] Hotfix 2024-11-21 - Fix analytics bug where routes without a query string do not emit the correct virtual page view event (SCC-4314) diff --git a/__test__/fixtures/bibFixtures.ts b/__test__/fixtures/bibFixtures.ts index 294842e95..999a549b4 100644 --- a/__test__/fixtures/bibFixtures.ts +++ b/__test__/fixtures/bibFixtures.ts @@ -1,3 +1,180 @@ +export const princetonRecord = { + "@context": + "http://discovery-api-qa.nypl.org/api/v0.1/discovery/context_all.jsonld", + "@type": ["nypl:Item", "nypl:Resource"], + "@id": "res:pb2608686", + buildingLocationIds: [], + carrierType: [ + { + "@id": "carriertypes:nc", + prefLabel: "volume", + }, + ], + contributorLiteral: ["Brown, David, journalist, joint author."], + createdString: ["1943"], + createdYear: 1943, + creatorLiteral: ["Wagg, Alfred."], + dateStartYear: 1943, + dateString: ["1943"], + dimensions: ["23 cm."], + extent: ["231 p."], + idLccn: [" 44003956 "], + identifier: [ + { + "@type": "nypl:Bnumber", + "@value": "2608686", + }, + { + "@type": "bf:Lccn", + "@value": " 44003956 ", + }, + { + "@type": "bf:Identifier", + "@value": "(OCoLC)ocm02088006", + }, + ], + issuance: [ + { + "@id": "urn:biblevel:m", + prefLabel: "monograph/item", + }, + ], + itemAggregations: [ + { + "@type": "nypl:Aggregation", + "@id": "res:location", + id: "location", + field: "location", + values: [], + }, + { + "@type": "nypl:Aggregation", + "@id": "res:format", + id: "format", + field: "format", + values: [], + }, + { + "@type": "nypl:Aggregation", + "@id": "res:status", + id: "status", + field: "status", + values: [ + { + value: "status:a", + count: 1, + label: "Available", + }, + ], + }, + ], + items: [ + { + "@id": "res:pi5153471", + accessMessage: [ + { + "@id": "accessMessage:1", + prefLabel: "Use in library", + }, + ], + catalogItemType: [ + { + "@id": "catalogItemType:1", + prefLabel: "non-circ", + }, + ], + eddFulfillment: { + "@id": "fulfillment:recap-edd", + }, + eddRequestable: true, + idBarcode: ["32101067443802"], + identifier: [ + { + "@type": "bf:ShelfMark", + "@value": "D763.I8 W3 1943", + }, + { + "@type": "bf:Barcode", + "@value": "32101067443802", + }, + ], + idNyplSourceId: { + "@type": "RecapPul", + "@value": "25791623", + }, + owner: [ + { + "@id": "orgs:0003", + prefLabel: "Princeton University Library", + }, + ], + physFulfillment: { + "@id": "fulfillment:recap-offsite", + }, + physRequestable: true, + requestable: [true], + shelfMark: ["D763.I8 W3 1943"], + specRequestable: false, + status: [ + { + "@id": "status:na", + prefLabel: "Not available", + }, + ], + uri: "pi5153471", + }, + ], + language: [ + { + "@id": "lang:eng", + prefLabel: "English", + }, + ], + lccClassification: ["D763.I8 W3"], + materialType: [ + { + "@id": "resourcetypes:txt", + prefLabel: "Text", + }, + ], + mediaType: [ + { + "@id": "mediatypes:n", + prefLabel: "unmediated", + }, + ], + note: [ + { + noteType: "Note", + "@type": "bf:Note", + prefLabel: '"First published in 1943."', + }, + ], + numAvailable: 1, + numItems: 1, + numItemsMatched: 1, + numItemsTotal: 1, + placeOfPublication: ["London,"], + publicationStatement: ["London, Nicholson & Watson [1943]"], + publisherLiteral: ["Nicholson & Watson"], + subjectLiteral: [ + "World War, 1939-1945 -- Campaigns -- Italy.", + "World War, 1939-1945 -- Naval operations.", + "World War, 1939-1945 -- Personal narratives.", + ], + title: ["No spaghetti for breakfast"], + titleDisplay: [ + "No spaghetti for breakfast [by] Alfred Wagg and David Brown.", + ], + type: ["nypl:Item"], + updatedAt: 1543536484228, + uri: "pb2608686", + updatedAtDate: "2018-11-30T00:08:04.228Z", + hasItemVolumes: false, + hasItemDates: false, + electronicResources: [], +} + export const bibWithItems = { resource: { "@context": diff --git a/__test__/pages/search/advancedSearchForm.test.tsx b/__test__/pages/search/advancedSearchForm.test.tsx index b07ec4842..69be2b364 100644 --- a/__test__/pages/search/advancedSearchForm.test.tsx +++ b/__test__/pages/search/advancedSearchForm.test.tsx @@ -1,8 +1,15 @@ import React from "react" -import { fireEvent, render, screen } from "../../../src/utils/testUtils" +import { + fireEvent, + render, + screen, + waitFor, + delay, +} from "../../../src/utils/testUtils" import mockRouter from "next-router-mock" import userEvent from "@testing-library/user-event" +import { textInputFields } from "../../../src/utils/advancedSearchUtils" import AdvancedSearch, { defaultEmptySearchErrorMessage, } from "../../../pages/search/advanced" @@ -12,52 +19,70 @@ import { searchAggregations } from "../../../src/config/aggregations" jest.mock("next/router", () => jest.requireActual("next-router-mock")) describe("Advanced Search Form", () => { + beforeEach(async () => { + render() + }) const submit = () => { - fireEvent( - screen.getByTestId("submit-advanced-search-button"), - new MouseEvent("click") - ) + fireEvent.click(screen.getByTestId("submit-advanced-search-button")) + } + const updateAllFields = async () => { + const [ + keywordInput, + contributorInput, + titleInput, + subjectInput, + callNumberInput, + uniqueIdentifierInput, + ] = [ + "Keyword", + "Title", + "Author/contributor", + "Subject", + "Call number", + "Unique identifier", + ].map((field) => screen.getByLabelText(field)) + fireEvent.change(subjectInput, { target: { value: "italian food" } }) + fireEvent.change(keywordInput, { target: { value: "spaghetti" } }) + fireEvent.change(contributorInput, { target: { value: "strega nonna" } }) + fireEvent.change(titleInput, { target: { value: "il amore di pasta" } }) + fireEvent.change(callNumberInput, { target: { value: "12345" } }) + fireEvent.change(uniqueIdentifierInput, { target: { value: "67890" } }) + + // without this delay, the input is not updated until after submit is called. + await delay(500) + return [ + keywordInput, + contributorInput, + titleInput, + subjectInput, + callNumberInput, + uniqueIdentifierInput, + ] } afterEach(async () => { await userEvent.click(screen.getByText("Clear fields")) }) it("displays alert when no fields are submitted", () => { - render() - submit() screen.getByText(defaultEmptySearchErrorMessage) }) - // this test is broken due to debounce/userEvent/timing weirdness. - // this functionality works in the browser, but won't include - // final input in output string in test. the broken test is - // commented out below. - it.todo("can set keyword, contributor, title, subject") - // async () => { - // render() - // const [keywordInput, contributorInput, titleInput, subjectInput] = [ - // "Keywords", - // "Title", - // "Author", - // "Subject", - // ].map((field) => screen.getByLabelText(field)) - // await act(async () => { - // await userEvent.type(subjectInput, "italian food") - // await userEvent.type(keywordInput, "spaghetti") - // await userEvent.type(contributorInput, "strega nonna") - // await userEvent.type(titleInput, "il amore di pasta") - // // this set stimeout is to ad - // // eslint-disable-next-line @typescript-eslint/no-empty-function - // setTimeout(() => {}, 300) - // submit() - // expect(mockRouter.asPath).toBe( - // "/search?q=spaghetti&contributor=il+amore+di+pasta&title=strega+nonna&subject=italian+food" - // ) - // }) - // }) - it("can select languages", async () => { - render() + it("can set keyword, contributor, title, subject", async () => { + await updateAllFields() + submit() + expect(mockRouter.asPath).toBe( + "/search?q=spaghetti&title=strega+nonna&contributor=il+amore+di+pasta&callnumber=12345&standard_number=67890&subject=italian+food" + ) + }) + it("renders inputs for all text input fields", () => { + textInputFields.map(({ label }) => { + const input = screen.getByLabelText(label) + expect(input).toBeInTheDocument() + }) + }) + + it("can select languages", async () => { const languageSelect = screen.getByLabelText("Language") await userEvent.selectOptions(languageSelect, "Azerbaijani") submit() @@ -67,7 +92,6 @@ describe("Advanced Search Form", () => { ) }) it("can check material checkboxes", async () => { - render() await userEvent.click(screen.getByLabelText("Notated music")) await userEvent.click(screen.getByLabelText("Cartographic")) submit() @@ -78,7 +102,6 @@ describe("Advanced Search Form", () => { ) }) it("can check location checkboxes", async () => { - render() const location = searchAggregations.buildingLocation[0] await userEvent.click(screen.getByLabelText(location.label as string)) submit() @@ -88,7 +111,6 @@ describe("Advanced Search Form", () => { }) it("can clear the form", async () => { - render() const notatedMusic = screen.getByLabelText("Notated music") await userEvent.click(notatedMusic) const cartographic = screen.getByLabelText("Cartographic") @@ -100,14 +122,8 @@ describe("Advanced Search Form", () => { }) await userEvent.click(schomburg) - const keywordInput = screen.getByLabelText("Keyword") - const titleInput = screen.getByLabelText("Title") - const contributorInput = screen.getByLabelText("Author") - const subjectInput = screen.getByLabelText("Subject") - await userEvent.type(keywordInput, "spaghetti") - await userEvent.type(contributorInput, "strega nonna") - await userEvent.type(titleInput, "il amore di pasta") - await userEvent.type(subjectInput, "italian food") + const [subjectInput, keywordInput, titleInput, contributorInput] = + await updateAllFields() await userEvent.click(screen.getByText("Clear fields")) ;[notatedMusic, cartographic, schomburg].forEach((input) => diff --git a/__test__/pages/search/searchResults.test.tsx b/__test__/pages/search/searchResults.test.tsx index b121f48b2..8969afa4f 100644 --- a/__test__/pages/search/searchResults.test.tsx +++ b/__test__/pages/search/searchResults.test.tsx @@ -1,6 +1,6 @@ import React from "react" import userEvent from "@testing-library/user-event" -import { render, screen } from "../../../src/utils/testUtils" +import { render, screen, waitFor } from "../../../src/utils/testUtils" import mockRouter from "next-router-mock" @@ -32,11 +32,10 @@ describe("Search Results page", () => { }) await userEvent.click(field) await userEvent.click(screen.getByText("Apply filters")) - // This was the only way to get this test to pass. waitFor was not in fact waiting, even with same timeout. - setTimeout(() => { + waitFor(() => { const resultsHeading = screen.getByTestId("search-results-heading") expect(resultsHeading).toHaveFocus() - }, 500) + }) }) it("focuses on search results heading after loading a keyword search", () => { mockRouter.push(`/search?q=${query}`) diff --git a/pages/bib/[id]/index.tsx b/pages/bib/[id]/index.tsx index 26074b8ec..9132d1388 100644 --- a/pages/bib/[id]/index.tsx +++ b/pages/bib/[id]/index.tsx @@ -84,7 +84,6 @@ export default function BibPage({ discoveryBibResult, annotatedMarc ) - const displayLegacyCatalogLink = isNyplBibID(bib.id) const filtersAreApplied = areFiltersApplied(appliedFilters) diff --git a/pages/search/advanced.tsx b/pages/search/advanced.tsx index d1d4130aa..f44cb1987 100644 --- a/pages/search/advanced.tsx +++ b/pages/search/advanced.tsx @@ -70,6 +70,7 @@ export default function AdvancedSearch({ searchFormReducer, initialSearchFormState ) + const { dateFormProps, validateDateRange, @@ -85,7 +86,6 @@ export default function AdvancedSearch({ e.preventDefault() alert && setAlert(false) const target = e.target as HTMLInputElement - dispatch({ type: type, field: target.name, @@ -106,7 +106,6 @@ export default function AdvancedSearch({ e.preventDefault() if (!validateDateRange()) return const queryString = getSearchQuery(searchFormState as SearchParams) - if (!queryString.length) { setErrorMessage(defaultEmptySearchErrorMessage) setAlert(true) @@ -164,7 +163,7 @@ export default function AdvancedSearch({ onSubmit={handleSubmit} > - + {textInputFields.map(({ name, label }) => { return ( @@ -201,7 +200,7 @@ export default function AdvancedSearch({ {} - + /^Recap/.test(item?.idNyplSourceId?.["@type"])) + // Only consider items with an `owner` (should be all, in practice) + const owner = firstRecapItem?.owner?.[0]?.prefLabel + if (owner) return [owner] + } + buildAnnotatedMarcDetails( annotatedMarc: AnnotatedMarcField[] ): AnyBibDetail[] { @@ -88,11 +101,14 @@ export default class BibDetails { { field: "creatorLiteral", label: "Author" }, ] .map((fieldMapping) => { - if (fieldMapping.field === "supplementaryContent") - return this.supplementaryContent - else if (fieldMapping.field === "creatorLiteral") - return this.buildInternalLinkedDetail(fieldMapping) - else return this.buildStandardDetail(fieldMapping) + switch (fieldMapping.field) { + case "supplementaryContent": + return this.supplementaryContent + case "creatorLiteral": + return this.buildSearchFilterUrl(fieldMapping) + default: + return this.buildStandardDetail(fieldMapping) + } }) .filter((f) => f) } @@ -121,20 +137,11 @@ export default class BibDetails { .map((fieldMapping: FieldMapping): AnyBibDetail => { let detail: AnyBibDetail if (fieldMapping.field === "contributorLiteral") - detail = this.buildInternalLinkedDetail(fieldMapping) - else if (fieldMapping.field === "subjectLiteral") - detail = this.subjectHeadings - else if (fieldMapping.field === "extent") detail = this.extent - else if (fieldMapping.field === "owner") - detail = this.bib.owner && { - label: convertToSentenceCase(fieldMapping.label), - value: [this.bib.owner?.prefLabel], - } + detail = this.buildSearchFilterUrl(fieldMapping) else detail = this.buildStandardDetail(fieldMapping) return detail }) .filter((f) => f) - const fieldsWithNotes = this.addNotes(resourceFields) const combinedFields = this.combineBibDetailsData( fieldsWithNotes, @@ -174,7 +181,9 @@ export default class BibDetails { } buildStandardDetail(fieldMapping: FieldMapping) { - const bibFieldValue = this.bib[fieldMapping.field] + const bibFieldValue = + this[fieldMapping.field] || this.bib[fieldMapping.field] + if (!bibFieldValue) return return this.buildDetail( convertToSentenceCase(fieldMapping.label), bibFieldValue @@ -190,7 +199,7 @@ export default class BibDetails { } } - buildInternalLinkedDetail(fieldMapping: { + buildSearchFilterUrl(fieldMapping: { label: string field: string }): LinkedBibDetail { @@ -326,7 +335,7 @@ export default class BibDetails { return Object.assign({}, bib, ...parallelFieldMatches) } - buildExtent(): BibDetail { + buildExtent(): string[] { let modifiedExtent: string[] const { extent, dimensions } = this.bib const removeSemiColon = (extent) => [extent[0].replace(/\s*;\s*$/, "")] @@ -342,10 +351,7 @@ export default class BibDetails { parts.push(dimensions[0]) modifiedExtent = [parts.join("; ")] } - return { - label: "Description", - value: modifiedExtent, - } + return modifiedExtent } buildSupplementaryContent(): LinkedBibDetail { @@ -367,21 +373,16 @@ export default class BibDetails { return this.buildExternalLinkedDetail(convertToSentenceCase(label), values) } - buildSubjectHeadings(): SubjectHeadingDetail { + buildSubjectHeadings(): BibDetailURL[][] { if (!this.bib.subjectHeadings) return const subjectHeadingsUrls = this.bib.subjectHeadings.map((heading) => this.flattenSubjectHeadingUrls(heading) ) - return ( - subjectHeadingsUrls?.length && { - label: "Subject", - value: subjectHeadingsUrls, - } - ) + return subjectHeadingsUrls?.length && subjectHeadingsUrls } - buildSubjectLiterals(): SubjectHeadingDetail { + buildSubjectLiterals(): BibDetailURL[][] { if (!this.bib.subjectLiteral) return const subjectLiteralUrls = this.bib.subjectLiteral.map( (subject: string) => { @@ -403,10 +404,7 @@ export default class BibDetails { }) } ) - return { - label: "Subject", - value: subjectLiteralUrls, - } + return subjectLiteralUrls } constructSubjectLiteralsArray(subject: string) { diff --git a/src/models/modelTests/BibDetails.test.ts b/src/models/modelTests/BibDetails.test.ts index 773426c8a..d91bd2514 100644 --- a/src/models/modelTests/BibDetails.test.ts +++ b/src/models/modelTests/BibDetails.test.ts @@ -4,6 +4,8 @@ import { parallelsBib, yiddishBib, bibWithSubjectHeadings, + bibNoItems, + princetonRecord, } from "../../../__test__/fixtures/bibFixtures" import type { LinkedBibDetail } from "../../types/bibDetailsTypes" import BibDetailsModel from "../BibDetails" @@ -31,6 +33,19 @@ describe("Bib model", () => { bibWithSubjectHeadings.resource, bibWithSubjectHeadings.annotatedMarc ) + describe("owner", () => { + it("populates owner when owner is present", () => { + const partnerBib = new BibDetailsModel(princetonRecord) + expect(partnerBib.owner).toStrictEqual(["Princeton University Library"]) + }) + it("does not populate owner if item is nypl", () => { + expect(bibWithRtlParallelsModel.owner).toBe(undefined) + }) + it("can handle no items", () => { + const noItemsBib = new BibDetailsModel(bibNoItems.resource) + expect(noItemsBib.owner).toBe(undefined) + }) + }) describe("note", () => { it("groups notes into an array of {label, value} details", () => { const model = bibWithParallelsModel @@ -58,96 +73,90 @@ describe("Bib model", () => { }) describe("subjectHeadings", () => { it("correctly formats the subjectHeading urls when the subjectHeadings are present in the bib result", () => { - const subjectHeadingsObject = { - label: "Subject", - value: [ - [ - { - url: "/subject_headings/cf347108-e1f2-4c0f-808a-ac4ace2f0765?label=Cortanze%2C%20G%C3%A9rard%20de", - urlLabel: "Cortanze, Gérard de", - }, - { - url: "/subject_headings/74746d11-638b-4cfb-a72a-9a2bd296e6fd?label=Cortanze%2C%20G%C3%A9rard%20de%20--%20Childhood%20and%20youth", - urlLabel: "Childhood and youth", - }, - ], - [ - { - url: "/subject_headings/5fd065df-b4e9-48cb-b13c-ea15f36b96b4?label=Authors%2C%20French", - urlLabel: "Authors, French", - }, - { - url: "/subject_headings/e43674a7-5f02-44f1-95cd-dbcc776331b7?label=Authors%2C%20French%20--%2020th%20century", - urlLabel: "20th century", - }, - { - url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors%2C%20French%20--%2020th%20century%20--%20Biography", - urlLabel: "Biography", - }, - ], - [ - { - url: "/subject_headings/3a779ed6-8a07-4d27-80ef-e0c2b10fe78e?label=Autobiographical%20Narrative", - urlLabel: "Autobiographical Narrative", - }, - ], + const subjectHeadings = [ + [ + { + url: "/subject_headings/cf347108-e1f2-4c0f-808a-ac4ace2f0765?label=Cortanze%2C%20G%C3%A9rard%20de", + urlLabel: "Cortanze, Gérard de", + }, + { + url: "/subject_headings/74746d11-638b-4cfb-a72a-9a2bd296e6fd?label=Cortanze%2C%20G%C3%A9rard%20de%20--%20Childhood%20and%20youth", + urlLabel: "Childhood and youth", + }, + ], + [ + { + url: "/subject_headings/5fd065df-b4e9-48cb-b13c-ea15f36b96b4?label=Authors%2C%20French", + urlLabel: "Authors, French", + }, + { + url: "/subject_headings/e43674a7-5f02-44f1-95cd-dbcc776331b7?label=Authors%2C%20French%20--%2020th%20century", + urlLabel: "20th century", + }, + { + url: "/subject_headings/9391bc26-e44c-44ac-98cc-e3800da51926?label=Authors%2C%20French%20--%2020th%20century%20--%20Biography", + urlLabel: "Biography", + }, + ], + [ + { + url: "/subject_headings/3a779ed6-8a07-4d27-80ef-e0c2b10fe78e?label=Autobiographical%20Narrative", + urlLabel: "Autobiographical Narrative", + }, ], - } - expect(bibWithSubjectHeadingsModel.subjectHeadings).toMatchObject( - subjectHeadingsObject + ] + expect(bibWithSubjectHeadingsModel.subjectLiteral).toStrictEqual( + subjectHeadings ) }) it("falls back to subject literals when subject headings are absent in the bib and correctly formats the urls", () => { const filterQueryForSubjectLiteral = "/search?filters[subjectLiteral]=" - const subjectHeadingsObject = { - label: "Subject", - value: [ - [ - { - url: `${filterQueryForSubjectLiteral}${encodeURI( - "Authors, French" - )}`, - urlLabel: "Authors, French", - }, - { - url: `${filterQueryForSubjectLiteral}${encodeURI( - "Authors, French -- 20th century" - )}`, - urlLabel: "20th century", - }, - { - url: `${filterQueryForSubjectLiteral}${encodeURI( - "Authors, French -- 20th century -- Biography" - )}`, - urlLabel: "Biography", - }, - ], - [ - { - url: `${filterQueryForSubjectLiteral}${encodeURI( - "Autobiographical Narrative" - )}`, - urlLabel: "Autobiographical Narrative", - }, - ], - [ - { - url: `${filterQueryForSubjectLiteral}${encodeURI( - "Cortanze, Gérard de" - )}`, - urlLabel: "Cortanze, Gérard de", - }, - { - url: `${filterQueryForSubjectLiteral}${encodeURI( - "Cortanze, Gérard de -- Childhood and youth" - )}`, - urlLabel: "Childhood and youth", - }, - ], + const subjectHeadings = [ + [ + { + url: `${filterQueryForSubjectLiteral}${encodeURI( + "Authors, French" + )}`, + urlLabel: "Authors, French", + }, + { + url: `${filterQueryForSubjectLiteral}${encodeURI( + "Authors, French -- 20th century" + )}`, + urlLabel: "20th century", + }, + { + url: `${filterQueryForSubjectLiteral}${encodeURI( + "Authors, French -- 20th century -- Biography" + )}`, + urlLabel: "Biography", + }, + ], + [ + { + url: `${filterQueryForSubjectLiteral}${encodeURI( + "Autobiographical Narrative" + )}`, + urlLabel: "Autobiographical Narrative", + }, + ], + [ + { + url: `${filterQueryForSubjectLiteral}${encodeURI( + "Cortanze, Gérard de" + )}`, + urlLabel: "Cortanze, Gérard de", + }, + { + url: `${filterQueryForSubjectLiteral}${encodeURI( + "Cortanze, Gérard de -- Childhood and youth" + )}`, + urlLabel: "Childhood and youth", + }, ], - } - expect(bibWithNoParallelsModel.subjectHeadings).toMatchObject( - subjectHeadingsObject + ] + expect(bibWithNoParallelsModel.subjectLiteral).toStrictEqual( + subjectHeadings ) }) }) @@ -158,7 +167,7 @@ describe("Bib model", () => { extent: ["99 bottles of beer"], dimensions: ["99 x 99 cm"], }) - expect(bib.extent.value[0].includes("; ")) + expect(bib.extent[0].includes("; ")) }) it("should append dimensions to extent", () => { const bib = new BibDetailsModel({ @@ -166,7 +175,7 @@ describe("Bib model", () => { extent: ["99 bottles of beer"], dimensions: ["99 x 99 cm"], }) - expect(bib.extent.value[0]).toBe("99 bottles of beer; 99 x 99 cm") + expect(bib.extent[0]).toBe("99 bottles of beer; 99 x 99 cm") }) it("should not add semicolon if it already is in extent", () => { const bib = new BibDetailsModel({ @@ -174,7 +183,7 @@ describe("Bib model", () => { extent: ["700 sheets of woven gold; "], dimensions: ["1 x 1 in."], }) - expect(bib.extent.value[0]).toBe("700 sheets of woven gold; 1 x 1 in.") + expect(bib.extent[0]).toBe("700 sheets of woven gold; 1 x 1 in.") }) it("should remove semicolon if there is no dimensions", () => { const bib = new BibDetailsModel({ @@ -185,15 +194,15 @@ describe("Bib model", () => { identifier: [{ uri: "123456" }], extent: ["700 sheets of woven gold;"], }) - expect(bib.extent.value[0]).toBe("700 sheets of woven gold") - expect(anotherBib.extent.value[0]).toBe("700 sheets of woven gold") + expect(bib.extent[0]).toBe("700 sheets of woven gold") + expect(anotherBib.extent[0]).toBe("700 sheets of woven gold") }) it("should display dimensions if there are dimensions and no extent", () => { const bib = new BibDetailsModel({ identifier: [{ uri: "123456" }], dimensions: ["1,000,000mm x 7ft"], }) - expect(bib.extent.value[0]).toBe("1,000,000mm x 7ft") + expect(bib.extent[0]).toBe("1,000,000mm x 7ft") }) it("should do nothing if there are no dimensions or extent", () => { const bib = new BibDetailsModel({ identifier: [{ uri: "123456" }] }) diff --git a/src/server/api/search.ts b/src/server/api/search.ts index 16a3d7bca..b9d475ecc 100644 --- a/src/server/api/search.ts +++ b/src/server/api/search.ts @@ -38,7 +38,6 @@ export async function fetchResults( ...journalParams, q: keywordsOrBibId, } - let queryString = getSearchQuery(modifiedSearchParams) // Fall back to a single "?" in the case of an empty query @@ -55,7 +54,6 @@ export async function fetchResults( // - drb results const client = await nyplApiClient({ apiName: DISCOVERY_API_NAME }) const drbClient = await nyplApiClient({ apiName: DRB_API_NAME }) - const [resultsResponse, aggregationsResponse, drbResultsResponse] = await Promise.allSettled([ client.get(`${DISCOVERY_API_SEARCH_ROUTE}${resultsQuery}`), diff --git a/src/types/searchTypes.ts b/src/types/searchTypes.ts index 2cf22674e..482c9d1ea 100644 --- a/src/types/searchTypes.ts +++ b/src/types/searchTypes.ts @@ -1,3 +1,4 @@ +/* eslint-disable @typescript-eslint/naming-convention */ import type { DiscoveryBibResult } from "./bibTypes" import type { DRBResults } from "./drbTypes" import type { Aggregation } from "./filterTypes" @@ -29,17 +30,13 @@ export interface Identifiers { lccn?: string } -export interface SearchParams { +export interface SearchParams extends AdvancedSearchQueryParams { q?: string field?: string sortBy?: SortKey order?: SortOrder filters?: SearchFilters - contributor?: string - title?: string journalTitle?: string - standardNumber?: string - subject?: string page?: number identifiers?: Identifiers } @@ -47,8 +44,6 @@ export interface SearchParams { export type SortKey = "relevance" | "title" | "date" export type SortOrder = "asc" | "desc" -type SearchFormField = { value: string } - export interface SearchResultsResponse { results?: SearchResults aggregations?: AggregationResults @@ -86,27 +81,20 @@ export interface SearchFormAction { payload: SearchParams | SearchFilters | string | string[] } -/* eslint-disable @typescript-eslint/naming-convention */ - -export interface SearchQueryParams extends Identifiers { - q?: string +export interface AdvancedSearchQueryParams { + callnumber?: string + standard_number?: string contributor?: string title?: string subject?: string +} + +export interface SearchQueryParams + extends Identifiers, + AdvancedSearchQueryParams { + q?: string sort?: SortKey sort_direction?: SortOrder search_scope?: string page?: string } - -export interface SearchFormEvent { - q?: SearchFormField - search_scope?: SearchFormField - title?: SearchFormField - contributor?: SearchFormField - subject?: SearchFormField - language?: SearchFormField - dateBefore?: SearchFormField - dateAfter?: SearchFormField - materialType?: SearchFormField -} diff --git a/src/utils/advancedSearchUtils.ts b/src/utils/advancedSearchUtils.ts index 0ab01474e..fa8f0947d 100644 --- a/src/utils/advancedSearchUtils.ts +++ b/src/utils/advancedSearchUtils.ts @@ -5,7 +5,9 @@ import { BASE_URL } from "../config/constants" export const textInputFields: SearchFormInputField[] = [ { name: "q", label: "Keyword" }, { name: "title", label: "Title" }, - { name: "contributor", label: "Author" }, + { name: "contributor", label: "Author/contributor" }, + { name: "callnumber", label: "Call number" }, + { name: "standard_number", label: "Unique identifier" }, { name: "subject", label: "Subject" }, ] @@ -14,6 +16,8 @@ export const initialSearchFormState: SearchParams = { title: "", contributor: "", subject: "", + callnumber: "", + standard_number: "", filters: { language: "", dateBefore: "", diff --git a/src/utils/searchUtils.ts b/src/utils/searchUtils.ts index 1f2605b98..6b6f81a86 100644 --- a/src/utils/searchUtils.ts +++ b/src/utils/searchUtils.ts @@ -1,6 +1,6 @@ import { isArray, isEmpty, mapObject, forEach } from "underscore" -import { textInputFields as advSearchFields } from "./advancedSearchUtils" +import { textInputFields as advancedSearchFields } from "./advancedSearchUtils" import type { SearchParams, SearchQueryParams, @@ -66,7 +66,7 @@ export function getSearchResultsHeading( // Shows the final part of the search query string (e.g. "for keyword 'cats'") function buildQueryDisplayString(searchParams: SearchParams): string { - const searchFields = advSearchFields + const searchFields = advancedSearchFields // Lowercase the adv search field labels: .map((field) => ({ ...field, label: field.label.toLowerCase() })) .concat([ @@ -190,18 +190,16 @@ function getFilterQuery(filters: SearchFilters) { * getSearchQuery * Builds a query string from a SearchParams object */ -export function getSearchQuery({ - sortBy = "relevance", - field = "all", - order, - filters = {}, - identifiers = {}, - q, - contributor, - title, - subject, - page = 1, -}: SearchParams): string { +export function getSearchQuery(params: SearchParams): string { + const { + sortBy = "relevance", + field = "all", + order, + filters = {}, + identifiers = {}, + q, + page = 1, + } = params const searchKeywordsQuery = encodeURIComponent(q) const sortQuery = getSortQuery(sortBy, order) @@ -210,21 +208,24 @@ export function getSearchQuery({ const identifierQuery = getIdentifierQuery(identifiers) const pageQuery = page !== 1 ? `&page=${page}` : "" - // advanced search query - const contributorQuery = contributor ? `&contributor=${contributor}` : "" - const titleQuery = title ? `&title=${title}` : "" - const subjectQuery = subject ? `&subject=${subject}` : "" + const advancedSearchQueryParams = advancedSearchFields + .map(({ name: advancedSearchParam }) => { + if (advancedSearchParam === "q") return + return params[advancedSearchParam] + ? `&${advancedSearchParam}=${params[advancedSearchParam]}` + : "" + }) + .join("") // if a search_scope is "all", we want to clear the advanced search query params // and default to the q param const isAdvancedSearchOrAllFields = field.length && field === "all" const advancedQuery = isAdvancedSearchOrAllFields - ? `${contributorQuery}${titleQuery}${subjectQuery}` + ? advancedSearchQueryParams : "" const completeQuery = `${searchKeywordsQuery}${advancedQuery}${filterQuery}${sortQuery}${fieldQuery}${pageQuery}${identifierQuery}` - return completeQuery?.length ? `?q=${completeQuery}` : "" } @@ -303,6 +304,8 @@ export function mapQueryToSearchParams({ search_scope, sort_direction, page, + callnumber, + standard_number, contributor, title, subject, @@ -315,11 +318,13 @@ export function mapQueryToSearchParams({ }: SearchQueryParams): SearchParams { const hasIdentifiers = issn || isbn || oclc || lccn const filters = collapseMultiValueQueryParams(queryFilters) - + //TODO: can we merge the SearchQueryParams and SearchParams types by renaming some params? e.g. search_scope, sort, sort_direction. Also maybe passing in identifiers so it maches this pattern. return { q, field: search_scope, page: page ? parseInt(page) : 1, + callnumber, + standard_number, contributor, title, subject, diff --git a/src/utils/testUtils.tsx b/src/utils/testUtils.tsx index 102d5299b..879254a56 100644 --- a/src/utils/testUtils.tsx +++ b/src/utils/testUtils.tsx @@ -11,5 +11,7 @@ const customRender = ( options?: Omit ) => render(ui, { wrapper: AllTheProviders, ...options }) +const delay = (ms) => new Promise((res) => setTimeout(res, ms)) + export * from "@testing-library/react" -export { customRender as render } +export { customRender as render, delay }