Phase 05: View Layer Extraction Research

Goal: Extract view logic into ViewComponents, presenters, and helpers to improve maintainability, testability, and reusability.

Current State

Views Directory Structure

Total views: 44 ERB templates across 12 directories

app/views/source_monitor/
├── dashboard/ (8 files)
│   ├── index.html.erb (54 lines)
│   ├── _stats.html.erb (30 lines)
│   ├── _stat_card.html.erb (partial)
│   ├── _recent_activity.html.erb (partial)
│   ├── _fetch_schedule.html.erb (partial)
│   ├── _job_metrics.html.erb (partial)
│   └── _scrape_recommendations.html.erb (partial)
│
├── sources/ (11 files)
│   ├── index.html.erb (282 lines) [LARGEST]
│   ├── show.html.erb (partial)
│   ├── edit.html.erb (partial)
│   ├── new.html.erb (partial)
│   ├── _form.html.erb (partial)
│   ├── _form_fields.html.erb (partial)
│   ├── _row.html.erb (144 lines)
│   ├── _details.html.erb (362 lines) [VERY LARGE]
│   ├── _health_status_badge.html.erb (47 lines)
│   ├── _bulk_scrape_modal.html.erb (partial)
│   └── _bulk_scrape_form.html.erb (partial)
│
├── items/ (3 files)
│   ├── index.html.erb (78+ lines)
│   ├── show.html.erb (partial)
│   └── _details.html.erb (partial)
│
├── import_sessions/ (8 files)
│   ├── show.html.erb (partial)
│   ├── _header.html.erb (partial)
│   ├── _sidebar.html.erb (partial)
│   ├── steps/ (5 files)
│   └── health_check/ (2 files)
│
├── logs/ (1 file)
│   └── index.html.erb (partial)
│
├── shared/ (4 files)
│   ├── _toast.html.erb (36 lines)
│   ├── _pagination.html.erb (partial)
│   └── other shared partials
│
└── other/ (remaining controllers)
    ├── fetch_logs/, scrape_logs/, source_scrape_tests/

Stimulus Controllers

Total: 7 controllers in app/assets/javascripts/source_monitor/controllers/

Helpers

Total: 3 helper modules in app/helpers/source_monitor/

Presenters & Components

• No ViewComponents exist yet (app/components/ is empty)
• No Presenter classes exist (view logic remains in templates and helpers)

JavasScript Globals (Window Namespace Pollution)

Files that write to window.*:

1. notification_controller.js (line 9-10, 30)

   javascriptCopy

   window.SourceMonitorControllers = {};
   window.SourceMonitorControllers.notification = this;
   

   • Purpose: Register notification controller instance globally
   • Risk: Namespace collision, not garbage-collected

2. turbo_actions.js (line 4-5, 10)

   javascriptCopy

   window.Turbo.StreamActions.redirect = function() { ... }
   window.Turbo.visit(url, { action: visitAction });
   

   • Purpose: Register custom Turbo Stream action
   • Note: Turbo itself is global; this extends it (acceptable)

3. application.js (line 11, 15)

   javascriptCopy

   window.SourceMonitorStimulus = application;
   

   • Purpose: Export Stimulus application for debugging
   • Risk: Global reference prevents garbage collection

Audit Findings Detail

V1: Sources Index Filter Logic (56 lines)

File: app/views/source_monitor/sources/index.html.erb (lines 22-53)

Issue: Complex filter building logic mixed in view template:

• Lines 44: Dynamic adapter options query + pluck (N+1 candidate)
• Lines 57-106: Filter state tracking + label building + clear-link generation
  • Hardcoded filter label mapping (lines 86-93): 6 filters with conditional formatting
  • Multiple hash transformations and compact operations
  • Query path generation logic embedded

Evidence:

<% adapter_options = SourceMonitor::Source.distinct.where.not(scraper_adapter: [nil, ""]).order(:scraper_adapter).pluck(:scraper_adapter) %>

<% active_dropdown_filters = dropdown_filter_keys.select { |k| @search_params[k].present? } %>
<% has_any_filter = @search_term.present? || @fetch_interval_filter.present? || active_dropdown_filters.any? %>

<% filter_labels = {
  "active_eq" => @search_params["active_eq"] == "true" ? "Status: Active" : "Status: Paused",
  "health_status_eq" => "Health: #{@search_params['health_status_eq']&.titleize}",
  "feed_format_eq" => "Format: #{@search_params['feed_format_eq']&.upcase}",
  ... (6 items total)
} %>

