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: Pagination in data browser not working #2624

Open
wants to merge 2 commits into
base: alpha
Choose a base branch
from

Conversation

muhleder
Copy link

@muhleder muhleder commented Oct 30, 2024

New Pull Request Checklist

Issue Description

Related: #1551

Approach

First issue is that the scrolling element in BrowserTable must have changed, so the scroll listener was never being fired.

Main issue really though was that the fetchNextPage code is much more complex than it needed to be. I've just used the existing state.lastMax variable to record how many records have been retrieved, and use the query.skip() function to do the pagination (as recommended in the query api documentation).

I've also added a state.fetchingNextPage variable to prevent the same fetchNextPage request being fired in quick succession. I noticed this happening during testing and it actually managed to crash my server on the bigger tables.

TODOs before merging

  • Add tests
  • Add changes to documentation (guides, repository pages, in-code descriptions)

Copy link

parse-github-assistant bot commented Oct 30, 2024

Thanks for opening this pull request!

  • ❌ Please link an issue that describes the reason for this pull request, otherwise your pull request will be closed. Make sure to write it as Closes: #123 in the PR description, so I can recognize it.

@muhleder muhleder changed the title Fix infinite scroll. Fixes #1551 Fix infinite scroll. Closes: #1551 Oct 30, 2024
@muhleder muhleder changed the title Fix infinite scroll. Closes: #1551 Closes: #1551 Oct 30, 2024
Copy link

uffizzi-cloud bot commented Oct 30, 2024

Uffizzi Ephemeral Environment deployment-57775

⌚ Updated Oct 30, 2024, 11:27 UTC

☁️ https://app.uffizzi.com/github.com/parse-community/parse-dashboard/pull/2624

📄 View Application Logs etc.

What is Uffizzi? Learn more

@mtrezza
Copy link
Member

mtrezza commented Oct 30, 2024

The history of this issue is almost as infinite as the scrolling itself. We believe that the only efficient and bug-free solution would be using parse-community/parse-server#7637. Everything else has failed so far. Surely pagination is easy in an environment where data does not change, but as soon as a new record is added between page loads, a simple skip won't do the trick anymore. Maybe the "complex pagination query" has been "over-engineered" to a point where a simple solution ends up working seemingly better. How many edge cases we'd be again opening up with going back to square 1 -- no idea. In any case, since pagination currently doesn't work at all, there's also not much to loose.

A few considerations:

  1. The "complex pagination query" that you've replaced is found at 2 places:

    /src/dashboard/Data/Browser/ObjectPickerDialog.react.js
    /src/dashboard/Data/Browser/Browser.react.js

    That begs the questions why duplicate code and should this be de-duplicated, or does the query need to modified only in 1 of the two places?

  2. We'd need to make sure not to introduce any new index requirements for the pagination to work.

  3. It'd be worth reading through Infinite Scroll query update #1706 before we reset the pagination logic, just to not repeat old mistakes.

  4. Which edge cases do we know of and want to handle (and how?) or explicitly ignore at this point? E.g.

    • New record added in-between page loads.
    • Multiple records with same createdAt (if sorted by createdAt).
    • Multiple records with same field name n (if sorted by n).

Note I've made a small edit in the issue description; this PR seems like a temporary workaround (better buggy pagination than none?), but a true fix would require cursor based pagination, so the PR as it currently is would not close #1551.

@mtrezza mtrezza changed the title Closes: #1551 fix: Pagination in data browser not working Oct 30, 2024
@mtrezza
Copy link
Member

mtrezza commented Nov 1, 2024

I think #2571 would be an alternative to allow pagination without getting into many of the issues mentioned in #2624 (comment), since only 1 page will be shown at a time.

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.

2 participants