Skip to content

Add no-match reason tags + post-vote follow-up strips to printing-tag queue - #12

Merged
WilfordGrimley merged 3 commits into
masterfrom
worktree-tag-taxonomy-followup
Jul 14, 2026
Merged

Add no-match reason tags + post-vote follow-up strips to printing-tag queue#12
WilfordGrimley merged 3 commits into
masterfrom
worktree-tag-taxonomy-followup

Conversation

@WilfordGrimley

Copy link
Copy Markdown

Summary

  • Seeds a new reason-code Tag taxonomy (custom-art, altered-frame, upscaled, ai-art, no-collector-line, non-english) via a management command, not a data migration — see docs/features/printing-tags.md Stage 4 for why the migration approach broke 5 unrelated tests and was reverted.
  • Adds two skippable follow-up strips to PrintingTagQueue.tsx: PrintingConfirmStrip (after a resolved vote, pre-filled from fullArt/isBorderless) and NoMatchReasonStrip (after an explicit no-match vote), styled with the queue's existing blue candidate-card look.
  • Adds isBorderless to PrintingCandidate (was missing; needed for the confirm strip's pre-fill).

Deviation from spec (flagging explicitly)

Spec said "data migration" for the tag seeding. First pass did exactly that; running the full backend suite surfaced 5 new failures (test_views.py::TestGetTags::*, test_tag_votes.py::TestPostTagConsensus::test_returns_an_entry_for_every_seeded_tag) because those tests assert a fresh DB has zero real Tag rows, and a migration seeds unconditionally at DB-setup time (including the test DB). Switched to a management command, mirroring the existing seed_default_tags pattern exactly (which is deliberately not migration-driven for the same reason). Full details + evidence in docs/features/printing-tags.md Stage 4.

Activation step required post-merge

manage.py seed_no_match_reason_tags must be run once after this deploys, or NoMatchReasonStrip votes 400 (Tag.objects.get misses raise BadRequestException, not a silent no-op). Confirmed live: Full Art/Borderless (used by the confirm strip) already exist in production, so that strip needs no activation step.

Test plan

  • Backend: full suite via testcontainers — 452 passed, 4 pre-existing unrelated failures (2 moxfield network, 2 test_sources.py path issues)
  • Frontend: tsc, prettier, eslint (0 errors), npm run build, Jest (123/123; comparison.test.ts skipped — pre-existing Node 20 vs required Node 22+ gap, unrelated to this PR)
  • Full Playwright suite: 159/159 passed, including new specs for both strips
  • Screenshots of both strip variants sent to reviewer

Holding as draft for review, per usual.

… queue

Seeds a new reason-code Tag taxonomy (custom-art, altered-frame, upscaled,
ai-art, no-collector-line, non-english) via a management command rather
than a data migration, and adds PrintingConfirmStrip/NoMatchReasonStrip
follow-ups after a printing vote, styled with the queue's existing blue
candidate-card visual language. Adds isBorderless to PrintingCandidate.

See docs/features/printing-tags.md Stage 4 for the migration-vs-command
rationale (backend suite evidence) and the seed_no_match_reason_tags
activation step required post-deploy.
WilfordGrimley and others added 2 commits July 14, 2026 08:24
Reuses the existing, already-cached useGetTagsQuery hook (no new
endpoint/fetch) to filter NoMatchReasonStrip down to tags that actually
exist, so an instance that hasn't run seed_no_match_reason_tags yet
degrades gracefully instead of 400ing on every tap.
@WilfordGrimley
WilfordGrimley marked this pull request as ready for review July 14, 2026 09:26
@WilfordGrimley
WilfordGrimley merged commit 3424e48 into master Jul 14, 2026
6 of 7 checks passed
@WilfordGrimley
WilfordGrimley deleted the worktree-tag-taxonomy-followup branch July 14, 2026 09:26
WilfordGrimley added a commit that referenced this pull request Jul 14, 2026
…#13)

Stage 4 (no-match reason tags + follow-up strips) is live: seed command
run, both strip variants proven end-to-end against the real production
API and cleaned up afterward. Also documents an operational gap hit
during activation: docker compose up -d django recreates the container
with a new internal IP, but nginx's upstream resolves the django
hostname once at its own startup and doesn't pick up the change --
causes a full 502 until nginx itself is restarted too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant