PR: https://github.com/Goatleads-Team/goat_leads/pull/3259
Author: charliesbananas
Branch: feature/ux-v2 → staging
Created: 2026-03-14 | Last Updated: 2026-03-20
Analysis Date: 2026-03-20
PR #3259 is a massive "big bang" pull request that bundles a complete UX overhaul into a single merge. It touches 594 files across 165 commits, adding 78,875 lines and deleting 30,187 lines. The PR includes 13 database migrations, 10+ new models, 14 new controllers, ~40 new Stimulus controllers, 200 new/modified view templates, and a full CSS architecture restructuring.
Overall Risk Level: HIGH
CI is green (all 8 test shards pass, lint passes, Jest passes, schema checks pass), which is a positive signal. However, the sheer scope of changes creates significant risk in areas that automated tests cannot fully cover: visual regressions, user flow regressions, and migration irreversibility.
| Category | Added | Modified | Removed | Total |
|---|---|---|---|---|
| Files | ~307 | ~257 | ~30 | 594 |
| Lines | 78,875 | — | 30,187 | net +48,688 |
| Commits | — | — | — | 165 |
| Layer | New | Modified | Key Concerns |
|---|---|---|---|
| Migrations | 13 | — | pgvector extension, 6 new tables, 7 column additions |
| Models | 10+ | 9 | New: Organization, SupportTicket, KbArticle, AI models, UserQuickLink |
| Controllers | 14 | 17 | New onboarding, support system, knowledge base, reports, settings |
| Concerns | 1 | 4 | Authentication, RequiresProfileCompletion, LeadOrdersCommon, MfaAuthentication |
| Views (HAML) | ~120 | ~80 | ~200 total view files touched |
| Stimulus (JS) | ~40 | ~15 | New onboarding, support, dashboard, mobile, table controllers |
| Jest Tests | ~40 | ~15 | Good coverage for new Stimulus controllers |
| RSpec | ~15 | ~25 | 40 spec files total; gaps in controller coverage (see below) |
| SCSS | ~25 | ~8 | Full CSS architecture restructure; 12,410 lines removed from _components.scss |
| Helpers | 1 | 6 | ApplicationHelper: +591 lines — significant growth |
| Routes | — | 1 | +77 lines, -11 lines |
13 new migrations:
| Migration | Risk | Notes |
|---|---|---|
create_user_quick_links |
Low | Simple table |
add_index_to_leads_lead_quality |
Medium | Index on partitioned leads table — verify partition-level behavior |
add_contact_window_to_users |
Low | Column addition |
add_branding_to_users |
Low | Column addition |
add_mfa_totp_last_used_at_to_users |
Low | Column addition |
create_support_tickets |
Medium | New table with associations |
create_support_ticket_replies |
Medium | New table with associations |
create_kb_articles |
Medium | New table |
create_kb_article_ratings |
Low | Simple join table |
enable_pgvector_and_create_ai_tables |
HIGH | Enables pgvector extension — requires superuser. Creates 3 AI tables. Extension may not be installed on production server. |
create_organizations |
Medium | New table with hierarchy model |
add_business_address_suite_to_users |
Low | Column addition |
add_ci_unique_index_to_organizations |
Low | Index addition |
Key concerns:
- The pgvector migration is the highest-risk single change. If the
pgvectorextension is not available on the production PostgreSQL server, this migration will fail and block all subsequent migrations. Verifypgvectoris installed on both staging and production before deploying. - Rollback complexity: With 13 migrations spanning 6 new tables, rolling back requires dropping tables and removing columns in reverse order. If any table accumulates production data before a problem is discovered, data loss becomes a factor.
- The
add_index_to_leads_lead_qualitymigration operates on the partitionedleadstable. Verify it handles partition-level indexing correctly (checkstructure.sqldiff for both parent and child indexes).
Three critical concerns are modified:
Authentication(app/controllers/concerns/authentication.rb) — touched on every authenticated requestRequiresProfileCompletion(app/controllers/concerns/requires_profile_completion.rb) — gates user access; now routes v2 onboarding users to/onboardingApplicationController— base class for all controllers
Specific risks:
- The
RequiresProfileCompletionchange introduces v2 onboarding routing. If the v1/v2 detection logic has edge cases, existing agents could be incorrectly redirected to the onboarding flow, effectively locking them out of the app. - Any bug in the
Authenticationconcern affects every authenticated request across the entire application. - These are the kind of changes that pass all existing tests but break in production due to session state, cookie formats, or user data edge cases that tests don't cover.
This is a full CSS architecture migration:
Removed:
_components.scss— 12,410 lines deleted (the old monolith stylesheet)_jeromy.scss— 197 lines removedapplication.bootstrap.scss— old entry point removedstyles/_bootswatch.scss,styles/_variables.scss— removed
Added (~20 new component files):
_phosphor-icons.scss— 9,252 lines (icon library)_tokens.scss— 251 lines (new design token system)- 20+ component-scoped SCSS files (
_buttons.scss,_cards.scss,_tables.scss,_sidebar.scss,_panels.scss,_mobile.scss, etc.)
Modified:
_dark-mode.scss— 1,710 additions, 745 deletions (major overhaul)_custom.scss— 118 additions, 304 deletions_variables.scss— 7 additions, 5 deletions
Specific risks:
- Deleting 12,410 lines from
_components.scssand redistributing across 20+ files is prone to missed selectors — any selector that wasn't migrated will silently disappear, causing visual breakage only discoverable through manual testing. - Dark mode overhaul (1,710 lines changed) makes visual regression likely on every page.
- No visual regression testing infrastructure exists — all visual validation must be manual.
- The phosphor icons file (9,252 lines) is a large addition that could impact CSS build times and bundle size.
New OnboardingController with OnboardingStepService implementing a 9-step wizard:
- Signup → 2. Agreements → 3. Verification → 4. Credentials → 5. Lead Preferences → 6. Business Goals → 7. Personalization → 8. Business Info → 9. Success
Specific risks:
- No RSpec controller/request specs for
OnboardingController— the entire 9-step flow is untested at the controller level. Jest tests exist for the Stimulus controller, but server-side validation, step transitions, and error handling are not covered. - The
profile_wizardJSONB schema moves to v2 — existing v1 users are claimed to be unaffected, but this needs manual verification with real user data. - ActiveStorage photo/logo uploads added to User model — file upload handling adds complexity (storage backend config, size limits, content type validation).
New models: SupportTicket, SupportTicketReply, plus decorators and service objects.
- Has service-level specs (
Support::ReplyCreator,Support::TicketCreator,Support::TicketStatusChanger) - Has model specs for
SupportTicketandSupportTicketReply - Missing: Controller-level specs for both admin and user-facing support ticket controllers (6 new controllers with no request/controller specs)
New models: KbArticle, KbArticleRating, AiConversation, AiMessage, AiSuggestion, AiContentEmbedding
Specific risks:
- pgvector dependency (see migration risks above)
- AI tables suggest an LLM integration — if this calls external APIs (OpenAI, etc.), it introduces a new external dependency not visible in the diff summary
AiConversationandAiMessagemodels imply a chat interface — verify rate limiting and cost controls exist
New Organization model with IMO/Agency hierarchy, admin CRUD, and autocomplete in onboarding.
- Has a migration with a case-insensitive unique index
- No model specs visible for
Organization - Introduces a new entity in the data model that other features may depend on
ApplicationHelper grows by 591 lines (from ~24 to ~615). This is a significant concern for maintainability:
- Helpers are globally available in all views — a 615-line helper is a "god module" anti-pattern
- High risk of method name collisions
- Should ideally be decomposed into focused helper modules
+77 lines, -11 lines in config/routes.rb. New routes for onboarding, support tickets, knowledge base, organizations, reports, user settings, quick links, and design system preview.
| Area | Coverage |
|---|---|
| Jest (Stimulus controllers) | Good — ~40 new test files, ~15 modified |
| Model specs | Partial — KbArticle, SupportTicket, SupportTicketReply, AiConversation, AiSuggestion covered |
| Service specs | Good — Support::ReplyCreator, TicketCreator, TicketStatusChanger covered |
| Helper specs | Partial — LeadFlowReportsHelper, LeadOrdersHelper updated |
| Existing feature specs | Updated — several lead/order filter specs adjusted |
| Area | Gap |
|---|---|
| OnboardingController | No controller/request specs — 9-step flow entirely untested server-side |
| ReportsController | No specs |
| UserSettingsController | No specs |
| UserQuickLinksController | No specs |
| Admin::OrganizationsController | No specs |
| Admin::SupportDashboardController | No specs |
| Admin::SupportTicketsController | No specs (admin side) |
| Help::ConversationsController | No specs |
| KbArticlesController (both namespaces) | No specs |
| Organization model | No model specs |
| UserQuickLink model | No model specs |
| AI models (AiMessage, AiContentEmbedding) | No model specs |
| BrandingAttachments concern | No specs |
| Visual regression | No automated testing — all CSS changes require manual validation |
| Pro | Con |
|---|---|
| Standard workflow | Pollutes staging branch with 594 changed files |
| Every other developer's branches will conflict | |
| Cannot easily un-merge if issues found — would require force-pushing staging | |
| Other work on staging gets blocked or entangled | |
| Hotfixing staging for unrelated issues becomes complex |
| Pro | Con |
|---|---|
| Staging branch stays clean | Non-standard deployment workflow |
Easy rollback: just deploy staging branch back |
Must remember to specify branch in cap command |
| Other developers unaffected | |
| Fix-and-redeploy cycle stays on the branch | |
| Merge to staging only after validation |
Deployment command:
cap staging deploy BRANCH=feature/ux-v2Rollback command (if needed):
cap staging deploy BRANCH=stagingImportant caveat: If migrations have already run on staging, a simple branch rollback won't undo them. You would need to manually run
db:rollback STEP=13before redeploying the staging branch. This is another argument for deploying the branch directly — you can test migrations in isolation first.
Before deploying to any environment:
- Verify pgvector is installed on the target PostgreSQL server (
SELECT * FROM pg_available_extensions WHERE name = 'vector';) - Verify DB user has extension creation privileges (pgvector migration requires
CREATE EXTENSION) - Run migrations separately first to isolate DB issues from app deployment issues
- Test with existing v1 profile wizard users — confirm they are NOT redirected to
/onboarding - Test new user registration flow — verify the 9-step onboarding completes successfully
- Manual visual QA in both light and dark mode on at minimum:
- Login page
- Agent dashboard
- Leads index and show pages
- Lead orders index
- Admin pages (users, leads, orders)
- Manager pages
- Reports pages
- Support ticket pages (admin and user)
- Knowledge base
- User settings/profile
- Mobile viewport (all above)
- Test ActiveStorage uploads — verify photo and logo uploads work (check storage backend config)
- Verify sidebar navigation — new routes and navigation structure
- Check for console JS errors on all major pages (40 new Stimulus controllers)
- Deploy the branch directly to staging (not merge-then-deploy) to keep the staging branch clean and allow easy rollback.
- Verify pgvector availability on staging and production PostgreSQL servers before attempting migration.
- Conduct thorough manual QA focusing on the pre-deployment checklist above, especially existing user login flows and visual regressions.
- Add controller specs for OnboardingController — the 9-step flow is business-critical and has zero server-side test coverage.
- Add model specs for Organization — this is a new domain entity with hierarchy logic.
- Test the v1 → v2 profile wizard transition with a representative sample of real user data to verify no one gets locked out.
- Verify AI/LLM integration costs and rate limits if the AI chat feature calls external APIs.
- Decompose
ApplicationHelper— 615 lines in a single helper is a maintenance risk. Extract into focused modules (e.g.,NavigationHelper,StatusHelper,FilterHelper). - Consider visual regression testing (e.g., Percy, BackstopJS) — a CSS restructure of this magnitude makes a strong case for automated visual testing.
- Establish a PR size policy — a 594-file PR is unreviewable in any meaningful way. Consider enforcing a maximum file count or setting expectations for incremental delivery.
| Risk Area | Level | Reversible? | Automated Test Coverage |
|---|---|---|---|
| Database migrations (pgvector) | HIGH | Difficult | Schema check only |
| Authentication/routing concerns | HIGH | Yes (code revert) | Partial |
| CSS architecture restructure | HIGH | Yes (code revert) | None (visual) |
| Onboarding V2 flow | MEDIUM-HIGH | Yes (code revert) | Jest only, no server-side |
| Knowledge Base + AI system | MEDIUM-HIGH | Difficult (migrations) | Partial |
| Support ticket system | MEDIUM | Difficult (migrations) | Service specs only |
| Organization model | MEDIUM | Difficult (migration) | None |
| ApplicationHelper growth | MEDIUM | N/A (quality concern) | Partial |
| Route changes | LOW-MEDIUM | Yes (code revert) | Partial |
| Stimulus controllers | LOW | Yes (code revert) | Good (Jest) |
Analysis performed on 2026-03-20 by Claude Code against PR #3259 (feature/ux-v2 → staging). CI status: all checks passing except claude-review (pending).