Skip to content

feat(board): a board and a seat carry a picture somebody chose - #955

Merged
ExtraToast merged 1 commit into
mainfrom
feat/board-photographs
Sep 1, 2026
Merged

feat(board): a board and a seat carry a picture somebody chose#955
ExtraToast merged 1 commit into
mainfrom
feat/board-photographs

Conversation

@ExtraToast

@ExtraToast ExtraToast commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Slice 3 of #922. A board answers with its group photograph and a seat with its portrait, both uploaded through the file service and served at a ladder of widths, and the twenty-six photographs the repository has always shipped are put onto the records the seed files name. The page that draws them is #930's.

Two new picture kinds

FileType gains BOARD_PHOTO and BOARD_PORTRAIT. Both are publicly readable and both admit the same content types as the existing page pictures.

directory max edge quality widths
BOARD_PHOTO board-photos 2560 82 320, 640, 960, 1280, 1920, 2560
BOARD_PORTRAIT board-portraits 960 85 160, 320, 640

A board photo is a game banner's twin — drawn full-bleed across a band, so it takes the same ceiling and the same ladder, and nothing about it is a new decision.

The portrait's ladder is argued the way ROSTER_ICON argues its cap down from the other icons. A seat shows a thumbnail of about 96px beside the name and opens to a picture of about 320px. So 160 covers the thumbnail, 320 covers it on a dense display and covers the opened picture, and 640 covers the opened picture on a dense display. Nothing on the page draws a portrait wider than that, so the ladder stops at 640 — a portrait carrying a banner's copies would be twenty-one more group photographs for a reader to download.

Two things about the portrait are not a copy of an icon's rules and are worth naming:

  • The ceiling is 960, not 640. maxImageEdge caps the longest edge, and a portrait is taller than it is wide, so 960 there is about 640 across — which is exactly the widest rung. An icon is square, so for GAME_ICON and ROSTER_ICON the ceiling and the widest rung are the same number; for a portrait they cannot be.
  • Quality 85 and a 15 MB limit. 85 rather than a banner's 82 because a face at 160px shows what a group photograph at 1920px hides — the same step ROSTER_ICON takes. 15 MB rather than an icon's 5 because what arrives is a phone camera's output: the ceiling is applied after the file is admitted rather than instead of admitting it.

What I found about the unique picture_id constraint

boards and board_members each carry a unique key on (picture_id, deleted_at)uk_boards_picture_deleted_at and uk_board_members_picture_deleted_at, both from V28. Storage is content-addressed, so two rows naming one art file resolve to one File row that only one of them can hold, and the second insert violates the key. Esports sidesteps this because a team's banner belongs to the fielding; a board's photograph has no such escape.

No two rows name the same art today. All twenty-six names are distinct, and so are all twenty-six files' SHA-256 digests, checked rather than assumed — so nothing could collide even by accident of identical bytes. Two things now hold that:

  • ShippedBoardArtFilesTest fails the build if a name is ever used twice, alongside the two checks its esports twin makes (a row naming art nobody committed, a committed file no row names). That is the real guard: the collision becomes a build failure rather than a start-up log line.
  • At run time the loader refuses rather than letting the key answer. Before it writes, ShippedBoardArt asks whether that stored picture is already held by another board or seat; if it is, it logs which one and leaves it there. A determinate winner and a sentence, instead of a constraint violation surfacing half way through a start. ShippedBoardArtIT asserts it by handing board 9's photograph to board 8 and running the loader again: board 8 keeps it, board 9 stays without one, nothing throws.

One consequence worth recording for #930 and #935, which I have not changed: because the key is on picture_id, one File row backs at most one board. If a person uploads byte-identical bytes for two different boards, storage hands both saves the same row and the second save fails on the key rather than being refused in words. That is the constraint working as written — a picture belongs to one board — but the answer a person gets is not a good one. It is not reachable through the shipped art and it is not what this ticket asks for, so it is left alone and named here.

The art

Twenty-six WebP masters under services/api/src/main/resources/db/seed/boards/art/, 2.1 MB in total, with a CREDITS.md beside them the way the esports art has one. Five board photographs (boards 5 to 9) and twenty-one portraits.

