pal-e-docs

Review: Landing page, public registration, and email verification flow (R2)

review-1927-2026-07-26-r2 Review

review approved
Project

Verdict: APPROVED

Re-review of board item #1927 after refinement. Previous review (review-1927-2026-07-26) found 4 issues; all have been addressed in the updated issue body.

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. Context section is exceptionally thorough with Keycloak docs citations.

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. Backing note story-landscaping-assistant-account-email exists with active status.
  • [x] arch:rails label -- Rails framework
  • [x] arch note verified -- arch-rails note exists in pal-e-docs
  • [x] arch:keycloak label -- Keycloak OIDC
  • [x] arch note verified -- arch-keycloak note exists in pal-e-docs
  • [x] Forgejo issue -- ldraney/landscaping-assistant#324, open

File Targets

  • [x] app/controllers/landing_controller.rb -- new file, does not exist yet (correct). Pattern matches existing PagesController which uses skip_before_action :authenticate_user!
  • [x] app/views/landing/index.html.erb -- new file, does not exist yet (correct)
  • [x] config/routes.rb -- verified: exists, current root is work_queue_items#index at line 69. Conditional root based on feature flag is a clean change.
  • [x] app/assets/stylesheets/application.css -- verified: exists
  • [x] keycloak-theme/landscaping/login/resources/css/login.css -- verified: #kc-registration { display: none !important; } confirmed at lines 405-407 with Apple Guideline 2.1(a) comment at lines 396-403
  • [x] pal-e-services/terraform/k3s.tfvars -- conditional on #321 (in_progress). Issue correctly states "if #321 is already merged when this work begins, no pal-e-services changes are needed"
  • [x] pal-e-platform/terraform/modules/keycloak/main.tf -- cross-repo, differentiated from #321: theme ConfigMap update (login.css content) vs #321's Postmark provisioning

Repo Placement

OK. Filed on landscaping-assistant (correct for primary work). Three repos identified with distinct scopes: landscaping-assistant (Rails landing page + login.css), pal-e-services (conditional SMTP config if #321 not merged), pal-e-platform (theme ConfigMap). Each repo's concern is clearly documented in File Targets.

Dependencies

  • #321 (board item #1926, in_progress) -- explicitly documented as dependency in Lineage: "Depends on #321 (Postmark SMTP + verify_email for landscaping Keycloak realm) -- SMTP must be live before the registration link is unhidden." Previous review issue #1: RESOLVED.
  • #290 (done) -- referenced in Lineage. This ticket undoes #290's CSS hide once registration is functional.
  • #282 (done) -- multi-tenancy, referenced in Lineage as unblocking public registration with business codes.

Acceptance Criteria

10 ACs total. ACs 1-3, 8, 10 are Rails-testable and covered by Test Expectations (4 request specs). ACs 4-7, 9 are Keycloak + Postmark integration validations that depend on #321 completing first -- these are infrastructure-level checks verified during validation, not automated tests. This is acceptable given the deploy sequencing model.

Blast Radius

Previous review flagged ungated registration link unhide risking App Store rejection. Now addressed via deploy sequencing: the Lineage section states "SMTP must be live before the registration link is unhidden, otherwise Apple reviewers see a broken registration flow (Guideline 2.1(a))." The Constraints section reinforces: "Registration link CSS change must NOT ship before #321 (SMTP + verify_email) is live." The Feature Flag section correctly separates concerns: public_landing flag gates the Rails landing page, deploy sequencing gates the Keycloak CSS change. Previous review issue #2: RESOLVED.
The skip_before_action :authenticate_user! pattern is established in the codebase (PagesController, ClientErrorsController, SessionsController), so the proposed LandingController follows existing convention. The feature_enabled? helper is used in 10+ controllers -- well-established pattern.

Decomposition Assessment

Technically exceeds 5-minute rule thresholds: 6 files across 2 repos (pal-e-services conditional), 10 ACs. Project owner has explicitly decided to keep this as one ticket. At 8 points, the sizing acknowledges the scope. The work is logically cohesive -- splitting the landing page from the registration link unhide would create artificial sequencing overhead. Owner override accepted.

Previous Review Issues -- Resolution Status

  • Scope overlap with #321 -- RESOLVED. #321 is now an explicit dependency in Lineage with clear conditional language in File Targets.
  • Ungated blast radius -- RESOLVED. Deploy sequencing documented in Lineage, Context, and Constraints. Feature flag correctly scoped to Rails-only changes.
  • Cross-repo scope clarity -- RESOLVED. File Targets differentiate #324's pal-e-platform touch (theme ConfigMap) from #321's (Postmark provisioning).
  • Decomposition -- OVERRIDDEN by project owner decision. Acknowledged at 8 points.

Recommendation

No action needed. Ticket is ready for implementation once #321 (in_progress) is merged.