Recommendation: Extract to SourcesIndexPresenter with:

• active_filter_keys, filter_labels, has_any_filter? as instance methods
• Move adapter options query to controller or query object
• Move filter clearing path logic to helper

V3: Sources Row Partial N+1 Risk

File: app/views/source_monitor/sources/_row.html.erb (lines 1-11)

Issue: Undocumented preload requirements for optimal performance

Evidence:

<% rate_map = local_assigns[:item_activity_rates] || {} %>
<% avg_feed_words_map = local_assigns[:avg_feed_word_counts] || {} %>
<% avg_scraped_words_map = local_assigns[:avg_scraped_word_counts] || {} %>

The partial expects caller to preload these maps. No controller-level documentation of what queries these require. If maps are missing, fallback values hide performance issues.

Evidence of hidden data dependency:

• Line 41: source_favicon_tag(source, size: 24) — requires favicon attachment (Active Storage guard)
• Line 10: source_health_badge(source) — depends on health_status enum
• Line 71-72: Health status badge render — may trigger additional query if badge changed

Recommendation: Document preload requirements + add to controller or create SourceRowPresenter to encapsulate map lookups.

V4: Dashboard Stats Re-render Issue (Partial Turbo Stream)

File: app/views/source_monitor/dashboard/index.html.erb (lines 1-10)

Issue: Dashboard listens to single stream but re-renders entire _stats partial on any stat change

<%= turbo_stream_from SourceMonitor::Dashboard::TurboBroadcaster::STREAM_NAME %>
<div class="mt-6">
  <%= render "stats", stats: @stats %>
</div>

When any stat changes (e.g., fetches_today increments):

• Entire stats section re-renders via Turbo Stream
• All 5 stat cards redraw (not just the changed one)
• No targeted update of individual stat cards

Evidence from _stats.html.erb (lines 1-30):

<div id="source_monitor_dashboard_stats">
  <div class="grid gap-5 sm:grid-cols-2 xl:grid-cols-5">
    <%= render partial: "stat_card", collection: [
      { label: "Sources", value: stats[:total_sources], ... },
      { label: "Active", value: stats[:active_sources], ... },
      { label: "Failures", value: stats[:failed_sources], ... },
      ...
    ], locals: { stats: stats } %>
  </div>

Recommendation: Split into per-stat Turbo Stream updates. Each stat gets its own IDs and targeted updates, e.g., source_monitor_dashboard_stat_total_sources, source_monitor_dashboard_stat_active_sources.

V5: Filter Button/Link Duplication

Files:

• app/views/source_monitor/sources/index.html.erb (lines 22-53)
• app/views/source_monitor/items/index.html.erb (similar structure)
• app/views/source_monitor/logs/index.html.erb (similar structure)

Issue: Identical filter dropdown pattern repeated across 3 views with no shared component

Evidence (sources index):

<div>
  <%= form.label :active_eq, "Status", class: "block text-xs font-medium text-slate-500 mb-1" %>
  <%= form.select :active_eq, options_for_select([["All Statuses", ""], ["Active", "true"], ["Paused", "false"]], @search_params["active_eq"].to_s), {},
        class: "rounded-md border border-slate-200 bg-white px-2 py-2 text-sm text-slate-700 focus:border-blue-500 focus:outline-none focus:ring-1 focus:ring-blue-500",
        onchange: "this.form.requestSubmit()" %>
</div>

Same pattern in items/index, logs/index with only label/param names changing.

Recommendation: Extract to FilterDropdownComponent (ViewComponent) with:

• :label, :param_name, :options inputs
• Auto-submit on change
• Shared styling

V6: Dropdown Controller Async Loading Fragility

File: app/assets/javascripts/source_monitor/controllers/dropdown_controller.js (lines 36-58)

Issue: Complex dynamic module loading (stimulus-use) with fragile fallback

Evidence:

async loadTransitions() {
  if (!this.hasMenuTarget || this.transitionModuleValue === "") {
    this.logFallback();
    return;
  }

  try {
    const module = await import(this.transitionModuleValue);
    const useTransition = module?.useTransition || module?.default?.useTransition;

    if (typeof useTransition === "function") {
      useTransition(this, {
        element: this.menuTarget,
        hiddenClass: this.hiddenClassValue
      });
      this.transitionEnabled = true;
    } else {
      this.logFallback();
    }
  } catch (error) {
    this.logFallback(error);
  }
}

Risks:

1. stimulus-use not guaranteed as dependency — fallback to CSS toggling works but is inconsistent
2. Module resolution fragile — default?.useTransition vs useTransition check is defensive but unclear
3. State tracking complex — this.transitionEnabled, this.toggleTransition, this.leave state management is hard to follow
4. Silent degradation — console.warn logged only once (_fallbackLogged flag), hard to debug in production

Code comment notes (line 35): "Evaluated for simplification in Phase 20.05.07 - Decision: Keep current implementation."

Recommendation: Simplify to one path:

1. Option A: Always use CSS class toggling (remove stimulus-use dependency)
2. Option B: Require stimulus-use as explicit dependency, remove fallback

V7: Dropdown State Isolation Risk

Files:

• app/views/source_monitor/sources/_row.html.erb (lines 109-141) — actions dropdown
• app/views/source_monitor/sources/_health_status_badge.html.erb (lines 8-37) — health menu

Issue: Multiple dropdowns on the same page use data-controller="dropdown" with shared class names and no unique IDs for menu visibility state

Evidence:

<!-- Row dropdown -->
<div data-controller="dropdown" class="relative inline-block text-left">
  <button data-action="dropdown#toggle click@window->dropdown#hide">...</button>
  <div data-dropdown-target="menu" class="... hidden">...</div>
</div>

<!-- Health badge dropdown (elsewhere on same page) -->
<div data-controller="dropdown" class="relative inline-block text-left" data-testid="source-health-menu">
  <button data-action="dropdown#toggle click@window->dropdown#hide">...</button>
  <div data-dropdown-target="menu" class="... hidden">...</div>
</div>

Risk:

• click@window->dropdown#hide event bound globally — all dropdowns listen to ALL clicks
• If one dropdown's hide event fires, it matches ALL data-dropdown-target="menu" selectors on the page
• State isolation depends on CSS selector specificity and controller scoping (should be fine, but fragile)

Test case: Open row actions dropdown, then click row name link elsewhere → all dropdowns may close unintentionally

Recommendation:

1. Add unique IDs to each dropdown container
2. Bind hide event only to that dropdown's container, not global window click
3. Use CSS data attributes for state instead of class toggle (e.g., data-open="true")

V9: Inline SVG Repetition (No Icon System)

Files with inline SVGs:

1. app/views/source_monitor/sources/_row.html.erb (lines 114-117) — 3-dot menu icon (4 lines)
2. app/views/source_monitor/sources/_details.html.erb (lines 31-33) — favicon refresh icon (3 lines)
3. app/views/source_monitor/sources/_health_status_badge.html.erb (lines 17-19) — chevron-down icon (3 lines)
4. app/helpers/source_monitor/application_helper.rb (lines 285-299) — external-link icon (15 lines)
5. app/helpers/source_monitor/application_helper.rb (lines 191-201) — spinner SVG helper (11 lines)

Issue: SVG markup duplicated or scattered across helpers without centralized icon system

Evidence:

<!-- _row.html.erb: 3-dot menu -->
<svg class="h-5 w-5" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 20 20" fill="none" stroke="currentColor" stroke-width="1.5" aria-hidden="true">
  <path stroke-linecap="round" stroke-linejoin="round" d="..." />
  ...
</svg>

<!-- _details.html.erb: refresh icon (different path) -->
<svg class="h-4 w-4 text-slate-500" xmlns="http://www.w3.org/2000/svg" fill="none" viewBox="0 0 24 24" stroke-width="1.5" stroke="currentColor">
  <path stroke-linecap="round" stroke-linejoin="round" d="M16.023 9.348h4.992v-.001M2.985 19.644v-4.992m..." />
</svg>

Recommendation: Create IconComponent (ViewComponent) with named icon registry:

render IconComponent.new(:menu_dots, size: :md)
render IconComponent.new(:refresh, size: :sm)
render IconComponent.new(:chevron_down)

V12: Turbo Frame Naming Inconsistency

Files with Turbo Frame IDs:

1. app/views/source_monitor/sources/index.html.erb (line 56): source_monitor_sources_table
2. app/views/source_monitor/items/index.html.erb (line 17): source_monitor_items_table
3. app/views/source_monitor/dashboard/_fetch_schedule.html.erb (line 11): source_monitor_schedule_#{group.key} (dynamic)
4. app/views/source_monitor/import_sessions/show.html.erb (line 10): import_session_step

