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 2021/10/01 12:05:12 UTC
[GitHub] [superset] villebro opened a new pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
villebro opened a new pull request #16933:
URL: https://github.com/apache/superset/pull/16933
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
When dropping a wrapped component (in this case a markdown component, but could be any other component, like a chart), the component is wrapped in a row component. However, the wrapper adds itself as it's own parent:
Sometimes the wrapped component adds the wrapper component as its parent, in which case the cross filter indicator goes into an infinite loop (the cross filter FF needs to be enabled - if it's not enabled, the bug isn't triggered). However, sometimes the wrapped component doesn't add the wrapper as its parent, in which case the dashboard doesn't crash.
Interestingly enough, after saving the dashboard, the layout metadata is fixed:
- the wrapped component correctly references the wrapper as its closest parent
- the wrapper no longer references itself
See the screenshot below for the post-save state:
This bug exposed several problems in the associated code, causing unnecessary performance problems. However, these should be fixed in follow-up PRs.
### TESTING INSTRUCTIONS
Local testing
### 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] kgabryje commented on pull request #16933: fix(dashboard): recursive parent on dashboard components
Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-933289525
Thanks for a very detailed description of the issue!
--
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] geido commented on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
geido commented on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932249984
/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS_SET=true FEATURE_DASHBOARD_FILTERS_EXPERIMENTAL=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 #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932298355
@geido Ephemeral environment spinning up at http://34.220.230.231: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] geido commented on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
geido commented on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932294737
/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS_SET=true FEATURE_DASHBOARD_FILTERS_EXPERIMENTAL=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] junlincc commented on pull request #16933: fix(dashboard): recursive parent on dashboard components
Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932314988
Tested by adding charts to different level of tabs in the sales dashboard, with charts, components being added and removed a few times, not seeing errors. Thank you for the fix! @villebro
Curious to know why this issue is not reproducible consistently?
--
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 #16933: fix(dashboard): recursive parent on dashboard components
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932318368
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] codecov[bot] commented on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932187559
# [Codecov](https://codecov.io/gh/apache/superset/pull/16933?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 [#16933](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (94e2953) into [master](https://codecov.io/gh/apache/superset/commit/87baac7650bdb6a8f8f82cf4992567a2c5c73cba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (87baac7) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
[](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16933 +/- ##
=======================================
Coverage 76.97% 76.97%
=======================================
Files 1022 1022
Lines 54903 54902 -1
Branches 7485 7485
=======================================
Hits 42262 42262
+ Misses 12393 12392 -1
Partials 248 248
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.03% <ø> (+<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://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `100.00% <ø> (ø)` | |
| [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `89.42% <0.00%> (+0.96%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [87baac7...94e2953](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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 #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932251676
@geido Ephemeral environment spinning up at http://54.213.253.119: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] edited a comment on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932187559
# [Codecov](https://codecov.io/gh/apache/superset/pull/16933?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 [#16933](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6beb9b5) into [master](https://codecov.io/gh/apache/superset/commit/87baac7650bdb6a8f8f82cf4992567a2c5c73cba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (87baac7) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
> :exclamation: Current head 6beb9b5 differs from pull request most recent head 8fe7d81. Consider uploading reports for the commit 8fe7d81 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16933 +/- ##
=======================================
Coverage 76.97% 76.97%
=======================================
Files 1022 1022
Lines 54903 54902 -1
Branches 7485 7485
=======================================
Hits 42262 42262
+ Misses 12393 12392 -1
Partials 248 248
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.03% <ø> (+<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://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `100.00% <ø> (ø)` | |
| [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `89.42% <0.00%> (+0.96%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [87baac7...8fe7d81](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] codecov[bot] edited a comment on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932187559
# [Codecov](https://codecov.io/gh/apache/superset/pull/16933?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 [#16933](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (8fe7d81) into [master](https://codecov.io/gh/apache/superset/commit/87baac7650bdb6a8f8f82cf4992567a2c5c73cba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (87baac7) will **not change** coverage.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16933 +/- ##
=======================================
Coverage 76.97% 76.97%
=======================================
Files 1022 1022
Lines 54903 54903
Branches 7485 7485
=======================================
Hits 42262 42262
Misses 12393 12393
Partials 248 248
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.03% <100.00%> (ø)` | |
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/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `100.00% <100.00%> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [87baac7...8fe7d81](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] villebro commented on a change in pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #16933:
URL: https://github.com/apache/superset/pull/16933#discussion_r720274222
##########
File path: superset-frontend/src/dashboard/util/newEntitiesFromDrop.js
##########
@@ -51,8 +51,8 @@ export default function newEntitiesFromDrop({ dropResult, layout }) {
rowWrapper.children = [newDropChild.id];
rowWrapper.parents = (dropEntity.parents || []).concat(dropEntity.id);
newEntities[rowWrapper.id] = rowWrapper;
- newDropChild = rowWrapper;
newDropChild.parents = rowWrapper.parents.concat(rowWrapper.id);
+ newDropChild = rowWrapper;
Review comment:
The intent here was clearly to add the wrapper as the parent to the wrapped component - due to the incorrect order here the wrapper's id was incorrectly added to its own parents, not the wrapped component's ids.
--
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] removed a comment on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
github-actions[bot] removed a comment on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932251676
@geido Ephemeral environment spinning up at http://54.213.253.119: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] edited a comment on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932187559
# [Codecov](https://codecov.io/gh/apache/superset/pull/16933?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 [#16933](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6beb9b5) into [master](https://codecov.io/gh/apache/superset/commit/87baac7650bdb6a8f8f82cf4992567a2c5c73cba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (87baac7) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
> :exclamation: Current head 6beb9b5 differs from pull request most recent head ca98bba. Consider uploading reports for the commit ca98bba to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16933 +/- ##
=======================================
Coverage 76.97% 76.97%
=======================================
Files 1022 1022
Lines 54903 54902 -1
Branches 7485 7485
=======================================
Hits 42262 42262
+ Misses 12393 12392 -1
Partials 248 248
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.03% <ø> (+<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://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `100.00% <ø> (ø)` | |
| [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `89.42% <0.00%> (+0.96%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [87baac7...ca98bba](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] villebro commented on a change in pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
villebro commented on a change in pull request #16933:
URL: https://github.com/apache/superset/pull/16933#discussion_r720274222
##########
File path: superset-frontend/src/dashboard/util/newEntitiesFromDrop.js
##########
@@ -51,8 +51,8 @@ export default function newEntitiesFromDrop({ dropResult, layout }) {
rowWrapper.children = [newDropChild.id];
rowWrapper.parents = (dropEntity.parents || []).concat(dropEntity.id);
newEntities[rowWrapper.id] = rowWrapper;
- newDropChild = rowWrapper;
newDropChild.parents = rowWrapper.parents.concat(rowWrapper.id);
+ newDropChild = rowWrapper;
Review comment:
The intent here was clearly to add the wrapper as the parent to the wrapped component - due to the incorrect order here the wrapper's id was incorrectly added to the wrapper's parents, not the wrapped component's ids.
--
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] edited a comment on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932187559
# [Codecov](https://codecov.io/gh/apache/superset/pull/16933?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 [#16933](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (94e2953) into [master](https://codecov.io/gh/apache/superset/commit/87baac7650bdb6a8f8f82cf4992567a2c5c73cba?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (87baac7) will **increase** coverage by `0.00%`.
> The diff coverage is `n/a`.
> :exclamation: Current head 94e2953 differs from pull request most recent head ca98bba. Consider uploading reports for the commit ca98bba to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #16933 +/- ##
=======================================
Coverage 76.97% 76.97%
=======================================
Files 1022 1022
Lines 54903 54902 -1
Branches 7485 7485
=======================================
Hits 42262 42262
+ Misses 12393 12392 -1
Partials 248 248
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `71.03% <ø> (+<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://codecov.io/gh/apache/superset/pull/16933?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | `100.00% <ø> (ø)` | |
| [...eFilters/FiltersConfigModal/FiltersConfigModal.tsx](https://codecov.io/gh/apache/superset/pull/16933/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdNb2RhbC50c3g=) | `89.42% <0.00%> (+0.96%)` | :arrow_up: |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [87baac7...ca98bba](https://codecov.io/gh/apache/superset/pull/16933?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?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] geido removed a comment on pull request #16933: fix(dashboard): don't add recursive parents on wrapper component
Posted by GitBox <gi...@apache.org>.
geido removed a comment on pull request #16933:
URL: https://github.com/apache/superset/pull/16933#issuecomment-932249984
/testenv up FEATURE_DASHBOARD_NATIVE_FILTERS=true FEATURE_DASHBOARD_CROSS_FILTERS=true FEATURE_DASHBOARD_NATIVE_FILTERS_SET=true FEATURE_DASHBOARD_FILTERS_EXPERIMENTAL=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] junlincc merged pull request #16933: fix(dashboard): recursive parent on dashboard components
Posted by GitBox <gi...@apache.org>.
junlincc merged pull request #16933:
URL: https://github.com/apache/superset/pull/16933
--
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