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

Fix application of maxed lists following page refresh #559

Merged
merged 3 commits into from
Nov 23, 2022

Conversation

richard-cox
Copy link
Member

@richard-cox richard-cox commented Nov 22, 2022

Fixes #560

Issue

  • When there are too many resources to show in a list we enter max state
  • When in max'd state we force the user to select filters to restrict the number of results (changing filters results in new http requests with the filters)
  • We persist the state of the list to local storage. This includes filters but does not include the max flag
  • When the user re-visits the list after page refresh instead of fetching all resources we only fetch those applicable to the filter (which will be a small subset of all resources). As the max flag isn't persisted we assume we've fetched all resources. Any changes to the filter are then applied locally to the subset of resources... which may not contain any applicable to the new filter

Fix

  • Persist the list's max state such that when the list loads again all filter changes correctly result in http requests
  • This needed a fix in the way we watch the filters for changes (we block this watch until all filters report as ready)
  • Fix downside
    • Lists cannot ever leave the max state. All changes to filters will result in http requests. User would need to clear their local cache to reset the list (we have a button for this in the user's profile page)

Tested

  • Different types of org/space filter setups
    • orgs with no spaces
    • orgs with some spaces under maxed, some over
  • Different types of lists
  • Tested with UI_LIST_ALLOW_LOAD_MAXED true (allow user to overrule maxed state and fetch everything)

Issue
- When there are too many resources to show in a list we enter `max` state
- When in max'd state we force the user to select filters to restrict the number of results (changing filters results in new https requests containing those filters)
- We persist the state of the list to local storage. This includes filters but does not include the `max` flag
- When the user re-vists the list after page refresh instead of fetching all resources we only fetch those applicable to the filter (which will be a small subset of all resources). As the `max` flag isn't there we assume we've fetched all resources. Any changes to the filter are then applied locally to the subset of resources... which won't contain any from the new filter

Fix
- Persist the list's `max` state such that when the list loads again all filter changes correctly result in http requests
- This needed a fix in the way we watch the filters for changes (we block this watch until all filters report as ready)
- Fix downside
  - Lists cannot ever leave the `max` state. All changes to filters will result in http requests. User would need to clear their local cache to reset the list (we have a button for this in the user's profile page)

Tested
- Different types of org/space filter setups
  - orgs with no spaces
  - orgs with some spaces under maxed, some over
- Different types of lists
- Tested with UI_LIST_ALLOW_LOAD_MAXED true (allow user to overrule maxed state and fetch everything)
@richard-cox richard-cox self-assigned this Nov 22, 2022
@richard-cox richard-cox force-pushed the fix-refresh-maxed-list branch from 182656c to 69f6a91 Compare November 23, 2022 11:19
@codecov-commenter
Copy link

Codecov Report

Merging #559 (69f6a91) into master (b4438a9) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #559   +/-   ##
=======================================
  Coverage   28.65%   28.65%           
=======================================
  Files          60       60           
  Lines        5126     5126           
=======================================
  Hits         1469     1469           
  Misses       3470     3470           
  Partials      187      187           

@richard-cox richard-cox merged commit 06242a6 into master Nov 23, 2022
@richard-cox richard-cox deleted the fix-refresh-maxed-list branch November 23, 2022 13:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

List filters fail to work after refreshing a 'max'ed list
3 participants