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 not work with groupBy #335

Closed
exbbxak opened this issue Jul 26, 2024 · 13 comments · Fixed by #337
Closed

pagination not work with groupBy #335

exbbxak opened this issue Jul 26, 2024 · 13 comments · Fixed by #337
Labels

Comments

@exbbxak
Copy link

exbbxak commented Jul 26, 2024

Hi, after remove line

->from('(' . $sql . ')', 'dbal_count_tbl')

in DBALQueryBuilderSubscriber pagination not work if in queryBuilder use groupBy

@garak
Copy link
Collaborator

garak commented Jul 26, 2024

Please provide an example

@exbbxak
Copy link
Author

exbbxak commented Jul 26, 2024

$qb = $this->connection->createQueryBuilder()
->select(
'm.id',
'TRIM(CONCAT( m.name_last, ' ', m.name_first, ' ', m.name_middle)) AS "name"',
'm.name_last AS "lastName"',
'm.name_first AS "firstName"',
'm.name_middle AS "middleName"',
'm.email',
'm.status',
'mg.name as groupName',
'mr.name as roleName',
'u.last_activity_at as lastActivityAt',
'u.role',
)
->from('lms2_members_members', 'm')
->leftJoin('m', 'user_users', 'u', 'm.id = u.id')
->leftJoin('m', 'lms2_members_members_groups', 'mmg', 'mmg.member_id = m.id')
->leftJoin('mmg', 'lms2_members_groups', 'mg', 'mmg.group_id = mg.id')
->leftJoin('m', 'lms2_members_members_roles', 'mmr', 'mmr.member_id = m.id')
->leftJoin('mmr', 'lms2_members_role', 'mr', 'mmr.role_id = mr.id');
$qb->groupBy('m.id');
if (!\in_array($sort, ['name', 'email', 'id', 'status', 'lastActivityAt', 'groupName', 'roleName'], true)) {
throw new \UnexpectedValueException('Cannot sort by ' . $sort);
}
$qb->orderBy($sort, $direction === 'desc' ? 'desc' : 'asc');
return $this->paginator->paginate($qb, $page, $size);

@garak
Copy link
Collaborator

garak commented Jul 26, 2024

Well, I know myself what a groupBy is, merely showing a query doesn't help too much to understand the problem.

@exbbxak
Copy link
Author

exbbxak commented Jul 26, 2024

ok sorry, problem in totalCount in pagination

if in DBALQueryBuilderSubscriber on line 35 add ->from('(' . $sql . ')', 'dbal_count_tbl') from v4.3.1 all good work
Screenshot 2024-07-26 190526

but if remove it line totalCount shows incorrect data
2Screenshot 2024-07-26 190548

and pagination not work

@garak garak added the bug label Jul 26, 2024
@Evgeny1973
Copy link

Evgeny1973 commented Jul 26, 2024

Hha, looks like my problem
image
image

@garak
Copy link
Collaborator

garak commented Jul 27, 2024

As commented in the related PR #333, the from has been removed on purpose.

Someone else has had the same issue with other paginators, see this doctrine/dbal#6371
It looks like the only way to fix this problem is injecting the DBAL Connection in the subscriber and re-building the count query from scratch.

Tagging @W0rma as the original proposer for the PR linked above

@garak
Copy link
Collaborator

garak commented Jul 29, 2024

Please check if the proposed fix works for you (I tried it myself and it works for me):

execute composer require --no-update knplabs/knp-components "dev-fix-dbal4 as 4.4.1"
add the following to your composer json:

"repositories": [
    {
        "type": "vcs",
        "url": "https://github.com/garak/knp-components.git"
    }
],

manually edit your vendor/knplabs/knp-paginator-bundle/config/paginator.xml file, adding an optional argument to the knp_paginator service. The service configuration should look like the following:

<service id="knp_paginator" class="Knp\Component\Pager\Paginator" public="true" lazy="true">
    <argument type="service" id="event_dispatcher" />
    <argument type="service" id="Knp\Component\Pager\ArgumentAccess\ArgumentAccessInterface" />
    <argument type="service" id="database_connection" on-invalid="null" /> <!-- this is the new line -->
    <tag name="proxy" interface="Knp\Component\Pager\PaginatorInterface" />
 </service>

run composer update

@Evgeny1973
Copy link

Yes!!!

@exbbxak
Copy link
Author

exbbxak commented Jul 30, 2024

You are awesome! it work! thanks!

@Evgeny1973
Copy link

@garak When will the fix be available?

@garak
Copy link
Collaborator

garak commented Jul 31, 2024

@garak When will the fix be available?

It's not so straightforward.
The PR is merged now, so you can use our dev-master alias (instead of the fork mentioned before).
But we can't release it in a patch or minor version, because it's a BC-break. Since we need a major version, I'd like to close other issues before.

Beside that, we need a PR to add the new argument to the bundle.

@Evgeny1973
Copy link

Evgeny1973 commented Aug 6, 2024

Ok, added a line to the composer,json for now, waiting for release info.
"knplabs/knp-paginator-bundle": "^6.0",
"knplabs/knp-components": "dev-master",

@Evgeny1973
Copy link

Oh.
On dev server

$qb
           ->groupBy('p.id, t.id')
            ->orderBy($sort, 'asc' === $direction ? 'asc' : 'desc') ;
       
return $this->paginator->paginate($qb, $page, $size);

i've got a error:
One of listeners must count and slice given target

On local comp all works fine.

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

Successfully merging a pull request may close this issue.

3 participants