Skip to content

Conversation

@CarlSchwan
Copy link
Member

This included three commits, see individual commit message for more in-depth explaination

  1. More type hinting and typo fixing
  2. Fix loading user lists
  3. Another user list loading fixing

@CarlSchwan CarlSchwan requested review from blizzz and come-nc December 10, 2021 15:43
@CarlSchwan CarlSchwan force-pushed the fix/ldap_issues branch 5 times, most recently from c8d90ec to b7d1a52 Compare December 12, 2021 14:19
@come-nc
Copy link
Contributor

come-nc commented Dec 13, 2021

Apart from the docblock remark, everything in there seems to improve the situation.

Signed-off-by: Carl Schwan <[email protected]>
Currently the check if an object is in intermediates is done using
group-{gid} for groups but only user- for users. This change the check
for user to user-{uid}.

This fix the listing of users in a group in the users settings.

Signed-off-by: Carl Schwan <[email protected]>
This fixes some cases observed with the debugger where we end up merging
a non empty list with null. The result is then null and the looping over
the items would then end.

Signed-off-by: Carl Schwan <[email protected]>
@come-nc
Copy link
Contributor

come-nc commented Dec 13, 2021

(Lots of psalm error through, please look into them as you added typing they may unravel real problems)

@CarlSchwan
Copy link
Member Author

The psaml errors are unfortunately normal in stable20 and earlier. There are none in the master equivalent of this: #30223

* @throws Exception
*/
private function getNameOfGroup(string $filter, string $cacheKey) {
private function getNameOfGroup(string $filter, string $cacheKey): ?string {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apps/user_ldap/lib/Group_LDAP.php#L472

The declared return type 'null|string' for OCA\User_LDAP\Group_LDAP::getNameOfGroup does not allow false, but the function returns 'false|string'

@CarlSchwan CarlSchwan closed this Jan 14, 2022
@skjnldsv skjnldsv deleted the fix/ldap_issues branch March 14, 2024 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants