Review: Catalog model, admin CRUD, and browsing UI

review-1791-2026-07-05 Review

review needs-refinement

Verdict: NEEDS_REFINEMENT

Template Completeness

  • [x] Type — Feature
  • [x] Lineage — Depends on #10, #6; Sprint 4
  • [x] Repo — ldraney/intelligentstaffingsystems
  • [x] User Story — present (lead/client browsing portfolio)
  • [x] Context — present (CatalogEntry model, pillar enum, ActiveStorage)
  • [x] File Targets — 7 files to create
  • [x] Feature Flag — none
  • [x] Acceptance Criteria — 8 items
  • [x] Test Expectations — 3 items + run command
  • [x] Constraints — present
  • [x] Checklist — present
  • [x] Related — present
All template sections present. Template is complete.

Traceability

  • [x] story:catalog label — Epic 3 (US-3.1, US-3.2): Browse portfolio by three pillars; admin CRUD
  • [x] story note verified — found in project-iss user-stories section (catalog row: Epic 3, Lead/Client role)
  • [x] arch:rails label — Rails application component
  • [ ] arch note MISSING — [SCOPE] Create architecture note arch-rails for component rails. Search for "arch-rails" returned no results in pal-e-docs.
  • [x] Forgejo issue — ldraney/intelligentstaffingsystems#11, open

File Targets

  • [x] app/models/catalog_entry.rb — verified: parent app/models/ exists; application_record.rb present as base class; no existing catalog model
  • [x] db/migrate/xxx_create_catalog_entries.rb — verified: db/migrate/ exists with existing migrations (enable_pgcrypto, create_leads)
  • [x] app/controllers/catalog_controller.rb — verified: app/controllers/ exists; application_controller.rb present as base class
  • [x] app/controllers/admin/catalog_controller.rb — verified: parent app/controllers/ exists; admin/ subdirectory does not exist yet (agent must create it — acceptable for first admin controller)
  • [x] app/views/catalog/ — verified: app/views/ exists; new directory needed
  • [x] app/views/admin/catalog/ — verified: app/views/ exists; new directory needed (first admin namespace)
  • [x] app/assets/stylesheets/catalog.css — verified: app/assets/stylesheets/ exists
All file targets are valid for creation. ActiveStorage engine is loaded (config/application.rb), storage.yml configured (local/test), image_processing gem present in Gemfile. No existing catalog code in codebase.

Repo Placement

OK. Issue filed on ldraney/intelligentstaffingsystems, all file targets within same repo, Repo section matches. No cross-repo work needed.

Dependencies

  • #6 Keycloak OIDC auth — in validation column (nearly done). Required for role-gating admin CRUD. Documented in issue Lineage.
  • #10 Bottom tab bar navigation — in backlog column (NOT started). Required for catalog tab to appear in nav. Documented in issue Lineage. This is a real blocker — catalog cannot be accessed without the tab bar.
  • #12 Messaging model — same sprint (4), no direct dependency. Parallel work.
  • #9 Dashboard — sprint 3, no direct dependency.
Dependencies are documented in the issue. #10 being in backlog is noted but not a scope problem — the sprint ordering handles sequencing.

Acceptance Criteria

8 criteria. Assessment:
  • AC 1-6: testable via model, controller, and integration tests. Test expectations align.
  • AC 7 (responsive card layout): harder to verify programmatically; CSS inspection or visual check needed. Acceptable for a feature ticket.
  • AC 8 (docs-in-PR rule): verifiable via PR diff. Standard ISS convention.
Test expectations reference model tests, controller role-matrix tests, and browse-as-lead tests. Run command rails test is valid. Testing-strategy.md confirms: "#11 Catalog: browse + admin CRUD: Model tests; browse across all roles; full role matrix on CRUD."

Blast Radius

Low risk — greenfield feature, no existing catalog code to conflict with. Points of note:
  • ActiveStorage not yet used by any existing model; this will be the first usage. Storage config is set to :local for all environments (including production). MinIO backend for prod mentioned in constraints and security.md but not yet configured in storage.yml — acceptable, as infrastructure config is a separate concern.
  • The admin/ namespace established here sets the pattern for future admin controllers (#14 user management). Consistency matters.
  • Routes additions needed; current routes.rb is clean and straightforward.

Decomposition Assessment

  • File targets: 7 files across 1 repo (does NOT meet the >3 across >2 repos criterion)
  • Acceptance criteria: 8 (EXCEEDS the >5 threshold)
  • Estimated agent work: model + migration + 2 controller namespaces + 6+ view templates + CSS + route additions + 3 test files = likely >5 minutes
NEEDS DECOMPOSITION — route to skill-decompose-ticket. Natural split:
  • CatalogEntry model, migration, pillar enum, published scope, model tests (AC: 6 partial)
  • Public browsing controller/views/routes + browse tests (AC: 1, 2, 3, 6, 7)
  • Admin CRUD controller/views/routes + ActiveStorage image upload + admin tests (AC: 4, 5)

Recommendation

  • [SCOPE] Create architecture note arch-rails for the Rails application component in pal-e-docs. Multiple board items share the arch:rails label (#3, #8, #9, #11, #12, #13, #14, #15, #16); this note should document the Rails application architecture patterns.
  • [DECOMPOSE] 8 AC across 7 file targets with model + 2 controller namespaces + views + CSS + tests. Route to skill-decompose-ticket with the three-way split described above.