pal-e-docs

Review: Catalog tab: app portfolio with App Store links

review-1820-2026-07-17-v2 Review

review needs-refinement
Project

Verdict: NEEDS_REFINEMENT

Well-scoped feature ticket with complete template sections, verified file targets, and clear acceptance criteria. Two fixable issues prevent READY status: a missing arch note and a minor factual inaccuracy in the issue body.

Template Completeness

  • [x] Type -- Feature
  • [x] Lineage -- references #11 scope replacement
  • [x] Repo -- ldraney/intelligentstaffingsystems
  • [x] User Story -- well-formed As/I want/So that
  • [x] Context -- clear target audience description
  • [x] File Targets -- 7 targets + 1 exclusion, all verified
  • [x] Feature Flag -- None (appropriate for core tab)
  • [x] Acceptance Criteria -- 4 new + 3 already shipped
  • [x] Test Expectations -- 6 expectations with run command
  • [x] Constraints -- 4 constraints including Turbo Native pattern
  • [x] Checklist -- standard 3-item
  • [x] Related -- project-iss, #11

Traceability

  • [x] story:catalog label -- "Browse portfolio by three pillars; admin CRUD"
  • [x] story note verified -- found in project-iss user-stories section (Epic 3, US-3.1-3.2, Lead/Client role)
  • [x] arch:rails label -- present on board item
  • [ ] arch note MISSING -- search for "arch-rails" returned no results. [SCOPE] Create architecture note arch-rails for the Rails component. Note: arch:rails is used across many ISS board items, so this is a systemic gap, not specific to this ticket.
  • [x] Forgejo issue -- #50, state: open

File Targets

  • [x] app/controllers/catalog_controller.rb -- verified: exists with index + show actions, scoped to published entries
  • [x] app/views/catalog/index.html.erb -- verified: exists with inline card grid (extraction to partial is correctly planned)
  • [x] app/views/catalog/_catalog_entry.html.erb -- does not exist yet (new file). Ticket correctly plans extracting inline cards to this partial.
  • [x] app/views/catalog/show.html.erb -- verified: exists with detail page layout, no app_store_url link yet
  • [x] app/assets/stylesheets/catalog.css -- verified: exists (7.8KB), responsive card layout already in place
  • [x] app/models/catalog_entry.rb -- verified: exists with pillar enum, validations, scopes. No app_store_url field yet.
  • [x] db/migrate/XXXXXX_add_app_store_url_to_catalog_entries.rb -- does not exist yet (new file). Schema confirms catalog_entries has no app_store_url column. Migration correctly scoped.
  • [x] NOT touching app/controllers/admin/catalog_controller.rb -- verified: exists, correctly excluded

Repo Placement

OK. All file targets are in ldraney/intelligentstaffingsystems. Issue is filed on the same repo. No cross-repo concerns.

Dependencies

  • Sprint 4 catalog work (board items #1814, #1815, #1816 -- PRs #38, #39, #41) are all in done. These created the CatalogEntry model, browsing UI, and admin CRUD that this ticket builds on. Prerequisite satisfied.
  • #11 (parent catalog story) is in backlog, marked as decomposed. No blocker.
  • Sprint A items (#58 landing page, #59 registration, #49 navigation) are in validation. Sprint B follows Sprint A by convention, but no direct code-level blockers between this ticket and Sprint A items.
  • No undocumented dependencies found.

Acceptance Criteria

4 new acceptance criteria, all testable by an agent:
  • Portfolio description text -- verifiable by checking view content
  • Target audience communication -- verifiable by checking content
  • Cards with App Store link -- verifiable by checking for app_store_url in card HTML
  • App Store links open externally -- verifiable by checking for data-turbo="false" attribute
3 already-shipped criteria properly marked as done. 6 test expectations are specific with a valid run command.
Minor inaccuracy: AC #4 states "This is the first use of this pattern in the app" for data-turbo="false". However, app/views/dashboard/show.html.erb:11 already uses data: { turbo: false } on the sign-out button_to. The distinction is that the existing use is on a form submission (button_to), while this ticket introduces it on a link for external URL navigation in Turbo Native. The claim is directionally correct but technically inaccurate.

Blast Radius

  • data-turbo="false" for external links: establishes a new pattern for link elements. Only existing use is on the dashboard sign-out button (form submission). If other pages link to external URLs in the future, they should follow this same pattern. Low blast radius.
  • CatalogEntry model change (adding app_store_url): column addition only. Admin catalog CRUD (admin/catalog_controller.rb) is correctly excluded. Admin form would need updating separately to allow setting app_store_url, but that is explicitly out of scope.
  • No shared partials or helpers affected outside the catalog feature.

Decomposition Assessment

7 file targets in 1 repo. 4 acceptance criteria (under 5 threshold). Work scope: add column + model validation, update 2 views (extract partial + add link), update CSS. Estimated agent time: under 5 minutes. No decomposition needed.

Recommendation

  • [SCOPE] Create architecture note arch-rails for the Rails component. This is a systemic gap -- arch:rails is used across many ISS board items. Could be addressed once for the whole project rather than per-ticket.
  • [BODY] Fix claim in AC #4: "This is the first use of this pattern in the app" -- dashboard sign-out already uses data: { turbo: false }. Clarify to: "This is the first use of data-turbo="false" for external link navigation (existing use is on sign-out form only)."