Merge #916

916: Multi-index search: fix placeholdersearch not working properly with filters and pagination r=bidoubiwa a=bidoubiwa

`placeholderSearch` lets you enable (default) or disable the rendering of the hits when an empty search is made.
An empty search is a search without any query and without any filters.

When `placeholderSearch` was set to false, hits did not render when a filter was required.
When `placeholderSearch` was set to false, the pagination was rendered of is placeholder search was set to true.

With this PR:

- When placeholderSearch is set to false, If you add a filter, by clicking on a facet for example, hits are rendered
- When placeholderSearch is set to false, when the query and the filters are empty, the pagination does not render any page. 
    - In finitePagination this is what is rendered:
        <img width="120" alt="Screenshot 2022-12-08 at 15 54 54" src="https://user-images.githubusercontent.com/33010418/206478647-a9922af9-2a24-4197-86d3-5b3377e99e0f.png">
   - in scroll pagination: the load more button is disabled
        <img width="113" alt="Screenshot 2022-12-08 at 15 55 51" src="https://user-images.githubusercontent.com/33010418/206478859-f142df89-fce6-4dc3-b358-a641c9be045e.png">




Co-authored-by: Charlotte Vermandel <[email protected]>

src/adapter/search-request-adapter/__tests__/search-params.tests.ts

