pal-e-docs

Review: Landing page, public registration, and email verification flow

review-1927-2026-07-26 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
All 12 required sections for the Feature template are present and well-written. The Context section is thorough with source references.

Traceability

  • [x] story:account-email label -- "Receive password reset and email verification emails via Postmark SMTP"
  • [x] story note verified -- found in project-landscaping-assistant user-stories section
  • [x] arch:rails label -- Rails framework
  • [x] arch note verified -- arch-rails note exists in pal-e-docs (shared across projects, ISS-specific but describes common Rails patterns)
  • [x] arch:keycloak label -- Keycloak OIDC
  • [x] arch note verified -- arch-keycloak note exists in pal-e-docs (shared infra component, pal-enterprises project)
  • [x] Forgejo issue -- ldraney/landscaping-assistant#324, open

File Targets

  • [x] app/controllers/landing_controller.rb -- new file, does not exist yet (correct for new feature)
  • [x] app/views/landing/index.html.erb -- new file, does not exist yet (correct for new feature)
  • [x] config/routes.rb -- verified: exists, current root is work_queue_items#index at line 69
  • [x] app/assets/stylesheets/application.css -- verified: exists (59k)
  • [x] keycloak-theme/landscaping/login/resources/css/login.css -- verified: exists, #kc-registration { display: none !important; } confirmed at lines 405-407 with comment at lines 400-403 about Apple Guideline 2.1(a)
  • [ ] pal-e-services/terraform/k3s.tfvars -- ISSUE: scope overlap with #321 (in_progress, same story:account-email). #321 "Configure Postmark SMTP for landscaping Keycloak realm" is already handling this file
  • [x] pal-e-platform/terraform/modules/keycloak/main.tf -- cross-repo target, plausible (theme ConfigMap update)

Repo Placement

Issue is filed on landscaping-assistant (correct for primary work). Three repos identified with separate PRs planned in the Checklist. However, the pal-e-services scope overlaps with #321 which is already in_progress on the same board.

Dependencies

  • #321 (board item #1926) -- "Configure Postmark SMTP for landscaping Keycloak realm" is in_progress with labels type:infra,arch:keycloak,story:account-email,track:devops. This ticket handles the pal-e-services SMTP config AND pal-e-platform Postmark provisioning. It is a direct prerequisite for #324 but is NOT referenced anywhere in the issue body, Lineage, or Related sections.
  • #290 (done) -- hid the Keycloak registration link for App Store 2.1(a) compliance. #324 explicitly undoes that work. Referenced in Lineage.
  • #282 (multi-tenancy, done) -- referenced in Lineage. No active dependency.
  • #309, #310 (next_up, SaaS sprint 2) -- no direct dependency, but both depend on Keycloak registration working. No conflict.

Acceptance Criteria

10 acceptance criteria total. Testability assessment:
  • [x] ACs 1, 2, 3, 8, 10 -- testable via Rails request specs (covered by Test Expectations section)
  • [ ] ACs 4-7, 9 -- require Keycloak + Postmark integration testing. No test coverage specified for these. These are infrastructure-level validations that depend on #321 completing first. The Test Expectations section only covers 4 of the 10 ACs.

Blast Radius

CRITICAL: The ticket says "Keycloak-side changes (verify_email, SMTP, registration link unhide) are infrastructure config and do not need an app-level flag." But removing #kc-registration { display: none !important; } from login.css (line 405-407) is a change in the landscaping-assistant repo's Keycloak theme, not infrastructure config. This CSS change is NOT gated by the public_landing feature flag.
Impact: When the CSS hide is removed but public_landing flag is off, iOS app users will see the Register link on the Keycloak login page. This is the exact scenario that #290 was created to prevent (Apple Guideline 2.1(a): apps that link to external registration mechanisms). Undoing it without a gate risks another App Store rejection.
The comment in login.css (lines 400-403) itself says: "Re-enable by removing this block when client self-registration is ready for production." This suggests the unhide should be a deliberate, coordinated step -- not an ungated change.
The business_codes flag (#312, done) already processes registration data in the sessions controller, confirming that Keycloak registration IS configured. But the register link was deliberately hidden from the login page UI.

Decomposition Assessment

Even after accounting for #321 handling the pal-e-services scope:
  • 5 file targets in primary repo + 1 in pal-e-platform = 6 files across 2 repos (>3 files across >2 repos -- note: threshold is "across >2 repos" and this is on the edge)
  • 10 acceptance criteria (>5 threshold)
  • Estimated agent work: >5 minutes (landing page design, route changes, CSS, tests, cross-repo ConfigMap)
NEEDS DECOMPOSITION -- route to skill-decompose-ticket. Suggested split:
  • Rails landing page + feature flag -- LandingController, view, routes.rb conditional root, application.css, feature flag registration, request specs
  • Keycloak registration link unhide -- login.css change, pal-e-platform theme ConfigMap update, flag-gating decision for the CSS change
  • Integration verification -- ACs 4-7 and 9, end-to-end registration flow validation (depends on #321 completing)

Recommendation

  • [BODY] Remove pal-e-services file targets -- #321 (in_progress) already handles k3s.tfvars SMTP config. Remove the "pal-e-services PR" Checklist item. Add #321 as explicit prerequisite in Lineage section.
  • [BODY] Address blast radius: document how the registration link unhide (login.css line 405-407) interacts with App Store Guideline 2.1(a). Either (a) gate the CSS unhide behind the public_landing flag via a separate theme file, (b) coordinate the CSS change to deploy only when the flag is enabled in production, or (c) document why it is now safe (e.g., App Store no longer enforces 2.1(a) for this case).
  • [BODY] Clarify pal-e-platform scope boundary with #321 (Postmark provisioning vs theme ConfigMap are in the same repo but different concerns).
  • [DECOMPOSE] 6 files across 2 repos, 10 ACs -- exceeds 5-minute rule. Route to skill-decompose-ticket.