You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org> on 2023/04/17 20:34:28 UTC
[GitHub] [superset] Antonio-RiveroMartnez opened a new pull request, #23715: fix(filters): Stop breaking translateToSql returns an object
Antonio-RiveroMartnez opened a new pull request, #23715:
URL: https://github.com/apache/superset/pull/23715
### SUMMARY
There might be times where after changing the dataset, the `translateToSql` is not just a string but an object and thus you get an error when trying to perform the substring operation. In that case we need to get the label from the `column_name` if present if not just return an empty string instead.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
### TESTING INSTRUCTIONS
1. Load a BigNumber chart
2. Swap the dataset to be something that has a time filter like Vehicle Sales in the examples DB
3. Change the chart type to be area chart
4. You should not get an error, instead, the filters should be populated using the new dataset info
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] Antonio-RiveroMartnez merged pull request #23715: fix(filters): Stop breaking if translateToSql returns an object
Posted by "Antonio-RiveroMartnez (via GitHub)" <gi...@apache.org>.
Antonio-RiveroMartnez merged PR #23715:
URL: https://github.com/apache/superset/pull/23715
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] kgabryje commented on a diff in pull request #23715: fix(filters): Stop breaking if translateToSql returns an object
Posted by "kgabryje (via GitHub)" <gi...@apache.org>.
kgabryje commented on code in PR #23715:
URL: https://github.com/apache/superset/pull/23715#discussion_r1187687226
##########
superset-frontend/src/explore/components/controls/FilterControl/AdhocFilter/index.js:
##########
@@ -135,6 +135,10 @@ export default class AdhocFilter {
getDefaultLabel() {
const label = this.translateToSql();
+ // If returned value is an object after changing dataset
+ if (typeof label === 'object') {
+ return label.column_name ?? '';
Review Comment:
Do I understand correctly that the bug occurs when `adhocFIlter.subject` is an object and not a string? If so, maybe we could move the logic of extracting column_name to `getSimpleSQLExpression`, line 298 - `let expression = subject ?? '';`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #23715: fix(filters): Stop breaking if translateToSql returns an object
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #23715:
URL: https://github.com/apache/superset/pull/23715#issuecomment-1538969373
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/23715?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#23715](https://app.codecov.io/gh/apache/superset/pull/23715?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (280894c) into [master](https://app.codecov.io/gh/apache/superset/commit/9681d859995af199da3f85112ecbc6fa1b472c3f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9681d85) will **increase** coverage by `0.00%`.
> The diff coverage is `52.00%`.
> :exclamation: Current head 280894c differs from pull request most recent head 41c4a07. Consider uploading reports for the commit 41c4a07 to get more accurate results
```diff
@@ Coverage Diff @@
## master #23715 +/- ##
=======================================
Coverage 68.18% 68.19%
=======================================
Files 1941 1941
Lines 75241 75242 +1
Branches 8158 8157 -1
=======================================
+ Hits 51306 51308 +2
+ Misses 21852 21851 -1
Partials 2083 2083
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `54.49% <52.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://app.codecov.io/gh/apache/superset/pull/23715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...d/src/SqlLab/components/TabbedSqlEditors/index.jsx](https://app.codecov.io/gh/apache/superset/pull/23715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmJlZFNxbEVkaXRvcnMvaW5kZXguanN4) | `50.00% <50.00%> (+0.52%)` | :arrow_up: |
| [...uperset-frontend/src/explore/exploreUtils/index.js](https://app.codecov.io/gh/apache/superset/pull/23715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvZXhwbG9yZVV0aWxzL2luZGV4Lmpz) | `78.76% <100.00%> (ø)` | |
... and [1 file with indirect coverage changes](https://app.codecov.io/gh/apache/superset/pull/23715/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org