Skip to content

Fix generated avatars cache#38143

Merged
szaimen merged 2 commits intomasterfrom
fix/generated-avatars-cache
May 16, 2023
Merged

Fix generated avatars cache#38143
szaimen merged 2 commits intomasterfrom
fix/generated-avatars-cache

Conversation

@provokateurin
Copy link
Member

@provokateurin provokateurin commented May 9, 2023 •
edited
Loading

Summary

Patch needs to be backported to 25 & 26 and have their respective latest released versions set in the comparison

Checklist

@provokateurin provokateurin changed the title Fix/generated avatars cache Fix generated avatars cache May 9, 2023
@provokateurin provokateurin requested review from skjnldsv and szaimen May 9, 2023 10:29
@provokateurin provokateurin added the 3. to review Waiting for reviews label May 9, 2023
@provokateurin provokateurin added this to the Nextcloud 27 milestone May 9, 2023
@provokateurin provokateurin force-pushed the fix/generated-avatars-cache branch from 1093166 to da264cc Compare May 9, 2023 10:31
@provokateurin
Copy link
Member Author

provokateurin commented May 9, 2023

/backport to stable26

@provokateurin
Copy link
Member Author

provokateurin commented May 9, 2023

/backport to stable25

szaimen
szaimen reviewed May 9, 2023
View reviewed changes
szaimen
szaimen reviewed May 9, 2023
View reviewed changes
szaimen
szaimen reviewed May 9, 2023
View reviewed changes
@provokateurin provokateurin force-pushed the fix/generated-avatars-cache branch from da264cc to 66b0be3 Compare May 9, 2023 14:45
@provokateurin provokateurin requested a review from szaimen May 9, 2023 14:45
@provokateurin provokateurin force-pushed the fix/generated-avatars-cache branch from 66b0be3 to 02a020f Compare May 9, 2023 14:46
szaimen
szaimen approved these changes May 9, 2023
View reviewed changes
Copy link
Contributor

@szaimen szaimen left a comment

Choose a reason for hiding this comment

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

LGTM but didnt test

@skjnldsv skjnldsv mentioned this pull request May 9, 2023
Merged
@provokateurin provokateurin force-pushed the fix/generated-avatars-cache branch from 02a020f to 1bc3572 Compare May 15, 2023 05:49
@provokateurin provokateurin requested review from a team, ArtificialOwl and nfebe and removed request for a team May 15, 2023 05:49
provokateurin
provokateurin commented May 15, 2023
View reviewed changes
lib/private/Repair/ClearGeneratedAvatarCache.php
return version_compare($versionFromBeforeUpdate, '25.0.0.10', '<=');
// This job only runs if the server was on a version lower than or equal to 27.0.0 before the upgrade.
// To clear the avatar cache again, bump the version to the currently released version (and change the operator to <= if it's not the master branch) and wait for the next release.
return version_compare($versionFromBeforeUpdate, '27.0.0', '<');
Copy link
Member Author

@provokateurin provokateurin May 15, 2023

Choose a reason for hiding this comment

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

I'm a little tempted to remove this check and always clear the avatar cache. That will make things easier in the future, but of course increase the load if it's not intended to clear the avatar cache.

@provokateurin provokateurin force-pushed the fix/generated-avatars-cache branch from 1bc3572 to 47385be Compare May 15, 2023 09:42
@provokateurin
Properly clear cached avatars
e294045 
Signed-off-by: jld3103 <jld3103yt@gmail.com>
@provokateurin
Clear generated avatar caches
2a19753 
Signed-off-by: jld3103 <jld3103yt@gmail.com>
@provokateurin provokateurin force-pushed the fix/generated-avatars-cache branch from 47385be to 2a19753 Compare May 15, 2023 16:18
@szaimen
Copy link
Contributor

szaimen commented May 16, 2023

CI failure seems unrelated. So merge @provokateurin ?

@provokateurin
Copy link
Member Author

provokateurin commented May 16, 2023

Yes, I don't have the permissions though. Can you?

@szaimen szaimen merged commit 48521d6 into master May 16, 2023
@szaimen szaimen deleted the fix/generated-avatars-cache branch May 16, 2023 09:37
This was referenced May 16, 2023
Merged
Merged
@blizzz blizzz mentioned this pull request May 17, 2023
Merged
@DaTweety
Copy link

DaTweety commented Sep 17, 2023

Don't know if this is the right place.
I found out that on most of the installations the "avatar.64.png" is simply missing but used in the image tag.
Update to 27.1.0.7 doesn't fix the issue unless the user logged into the system. By using smartphone/desktop-clients they never do.
Is there still no occ routine to build up/generate the .64.png files? Tried it for testing purposes with converting the .512.png to .64x64px and named that .64.png

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bigcat88 bigcat88 bigcat88 approved these changes

@szaimen szaimen szaimen approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl is a code owner automatically assigned from nextcloud/server-backend

@nfebe nfebe Awaiting requested review from nfebe nfebe is a code owner automatically assigned from nextcloud/server-backend

Assignees

No one assigned

Labels

3. to review Waiting for reviews bug

Projects

None yet

Milestone

Nextcloud 27

Development

Successfully merging this pull request may close these issues.

[Bug]: Single letter avatars in Nextcloud 24 have become all white icons in Nextcloud 25 on Alpine Linux with imagemagick installed

4 participants

@provokateurin @szaimen @DaTweety @bigcat88