-
Notifications
You must be signed in to change notification settings - Fork 229
feat(compass-aggregations): disable search stages for incompatible views COMPASS-9693 #7217
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
base: main
Are you sure you want to change the base?
Conversation
(!pipelineIsSearchQueryable || | ||
versionIncompatibleCompass || | ||
versionIncompatibleDE); | ||
|
||
return ( | ||
<Combobox | ||
value={selectedStage} |
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.
One problem I'm having is that the text cuts off in 0:38 of the video.
It should say 'Atlas only. Only views containing $addFields, $set or $match stages with the $expr operator are compatible with search indexes. Performs a full-text search on the specified field(s) and gets back only the generated search meta data from a query.' but it cuts off at 'gets back'.
I've tried using chipCharacterLimit and chipTruncationLocation as specified : https://www.mongodb.design/component/combobox/code-docs but this doesn't seem to change anything?
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.
This seems to be an option related to multi select combobox, not to the descriptions
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.
Pull Request Overview
This PR adds support for search stages in aggregation views with version and compatibility checks. The changes implement conditional disabling of search stages based on MongoDB version compatibility and view pipeline queryability requirements.
- Adds version compatibility checks for search stages on views (MongoDB 8.0+/8.1+ requirement)
- Implements pipeline queryability validation for search index compatibility
- Updates collection stats to include pipeline information for view compatibility checks
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
packages/compass-aggregations/src/modules/collection-stats.ts | Adds pipeline field to collection stats to support view compatibility checks |
packages/compass-aggregations/src/components/stage-toolbar/stage-operator-select.tsx | Implements search stage validation logic with version checks and dynamic descriptions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Can you explain somewhere why DE and Compass have different minimum server versions? |
Sorry I misunderstood something but yes you're right DE itself isn't related to server version 8.1+. ̶W̶e̶ ̶a̶r̶e̶ ̶n̶o̶t̶ ̶d̶i̶s̶p̶l̶a̶y̶i̶n̶g̶ ̶t̶h̶e̶ ̶s̶e̶a̶r̶c̶h̶ ̶s̶t̶a̶g̶e̶s̶ ̶i̶n̶ ̶D̶a̶t̶a̶ ̶E̶x̶p̶l̶o̶r̶e̶r̶,̶ ̶o̶n̶l̶y̶ ̶C̶o̶m̶p̶a̶s̶s̶. I removed the extra check and made it only check Compass version for compatibility. |
Hey @DarshanaVenkatesh, can you clarify, if we don't need to differenciate by version here, does it also mean we didn't need to do it in this PR? Because Le Roux is basically asking the same thing I asked you before, so just want to make sure we're not introducing unexpected differences in behavior for desktop and web that otherwise in most cases always behave the same |
That pr should differentiate in the banners because we want to direct users to the atlas search indexes page for data explorer. So yes, we want banners with the server version difference because that's mentioning the server version in Atlas vs Compass (not necessarily data explorer). |
// filter out search stages for data explorer | ||
const filteredStages = | ||
isReadonlyView && !enableAtlasSearchIndexes | ||
? stages.filter((stage) => !isSearchStage(stage.name)) | ||
: stages; |
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.
Where is this requirement coming from? This is a weird change in compass behavior only for data explorer. Why are you not allowed to see actually working stages in aggregation builder based on a completely different feature?
Description
aggregation_selector.mov
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes