Skip to content

Add typo tolerance settings #371

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

Merged
merged 14 commits into from
May 25, 2022
Merged

Add typo tolerance settings #371

merged 14 commits into from
May 25, 2022

Conversation

alallema
Copy link
Contributor

@alallema alallema commented Apr 21, 2022

⚠️ This PR is a WIP it will be merged after the v0.27.0

This PR introduces the new setting: typoTolerance

new methods:
index.getTypoTolerance()
index.updateTypoTolerance(params)
index.resetTypoTolerance()

@alallema alallema marked this pull request as ready for review May 4, 2022 12:39
@alallema alallema requested a review from brunoocasali May 4, 2022 12:39
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

@alallema I didn't review the tests yet :)

@alallema alallema requested a review from brunoocasali May 5, 2022 09:23
@alallema alallema linked an issue May 5, 2022 that may be closed by this pull request
5 tasks
Copy link
Member

@brunoocasali brunoocasali left a comment

Choose a reason for hiding this comment

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

This time I could review the tests, awesome work @alallema :D

@alallema alallema changed the base branch from bump-meilisearch-v0.27.0 to main May 5, 2022 14:39
@alallema alallema self-assigned this May 5, 2022
@alallema alallema requested a review from brunoocasali May 18, 2022 13:57

@Test
@DisplayName("Test update typo tolerance settings")
public void testUpdateTypoTolerance() throws Exception {
Copy link
Member

Choose a reason for hiding this comment

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

Can you change this test (or add another) with the update of just a couple of fields like only setDisableOnAttributes and minWordSizeTypos? Just to ensure all the other fields remain the same (because the update is partial)

Copy link
Contributor Author

@alallema alallema May 19, 2022

Choose a reason for hiding this comment

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

I have already checked this field. Do you mean to check it before the update?

Copy link
Member

Choose a reason for hiding this comment

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

The assertions phase is great, I mean the exercise part is my point.

Your test is perfect as well, I just would like to have another test that is more "real-life", which means in real life the users may not update all the fields at once like the test is doing, they maybe just change one or two, and my suggestion is to have test with this kind of case.

I don't know if it was clear enough, but let me know!

Copy link
Member

Choose a reason for hiding this comment

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

btw @alallema this is the last comment, after this I'll approve it 👯

@alallema alallema requested a review from brunoocasali May 19, 2022 15:46
Copy link
Contributor

@bidoubiwa bidoubiwa left a comment

Choose a reason for hiding this comment

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

LGTM ! Only issues are not linked to this PR and can be found in the issues section:

#392
#393
#394

@alallema
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented May 25, 2022

Build succeeded:

@bors bors bot merged commit 9073c17 into main May 25, 2022
@bors bors bot deleted the typo-tolerance branch May 25, 2022 10:05
bors bot added a commit that referenced this pull request Nov 14, 2022
488: Update version for the next release (v0.8.0) r=alallema a=alallema

This version includes a major redesign of the SDK. ([#425](#425)) [`@alallema](https://github.com/alallema)`

## ⚠️ Breaking changes

* Redesign of the client (#449) `@alallema:`
    * Use `OkHttp` library by default for the Meilisearch client
    * No interface for the creation of a Client disappearing of the class `GenericServiceTemplate`, `ServiceTemplate`, `AbstractClient`, `ApacheClient`, and `DefaultHttpClient`.
* Rename `getAllIndexes` method in `getIndexes` (#477) `@alallema`
* All parameters of the managers accessible in the client are now private: `Config`, `IndexesHandler`, `InstanceHandler`, ` TasksHandler`, `KeysHandler`, `JsonHandler`.
* Factories for the answer and the response disappeared, those classes were removed: `BasicRequestFactory`, `BasicHttpResponse`, `BasicHttpRequest`, and `MeilisearchHttpRequest`.
* Rewriting of the JsonHandler
    * Offering the possibility to use `Gson`, `Jackson` or create your own handler.([#432]
    * Remove `JsonbJsonHandler`.
* Dump returns a `Task`from v0.28.0 so the `createDump` method has been removed just as the `DumpHandler` class.
* Renaming class `Details` in `TaskDetails`
* All methods return now a `MeilisearchException` instead of a `Exception`.

## 🚀 Enhancements

* Replaced traditional getter setter by ``@Getter`` ``@Setter`` from Lombok library ([#385](#385)) [`@ghousek1](https://github.com/ghousek1)`
* Improve Docker configuration in the package ([#399](#399))
* Add code-coverage tool (jacoco) ([#422](#422)) [`@brunoocasali](https://github.com/brunoocasali)`
* Refactoring:
    * Rewriting of the Error Handler ([#438](#438)) [`@alallema](https://github.com/alallema)`
        * Like the other SDKs, this one now contains `MeilisearchApiError`, `MeiliSearchCommunicationError`, `MeilisearchTimeoutError`, `JsonDecodingException` as well as `JsonEncodingException`.
        * All methods return now a `MeilisearchException` instead of a Exception.
(#432))  [`@alallema](https://github.com/alallema)`
    * Rewrite some missing method (#473) [`@alallema](https://github.com/alallema)`
        * health()
        * isHealthy()
        * getVersion()
        * getStats()
        * index.getStats()
        * updateKey() ([#476](#476)) [`@alallema](https://github.com/alallema)`
    * Add typo tolerance settings ([#371](#371)) [`@alallema](https://github.com/alallema)`
    * Add toString method to SearchRequest Class ([#451](#451)) [`@alallema](https://github.com/alallema)`
    * Add support to PATCH HTTP method ([#472](#472)) [`@alallema](https://github.com/alallema)`


Thanks again to `@alallema,`  `@brunoocasali,`   `@ghousek1,` `@kisaga` ! 🎉


Co-authored-by: alallema <[email protected]>
Co-authored-by: Amélie <[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.

Add support to the typo tolerance customization
3 participants