feat: add new foreign keys experimental feature - #801
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (19)
📝 WalkthroughWalkthroughThe SDK adds experimental foreign-key models and feature flags, public index APIs for retrieving, updating, and resetting foreign keys, integration tests using related indexes, documentation samples, and regenerated mock contracts. ChangesForeign keys support
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related issues
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
6013873 to
f37d8fa
Compare
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #801 +/- ##
==========================================
- Coverage 89.72% 89.69% -0.04%
==========================================
Files 27 27
Lines 3612 3657 +45
==========================================
+ Hits 3241 3280 +39
- Misses 214 217 +3
- Partials 157 160 +3 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
@ElyarSadig Please improve tests and generate mocks for new APIs. |
|
@ja7ad |
fjbarrett
left a comment
There was a problem hiding this comment.
I reviewed this alongside #803, with OpenAI Codex assistance. The endpoint methods and live-server test flow look consistent with the foreign-keys contract. A few changes would make the public API and diff easier to maintain:
- Please rename
ForeignIndexUidtoForeignIndexUIDwhile this type is still new. The repository consistently uses Go initialisms in exported API fields (IndexUID,TaskUID, andAPIKeyUID); the JSON tag can remainforeignIndexUid. - The mock regeneration changes 22 unrelated mock files and also pulls in unrelated
GetStatssignature andinterface{}toanychurn. Only the mocks embeddingSettingsManagerorSettingsReaderneed foreign-key methods. Reverting the other generated changes will keep this scoped and avoid a conflict with #803 inmeilisearch_service_manager_mock.go. - The non-context methods are covered, but the six new
WithContextpaths are not. Existing settings tests generally exercise context/custom-client variants; adding at least one explicit context-path case for get, update, and reset would cover request construction and cancellation plumbing.
The current integration failures are from the v1.50 Dynamic Search Rules break and should be resolved by #803 rather than this branch.
Pull Request
Related issue
Fixes #793
What does this PR do?
PR checklist
Please check if your PR fulfills the following requirements:
Thank you so much for contributing to Meilisearch!
Summary by CodeRabbit
foreignKeyssetting (with and without context).SetForeignKeys(enable bool)toggle, propagated through feature updates.nilhandling), retrieving, and resetting foreign keys, and updated related settings expectations.