fix: [sc-54864] Adds safety check to provide near term fix to save query #21034
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
… saving There is an error when casting the columns array to String() for saving queries where the objects in the array are missing the toString method. This is a near term rapid patch to fix workflow in production which will have a follow up to identify root cause.
eschutho
reviewed
Aug 10, 2022
superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts
Outdated
Show resolved
Hide resolved
eschutho
approved these changes
Aug 10, 2022
This fixes issue caught by unit test where the if statement was using a truthy check where it should be explicitly checking for value of undefined
eric-briscoe
changed the title
eschutho
approved these changes
Aug 10, 2022
|
/testenv up |
|
@jinghua-qa Ephemeral environment spinning up at http://54.214.187.229:8080. Credentials are |
Codecov Report
@@ Coverage Diff @@
## master #21034 +/- ##
==========================================
- Coverage 66.34% 66.26% -0.08%
==========================================
Files 1767 1769 +2
Lines 67358 67475 +117
Branches 7147 7171 +24
==========================================
+ Hits 44686 44711 +25
- Misses 20844 20933 +89
- Partials 1828 1831 +3
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 |
|
Ephemeral environment shutdown and build artifacts deleted. |
jinghua-qa
pushed a commit
to preset-io/superset
that referenced
this pull request
Aug 10, 2022
…ery (apache#21034) * [sc-54864] Adds safety check to provide near term fix for Queries not saving There is an error when casting the columns array to String() for saving queries where the objects in the array are missing the toString method. This is a near term rapid patch to fix workflow in production which will have a follow up to identify root cause. * fix typo Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> * Adjusted chekc to be explicit for undefined instead of truthy This fixes issue caught by unit test where the if statement was using a truthy check where it should be explicitly checking for value of undefined * Adds new unit test to get 100% coverage for callApi Co-authored-by: Elizabeth Thompson <eschutho@gmail.com> (cherry picked from commit ab6ec89)
mistercrunch
added
🏷️ bot
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds safety check to provide near term fix to save query
SUMMARY
There is an error when casting the columns array to String() for saving queries where the objects in the array are missing the toString method. This is a near term rapid patch to fix workflow in production which will have a follow up to identify root cause.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
Repro steps:
1, go to sql lab and run query (click "RUN")
2, save query using save split button
3, observe confirmation msg
4, go to saved query list and look for save query
Expect:
user can save query using save split button
Actual:
user saw query can not be saved msg and the query did not show in save query list
ADDITIONAL INFORMATION