Skip to content

Commit

Permalink
fix(sanity): do not order by _updatedAt when relevance ordering is …
Browse files Browse the repository at this point in the history
…used with Text Search API search strategy (#6537)

* fix(sanity): do not order by `_updatedAt` when relevance ordering is used with Text Search API search strategy

* fix(sanity): findability comment

* test(sanity): ensure ordering is applied correctly
  • Loading branch information
juice49 committed May 2, 2024
1 parent 7df5396 commit 0ede4cf
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ export const createTextSearch: SearchStrategyFactory<TextSearchResults> = (
...searchTerms.params,
},
types: getDocumentTypeConfiguration(searchOptions, searchTerms),
order: getOrder(searchOptions.sort),
...(searchOptions.sort ? {order: getOrder(searchOptions.sort)} : {}),
includeAttributes: ['_id', '_type'],
fromCursor: searchOptions.cursor,
limit: searchOptions.limit ?? DEFAULT_LIMIT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,28 +2,19 @@ import {SortIcon} from '@sanity/icons'
import {Card, Flex, Menu, MenuDivider} from '@sanity/ui'
import {isEqual} from 'lodash'
import {useCallback, useId, useMemo} from 'react'
import {useWorkspace} from 'sanity'
import {styled} from 'styled-components'

import {Button, MenuButton, MenuItem} from '../../../../../../ui-components'
import {useTranslation} from '../../../../../i18n'
import {useSearchState} from '../contexts/search/useSearchState'
import {ORDERINGS} from '../definitions/orderings'
import {getOrderings} from '../definitions/getOrderings'
import {type SearchOrdering} from '../types'

interface SearchDivider {
type: 'divider'
}

const MENU_ORDERINGS: (SearchDivider | SearchOrdering)[] = [
ORDERINGS.relevance,
{type: 'divider'},
ORDERINGS.createdAsc,
ORDERINGS.createdDesc,
{type: 'divider'},
ORDERINGS.updatedAsc,
ORDERINGS.updatedDesc,
]

const SortMenuContentFlex = styled(Flex)`
box-sizing: border-box;
`
Expand Down Expand Up @@ -57,13 +48,27 @@ function CustomMenuItem({ordering}: {ordering: SearchOrdering}) {

export function SortMenu() {
const {t} = useTranslation()
const {enableLegacySearch = false} = useWorkspace().search
const {
state: {ordering},
} = useSearchState()

const menuButtonId = useId()

const currentMenuItem = MENU_ORDERINGS.find(
const menuOrderings: (SearchDivider | SearchOrdering)[] = useMemo(() => {
const orderings = getOrderings({enableLegacySearch})
return [
orderings.relevance,
{type: 'divider'},
orderings.createdAsc,
orderings.createdDesc,
{type: 'divider'},
orderings.updatedAsc,
orderings.updatedDesc,
]
}, [enableLegacySearch])

const currentMenuItem = menuOrderings.find(
(item): item is SearchOrdering => isEqual(ordering, item) && !isSearchDivider(item),
)

Expand All @@ -79,7 +84,7 @@ export function SortMenu() {
id={menuButtonId || ''}
menu={
<Menu>
{MENU_ORDERINGS.map((item, index) => {
{menuOrderings.map((item, index) => {
if (isSearchDivider(item)) {
// eslint-disable-next-line react/no-array-index-key
return <MenuDivider key={index} />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ export function SearchProvider({children, fullscreen}: SearchProviderProps) {
const schema = useSchema()
const currentUser = useCurrentUser()
const {
search: {operators, filters},
search: {operators, filters, enableLegacySearch},
} = useSource()

// Create field, filter and operator dictionaries
Expand All @@ -60,8 +60,16 @@ export function SearchProvider({children, fullscreen}: SearchProviderProps) {
cursor: null,
nextCursor: null,
},
enableLegacySearch,
}),
[currentUser, fieldDefinitions, filterDefinitions, fullscreen, operatorDefinitions],
[
currentUser,
fieldDefinitions,
filterDefinitions,
fullscreen,
operatorDefinitions,
enableLegacySearch,
],
)
const [state, dispatch] = useReducer(searchReducer, initialState)

Expand Down Expand Up @@ -112,9 +120,13 @@ export function SearchProvider({children, fullscreen}: SearchProviderProps) {
const termsChanged = !isEqual(terms, previousTermsRef.current)

if (orderingChanged || cursorChanged || termsChanged) {
// Use a custom label if provided, otherwise return field and direction, e.g. `_updatedAt desc`
const sortLabel =
ordering?.customMeasurementLabel || `${ordering.sort.field} ${ordering.sort.direction}`
let sortLabel = 'findability-sort:'

if (ordering?.customMeasurementLabel || ordering.sort) {
// Use a custom label if provided, otherwise return field and direction, e.g. `_updatedAt desc`
sortLabel +=
ordering?.customMeasurementLabel || `${ordering.sort?.field} ${ordering.sort?.direction}`
}

handleSearch({
options: {
Expand All @@ -124,13 +136,13 @@ export function SearchProvider({children, fullscreen}: SearchProviderProps) {
? [`findability-recent-search:${terms.__recent.index}`]
: []),
`findability-selected-types:${terms.types.length}`,
`findability-sort:${sortLabel}`,
sortLabel,
`findability-source: global`,
`findability-filter-count:${completeFilters.length}`,
],
limit: SEARCH_LIMIT,
skipSortByScore: ordering.ignoreScore,
sort: [ordering.sort],
...(ordering.sort ? {sort: [ordering.sort]} : {}),
cursor: cursor || undefined,
},
terms: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {useReducer} from 'react'
import {type RecentSearch} from '../../datastores/recentSearches'
import {type SearchOrdering} from '../../types'
import {
type InitialSearchState,
initialSearchState,
type SearchAction,
searchReducer,
Expand Down Expand Up @@ -39,12 +40,15 @@ const recentSearchTerms = {
query: 'foo',
types: [],
} as RecentSearch

const initialStateContext: InitialSearchState = {
currentUser: mockUser,
definitions: {fields: {}, filters: {}, operators: {}},
pagination: {cursor: null, nextCursor: null},
}

const initialState: SearchReducerState = {
...initialSearchState({
currentUser: mockUser,
definitions: {fields: {}, filters: {}, operators: {}},
pagination: {cursor: null, nextCursor: null},
}),
...initialSearchState(initialStateContext),
terms: recentSearchTerms,
}

Expand Down Expand Up @@ -119,6 +123,52 @@ describe('searchReducer', () => {
expect((state.terms as RecentSearch).__recent).toBeUndefined()
})

it('should not include an order when using Text Search API strategy and ordering by relevance', () => {
const {result} = renderHook(() =>
useReducer(
searchReducer,
initialSearchState({
...initialStateContext,
enableLegacySearch: false,
}),
),
)

const [state] = result.current

expect(state.ordering).toMatchInlineSnapshot(`
Object {
"customMeasurementLabel": "relevance",
"titleKey": "search.ordering.best-match-label",
}
`)
})

it('should order by `_updatedAt` when using GROQ Query API strategy and ordering by relevance', () => {
const {result} = renderHook(() =>
useReducer(
searchReducer,
initialSearchState({
...initialStateContext,
enableLegacySearch: true,
}),
),
)

const [state] = result.current

expect(state.ordering).toMatchInlineSnapshot(`
Object {
"customMeasurementLabel": "relevance",
"sort": Object {
"direction": "desc",
"field": "_updatedAt",
},
"titleKey": "search.ordering.best-match-label",
}
`)
})

it('should merge results after fetching an additional page', () => {
const {result} = renderHook(() => useReducer(searchReducer, initialState))
const [, dispatch] = result.current
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import {getPublishedId} from '../../../../../../util'
import {type RecentSearch} from '../../datastores/recentSearches'
import {type SearchFieldDefinitionDictionary} from '../../definitions/fields'
import {type SearchFilterDefinitionDictionary} from '../../definitions/filters'
import {getOrderings} from '../../definitions/getOrderings'
import {
getOperatorDefinition,
getOperatorInitialValue,
type SearchOperatorDefinitionDictionary,
} from '../../definitions/operators'
import {ORDERINGS} from '../../definitions/orderings'
import {type SearchFilter, type SearchOrdering} from '../../types'
import {debugWithName, isDebugMode} from '../../utils/debug'
import {
Expand Down Expand Up @@ -40,6 +40,7 @@ export type SearchReducerState = PaginationState & {
ordering: SearchOrdering
result: SearchResult
terms: RecentSearch | SearchTerms
enableLegacySearch?: boolean
}

export interface SearchDefinitions {
Expand All @@ -61,13 +62,15 @@ export interface InitialSearchState {
fullscreen?: boolean
definitions: SearchDefinitions
pagination: PaginationState
enableLegacySearch?: boolean
}

export function initialSearchState({
currentUser,
fullscreen,
definitions,
pagination,
enableLegacySearch,
}: InitialSearchState): SearchReducerState {
return {
currentUser,
Expand All @@ -77,7 +80,7 @@ export function initialSearchState({
filtersVisible: true,
fullscreen,
lastActiveIndex: -1,
ordering: ORDERINGS.relevance,
ordering: getOrderings({enableLegacySearch}).relevance,
...pagination,
result: {
error: null,
Expand All @@ -91,6 +94,7 @@ export function initialSearchState({
types: [],
},
definitions,
enableLegacySearch,
}
}

Expand Down Expand Up @@ -173,7 +177,7 @@ export function searchReducer(state: SearchReducerState, action: SearchAction):
case 'ORDERING_RESET':
return {
...state,
ordering: ORDERINGS.relevance,
ordering: getOrderings({enableLegacySearch: state.enableLegacySearch}).relevance,
terms: stripRecent(state.terms),
result: {
...state.result,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import {type SearchOrdering} from '../types'

export const ORDERINGS: Record<string, SearchOrdering> = {
export const getOrderings: (context: {
enableLegacySearch?: boolean
}) => Record<string, SearchOrdering> = ({enableLegacySearch}) => ({
createdAsc: {
ignoreScore: true,
sort: {direction: 'asc', field: '_createdAt'},
Expand All @@ -13,7 +15,7 @@ export const ORDERINGS: Record<string, SearchOrdering> = {
},
relevance: {
customMeasurementLabel: 'relevance',
sort: {direction: 'desc', field: '_updatedAt'},
...(enableLegacySearch ? {sort: {direction: 'desc', field: '_updatedAt'}} : {}),
titleKey: 'search.ordering.best-match-label',
},
updatedAsc: {
Expand All @@ -26,4 +28,4 @@ export const ORDERINGS: Record<string, SearchOrdering> = {
sort: {direction: 'desc', field: '_updatedAt'},
titleKey: 'search.ordering.updated-descending-label',
},
}
})
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export type SearchFilterValues = {
export interface SearchOrdering {
customMeasurementLabel?: string
ignoreScore?: boolean
sort: SearchSort
sort?: SearchSort
/**
* i18n key for title
*/
Expand Down

0 comments on commit 0ede4cf

Please sign in to comment.