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.

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

Improve ldap avatar handling #50162

Merged
merged 6 commits into from
Jan 16, 2025
Merged

Improve ldap avatar handling #50162

merged 6 commits into from
Jan 16, 2025

Conversation

come-nc
Copy link
Contributor

@come-nc come-nc commented Jan 13, 2025 •
edited
Loading

  • Resolves: #

Summary

  • Improve typing in user_ldap User class
  • Remove obsolete limitation of only updating avatar after login

Checklist

@come-nc come-nc added the 3. to review Waiting for reviews label Jan 13, 2025
@come-nc come-nc added this to the Nextcloud 31 milestone Jan 13, 2025
@come-nc come-nc self-assigned this Jan 13, 2025
@come-nc come-nc requested a review from blizzz January 13, 2025 17:27
@come-nc come-nc requested review from a team, ArtificialOwl, artonge and yemkareems and removed request for a team January 13, 2025 17:27
blizzz
blizzz approved these changes Jan 13, 2025
View reviewed changes
Copy link
Member

@blizzz blizzz left a comment

Choose a reason for hiding this comment

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

A question, code looks good.

apps/user_ldap/lib/User/User.php Show resolved Hide resolved
@skjnldsv skjnldsv mentioned this pull request Jan 14, 2025
Merged
kesselb
kesselb approved these changes Jan 16, 2025
View reviewed changes
Copy link
Contributor

@kesselb kesselb left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM 👍

  • Login with a clean user does load the avatar ☑️
  • ldap:check-user --update also load the avatar ☑️
  • Visiting the profile of a user, that has never logged in, as admin does not trigger the avatar loading for me.

Nice code cleanup 🧹

@skjnldsv skjnldsv mentioned this pull request Jan 16, 2025
Merged
@blizzz blizzz merged commit 2de855f into master Jan 16, 2025
188 checks passed
@blizzz blizzz deleted the fix/improve-ldap-avatar-handling branch January 16, 2025 20:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@blizzz blizzz blizzz approved these changes

@kesselb kesselb kesselb approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@artonge artonge Awaiting requested review from artonge artonge was automatically assigned from nextcloud/server-backend

@yemkareems yemkareems Awaiting requested review from yemkareems yemkareems was automatically assigned from nextcloud/server-backend

@provokateurin provokateurin Awaiting requested review from provokateurin

Assignees

@come-nc come-nc

Labels
3. to review Waiting for reviews feature: ldap
Projects
None yet
Milestone
Nextcloud 31
Development

Successfully merging this pull request may close these issues.
3 participants
@come-nc @blizzz @kesselb