-
Notifications
You must be signed in to change notification settings - Fork 47
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
GO-3733 Search: do not inject tag/types results for deleted/archived objects #1378
GO-3733 Search: do not inject tag/types results for deleted/archived objects #1378
Conversation
WalkthroughA check for Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/lib/localstore/objectstore/queries.go (1 hunks)
Additional comments not posted (2)
pkg/lib/localstore/objectstore/queries.go (2)
59-63
: LGTM! Ensure edge cases are covered.The added checks for
isDeleted
andisArchived
flags are appropriate to prevent injecting results for deleted or archived objects.Verify if there are any edge cases where the flags might not be set correctly or additional flags need to be considered.
70-71
: LGTM! Default case in switch statement.The default case in the switch statement is a good safeguard to handle unrecognized object types.
Coverage provided by https://github.com/seriousben/go-patch-cover-action |
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.
tests?
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/lib/localstore/objectstore/queries_test.go (2 hunks)
Additional comments not posted (3)
pkg/lib/localstore/objectstore/queries_test.go (3)
254-261
: Addition ofrelObjDeleted
looks good.The new test object
relObjDeleted
is correctly defined with the appropriate fields set to simulate a deleted relation.
270-270
: Inclusion ofrelObjDeleted
in the test setup.The
relObjDeleted
object is correctly included in theAddObjects
call to ensure it is part of the test data.
553-568
: New test case for full-text search by deleted tag.The test case correctly verifies that a full-text search by the deleted tag returns no results. This ensures that deleted objects are not included in search results.
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.
1
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/lib/localstore/objectstore/queries_test.go (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- pkg/lib/localstore/objectstore/queries_test.go
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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- pkg/lib/localstore/objectstore/queries_test.go (6 hunks)
Files skipped from review as they are similar to previous changes (1)
- pkg/lib/localstore/objectstore/queries_test.go
bundle.RelationKeyId: pbtypes.String("relid3"), | ||
bundle.RelationKeyRelationKey: pbtypes.String("bsonid1"), | ||
bundle.RelationKeyName: pbtypes.String("archived"), | ||
bundle.RelationKeyIsDeleted: pbtypes.Bool(true), |
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.
@requilence this should be bundle.RelationKeyIsArchived
?
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.
nope, it's just a name of the object I use to search :)
No description provided.