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

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

Merged
merged 3 commits into from
May 2, 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
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
Loading