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

Fix generated avatars cache #38143

Merged
merged 2 commits into from
May 16, 2023
Merged

Fix generated avatars cache #38143

merged 2 commits into from
May 16, 2023

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

/backport to stable26

@provokateurin
Copy link
Member Author

/backport to stable25

@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

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

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

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

@szaimen szaimen szaimen approved these changes

@bigcat88 bigcat88 bigcat88 approved these changes

@skjnldsv skjnldsv Awaiting requested review from skjnldsv

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

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

Assignees
No one assigned
Labels
3. to review Waiting for reviews bug
Projects
None yet
Milestone
Nextcloud 27
4 participants
@provokateurin @szaimen @DaTweety @bigcat88