From 02156ec94c9ee9303c55c18a912e0c13d9a7e615 Mon Sep 17 00:00:00 2001 From: Caryl Wyatt Date: Thu, 4 Dec 2025 15:51:33 -0500 Subject: [PATCH 1/4] filter out empty form fields in search query and update ls boolean logic --- .../AdvancedSearchForm/index.svelte | 44 ++++++++++++++----- 1 file changed, 32 insertions(+), 12 deletions(-) diff --git a/src/js/components/AdvancedSearchForm/index.svelte b/src/js/components/AdvancedSearchForm/index.svelte index b1a45b2..fb28ba7 100644 --- a/src/js/components/AdvancedSearchForm/index.svelte +++ b/src/js/components/AdvancedSearchForm/index.svelte @@ -63,8 +63,9 @@ let types = $state(['ocr', 'all', 'title', 'author']); let lookFors = $state(new Array(4)); lookFors.fill(''); - let bools = $state(new Array(4)); + let bools = $state(new Array(3)); bools.fill('AND'); + bools[0] = null; let anyalls = $state(new Array(4)); anyalls.fill('all'); let isFullView = $state(true); @@ -200,28 +201,46 @@ searchParams.set('c', collid); } + const filteredLookFors = []; + const filteredTypes = []; + const filteredAnyAlls = []; + const filteredBools = []; + + for (let i = 0; i < lookFors.length; i++) { + if (lookFors[i] !== null && lookFors[i] !== '') { + filteredLookFors.push(lookFors[i]); + filteredTypes.push(types[i]); + filteredAnyAlls.push(anyalls[i]); + filteredBools.push(bools[i]); + } + } + let hasSearchTerms = false; - lookFors.forEach((value, idx) => { + filteredLookFors.forEach((value, idx) => { if (value) { searchParams.set(`q${idx + 1}`, value); hasSearchTerms = true; } }); - types.forEach((value, idx) => { - if (value && lookFors[idx]) { + filteredTypes.forEach((value, idx) => { + if (value && filteredLookFors[idx]) { searchParams.set(`field${idx + 1}`, value == 'everything' ? 'ocr' : value); } }); - anyalls.forEach((value, idx) => { - if (value && lookFors[idx]) { + filteredAnyAlls.forEach((value, idx) => { + if (value && filteredLookFors[idx]) { searchParams.set(`anyall${idx + 1}`, value); } }); - bools.forEach((value, idx) => { - if (value && lookFors[idx]) { - searchParams.set(`op${idx + 1}`, value); - } - }); + // if there's only one keyword, we don't need a boolean + if (filteredLookFors.length > 1) { + filteredBools.slice(1).forEach((value, idx) => { + if (value && filteredLookFors[idx]) { + //this is messy, op1 doesn't do anything so we start at 2 + searchParams.set(`op${idx + 2}`, value); + } + }); + } if (!hasSearchTerms) { errors.lookFors = true; @@ -341,6 +360,7 @@ lookFors[0] = params.get('q1'); types[0] = params.get('field1') || 'ocr'; anyalls[0] = params.get('anyall1') || 'all'; + bools[0] = null; lookFors[1] = params.get('q2'); types[1] = params.get('field2') || 'all'; anyalls[1] = params.get('anyall2') || 'all'; @@ -352,7 +372,7 @@ lookFors[3] = params.get('q4'); types[3] = params.get('field4') || 'author'; anyalls[3] = params.get('anyall4') || 'all'; - bools[4] = params.get('op3') || 'AND'; + bools[3] = params.get('op4') || 'AND'; if (params.get('facet_lang')) { params.getAll('facet_lang').forEach((value) => { From 7513486a56b2ab53db9de89133514ec5859dd96d Mon Sep 17 00:00:00 2001 From: Caryl Wyatt Date: Thu, 4 Dec 2025 22:19:38 -0500 Subject: [PATCH 2/4] refactor form logic for testability; add decorator and tests --- .../AdvancedSearchForm.stories.js | 71 ++++++++++++++++++- .../AdvancedSearchForm/index.svelte | 28 ++++++-- .../decorators/AdvancedSearchDecorator.svelte | 12 ++++ 3 files changed, 103 insertions(+), 8 deletions(-) create mode 100644 src/js/decorators/AdvancedSearchDecorator.svelte diff --git a/src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js b/src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js index 56ed30a..15febc1 100644 --- a/src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js +++ b/src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js @@ -1,9 +1,14 @@ import AdvancedSearchForm from './index.svelte'; -import { userEvent, within } from 'storybook/test'; +import AdvancedSearchDecorator from '../../decorators/AdvancedSearchDecorator.svelte' +import { fn, expect, waitFor } from 'storybook/test'; export default { title: 'AdvancedSearchForm', component: AdvancedSearchForm, + decorators: [() => AdvancedSearchDecorator], + args: { + mockSubmit: fn(), + }, }; export const MobileDefault = { @@ -22,4 +27,68 @@ export const DefaultDesktop = { isRotated: false, }, }, + play: async ({ args, canvas, userEvent }) => { + + await canvas.getByLabelText('Search Term 1').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 1'), 'elephant'); + await userEvent.click(canvas.getByTestId('advanced-search-submit')); + + await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled()) + + const calledUrl = args.mockSubmit.mock.calls[0][0]; + expect(calledUrl).toContain('q1=elephant') + + } }; + +export const TwoFieldsWithAnd = { + globals: { + viewport: { + value: 'bsXl', + isRotated: false, + }, + }, + play: async ({ args, canvas, userEvent }) => { + + await canvas.getByLabelText('Search Term 1').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 1'), 'elephant'); + const searchField2 = canvas.getByLabelText('Selected field 2'); + await userEvent.selectOptions(searchField2, 'Subject'); + await canvas.getByLabelText('Search Term 2').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 2'), 'conservation'); + await userEvent.click(canvas.getByTestId('advanced-search-submit')); + + await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled()) + + const calledUrl = args.mockSubmit.mock.calls[0][0]; + expect(calledUrl).toContain('lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=AND') + + } +} +export const Fields1And4WithOr = { + globals: { + viewport: { + value: 'bsXl', + isRotated: false, + }, + }, + play: async ({ args, canvas, userEvent }) => { + + await canvas.getByLabelText('Search Term 1').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 1'), 'elephant'); + const searchField4 = canvas.getByLabelText('Selected field 4'); + await userEvent.selectOptions(searchField4, 'Subject'); + await canvas.getByLabelText('Search Term 4').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 4'), 'conservation'); + const thirdRadioGroupOr = canvas.getAllByRole('radio', { name: 'OR' })[2]; + await userEvent.click(thirdRadioGroupOr); + await userEvent.click(canvas.getByTestId('advanced-search-submit')); + + await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled()) + + const calledUrl = args.mockSubmit.mock.calls[0][0]; + expect(calledUrl).toContain('lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=OR') + + } +} + diff --git a/src/js/components/AdvancedSearchForm/index.svelte b/src/js/components/AdvancedSearchForm/index.svelte index fb28ba7..a44045a 100644 --- a/src/js/components/AdvancedSearchForm/index.svelte +++ b/src/js/components/AdvancedSearchForm/index.svelte @@ -18,7 +18,14 @@ */ /** @type {Props} */ - let { formatData = [], languageData = [], locationData = [], collid = null, collectionName = null } = $props(); + let { + formatData = [], + languageData = [], + locationData = [], + collid = null, + collectionName = null, + mockSubmit = null, + } = $props(); // export let useAnyAll = true; @@ -58,12 +65,10 @@ let lang = $state([]); let format = $state([]); let originalLocation = $state(''); - let modal; - // let types = new Array(4); types.fill('ocr'); let types = $state(['ocr', 'all', 'title', 'author']); let lookFors = $state(new Array(4)); lookFors.fill(''); - let bools = $state(new Array(3)); + let bools = $state(new Array(4)); bools.fill('AND'); bools[0] = null; let anyalls = $state(new Array(4)); @@ -89,7 +94,7 @@ } } - function submitForm(event) { + function buildSearchUrl(event) { // which are we targeting? errors.lookFors = false; errors.yop = false; @@ -294,7 +299,16 @@ return; } - location.assign(url.toString()); + return url.toString(); + } + + function submitForm(event) { + const url = buildSearchUrl(event); + if (mockSubmit) { + mockSubmit(url); + } else { + location.assign(url); + } } onMount(() => { @@ -599,7 +613,7 @@ {/each}
- diff --git a/src/js/decorators/AdvancedSearchDecorator.svelte b/src/js/decorators/AdvancedSearchDecorator.svelte new file mode 100644 index 0000000..cc28d8b --- /dev/null +++ b/src/js/decorators/AdvancedSearchDecorator.svelte @@ -0,0 +1,12 @@ + + +
+ {@render children()} +
From 998b40d9c07bef130381c6d1bc5c5d0598ab2ea8 Mon Sep 17 00:00:00 2001 From: Caryl Wyatt Date: Fri, 12 Dec 2025 14:04:15 -0500 Subject: [PATCH 3/4] update bool logic for ls and catalog searches --- .../AdvancedSearchForm/index.svelte | 61 +++++++++++-------- 1 file changed, 37 insertions(+), 24 deletions(-) diff --git a/src/js/components/AdvancedSearchForm/index.svelte b/src/js/components/AdvancedSearchForm/index.svelte index a44045a..8d848ee 100644 --- a/src/js/components/AdvancedSearchForm/index.svelte +++ b/src/js/components/AdvancedSearchForm/index.svelte @@ -109,6 +109,36 @@ break; } } + + const filteredLookFors = []; + const filteredTypes = []; + const filteredAnyAlls = []; + const filteredBools = [null]; //first will be null + + let lastValidIndex = null; + + for (let i = 0; i < lookFors.length; i++) { + if (lookFors[i] !== null && lookFors[i] !== '') { + filteredLookFors.push(lookFors[i]); + filteredTypes.push(types[i]); + filteredAnyAlls.push(anyalls[i]); + + // use OR if it's the connecting bool between two lines, otherwise use AND + if (lastValidIndex !== null) { + let connectingBool = 'AND'; //default to AND + for (let j = lastValidIndex + 1; j <= i; j++) { + if (bools[j] === 'OR') { + connectingBool = 'OR'; + break; + } + } + filteredBools.push(connectingBool); + } + + lastValidIndex = i; + } + } + if (target == 'catalog') { url = new URL(`${protocol}//${HT.catalog_domain}/Search/Home`); let searchParams = new URLSearchParams(); @@ -172,11 +202,9 @@ } } - bools.forEach((value, idx) => { - if (idx === 0) { - return; - } - if (value && lookFors[idx] && lookFors[idx - 1]) { + filteredBools.forEach((value, idx) => { + if (value && filteredLookFors[idx]) { + console.log('bool index has value ' + value + ', lookfor ' + idx, filteredLookFors[idx]); searchParams.append('bool[]', value); } }); @@ -206,20 +234,6 @@ searchParams.set('c', collid); } - const filteredLookFors = []; - const filteredTypes = []; - const filteredAnyAlls = []; - const filteredBools = []; - - for (let i = 0; i < lookFors.length; i++) { - if (lookFors[i] !== null && lookFors[i] !== '') { - filteredLookFors.push(lookFors[i]); - filteredTypes.push(types[i]); - filteredAnyAlls.push(anyalls[i]); - filteredBools.push(bools[i]); - } - } - let hasSearchTerms = false; filteredLookFors.forEach((value, idx) => { if (value) { @@ -239,12 +253,11 @@ }); // if there's only one keyword, we don't need a boolean if (filteredLookFors.length > 1) { - filteredBools.slice(1).forEach((value, idx) => { - if (value && filteredLookFors[idx]) { - //this is messy, op1 doesn't do anything so we start at 2 - searchParams.set(`op${idx + 2}`, value); + for (let idx = 1; idx < filteredBools.length; idx++) { + if (filteredBools[idx] && filteredLookFors[idx]) { + searchParams.set(`op${idx + 1}`, filteredBools[idx]); } - }); + } } if (!hasSearchTerms) { From b832ccee14054e1da24a0fd59e2a7dfb48c84d2f Mon Sep 17 00:00:00 2001 From: Caryl Wyatt Date: Fri, 12 Dec 2025 14:19:21 -0500 Subject: [PATCH 4/4] add tests for full text variants and catalog --- .../AdvancedSearchForm.stories.js | 56 ++++++++++++++++++- .../AdvancedSearchForm/index.svelte | 1 - .../decorators/AdvancedSearchDecorator.svelte | 1 + 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js b/src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js index 15febc1..82ac646 100644 --- a/src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js +++ b/src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js @@ -65,7 +65,7 @@ export const TwoFieldsWithAnd = { } } -export const Fields1And4WithOr = { +export const FullTextFields1And4WithLastOr = { globals: { viewport: { value: 'bsXl', @@ -92,3 +92,57 @@ export const Fields1And4WithOr = { } } +export const FullTextFields1And4WithFirstOr = { + globals: { + viewport: { + value: 'bsXl', + isRotated: false, + }, + }, + play: async ({ args, canvas, userEvent }) => { + + await canvas.getByLabelText('Search Term 1').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 1'), 'elephant'); + const firstRadioGroupOr = canvas.getAllByRole('radio', { name: 'OR' })[0]; + await userEvent.click(firstRadioGroupOr); + const searchField4 = canvas.getByLabelText('Selected field 4'); + await userEvent.selectOptions(searchField4, 'Subject'); + await canvas.getByLabelText('Search Term 4').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 4'), 'conservation'); + await userEvent.click(canvas.getByTestId('advanced-search-submit')); + + await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled()) + + const calledUrl = args.mockSubmit.mock.calls[0][0]; + expect(calledUrl).toContain('lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=OR') + + } +} +export const CatalogFields1And4WithLastOr = { + globals: { + viewport: { + value: 'bsXl', + isRotated: false, + }, + }, + play: async ({ args, canvas, userEvent }) => { + + const searchField1 = canvas.getByLabelText('Selected field 1'); + await userEvent.selectOptions(searchField1, 'Title'); + await canvas.getByLabelText('Search Term 1').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 1'), 'apple'); + const searchField4 = canvas.getByLabelText('Selected field 4'); + await userEvent.selectOptions(searchField4, 'Author'); + await canvas.getByLabelText('Search Term 4').focus(); + await userEvent.type(canvas.getByLabelText('Search Term 4'), 'orange'); + const thirdRadioGroupOr = canvas.getAllByRole('radio', { name: 'OR' })[2]; + await userEvent.click(thirdRadioGroupOr); + await userEvent.click(canvas.getByTestId('advanced-search-submit')); + + await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled()) + + const calledUrl = args.mockSubmit.mock.calls[0][0]; + expect(calledUrl).toContain('adv=1&setft=true&ft=ft&lookfor%5B%5D=apple&lookfor%5B%5D=orange&type%5B%5D=title&type%5B%5D=author&bool%5B%5D=OR') + + } +} diff --git a/src/js/components/AdvancedSearchForm/index.svelte b/src/js/components/AdvancedSearchForm/index.svelte index 8d848ee..d0d10a1 100644 --- a/src/js/components/AdvancedSearchForm/index.svelte +++ b/src/js/components/AdvancedSearchForm/index.svelte @@ -204,7 +204,6 @@ filteredBools.forEach((value, idx) => { if (value && filteredLookFors[idx]) { - console.log('bool index has value ' + value + ', lookfor ' + idx, filteredLookFors[idx]); searchParams.append('bool[]', value); } }); diff --git a/src/js/decorators/AdvancedSearchDecorator.svelte b/src/js/decorators/AdvancedSearchDecorator.svelte index cc28d8b..7b2a155 100644 --- a/src/js/decorators/AdvancedSearchDecorator.svelte +++ b/src/js/decorators/AdvancedSearchDecorator.svelte @@ -3,6 +3,7 @@ const HT = {}; HT.service_domain = 'babel.hathitrust.org'; + HT.catalog_domain = 'catalog.hathitrust.org'; globalThis.HT = HT;