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

[v1.8] New searchCutoffMs index settings #1639

Closed
6 tasks done
curquiza opened this issue Apr 5, 2024 · 5 comments · Fixed by #1643
Closed
6 tasks done

[v1.8] New searchCutoffMs index settings #1639

curquiza opened this issue Apr 5, 2024 · 5 comments · Fixed by #1643
Labels
Meilisearch bump Changes related to the Meilisearch bump version

Comments

@curquiza
Copy link
Member

curquiza commented Apr 5, 2024

Related to meilisearch/integration-guides#299

Explanation of the feature

Introduction of the new index setting named searchCutoffMs. Available via /indexes/:uid:/settings (GET, PATCH, DELETE) and /indexes/:uid:/settings/search-cutoff-ms (GET, PUT, DELETE)
Default value is null. Expects an int as value.

More info about the usage of this new feature: https://meilisearch.notion.site/Search-cutoff-usage-fb5e9a07a7624965a541d0a6f8eb37bc

TODO

  • Ensure the library accepts the new field searchCutoffMs when calling the methods related to the /indexes/:uid:/settings (GET, PATCH, DELETE) which are getSettings(), updateSettings() and resetSettings
  • Add new methods corresponding to the new routes
  • GET /indexes/:uid:/settings/search-cutoff-ms: create getSearchCutoffMs()
  • PUT /indexes/:uid:/settings/search-cutoff-ms: create updateSearchCutoffMs()
  • DELETE /indexes/:uid:/settings/search-cutoff-ms: create resetSearchCutoffMs()
  • Add tests for each of these additions

⚠️ Make PRs pointing to bump-meilisearch-v1.8.0 and NOT main. Please do 1 PR for all of these changes, and not several.

@curquiza curquiza added the Meilisearch bump Changes related to the Meilisearch bump version label Apr 5, 2024
@curquiza curquiza linked a pull request Apr 17, 2024 that will close this issue
3 tasks
@curquiza curquiza reopened this Apr 18, 2024
@mdubus
Copy link
Member

mdubus commented Apr 18, 2024

TODO before closing this issue:

  • Update the tests/settings.test.ts test file to test the new searchCutoffMs field
  • Update failing snapshots (addition of the new searchCutoffMs field)
  • Update the Readme with the new available routes
  • Update the PUT /indexes/:uid:/settings/search-cutoff-ms route to be a PUT instead of a PATCH

@amit-ksh
Copy link
Contributor

Hey @mdubus, I should have tested this PR before creating it, I didn't understand all the requirements and made a bad PR. If possible, can you revert/remove the merged PR, so that I can create a fresh PR or should I create a new PR based on bump-meilisearch-v1.8.0 branch?

Also, the getSearchCutoffMs method should return 1500 or null when the meilisearch engine returns null value?

@mdubus
Copy link
Member

mdubus commented Apr 18, 2024

Hey @amit-ksh 👋

No worries, your PR was great, and we're really thankful for the work you've done here!
There are just a few things missing here and there, but you've already done most of the work so thank you again for that 🙏

If you want to continue the work here, I suggest you create a new PR based on the bump-meilisearch-v1.8.0 branch, that would be perfect 👌

As for your last question, the getSearchCutoffMs should return the exact same value as the engine. So if the engine returns null, so should meilisearch-js :)

I hope it helps, let me know if you have any questions, and I'll do my best to help you 🌻

@amit-ksh
Copy link
Contributor

@mdubus for the answering.
Ok I'll create the new PR based of bump-meilisearch-1.8.0 address all the issues including updating the documentation.

@curquiza
Copy link
Member Author

curquiza commented Apr 18, 2024

@amit-ksh your PR was definitely not bad! Thank you for opening it. And thank you for being that involved!!

We just listed the missing things in the issue. It's completely ok to make part of the job and merge even if the full issue is not done 😉

This was referenced Apr 19, 2024
@mdubus mdubus closed this as completed Apr 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meilisearch bump Changes related to the Meilisearch bump version
Projects
None yet
3 participants