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

pageParameterName not supported #815

Open
verticka opened this issue Oct 18, 2024 · 10 comments
Open

pageParameterName not supported #815

verticka opened this issue Oct 18, 2024 · 10 comments
Labels

Comments

@verticka
Copy link

Bug Report

Q A
BC Break yes
Bundle version 6.6.1
Symfony version 7.1.5
PHP version 8.3.4

Summary

Parameter "pageParameterName" is not supported in links

Current behavior

Exemple with bootstrap 4: link is /suivi?page=2

How to reproduce

$paginator->paginate($repo->relanceDemande(),$request->query->getInt('pageRelanceDemande',1),50,['pageParameterName'=>'pageRelanceDemande']);

Expected behavior

link : /suivi?pageRelanceDemande=2

@garak
Copy link
Collaborator

garak commented Oct 18, 2024

I just tried and see that the expected behaviour is what I get.
Please provide more information.

@verticka
Copy link
Author

your new knp_pagination_query function no longer supports "pageParameterName"

@garak garak added bug and removed Help user labels Oct 18, 2024
@garak
Copy link
Collaborator

garak commented Oct 18, 2024

The link is generated correctly for me, but the page is not advancing indeed.
Do you have a solution for that? Could you propose a fix?

@verticka
Copy link
Author

Yes i have copied Older version
in my local template and change in configuration

@Jalliuz
Copy link

Jalliuz commented Oct 28, 2024

Same problem here:
When looking at this twig function
new TwigFunction('knp_pagination_query', [PaginationRuntime::class, 'getQueryParams']),
The getQeuryParams does not take the current paginator options into account and we always get the default 'page' instead of 'my-custom-page-name'

@garak
Copy link
Collaborator

garak commented Oct 28, 2024

Could you try this patch on your vendor knp-paginator-bundle dir?
After that, passing the pagination as the third argument of the knp_pagination_query Twig method should work.

--- a/src/Twig/Extension/PaginationRuntime.php
+++ b/src/Twig/Extension/PaginationRuntime.php
@@ -113,16 +113,18 @@ final class PaginationRuntime implements RuntimeExtensionInterface
      * @param int $page
      * @return array<string, mixed>
      */
-    public function getQueryParams(array $query, int $page): array
+    public function getQueryParams(array $query, int $page, ?SlidingPaginationInterface $pagination = null): array
     {
+        $pageName = $pagination?->getPaginatorOption('page_name') ?? $this->pageName;
+
         if ($page === 1 && $this->skipFirstPageLink) {
-            if (isset($query[$this->pageName])) {
-                unset($query[$this->pageName]);
+            if (isset($query[$pageName])) {
+                unset($query[$pageName]);
             }
 
             return $query;
         }
 
-        return array_merge($query, [$this->pageName => $page]);
+        return array_merge($query, [$pageName => $page]);
     }
 }

if you confirm it works, I'll release a patch version later today

@verticka
Copy link
Author

Hello,
It doesn't work because I can't find the variable to put in your template model

It work for me :

`public function getQueryParams(array $query, int $page, ?string $pageParameterName = null): array
{
$pageName = $pageParameterName ?? $this->pageName;

    if ($page === 1 && $this->skipFirstPageLink) {
        if (isset($query[$pageName])) {
            unset($query[$pageName]);
        }

        return $query;
    }



    return array_merge($query, [$pageName => $page]);
}`

on Twig function add pageParameterName
knp_pagination_query(query, previous,pageParameterName)

@garak
Copy link
Collaborator

garak commented Oct 30, 2024

The variable is the pagination object passed to your template

@Jalliuz
Copy link

Jalliuz commented Oct 30, 2024

The solution of Vertica works indeed

public function getQueryParams(
    array $query,
    int $page,
    ?string $pageParameterName = null,
): array {
    $pageName = $pageParameterName ?? $this->pageName;

    if ($page === 1 && $this->skipFirstPageLink) {
        if (isset($query[$pageName])) {
            unset($query[$pageName]);
        }

        return $query;
    }
    return array_merge($query, [$pageName => $page]);
}

And in all places (all templates) change
knp_pagination_query(query, page) to knp_pagination_query(query, page, pageParameterName)

@garak
Copy link
Collaborator

garak commented Oct 30, 2024

I'm sure it works, but it would force use to extract the pageParameterName to be passed everywhere. Moreover, it's not future-proof: if one day we find in the need of another option, we would be forced to add a new argument.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants