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/01/20 13:10:05 UTC

[GitHub] [superset] kgabryje opened a new pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

kgabryje opened a new pull request #12619:
URL: https://github.com/apache/superset/pull/12619


   ### SUMMARY
   Saved metrics added to controls did not have a unique id - we distinguished them only by their names. If we added the same saved metric twice and then tried to edit 1, both items were being edited, because their names were equal.
   This PR introduces a `savedMetricId`, which is added to a metric when it's added to controls via the metrics popover.
   The saved metrics that had been added before this PR will not have this param until they are removed and added again, however everything should still work as expected.
   The only case when the bug will still happen is if 2 or more exactly the same saved metrics are already in controls panel before this change is introduced. In such case, removing the metric and adding it again in control panel will fix the issue.
   Fixes https://github.com/apache/superset/issues/12611
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   https://user-images.githubusercontent.com/15073128/105178344-2c995d80-5b28-11eb-88e3-2e55f0f4a63a.mov
   
   ### TEST PLAN
   <!--- What steps should be taken to verify the changes -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: Fixes https://github.com/apache/superset/issues/12611
   - [ ] Changes UI
   - [ ] Requires DB Migration.
   - [ ] Confirm DB Migration upgrade and downgrade tested.
   - [ ] Introduces new feature or API
   - [ ] Removes existing feature or API
   
   CC: @junlincc @rusackas  @john-bodley @adam-stasiak


----------------------------------------------------------------
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.

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 edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764334355


   I'm glad that you guys are all aligned, I originally thought we should not restrict users from adding the same metrics twice especially in table viz. well........ since there's no conflict between adding unique ID and auto removing same metric, should we move forward with this solution for now? @villebro 


----------------------------------------------------------------
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.

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] ktmud edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763944073


   We should probably just remove the added saved metric from the options list instead (which I believe is also the original behavior before the control redesign?).


----------------------------------------------------------------
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.

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 #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764334355






----------------------------------------------------------------
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.

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] ktmud commented on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763944073


   We should probably just remove the added save metric from the options list instead (which I believe is also the original behavior before the control redesign?).


----------------------------------------------------------------
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.

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 edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
junlincc edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764334355


   I'm glad that you guys are all aligned, I originally thought we should not restrict users from adding the same metrics twice especially in table viz. well........ since there's no conflict between adding unique ID and auto removing same metric, should we move forward with this solution for now? @villebro 


----------------------------------------------------------------
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.

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 #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764663504


   > I assumed that we should keep the possibility of adding the same metric twice. If that's not the case, simply removing added metrics from options in popover seems more sensible. I'll refactor it.
   
   That's my assumption too. i'm sure we will hear that business need someday. thanks for the PR though 🙏


----------------------------------------------------------------
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.

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-io edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296