Named flat, lowercase and hyphenated, and the name is derived from the CSV cell with no table in between. A row's photo or portrait cell is the file's name with .webp for an extension — board6 and board6-amber — so there is nothing that can fall out of step with the files. I considered the nested rule (art/board9/board9.webp, taken from the row's image value) and kept the flat one: it derives the filename from a cell just as directly, it does not tie the art's layout to a column #935 deletes, and it matches the esports house style. board7-reini is board7/reinout.jpg, named for the seat rather than for the asset; CREDITS.md records that.

Board 6's portraits are cut from the *BIG.jpg originals, not the small copies beside them — 3283x4924 up to 3803x5705 rather than about 1100x1700. A portrait is enlarged when a seat opens, so resolution thrown away here cannot come back; those six are the only portraits in the repository with a larger copy available, and they are the only six that fill all three rungs. I checked the framing matches before switching (RMSE under 0.2% against the small copies at a common size, which is resampling noise).

Sizes: photographs fitted inside 2560x1440 at quality 82, portraits to 960 on the longest edge at quality 85, resized with ImageMagick's Lanczos filter where a resize was needed and encoded with cwebp. Nothing is upscaled — three photographs and fifteen portraits keep their originals' own size, which is why fifteen portraits' ladders stop at 320. CREDITS.md has the table and the rights note: these are the association's own photographs of its own boards, so none of the publisher-art caveats in db/seed/esports/art apply.

Nothing is deleted from src/assets/. That is #935's, and the image columns must keep working until then.

The seed files, and the checksum

boards.csv gains a photo column and seats.csv a portrait column, each naming an art file without its extension — exactly the way teams.csv names a banner. Five and twenty-one cells are filled; the rest are blank, because boards 1 to 4 were never photographed and board 10 has not sat.

The rows were rewritten through a real CSV reader rather than by appending to lines, because one blurb (Taha's) carries a newline inside a quoted field and is two physical lines. A no-op round trip through that reader is byte-identical to the committed file, so the only change in either file is a trailing column.

R__Boards_seed does not store pictures and is unchanged except for its header. Storing a picture needs the storage volume and the converter, which a migration runner has neither of, so the two new columns are read by ShippedBoardArt once the application is up. Its counters are untouched, so the CodeQL java/constant-comparison fix in flight elsewhere does not conflict.

Editing the files moves the migration's checksum, and that is correct. getChecksum() hashes the two files' contents, which is what makes an edit take effect at all. Flyway re-runs the repeatable seed, the seed is idempotent — the update statements carry a NOT (a <=> ? AND …) guard — and a row that already agrees is not even written. BoardSeedLoadIT's "a second run changes nothing" case still passes.

ShippedBoardArt

A @Component in the board module, applied on ApplicationReadyEvent by ShippedBoardArtOnStartup so the work goes through the proxy. Every rule is the esports loader's, and every one is asserted:

  • Every shipped picture is stored on every start, whether or not anything is waiting for it, so a lost storage volume repairs itself and an address served for a year answers again. Asserted by deleting a stored master's bytes and running the loader again — for a photograph and for a portrait separately, because they are stored by separate steps and a volume that repairs half of itself leaves the page half drawn.
  • A picture already chosen is never replaced. Only an empty slot is written. Asserted for both kinds against an uploaded picture, and by a second run reporting nothing applied.
  • Credited to the site's own system account, because nobody chose them. A missing account is a warning and no work rather than a failed start.
  • A failure is reported and swallowed. Per record in attempt, and again in the startup bean, so art that did not land never stops the application coming up.

Responses and requests

BoardResponse.photo and BoardMemberResponse.portrait are the same Image shape the esports payloads already carry — address, path, own dimensions and stored widths — mapped through the existing File.asImage() rather than a second mapper. So the frontend's srcsetOf/sizeOf in components/island/pictures.ts work on them unchanged.

Create and update take a stored picture path, resolved through StoredPictures, so a path naming nothing stored is refused with 400 PictureNotStored and a path naming a picture of the wrong kind is refused the same way — a photograph field will not take a portrait. Both are asserted.

pictureId is gone from the board payloads, replaced by photo. It was a raw files row id, no client ever sent it, and leaving two request fields writing one column would have been two ways to set a picture that could disagree. photo carries the same fact plus the address and the widths.

image deliberately survives, on both the board and the seat, and answers alongside the new pictures. This is the expand half of an expand–contract: /board still draws the asset file names and #935 drops the columns. BoardControllerIT asserts a board answering with image and photo in the same payload, so a future change that quietly drops one fails.

Board.vue and BoardManager.vue are untouched — they read image, which still answers.

Permissions are unchanged. The upload endpoint is hasPermission('__NO_TARGET__', 'Team', 'write'), which is BOARD and above, and BoardPermission grants write to exactly the same people, so nothing needed widening for a board member to upload a photograph.

Generated client

Regenerated the way CI's Validate OpenAPI client generation job does — ./gradlew :services:api:dumpOpenApiSpec, normalize_api_spec, yarn gen:blueshell, yarn lint:gen. The diff is services/api/openapi.json plus types.gen.ts, and every changed line in types.gen.ts is a board type or one of the two new FileType values. Nothing else moved.

Tests

Exit codes checked rather than output grepped. The integration suite ran against a throwaway MariaDB 10.11.10 container on port 3315 (and again on 3316 after the art was recut) with blueshell-test created from services/api/0_init.sql and CI's own MYSQL_* env, cwebp and ImageMagick present.

command result
./gradlew :services:api:compileKotlin :services:api:compileTestKotlin :services:api:compileIntegrationTestKotlin exit 0
./gradlew :tests:system:compileTestKotlin exit 0
./gradlew :services:api:test exit 0 — 851 tests
./gradlew :services:api:integrationTest --tests "net.blueshell.api.board.*" exit 0 — 94 tests
./gradlew :services:api:integrationTest (full suite) exit 0 — 1200 tests
yarn typecheck exit 0
yarn lint --max-warnings=0 exit 0
yarn test:unit exit 0 — 130 files, 854 tests
npx playwright test --grep "board" exit 0 — 38 passed, 2 skipped
npx playwright test (whole e2e suite) exit 0 — 611 passed, 15 skipped

New tests:

  • ShippedBoardArtIT — a Spring integration test against the real converter and the real storage volume. A board the file names has a photograph of the right kind; it is stored at the widths its kind lists, stopping at the master's own width; a seat has a portrait at a portrait's widths; a portrait narrower than the ladder is stored at the rungs it can fill; every picture the loader stores has more than one width and none wider than itself; five boards and twenty-one seats get art and the five boards the files do not name get none; a second run changes nothing; deleted bytes are written again at the address they had, for both kinds; a chosen photograph and a chosen portrait are not replaced; and a photograph another board already holds is left there.
  • ShippedBoardArtFilesTest — a pure unit test on the files themselves: every name is on the classpath, every committed file is named by a row, no picture is named twice, and the counts are what they should be so the other three cannot pass against files that name nothing.
  • BoardControllerIT — extended: a photograph round-tripping through create, update and a public read with its size and its widths, beside image; a portrait round-tripping through add, update and read; a path naming nothing stored refused; and a portrait refused where a photograph is expected.
  • BoardUseCasesTest — the photograph and the portrait taken from a stored path, cleared when none is named, and applied to a seat somebody already holds.
  • BoardArtSystemTest — against the real stack: the api answers with a seeded photograph, a real browser composes a srcset from the published widths, fetches one of the narrow copies rather than the master and decodes it; and the same for a portrait, whose widest copy is 640 rather than a banner's. Driven from the api's own origin, since the board page that will draw these is The board page reads as a timeline #930's and nothing here should depend on a CORS rule.

What I could not verify

yarn test:e2e fails wholesale on this machine, on clean main as much as on this branch. Every test times out in page.goto, because the page requests https://fonts.googleapis.com/css?family=Roboto… and this machine's DNS cannot resolve it, so the load event never fires. I proved it is not this branch by reverting the two frontend files, rebuilding and watching the same test fail, then found the pending request with a probe script. The numbers above come from npx playwright test with --host-resolver-rules=MAP fonts.googleapis.com ~NOTFOUND in a throwaway config, which is not committed; with the request failing fast instead of hanging, the whole suite is green. CI has network access and is unaffected.

Closes #929


Diff breakdown added removed, scaled to the largest row.

api                                              +1210   -100   48
  production         ██████████████████████░░░░   +518    -82   44
  unit tests         ████████░                    +201    -18    2
  integration tests  █████████████████████        +491     -0    2
  generated          █░                             +1     -1    1  ~

frontend                                           +64     -6    1
  production         ██░                           +64     -6    1
  generated          █░                            +27     -4    1  ~

system-tests                                      +167     -0    1
  system tests       ███████                      +167     -0    1

repo                                                +3     -2    1
  build & config     █░                             +3     -2    1

──────────────────────────────────────────────────────────────────
production                                        +582    -88
tests                                             +859    -18  1.48 test lines per prod line
total (hand-written)                             +1444   -108  51 files
~ generated (excluded)                             +28     -5  2 files

@ExtraToast ExtraToast added the ready-for-agent Sized and specified for an agent to pick up label Sep 1, 2026
@ExtraToast
ExtraToast force-pushed the feat/board-photographs branch from 49d1534 to 0c176ed Compare September 1, 2026 15:36
@ExtraToast

Copy link
Copy Markdown
Contributor Author

The one red check — System tests (shard 3/6), BoardArtSystemTest > a seeded board photograph is stored, served and drawn, expected: 640 but was: 400 — was the test asserting the wrong number. Nothing is wrong with the stored art.

What the number was

The last assertion was assertThat(photo.naturalWidth).isEqualTo(photo.chosenWidth): the decoded width against the width named in the copy the browser fetched.

naturalWidth is not that. The html spec has it return the density-corrected intrinsic width, and under a srcset of w descriptors the current pixel density is the chosen descriptor divided by the sizes value. The helper sets sizes = 400px, the browser picks the 640 rung, so the density is 640 / 400 = 1.6 and naturalWidth reports 640 / 1.6 = 400. The 400 in the failure message is the sizes value, and under a w srcset it always will be — the assertion could only ever have passed by accident, on a picture whose chosen rung happened to be 400 wide.

Checked rather than reasoned about, in the same headless chromium the suite drives:

chosen naturalWidth same bytes on an img with no srcset
dpr 1, sizes: 400px -640.webp 400 640
dpr 2, sizes: 400px -960.webp 400 960

Why the art is exonerated by the failure itself

Because the correction divides by a density taken from the descriptor, not from the bytes, naturalWidth under this srcset is actualPixelWidth / 1.6. A ladder whose 640 rung is served 320-wide bytes reports 200, which I confirmed in the same probe. So 400 is reachable only if the bytes behind -640.webp really are 640 pixels wide. The renditions are right, ImageRenditionWriter is right, nothing is cross-wired and no two boards share a File row.

The change

Only BoardArtSystemTest.kt. The helper now reads the fetched copys own width off a second img carrying no srcset — where nothing is density-corrected — and reports it as fetchedWidth:

assertThat(photo.fetchedWidth)
    .describedAs("the fetched copys own pixel width, against the width its name claims")
    .isEqualTo(photo.chosenWidth)

The property being proved is unchanged and is now proved directly rather than through a number that means something else: a band asking for 400 pixels is sent a narrow copy, and the bytes behind that address are as wide as its name says. The two assertions that carry the saving — chosen matches -\d+\.webp and chosenWidth < candidates.max() — are untouched, so the master is still never accepted.

Verification

.api.env and .db.env are gitignored and so are absent from a fresh worktree, and the dev compose pins container_name: db and ports 8080/3307, so I could not stand the branchs stack up beside the one already running — I have not run the full suite locally and am not claiming a local pass. What I did verify: the evaluate block extracted verbatim from the Kotlin source, run in chromium at the base classs 1600x900 and dpr 1 against a stub /boards with real webp ladders, returns fetchedWidth 640 against chosenWidth 640 for the photograph and 640/640 for the portrait, with every other assertion in both tests holding. :tests:system:compileTestKotlin exits 0. Everything else in that shard, including the sibling portrait test, was already green — line 59 was the only failure.

@ExtraToast
ExtraToast force-pushed the feat/board-photographs branch from 0c176ed to ba44142 Compare September 1, 2026 15:38
The system test's last assertion compared `img.naturalWidth` against the
width named in the copy the browser fetched, and those are not the same
number. Under a `srcset` of `w` descriptors the html spec has a browser
divide a picture's intrinsic width by its current pixel density, and that
density is the chosen descriptor over the `sizes` value — so a 640 copy
asked for at 400px reports 400, and that number can only ever be the
`sizes` value. Nothing is wrong with the stored art: 400 is 640 / 1.6,
which is reachable only if the bytes behind `-640.webp` really are 640
pixels wide. Confirmed in chromium, where a ladder whose 640 rung is
served 320-wide bytes reports 200 rather than 400.

So the test reads the fetched copy's own width off a second `img` carrying
no `srcset`, where nothing is density-corrected, and asserts that against
the width its address claims. What is being proved is unchanged and is now
proved directly: a band asking for 400 pixels is sent a narrow copy, and
the bytes behind that address are as wide as its name says.
@ExtraToast
ExtraToast force-pushed the feat/board-photographs branch from ba44142 to b889703 Compare September 1, 2026 15:46
@ExtraToast

Copy link
Copy Markdown
Contributor Author

Rebased onto main after #953, #954 and #951 merged.

One conflict, in the boards adapter's import block: main had gained reasonFor from the refusal in #951, this branch had added Image and PictureStore. Both sides were genuinely in use — reasonFor in dropBoard, PictureStore in storeBoardPhoto/storeSeatPortrait — so the resolution keeps all three imports. The Kotlin files auto-merged, including R__Boards_seed.kt against #954's counter refactor and BoardController/BoardUseCases against #951's refusal.

Verified after the rebase, exit codes not grepped output:

yarn typecheck 0
yarn lint --max-warnings=0 0
yarn test:unit 0 — 956 passed, 133 files
:services:api:compileKotlin :compileTestKotlin :compileIntegrationTestKotlin 0 — zero e: lines
:services:api:test 0

The integration and system suites are CI's to run, since they need a real database and the compose stack.

@ExtraToast
ExtraToast merged commit 47bf7e3 into main Sep 1, 2026
25 checks passed
@github-actions github-actions Bot mentioned this pull request Sep 1, 2026
@ExtraToast ExtraToast mentioned this pull request Sep 1, 2026
15 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-agent Sized and specified for an agent to pick up

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A board and a seat carry an uploaded photograph

1 participant