-
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: make database connection modal ace fields uncontrolled #22350
Conversation
b6d56d3
to
a97943a
Compare
/testenv up |
@lyndsiWilliams Container image not yet published for this PR. Please try again when build is complete. |
@lyndsiWilliams Ephemeral environment creation failed. Please check the Actions logs for details. |
a97943a
to
5b7c5fc
Compare
Codecov Report
@@ Coverage Diff @@
## master #22350 +/- ##
==========================================
+ Coverage 66.86% 66.88% +0.01%
==========================================
Files 1847 1847
Lines 70574 70580 +6
Branches 7748 7749 +1
==========================================
+ Hits 47190 47204 +14
+ Misses 21383 21360 -23
- Partials 2001 2016 +15
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 |
/testenv up |
@lyndsiWilliams Ephemeral environment spinning up at http://34.221.105.24:8080. Credentials are |
/testenv up |
@eschutho Ephemeral environment spinning up at http://35.89.15.116:8080. Credentials are |
5b7c5fc
to
fa47339
Compare
1d9e1c9
to
64120a0
Compare
superset-frontend/src/views/CRUD/data/database/DatabaseModal/index.test.tsx
Outdated
Show resolved
Hide resolved
6b108c1
to
4b5fd1c
Compare
4b5fd1c
to
f3cb414
Compare
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.
Tested locally with a gsheet, every seems to be working properly! Nice fix!
Ephemeral environment shutdown and build artifacts deleted. |
SUMMARY
I recently refactored the extraJson field #21523 so that we didn't have to do any data munging on database connection save, but rather can use the state that is in the local component and just push it to the api. The tricky situation here is that ace expects a string value, but we are pasting stringified objects into the field, so when we have to parse it later to add to the extra field, it also parses the ace contents. This pr makes the ace editor and input fields in the extra options component uncontrolled so that we don't have to parse the state back into a string again.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
You should be able to edit the engine parameters field in the "Other" section of the database connection form on creation and edit for all databases. There should be no extra characters added to the editor as you type.
ADDITIONAL INFORMATION