Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIIN-3137: Set default sorting to URL in componentDidMount and componentDidUpdate if it is missing. #2687

Merged
merged 1 commit into from
Dec 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
35 changes: 12 additions & 23 deletions src/components/InstancesList/InstancesList.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
49 changes: 26 additions & 23 deletions src/components/InstancesList/InstancesList.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand All @@ -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'),
}));
});
});

Expand Down
Loading