Do not try to load avatar for users that never logged in #25802
Conversation
|
@butonic, thanks for your PR! By analyzing the annotation information on this pull request, we identified @rullzer, @blizzz and @icewind1991 to be potential reviewers |
| @@ -81,6 +81,8 @@ public function getAvatar($userId) { | |||
| $user = $this->userManager->get($userId); | |||
| if (is_null($user)) { | |||
| throw new \Exception('user does not exist'); | |||
| } else if ($user->getLastLogin() === 0) { | |||
|
@butonic indeed, you're right. Unfortunately this doesn't work with LDAP avatars for users who just logged in. To reproduce:
|
|
I suppose this could be extended to have a check for "is this the user's first login ?" and if yes, proceed to sync the whole LDAP profile. I think the LDAP profile sync already happens but too early at a time where last login is still 0. |
|
👎 as it doesn't work with LDAP |
|
The original issue was about carddav sync. How about skipping users who never logged in in the carddav sync job ? Unfortunately still a quick fix but might do the job for this specific case. |
|
@butonic looks like there's a bug on stable9.1, it turns out that even without your PR the LDAP avatar is only loaded during the second login. So either it's a regression or not... either way your PR might be valid and useful after all... |
|
Seems it's an older issue that avatar only loaded at second login: #25894 |
|
I suggest to close this since we have another solution: #26124 But I'd also like to explore having a "first login" flag and "first login hook" as @DeepDiver1975 mentionned, and then use it to copy skeleton files, etc. It would be useful to have a function "hasThisUserEverLoggedInBefore()" in various parts of the code so we could skip those quickly. |
|
For the first login event there is a pr open |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
alternative approach to not initialize the fs for every user, see #25286 (comment)
supersedes #25286
ref #25790
cc @PVince81 @DeepDiver1975 My mind keeps telling me that the user backend should provide the avatar. In case of ldap that means some jpg from the ldap server. Which is theoretically possible even if the user never logged in before.