Skip to content

Commit

Permalink
DCAT-2: Ensure that sort query parameters are normalized (#6)
Browse files Browse the repository at this point in the history
* DCAT-2: Normalize incoming sort value so that the order is consistent and invalid sort orders return user to default sort order
  • Loading branch information
eudoroolivares2016 authored Feb 4, 2025
1 parent b78f202 commit 6995cc0
Show file tree
Hide file tree
Showing 13 changed files with 132 additions and 45 deletions.
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
#### Running Data-Catalog locally

```bash
npm run dev
npm run start
```

#### Local Testing
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
"version": "0.0.0",
"type": "module",
"scripts": {
"dev": "vite",
"start": "vite",
"build": "tsc -b && vite build",
"lint": "eslint . --ext .tsx,.ts",
"preview": "vite preview",
Expand Down
12 changes: 10 additions & 2 deletions src/ts/component/AppliedFilters/AppliedFilters.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Button } from 'react-bootstrap'
import { omit } from 'lodash-es'

import { Facet } from '../../../types/global'
import { getValidSortkey } from '../../utils/getValidSortkey'

import getAppliedFacets from '../../utils/getAppliedFacets'

Expand Down Expand Up @@ -59,8 +60,7 @@ export const AppliedFilters: React.FC<AppliedFiltersProps> = ({
isLoading,
setQueryString
}) => {
const { temporal, bounding_box: boundingBox } = filterValues

const { temporal, bounding_box: boundingBox, sort_key: sortKey } = filterValues
const formik = useFormikContext()
const [applied, setApplied] = useState<AppliedFilter[]>([])

Expand Down Expand Up @@ -111,6 +111,14 @@ export const AppliedFilters: React.FC<AppliedFiltersProps> = ({
})
}

if (sortKey) {
const validSortKey = getValidSortkey(sortKey)
// Remove invalid sort keys to fallback to the default sort key
if (!validSortKey) {
formik.setFieldValue('sort_key', null)
}
}

setApplied(nextApplied)
}, [facets, temporal, boundingBox, isLoading, formik.dirty])

Expand Down
31 changes: 31 additions & 0 deletions src/ts/component/AppliedFilters/__tests__/AppliedFilters.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,37 @@ describe('AppliedFilters', () => {
})
})

describe('when the passing a sort_key in the filterValues', () => {
describe('when the sort key is invalid', () => {
test('calls setFieldValue to remove the sort key', () => {
const { mockFormik } = setup({
overrideProps: {
filterValues: {
sort_key: 'usage_score'
}
}
})

expect(mockFormik.setFieldValue).toHaveBeenCalledTimes(1)
expect(mockFormik.setFieldValue).toHaveBeenCalledWith('sort_key', null)
})
})

describe('when the sort key is valid', () => {
test('passes the sort key in the CMR call', () => {
const { mockFormik } = setup({
overrideProps: {
filterValues: {
sort_key: '-usage_score'
}
}
})

expect(mockFormik.setFieldValue).toHaveBeenCalledTimes(0)
})
})
})

describe('when removing a filter', () => {
test('should remove the selected filter', async () => {
const { user, props } = setup({})
Expand Down
34 changes: 6 additions & 28 deletions src/ts/component/SearchResultsList/SearchResultsList.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,10 @@ import {
} from 'react-bootstrap'
import SearchResultItem from '../SearchResultItem/SearchResultItem'

import { getValidSortkey } from '../../utils/getValidSortkey'

import { collectionSortKeys, defaultSortKey } from '../../constants/sortKeys'

interface DOI {
DOI?: string;
Authority?: string
Expand Down Expand Up @@ -73,11 +77,6 @@ interface SearchResultsListProps {
setSidebarOpened: (isOpened: boolean) => void;
}

interface SortKey {
key: string;
value: string;
}

export const SearchResultsList: React.FC<SearchResultsListProps> = ({
collections,
currentPage,
Expand All @@ -93,27 +92,6 @@ export const SearchResultsList: React.FC<SearchResultsListProps> = ({

const rows = [10, 20, 50, 100]

const descUsageScore = '-usage_score'

const sortKeys: SortKey[] = [
{
key: 'relevance',
value: 'Relevance'
},
{
key: descUsageScore,
value: 'Usage'
},
{
key: 'start_date',
value: 'Start Date'
},
{
key: 'end_date',
value: 'End Date'
}
]

const totalPages = Math.ceil(collectionCount / currentPageSize)

const handleScroll = () => {
Expand Down Expand Up @@ -257,14 +235,14 @@ export const SearchResultsList: React.FC<SearchResultsListProps> = ({
<Dropdown className="search-result-sort__dropdown">
<Dropdown.Toggle variant="none">
<span className="search-result-sort__label">SORT:</span>
<span className="search-result-sort__value">{currentSortKey === descUsageScore ? 'usage' : currentSortKey }</span>
<span className="search-result-sort__value">{currentSortKey ? getValidSortkey(currentSortKey) : defaultSortKey}</span>
<svg className="search-result-sort__svg-icon" width="10" height="10" viewBox="0 0 8 6" fill="none" xmlns="http://www.w3.org/2000/svg">
<path fillRule="evenodd" clipRule="evenodd" d="M4 3.855L7.233.6 8 1.372 4 5.4 0 1.372.767.6 4 3.855z" fill="black" />
</svg>
</Dropdown.Toggle>
<Dropdown.Menu>
{
sortKeys.map((sortKey) => {
collectionSortKeys.map((sortKey) => {
const { key, value } = sortKey

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,19 @@ describe('SearchResultList', () => {
expect(props.setQuerySort).toHaveBeenCalledWith('start_date')
})

test('calls setQuerySort with end_data', async () => {
test('calls setQuerySort with relevance', async () => {
const { props, user } = setup()

await waitFor(async () => {
await user.click(screen.getByRole('button', { name: /SORT:/ }))
await user.click(screen.getByRole('button', { name: 'Relevance' }))
})

expect(props.setQuerySort).toHaveBeenCalledTimes(1)
expect(props.setQuerySort).toHaveBeenCalledWith('relevance')
})

test('calls setQuerySort with -ongoing', async () => {
const { props, user } = setup()

await waitFor(async () => {
Expand All @@ -299,17 +311,17 @@ describe('SearchResultList', () => {
})

expect(props.setQuerySort).toHaveBeenCalledTimes(1)
expect(props.setQuerySort).toHaveBeenCalledWith('end_date')
expect(props.setQuerySort).toHaveBeenCalledWith('-ongoing')
})
})

describe('when the sort key is usage_score', () => {
test('displays usage as the current selected sort key', () => {
setup({
currentSortKey: 'usage_score'
currentSortKey: '-usage_score'
})

const sortKeyDropdown = screen.getByRole('button', { name: /SORT: usage/ })
const sortKeyDropdown = screen.getByRole('button', { name: /SORT: Usage/ })
expect(sortKeyDropdown).toBeInTheDocument()
})
})
Expand Down
27 changes: 27 additions & 0 deletions src/ts/constants/sortKeys.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* The default sort key used for sorting collections.
*/
export const defaultSortKey = 'relevance'

/**
* An array of objects representing the available sort keys for collections.
* Each object contains a `key` which is used for sorting and a `value` which is the display name.
*/
export const collectionSortKeys = [
{
key: 'relevance',
value: 'Relevance'
},
{
key: '-usage_score',
value: 'Usage'
},
{
key: 'start_date',
value: 'Start Date'
},
{
key: '-ongoing',
value: 'End Date'
}
]
8 changes: 6 additions & 2 deletions src/ts/pages/DataCatalog/DataCatalog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,16 @@ import { getConfig } from '../../utils/getConfig'
import getAppliedFacets from '../../utils/getAppliedFacets'
import { queryFacetedCollections } from '../../utils/queryFacetedCollections'
import { stringifyCollectionsQuery } from '../../utils/stringifyCollectionsQuery'
import { getValidSortkey } from '../../utils/getValidSortkey'

import LoadingBanner from '../../component/LoadingBanner/LoadingBanner'
import ErrorBanner from '../../component/ErrorBanner/ErrorBanner'
import AppliedFilters from '../../component/AppliedFilters/AppliedFilters'
import SearchFilters from '../../component/SearchFilters/SearchFilters'
import SearchResultsList from '../../component/SearchResultsList/SearchResultsList'

import { defaultSortKey } from '../../constants/sortKeys'

import {
Facet,
Params,
Expand Down Expand Up @@ -105,7 +109,7 @@ const DataCatalog: React.FC = () => {
const {
page_num: currentPage = 1,
page_size: currentPageSize = getConfig('defaultPageSize'),
sort_key: currentSortKey = 'Relevance'
sort_key: currentSortKey = defaultSortKey
} = parsedQueryString

const [collectionSearchParams, setCollectionSearchParams] = useState(parsedQueryString)
Expand Down Expand Up @@ -234,7 +238,7 @@ const DataCatalog: React.FC = () => {
const setQuerySort = (sortKey: string) => {
let updatedSearchParam = null

if (sortKey === 'relevance') {
if (!getValidSortkey(sortKey) || sortKey === defaultSortKey) {
delete collectionSearchParams.sort_key
updatedSearchParam = collectionSearchParams
} else {
Expand Down
13 changes: 8 additions & 5 deletions src/ts/pages/DataCatalog/__tests__/DataCatalog.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,10 @@ describe('DataCatalog', () => {

expect(screen.getByTestId('loading-banner__spinner')).toBeTruthy()

const searchbox = await screen.findByRole('searchbox')

await waitFor(async () => {
await user.type(screen.getByRole('searchbox'), 'C002-FAKE')
await user.type(searchbox, 'C002-FAKE')
})

setupMockResponse('keyword=C002-FAKE', 1, 1, 'Found ')
Expand Down Expand Up @@ -413,11 +415,12 @@ describe('DataCatalog', () => {
})

setupMockResponse('keyword=C002-FAKE', 1, 1, 'Found ')
const nextButton = await screen.findByRole('button', { name: 'Next' })

// Click the Next button
await waitFor(async () => {
await user.click(await screen.findByRole('button', { name: 'Next' }))
}, { timeout: 5000 })
await user.click(nextButton)
})

const pagination = await screen.findByRole('list', { name: /pagination/i })
const pageItems = within(pagination).getAllByRole('listitem')
Expand Down Expand Up @@ -471,7 +474,7 @@ describe('DataCatalog', () => {

const rowElement = screen.getByRole('button', { name: /SORT: / })

expect(within(rowElement).getByText('usage')).toBeInTheDocument()
expect(within(rowElement).getByText('Usage')).toBeInTheDocument()
})
})

Expand All @@ -487,7 +490,7 @@ describe('DataCatalog', () => {

const rowElement = screen.getByRole('button', { name: /SORT: / })

expect(within(rowElement).getByText('usage')).toBeInTheDocument()
expect(within(rowElement).getByText('Usage')).toBeInTheDocument()

await waitFor(async () => {
await user.click(screen.getByRole('button', { name: /SORT:/ }))
Expand Down
14 changes: 14 additions & 0 deletions src/ts/utils/__tests__/getvalidSortKey.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
import { getValidSortkey } from '../getValidSortkey'

describe('getCollectionSortKeys returns valid sort key configuration', () => {
test('should return the correct value for a valid sort key', () => {
expect(getValidSortkey('relevance')).toBe('Relevance')
expect(getValidSortkey('-usage_score')).toBe('Usage')
expect(getValidSortkey('start_date')).toBe('Start Date')
expect(getValidSortkey('-ongoing')).toBe('End Date')
})

test('should return null for an invalid sort key', () => {
expect(getValidSortkey('invalid_key')).toBeNull()
})
})
11 changes: 11 additions & 0 deletions src/ts/utils/getValidSortkey.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import { collectionSortKeys } from '../constants/sortKeys'
/**
* Query the list of valid collection sort keys and return the value for the key
* @param {string} sortKey The sort key to check
* @returns {string|null} The valid sort key or null
*/
export const getValidSortkey = (sortKey: string) => {
const found = collectionSortKeys.find((validSortKey) => validSortKey.key === sortKey)

return found ? found.value : null
}
2 changes: 0 additions & 2 deletions src/ts/utils/stringifyCollectionsQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ export const stringifyCollectionsQuery = (params: Params, removeDefaults = false
delete toStringify.temporal
}

// ...

if (removeDefaults) {
Object.entries(toStringify as Record<string, unknown>).forEach(([k, v]) => {
if ((k in facetDefaultParams
Expand Down
1 change: 1 addition & 0 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export default defineConfig({
environment: 'jsdom',
setupFiles: 'test-setup.ts',
clearMocks: true,
fileParallelism: false,
coverage: {
enabled: true,
include: [
Expand Down

0 comments on commit 6995cc0

Please sign in to comment.