Replace the fixed group colour palette with a free hex picker #136
Loading…
Reference in a new issue
No description provided.
Delete branch "task-195-colour-picker"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Closes task 195.
GroupColour(Domain and Contracts) andGroupColourPaletteare gone; a group's colour is a free six-digit hex string end to end, validated by[RegularExpression]on the contracts and again, independently, byGroupRepository.NormaliseColourbefore it is written (case-normalised to lower-case there too).GroupFormreplaces the<select>with a native colour picker plus a synced hex text input; a new group's form pre-selects at random from ADR-0098's own six values rather than uniform-over-all-hex, to avoid a near-white or near-invisible marker.0004-free-group-colour.sqlmigrates every existing row's category value to its exact current hex (so no group visibly changes colour on deploy) and replacesck_groups_colour's closed value list with a shape check. ItsCASE(noELSE, so an unmapped value fails theNOT NULLconstraint rather than silently defaulting) was mutation-tested by hand — reproduced during development and again independently in review — but no regression test is committed for it, deliberately:0004-free-group-colour.sqlis a DbUp-journalled script (ADR-0021), applied at most once per database and then permanently skipped, and immutable once merged — a later fix can only be a new script, never an edit to this one, so there is no future change to this file a test could ever catch a regression in. The unmapped-value case the guard defends against is also unreachable in practice:0003-group-colour.sql's own priorck_groups_colouralready refused any row outside the six categories before0004ever runs. The repository guard and the contract's[RegularExpression], by contrast, run on every future write and are both backed by committed tests, mutation-tested the same way.ADR-0117 partially supersedes ADR-0098 — the palette and its distinguishability guarantee are what's displaced; the schema-shape convention and the
MapMarker/map.jsinterop design survive unchanged.Considered and set aside rather than built: quick-pick swatches from the six ADR-0098 values alongside the free picker. The ticket's own ask is a free picker; raised to the coordinator rather than shipped unannounced.
1795 total / 1785 passing / 10 skipped (4 WebUI repro tests, 6 E2E — no browser in this environment).
dotnet format --verify-no-changesclean.Verdict: mergeable
Reviewed at
56f3536(current head — d1b40bc'sbuildjob genuinely failed, confirmed CI-side and reproduced nothing locally that explained it;56f3536fixes it, and CI is now green includinge2e, both jobs, confirmed directly against the Actions API).The CI failure, since it's worth recording: not a content defect.
NormaliseColour's original[GeneratedRegex]partial method emits its own synthetic matcher class carrying coverable lines/branches the four malformed-input cases didn't fully exercise, droppingPlaceMark.Infrastructurebelow its ADR-0080 coverage ratchet — adotnet build/dotnet testlocally can't see this, since ratchet enforcement is a separate CI step over merged coverage XML.56f3536swaps it for a plain compiledRegexinstance, documented as such, with the same pattern and behaviour — I rebuilt, reformatted and re-ran the full suite against this exact head and got the identical 1795/1785/10 the original commit reported.Migration — reproduced against a real database, not read. Seeded a fresh Postgres through scripts 0001–0003 with one row per legacy category value, ran 0004 for real: all six landed on the exact hex
GroupColourPalette.HexForused to produce (#08306b,#8e0000,#e65100,#ffca28,#ba68c8,#c2185b), byte for byte. Then reproduced the mutation test itself — seeded a row holding an unmapped value and ran 0004 against it: fails on theNOT NULLviolation exactly as claimed, not a silent default. One gap: this exact verification isn't preserved as a committed, automated test — only the journal-order tests (SchemaUpgraderTests/SchemaUpgradeCommandTests) reference 0004. Not a blocker (the script is frozen post-merge per ADR-0021, so the risk window is now), but worth naming rather than letting the PR body's "mutation-tested" read as "there's a regression test for this."Validation — drove the exact malformed values myself.
red,#GGGGGG,#abc, missing#, 8-digit all refused;#C2185B(uppercase) accepted, againstGroupColourFormat.Patterndirectly. Contract pattern accepts either case; repository lower-cases first and only then checks its own lower-case-only pattern, so the two can't disagree — a repository-only caller gets the identical refusal an HTTP caller already does.OpenAPI/data-model — both checked, both accurate.
OpenApiSnapshotTestsis a genuine live-fetch-vs-committed comparison, not tautological, and passed in my own run.docs/data-model.md's default,CHECKtext, ERD annotation and provenance paragraph all match the schema and the migration exactly, checked by hand since PR #134 established there's no mechanical guard for that file.ADR-0117 states the random-from-six narrowing in its own named paragraph, and gives up ADR-0098's distinguishability guarantee in plain language ("not a caveat on it") rather than softening it. Supersession metadata is reciprocal and correct in both files, and
AdrIndexTests' cross-record check (120/120) confirms it mechanically.Count: 1795/1785/10, re-derived independently, matches on both the original and fixed head.
CreateGroupRequestTests/UpdateGroupRequestTestsdiffs show enum-literal swaps plus new hex-validation cases added, not removed — no coverage lost in translation.