pal-e-docs

Review: Link groups model and grouped card containers

review-1407-2026-06-09 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type
  • [x] Lineage
  • [x] Repo
  • [x] User Story
  • [x] Context
  • [x] File Targets
  • [x] Feature Flag
  • [x] Acceptance Criteria
  • [x] Test Expectations
  • [x] Constraints
  • [x] Checklist
  • [x] Related

Traceability

  • [x] story:canvas-layout label — present on board item #1407
  • [ ] story note MISSING — story:canvas-layout is not listed in the project-palinks user-stories section. Current entries: containerize-palinks, sortable-links, auth-roles, custom-domain. [SCOPE] Create user story entry for story:canvas-layout on project-palinks user-stories section.
  • [x] arch:palinks label — present on board item
  • [ ] arch note MISSING — no arch-palinks note found in pal-e-docs. Acknowledged as a board-wide gap (not blocking individual tickets per #36 review precedent). [SCOPE] Create architecture note arch-palinks for component palinks.
  • [x] Forgejo issue — ldraney/palinks#41, state: open

File Targets

  • [x] db/migrate/*_create_link_groups.rb — new file (create). Migration directory exists at db/migrate/. OK.
  • [x] db/migrate/*_add_group_id_to_links.rb — new file (create). OK.
  • [x] app/models/link_group.rb — new file (create). Models directory exists. OK.
  • [x] app/models/link.rb — exists. Currently has no group association. Will need belongs_to :link_group added. OK.
  • [x] app/views/links/_link_group.html.erb — new file (create). Views directory exists. OK.
  • [x] app/views/links/index.html.erb — exists. Currently renders a flat link-grid with sortable controller. Will need canvas-aware rendering. OK.
  • [ ] app/javascript/controllers/canvas_controller.js — ISSUE: does not exist yet. This file is created by issue #40 (canvas view). This ticket cannot add group drag behavior to a controller that doesn't exist. Dependency must land first.
  • [x] app/controllers/link_groups_controller.rb — new file (create). Controllers directory exists. OK.
  • [x] config/routes.rb — exists. Currently has only resources :links. Will need link_groups resource added. OK.
  • [x] app/javascript/controllers/sortable_controller.js — correctly listed as DO NOT TOUCH. Exists. OK.

Repo Placement

OK. Issue filed on ldraney/palinks, all file targets are in the same repo. No cross-repo work needed.

Dependencies

  • #36 (Create clicks table and tracking) — board item #1400, column: todo, state: open. This ticket's group aggregate click tracking depends on the clicks table existing. Not yet implemented.
  • #40 (Canvas view with activity-based card sizing) — board item #1406, column: backlog, state: open. This ticket's canvas rendering and canvas_controller.js modifications depend on the canvas view being implemented first. Not yet implemented.
  • Feature flag system — per docs/feature-flags.md, the feature flag infrastructure is not yet implemented ("Feature flag system pending implementation"). The ticket specifies a link_groups flag gated to superadmin, but there is no FeatureFlag model, no feature_enabled? helper, and no flags in the database. The flag cannot be implemented until the flag system itself exists.
  • Lineage section references wrong issue number — the issue body says "Depends on canvas view (ldraney/palinks #39 or next)" but the actual canvas view issue is #40. This should be corrected.

Acceptance Criteria

7 acceptance criteria. Most are testable by an agent:
  • "Can create, rename, and delete link groups" — testable via integration test
  • "Can add/remove links to/from a group via UI" — testable but requires canvas view to exist
  • "Grouped links render inside a parent container card on the canvas" — requires canvas view (#40) to be done first
  • "Group container size reflects aggregate click activity" — requires clicks table (#36) to be done first
  • "Dragging a group card moves all child link cards with it" — requires canvas_controller.js from #40
  • "Ungrouped links continue to render independently on the canvas" — requires canvas view
  • "A link can belong to at most one group" — testable via model validation
Several AC are untestable until dependencies #36 and #40 land. The AC themselves are well-written and verifiable once prerequisites exist.

Blast Radius

  • Adding link_group_id to links is a schema change that affects the existing Link model. The migration should be reversible.
  • Modifying index.html.erb to support grouped rendering changes the primary view for all users — must be behind the feature flag (which doesn't exist yet).
  • No sibling services or downstream consumers identified. Palinks is self-contained.

Decomposition Assessment

7 acceptance criteria + 4 test expectations across 9 file targets (7 creates + 2 modifications). This exceeds the 5-minute rule on multiple dimensions:
  • >5 acceptance criteria (7)
  • 9 file targets across model, controller, view, migration, JS, and routes layers
  • Estimated agent work: well beyond 5 minutes
However, decomposition should wait until dependencies are resolved. The ticket cannot be meaningfully decomposed when its core prerequisites (#36 clicks table, #40 canvas view, feature flag system) are not yet implemented. Once those land, decompose into: (1) model + migrations + routes, (2) controller + CRUD, (3) view partials + canvas integration.

Recommendations

  • [BODY] Fix Lineage reference: ldraney/palinks #39 → ldraney/palinks #40
  • [SCOPE] Create user story entry for story:canvas-layout on project-palinks user-stories section
  • [SCOPE] Create architecture note arch-palinks (board-wide gap, not unique to this ticket)
  • [SCOPE] Feature flag system must be implemented before this ticket can use the link_groups flag. Either add the flag system as an explicit dependency or remove the Feature Flag section and note it as a follow-up.
  • [DECOMPOSE] 7 AC + 9 file targets exceeds 5-minute rule. Route to skill-decompose-ticket after dependencies land. Suggested split: (1) model/migrations/routes, (2) controller/CRUD, (3) view/canvas integration.