Skip to content

Commit

Permalink
pagination: Fix disappearing pagination on back nav and refresh (#1060)
Browse files Browse the repository at this point in the history
Previously, there were two pagination persistence bugs.

Land on a vertical page with a query with results. Go to page 2. Go
to page 3. Try to navigate backwards in browser history. You will go
to page 1 instead of page 3.

Land on a vertical page w a query w results. Go to page 2. Refresh
the page. You will be on page 1 instead of page 2.

Both of these related bugs were because we were defaulting to resetting
pagination on any searchbar search. In 1, backwards navigation updates the
query and the searchbar takes the updated query and searches. In 2,
landing with a query query parameter is determined in the SearchBar
component onCreate.

To fix this, we add another value for the pre-existing queryTrigger
global storage key, QUERY_PARAMETER, meaning the query was triggered by
a query parameter.

Note: as a result of this fix, we were no longer able to go from Page 2
to Page 1. This is because of another bug, where navigating backwards in
history from presence of a query param to absence of the param will not
erase the parameter in storage. This is fixed for the pagination param here
but should be addressed more widely in SLAP-616.

J=SLAP-610
TEST=manual

Test on a local Jambo HH Theme site attached to a local dist of the SDK.
Test on a local site using the SDK directly.

Test that navigating on a vertical to Page 2 and then Page 3 allows you
to navigate back in browser history to Page 2 and Page 1 respectively.

Test that navigating to a page and then refreshing maintains pagination
offset. (IE Test that query params for search-offset work as intended).

Test that navigating to a page and then searching another query in the
searchbar /will/ reset pagination (and remove the associated query
parameter).

Test that queryTrigger in the liveapi request in all the situations
are null.

Test that with defaultInitialSearch set, queryTrigger still sends as
'initialize' in the liveapi request.
  • Loading branch information
creotutar committed Sep 14, 2020
1 parent 32c65d2 commit 6995b53
Show file tree
Hide file tree
Showing 6 changed files with 68 additions and 10 deletions.
12 changes: 11 additions & 1 deletion src/answers-umd.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import GlobalStorage from './core/storage/globalstorage';
import { AnswersComponentError, AnswersCoreError } from './core/errors/errors';
import AnalyticsEvent from './core/analytics/analyticsevent';
import StorageKeys from './core/storage/storagekeys';
import QueryTriggers from './core/models/querytriggers';
import SearchConfig from './core/models/searchconfig';
import AutoCompleteApi from './core/search/autocompleteapi';
import MockAutoCompleteService from './core/search/mockautocompleteservice';
Expand Down Expand Up @@ -161,6 +162,12 @@ class Answers {
resetListener: data => {
if (!data[StorageKeys.QUERY]) {
this.core.clearResults();
} else {
this.core.globalStorage.set(StorageKeys.QUERY_TRIGGER, QueryTriggers.QUERY_PARAMETER);
}

if (!data[StorageKeys.SEARCH_OFFSET]) {
this.core.globalStorage.set(StorageKeys.SEARCH_OFFSET, 0);
}
globalStorage.setAll(data);
}
Expand All @@ -171,6 +178,9 @@ class Answers {
globalStorage.set(StorageKeys.LOCALE, parsedConfig.locale);
globalStorage.set(StorageKeys.SESSIONS_OPT_IN, parsedConfig.sessionTrackingEnabled);
parsedConfig.noResults && globalStorage.set(StorageKeys.NO_RESULTS_CONFIG, parsedConfig.noResults);
if (globalStorage.getState(StorageKeys.QUERY)) {
globalStorage.set(StorageKeys.QUERY_TRIGGER, QueryTriggers.QUERY_PARAMETER);
}

this._masterSwitchApi = statusPage
? new MasterSwitchApi({ apiKey: parsedConfig.apiKey, ...statusPage }, globalStorage)
Expand Down Expand Up @@ -418,7 +428,7 @@ class Answers {
if (prepopulatedQuery != null) {
return;
}
this.core.globalStorage.set('queryTrigger', 'initialize');
this.core.globalStorage.set(StorageKeys.QUERY_TRIGGER, QueryTriggers.INITIALIZE);
this.core.setQuery(searchConfig.defaultInitialSearch);
}

Expand Down
29 changes: 24 additions & 5 deletions src/core/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import Navigation from './models/navigation';
import AlternativeVerticals from './models/alternativeverticals';
import DirectAnswer from './models/directanswer';
import LocationBias from './models/locationbias';
import QueryTriggers from './models/querytriggers';

import StorageKeys from './storage/storagekeys';
import AnalyticsEvent from './analytics/analyticsevent';
Expand Down Expand Up @@ -136,14 +137,17 @@ export default class Core {
this.globalStorage.set(StorageKeys.LOCATION_BIAS, {});
}

const queryTrigger = this.getQueryTriggerForSearchApi(
this.globalStorage.getState(StorageKeys.QUERY_TRIGGER)
);
return this._searcher
.verticalSearch(verticalKey, {
limit: this.globalStorage.getState(StorageKeys.SEARCH_CONFIG).limit,
geolocation: this.globalStorage.getState(StorageKeys.GEOLOCATION),
...query,
isDynamicFiltersEnabled: this._isDynamicFiltersEnabled,
skipSpellCheck: this.globalStorage.getState('skipSpellCheck'),
queryTrigger: this.globalStorage.getState('queryTrigger'),
queryTrigger: queryTrigger,
sessionTrackingEnabled: this.globalStorage.getState(StorageKeys.SESSIONS_OPT_IN),
sortBys: this.globalStorage.getState(StorageKeys.SORT_BYS)
})
Expand Down Expand Up @@ -172,7 +176,7 @@ export default class Core {
this.globalStorage.set(StorageKeys.LOCATION_BIAS, data[StorageKeys.LOCATION_BIAS]);
}
this.globalStorage.delete('skipSpellCheck');
this.globalStorage.delete('queryTrigger');
this.globalStorage.delete(StorageKeys.QUERY_TRIGGER);

const exposedParams = {
verticalKey: verticalKey,
Expand Down Expand Up @@ -229,12 +233,15 @@ export default class Core {
this.globalStorage.set(StorageKeys.SPELL_CHECK, {});
this.globalStorage.set(StorageKeys.LOCATION_BIAS, {});

const queryTrigger = this.getQueryTriggerForSearchApi(
this.globalStorage.getState(StorageKeys.QUERY_TRIGGER)
);
return this._searcher
.universalSearch(queryString, {
geolocation: this.globalStorage.getState(StorageKeys.GEOLOCATION),
skipSpellCheck: this.globalStorage.getState('skipSpellCheck'),
queryTrigger: this.globalStorage.getState('queryTrigger'),
sessionTrackingEnabled: this.globalStorage.getState(StorageKeys.SESSIONS_OPT_IN)
queryTrigger: queryTrigger,
sessionTrackingEnabled: this.globalStorage.getState(StorageKeys.SESSIONS_OPT_IN),
})
.then(response => SearchDataTransformer.transform(response, urls, this._fieldFormatters))
.then(data => {
Expand All @@ -246,7 +253,7 @@ export default class Core {
this.globalStorage.set(StorageKeys.SPELL_CHECK, data[StorageKeys.SPELL_CHECK]);
this.globalStorage.set(StorageKeys.LOCATION_BIAS, data[StorageKeys.LOCATION_BIAS]);
this.globalStorage.delete('skipSpellCheck');
this.globalStorage.delete('queryTrigger');
this.globalStorage.delete(StorageKeys.QUERY_TRIGGER);

const exposedParams = {
queryString: queryString,
Expand Down Expand Up @@ -381,6 +388,18 @@ export default class Core {
this.globalStorage.set(`${StorageKeys.FACET_FILTER}.${namespace}`, filter);
}

/**
* Returns the query trigger for the search API given the SDK query trigger
* @param {QueryTriggers} queryTrigger SDK query trigger
* @returns {QueryTriggers} query trigger if accepted by the search API, null o/w
*/
getQueryTriggerForSearchApi (queryTrigger) {
if (queryTrigger === QueryTriggers.QUERY_PARAMETER) {
return null;
}
return queryTrigger;
}

enableDynamicFilters () {
this._isDynamicFiltersEnabled = true;
}
Expand Down
12 changes: 12 additions & 0 deletions src/core/models/querytriggers.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/** @module QueryTriggers */

/**
* QueryTriggers is an ENUM of the possible triggers for a
* query update.
*
* @enum {string}
*/
export default {
INITIALIZE: 'initialize',
QUERY_PARAMETER: 'query-parameter'
};
3 changes: 2 additions & 1 deletion src/core/storage/storagekeys.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,6 @@ export default {
VERTICAL_PAGES_CONFIG: 'vertical-pages-config',
LOCALE: 'locale',
SORT_BYS: 'sort-bys',
NO_RESULTS_CONFIG: 'no-results-config'
NO_RESULTS_CONFIG: 'no-results-config',
QUERY_TRIGGER: 'queryTrigger'
};
18 changes: 17 additions & 1 deletion src/ui/components/search/searchcomponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import Component from '../component';
import DOM from '../../dom/dom';
import Filter from '../../../core/models/filter';
import StorageKeys from '../../../core/storage/storagekeys';
import QueryTriggers from '../../../core/models/querytriggers';
import SearchParams from '../../dom/searchparams';

const IconState = {
Expand Down Expand Up @@ -138,6 +139,15 @@ export default class SearchComponent extends Component {
*/
this._defaultInitialSearch = this._globalSearchConfig.defaultInitialSearch;

/**
* The default options for core search
* @type {Object}
*/
this._defaultSearchOptions = {
setQueryParams: true,
resetPagination: !!this._verticalKey
};

/**
* The query string to use for the input box, provided to template for rendering.
* Optionally provided
Expand All @@ -151,10 +161,16 @@ export default class SearchComponent extends Component {
}
if (q === null) {
if (this._defaultInitialSearch || this._defaultInitialSearch === '') {
this.core.globalStorage.set(StorageKeys.QUERY_TRIGGER, QueryTriggers.INITIALIZE);
this.core.setQuery(this._defaultInitialSearch);
}
return;
}

const queryTrigger = this.core.globalStorage.getState(StorageKeys.QUERY_TRIGGER);
const resetPagination = this._verticalKey &&
queryTrigger !== QueryTriggers.QUERY_PARAMETER &&
queryTrigger !== QueryTriggers.INITIALIZE;
this.debouncedSearch(q);
});

Expand Down Expand Up @@ -458,7 +474,7 @@ export default class SearchComponent extends Component {
this.core.persistentStorage.delete(StorageKeys.SEARCH_OFFSET);
this.core.globalStorage.delete(StorageKeys.SEARCH_OFFSET);
this.core.setQuery(query);
this.debouncedSearch(query);
this.debouncedSearch(query, this._defaultSearchOptions);
return false;
}

Expand Down
4 changes: 2 additions & 2 deletions src/ui/components/search/spellcheckcomponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default class SpellCheckComponent extends Component {

onCreate () {
this.core.persistentStorage.delete('skipSpellCheck', true);
this.core.persistentStorage.delete('queryTrigger', true);
this.core.persistentStorage.delete(StorageKeys.QUERY_TRIGGER, true);
}

setState (data, val) {
Expand All @@ -49,7 +49,7 @@ export default class SpellCheckComponent extends Component {
let params = new SearchParams(window.location.search.substring(1));
params.set('query', query.value);
params.set('skipSpellCheck', true);
params.set('queryTrigger', type.toLowerCase());
params.set(StorageKeys.QUERY_TRIGGER, type.toLowerCase());
return '?' + params.toString();
}

Expand Down

0 comments on commit 6995b53

Please sign in to comment.