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/05/24 17:44:06 UTC

[GitHub] [superset] villebro opened a new pull request #14788: fix(native-filters): apply null as unset state

villebro opened a new pull request #14788:
URL: https://github.com/apache/superset/pull/14788


   ### SUMMARY
   #14710 caused a regression that sometimes froze the browser if the default value of a native filter vas set to an empty array. In addition, the lodash debounce import is replaced with one that is slightly more lightweight.
   
   ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
   <!--- Skip this if not applicable -->
   
   ### TESTING INSTRUCTIONS
   <!--- Required! What steps can be taken to manually verify the changes? -->
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [ ] Has associated issue:
   - [ ] 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.

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 #14788: fix(native-filters): fix loop caused by external state handler

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14788:
URL: https://github.com/apache/superset/pull/14788#issuecomment-847229865


   # [Codecov](https://codecov.io/gh/apache/superset/pull/14788?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 [#14788](https://codecov.io/gh/apache/superset/pull/14788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ac3b9c) into [master](https://codecov.io/gh/apache/superset/commit/9bf07cc428f0b88958d9a5c013b1f436587da529?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9bf07cc) will **decrease** coverage by `0.00%`.
   > The diff coverage is `59.09%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/14788/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/14788?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   #14788      +/-   ##
   ==========================================
   - Coverage   77.55%   77.54%   -0.01%     
   ==========================================
     Files         962      962              
     Lines       49156    49153       -3     
     Branches     6183     6181       -2     
   ==========================================
   - Hits        38122    38116       -6     
   - Misses      10831    10835       +4     
   + Partials      203      202       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `72.44% <59.09%> (-0.02%)` | :arrow_down: |
   
   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/14788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...tersConfigModal/FiltersConfigForm/DefaultValue.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdGb3JtL0RlZmF1bHRWYWx1ZS50c3g=) | `26.31% <0.00%> (-1.47%)` | :arrow_down: |
   | [...c/filters/components/Select/SelectFilterPlugin.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvU2VsZWN0RmlsdGVyUGx1Z2luLnRzeA==) | `64.86% <61.90%> (-4.37%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14788?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/14788?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 [9bf07cc...4ac3b9c](https://codecov.io/gh/apache/superset/pull/14788?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.

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 #14788: [WIP] fix(native-filters): apply null as unset state

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14788:
URL: https://github.com/apache/superset/pull/14788#issuecomment-847229865


   # [Codecov](https://codecov.io/gh/apache/superset/pull/14788?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 [#14788](https://codecov.io/gh/apache/superset/pull/14788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ba34d7d) into [master](https://codecov.io/gh/apache/superset/commit/e9657afe4b68663039b0bd8851cbafa8dfad968b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e9657af) will **increase** coverage by `0.08%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head ba34d7d differs from pull request most recent head 4842547. Consider uploading reports for the commit 4842547 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/14788/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/14788?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   #14788      +/-   ##
   ==========================================
   + Coverage   77.54%   77.63%   +0.08%     
   ==========================================
     Files         962      962              
     Lines       49147    49152       +5     
     Branches     6182     6184       +2     
   ==========================================
   + Hits        38111    38157      +46     
   + Misses      10832    10792      -40     
   + Partials      204      203       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `72.44% <100.00%> (+0.18%)` | :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/14788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...c/filters/components/Select/SelectFilterPlugin.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvU2VsZWN0RmlsdGVyUGx1Z2luLnRzeA==) | `70.00% <100.00%> (+0.76%)` | :arrow_up: |
   | [...RUD/data/database/DatabaseModal/SqlAlchemyForm.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL1NxbEFsY2hlbXlGb3JtLnRzeA==) | `100.00% <0.00%> (ø)` | |
   | [.../database/DatabaseModal/DatabaseConnectionForm.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL0RhdGFiYXNlQ29ubmVjdGlvbkZvcm0udHN4) | `54.54% <0.00%> (+4.54%)` | :arrow_up: |
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `53.48% <0.00%> (+6.51%)` | :arrow_up: |
   | [...c/views/CRUD/data/database/DatabaseModal/styles.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL3N0eWxlcy50cw==) | `97.36% <0.00%> (+10.52%)` | :arrow_up: |
   | [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `76.12% <0.00%> (+12.31%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14788?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/14788?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 [e9657af...4842547](https://codecov.io/gh/apache/superset/pull/14788?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.

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 #14788: [WIP] fix(native-filters): fix loop caused by external state handler

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



##########
File path: superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
##########
@@ -100,29 +99,36 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
     defaultToFirstItem,
     searchAllOptions,
   } = formData;
+  const groupby = ensureIsArray<string>(formData.groupby);
+  const [col] = groupby;
+  const [currentSuggestionSearch, setCurrentSuggestionSearch] = useState('');
+  const [dataMask, dispatchDataMask] = useReducer<DataMaskReducer>(reducer, {
+    filterState,
+    ownState: {
+      coltypeMap,
+    },
+  });

Review comment:
       This was moved changed to always initialize `ownState` to make sure the column type mappings are available if the `searchAllOptions` option is enabled after mounting.




-- 
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 #14788: fix(native-filters): fix loop caused by external state handler

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


   > Could you improve types after recent changes? I'm getting a few new TS errors after the previous PR
   > ![image](https://user-images.githubusercontent.com/15073128/119485083-f4648e80-bd56-11eb-9ca8-2e38163547c7.png)
   
   Hmm, these should have been fixed by the `superset-ui` bump in #14710


-- 
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[bot] edited a comment on pull request #14788: fix(native-filters): fix loop caused by external state handler

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14788:
URL: https://github.com/apache/superset/pull/14788#issuecomment-847229865


   # [Codecov](https://codecov.io/gh/apache/superset/pull/14788?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 [#14788](https://codecov.io/gh/apache/superset/pull/14788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (37eedcf) into [master](https://codecov.io/gh/apache/superset/commit/9bf07cc428f0b88958d9a5c013b1f436587da529?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (9bf07cc) will **decrease** coverage by `0.02%`.
   > The diff coverage is `47.82%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/14788/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/14788?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   #14788      +/-   ##
   ==========================================
   - Coverage   77.39%   77.37%   -0.03%     
   ==========================================
     Files         962      962              
     Lines       49156    49154       -2     
     Branches     6183     6182       -1     
   ==========================================
   - Hits        38046    38031      -15     
   - Misses      10907    10922      +15     
   + Partials      203      201       -2     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `72.40% <47.82%> (-0.06%)` | :arrow_down: |
   
   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/14788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...tersConfigModal/FiltersConfigForm/DefaultValue.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyc0NvbmZpZ01vZGFsL0ZpbHRlcnNDb25maWdGb3JtL0RlZmF1bHRWYWx1ZS50c3g=) | `26.31% <0.00%> (-1.47%)` | :arrow_down: |
   | [...c/filters/components/Select/SelectFilterPlugin.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvU2VsZWN0RmlsdGVyUGx1Z2luLnRzeA==) | `54.66% <50.00%> (-14.57%)` | :arrow_down: |
   | [superset-frontend/src/dataMask/actions.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2RhdGFNYXNrL2FjdGlvbnMudHM=) | `71.42% <0.00%> (-7.15%)` | :arrow_down: |
   | [...board/components/nativeFilters/FilterBar/index.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL25hdGl2ZUZpbHRlcnMvRmlsdGVyQmFyL2luZGV4LnRzeA==) | `98.87% <0.00%> (-1.13%)` | :arrow_down: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14788?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/14788?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 [9bf07cc...37eedcf](https://codecov.io/gh/apache/superset/pull/14788?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.

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 #14788: fix(native-filters): fix loop caused by external state handler

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



##########
File path: superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
##########
@@ -100,29 +99,36 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
     defaultToFirstItem,
     searchAllOptions,
   } = formData;
+  const groupby = ensureIsArray<string>(formData.groupby);
+  const [col] = groupby;
+  const [currentSuggestionSearch, setCurrentSuggestionSearch] = useState('');
+  const [dataMask, dispatchDataMask] = useReducer<DataMaskReducer>(reducer, {
+    filterState,
+    ownState: {
+      coltypeMap,
+    },
+  });

Review comment:
       This was changed to always initialize `ownState` to make sure the column type mappings are available if the `searchAllOptions` option is enabled after mounting.




-- 
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 #14788: fix(native-filters): fix loop caused by external state handler

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


   > > Hmm, these should have been fixed by the `superset-ui` bump in #14710
   > 
   > I re-pulled the changes and rerun npm install just to be sure, but I'm getting those errors anyway 🙁
   
   This is what I get:
   
   ![image](https://user-images.githubusercontent.com/33317356/119488602-6d69e300-bd63-11eb-97ab-285e22d2a28c.png)
   


-- 
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 merged pull request #14788: fix(native-filters): fix loop caused by external state handler

Posted by GitBox <gi...@apache.org>.
villebro merged pull request #14788:
URL: https://github.com/apache/superset/pull/14788


   


-- 
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 #14788: fix(native-filters): fix loop caused by external state handler

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


   Could you improve types after recent changes? I'm getting a few new TS errors after the previous PR 
   ![image](https://user-images.githubusercontent.com/15073128/119485083-f4648e80-bd56-11eb-9ca8-2e38163547c7.png)
   


-- 
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[bot] edited a comment on pull request #14788: [WIP] fix(native-filters): apply null as unset state

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14788:
URL: https://github.com/apache/superset/pull/14788#issuecomment-847229865


   # [Codecov](https://codecov.io/gh/apache/superset/pull/14788?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 [#14788](https://codecov.io/gh/apache/superset/pull/14788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ba34d7d) into [master](https://codecov.io/gh/apache/superset/commit/e9657afe4b68663039b0bd8851cbafa8dfad968b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e9657af) will **increase** coverage by `0.08%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head ba34d7d differs from pull request most recent head 2c94330. Consider uploading reports for the commit 2c94330 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/14788/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/14788?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   #14788      +/-   ##
   ==========================================
   + Coverage   77.54%   77.63%   +0.08%     
   ==========================================
     Files         962      962              
     Lines       49147    49152       +5     
     Branches     6182     6184       +2     
   ==========================================
   + Hits        38111    38157      +46     
   + Misses      10832    10792      -40     
   + Partials      204      203       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `72.44% <100.00%> (+0.18%)` | :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/14788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...c/filters/components/Select/SelectFilterPlugin.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvU2VsZWN0RmlsdGVyUGx1Z2luLnRzeA==) | `70.00% <100.00%> (+0.76%)` | :arrow_up: |
   | [...RUD/data/database/DatabaseModal/SqlAlchemyForm.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL1NxbEFsY2hlbXlGb3JtLnRzeA==) | `100.00% <0.00%> (ø)` | |
   | [.../database/DatabaseModal/DatabaseConnectionForm.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL0RhdGFiYXNlQ29ubmVjdGlvbkZvcm0udHN4) | `54.54% <0.00%> (+4.54%)` | :arrow_up: |
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `53.48% <0.00%> (+6.51%)` | :arrow_up: |
   | [...c/views/CRUD/data/database/DatabaseModal/styles.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL3N0eWxlcy50cw==) | `97.36% <0.00%> (+10.52%)` | :arrow_up: |
   | [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `76.12% <0.00%> (+12.31%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14788?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/14788?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 [e9657af...2c94330](https://codecov.io/gh/apache/superset/pull/14788?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.

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] michael-s-molina commented on pull request #14788: fix(native-filters): fix loop caused by external state handler

Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #14788:
URL: https://github.com/apache/superset/pull/14788#issuecomment-847805333


   > > > Hmm, these should have been fixed by the `superset-ui` bump in #14710
   > > 
   > > 
   > > I re-pulled the changes and rerun npm install just to be sure, but I'm getting those errors anyway 🙁
   > 
   > I got these errors before. however, run `npm ci`, these errors are no more.
   
   No errors for me also


-- 
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[bot] edited a comment on pull request #14788: fix(native-filters): fix loop caused by external state handler

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14788:
URL: https://github.com/apache/superset/pull/14788#issuecomment-847229865


   # [Codecov](https://codecov.io/gh/apache/superset/pull/14788?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 [#14788](https://codecov.io/gh/apache/superset/pull/14788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ba34d7d) into [master](https://codecov.io/gh/apache/superset/commit/252c64b3979f15789f3f56c96ce896fa6cff8164?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (252c64b) will **increase** coverage by `0.23%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head ba34d7d differs from pull request most recent head 37eedcf. Consider uploading reports for the commit 37eedcf to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/14788/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/14788?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   #14788      +/-   ##
   ==========================================
   + Coverage   77.39%   77.63%   +0.23%     
   ==========================================
     Files         962      962              
     Lines       49154    49152       -2     
     Branches     6183     6184       +1     
   ==========================================
   + Hits        38044    38157     +113     
   + Misses      10907    10792     -115     
     Partials      203      203              
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `72.44% <100.00%> (-0.02%)` | :arrow_down: |
   
   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/14788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...c/filters/components/Select/SelectFilterPlugin.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvU2VsZWN0RmlsdGVyUGx1Z2luLnRzeA==) | `70.00% <100.00%> (+0.76%)` | :arrow_up: |
   | [...erset-frontend/src/SqlLab/components/ResultSet.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1Jlc3VsdFNldC50c3g=) | `67.44% <0.00%> (-1.56%)` | :arrow_down: |
   | [superset/db\_engine\_specs/redshift.py](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL3JlZHNoaWZ0LnB5) | `94.44% <0.00%> (-0.80%)` | :arrow_down: |
   | [superset/views/chart/views.py](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQvdmlld3MvY2hhcnQvdmlld3MucHk=) | `88.63% <0.00%> (ø)` | |
   | [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `58.97% <0.00%> (ø)` | |
   | [superset/utils/core.py](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQvdXRpbHMvY29yZS5weQ==) | `89.04% <0.00%> (+0.12%)` | :arrow_up: |
   | [superset/models/core.py](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQvbW9kZWxzL2NvcmUucHk=) | `89.97% <0.00%> (+0.26%)` | :arrow_up: |
   | [superset/db\_engine\_specs/base.py](https://codecov.io/gh/apache/superset/pull/14788/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQvZGJfZW5naW5lX3NwZWNzL2Jhc2UucHk=) | `88.30% <0.00%> (+0.40%)` | :arrow_up: |
   | [superset/databases/schemas.py](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQvZGF0YWJhc2VzL3NjaGVtYXMucHk=) | `99.57% <0.00%> (+0.85%)` | :arrow_up: |
   | [superset/connectors/sqla/models.py](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQvY29ubmVjdG9ycy9zcWxhL21vZGVscy5weQ==) | `89.74% <0.00%> (+1.66%)` | :arrow_up: |
   | ... and [4 more](https://codecov.io/gh/apache/superset/pull/14788/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14788?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/14788?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 [9bf07cc...37eedcf](https://codecov.io/gh/apache/superset/pull/14788?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.

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 #14788: fix(native-filters): fix loop caused by external state handler

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



##########
File path: superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
##########
@@ -100,29 +99,36 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
     defaultToFirstItem,
     searchAllOptions,
   } = formData;
+  const groupby = ensureIsArray<string>(formData.groupby);
+  const [col] = groupby;
+  const [currentSuggestionSearch, setCurrentSuggestionSearch] = useState('');
+  const [dataMask, dispatchDataMask] = useReducer<DataMaskReducer>(reducer, {
+    filterState,
+    ownState: {
+      coltypeMap,
+    },
+  });

Review comment:
       This was changed to always initialize `ownState` to make sure the column type mappings are available if the `searchAllOptions` option is enabled after initialization.




-- 
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 #14788: fix(native-filters): fix loop caused by external state handler

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


   > Hmm, these should have been fixed by the `superset-ui` bump in #14710
   
   I re-pulled the changes and rerun npm install just to be sure, but I'm getting those errors anyway 🙁 


-- 
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] zhaoyongjie commented on pull request #14788: fix(native-filters): fix loop caused by external state handler

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


   > > Hmm, these should have been fixed by the `superset-ui` bump in #14710
   > 
   > I re-pulled the changes and rerun npm install just to be sure, but I'm getting those errors anyway 🙁
   
   I got these errors before. however, run `npm ci`, these errors are no more.


-- 
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 a change in pull request #14788: [WIP] fix(native-filters): fix loop caused by external state handler

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



##########
File path: superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
##########
@@ -100,29 +99,36 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
     defaultToFirstItem,
     searchAllOptions,
   } = formData;
+  const groupby = ensureIsArray<string>(formData.groupby);
+  const [col] = groupby;
+  const [currentSuggestionSearch, setCurrentSuggestionSearch] = useState('');
+  const [dataMask, dispatchDataMask] = useReducer<DataMaskReducer>(reducer, {
+    filterState,
+    ownState: {
+      coltypeMap,
+    },
+  });
+  const updateDataMask = (values: SelectValue) => {
+    const emptyFilter =
+      enableEmptyFilter && !inverseSelection && values?.length === 0;
+
+    dispatchDataMask({
+      type: 'filterState',
+      extraFormData: getSelectExtraFormData(
+        col,
+        values,
+        emptyFilter,
+        inverseSelection,
+      ),
+      filterState: {
+        value: values,
+      },
+    });
+  };
 
   const isDisabled =
     appSection === AppSection.FILTER_CONFIG_MODAL && defaultToFirstItem;
 
-  const groupby = ensureIsArray<string>(formData.groupby);
-  // Correct initial value for Ant Select
-
-  // If we are in config modal we always need show empty select for `defaultToFirstItem`
-  const [values, setValues] = useState<SelectValue>(
-    !isDisabled && defaultValue?.length ? defaultValue : [],
-  );

Review comment:
       state handling is now deferred to the application, i.e. filter state is only being set via `setDataMask`.




-- 
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[bot] commented on pull request #14788: fix(native-filters): apply null as unset state

Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #14788:
URL: https://github.com/apache/superset/pull/14788#issuecomment-847229865


   # [Codecov](https://codecov.io/gh/apache/superset/pull/14788?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 [#14788](https://codecov.io/gh/apache/superset/pull/14788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ba34d7d) into [master](https://codecov.io/gh/apache/superset/commit/e9657afe4b68663039b0bd8851cbafa8dfad968b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e9657af) will **increase** coverage by `0.08%`.
   > The diff coverage is `100.00%`.
   
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/14788/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/14788?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   #14788      +/-   ##
   ==========================================
   + Coverage   77.54%   77.63%   +0.08%     
   ==========================================
     Files         962      962              
     Lines       49147    49152       +5     
     Branches     6182     6184       +2     
   ==========================================
   + Hits        38111    38157      +46     
   + Misses      10832    10792      -40     
   + Partials      204      203       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `72.44% <100.00%> (+0.18%)` | :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/14788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...c/filters/components/Select/SelectFilterPlugin.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvU2VsZWN0RmlsdGVyUGx1Z2luLnRzeA==) | `70.00% <100.00%> (+0.76%)` | :arrow_up: |
   | [...RUD/data/database/DatabaseModal/SqlAlchemyForm.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL1NxbEFsY2hlbXlGb3JtLnRzeA==) | `100.00% <0.00%> (ø)` | |
   | [.../database/DatabaseModal/DatabaseConnectionForm.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL0RhdGFiYXNlQ29ubmVjdGlvbkZvcm0udHN4) | `54.54% <0.00%> (+4.54%)` | :arrow_up: |
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `53.48% <0.00%> (+6.51%)` | :arrow_up: |
   | [...c/views/CRUD/data/database/DatabaseModal/styles.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL3N0eWxlcy50cw==) | `97.36% <0.00%> (+10.52%)` | :arrow_up: |
   | [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `76.12% <0.00%> (+12.31%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14788?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/14788?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 [e9657af...ba34d7d](https://codecov.io/gh/apache/superset/pull/14788?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.

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 #14788: [WIP] fix(native-filters): fix loop caused by external state handler

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



##########
File path: superset-frontend/src/filters/components/Select/SelectFilterPlugin.tsx
##########
@@ -154,67 +160,44 @@ export default function PluginFilterSelect(props: PluginFilterSelectProps) {
     }
   };
 
-  useEffect(() => {
-    const firstItem: SelectValue = data[0]
-      ? (groupby.map(col => data[0][col]) as string[])
-      : null;
-    if (!isDisabled && defaultToFirstItem && firstItem) {
-      // initialize to first value if set to default to first item
-      setValues(firstItem);
-    } else if (!isDisabled && defaultValue?.length) {
-      // initialize to saved value
-      setValues(defaultValue);
-    }
-  }, [defaultToFirstItem, defaultValue]);
-

Review comment:
       This `useEffect` has been combined with the other one that was checking for changes in controls (e.g. inverse selection etc). Now there are only two `useEffects` left - one to check for changes in data mask, and another to check for changes in control values.




-- 
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 #14788: fix(native-filters): fix loop caused by external state handler

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


   > > > Hmm, these should have been fixed by the `superset-ui` bump in #14710
   > > 
   > > 
   > > I re-pulled the changes and rerun npm install just to be sure, but I'm getting those errors anyway 🙁
   > 
   > I got these errors before. however, run `npm ci`, these errors are no more.
   
   Thanks Yongjie, that solved the errors!


-- 
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[bot] edited a comment on pull request #14788: [WIP] fix(native-filters): fix loop caused by external state handler

Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14788:
URL: https://github.com/apache/superset/pull/14788#issuecomment-847229865


   # [Codecov](https://codecov.io/gh/apache/superset/pull/14788?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 [#14788](https://codecov.io/gh/apache/superset/pull/14788?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (ba34d7d) into [master](https://codecov.io/gh/apache/superset/commit/e9657afe4b68663039b0bd8851cbafa8dfad968b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (e9657af) will **increase** coverage by `0.08%`.
   > The diff coverage is `100.00%`.
   
   > :exclamation: Current head ba34d7d differs from pull request most recent head 0b65ad1. Consider uploading reports for the commit 0b65ad1 to get more accurate results
   [![Impacted file tree graph](https://codecov.io/gh/apache/superset/pull/14788/graphs/tree.svg?width=650&height=150&src=pr&token=KsB0fHcx6l&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)](https://codecov.io/gh/apache/superset/pull/14788?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   #14788      +/-   ##
   ==========================================
   + Coverage   77.54%   77.63%   +0.08%     
   ==========================================
     Files         962      962              
     Lines       49147    49152       +5     
     Branches     6182     6184       +2     
   ==========================================
   + Hits        38111    38157      +46     
   + Misses      10832    10792      -40     
   + Partials      204      203       -1     
   ```
   
   | Flag | Coverage Δ | |
   |---|---|---|
   | javascript | `72.44% <100.00%> (+0.18%)` | :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/14788?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
   |---|---|---|
   | [...c/filters/components/Select/SelectFilterPlugin.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2ZpbHRlcnMvY29tcG9uZW50cy9TZWxlY3QvU2VsZWN0RmlsdGVyUGx1Z2luLnRzeA==) | `70.00% <100.00%> (+0.76%)` | :arrow_up: |
   | [...RUD/data/database/DatabaseModal/SqlAlchemyForm.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL1NxbEFsY2hlbXlGb3JtLnRzeA==) | `100.00% <0.00%> (ø)` | |
   | [.../database/DatabaseModal/DatabaseConnectionForm.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL0RhdGFiYXNlQ29ubmVjdGlvbkZvcm0udHN4) | `54.54% <0.00%> (+4.54%)` | :arrow_up: |
   | [superset-frontend/src/views/CRUD/hooks.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvaG9va3MudHM=) | `53.48% <0.00%> (+6.51%)` | :arrow_up: |
   | [...c/views/CRUD/data/database/DatabaseModal/styles.ts](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL3N0eWxlcy50cw==) | `97.36% <0.00%> (+10.52%)` | :arrow_up: |
   | [...c/views/CRUD/data/database/DatabaseModal/index.tsx](https://codecov.io/gh/apache/superset/pull/14788/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL3ZpZXdzL0NSVUQvZGF0YS9kYXRhYmFzZS9EYXRhYmFzZU1vZGFsL2luZGV4LnRzeA==) | `76.12% <0.00%> (+12.31%)` | :arrow_up: |
   
   ------
   
   [Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14788?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/14788?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 [e9657af...0b65ad1](https://codecov.io/gh/apache/superset/pull/14788?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.

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