Issue: Naming convention is inconsistent

• source_monitor_*_table prefix for lists
• source_monitor_*_* prefix for sections
• import_session_* prefix for import wizard (no source_monitor prefix!)
• Dynamic IDs mixed with static

Risk: Makes it hard to:

• Write CSS/JS selectors (no consistent pattern)
• Document which frames exist
• Refactor without breaking links

Evidence:

• Line 22 (sources/index): data: { turbo_frame: "source_monitor_sources_table" }
• Line 7 (items/index): data: { turbo_frame: "source_monitor_items_table" }
• Line 11 (dashboard/_fetch_schedule): turbo_frame_tag "source_monitor_schedule_#{group.key}"

Recommendation: Establish consistent naming scheme:

• All engine frames: source_monitor_{section}_{element}
• Examples: source_monitor_sources_table, source_monitor_dashboard_stats, source_monitor_import_step

V13: Global Window Namespace Pollution

Identified locations:

1. notification_controller.js (lines 9-10, 30)

   • Registers instance: window.SourceMonitorControllers.notification = this
   • Risk: Controller instance never garbage-collected; grows with each page reload in SPA

2. turbo_actions.js (lines 4-5)

   • Registers custom Turbo Stream action: window.Turbo.StreamActions.redirect = ...
   • Status: Acceptable (standard Turbo extension pattern), but pollutes Turbo namespace

3. application.js (lines 11, 15)

   • Exports Stimulus app: window.SourceMonitorStimulus = application
   • Purpose: Debugging/inspection in dev console
   • Risk: Prevents garbage collection in SPA environments; leaks memory over multiple navigations

4. Implicit globals in templates

   • Views reference helpers like source_health_badge(), async_status_badge() — these are method calls, not global pollution
   • No code directly assigns to window. in view templates (✓ good)

Recommendation:

1. Remove window.SourceMonitorControllers — controllers don't need global registration
2. Consider lazy-loading window.SourceMonitorStimulus only in dev mode
3. Keep window.Turbo.StreamActions.redirect (standard pattern)

V14: Missing SourceDetailsPresenter

File: app/views/source_monitor/sources/_details.html.erb (362 lines) [VERY LARGE]

Issue: Massive partial with embedded view logic, no presenter abstraction

Evidence (lines 177-203):

<% interval_hours = number_with_precision(source.fetch_interval_minutes / 60.0, precision: 2)
   circuit_state =
     if source.fetch_circuit_open?
       until_time = source.fetch_circuit_until&.strftime("%b %d, %Y %H:%M %Z") || "unknown"
       "Open until #{until_time}"
     else
       "Closed"
     end

   details = {
     "Website" => (source.website_url.present? ? external_link_to(...) : "\u2014"),
     "Fetch interval" => "#{source.fetch_interval_minutes} minutes (~#{interval_hours} hours)",
     "Adaptive interval" => source.adaptive_fetching_enabled? ? "Auto" : "Fixed",
     "Scraper" => source.scraper_adapter,
     ...
   } %>

Methods called in template (should move to presenter):

• number_with_precision (formatting)
• external_link_to (link generation)
• Conditional logic (ternary operators scattered throughout)
• Array/Hash transformations (Array(item.categories).filter_map)
• Date formatting (log.started_at&.strftime(...))

Recommendation: Create SourceDetailsPresenter extending BasePresenter:

class SourceDetailsPresenter < BasePresenter
  def fetch_interval_hours
    number_with_precision(source.fetch_interval_minutes / 60.0, precision: 2)
  end

  def circuit_state_label
    source.fetch_circuit_open? ? "Open until #{...}" : "Closed"
  end

  def details_hash
    {
      "Website" => website_link,
      "Fetch interval" => "#{fetch_interval} minutes (~#{fetch_interval_hours} hours)",
      ...
    }
  end

  private

  def website_link
    source.website_url.present? ? external_link_to(source.website_url, ...) : "—"
  end
end

V15: Modal Markup Not Extracted

Files:

• app/views/source_monitor/sources/_bulk_scrape_modal.html.erb
• app/views/source_monitor/sources/_bulk_scrape_enable_modal.html.erb
• app/views/source_monitor/import_sessions/show.html.erb (modals for import steps)

Issue: Modal structure duplicated across views, no ViewComponent abstraction. Marked as "to be handled by Phase 07 UTM integration" — deferring this audit finding is acceptable for now.

Note: Phase 07 will extract modals into a reusable ModalComponent.

