Repository navigation
Choosing a group picture replaced your own avatar (GRYT-1182) - #185
Merged
Merged
Conversation
Group pictures went through POST /api/uploads/avatar, which always writes
the uploader's avatar_file_id. So picking a picture for a group also made
it your avatar on that server.
There's a separate POST /api/uploads/group-icon now. It runs the same
pipeline as the avatar route (size limit, SVG sanitising, sharp, the
animated resize) but never touches the user row, and it answers
{ fileId, processing }. It's gated on send_direct_messages, which is what
dm:group:create and dm:group:update check.
The media sweep only kept files that a message or a user avatar pointed
at. Group pictures got away with that because they were also somebody's
avatar. Once they aren't, the sweep deletes them 30 minutes after upload,
so it keeps anything a group wears as its icon too.
Co-Authored-By: Claude Opus 5 <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review-required path: this touches
src/db/sqlite/conversations.ts. It's one new read-only query, used by the media sweep.Vikunja: GRYT-1182. Client: Gryt-chat/client#561. Mobile: Gryt-chat/mobile#214. Docs: Gryt-chat/docs#108.
What was wrong
The client and the mobile app both uploaded a group picture through
POST /api/uploads/avatar. That route always sets the uploader'savatar_file_id, so choosing a picture for a group changed your own avatar on that server too. I reproduced it on main with a throwaway server and three guests. After picking the picture, the uploader'susers.avatar_file_idwas the group icon's file id.What changed
POST /api/uploads/group-iconruns the same code as the avatar route. That handler moved intostoreAvatarImage(purpose), so it's the same size limit, SVG sanitising, sharp re-encode and background resize for big animated files. The group path doesn't write the user row, and it replies{ fileId, processing }.send_direct_messages, which is whatdm:group:createanddm:group:updatealready check. The avatar route stays onupload_avatar_image.uploaded_by_server_user_id(A file token read any file, private channels and DMs included (GRYT-921) #183). So the uploader can read it anddm:group:updateaccepts it. The check from Older files nobody is recorded as uploading stay readable (GRYT-921) #184 is untouched.unreferencedAmongnow keep files a group uses as its icon.Please look at
send_direct_messagesto match the group handlers. If group pictures should also needupload_avatar_image, that's a one-line change.uploads.tsis mostly the handler moving into a function.git diff -wshows the real change.Tests
groupIconUpload.test.tsgoes through the real router with filesystem storage. Raster and SVG uploads leave the avatar alone, a non-image is refused, the avatar route still sets the avatar, and the sweep keeps the file once a group wears it. I mutation-checked it: putting the avatar write back fails three tests, and dropping the sweep change fails one.uploadAuthOrder.test.tslists the new route.uploadStorage.test.tsread the attachment route's source up to the nextuploadsRouter.post(, which is now past the moved handler. It stops at the route's own closing line instead.yarn test,yarn test:examples,yarn build, eslint and the comment check pass locally.The client needs a server release with this route before group pictures work again. It won't fall back to the avatar route. Details are in the client PR.
Left for later
Webhook avatars look like they have the same sweep gap, and the webhook settings tab reads
file_idfrom an upload reply that sendsfileId. I found that by reading the code and haven't checked it. It's GRYT-1183.🤖 Generated with Claude Code