From 1bb3c4cafc57dee90c0d4faef5fab9cc51ae08c2 Mon Sep 17 00:00:00 2001 From: Dmytro-Melnyshyn <77053927+Dmytro-Melnyshyn@users.noreply.github.com> Date: Tue, 3 Dec 2024 18:01:17 +0200 Subject: [PATCH] UIIN-3137: Set default sorting to URL in componentDidMount and componentDidUpdate if it is missing. (#2687) --- CHANGELOG.md | 4 ++ src/components/InstancesList/InstancesList.js | 35 +++++-------- .../InstancesList/InstancesList.test.js | 49 ++++++++++--------- 3 files changed, 42 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f22b9109..3c1be4acf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ * React v19: refactor away from default props for functional components. Refs UIIN-2890. * User can edit Source consortium "Holdings sources" in member tenant but not in Consortia manager. Refs UIIN-3147. +## [12.0.5] (IN PROGRESS) + +* Set default sorting to URL in componentDidMount and componentDidUpdate if it is missing. Fixes UIIN-3137. + ## [12.0.4] (IN PROGRESS) * Display informative error message when editing same instance, holdings, item in two tabs. Fixes UIIN-3127. diff --git a/src/components/InstancesList/InstancesList.js b/src/components/InstancesList/InstancesList.js index f3036fecd..7b6768ebc 100644 --- a/src/components/InstancesList/InstancesList.js +++ b/src/components/InstancesList/InstancesList.js @@ -182,7 +182,6 @@ class InstancesList extends React.Component { replace: PropTypes.func, }), getLastBrowse: PropTypes.func.isRequired, - getLastSearch: PropTypes.func.isRequired, getLastSearchOffset: PropTypes.func.isRequired, storeLastSearch: PropTypes.func.isRequired, storeLastSearchOffset: PropTypes.func.isRequired, @@ -251,9 +250,14 @@ class InstancesList extends React.Component { const searchParams = new URLSearchParams(_location.search); const isStaffSuppressFilterChanged = this.applyDefaultStaffSuppressFilter(searchParams); + let isSortingUpdated = false; - if (params.sort !== defaultSort || isStaffSuppressFilterChanged) { + if (!params.sort) { + isSortingUpdated = true; searchParams.set('sort', defaultSort); + } + + if (isSortingUpdated || isStaffSuppressFilterChanged) { this.redirectToSearchParams(searchParams); } } @@ -287,14 +291,19 @@ class InstancesList extends React.Component { const searchParams = new URLSearchParams(location.search); let isStaffSuppressFilterChanged = false; + let isSortingUpdated = false; if (prevProps.segment !== this.props.segment) { isStaffSuppressFilterChanged = this.applyDefaultStaffSuppressFilter(searchParams); } // `sort` is missing after reset button is hit - if (!sortBy || isStaffSuppressFilterChanged) { + if (!sortBy) { + isSortingUpdated = true; searchParams.set('sort', data.displaySettings.defaultSort); + } + + if (isSortingUpdated || isStaffSuppressFilterChanged) { this.redirectToSearchParams(searchParams); } @@ -390,34 +399,14 @@ class InstancesList extends React.Component { processLastSearchTermsOnMount = () => { const { getParams, - location, parentMutator, getLastSearchOffset, - getLastSearch, - storeLastSearch, segment: currentSegment, } = this.props; const params = getParams(); const lastSearchOffset = getLastSearchOffset(currentSegment); const offset = params.selectedBrowseResult === 'true' ? 0 : lastSearchOffset; - // Remove the sort option on mount from last searches, as the sort option from Settings should be used - // until it is changed there or via the result headers or actions. - const storeLastSearchWithoutSortParam = (search, segment) => { - const searchParams = new URLSearchParams(search); - searchParams.delete('sort'); - storeLastSearch(`?${searchParams.toString()}`, segment); - }; - - Object.values(segments).forEach(segment => { - if (segment === currentSegment) { - storeLastSearchWithoutSortParam(location.search, currentSegment); - } else { - const lastSegmentSearch = getLastSearch(segment); - storeLastSearchWithoutSortParam(lastSegmentSearch, segment); - } - }); - parentMutator.resultOffset.replace(offset); } diff --git a/src/components/InstancesList/InstancesList.test.js b/src/components/InstancesList/InstancesList.test.js index 9bfe18b39..0ad43ffe3 100644 --- a/src/components/InstancesList/InstancesList.test.js +++ b/src/components/InstancesList/InstancesList.test.js @@ -291,33 +291,38 @@ describe('InstancesList', () => { }); describe('when the component is mounted', () => { - it('should write location.search to the session storage and delete "sort" from all stored searches', () => { - history.push({ search: '?qindex=title&query=book&sort=title' }); + describe('and sort parameter does not match the one selected in Settings', () => { + it('should not be replaced', () => { + jest.spyOn(history, 'replace'); - const getLastSearch = jest.fn(segment => { - if (segment === segments.holdings) return '?query=foo&segment=holdings&sort=title'; - if (segment === segments.items) return '?query=foo&segment=items&sort=title'; - return '?qindex=title&query=book&sort=title'; - }); + history.push('/inventory?filters=staffSuppress.false&sort=title'); - renderInstancesList({ - segment: segments.instances, - getLastSearch, - }); + renderInstancesList({ + segment: 'instances', + data: { + ...data, + query: { + query: '', + }, + displaySettings: { + defaultSort: SORT_OPTIONS.RELEVANCE, + }, + }, + }); - expect(mockStoreLastSearch).toHaveBeenNthCalledWith(1, '?qindex=title&query=book', segments.instances); - expect(mockStoreLastSearch).toHaveBeenNthCalledWith(2, '?query=foo&segment=holdings', segments.holdings); - expect(mockStoreLastSearch).toHaveBeenNthCalledWith(3, '?query=foo&segment=items', segments.items); + expect(history.replace).not.toHaveBeenLastCalledWith(expect.objectContaining({ + search: expect.stringContaining('sort=relevance'), + })); + }); }); - describe('and sort parameter does not match the one selected in Settings', () => { - it('should call history.replace with sort parameter from Settings', () => { + describe('and sort parameter is missing', () => { + it('should call history.replace with the default sort parameter', () => { jest.spyOn(history, 'replace'); - history.push('/inventory?filters=staffSuppress.false&sort=title'); + history.push('/inventory?filters=staffSuppress.false'); renderInstancesList({ - segment: 'instances', data: { ...data, query: { @@ -329,11 +334,9 @@ describe('InstancesList', () => { }, }); - expect(history.replace).toHaveBeenLastCalledWith({ - pathname: '/inventory', - search: 'filters=staffSuppress.false&sort=relevance', - state: undefined, - }); + expect(history.replace).toHaveBeenLastCalledWith(expect.objectContaining({ + search: expect.stringContaining('sort=relevance'), + })); }); });