Skip to content

Commit 254b0ed

Browse files
Merge #1161
1161: Fix bug when sort-by contains geo points r=bidoubiwa a=bidoubiwa Fixes: #1159 The old implementation did not take into account that a sorting rule could contain the `,` character and naively splited on that char. Which resulted in ``` "indexName:_geoPoint(37.8153, -122.4784):asc" ``` Being transformed into: ``` [ "indexName:_geoPoint(37.8153", "-122.4784):asc" ] ``` Instead of splitting solely on `,`, we now split on a coma if it is prepended by `asc` or `desc`. Co-authored-by: Charlotte Vermandel <[email protected]> Co-authored-by: cvermand <[email protected]>
2 parents 5ae277c + ea482aa commit 254b0ed

File tree

4 files changed

+70
-13
lines changed

4 files changed

+70
-13
lines changed

.changeset/dirty-mangos-tie.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@meilisearch/instant-meilisearch": patch
3+
---
4+
5+
Fix bug where sort-by failed on geo_points

packages/instant-meilisearch/__tests__/sort.test.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ import {
55
meilisearchClient,
66
} from './assets/utils'
77

8+
import { splitSortString } from '../src/contexts/sort-context'
9+
810
describe('Sort browser test', () => {
911
beforeAll(async () => {
1012
const deleteTask = await meilisearchClient.deleteIndex('movies')
@@ -48,4 +50,28 @@ describe('Sort browser test', () => {
4850
const hits = response.results[0].hits
4951
expect(hits.length).toBe(1)
5052
})
53+
54+
test('split multiple sorting rules', () => {
55+
const sortRules = splitSortString(
56+
'_geoPoint(37.8153, -122.4784):asc,title:asc,description:desc'
57+
)
58+
59+
expect(sortRules).toEqual([
60+
'_geoPoint(37.8153, -122.4784):asc',
61+
'title:asc',
62+
'description:desc',
63+
])
64+
})
65+
66+
test('split one sorting rule', () => {
67+
const sortRules = splitSortString('_geoPoint(37.8153, -122.4784):asc')
68+
69+
expect(sortRules).toEqual(['_geoPoint(37.8153, -122.4784):asc'])
70+
})
71+
72+
test.only('split no sorting rule', () => {
73+
const sortRules = splitSortString('')
74+
75+
expect(sortRules).toEqual([])
76+
})
5177
})

packages/instant-meilisearch/src/contexts/search-context.ts

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,25 @@ import {
33
AlgoliaMultipleQueriesQuery,
44
SearchContext,
55
} from '../types'
6-
6+
import { splitSortString } from './sort-context'
77
import { createPaginationState } from './pagination-context'
8-
import { createSortState } from './sort-context'
8+
9+
function separateIndexFromSortRules(indexName: string): {
10+
indexUid: string
11+
sortBy: string
12+
} {
13+
const colonIndex = indexName.indexOf(':')
14+
if (colonIndex === -1) {
15+
return {
16+
indexUid: indexName,
17+
sortBy: '',
18+
}
19+
}
20+
return {
21+
indexUid: indexName.substring(0, colonIndex),
22+
sortBy: indexName.substring(colonIndex + 1),
23+
}
24+
}
925

1026
/**
1127
* @param {AlgoliaMultipleQueriesQuery} searchRequest
@@ -16,23 +32,21 @@ export function createSearchContext(
1632
searchRequest: AlgoliaMultipleQueriesQuery,
1733
options: InstantMeiliSearchOptions
1834
): SearchContext {
35+
const { query, indexName, params: instantSearchParams } = searchRequest
1936
// Split index name and possible sorting rules
20-
const [indexUid, ...sortByArray] = searchRequest.indexName.split(':')
21-
const { query, params: instantSearchParams } = searchRequest
37+
const { indexUid, sortBy } = separateIndexFromSortRules(indexName)
2238

2339
const paginationState = createPaginationState(
2440
options.finitePagination,
2541
instantSearchParams?.hitsPerPage,
2642
instantSearchParams?.page
2743
)
2844

29-
const sortState = createSortState(sortByArray.join(':'))
30-
3145
const searchContext: SearchContext = {
3246
...options,
3347
query,
3448
...instantSearchParams,
35-
sort: sortState,
49+
sort: splitSortString(sortBy),
3650
indexUid,
3751
pagination: paginationState,
3852
placeholderSearch: options.placeholderSearch !== false, // true by default
Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,22 @@
11
/**
2-
* @param {string} rawSort
2+
* Split sort string into an array.
3+
*
4+
* Example:
5+
* '_geoPoint(37.8153, -122.4784):asc,title:asc,description:desc'
6+
*
7+
* becomes:
8+
* [
9+
* '_geoPoint(37.8153, -122.4784):asc',
10+
* 'title:asc',
11+
* 'description:desc',
12+
* ]
13+
*
14+
* @param {string} sortStr
315
* @returns {string[]}
416
*/
5-
export function createSortState(rawSort: string): string[] {
6-
return rawSort
7-
.split(',')
8-
.map((sort) => sort.trim())
9-
.filter((sort) => !!sort)
17+
export function splitSortString(sortStr: string): string[] {
18+
if (!sortStr) return []
19+
const sortRules = sortStr.split(/,(?=\w+:(?:asc|desc))/)
20+
21+
return sortRules
1022
}

0 commit comments

Comments
 (0)