----------------------------------------------------------------
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.

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-io edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=h1) Report
   > Merging [#12619](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=desc) (345dd02) into [master](https://codecov.io/gh/apache/superset/commit/a422c765c7601058e13dcf50d5251a166d542aec?el=desc) (a422c76) will **decrease** coverage by `2.94%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12619/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12619      +/-   ##
   ==========================================
   - Coverage   66.79%   63.85%   -2.95%     
   ==========================================
     Files        1015      486     -529     
     Lines       49676    29984   -19692     
     Branches     4847        0    -4847     
   ==========================================
   - Hits        33183    19147   -14036     
   + Misses      16371    10837    -5534     
   + Partials      122        0     -122     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.85% <ø> (-0.14%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.37% <0.00%> (-8.66%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.58% <0.00%> (-0.28%)` | :arrow_down: |
   | [...uperset-frontend/src/common/components/Popover.tsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL1BvcG92ZXIudHN4) | | |
   | [.../explore/components/controls/DatasourceControl.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9EYXRhc291cmNlQ29udHJvbC5qc3g=) | | |
   | [superset-frontend/src/components/Menu/NewMenu.tsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTWVudS9OZXdNZW51LnRzeA==) | | |
   | [...d/src/dashboard/components/gridComponents/index.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL2luZGV4Lmpz) | | |
   | [...rc/dashboard/util/getLayoutComponentFromChartId.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldExheW91dENvbXBvbmVudEZyb21DaGFydElkLmpz) | | |
   | [...c/visualizations/FilterBox/FilterBoxChartPlugin.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3Zpc3VhbGl6YXRpb25zL0ZpbHRlckJveC9GaWx0ZXJCb3hDaGFydFBsdWdpbi5qcw==) | | |
   | [...end/src/filters/components/Range/transformProps.ts](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9SYW5nZS90cmFuc2Zvcm1Qcm9wcy50cw==) | | |
   | [...frontend/src/dashboard/reducers/getInitialState.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9yZWR1Y2Vycy9nZXRJbml0aWFsU3RhdGUuanM=) | | |
   | ... and [518 more](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=footer). Last update [a422c76...345dd02](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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 pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763961820


   @ktmud I agree, I don't see the value in adding an identical metric twice. In addition to causing unnecessary confusion for users it will cause problems in the backend where column names are assumed to be unique.


----------------------------------------------------------------
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.

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 pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
villebro commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763961820


   @ktmud I agree, I don't see the value in adding an identical metric twice. In addition to causing unnecessary confusion for users it will cause problems in the backend where column names are assumed to be unique.


----------------------------------------------------------------
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.

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 #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764491864


   I assumed that we should keep the possibility of adding the same metric twice. If that's not the case, simply removing added metrics from options in popover seems more sensible. I'll refactor it.


----------------------------------------------------------------
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.

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] ktmud commented on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764409020


   We shall not need current solution in this PR. It adds unnecessary complexity to the code.


----------------------------------------------------------------
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.

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] adam-stasiak commented on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
adam-stasiak commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763959792


   I saw in console warning related to this PR:
   To reproduce:
   Add metric
   Set Custom name for this
   Save chart and revisit page 
   
   ```Warning: Failed prop type: The prop `savedMetric.metric_name` is marked as required in `AdhocMetricOption`, but its value is `undefined`.
       in AdhocMetricOption (at MetricDefinitionValue.jsx:80)
       in MetricDefinitionValue (at MetricsControl.jsx:126)
       in div (created by Context.Consumer)
       in Styled(div) (at MetricsControl.jsx:380)
       in div (at MetricsControl.jsx:363)
       in MetricsControl (created by Context.Consumer)
       in WithTheme(MetricsControl) (created by DragDropContext(WithTheme(MetricsControl)))
       in DragDropContext(WithTheme(MetricsControl)) (at Control.tsx:42)
       in div (at Control.tsx:41)
       in Control (at ControlPanelsContainer.jsx:125)
       in div (at ControlRow.jsx:33)
       in div (at ControlRow.jsx:31)
       in ControlSetRow (at ControlPanelsContainer.jsx:182)
       in div (created by PanelBody)
       in PanelBody (at ControlPanelSection.jsx:109)
       in div (created by PanelCollapse)
       in Transition (created by Collapse)
       in Collapse (created by PanelCollapse)
       in PanelCollapse (at ControlPanelSection.jsx:108)
       in div (created by Panel)
       in Panel (created by Uncontrolled(Panel))
       in Uncontrolled(Panel) (created by ForwardRef)
       in ForwardRef (at ControlPanelSection.jsx:100)
       in ControlPanelSection (at ControlPanelsContainer.jsx:149)
       in div (created by TabPane)
       in TabPane (created by Context.Consumer)
       in Styled(TabPane) (at ControlPanelsContainer.jsx:245)
       in div (created by TabPanelList)
       in div (created by TabPanelList)
       in TabPanelList (created by ForwardRef(Tabs))
       in div (created by ForwardRef(Tabs))
       in ForwardRef(Tabs) (created by Tabs)
       in Tabs (created by Context.Consumer)
       in Styled(Tabs) (at ControlPanelsContainer.jsx:240)
       in div (created by Context.Consumer)
       in Styled(div) (at ControlPanelsContainer.jsx:226)
       in ControlPanelsContainer (created by ConnectFunction)
       in ConnectFunction (at ExploreViewContainer.jsx:475)
       in div (created by Resizable)
       in Resizable (at ExploreViewContainer.jsx:458)
       in div (created by Context.Consumer)
       in Styled(div) (at ExploreViewContainer.jsx:373)
       in ExploreViewContainer (created by ConnectFunction)
       in ConnectFunction (at App.jsx:38)
       in DynamicPluginProvider (at App.jsx:37)
       in ThemeProvider (at App.jsx:36)
       in Provider (at App.jsx:35)
       in App (created by HotExportedApp)
       in AppContainer (created by HotExportedApp)
       in HotExportedApp (at explore/index.jsx:59)```
       
       


----------------------------------------------------------------
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.

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-io edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=h1) Report
   > Merging [#12619](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=desc) (345dd02) into [master](https://codecov.io/gh/apache/superset/commit/a422c765c7601058e13dcf50d5251a166d542aec?el=desc) (a422c76) will **decrease** coverage by `2.94%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12619/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12619      +/-   ##
   ==========================================
   - Coverage   66.79%   63.85%   -2.95%     
   ==========================================
     Files        1015      486     -529     
     Lines       49676    29984   -19692     
     Branches     4847        0    -4847     
   ==========================================
   - Hits        33183    19147   -14036     
   + Misses      16371    10837    -5534     
   + Partials      122        0     -122     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.85% <ø> (-0.14%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.37% <0.00%> (-8.66%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.58% <0.00%> (-0.28%)` | :arrow_down: |
   | [.../explore/components/controls/CollectionControl.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9Db2xsZWN0aW9uQ29udHJvbC5qc3g=) | | |
   | [...end/src/components/Select/SupersetStyledSelect.tsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L1N1cGVyc2V0U3R5bGVkU2VsZWN0LnRzeA==) | | |
   | [...perset-frontend/src/components/AlteredSliceTag.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQWx0ZXJlZFNsaWNlVGFnLmpzeA==) | | |
   | [...rontend/src/dashboard/components/dnd/handleDrop.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVEcm9wLmpz) | | |
   | [superset-frontend/src/modules/AnnotationTypes.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL21vZHVsZXMvQW5ub3RhdGlvblR5cGVzLmpz) | | |
   | [...rc/dashboard/components/gridComponents/Divider.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL0RpdmlkZXIuanN4) | | |
   | [...ntend/src/explore/components/ExploreChartPanel.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9FeHBsb3JlQ2hhcnRQYW5lbC5qc3g=) | | |
   | [...rc/explore/components/controls/ViewportControl.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9WaWV3cG9ydENvbnRyb2wuanN4) | | |
   | ... and [518 more](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=footer). Last update [a422c76...345dd02](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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 change in pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #12619:
URL: https://github.com/apache/superset/pull/12619#discussion_r561709289



##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
##########
@@ -125,7 +125,15 @@ export default class AdhocMetricEditPopover extends React.Component {
       label = metricLabel;
     }
 
-    const metric = savedMetric?.metric_name ? savedMetric : adhocMetric;
+    // if saved metric, generate a unique savedMetricId
+    const metric = savedMetric?.metric_name
+      ? {
+          ...savedMetric,
+          savedMetricId: `${Math.random()

Review comment:
       I did not want to add another dependency to the project and that method of generating ID was already in use in `AdhocMetric.js`




----------------------------------------------------------------
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.

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 change in pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
kgabryje commented on a change in pull request #12619:
URL: https://github.com/apache/superset/pull/12619#discussion_r561709289



##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
##########
@@ -125,7 +125,15 @@ export default class AdhocMetricEditPopover extends React.Component {
       label = metricLabel;
     }
 
-    const metric = savedMetric?.metric_name ? savedMetric : adhocMetric;
+    // if saved metric, generate a unique savedMetricId
+    const metric = savedMetric?.metric_name
+      ? {
+          ...savedMetric,
+          savedMetricId: `${Math.random()

Review comment:
       I did not want to add another dependency to the project and that method of generating ID was already in use in `AdhocMetric.js`




----------------------------------------------------------------
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.

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] ktmud commented on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
ktmud commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763944073






----------------------------------------------------------------
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.

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 #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764623574


   I'm closing this PR in favour of https://github.com/apache/superset/pull/12657, which implements removing added metrics from options in popover.


----------------------------------------------------------------
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.

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] ktmud edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
ktmud edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763944073


   We should probably just remove the added saved metric from the options list instead (which I believe is also the original behavior before the control redesign?).


----------------------------------------------------------------
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.

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-io edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=h1) Report
   > Merging [#12619](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=desc) (8c96537) into [master](https://codecov.io/gh/apache/superset/commit/a422c765c7601058e13dcf50d5251a166d542aec?el=desc) (a422c76) will **decrease** coverage by `3.60%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12619/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12619      +/-   ##
   ==========================================
   - Coverage   66.79%   63.19%   -3.61%     
   ==========================================
     Files        1015      486     -529     
     Lines       49676    29982   -19694     
     Branches     4847        0    -4847     
   ==========================================
   - Hits        33183    18948   -14235     
   + Misses      16371    11034    -5337     
   + Partials      122        0     -122     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.19% <ø> (-0.80%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `54.61% <0.00%> (-29.24%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.31% <0.00%> (-6.28%)` | :arrow_down: |
   | [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
   | [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.59% <0.00%> (-3.27%)` | :arrow_down: |
   | ... and [536 more](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=footer). Last update [a422c76...8c96537](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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-io edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=h1) Report
   > Merging [#12619](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=desc) (8c96537) into [master](https://codecov.io/gh/apache/superset/commit/a422c765c7601058e13dcf50d5251a166d542aec?el=desc) (a422c76) will **decrease** coverage by `3.60%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12619/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12619      +/-   ##
   ==========================================
   - Coverage   66.79%   63.19%   -3.61%     
   ==========================================
     Files        1015      486     -529     
     Lines       49676    29982   -19694     
     Branches     4847        0    -4847     
   ==========================================
   - Hits        33183    18948   -14235     
   + Misses      16371    11034    -5337     
   + Partials      122        0     -122     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.19% <ø> (-0.80%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `54.61% <0.00%> (-29.24%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
   | [superset/databases/commands/create.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL2NyZWF0ZS5weQ==) | `83.67% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/databases/commands/update.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3VwZGF0ZS5weQ==) | `85.71% <0.00%> (-8.17%)` | :arrow_down: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `84.31% <0.00%> (-6.28%)` | :arrow_down: |
   | [superset/databases/commands/test\_connection.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGF0YWJhc2VzL2NvbW1hbmRzL3Rlc3RfY29ubmVjdGlvbi5weQ==) | `84.78% <0.00%> (-4.35%)` | :arrow_down: |
   | [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `85.59% <0.00%> (-3.27%)` | :arrow_down: |
   | ... and [536 more](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=footer). Last update [a422c76...8c96537](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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 closed pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
kgabryje closed pull request #12619:
URL: https://github.com/apache/superset/pull/12619


   


----------------------------------------------------------------
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.

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] adam-stasiak commented on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
adam-stasiak commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763959792


   I saw in console warning related to this PR:
   To reproduce:
   Add metric
   Set Custom name for this
   Save chart and revisit page 
   
   ```Warning: Failed prop type: The prop `savedMetric.metric_name` is marked as required in `AdhocMetricOption`, but its value is `undefined`.
       in AdhocMetricOption (at MetricDefinitionValue.jsx:80)
       in MetricDefinitionValue (at MetricsControl.jsx:126)
       in div (created by Context.Consumer)
       in Styled(div) (at MetricsControl.jsx:380)
       in div (at MetricsControl.jsx:363)
       in MetricsControl (created by Context.Consumer)
       in WithTheme(MetricsControl) (created by DragDropContext(WithTheme(MetricsControl)))
       in DragDropContext(WithTheme(MetricsControl)) (at Control.tsx:42)
       in div (at Control.tsx:41)
       in Control (at ControlPanelsContainer.jsx:125)
       in div (at ControlRow.jsx:33)
       in div (at ControlRow.jsx:31)
       in ControlSetRow (at ControlPanelsContainer.jsx:182)
       in div (created by PanelBody)
       in PanelBody (at ControlPanelSection.jsx:109)
       in div (created by PanelCollapse)
       in Transition (created by Collapse)
       in Collapse (created by PanelCollapse)
       in PanelCollapse (at ControlPanelSection.jsx:108)
       in div (created by Panel)
       in Panel (created by Uncontrolled(Panel))
       in Uncontrolled(Panel) (created by ForwardRef)
       in ForwardRef (at ControlPanelSection.jsx:100)
       in ControlPanelSection (at ControlPanelsContainer.jsx:149)
       in div (created by TabPane)
       in TabPane (created by Context.Consumer)
       in Styled(TabPane) (at ControlPanelsContainer.jsx:245)
       in div (created by TabPanelList)
       in div (created by TabPanelList)
       in TabPanelList (created by ForwardRef(Tabs))
       in div (created by ForwardRef(Tabs))
       in ForwardRef(Tabs) (created by Tabs)
       in Tabs (created by Context.Consumer)
       in Styled(Tabs) (at ControlPanelsContainer.jsx:240)
       in div (created by Context.Consumer)
       in Styled(div) (at ControlPanelsContainer.jsx:226)
       in ControlPanelsContainer (created by ConnectFunction)
       in ConnectFunction (at ExploreViewContainer.jsx:475)
       in div (created by Resizable)
       in Resizable (at ExploreViewContainer.jsx:458)
       in div (created by Context.Consumer)
       in Styled(div) (at ExploreViewContainer.jsx:373)
       in ExploreViewContainer (created by ConnectFunction)
       in ConnectFunction (at App.jsx:38)
       in DynamicPluginProvider (at App.jsx:37)
       in ThemeProvider (at App.jsx:36)
       in Provider (at App.jsx:35)
       in App (created by HotExportedApp)
       in AppContainer (created by HotExportedApp)
       in HotExportedApp (at explore/index.jsx:59)```
       
       


----------------------------------------------------------------
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.

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] john-bodley commented on a change in pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
john-bodley commented on a change in pull request #12619:
URL: https://github.com/apache/superset/pull/12619#discussion_r561145644



##########
File path: superset-frontend/src/explore/components/controls/MetricControl/AdhocMetricEditPopover.jsx
##########
@@ -125,7 +125,15 @@ export default class AdhocMetricEditPopover extends React.Component {
       label = metricLabel;
     }
 
-    const metric = savedMetric?.metric_name ? savedMetric : adhocMetric;
+    // if saved metric, generate a unique savedMetricId
+    const metric = savedMetric?.metric_name
+      ? {
+          ...savedMetric,
+          savedMetricId: `${Math.random()

Review comment:
       This unique ID seems somewhat hard to grok. Why not use a UUID?




----------------------------------------------------------------
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.

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-io edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=h1) Report
   > Merging [#12619](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=desc) (345dd02) into [master](https://codecov.io/gh/apache/superset/commit/a422c765c7601058e13dcf50d5251a166d542aec?el=desc) (a422c76) will **decrease** coverage by `3.35%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12619/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12619      +/-   ##
   ==========================================
   - Coverage   66.79%   63.44%   -3.36%     
   ==========================================
     Files        1015      486     -529     
     Lines       49676    29969   -19707     
     Branches     4847        0    -4847     
   ==========================================
   - Hits        33183    19014   -14169     
   + Misses      16371    10955    -5416     
   + Partials      122        0     -122     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.44% <ø> (-0.55%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `54.61% <0.00%> (-29.24%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.56% <0.00%> (-11.48%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
   | [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
   | [superset/result\_set.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `96.69% <0.00%> (-1.66%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.04% <0.00%> (-0.82%)` | :arrow_down: |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `85.45% <0.00%> (-0.52%)` | :arrow_down: |
   | [superset/reports/notifications/base.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvcmVwb3J0cy9ub3RpZmljYXRpb25zL2Jhc2UucHk=) | `95.00% <0.00%> (-0.46%)` | :arrow_down: |
   | ... and [533 more](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=footer). Last update [a422c76...345dd02](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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 #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
junlincc commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764334355


   I'm glad that you guys are all aligned, I originally thought we should not restrict users from adding the same metrics twice especially in table viz. well........ since there's no conflict between adding unique ID and removing same metric, should we move forward with this solution for now? @villebro 


----------------------------------------------------------------
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.

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-io commented on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=h1) Report
   > Merging [#12619](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=desc) (345dd02) into [master](https://codecov.io/gh/apache/superset/commit/a422c765c7601058e13dcf50d5251a166d542aec?el=desc) (a422c76) will **decrease** coverage by `3.41%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12619/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12619      +/-   ##
   ==========================================
   - Coverage   66.79%   63.38%   -3.42%     
   ==========================================
     Files        1015      486     -529     
     Lines       49676    29969   -19707     
     Branches     4847        0    -4847     
   ==========================================
   - Hits        33183    18997   -14186     
   + Misses      16371    10972    -5399     
   + Partials      122        0     -122     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.38% <ø> (-0.61%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engines/hive.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lcy9oaXZlLnB5) | `0.00% <0.00%> (-85.72%)` | :arrow_down: |
   | [superset/sql\_validators/postgres.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvc3FsX3ZhbGlkYXRvcnMvcG9zdGdyZXMucHk=) | `50.00% <0.00%> (-50.00%)` | :arrow_down: |
   | [superset/db\_engine\_specs/hive.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2hpdmUucHk=) | `54.61% <0.00%> (-29.24%)` | :arrow_down: |
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `70.56% <0.00%> (-11.48%)` | :arrow_down: |
   | [superset/db\_engine\_specs/sqlite.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3NxbGl0ZS5weQ==) | `65.62% <0.00%> (-9.38%)` | :arrow_down: |
   | [superset/utils/celery.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdXRpbHMvY2VsZXJ5LnB5) | `96.42% <0.00%> (-3.58%)` | :arrow_down: |
   | [superset/db\_engine\_specs/postgres.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3Bvc3RncmVzLnB5) | `93.42% <0.00%> (-2.64%)` | :arrow_down: |
   | [superset/views/database/mixins.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvdmlld3MvZGF0YWJhc2UvbWl4aW5zLnB5) | `80.70% <0.00%> (-1.76%)` | :arrow_down: |
   | [superset/result\_set.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvcmVzdWx0X3NldC5weQ==) | `96.69% <0.00%> (-1.66%)` | :arrow_down: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.65% <0.00%> (-0.93%)` | :arrow_down: |
   | ... and [536 more](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=footer). Last update [a422c76...345dd02](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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 closed pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
kgabryje closed pull request #12619:
URL: https://github.com/apache/superset/pull/12619


   


----------------------------------------------------------------
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.

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 #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763627981


   Converted to draft, because I noticed a bug - chart is not rendering when only a single metric is allowed. I'll investigate and reopen when it's ready


----------------------------------------------------------------
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.

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 #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
kgabryje commented on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-764491864






----------------------------------------------------------------
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.

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-io edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=h1) Report
   > Merging [#12619](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=desc) (345dd02) into [master](https://codecov.io/gh/apache/superset/commit/a422c765c7601058e13dcf50d5251a166d542aec?el=desc) (a422c76) will **decrease** coverage by `2.94%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12619/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12619      +/-   ##
   ==========================================
   - Coverage   66.79%   63.85%   -2.95%     
   ==========================================
     Files        1015      486     -529     
     Lines       49676    29984   -19692     
     Branches     4847        0    -4847     
   ==========================================
   - Hits        33183    19147   -14036     
   + Misses      16371    10837    -5534     
   + Partials      122        0     -122     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.85% <ø> (-0.14%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.37% <0.00%> (-8.66%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.58% <0.00%> (-0.28%)` | :arrow_down: |
   | [...src/dashboard/components/DeleteComponentButton.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL0RlbGV0ZUNvbXBvbmVudEJ1dHRvbi5qc3g=) | | |
   | [...frontend/src/dashboard/util/newEntitiesFromDrop.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL25ld0VudGl0aWVzRnJvbURyb3AuanM=) | | |
   | [superset-frontend/src/api/dataset.ts](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2FwaS9kYXRhc2V0LnRz) | | |
   | [superset-frontend/src/filters/components/index.ts](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9pbmRleC50cw==) | | |
   | [...uperset-frontend/src/common/components/Popover.tsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9jb21wb25lbnRzL1BvcG92ZXIudHN4) | | |
   | [...ontend/src/dashboard/util/getChartIdsFromLayout.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL2dldENoYXJ0SWRzRnJvbUxheW91dC5qcw==) | | |
   | [...shboard/components/filterscope/FilterScopeTree.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2ZpbHRlcnNjb3BlL0ZpbHRlclNjb3BlVHJlZS5qc3g=) | | |
   | [...d/src/dashboard/components/gridComponents/Tabs.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2dyaWRDb21wb25lbnRzL1RhYnMuanN4) | | |
   | ... and [518 more](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=footer). Last update [a422c76...345dd02](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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-io edited a comment on pull request #12619: fix: Set a unique savedMetricId when adding a new saved metric to controls

Posted by GitBox <gi...@apache.org>.
codecov-io edited a comment on pull request #12619:
URL: https://github.com/apache/superset/pull/12619#issuecomment-763610296


   # [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=h1) Report
   > Merging [#12619](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=desc) (345dd02) into [master](https://codecov.io/gh/apache/superset/commit/a422c765c7601058e13dcf50d5251a166d542aec?el=desc) (a422c76) will **decrease** coverage by `2.94%`.
   > The diff coverage is `n/a`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/12619/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l)](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree)
   
   ```diff
   @@            Coverage Diff             @@
   ##           master   #12619      +/-   ##
   ==========================================
   - Coverage   66.79%   63.85%   -2.95%     
   ==========================================
     Files        1015      486     -529     
     Lines       49676    29984   -19692     
     Branches     4847        0    -4847     
   ==========================================
   - Hits        33183    19147   -14036     
   + Misses      16371    10837    -5534     
   + Partials      122        0     -122     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | cypress | `?` | |
   | javascript | `?` | |
   | python | `63.85% <ø> (-0.14%)` | :arrow_down: |
   
   Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags#carryforward-flags-in-the-pull-request-comment) to find out more.
   
   | [Impacted Files](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=tree) | Coverage Δ | |
   |---|---|---|
   | [superset/db\_engine\_specs/presto.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3ByZXN0by5weQ==) | `73.37% <0.00%> (-8.66%)` | :arrow_down: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `88.58% <0.00%> (-0.28%)` | :arrow_down: |
   | [...-frontend/src/SqlLab/reducers/localStorageUsage.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9yZWR1Y2Vycy9sb2NhbFN0b3JhZ2VVc2FnZS5qcw==) | | |
   | [...d/src/dashboard/util/updateComponentParentsList.js](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC91dGlsL3VwZGF0ZUNvbXBvbmVudFBhcmVudHNMaXN0Lmpz) | | |
   | [...end/src/dashboard/components/dnd/DragDroppable.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9EcmFnRHJvcHBhYmxlLmpzeA==) | | |
   | [...Control/AdhocFilterEditPopoverSimpleTabContent.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTaW1wbGVUYWJDb250ZW50LmpzeA==) | | |
   | [...frontend/src/dashboard/containers/FiltersBadge.tsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb250YWluZXJzL0ZpbHRlcnNCYWRnZS50c3g=) | | |
   | [...ntend/src/components/dataViewCommon/Pagination.tsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvZGF0YVZpZXdDb21tb24vUGFnaW5hdGlvbi50c3g=) | | |
   | [...rset-frontend/src/setup/setupErrorMessagesExtra.ts](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3NldHVwL3NldHVwRXJyb3JNZXNzYWdlc0V4dHJhLnRz) | | |
   | [superset-frontend/src/components/CachedLabel.jsx](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvQ2FjaGVkTGFiZWwuanN4) | | |
   | ... and [518 more](https://codecov.io/gh/apache/superset/pull/12619/diff?src=pr&el=tree-more) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=continue).
   > **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
   > `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
   > Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=footer). Last update [a422c76...345dd02](https://codecov.io/gh/apache/superset/pull/12619?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
   


----------------------------------------------------------------
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.

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