Skip to content

[Ldap] Fix pagination #39031

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Nov 9, 2020
Merged

[Ldap] Fix pagination #39031

merged 1 commit into from
Nov 9, 2020

Conversation

jderusse
Copy link
Member

@jderusse jderusse commented Nov 7, 2020

Q A
Branch? 4.4
Bug fix? yes
New feature? no
Deprecations? no
Tickets #38874
License MIT
Doc PR N/A

This replaces #38875 to fix a bug introduced by #38392

@carsonbot carsonbot added this to the 4.4 milestone Nov 7, 2020
@jderusse jderusse removed this from the 4.4 milestone Nov 7, 2020
@jderusse jderusse added the Ldap label Nov 7, 2020
@jderusse jderusse added this to the 4.4 milestone Nov 7, 2020
@stof
Copy link
Member

stof commented Nov 7, 2020

would be great to have tests covering that

@jderusse
Copy link
Member Author

jderusse commented Nov 7, 2020 •
edited
Loading

would be great to have tests covering that

This is already covered by

symfony/src/Symfony/Component/Ldap/Tests/Adapter/ExtLdap/AdapterTest.php

Lines 132 to 135 in 7b4d22f

$fully_paged_query = $ldap->createQuery('dc=symfony,dc=com', '(&(objectClass=applicationProcess)(cn=user*))', [
'scope' => Query::SCOPE_ONE,
'pageSize' => 25,
]);
. With a local LDAP runing, the test suite failed without this patch.

note: Tests are not executed in our CI=> This should be solved by #39030

This PR is about fixing the issue in case neither #38875 is updated with the correct fix, nor #39030 is accepted/ready to merge.

@fabpot fabpot mentioned this pull request Nov 9, 2020
Closed
@fabpot
Copy link
Member

fabpot commented Nov 9, 2020

Thank you @jderusse.

@fabpot fabpot merged commit 46410f8 into symfony:4.4 Nov 9, 2020
@jderusse jderusse deleted the fix-pagination branch November 9, 2020 18:14
@fabpot fabpot mentioned this pull request Nov 10, 2020
Merged
This was referenced Nov 29, 2020
Merged
Merged
@jderusse jderusse mentioned this pull request May 14, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fabpot fabpot fabpot approved these changes

@stof stof stof approved these changes

Assignees
No one assigned
Labels
Bug Ldap Status: Reviewed
Projects
None yet
Milestone
4.4
Development

Successfully merging this pull request may close these issues.
4 participants
@jderusse @stof @fabpot @carsonbot