@@ -5,6 +5,8 @@ const DEFAULT_CONTEXT = {
   indexUid: 'test',
   pagination: { page: 0, hitsPerPage: 6, finite: false },
   defaultFacetDistribution: {},
+  placeholderSearch: true,
+  keepZeroFacets: false,
 }
 
 describe('Parameters adapter', () => {
@@ -155,6 +157,31 @@ describe('Pagination adapter', () => {
     expect(searchParams.hitsPerPage).toBe(0)
   })
 
+  test('adapting a finite pagination with no placeholderSearch and a query', () => {
+    const searchParams = adaptSearchParams({
+      ...DEFAULT_CONTEXT,
+      query: 'a',
+      pagination: { page: 4, hitsPerPage: 6, finite: true },
+      placeholderSearch: false,
+    })
+
+    expect(searchParams.page).toBe(5)
+    expect(searchParams.hitsPerPage).toBeGreaterThan(0)
+  })
+
+  test('adapting a finite pagination with no placeholderSearch and a facetFilter', () => {
+    const searchParams = adaptSearchParams({
+      ...DEFAULT_CONTEXT,
+      query: '',
+      pagination: { page: 4, hitsPerPage: 6, finite: true },
+      placeholderSearch: false,
+      facetFilters: ['genres:Action'],
+    })
+
+    expect(searchParams.page).toBe(5)
+    expect(searchParams.hitsPerPage).toBeGreaterThan(0)
+  })
+
   test('adapting a scroll pagination with no placeholderSearch', () => {
     const searchParams = adaptSearchParams({
       ...DEFAULT_CONTEXT,
@@ -166,4 +193,29 @@ describe('Pagination adapter', () => {
     expect(searchParams.limit).toBe(0)
     expect(searchParams.offset).toBe(0)
   })
+
+  test('adapting a scroll pagination with no placeholderSearch and a query', () => {
+    const searchParams = adaptSearchParams({
+      ...DEFAULT_CONTEXT,
+      query: 'a',
+      pagination: { page: 4, hitsPerPage: 6, finite: false },
+      placeholderSearch: false,
+    })
+
+    expect(searchParams.limit).toBeGreaterThan(0)
+    expect(searchParams.offset).toBeGreaterThan(0)
+  })
+
+  test('adapting a scroll pagination with no placeholderSearch and a facetFilter', () => {
+    const searchParams = adaptSearchParams({
+      ...DEFAULT_CONTEXT,
+      query: 'a',
+      pagination: { page: 4, hitsPerPage: 6, finite: false },
+      placeholderSearch: false,
+      facetFilters: ['genres:Action'],
+    })
+
+    expect(searchParams.limit).toBeGreaterThan(0)
+    expect(searchParams.offset).toBeGreaterThan(0)
+  })
 })

src/adapter/search-request-adapter/search-params-adapter.ts

@@ -1,18 +1,38 @@
-import type { MeiliSearchParams, SearchContext } from '../../types'
+import type {
+  MeiliSearchParams,
+  SearchContext,
+  Filter,
+  PaginationState,
+} from '../../types'
 
 import {
   adaptGeoPointsRules,
   createGeoSearchContext,
 } from './geo-rules-adapter'
 import { adaptFilters } from './filter-adapter'
 
-function setScrollPagination(
-  hitsPerPage: number,
-  page: number,
+function isPaginationRequired(
+  filter: Filter,
   query?: string,
   placeholderSearch?: boolean
+): boolean {
+  // To disable pagination:
+  // placeholderSearch must be disabled
+  // The search query must be empty
+  // There must be no filters
+  if (!placeholderSearch && !query && (!filter || filter.length === 0)) {
+    return false
+  }
+  return true
+}
+
+function setScrollPagination(
+  pagination: PaginationState,
+  paginationRequired: boolean
 ): { limit: number; offset: number } {
-  if (!placeholderSearch && query === '') {
+  const { page, hitsPerPage } = pagination
+
+  if (!paginationRequired) {
     return {
       limit: 0,
       offset: 0,
@@ -26,12 +46,12 @@ function setScrollPagination(
 }
 
 function setFinitePagination(
-  hitsPerPage: number,
-  page: number,
-  query?: string,
-  placeholderSearch?: boolean
+  pagination: PaginationState,
+  paginationRequired: boolean
 ): { hitsPerPage: number; page: number } {
-  if (!placeholderSearch && query === '') {
+  const { page, hitsPerPage } = pagination
+
+  if (!paginationRequired) {
     return {
       hitsPerPage: 0,
       page: page + 1,
@@ -58,9 +78,6 @@ export function MeiliParamsCreator(searchContext: SearchContext) {
     attributesToSnippet,
     snippetEllipsisText,
     attributesToRetrieve,
-    filters,
-    numericFilters,
-    facetFilters,
     attributesToHighlight,
     highlightPreTag,
     highlightPostTag,
@@ -69,8 +86,13 @@ export function MeiliParamsCreator(searchContext: SearchContext) {
     sort,
     pagination,
     matchingStrategy,
+    filters,
+    numericFilters,
+    facetFilters,
   } = searchContext
 
+  const meilisearchFilters = adaptFilters(filters, numericFilters, facetFilters)
+
   return {
     getParams() {
       return meiliSearchParams
@@ -99,9 +121,8 @@ export function MeiliParamsCreator(searchContext: SearchContext) {
       }
     },
     addFilters() {
-      const filter = adaptFilters(filters, numericFilters, facetFilters)
-      if (filter.length) {
-        meiliSearchParams.filter = filter
+      if (meilisearchFilters.length) {
+        meiliSearchParams.filter = meilisearchFilters
       }
     },
     addAttributesToHighlight() {
@@ -122,21 +143,22 @@ export function MeiliParamsCreator(searchContext: SearchContext) {
       }
     },
     addPagination() {
+      const paginationRequired = isPaginationRequired(
+        meilisearchFilters,
+        query,
+        placeholderSearch
+      )
       if (pagination.finite) {
         const { hitsPerPage, page } = setFinitePagination(
-          pagination.hitsPerPage,
-          pagination.page,
-          query,
-          placeholderSearch
+          pagination,
+          paginationRequired
         )
         meiliSearchParams.hitsPerPage = hitsPerPage
         meiliSearchParams.page = page
       } else {
         const { limit, offset } = setScrollPagination(
-          pagination.hitsPerPage,
-          pagination.page,
-          query,
-          placeholderSearch
+          pagination,
+          paginationRequired
         )
         meiliSearchParams.limit = limit
         meiliSearchParams.offset = offset

src/adapter/search-request-adapter/search-resolver.ts

@@ -25,8 +25,6 @@ export function SearchResolver(
       searchContext: SearchContext,
       searchParams: MeiliSearchParams
     ): Promise<MeiliSearchResponse<Record<string, any>>> {
-      const { placeholderSearch, query } = searchContext
-
       // Create cache key containing a unique set of search parameters
       const key = cache.formatKey([
         searchParams,
@@ -54,13 +52,9 @@ export function SearchResolver(
         )
       }
 
-      // query can be: empty string, undefined or null
-      // all of them are falsy's
-      if (!placeholderSearch && !query) {
-        searchResponse.hits = []
-      }
       // Cache response
       cache.setEntry<MeiliSearchResponse>(key, searchResponse)
+
       return searchResponse
     },
   }

src/types/types.ts

@@ -78,11 +78,11 @@ export type SearchContext = Omit<InstantSearchParams, 'insideBoundingBox'> &
     defaultFacetDistribution: FacetDistribution
     pagination: PaginationState
     indexUid: string
+    placeholderSearch: boolean
+    keepZeroFacets: boolean
     insideBoundingBox?: InsideBoundingBox
-    keepZeroFacets?: boolean
     cropMarker?: string
     sort?: string
-    placeholderSearch?: boolean
     primaryKey?: string
     matchingStrategy?: MatchingStrategies
   }

tests/placeholder-search.tests.ts

@@ -51,4 +51,48 @@ describe('Pagination browser test', () => {
     const hits = response.results[0].hits
     expect(hits.length).toBe(0)
   })
+
+  test('placeholdersearch with query', async () => {
+    const customClient = instantMeiliSearch(
+      'http://localhost:7700',
+      'masterKey',
+      {
+        placeholderSearch: false,
+      }
+    )
+
+    const response = await customClient.search<Movies>([
+      {
+        indexName: 'movies',
+        params: {
+          query: 'a',
+        },
+      },
+    ])
+
+    const hits = response.results[0].hits
+    expect(hits.length).toBeGreaterThan(0)
+  })
+
+  test('placeholdersearch set to false with filter', async () => {
+    const customClient = instantMeiliSearch(
+      'http://localhost:7700',
+      'masterKey',
+      {
+        placeholderSearch: false,
+      }
+    )
+
+    const response = await customClient.search<Movies>([
+      {
+        indexName: 'movies',
+        params: {
+          facetFilters: ['genres:Action'],
+        },
+      },
+    ])
+
+    const hits = response.results[0].hits
+    expect(hits.length).toBeGreaterThan(0)
+  })
 })