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.

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

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

would be great to have tests covering that

This is already covered by

$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
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
This was referenced Nov 29, 2020
@jderusse jderusse mentioned this pull request May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants