codeigniter4 / CodeIgniter4

Open Source PHP Framework (originally from EllisLab)
https://codeigniter.com/
MIT License
5.4k stars 1.9k forks source link

Bug: Call to undefined method CodeIgniter\Pager\PagerRenderer::getDetails() with multiple paginations in single view #6230

Closed jozefrebjak closed 2 years ago

jozefrebjak commented 2 years ago

PHP Version

8.0

CodeIgniter4 Version

4.1.2

CodeIgniter4 Installation Method

Composer (as dependency to an existing project)

Which operating systems have you tested for this bug?

macOS, Linux

Which server did you use?

apache

Database

MariaDB 10.6.5

What happened?

After PR https://github.com/codeigniter4/CodeIgniter4/pull/6211, I can access multiple pagers on a single page. It works when I use:

<?= $pager->links('contacts') ?>
<?= $pager->links('orders') ?>

on the same page, but when I'm trying to use template where I'm using function

$pager->getDetails($group ?? 'default')

to get details about pagination it doesn't work on the same page.

Steps to Reproduce

See https://github.com/codeigniter4/CodeIgniter4/issues/6230#issuecomment-1174641005

I'm using partial template for Full pagination like:

<?php
    $details = $pager->getDetails($group ?? 'default');
    $perPage = $details['perPage'];
    $showing = $perPage * $details['currentPage'];
    $total   = $details['total'];
    $from    = $showing + 1 - $perPage;
?>

<div class="row g-0 text-center text-sm-start align-items-center">
    <div class="col-sm-6">
        <div>
            <p class="mb-sm-0"><?= lang('Pager.showingResults', [
                $from < 0 ? 0 : $from,
                $showing > $total ? $total : $showing,
                $total,
            ]) ?></p>
        </div>
    </div> <!-- end col -->
    <div class="col-sm-6">
        <?= $pager->links($group ?? 'default') ?>
    </div><!-- end col -->
</div><!-- end row -->

and calling that partial from view like:

<?= view('pager/_pager', ['group' => 'contacts']) ?>
<?= view('pager/_pager', ['group' => 'orders']) ?>

I'm getting Error like:

Call to undefined method CodeIgniter\Pager\PagerRenderer::getDetails()

Expected Output

Expected is to have an option to call getDetails() from all of the groups.

Anything else?

No response

kenjis commented 2 years ago

Try to change the key name pager to something else.

        $data = [
            'users' => $model->paginate(10),
            'pager' => $model->pager, // the key
        ];

        return view('users/index', $data);
jozefrebjak commented 2 years ago

Try to change the key name pager to something else.

        $data = [
            'users' => $model->paginate(10),
            'pager' => $model->pager, // the key
        ];

        return view('users/index', $data);

@kenjis It works after change from pager -> test and accessing with test variable.

kenjis commented 2 years ago

It seems this is a bug in Pager class, the variable name collision.

pager is used here: https://github.com/codeigniter4/CodeIgniter4/blob/edb894f2fde3eac146094eb3e5e0491a62d47f96/system/Pager/Pager.php#L123-L125 When this code runs, PagerRenderer will be set in the pager in View.

jozefrebjak commented 2 years ago

It seems this is a bug in Pager class, the variable name collision.

pager is used here:

https://github.com/codeigniter4/CodeIgniter4/blob/edb894f2fde3eac146094eb3e5e0491a62d47f96/system/Pager/Pager.php#L123-L125

When this code runs, PagerRenderer will be set in the pager in View.

@kenjis Yes, I can confirm this behaviour.

If I use test instead pager in debug toolbar now I can see what I expect:

CleanShot 2022-07-04 at 14 51 49@2x

After change back to pager it looks like this with used $pager in view like:

<?= $pager->links() ?>

CleanShot 2022-07-04 at 15 06 24@2x

So it's not anymore CodeIgniter\Pager\Pager but CodeIgniter\Pager\PagerRenderer.

jozefrebjak commented 2 years ago

@kenjis

So I think that the whole logic of pager is wrapped around a single pagination on the page. This code will add pager var to a view if there is used different key like pagination instead of the pager.

If the key is also pager, then it will rewrite the whole pager var to PagerRenderrer because of all of the functions like setSurroundCount(2) etc. used in the pager template by default.

As a workaround to get multiple paginations on the same view, we can use a different name for a pager and then it seems to work as needed. It will just add a var pager with a group from which we call a pager.

kenjis commented 2 years ago
<?php

namespace App\Controllers;

class Home extends BaseController
{
    public function index()
    {
        $model = new \App\Models\UserModel();

        $data = [
            'users' => $model->paginate(1),
            'pager' => $model->pager,
        ];

        return view('user', $data);
    }
}
<?php

namespace App\Models;

use CodeIgniter\Model;

class UserModel extends Model
{
    protected $table = 'user';
}

Views/user.php

<?= view('partial') ?>
<?= view('partial') ?>

Views/partial.php

<?php $details = $pager->getDetails() ?>
<?= $pager->links() ?>

Screenshot 2022-07-05 14 46 11

kenjis commented 2 years ago

@jozefrebjak Can you try this?

--- a/system/Pager/Pager.php
+++ b/system/Pager/Pager.php
@@ -122,7 +122,8 @@ class Pager implements PagerInterface

         $pager = new PagerRenderer($this->getDetails($group));

-        return $this->view->setVar('pager', $pager)->render($this->config->templates[$template]);
+        return $this->view->setVar('pager', $pager)
+            ->render($this->config->templates[$template], null, false);
     }

     /**
jozefrebjak commented 2 years ago

@jozefrebjak Can you try this?

--- a/system/Pager/Pager.php
+++ b/system/Pager/Pager.php
@@ -122,7 +122,8 @@ class Pager implements PagerInterface

         $pager = new PagerRenderer($this->getDetails($group));

-        return $this->view->setVar('pager', $pager)->render($this->config->templates[$template]);
+        return $this->view->setVar('pager', $pager)
+            ->render($this->config->templates[$template], null, false);
     }

     /**

@kenjis I can confirm that it works.

kenjis commented 2 years ago

@jozefrebjak Thank you for confirmation. I sent a PR #6251

kenjis commented 1 year ago

@jozefrebjak That fix was not correct. Please check #7758 if you have time.