feat(explore): Don't discard controls with custom sql when changing datasource #20934
Conversation
| if ( | ||
| typeof value === 'object' && | ||
| isDefined(value) && | ||
| 'datasourceWarning' in value |
There was a problem hiding this comment.
Minor. Do you think we can have Typescript helping here or maybe setting it in a constant? I am not very fond of using a plain string for datasourceWarning
There was a problem hiding this comment.
Unfortunately the typing of that area of code is very poor and value's type is basically any object, so addind datasourceWarning to typing here would require a lot of restructuring which is not a focus of this PR.
However, I think 'fieldName' in objectName is a rather common pattern, we use it in many places in the code, including typescript files
There was a problem hiding this comment.
To be precise, there are 21 occurences across 9 typescript files of this pattern 🙂
kgabryje
force-pushed
the
feat/dont-discard-adhoc
branch
from
bf02406 to
1cb86c9
Compare
|
@jinghua-qa May I ask for help with testing? |
|
/testenv up |
Codecov Report
@@ Coverage Diff @@
## master #20934 +/- ##
==========================================
- Coverage 66.89% 66.87% -0.03%
==========================================
Files 1805 1805
Lines 69071 69056 -15
Branches 7369 7364 -5
==========================================
- Hits 46208 46180 -28
- Misses 20956 20966 +10
- Partials 1907 1910 +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 |
|
@kgabryje Ephemeral environment spinning up at http://34.217.90.254:8080. Credentials are |
|
Ephemeral environment shutdown and build artifacts deleted. |
mistercrunch
added
🏷️ bot
SUMMARY
Currently, when user changes datasource in Explore, we try to keep controls that use columns available in the new datasource, but we always discard adhoc columns/metrics/filters that use custom SQL. This PR changes that behaviour by always keeping the controls with custom sql, but with added warning icon. We can't reliably verify if controls with custom SQL are compatible with the new datasource, so we let the user decide (since it's quicker to remove a control than to create it from scratch). If user runs a successful query, the warning icons disappear.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Screen.Recording.2022-08-01.at.16.07.53.mov
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION