Merge pull request #406 from NYPL/SCC-4326/adv-search-text-fields-2

Scc 4326/adv search text fields 2
NYPL · Dec 3, 2024 · 3b52f1f · 3b52f1f
2 parents 8c4bf84 + b77ce32
commit 3b52f1f
Show file tree

Hide file tree

Showing 7 changed files with 76 additions and 69 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -5,13 +5,21 @@ 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).
 
+
+## Prerelease
+
+### Updated
+
+- add call number and standard numbers to advanced search ([SCC-4326](https://newyorkpubliclibrary.atlassian.net/browse/SCC-4326))
+
 ## Unreleased
 
 ### Updated
 
 - Updated phone, email, notification preference and home library to be individually editable in Account Settings (SCC-4337, SCC-4254, SCC-4253)
 - Updated username to be editable in My Account header (SCC-4236)
 
+
 ## [1.3.6] 2024-11-6
 
 ## Added

diff --git a/__test__/pages/search/advancedSearchForm.test.tsx b/__test__/pages/search/advancedSearchForm.test.tsx
@@ -3,6 +3,7 @@ import { fireEvent, render, screen } 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,6 +13,9 @@ import { searchAggregations } from "../../../src/config/aggregations"
 jest.mock("next/router", () => jest.requireActual("next-router-mock"))
 
 describe("Advanced Search Form", () => {
+  beforeEach(async () => {
+    render(<AdvancedSearch isAuthenticated={true} />)
+  })
   const submit = () => {
     fireEvent(
       screen.getByTestId("submit-advanced-search-button"),
@@ -22,8 +26,6 @@ describe("Advanced Search Form", () => {
     await userEvent.click(screen.getByText("Clear fields"))
   })
   it("displays alert when no fields are submitted", () => {
-    render(<AdvancedSearch isAuthenticated={true} />)
-
     submit()
     screen.getByText(defaultEmptySearchErrorMessage)
   })
@@ -32,32 +34,36 @@ describe("Advanced Search Form", () => {
   // final input in output string in test. the broken test is
   // commented out below.
   it.todo("can set keyword, contributor, title, subject")
-  // async () => {
-  //   render(<AdvancedSearch isAuthenticated={true}/>)
+  // , async () => {
+  //
 
   //   const [keywordInput, contributorInput, titleInput, subjectInput] = [
-  //     "Keywords",
+  //     "Keyword",
   //     "Title",
   //     "Author",
   //     "Subject",
+  //     "Call number",
+  //     "Unique identifier",
   //   ].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()
+  //   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" } })
+  //   submit()
+  //   await waitFor(() =>
   //     expect(mockRouter.asPath).toBe(
   //       "/search?q=spaghetti&contributor=il+amore+di+pasta&title=strega+nonna&subject=italian+food"
   //     )
-  //   })
+  //   )
   // })
-  it("can select languages", async () => {
-    render(<AdvancedSearch isAuthenticated={true} />)
+  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 +73,6 @@ describe("Advanced Search Form", () => {
     )
   })
   it("can check material checkboxes", async () => {
-    render(<AdvancedSearch isAuthenticated={true} />)
     await userEvent.click(screen.getByLabelText("Notated music"))
     await userEvent.click(screen.getByLabelText("Cartographic"))
     submit()
@@ -78,7 +83,6 @@ describe("Advanced Search Form", () => {
     )
   })
   it("can check location checkboxes", async () => {
-    render(<AdvancedSearch isAuthenticated={true} />)
     const location = searchAggregations.buildingLocation[0]
     await userEvent.click(screen.getByLabelText(location.label as string))
     submit()
@@ -88,7 +92,6 @@ describe("Advanced Search Form", () => {
   })
 
   it("can clear the form", async () => {
-    render(<AdvancedSearch isAuthenticated={true} />)
     const notatedMusic = screen.getByLabelText("Notated music")
     await userEvent.click(notatedMusic)
     const cartographic = screen.getByLabelText("Cartographic")

diff --git a/pages/search/advanced.tsx b/pages/search/advanced.tsx
@@ -85,7 +85,6 @@ export default function AdvancedSearch({
     e.preventDefault()
     alert && setAlert(false)
     const target = e.target as HTMLInputElement
-
     dispatch({
       type: type,
       field: target.name,
@@ -106,7 +105,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 +162,7 @@ export default function AdvancedSearch({
           onSubmit={handleSubmit}
         >
           <Flex flexDirection={{ base: "column", md: "row" }}>
-            <Flex id="advancedSearchLeft" gap="s" direction="column">
+            <Flex id="advancedSearchLeft" gap="s" direction="column" grow="1">
               {textInputFields.map(({ name, label }) => {
                 return (
                   <FormField key={name}>
@@ -201,7 +199,7 @@ export default function AdvancedSearch({
               </FormField>
               <FormField>{<DateForm {...dateFormProps} />}</FormField>
             </Flex>
-            <Flex direction="column" gap="l">
+            <Flex direction="column" gap="l" grow="1">
               <SearchFilterCheckboxField
                 options={searchAggregations.buildingLocation}
                 name="location"

diff --git a/src/server/api/search.ts b/src/server/api/search.ts
@@ -37,7 +37,6 @@ export async function fetchResults(
     ...journalParams,
     q: keywordsOrBibId,
   }
-
   let queryString = getSearchQuery(modifiedSearchParams)
 
   // Fall back to a single "?" in the case of an empty query
@@ -54,7 +53,6 @@ export async function fetchResults(
   //  - drb results
   const client = await nyplApiClient()
   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
@@ -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,26 +30,20 @@ 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
 }
 
 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
@@ -7,13 +7,17 @@ export const textInputFields: SearchFormInputField[] = [
   { name: "title", label: "Title" },
   { name: "contributor", label: "Author" },
   { name: "subject", label: "Subject" },
+  { name: "callnumber", label: "Call number" },
+  { name: "standard_number", label: "Unique identifier" },
 ]
 
 export const initialSearchFormState: SearchParams = {
   q: "",
   title: "",
   contributor: "",
   subject: "",
+  callnumber: "",
+  standard_number: "",
   filters: {
     language: "",
     dateBefore: "",

diff --git a/src/utils/searchUtils.ts b/src/utils/searchUtils.ts
@@ -1,6 +1,9 @@
 import { isArray, isEmpty, mapObject, forEach } from "underscore"
 
-import { textInputFields as advSearchFields } from "./advancedSearchUtils"
+import {
+  textInputFields as advSearchFields,
+  textInputFields,
+} from "./advancedSearchUtils"
 import type {
   SearchParams,
   SearchQueryParams,
@@ -190,18 +193,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 +211,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 = textInputFields
+    .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 +307,8 @@ export function mapQueryToSearchParams({
   search_scope,
   sort_direction,
   page,
+  callnumber,
+  standard_number,
   contributor,
   title,
   subject,
@@ -315,11 +321,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,