-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
chore: should return if get a exception in Dashboard edit modal #21524
Conversation
import { AsyncSelect, Row, Col, AntdForm } from 'src/components'; | ||
import { AntdForm, AsyncSelect, Col, Row } from 'src/components'; | ||
import rison from 'rison'; | ||
import { | ||
styled, | ||
t, | ||
SupersetClient, | ||
getCategoricalSchemeRegistry, | ||
ensureIsArray, | ||
getCategoricalSchemeRegistry, | ||
getSharedLabelColor, | ||
styled, | ||
SupersetClient, | ||
t, |
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.
resort by IDE
let metadata; | ||
try { | ||
if ( | ||
!currentJsonMetadata.startsWith('{') || | ||
!currentJsonMetadata.endsWith('}') | ||
) { | ||
throw new Error(); | ||
} | ||
metadata = JSON.parse(currentJsonMetadata); | ||
} catch (error) { | ||
addDangerToast(t('JSON metadata is invalid!')); | ||
return; | ||
} |
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.
Move the validated logic out of the if clause
. Add return
statement in catch case,
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 #21524 +/- ##
==========================================
- Coverage 66.67% 66.66% -0.01%
==========================================
Files 1793 1793
Lines 68493 68500 +7
Branches 7275 7279 +4
==========================================
Hits 45665 45665
- Misses 20966 20971 +5
- Partials 1862 1864 +2
Flags with carried forward coverage won't be shown. Click here to find out more.
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
SUMMARY
According to the discussion here, make a minor refactor to validate the JSON string to make sure the
currentJsonMetadata
must be a validated JSON string and it should be in '{}'BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
After
validate.JSON.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION