-
Notifications
You must be signed in to change notification settings - Fork 106
feat: add skipCreation parameter for adding/replacing #739
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
Conversation
📝 WalkthroughWalkthroughThis PR adds support for the Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
.code-samples.meilisearch.yaml (1)
70-71: Consider omitting the explicit false value for SkipCreation.Setting
SkipCreation: falseis redundant sincefalseis the zero value for booleans in Go. For code samples demonstrating best practices, consider either:
- Passing
nilwhen using default behavior, or- Omitting the
SkipCreationfield entirely from the struct📝 Suggested alternatives
Option 1: Pass nil for default behavior
- options := &meilisearch.DocumentOptions{SkipCreation: false} - client.Index("movies").AddDocuments(documents, options) + client.Index("movies").AddDocuments(documents, nil)Option 2: Omit the field
- options := &meilisearch.DocumentOptions{SkipCreation: false} + options := &meilisearch.DocumentOptions{} client.Index("movies").AddDocuments(documents, options)types.go (2)
680-681: Add documentation for the SkipCreation field.The
SkipCreationfield lacks a comment explaining its purpose. Adding documentation would help users understand when and why to use this option.📝 Suggested documentation
type DocumentOptions struct { PrimaryKey *string `json:"primaryKey,omitempty"` + // SkipCreation when set to true, prevents creating the index if it doesn't exist. + // Documents will only be added/updated if the index already exists. SkipCreation bool `json:"skipCreation,omitempty"` // TaskCustomMetadata is the custom metadata to add to the task.
691-691: Add documentation for the SkipCreation field.The
SkipCreationfield lacks a comment explaining its purpose. Adding documentation would help users understand when and why to use this option.📝 Suggested documentation
type CsvDocumentsQuery struct { PrimaryKey string `json:"primaryKey,omitempty"` CsvDelimiter string `json:"csvDelimiter,omitempty"` + // SkipCreation when set to true, prevents creating the index if it doesn't exist. + // Documents will only be added/updated if the index already exists. SkipCreation bool `json:"skipCreation,omitempty"` // TaskCustomMetadata is the custom metadata to add to the task.integration/index_document_test.go (1)
1114-1117: Consider adding a nil check for defensive coding.While currently safe (all test cases provide non-nil
options), accessingtt.args.options.SkipCreationdirectly could cause a nil pointer dereference if a future test case passesniloptions.📝 Suggested defensive check
- var wantDocs []map[string]interface{} - if !tt.args.options.SkipCreation { - wantDocs = testParseCsvDocuments(t, bytes.NewReader(tt.args.documents)) - } + var wantDocs []map[string]interface{} + if tt.args.options == nil || !tt.args.options.SkipCreation { + wantDocs = testParseCsvDocuments(t, bytes.NewReader(tt.args.documents)) + }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.code-samples.meilisearch.yamlhelper.gohelper_test.gointegration/index_document_test.gotypes.go
🧰 Additional context used
🧬 Code graph analysis (1)
helper_test.go (1)
types.go (2)
DocumentOptions(679-686)CsvDocumentsQuery(688-696)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: integration-tests (go current version)
- GitHub Check: integration-tests (go latest version)
🔇 Additional comments (7)
helper.go (2)
116-119: LGTM! Correct handling of SkipCreation for CSV queries.The implementation correctly serializes the
SkipCreationfield only whentrue, avoiding unnecessaryskipCreation=falsein query parameters. This follows the established pattern for optional boolean flags.
186-189: LGTM! Correct handling of SkipCreation for document options.The implementation correctly serializes the
SkipCreationfield only whentrue, which is the appropriate behavior for query parameters..code-samples.meilisearch.yaml (1)
80-81: LGTM! Good demonstration of SkipCreation usage.The code sample clearly demonstrates how to use
SkipCreation: truewith document updates.helper_test.go (2)
194-221: LGTM! Comprehensive test coverage for DocumentOptions.SkipCreation.The test cases properly cover:
SkipCreation: trueadds the field to the output mapSkipCreation: falseresults in an empty map (field omitted)- Integration with
PrimaryKeyandTaskCustomMetadatafields
275-305: LGTM! Comprehensive test coverage for CsvDocumentsQuery.SkipCreation.The test cases properly cover:
SkipCreation: trueadds the field to the output mapSkipCreation: falseresults in an empty map (field omitted)- Integration with other CSV-specific fields
integration/index_document_test.go (2)
408-434: LGTM! Good test coverage for SkipCreation functionality.The test properly verifies that when
SkipCreation: trueis set, no documents are returned (empty results), which aligns with the expected behavior when the index doesn't exist or documents should not be created.
1078-1093: LGTM! Good test coverage for CSV with SkipCreation.The test case appropriately covers the
SkipCreationflag for CSV document operations.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #739 +/- ##
==========================================
+ Coverage 88.52% 88.53% +0.01%
==========================================
Files 22 22
Lines 3258 3262 +4
==========================================
+ Hits 2884 2888 +4
Misses 216 216
Partials 158 158 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Pull Request
Related issue
Fixes #738
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
SkipCreationoption to document operations, enabling users to enqueue document additions and updates without immediately creating documents in the index.✏️ Tip: You can customize this high-level summary in your review settings.