Skip to content

Commit

Permalink
fix(atomic): fix atomic-commerce-search-box needlessly requesting sug…
Browse files Browse the repository at this point in the history
…gestions when input is disabled (#4096)

Calling `searchBox.updateText()` triggers a query suggestion request in
Headless: This is fine, and generally what implementers requires the
search box to do.

However, in atomic, we've "split" the suggestion outside the searchbox
component itself into a `SuggestionManager` utility class, which should
be the one in charge of requesting suggestions when necessary.

To fix, exported `loadQuerySetActions` from Headless commerce, and use
this action directly to update the state of the search box, without
requesting `/querySuggest`.

https://coveord.atlassian.net/browse/KIT-3320

---------

Co-authored-by: Frederic Beaudoin <fbeaudoin@coveo.com>
  • Loading branch information
olamothe and fbeaudoincoveo authored Jun 26, 2024
1 parent 360b604 commit 49e9415
Show file tree
Hide file tree
Showing 10 changed files with 166 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
SearchBoxState,
buildSearchBox,
buildStandaloneSearchBox,
loadQuerySetActions,
} from '@coveo/headless/commerce';
import {
Component,
Expand Down Expand Up @@ -354,7 +355,7 @@ export class AtomicCommerceSearchBox
private updateBreakpoints = once(() => updateBreakpoints(this.host));

private async onInput(value: string) {
this.searchBox.updateText(value);
this.updateQueryWithoutQuerySuggestionTrigger(value);

if (this.isSearchDisabledForEndUser(value)) {
this.suggestionManager.clearSuggestions();
Expand All @@ -366,6 +367,9 @@ export class AtomicCommerceSearchBox
}

private async onFocus() {
if (this.isSearchDisabledForEndUser(this.searchBoxState.value)) {
return;
}
this.isExpanded = true;
await this.suggestionManager.triggerSuggestions();
this.announceNewSuggestionsToScreenReader();
Expand Down Expand Up @@ -654,6 +658,16 @@ export class AtomicCommerceSearchBox
: this.bindings.i18n.t('query-suggestions-unavailable');
}

private updateQueryWithoutQuerySuggestionTrigger(query: string) {
const {engine} = this.bindings;
engine.dispatch(
loadQuerySetActions(engine).updateQuerySetQuery({
id: this.id,
query,
})
);
}

public render() {
this.updateBreakpoints();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,11 +127,19 @@ test.describe('with instant results & query suggestions', () => {
});

test.describe('with disable-search=true and minimum-query-length=1', () => {
test.beforeEach(async ({searchBox}) => {
let querySuggestionRequestPerformed = false;

test.beforeEach(async ({page, searchBox}) => {
querySuggestionRequestPerformed = false;
page.on('request', (request) => {
if (request.url().includes('/querySuggest')) {
querySuggestionRequestPerformed = true;
}
});
await searchBox.load({
suggestionTimeout: 5000,
disableSearch: true,
minimumQueryLength: 1,
suggestionTimeout: 5000,
});
});

Expand All @@ -149,6 +157,10 @@ test.describe('with disable-search=true and minimum-query-length=1', () => {
const accessibilityResults = await makeAxeBuilder().analyze();
expect(accessibilityResults.violations).toEqual([]);
});

test('should not perform requests against the query suggest endpoint', () => {
expect(querySuggestionRequestPerformed).toBe(false);
});
};

testCases();
Expand All @@ -171,7 +183,14 @@ test.describe('with disable-search=true and minimum-query-length=1', () => {
});

test.describe('with minimum-query-length=4', () => {
test.beforeEach(async ({searchBox}) => {
let querySuggestionRequestPerformed = false;
test.beforeEach(async ({page, searchBox}) => {
querySuggestionRequestPerformed = false;
page.on('request', (request) => {
if (request.url().includes('/querySuggest')) {
querySuggestionRequestPerformed = true;
}
});
await searchBox.load({minimumQueryLength: 4, suggestionTimeout: 5000});
});

Expand Down Expand Up @@ -214,13 +233,17 @@ test.describe('with minimum-query-length=4', () => {
await page.getByPlaceholder('Search').fill('kayak');
});

test('the submit button is disabled', async ({searchBox}) => {
test('the submit button is enabled', async ({searchBox}) => {
await expect(searchBox.submitButton).toBeEnabled();
});

test('there are no search suggestions', async ({searchBox}) => {
test('there are search suggestions', async ({searchBox}) => {
await expect(searchBox.searchSuggestions().first()).toBeVisible();
});

test('should perform requests against the query suggest endpoint', () => {
expect(querySuggestionRequestPerformed).toBe(true);
});
});
});

Expand Down Expand Up @@ -261,3 +284,5 @@ test.describe('with a facet & clear-filters set to false', () => {
await expect(facets.clearFilters()).toBeVisible();
});
});

test.describe('with a facet & clear-filters set to true', () => {});
1 change: 1 addition & 0 deletions packages/headless/src/commerce.index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export * from './features/configuration/configuration-actions-loader';
export * from './features/commerce/query/query-actions-loader';
export * from './features/commerce/search-parameters/search-parameters-actions-loader';
export * from './features/commerce/product-listing-parameters/product-listing-parameters-actions-loader';
export * from './features/commerce/query-set/query-set-actions-loader';

// Selectors
export {Selectors};
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import {configuration} from '../../../app/common-reducers';
import {deselectAllBreadcrumbs} from '../../../features/breadcrumb/breadcrumb-actions';
import {
registerQuerySetQuery,
updateQuerySetQuery,
} from '../../../features/commerce/query-set/query-set-actions';
import {fetchQuerySuggestions} from '../../../features/commerce/query-suggest/query-suggest-actions';
import {queryReducer as commerceQuery} from '../../../features/commerce/query/query-slice';
import {
executeSearch,
prepareForSearchWithQuery,
} from '../../../features/commerce/search/search-actions';
import {commerceSearchReducer as commerceSearch} from '../../../features/commerce/search/search-slice';
import {
registerQuerySetQuery,
updateQuerySetQuery,
} from '../../../features/query-set/query-set-actions';
import {querySetReducer as querySet} from '../../../features/query-set/query-set-slice';
import {
registerQuerySuggest,
Expand All @@ -35,7 +35,7 @@ import {
jest.mock('../../../features/query-suggest/query-suggest-actions');
jest.mock('../../../features/commerce/query-suggest/query-suggest-actions');
jest.mock('../../../features/commerce/search/search-actions');
jest.mock('../../../features/query-set/query-set-actions');
jest.mock('../../../features/commerce/query-set/query-set-actions');
jest.mock('../../../features/facets/generic/facet-actions');
jest.mock('../../../features/breadcrumb/breadcrumb-actions');
jest.mock('../../../features/commerce/pagination/pagination-actions');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
import {CommerceEngine} from '../../../app/commerce-engine/commerce-engine';
import {configuration} from '../../../app/common-reducers';
import {stateKey} from '../../../app/state-key';
import {
registerQuerySetQuery,
updateQuerySetQuery,
} from '../../../features/commerce/query-set/query-set-actions';
import {fetchQuerySuggestions} from '../../../features/commerce/query-suggest/query-suggest-actions';
import {UpdateQueryActionCreatorPayload} from '../../../features/commerce/query/query-actions';
import {queryReducer as commerceQuery} from '../../../features/commerce/query/query-slice';
Expand All @@ -10,10 +14,6 @@ import {
prepareForSearchWithQuery,
} from '../../../features/commerce/search/search-actions';
import {commerceSearchReducer as commerceSearch} from '../../../features/commerce/search/search-slice';
import {
registerQuerySetQuery,
updateQuerySetQuery,
} from '../../../features/query-set/query-set-actions';
import {querySetReducer as querySet} from '../../../features/query-set/query-set-slice';
import {
clearQuerySuggest,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import {configuration} from '../../../app/common-reducers';
import {stateKey} from '../../../app/state-key';
import {
registerQuerySetQuery,
updateQuerySetQuery,
} from '../../../features/commerce/query-set/query-set-actions';
import {updateQuery} from '../../../features/commerce/query/query-actions';
import {queryReducer as commerceQuery} from '../../../features/commerce/query/query-slice';
import {
Expand All @@ -8,10 +12,6 @@ import {
resetStandaloneSearchBox,
} from '../../../features/commerce/standalone-search-box-set/standalone-search-box-set-actions';
import {commerceStandaloneSearchBoxSetReducer as commerceStandaloneSearchBoxSet} from '../../../features/commerce/standalone-search-box-set/standalone-search-box-set-slice';
import {
registerQuerySetQuery,
updateQuerySetQuery,
} from '../../../features/query-set/query-set-actions';
import {selectQuerySuggestion} from '../../../features/query-suggest/query-suggest-actions';
// TODO: KIT-3127: import from commerce
import {querySuggestReducer as querySuggest} from '../../../features/query-suggest/query-suggest-slice';
Expand All @@ -29,7 +29,7 @@ import {
} from './headless-standalone-search-box';
import {StandaloneSearchBoxOptions} from './headless-standalone-search-box-options';

jest.mock('../../../features/query-set/query-set-actions'); // TODO: KIT-3127: add missing commerce actions
jest.mock('../../../features/commerce/query-set/query-set-actions');
jest.mock('../../../features/query-suggest/query-suggest-actions'); // TODO: KIT-3127: add missing commerce actions
jest.mock('../../../features/commerce/query/query-actions');
jest.mock(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import {PayloadAction} from '@reduxjs/toolkit';
import {CommerceEngine} from '../../../app/commerce-engine/commerce-engine';
import {
RegisterQuerySetQueryActionCreatorPayload,
UpdateQuerySetQueryActionCreatorPayload,
} from '../../query-set/query-set-actions';
import {querySetReducer} from '../../query-set/query-set-slice';
import {registerQuerySetQuery, updateQuerySetQuery} from './query-set-actions';

export type {
RegisterQuerySetQueryActionCreatorPayload,
UpdateQuerySetQueryActionCreatorPayload,
};

/**
* The query set action creators.
*
* In Open Beta. Reach out to your Coveo team for support in adopting this.
*/
export interface QuerySetActionCreators {
/**
* Registers a query set query.
*
* @param payload - The action creator payload.
* @returns A dispatchable action.
*/
registerQuerySetQuery(
payload: RegisterQuerySetQueryActionCreatorPayload
): PayloadAction<RegisterQuerySetQueryActionCreatorPayload>;

/**
* Updates a query set query.
*
* @param payload - The action creator payload.
* @returns A dispatchable action.
*/
updateQuerySetQuery(
payload: UpdateQuerySetQueryActionCreatorPayload
): PayloadAction<UpdateQuerySetQueryActionCreatorPayload>;
}

/**
* Loads the query set reducer and returns the possible commerce query set action creators.
*
* In Open Beta. Reach out to your Coveo team for support in adopting this.
*
* @param engine - The commerce engine.
* @returns An object holding the action creators.
*/
export function loadQuerySetActions(engine: CommerceEngine) {
engine.addReducers({querySetReducer});
return {
registerQuerySetQuery,
updateQuerySetQuery,
};
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
import {createAction} from '@reduxjs/toolkit';
import {validatePayload} from '../../../utils/validate-payload';
import {
RegisterQuerySetQueryActionCreatorPayload,
UpdateQuerySetQueryActionCreatorPayload,
querySetDefinition,
} from '../../query-set/query-set-actions';

export const registerQuerySetQuery = createAction(
'commerce/querySet/register',
(payload: RegisterQuerySetQueryActionCreatorPayload) =>
validatePayload(payload, querySetDefinition)
);

export const updateQuerySetQuery = createAction(
'commerce/querySet/update',
(payload: UpdateQuerySetQueryActionCreatorPayload) =>
validatePayload(payload, querySetDefinition)
);
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
requiredEmptyAllowedString,
} from '../../utils/validate-payload';

const querySetDefinition = {
export const querySetDefinition = {
id: requiredNonEmptyString,
query: requiredEmptyAllowedString,
};
Expand Down
39 changes: 30 additions & 9 deletions packages/headless/src/features/query-set/query-set-slice.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
import {isNullOrUndefined} from '@coveo/bueno';
import {createReducer} from '@reduxjs/toolkit';
import {
registerQuerySetQuery as registerCommerceQuerySetQuery,
updateQuerySetQuery as updateCommerceQuerySetQuery,
} from '../commerce/query-set/query-set-actions';
import {
CommerceSearchParameters,
restoreSearchParameters as commerceRestoreSearchParameters,
Expand All @@ -12,23 +16,28 @@ import {
restoreSearchParameters,
} from '../search-parameters/search-parameter-actions';
import {executeSearch} from '../search/search-actions';
import {registerQuerySetQuery, updateQuerySetQuery} from './query-set-actions';
import {
RegisterQuerySetQueryActionCreatorPayload,
registerQuerySetQuery,
updateQuerySetQuery,
} from './query-set-actions';
import {getQuerySetInitialState, QuerySetState} from './query-set-state';

export const querySetReducer = createReducer(
getQuerySetInitialState(),
(builder) => {
builder
.addCase(registerQuerySetQuery, (state, action) => {
.addCase(registerQuerySetQuery, (state, action) =>
registerQuery(state, action.payload)
)
.addCase(registerCommerceQuerySetQuery, (state, action) =>
registerQuery(state, action.payload)
)
.addCase(updateQuerySetQuery, (state, action) => {
const {id, query} = action.payload;

if (id in state) {
return;
}

state[id] = query;
updateQuery(state, id, query);
})
.addCase(updateQuerySetQuery, (state, action) => {
.addCase(updateCommerceQuerySetQuery, (state, action) => {
const {id, query} = action.payload;
updateQuery(state, id, query);
})
Expand Down Expand Up @@ -78,3 +87,15 @@ const updateQuery = (state: QuerySetState, id: string, query: string) => {
state[id] = query;
}
};

const registerQuery = (
state: QuerySetState,
actionPayload: RegisterQuerySetQueryActionCreatorPayload
) => {
const {id, query} = actionPayload;
if (id in state) {
return;
}

state[id] = query;
};

0 comments on commit 49e9415

Please sign in to comment.