-
Notifications
You must be signed in to change notification settings - Fork 245
feat(databases-collections): handle non-existent namespaces COMPASS-5750 #6664
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
| // If the collection is not unprovisioned `is_non_existent` anymore, | ||
| // let's update the parent database model to reflect the change. | ||
| // This happens when a user tries to insert first document into a | ||
| // collection that doesn't exist yet or creates a new collection | ||
| // for an unprovisioned database. | ||
| if (!this.is_non_existent) { | ||
| getParentByType(this, 'Database').set({ | ||
| is_non_existent: false, | ||
| }); | ||
| } |
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.
Not sure if this is the right place for this, but if you feel it does not belong here, let me know
| void dispatch(fetchCollectionInfo(workspaceOptions)); | ||
| } | ||
|
|
||
| if (workspaceOptions.type === 'Collections') { | ||
| // Fetching extra metadata for database should not block tab opening | ||
| void dispatch(fetchDatabaseInfo(workspaceOptions)); |
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.
Created COMPASS-8897 to clean this up.
| isTimeSeries: collection.isTimeSeries, | ||
| isReadonly: collection.readonly ?? collection.isView, | ||
| sourceName: collection.sourceName, |
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.
For case like this I'd probably prefer to either have a dedicated action or subscribe to all fields you're updating, otherwise you're listening here to one thing, but updating others, it's a but confusing to navigate. Taking into account we're going to refactor this, that's fine for now
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.
great point. with refactor in mind, i'll merge this for now!
Light Mode
Dark Mode
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes