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

Partial pagination is broken #379

Closed
jvmanji opened this issue May 22, 2021 · 3 comments
Closed

Partial pagination is broken #379

jvmanji opened this issue May 22, 2021 · 3 comments

Comments

@jvmanji
Copy link

jvmanji commented May 22, 2021

API Platform version(s) affected: 2.6.3 & 2.6.4 and probably older versions

Description
When partial pagination is enabled, pagination in admin stops working. Each time the user clicks “Next” a proper request is being sent, but just after admin gets a response, there is another request triggered returning to page 1.

How to reproduce
You may just use Api Platform distribution: https://github.com/api-platform/api-platform/releases, then after setting it up add some records to the DB (e.g. INSERT INTO greeting VALUES (generate_series(100,1000), 'Hello no. ' || generate_series(100,1000)::text);) and then, enable partial pagination in src/Entity/Greeting.php:

/**
 * This is a dummy entity. Remove it!
 * @ApiResource(
 *     attributes={
 *         "pagination_partial"=true
 *     },
 *     mercure=true
 * )
 * @ORM\Entity
 */
class Greeting

Possible Solution

Looks like solution with negative total should be abandoned, and it should be replaced with something else, maybe that meta feature would help: marmelab/react-admin#4834, but still I don't have any idea what should be put inside total when we don't really know how many records there are... I couldn't find the code that checks the value of total parameter and causes that redirect to page=1, so I don't know if there's a possibility to suppress that behaviour. If anyone knows where that code is, I'd be grateful for leaving a comment :-).

Additional Context

Looks like the problem is in admin/src/hydra/dataProvider.js lines 420 - 429. Partial pagination support has been added (#297) by setting total to negative numbers:

                ? -2 // there is a next page
                : -1 // no next page
              : -3, // no information

Looks like now something (react-admin?) validates that number and if it's negative or even “invalid” (AFAIR I found it has to be bigger than page * perPage), then it triggers some kind of refresh / refetch with page set to 1.

When I commented out:

            total: response.json.hasOwnProperty('hydra:totalItems')
              ? response.json['hydra:totalItems']
              : response.json['hydra:view']
              ? response.json['hydra:view']['hydra:next']
                ? -2 // there is a next page
                : -1 // no next page
              : -3, // no information

and hardcoded total: 1000, and then changed admin/src/list/Pagination.js to behave as if there was total < -1:

export default (props) => {
  const { page, total, setPage, ...rest } = props;

  // if (total >= 0) {
  //   return <Pagination page={page} total={total} setPage={setPage} {...rest} />;
  // }

  const classes = useStyles(props);
  const theme = useTheme();
  const translate = useTranslate();

  return (
    <Toolbar>
      <div className={classes.spacer} />
      {page > 1 && (
        <Button color="primary" key="prev" onClick={() => setPage(page - 1)}>
          {theme.direction === 'rtl' ? <ChevronRight /> : <ChevronLeft />}
          {translate('ra.navigation.prev')}
        </Button>
      )}
//      {total < -1 && (
        <Button color="primary" key="next" onClick={() => setPage(page + 1)}>
          {translate('ra.navigation.next')}
          {theme.direction === 'rtl' ? <ChevronLeft /> : <ChevronRight />}
        </Button>
//      )}
    </Toolbar>
  );
};

The pagination started to work properly.

@jvmanji
Copy link
Author

jvmanji commented May 24, 2021

I've found the code resposnsible for this problem. It's this piece of code in react-admin in file packages/ra-core/src/controller/useListController.ts (starting in line 180 in current master branch):

    const totalPages = Math.ceil(total / query.perPage) || 1;

    useEffect(() => {
        if (
            query.page <= 0 ||
            (!loading && query.page > 1 && ids.length === 0)
        ) {
            // Query for a page that doesn't exist, set page to 1
            queryModifiers.setPage(1);
        } else if (!loading && query.page > totalPages) {
            // Query for a page out of bounds, set page to the last existing page
            // It occurs when deleting the last element of the last page
            queryModifiers.setPage(totalPages);
        }
    }, [loading, query.page, ids, queryModifiers, total, totalPages]);

Changing else if condition to: (!loading && query.page > totalPages && total > 0) solves the problem. What do you think, should it be changed there in ra-core, or should current implementation (with negative total) be rewritten?

@alanpoulain
Copy link
Member

Hello,
Thank you for your investigation. Yes it seems to have been changed in this PR: marmelab/react-admin#4894.
I'm not sure if we would be able to find another way to hand over the information about the page (next / previous).
If you find a way to do it, don't hesitate to tell me or to make the PR 🙂

@alanpoulain
Copy link
Member

Partial pagination will be back in 3.0.0 version (RC 0 is already out).

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

No branches or pull requests

2 participants