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

Add multi index and disjunctive facet search #888

Conversation

bidoubiwa
Copy link
Contributor

@bidoubiwa bidoubiwa commented Nov 23, 2022

Using the Index widget, it is now possible to do mult-index search, see example bellow.

By introducing the multi index search. we automatically introduce disjunctive facet search.

Related to: #774 #789

Multi index search

    <InstantSearch indexName="movies" searchClient={searchClient}>
      <SearchBox />
      <h2>Movies</h2>
      <Index indexName="movies">
        <Hits hitComponent={Hit} />
      </Index>
      <h2>Games</h2>
      <Index indexName="games">
        <Hits hitComponent={Hit} />
      </Index>
    </InstantSearch>
multi-index-1.mp4

Disjunctive facet search

disjunctive.mp4

Multi index + disjunctive facet search

multi-disjunctive.mp4

TODO

  • Should work on simple search
  • Should be compatible with following widgets:
    • Pagination
    • RefinementList
  • Should be compatible with all Meilisearch parameters
  • check state of cache

src/client/instant-meilisearch-client.ts Outdated Show resolved Hide resolved
tsconfig.test.json Show resolved Hide resolved
@bidoubiwa bidoubiwa marked this pull request as ready for review November 24, 2022 15:45
mdubus
mdubus previously approved these changes Nov 24, 2022
Copy link
Member

@mdubus mdubus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✨ 🦕

@bidoubiwa bidoubiwa marked this pull request as draft November 24, 2022 15:50
Base automatically changed from bump-meilisearch-v0.30.0 to main November 28, 2022 15:25
@bidoubiwa bidoubiwa added the enhancement New feature or request label Nov 29, 2022
@bidoubiwa bidoubiwa changed the title Add multi index search Add multi index and disjunctive facet search Nov 29, 2022
@bidoubiwa bidoubiwa marked this pull request as ready for review November 29, 2022 15:17
@bidoubiwa bidoubiwa requested a review from mdubus November 29, 2022 18:47
mdubus
mdubus previously approved these changes Nov 30, 2022
Copy link
Member

@mdubus mdubus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ✨🦕

expect(emptyMovies.hits.length).toBe(0)
expect(emptyGames.hits.length).toBe(0)

const response = await customClient.search<Movies>([
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
const response = await customClient.search<Movies>([
const response = await customClient.search<Movies>([

I want to understand this signature better since now you'll return two different types. What do these Movies does in this case? Can we get rid of it?

Copy link
Contributor Author

@bidoubiwa bidoubiwa Dec 7, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instantsearch requires a search method with a generic. Nonetheless, you are right that it makes no sense as there are multiple indexes now.

I looked at the code of algoliasearch and it seems the issue is at their side as well. multipleQueries is the method used for searching in instantsearch, see their searchClient.

brunoocasali
brunoocasali previously approved these changes Dec 7, 2022
cypress/integration/search-ui.spec.js Show resolved Hide resolved
cypress/integration/search-ui.spec.js Outdated Show resolved Hide resolved
@@ -78,6 +81,6 @@ describe(`${playground} playground test`, () => {

it('Paginate Search', () => {
cy.get('.ais-InfiniteHits-loadMore').click()
cy.get(HIT_ITEM_CLASS).should('have.length', 11)
cy.get(HIT_ITEM_CLASS).should('have.length', 12)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this has been changed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

because of the disjunctive facet search accepting more hits in return 😅

await client.deleteIndex('movies')
const task = await client.deleteIndex('games')

await client.waitForTask(task.taskUid)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't you have a method that accepts more than one uid? (or I'm misleading because of other SDKs...)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have one in js :(

@bidoubiwa
Copy link
Contributor Author

bors merge

@bidoubiwa
Copy link
Contributor Author

bors r-

meili-bors bot added a commit that referenced this pull request Dec 8, 2022
888: Add multi index and disjunctive facet search r=bidoubiwa a=bidoubiwa

⚠️ NOT working with `keepzerofacet`


Using the `Index` widget, it is now possible to do mult-index search, see example bellow.

By introducing the multi index search. we automatically introduce disjunctive facet search.

Related to: #774 #789 


# Multi index search

```jsx
    <InstantSearch indexName="movies" searchClient={searchClient}>
      <SearchBox />
      <h2>Movies</h2>
      <Index indexName="movies">
        <Hits hitComponent={Hit} />
      </Index>
      <h2>Games</h2>
      <Index indexName="games">
        <Hits hitComponent={Hit} />
      </Index>
    </InstantSearch>
```
https://user-images.githubusercontent.com/33010418/203822998-8fc99a62-970f-42a9-95bb-ab09d3c67a9d.mp4

## Disjunctive facet search


https://user-images.githubusercontent.com/33010418/204564207-576bca0c-344b-427c-a1a1-e10ddd7a677f.mp4

# Multi index + disjunctive facet search


https://user-images.githubusercontent.com/33010418/204565010-d4564fdb-88a0-46ad-8dc0-bb6761798e49.mp4






## TODO

- [x] Should work on simple search
- [x] Should be compatible with following widgets:
    - [x] Pagination
    - [x] RefinementList
- [x] Should be compatible with all Meilisearch parameters
    - [x] placeholder search
    - [x] finite pagination
    - [ ] keep zero facets ⚠️ Fixed in disjunctive facet search PR
- [x] check state of cache



Co-authored-by: Charlotte Vermandel <[email protected]>
Co-authored-by: cvermand <[email protected]>
@meili-bors
Copy link
Contributor

meili-bors bot commented Dec 8, 2022

Canceled.

@bidoubiwa bidoubiwa changed the base branch from main to beta/multi-index-search December 8, 2022 14:13
@bidoubiwa
Copy link
Contributor Author

bors merge

@meili-bors
Copy link
Contributor

meili-bors bot commented Dec 8, 2022

@meili-bors meili-bors bot merged commit 170fed7 into beta/multi-index-search Dec 8, 2022
@meili-bors meili-bors bot deleted the bump-meilisearch-v0.30.0-add_multi_index_search branch December 8, 2022 14:16
meili-bors bot added a commit that referenced this pull request Dec 15, 2022
918: Fix keepZeroFacets on disjunctive facet search r=bidoubiwa a=bidoubiwa

Since introducing the multi-index search here #888 the setting `keepZeroFacets` was broken.

This PR introduces the fix to this issue


https://user-images.githubusercontent.com/33010418/207385436-e49ded20-e76b-4974-86de-904cfeecfbd7.mp4

What was done in this pr:

- keepZeroFacet management has been moved to the response adapter instead of the request adapter
- Filtering methods and filtering tests were removed as the new facet distribution management has completely changed
- `defaultFacetDistribution` is renamed `initialFacetDistribution` and contains the distribution of an index without any other requests
- Initialization of facet distribution was moved from client to its own file


Playgrounds:
- New SingleMovieIndex playground to create a new environment for testing
- Updated `SingleIndex` with keepZeroFacets
- Updated `MultiIndex` with keepZeroFacets


Co-authored-by: Charlotte Vermandel <[email protected]>
meili-bors bot added a commit that referenced this pull request Feb 14, 2023
915: Beta: Multi index search and disjunctive facet search r=bidoubiwa a=bidoubiwa

see #888 


Body of the release:

## ⚠️ Breaking change

- Facets behavior changed and uses `disjunctive facet search` see  #884 added in #888

## 🚀 Enchancement

- The `Index` widget is now compatible with instant-meilisearch #888

Thanks again to `@bidoubiwa!` 🎉



Co-authored-by: Charlotte Vermandel <[email protected]>
Co-authored-by: cvermand <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants