Add disableOnNumbers to typo tolerance settings
#1951
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughA new optional property, Changes
Poem
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR updates the typo tolerance settings to include a new boolean field, disableOnNumbers, allowing the deactivation of typo tolerance on high entropy words. Key changes include:
- Updating tests/typo_tolerance.test.ts to use snapshot testing and validate the new disableOnNumbers field.
- Refreshing related snapshot files (tests/snapshots/typo_tolerance.test.ts.snap and tests/snapshots/settings.test.ts.snap) with the new property.
- Updating the type definition in src/types/types.ts to include disableOnNumbers.
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tests/typo_tolerance.test.ts | Removed defaultTypoTolerance variable and switched to snapshots with the new disableOnNumbers property. |
| tests/snapshots/typo_tolerance.test.ts.snap | Updated snapshots to reflect the new disableOnNumbers field. |
| tests/snapshots/settings.test.ts.snap | Updated snapshots to include disableOnNumbers in settings tests. |
| src/types/types.ts | Added the disableOnNumbers field to the TypoTolerance type. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
tests/typo_tolerance.test.ts (1)
45-45: Proper test coverage for the new property.The addition of
disableOnNumbers: trueto the test object appropriately validates the new functionality. The test correctly verifies that the property can be set and retrieved.Consider adding test cases for the other possible values to enhance coverage:
test(`${permission} key: Update typo tolerance settings`, async () => { const client = await getClient(permission); const newTypoTolerance = { enabled: false, minWordSizeForTypos: { oneTypo: 1, twoTypos: 2, }, disableOnWords: ["title"], disableOnAttributes: ["hello"], disableOnNumbers: true, }; await client .index(index.uid) .updateTypoTolerance(newTypoTolerance) .waitTask(); const response = await client.index(index.uid).getTypoTolerance(); expect(response).toEqual(newTypoTolerance); }); + + test(`${permission} key: Update typo tolerance with disableOnNumbers false`, async () => { + const client = await getClient(permission); + const newTypoTolerance = { + disableOnNumbers: false, + }; + await client + .index(index.uid) + .updateTypoTolerance(newTypoTolerance) + .waitTask(); + + const response = await client.index(index.uid).getTypoTolerance(); + expect(response.disableOnNumbers).toBe(false); + });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/__snapshots__/settings.test.ts.snapis excluded by!**/*.snaptests/__snapshots__/typo_tolerance.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (2)
src/types/types.ts(1 hunks)tests/typo_tolerance.test.ts(4 hunks)
🔇 Additional comments (2)
src/types/types.ts (1)
526-526: LGTM! Consistent implementation with existing patterns.The addition of
disableOnNumbersproperty follows the established pattern for typo tolerance options. The type definition is correct (boolean | null), the naming is consistent with similar properties (disableOnAttributes,disableOnWords), and it's properly marked as optional.tests/typo_tolerance.test.ts (1)
32-32: Good improvement: Switch to snapshot testing for default values.Using
toMatchSnapshot()instead of hardcoded expected values makes the tests more maintainable and flexible as the API evolves. This approach prevents brittle tests that break when new properties are added to the default response.Also applies to: 62-62, 71-71
Strift
changed the title
disableOnNumbersdisableOnNumbers to typo tolerance settings
Pull Request
This PR updates the typo tolerance settings to allow deactivating typo tolerance on high entropy words.
Related engine issue: meilisearch/meilisearch#5344
What does this PR do?
disableOnNumbers: boolfieldtoMatchSnapshot()instead of manually testing against a "default settings" objectPR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
New Features
Tests