You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2023/01/02 08:24:29 UTC
[GitHub] [superset] villebro opened a new pull request, #22565: fix(explore): support saving undefined time grain
villebro opened a new pull request, #22565:
URL: https://github.com/apache/superset/pull/22565
### SUMMARY
When saving a chart without a time grain, the chart reverts to the default time grain after the chart reloads in Explore view. To get around this, the user must choose the "Original value" option from the time grain dropdown. While this works, there should only be one way of clearing the time grain. A summary of changes introduced by this PR:
- When saving the chart with the time grain control in cleared state, the `time_grain_sqla` entry is removed from the form data. To get around this, we check if the chart has been saved by looking at the `metadata` property of the chart state - if it's defined, we know the chart has been saved, in which case we know that the time grain has been explicitly set to `undefined`.
- Remove the "Original value" option from the time grain dropdown. After this the time grain should be cleared to be removed.
- Add a "None" placeholder when the time grain is cleared. Use the same placeholder for "Series limit", as that seems more appropriate for that one, too (currently it shows ")
The change has been tested to work in the following scenarios:
- Chart saved with "Original value" prior to the change - it renders the chart without the time grain as expected, but with the time grain control in cleared state (as opposed to with "Original value")
- Chart saved with time grain
- Chart saved without time grain
- All of the above on a dashboard (verified by looking at the rendered query to confirm that the time grain was applied correctly)
### AFTER
With these changes, the time grain remains cleared even after saving:
https://user-images.githubusercontent.com/33317356/210207589-6d6a3fb2-2b3a-4443-aa61-b5e3d99db666.mp4
### BEFORE
Previously, the chart would revert back to the default time grain `P1D` when the chart reloaded in Explore view:
https://user-images.githubusercontent.com/33317356/210207787-51cd2089-213d-4762-baf8-213a3ec425bc.mp4
### TESTING INSTRUCTIONS
1. Create a line chart and remove the time grain
2. Save the chart and notice that the default time grain is reapplied.
### 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] github-actions[bot] commented on pull request #22565: fix(explore): support saving undefined time grain
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22565:
URL: https://github.com/apache/superset/pull/22565#issuecomment-1369038956
@zhaoyongjie Ephemeral environment spinning up at http://54.213.15.196:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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] zhaoyongjie commented on pull request #22565: fix(explore): support saving undefined time grain
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on PR #22565:
URL: https://github.com/apache/superset/pull/22565#issuecomment-1369048101
/testenv up FEATURE_GENERIC_CHART_AXES=true
--
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] github-actions[bot] commented on pull request #22565: fix(explore): support saving undefined time grain
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22565:
URL: https://github.com/apache/superset/pull/22565#issuecomment-1369050672
@zhaoyongjie Ephemeral environment spinning up at http://35.91.67.201:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup.
--
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 #22565: fix(explore): support saving undefined time grain
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on PR #22565:
URL: https://github.com/apache/superset/pull/22565#issuecomment-1368738880
# [Codecov](https://codecov.io/gh/apache/superset/pull/22565?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 [#22565](https://codecov.io/gh/apache/superset/pull/22565?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f97dcce) into [master](https://codecov.io/gh/apache/superset/commit/734d423ca094746ee1c2f2c1e72138ed9e69cdee?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (734d423) will **decrease** coverage by `11.10%`.
> The diff coverage is `0.00%`.
```diff
@@ Coverage Diff @@
## master #22565 +/- ##
===========================================
- Coverage 66.91% 55.81% -11.11%
===========================================
Files 1851 1851
Lines 70709 70709
Branches 7766 7766
===========================================
- Hits 47316 39466 -7850
- Misses 21371 29221 +7850
Partials 2022 2022
```
| Flag | Coverage Δ | |
|---|---|---|
| hive | `52.46% <ø> (ø)` | |
| mysql | `?` | |
| postgres | `?` | |
| presto | `52.36% <ø> (ø)` | |
| python | `57.96% <ø> (-23.31%)` | :arrow_down: |
| sqlite | `?` | |
| unit | `51.19% <ø> (ø)` | |
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://codecov.io/gh/apache/superset/pull/22565?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...rt-controls/src/shared-controls/sharedControls.tsx](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3NoYXJlZC1jb250cm9scy9zaGFyZWRDb250cm9scy50c3g=) | `53.44% <0.00%> (ø)` | |
| [...d/packages/superset-ui-chart-controls/src/types.ts](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvcGFja2FnZXMvc3VwZXJzZXQtdWktY2hhcnQtY29udHJvbHMvc3JjL3R5cGVzLnRz) | `100.00% <ø> (ø)` | |
| [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `75.85% <ø> (-13.40%)` | :arrow_down: |
| [superset/utils/dashboard\_import\_export.py](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdXRpbHMvZGFzaGJvYXJkX2ltcG9ydF9leHBvcnQucHk=) | `0.00% <0.00%> (-100.00%)` | :arrow_down: |
| [superset/tags/core.py](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvdGFncy9jb3JlLnB5) | `4.54% <0.00%> (-95.46%)` | :arrow_down: |
| [superset/key\_value/commands/update.py](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `0.00% <0.00%> (-90.91%)` | :arrow_down: |
| [superset/key\_value/commands/delete.py](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZS5weQ==) | `0.00% <0.00%> (-87.88%)` | :arrow_down: |
| [superset/key\_value/commands/delete\_expired.py](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQva2V5X3ZhbHVlL2NvbW1hbmRzL2RlbGV0ZV9leHBpcmVkLnB5) | `0.00% <0.00%> (-84.00%)` | :arrow_down: |
| [superset/dashboards/commands/importers/v0.py](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGFzaGJvYXJkcy9jb21tYW5kcy9pbXBvcnRlcnMvdjAucHk=) | `15.62% <0.00%> (-76.25%)` | :arrow_down: |
| [superset/datasets/commands/create.py](https://codecov.io/gh/apache/superset/pull/22565/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGF0YXNldHMvY29tbWFuZHMvY3JlYXRlLnB5) | `30.61% <0.00%> (-69.39%)` | :arrow_down: |
| ... and [286 more](https://codecov.io/gh/apache/superset/pull/22565/diff?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
[GitHub] [superset] github-actions[bot] commented on pull request #22565: fix(explore): support saving undefined time grain
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #22565:
URL: https://github.com/apache/superset/pull/22565#issuecomment-1369515598
Ephemeral environment shutdown and build artifacts deleted.
--
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] zhaoyongjie commented on a diff in pull request #22565: fix(explore): support saving undefined time grain
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on code in PR #22565:
URL: https://github.com/apache/superset/pull/22565#discussion_r1060256291
##########
superset/db_engine_specs/base.py:
##########
@@ -97,7 +97,6 @@ class TimeGrain(NamedTuple):
builtin_time_grains: Dict[Optional[str], str] = {
- None: __("Original value"),
Review Comment:
Thanks for getting rid of this option.
--
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] zhaoyongjie commented on pull request #22565: fix(explore): support saving undefined time grain
Posted by GitBox <gi...@apache.org>.
zhaoyongjie commented on PR #22565:
URL: https://github.com/apache/superset/pull/22565#issuecomment-1369037207
/testenv up
--
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] villebro merged pull request #22565: fix(explore): support saving undefined time grain
Posted by GitBox <gi...@apache.org>.
villebro merged PR #22565:
URL: https://github.com/apache/superset/pull/22565
--
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