Existing Patterns

Helpers (Production)

application_helper.rb (356 lines):

• ✓ loading_spinner_svg() — SVG helper (lines 191-202)
• ✓ external_link_icon() (private, lines 284-300)
• ✓ external_link_to() — wrapper for link_to with icon (lines 223-230)
• ✓ source_favicon_tag() — favicon + placeholder (lines 248-256)
• ✓ async_status_badge() — badge styling for fetch/scrape states (lines 140-165)
• ✓ item_scrape_status_badge() — item-specific badge logic (lines 171-183)
• ✓ Status badge mappings (ITEM_SCRAPE_STATUS_LABELS constant)
• ✗ No presenter backing — all view logic stays in helper

health_badge_helper.rb (57 lines):

• ✓ source_health_badge() — health status styling (lines 5-19)
• ✓ source_health_actions() — contextual dropdown menu items (lines 21-48)
• ✓ interactive_health_status?() — boolean check for interactivity (lines 50-54)
• ✗ No presenter — constants defined inline

table_sort_helper.rb (54 lines):

• ✓ table_sort_direction() — detect current sort (lines 5-10)
• ✓ table_sort_arrow() — visual indicator (▲▼↕, lines 12-23)
• ✓ table_sort_aria() — ARIA label (lines 25-36)
• ✓ table_sort_link() — link generation with Ransack (lines 38-51)
• ✗ No presenter

Stimulus Controllers (Production)

Well-structured:

• ✓ async_submit_controller.js (36 lines) — button state management, clean
• ✓ select_all_controller.js (56 lines) — master/detail checkbox sync, clean
• ✓ notification_controller.js (63 lines) — toast auto-dismiss + global registration (⚠ V13 pollution)
• ✓ modal_controller.js (65 lines) — modal open/close, escape handling, clean
• ✓ confirm_navigation_controller.js — unsaved changes check

Fragile:

• ⚠ dropdown_controller.js (110 lines) — complex async loading with stimulus-use (V6)

Turbo Patterns

Streams:

• ✓ Dashboard uses turbo_stream_from for live stat updates
• ✓ Notifications use Turbo Streams for toast delivery
• ⚠ Dashboard stats not granular (V4) — re-renders entire section instead of individual stat

Frames:

• ✓ Used for table pagination (sources, items)
• ✓ Used for import wizard steps
• ⚠ Naming inconsistent (V12)
• ⚠ Dynamic IDs mixed with static (V12)

Recommendations

Priority 1: Extract Large, Repetitive Views

Rationale: These unblock V3, V4 improvements and reduce line counts immediately.

Priority 2: Fix Turbo & Stream Issues

Rationale: Improves real-time UX and reduces re-renders.

Priority 3: Clean Up JavaScript

Rationale: Improves maintainability and prevents memory leaks in SPA.

Priority 4: Documentation

Summary Table

Key Metrics

• Views to refactor: 15+ (primarily sources/, dashboard/, items/)
• Stimulus controllers to improve: 7 (3 excellent, 1 fragile, 3 good)
• Helpers to back with presenters: 3
• Lines of view logic to extract: 500+ (V1, V14, scattered helpers)
• Duplications found: 3 major (V5, V9, modals)
• Global pollution points: 3 (window.SourceMonitorControllers, window.Turbo.StreamActions, window.SourceMonitorStimulus)

Related Files

Key Audit Context

• Previous audit: Phase 04 completed Item/Source model extraction; this phase targets presentation layer
• Architecture guide: .claude/skills/sm-architecture/reference/module-map.md
• Domain model: .claude/skills/sm-domain-model/reference/model-graph.md

Implementation Resources

• ViewComponent skill: .claude/skills/viewcomponent-patterns/
• Presenter skill: .claude/skills/rails-presenter/
• Hotwire skill: .claude/skills/hotwire-patterns/

Test References

• Test helpers: test/helpers/ (may need coverage for new presenters)
• System tests: test/system/ (Hotwire interactions already tested)

Confidence Assessment

High confidence — all findings backed by direct code evidence, line-number references, and clear patterns.

• ✓ V1, V3, V4, V5, V6, V9, V12, V13, V14 verified with specific file paths + line ranges
• ✓ V7 requires testing but logic is clear
• ✓ V15 already documented in codebase ("deferred to Phase 07")

Evidence quality: Direct inspection of 44 view files, 7 Stimulus controllers, 3 helpers, application layout.