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

Pagination Logic Issue in paginate() Method #27

Open
mudassaralichouhan opened this issue Dec 12, 2024 · 0 comments
Open

Pagination Logic Issue in paginate() Method #27

mudassaralichouhan opened this issue Dec 12, 2024 · 0 comments

Comments

@mudassaralichouhan
Copy link

Description

The paginate() method in the repository has a potential issue with pagination logic that may lead to incorrect results in certain scenarios.

Steps to Reproduce

  1. Call the paginate() method with a specific take and page_number.
  2. Observe the values returned for last_page, next_page, and prev_page.

Expected Behavior

The pagination should correctly calculate the total number of pages and provide accurate next and previous page numbers.

Actual Behavior

The current implementation may not handle edge cases correctly, especially when the total count of items is not a multiple of take.

Suggested Fix

The following changes are recommended to improve the pagination logic:

public function paginate(int $take = 15, int $page_number = null)
{
    // Ensure the page number is at least 1
    if ($page_number <= 0) {
        $page_number = 1;
    }

    // Fetch the list of items for the current page
    $list = $this->page($page_number - 1, $take);
    // Get the total count of items
    $count = $this->clone()->count();

    // Calculate the total number of pages
    $params = new stdClass;
    $params->total = $count;
    $params->per_page = $take;
    $params->count = count($list);
    $params->current_page = $page_number;

    // Calculate last page
    $params->last_page = (int) ceil($count / $take); // Use ceil to ensure we round up

    // Determine next and previous page numbers
    $params->next_page = ($page_number < $params->last_page) ? ($page_number + 1) : false;
    $params->prev_page = ($page_number > 1) ? ($page_number - 1) : false;

    // Assign the data to the params
    $params->data = $list;

    return $params;
}

Problem:

{ "last_page": 1, "total": 12, "count": 2, "per_page": 10, "prev_page": false, "next_page": false, "current_page": 2, "data": [ { "uid": 29361, "name": "11/15/2024 10:23 am (New1)", "free": 0, "locked": 1, "created": 1731684217 }, { "uid": 29356, "name": "11/14/2024 08:46 pm (Brooklyn 2 11.15.2024)", "free": 0, "locked": 1, "created": 1731635209 } ] }

The response indicates that there are a total of 12 items, but the last_page is 1. This suggests that there is only one page of results, which is inconsistent with the total count of 12 items.

If there are 12 total items and the per_page is set to 10, then you would expect to have at least 2 pages (the first page containing 10 items and the second page containing the remaining 2 items). Therefore, the last_page should be 2, not 1.

In summary, the response is incorrect because the last_page value does not align with the total number of items and the items per page.

src/Builder.php issue in Builder Class on action pagination.

public function paginate(int $take = 15, int $page_number = null)
  {
    if ($page_number <= 0) {
      $page_number = 1;
    }


    $list = $this->page($page_number - 1, $take);
    $count = $this->clone()->count();

    $params = new stdClass;
    $params->last_page = round($count / $take);


    $nextpage = (($page_number) < $params->last_page) ? ($page_number + 1) : false;

    $prevpage = false;
    if ($page_number <= $params->last_page && $page_number > 1) {
      $prevpage = $page_number - 1;
    }


    $params->total = $count;
    $params->count = count($list);
    $params->per_page = $take;
    $params->prev_page = $prevpage;
    $params->next_page = $nextpage;
    $params->current_page = $page_number;
    $params->data = $list;

    return $params;
  }

Solution:

The pagination logic in your paginate function has a couple of issues that need to be addressed to ensure that the last_page calculation is correct and that the pagination works as expected. Here’s a revised version of your function:

public function paginate(int $take = 15, int $page_number = null)
{
    // Ensure the page number is at least 1
    if ($page_number <= 0) {
        $page_number = 1;
    }

    // Fetch the list of items for the current page
    $list = $this->page($page_number - 1, $take);
    // Get the total count of items
    $count = $this->clone()->count();

    // Calculate the total number of pages
    $params = new stdClass;
    $params->total = $count;
    $params->per_page = $take;
    $params->count = count($list);
    $params->current_page = $page_number;

    // Calculate last page
    $params->last_page = (int) ceil($count / $take); // Use ceil to ensure we round up

    // Determine next and previous page numbers
    $params->next_page = ($page_number < $params->last_page) ? ($page_number + 1) : false;
    $params->prev_page = ($page_number > 1) ? ($page_number - 1) : false;

    // Assign the data to the params
    $params->data = $list;

    return $params;
}

Key Changes Made:

  • Last Page Calculation: Changed the calculation of last_page to use ceil() instead of round(). This ensures that if there are any remaining items that don't fill a complete page, it will still count as an additional page.

  • Parameter Initialization: Moved the initialization of total, per_page, and count before calculating last_page to ensure they are set correctly.

  • Next and Previous Page Logic: Simplified the logic for determining next_page and prev_page to make it clearer.

Explanation:

  • The ceil() function ensures that if there are any leftover items that don't fill a complete page, they will still count towards an additional page.
  • The checks for next_page and prev_page ensure that they are only set when appropriate, preventing any invalid page numbers from being returned.
    This should fix the pagination logic and ensure that the response is accurate based on the total number of items and the items per page.

This should fix the pagination logic and ensure that the response is accurate based on the total number of items and the items per page.

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

1 participant