-
Notifications
You must be signed in to change notification settings - Fork 66
Fix default zero facets on placeholdersearch and routing #830
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
Changes from 1 commit
5e39f07
4f8a714
7750a6c
f986d17
a4c5cf9
67d5ca8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,14 +1,20 @@ | ||
| import { FacetDistribution } from '../types' | ||
| import { FacetDistribution, SearchContext } from '../types' | ||
| import { MeiliParamsCreator } from '../adapter' | ||
|
|
||
| export function cacheFirstFacetDistribution( | ||
| defaultFacetDistribution: FacetDistribution, | ||
| searchResponse: any | ||
| ): FacetDistribution { | ||
| if ( | ||
| searchResponse.query === '' && | ||
| Object.keys(defaultFacetDistribution).length === 0 | ||
| ) { | ||
| return searchResponse.facetDistribution | ||
| } | ||
| return defaultFacetDistribution | ||
| export async function cacheFirstFacetDistribution( | ||
| searchResolver: any, | ||
| searchContext: SearchContext | ||
| ): Promise<FacetDistribution> { | ||
| const meilisearchParams = MeiliParamsCreator(searchContext) | ||
| meilisearchParams.addFacets() | ||
| meilisearchParams.addAttributesToRetrieve() | ||
|
|
||
| // Search response from Meilisearch | ||
| const searchResponse = await searchResolver.searchResponse( | ||
| // placeholdersearch true to ensure a request is made | ||
| // query set to empty to ensure default facetdistributionap | ||
bidoubiwa marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| { ...searchContext, placeholderSearch: true, query: '' }, | ||
| meilisearchParams.getParams() | ||
| ) | ||
| return searchResponse.facetDistribution || {} | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| import { dataset, meilisearchClient, HOST, API_KEY } from './assets/utils' | ||
| import { instantMeiliSearch } from '../src' | ||
|
|
||
| describe('Firt facet distribution', () => { | ||
bidoubiwa marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| beforeAll(async () => { | ||
| const deleteTask = await meilisearchClient.deleteIndex('movies') | ||
| await meilisearchClient.waitForTask(deleteTask.taskUid) | ||
| await meilisearchClient | ||
| .index('movies') | ||
| .updateFilterableAttributes(['genres']) | ||
| const documentsTask = await meilisearchClient | ||
| .index('movies') | ||
| .addDocuments(dataset) | ||
| await meilisearchClient.index('movies').waitForTask(documentsTask.taskUid) | ||
| }) | ||
|
|
||
| test('creation of facet distribution without facets', async () => { | ||
| const searchClient = instantMeiliSearch(HOST, API_KEY) | ||
| const response = await searchClient.search([ | ||
| { | ||
| indexName: 'movies', | ||
| params: { | ||
| facets: [], | ||
| query: '', | ||
| }, | ||
| }, | ||
| ]) | ||
| expect(response.results[0].facets).toEqual({}) | ||
| }) | ||
|
|
||
| test('creation of facet distribution without facets and with keepZeroFacets to true', async () => { | ||
| const searchClient = instantMeiliSearch(HOST, API_KEY, { | ||
| keepZeroFacets: false, | ||
| }) | ||
bidoubiwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| const response = await searchClient.search([ | ||
| { | ||
| indexName: 'movies', | ||
| params: { | ||
| facets: [], | ||
| query: '', | ||
| }, | ||
| }, | ||
| ]) | ||
| expect(response.results[0].facets).toEqual({}) | ||
| }) | ||
bidoubiwa marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| test('creation of facet distribution with facets', async () => { | ||
| const searchClient = instantMeiliSearch(HOST, API_KEY) | ||
| const response = await searchClient.search([ | ||
| { | ||
| indexName: 'movies', | ||
| params: { | ||
| facets: ['genres'], | ||
| query: '', | ||
| }, | ||
| }, | ||
| ]) | ||
| expect(response.results[0].facets).toEqual({ | ||
| genres: { | ||
| Action: 3, | ||
| Adventure: 1, | ||
| Animation: 1, | ||
| Comedy: 2, | ||
| Crime: 4, | ||
| Drama: 1, | ||
| 'Science Fiction': 2, | ||
| Thriller: 1, | ||
| }, | ||
| }) | ||
| }) | ||
|
|
||
| test('creation of facet distribution with facets and keepZeroFacets to true', async () => { | ||
| const searchClient = instantMeiliSearch(HOST, API_KEY, { | ||
| keepZeroFacets: true, | ||
| }) | ||
| const response = await searchClient.search([ | ||
| { | ||
| indexName: 'movies', | ||
| params: { | ||
| facets: ['genres'], | ||
| query: '', | ||
| }, | ||
| }, | ||
| ]) | ||
| expect(response.results[0].facets).toEqual({ | ||
| genres: { | ||
| Action: 3, | ||
| Adventure: 1, | ||
| Animation: 1, | ||
| Comedy: 2, | ||
| Crime: 4, | ||
| Drama: 1, | ||
| 'Science Fiction': 2, | ||
| Thriller: 1, | ||
| }, | ||
| }) | ||
|
|
||
| const response2 = await searchClient.search([ | ||
| { | ||
| indexName: 'movies', | ||
| params: { | ||
| facets: ['genres'], | ||
| query: 'no results', | ||
| }, | ||
| }, | ||
| ]) | ||
|
|
||
| expect(response2.results[0].facets).toEqual({ | ||
| genres: { | ||
| Action: 0, | ||
| Adventure: 0, | ||
| Animation: 0, | ||
| Comedy: 0, | ||
| Crime: 0, | ||
| Drama: 0, | ||
| 'Science Fiction': 0, | ||
| Thriller: 0, | ||
| }, | ||
| }) | ||
| }) | ||
|
|
||
|
Comment on lines
+234
to
+257
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you can remove this, because you have the same setup/exercise in the test below.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is actually a difference for me!
Second test:
There is a cache system between the multiple calls on the same search client. These two tests ensure they are helping each other. |
||
| test('creation of facet distribution with facets, keepZeroFacets to true, and query', async () => { | ||
| const searchClient = instantMeiliSearch(HOST, API_KEY, { | ||
| keepZeroFacets: true, | ||
| }) | ||
| const response = await searchClient.search([ | ||
| { | ||
| indexName: 'movies', | ||
| params: { | ||
| facets: ['genres'], | ||
| query: 'no results', | ||
| }, | ||
| }, | ||
| ]) | ||
| expect(response.results[0].facets).toEqual({ | ||
| genres: { | ||
| Action: 0, | ||
| Adventure: 0, | ||
| Animation: 0, | ||
| Comedy: 0, | ||
| Crime: 0, | ||
| Drama: 0, | ||
| 'Science Fiction': 0, | ||
| Thriller: 0, | ||
| }, | ||
| }) | ||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -51,7 +51,7 @@ describe('Pagination browser test', () => { | |
| apiKey: '', | ||
| clientAgents: [`Meilisearch instant-meilisearch (v${PACKAGE_VERSION})`], | ||
| }) | ||
| expect(mockedSearch).toHaveBeenCalledTimes(1) | ||
| expect(mockedSearch).toHaveBeenCalledTimes(2) | ||
| }) | ||
|
|
||
| test('two different search parameters', async () => { | ||
|
|
@@ -77,7 +77,7 @@ describe('Pagination browser test', () => { | |
| apiKey: '', | ||
| clientAgents: [`Meilisearch instant-meilisearch (v${PACKAGE_VERSION})`], | ||
| }) | ||
| expect(mockedSearch).toHaveBeenCalledTimes(2) | ||
| expect(mockedSearch).toHaveBeenCalledTimes(3) | ||
| }) | ||
|
|
||
| test('two identical and one different search parameters', async () => { | ||
|
|
@@ -104,7 +104,7 @@ describe('Pagination browser test', () => { | |
| apiKey: '', | ||
| clientAgents: [`Meilisearch instant-meilisearch (v${PACKAGE_VERSION})`], | ||
| }) | ||
| expect(mockedSearch).toHaveBeenCalledTimes(2) | ||
| expect(mockedSearch).toHaveBeenCalledTimes(3) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we have +1 calls now?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because the first call always sets up the default facet distribution by making a call with zero filters (except for the facet field) to ensure we have the facet distribution of an empty search not impacted by any filter.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See #830 (comment) |
||
| }) | ||
|
|
||
| test('two same and two different search parameter', async () => { | ||
|
|
@@ -132,6 +132,6 @@ describe('Pagination browser test', () => { | |
| apiKey: '', | ||
| clientAgents: [`Meilisearch instant-meilisearch (v${PACKAGE_VERSION})`], | ||
| }) | ||
| expect(mockedSearch).toHaveBeenCalledTimes(2) | ||
| expect(mockedSearch).toHaveBeenCalledTimes(3) | ||
| }) | ||
| }) | ||
Uh oh!
There was an error while loading. Please reload this page.