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 2022/03/01 11:43:01 UTC
[GitHub] [superset] codemaster08240328 opened a new pull request #18981: Fix: Dataset Fuzzy search issue on creating a new chart.
codemaster08240328 opened a new pull request #18981:
URL: https://github.com/apache/superset/pull/18981
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Dataset Fuzzy search in "Create a new Chart" UI isn't searching all available datasets when there are more than 100 datasets.
[ISSUE](https://github.com/apache/superset/issues/18034)
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina removed a comment on pull request #18981: Fix: Dataset Fuzzy search issue on creating a new chart.
Posted by GitBox <gi...@apache.org>.
michael-s-molina removed a comment on pull request #18981:
URL: https://github.com/apache/superset/pull/18981#issuecomment-1055696835
@codemaster08240328 Thanks for looking into this. We can't fix this problem using `fetchOnlyOnSearch` because we intend to keep the behavior of opening a Select and scrolling through its options without typing anything. To fix this we need to investigate why that particular text was not retrieved from the server-side. If instead of setting `fetchOnlyOnSearch` you simulate a page size smaller than the total results like `pageSize={10}` you can see the requests fired to the server with the typed text. The only case where we won't fire a request to the server is if all the options are already loaded on the client-side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina edited a comment on pull request #18981: Fix: Dataset Fuzzy search issue on creating a new chart.
Posted by GitBox <gi...@apache.org>.
michael-s-molina edited a comment on pull request #18981:
URL: https://github.com/apache/superset/pull/18981#issuecomment-1055696835
@codemaster08240328 Thanks for looking into this. We can't fix this problem using `fetchOnlyOnSearch` because we intend to keep the behavior of opening a Select and scrolling through its options without typing anything. To fix this we need to investigate why that particular text was not retrieved from the server-side. If instead of setting `fetchOnlyOnSearch` you simulate a page size smaller than the total results like `pageSize={10}` you can see the requests fired to the server with the typed text. The only case where we won't fire a request to the server is if all the options are already loaded on the client-side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] michael-s-molina commented on pull request #18981: Fix: Dataset Fuzzy search issue on creating a new chart.
Posted by GitBox <gi...@apache.org>.
michael-s-molina commented on pull request #18981:
URL: https://github.com/apache/superset/pull/18981#issuecomment-1055696835
@codemaster08240328 Thanks for looking into this. We can't fix this problem using `fetchOnlyOnSearch` because we intend to keep the behavior of opening a Select and scrolling through its options without typing anything. To fix this we need to investigate why that particular text was not retrieved from the server-side. If instead of setting `fetchOnlyOnSearch` you simulate a page size smaller than the total results like pageSize={10} you can see the requests fired to the server with the typed text. The only case where we won't fire a request to the server is if all the options are already loaded on the client-side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #18981: Fix: Dataset Fuzzy search issue on creating a new chart.
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #18981:
URL: https://github.com/apache/superset/pull/18981#issuecomment-1055511098
It would be great to add a test if possible, e.g. mocking 101 datasets and making sure a fuzzy search yeilds the 101st result.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas commented on pull request #18981: fix: Dataset Fuzzy search issue on creating a new chart.
Posted by GitBox <gi...@apache.org>.
rusackas commented on pull request #18981:
URL: https://github.com/apache/superset/pull/18981#issuecomment-1063674232
FWIW, it looks like we lost some of the PR template in the description, but you can add "Fixes: #xyz" in a PR description to automatically close the issue when a PR merges.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #18981: Fix: Dataset Fuzzy search issue on creating a new chart.
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #18981:
URL: https://github.com/apache/superset/pull/18981#issuecomment-1055394177
# [Codecov](https://codecov.io/gh/apache/superset/pull/18981?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 [#18981](https://codecov.io/gh/apache/superset/pull/18981?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (d97d61e) into [master](https://codecov.io/gh/apache/superset/commit/2072225a8637299c23c337966c9d5aaaeaaa732b?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (2072225) will **not change** coverage.
> The diff coverage is `n/a`.
> :exclamation: Current head d97d61e differs from pull request most recent head 77d3fbc. Consider uploading reports for the commit 77d3fbc to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/18981?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 #18981 +/- ##
=======================================
Coverage 66.40% 66.40%
=======================================
Files 1641 1641
Lines 63520 63520
Branches 6422 6422
=======================================
Hits 42178 42178
Misses 19681 19681
Partials 1661 1661
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `51.01% <ø> (ø)` | |
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/18981?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...perset-frontend/src/addSlice/AddSliceContainer.tsx](https://codecov.io/gh/apache/superset/pull/18981/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-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2FkZFNsaWNlL0FkZFNsaWNlQ29udGFpbmVyLnRzeA==) | `61.76% <ø> (ø)` | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/18981?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/18981?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 [2072225...77d3fbc](https://codecov.io/gh/apache/superset/pull/18981?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] rusackas edited a comment on pull request #18981: Fix: Dataset Fuzzy search issue on creating a new chart.
Posted by GitBox <gi...@apache.org>.
rusackas edited a comment on pull request #18981:
URL: https://github.com/apache/superset/pull/18981#issuecomment-1055511098
It would be great to add a test if possible, e.g. mocking 101 datasets and making sure a fuzzy search yeilds the 101st result.
I think PR Lint is failing just because of the capital F in Fix the PR title.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org