-
Notifications
You must be signed in to change notification settings - Fork 247
chore(data-modeling): avoid storing diagrams for web COMPASS-9316 #6944
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 save fails in storage, we don't want to update the state | ||
| // to avoid showing the item in the list. |
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.
Why? We do this for all the other storages
| optInDataExplorerGenAIFeatures: | ||
| isAtlas && !!optInDataExplorerGenAIFeatures, | ||
| enableDataModeling: false, | ||
| enableDataModeling: 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.
@Anemy asked before to keep it switched off to make sure it's aligned with desktop
| // If delete fails in storage, we don't want to update the state | ||
| // to avoid showing the item in the list. |
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.
Same question, if we are not updating state on saves like that, then we need to consider how to notify user about the failure, otherwise it's confusing. I don't understand why we need this change here though
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.
Like in save, if any storage method fails to change the state (within storage), changing state here means that the two are not in sync and for that reason i am checking if storage changed successfully, then update the state.
And yes, i agree that we should notify the user if storage op fails (maybe a toast?)
|
Closing this PR as we agreed not hide created diagrams from the users. Instead we will use session storage to store them on web so that users have nice and consistent user experience. |
Description
Checklist
Motivation and Context
Open Questions
Dependents
Types of changes