-
Notifications
You must be signed in to change notification settings - Fork 14.1k
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
fix(dashboard): add toast if JSON metadata is invalid #20823
fix(dashboard): add toast if JSON metadata is invalid #20823
Conversation
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.
LGTM
Codecov Report
@@ Coverage Diff @@
## master #20823 +/- ##
==========================================
+ Coverage 66.30% 66.32% +0.01%
==========================================
Files 1756 1757 +1
Lines 66735 66766 +31
Branches 7049 7059 +10
==========================================
+ Hits 44251 44284 +33
+ Misses 20688 20684 -4
- Partials 1796 1798 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. |
try { | ||
metadata = JSON.parse(currentJsonMetadata); | ||
} catch (error) { | ||
addDangerToast(t('JSON metadata is invalid!')); |
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.
@zhaoyongjie shouldn't this re-throw the error (or at least return from the function) if we don't have valid metadata here so we don't try to save an invalid payload?
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.
@etr2460 good point, let's try to make an invalid JSON to verify.
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.
@zhaoyongjie is there any update regarding a fix for this?
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.
@john-bodley @etr2460 add a new PR at here.
SUMMARY
When the json metadata of the dashboard is not legal, the user clicks the save button without any response and no other prompt, this PR adds toast to inform the user.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
before
2022-07-22.5.42.59.mov
after
2022-07-22.5.41.31.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION