Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Filter by repo not showing pkgs if exceeding pagination #5165

Closed
antgamdia opened this issue Aug 4, 2022 · 3 comments
Closed

Filter by repo not showing pkgs if exceeding pagination #5165

antgamdia opened this issue Aug 4, 2022 · 3 comments
Assignees
Labels
component/ui Issue related to kubeapps UI kind/bug An issue that reports a defect in an existing feature
Milestone

Comments

@antgamdia
Copy link
Contributor

Describe the bug

When debugging why my PR #5115 was failing I noticed there is an actual issue when several repos have been added and we try to filter out by repos: the filter yields a No application matches the current filter. even if when removing some repos the package pops up.

To Reproduce

  1. Add the bitnami repo (as it contains several packages)
  2. Add another repo with at least 1 package (like the chartmusseum one that we install when e2e-testing)
  3. Do not scroll down to the bottom (to prevent the whole packages to be fetched)
  4. Click on the repo filter and select the additional one you added (so the URL becomes sth like /catalog?Repository=my-repo1)
  5. Notice the No application matches the current filter. message.

Expected behavior
The packages from the additional repo should appear.

Screenshots
N/A

Desktop (please complete the following information):

N/A

Additional context
This is blocking #5115 (and therefore the release)

@antgamdia antgamdia added the kind/bug An issue that reports a defect in an existing feature label Aug 4, 2022
@antgamdia antgamdia self-assigned this Aug 4, 2022
@antgamdia
Copy link
Contributor Author

antgamdia commented Aug 4, 2022

While debugging this issue I noticed it can be somehow related to #5168. At some point the react state gets reset (because of a new repos filter item) and the isFetching param is wrongly set (because the carvel plugin is returning data when shouldn't, I guess).
I'm still looking into it, but probably will send a fix for #5168 on top of #5115 to see if this works around the problem.

Edit: it does not (although locally I wasn't able to reproduce the issue after the fix...)
Tomorrow, I'll try just enabling the helm plugin, as the e2e does... but I'd bet it has to do with a sort of race condition. Locally, the issue was intermittently reproducible (but it is consistently failing in CI...)

@antgamdia
Copy link
Contributor Author

Well... after some investigation here I think I've discovered some problems:

  • The pagination wasn't working as designed, that is, on the very first load we were retrieving the whole list of packages regardless of the scrolling.
    • Proposed solution: make the UI decide when to paginate, but leave the nextPageToken selection to the backend.

Example of my local's vs what is currently on main just accessing /#/c/default/ns/default/catalog:
image

  • Because of the above (well, not because, but above behavior was increasing the frecency) a filter checkbox could be clicked arbitrarily (which may be OK) leading, in some cases, to a component re-render with in-flight state updates. A solution was given at Don't reset catalog on first render. #4714, but the conditional ignore was leading to this issue.
    • Proposed solution: disable the checkboxes when loading to prevent multiple clicks.
Show demo video https://user-images.githubusercontent.com/11535726/183107750-2c39f44a-0b44-4a7c-984b-30da7e8cb8a5.gif
  • Even if we implement the aforementioned mitigations, there are still some edge cases where the state remains inconsistent. For instance, when selecting a repo filter, scrolling down (to trigger pagination), selecting another repo filter (to unmount the component)... would lead to a request made to the API when the app is in an isFetching=true state.
    • Proposed solution: it is twofold:
      • Absorb small race conditions with a dummy wait: this will prevent most of the errors to appear, because it will simply retry the request in 1 or 2 seconds. It requires storing a local copy (compnent's state) of the isFetching global state var.
      • Return an error if a "request_packages" request arrives at the state reducer while in an isFetching=true state. The UI will simply show a "try again" button, and the user can click on it. This check is to ensure no inconsistent data is shown to the user.

image

@antgamdia
Copy link
Contributor Author

Closing as all the related PRs have already been merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/ui Issue related to kubeapps UI kind/bug An issue that reports a defect in an existing feature
Projects
Archived in project
Development

No branches or pull requests

1 participant