Repository navigation
Host team avatars instead of only linking them - #1280
Merged
Merged
Conversation
Replays the team avatar work onto main, which now carries the team images feature from #1272. That landed as a squash, so the avatar branch's own copy of team images conflicted with the shipped version rather than merging with it; only the avatar half is unique and is what is replayed here. The avatar migration moves from 124 to 125, since main's 124 is the team owner role and challenge ownership. It only references groups and users, so it transplants unchanged.
The avatar endpoints were written before the team images feature landed and ended up restating pieces of it. Where the two now sit side by side, the avatar side defers to what is already there. The repository drops its hand-written row parser for the macro parser the query's aliases already suit, and its inline copy of TeamImageRepository's dataParser for the shared one. TeamAvatar's aliases for the format and size rules are gone; callers read them off TeamImage, which is where the single rule lives. The multipart parser bound moves to TeamImage.MAX_UPLOAD_BYTES. The image endpoint had reasoned its way to MAX_SIZE_BYTES * 2 and written down why; the avatar endpoint had no bound at all and fell back to the 15MB disk buffer, spooling an oversized upload in full before rejecting it for exceeding 2MB. Serving an avatar now builds the ETag from the metadata row and only reads the bytes on a cache miss, so revalidating no longer pulls up to 2MB out of Postgres to answer 304. That query existed already and was simply never wired up. Deleting an avatar commits the delete and the url clear together, as uploading already did, and the serving path is built once rather than spelled out in both urlFor and isStoredAvatarUrl.
Four things that were wrong with the avatar endpoints, all of them visible only once the team images feature landed beside them. updateTeam announced itself over the websocket even when running inside a caller's transaction, so a rollback broadcast an update that never committed. Old callers always committed immediately, but both avatar paths now wrap it, which makes the bug reachable. The broadcast is now the caller's to make once its transaction has committed. Reading an upload and writing image bytes back out were spelled out twice, once per controller, down to three identical error strings and the ETag and cache headers. They move to a mixin both controllers use. Duplicating the two security headers is the part worth avoiding: a change made in one place and missed in the other fails silently. The avatar rules - check ordering, the transaction, which url is ours - lived in Action bodies where nothing else could reach them and no test could reach them either. They move to TeamAvatarService, matching the route the image endpoints already take through TeamImageService. Removing an avatar decided whether the stored url was ours by matching a route prefix, which the frontend then had to reimplement. It now reconstructs the url from the stored row and compares exactly, so a team that has since typed their own url keeps it. Serving an avatar is also cacheable indefinitely rather than for a day, since the url carries the upload timestamp and the bytes behind one can never change.
|
CollinBeczak
added a commit
that referenced
this pull request
Sep 15, 2026
Main squash-merged the team images (#1272) and team avatars (#1280) work this branch was sitting on top of, so its own copies of both collided with the shipped versions rather than merging with them. Every one of those conflicts resolves to main's side: the branch carried the pre-review originals, and main has the evolved versions, including the avatar service and shared image mixin that landed with #1280. The challenge reports evolution moves from 125 to 126, since main's 125 is now the team_avatars table. The only genuine merges were the three test registries where both sides had added an entry - the master suite, the helper's tags and TestSpec's service mocks - which take both.
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.
Split from #1268
dependent on: #1272
Replays the team avatar work onto main, which now carries the team images feature from #1272. That landed as a squash, so the avatar branch's own copy of team images conflicted with the shipped version rather than merging with it; only the avatar half is unique and is what is replayed here.
The avatar migration moves from 124 to 125, since main's 124 is the team owner role and challenge ownership. It only references groups and users, so it transplants unchanged.