From 375b16b26ef09a522259fbd3a69e5e277abfdc8b Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@hey.com>
Date: Thu, 18 Sep 2025 11:37:48 +0200
Subject: [PATCH] Introduce columns and column entity

To refactor some messy code used to render the cards index screen
---
 app/controllers/cards/drops_controller.rb     |  9 ++--
 app/controllers/cards_controller.rb           | 10 ++---
 .../concerns/collections/columns_scoped.rb    | 23 -----------
 app/models/cards/columns.rb                   | 41 +++++++++++++++++++
 app/models/cards/columns/column.rb            | 18 ++++++++
 app/views/cards/index.html.erb                |  2 +-
 app/views/cards/index/_columns.html.erb       | 12 +++---
 .../cards/index/engagement/_closed.html.erb   | 10 ++---
 .../index/engagement/_considering.html.erb    | 10 ++---
 .../cards/index/engagement/_doing.html.erb    | 14 +++----
 .../cards/index/engagement/_on_deck.html.erb  | 10 ++---
 11 files changed, 97 insertions(+), 62 deletions(-)
 delete mode 100644 app/controllers/concerns/collections/columns_scoped.rb
 create mode 100644 app/models/cards/columns.rb
 create mode 100644 app/models/cards/columns/column.rb

diff --git a/app/controllers/cards/drops_controller.rb b/app/controllers/cards/drops_controller.rb
index a404d2d3e..468290fd9 100644
--- a/app/controllers/cards/drops_controller.rb
+++ b/app/controllers/cards/drops_controller.rb
@@ -1,5 +1,6 @@
 class Cards::DropsController < ApplicationController
-  include Collections::ColumnsScoped
+  include FilterScoped
+
   before_action :set_card, :set_drop_target
 
   def create
@@ -34,11 +35,13 @@ class Cards::DropsController < ApplicationController
     end
 
     def render_column_replacement
-      page_and_filter = page_and_filter_for @filter.with(engagement_status: @drop_target.to_s), per_page: CardsController::PAGE_SIZE
+      columns = Cards::Columns.new(user_filtering: @user_filtering, page_size: CardsController::PAGE_SIZE)
+      column = columns.public_send(@drop_target)
+
       render \
         turbo_stream: turbo_stream.replace("#{@drop_target.to_s.gsub('_', '-')}-cards",
         method: :morph,
         partial: "cards/index/engagement/#{@drop_target}",
-        locals: { user_filtering: @user_filtering, **page_and_filter.to_h })
+        locals: { column: column })
     end
 end
diff --git a/app/controllers/cards_controller.rb b/app/controllers/cards_controller.rb
index d14a87c88..aa4aa5263 100644
--- a/app/controllers/cards_controller.rb
+++ b/app/controllers/cards_controller.rb
@@ -1,7 +1,7 @@
 require "ostruct"
 
 class CardsController < ApplicationController
-  include CardsHelper, Collections::ColumnsScoped
+  include CardsHelper, FilterScoped
 
   before_action :set_collection, only: %i[ create ]
   before_action :set_card, only: %i[ show edit update destroy ]
@@ -11,13 +11,9 @@ class CardsController < ApplicationController
   PAGE_SIZE = 25
 
   def index
-    @considering = page_and_filter_for @filter.with(engagement_status: "considering"), per_page: PAGE_SIZE
-    @on_deck = page_and_filter_for @filter.with(engagement_status: "on_deck"), per_page: PAGE_SIZE
-    @doing = page_and_filter_for @filter.with(engagement_status: "doing"), per_page: PAGE_SIZE
-    @closed = page_and_filter_for_closed_cards
+    @columns = Cards::Columns.new(user_filtering: @user_filtering, page_size: PAGE_SIZE)
 
-    @cache_key = [ @considering, @on_deck, @doing, @closed ].collect { it.page.records }.including([ Workflow.all, @user_filtering ])
-    fresh_when etag: [ @cache_key, @user_filtering ]
+    fresh_when etag: @columns
   end
 
   def create
diff --git a/app/controllers/concerns/collections/columns_scoped.rb b/app/controllers/concerns/collections/columns_scoped.rb
deleted file mode 100644
index a588a15b6..000000000
--- a/app/controllers/concerns/collections/columns_scoped.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-module Collections::ColumnsScoped
-  extend ActiveSupport::Concern
-
-  included do
-    include FilterScoped
-  end
-
-  def page_and_filter_for(filter, per_page: nil)
-    cards = block_given? ? yield(filter.cards) : filter.cards
-
-    OpenStruct.new \
-      page: GearedPagination::Recordset.new(cards, per_page:).page(1),
-      filter: filter
-  end
-
-  def page_and_filter_for_closed_cards
-    if @filter.indexed_by.stalled?
-      page_and_filter_for(@filter, per_page: CardsController::PAGE_SIZE) { |cards| cards.recently_closed_first }
-    else
-      page_and_filter_for(@filter.with(indexed_by: "closed"), per_page: CardsController::PAGE_SIZE) { |cards| cards.recently_closed_first }
-    end
-  end
-end
diff --git a/app/models/cards/columns.rb b/app/models/cards/columns.rb
new file mode 100644
index 000000000..269e06797
--- /dev/null
+++ b/app/models/cards/columns.rb
@@ -0,0 +1,41 @@
+class Cards::Columns
+  attr_reader :user_filtering, :page_size
+
+  delegate :filter, to: :user_filtering
+
+  def initialize(user_filtering:, page_size:)
+    @user_filtering = user_filtering
+    @page_size = page_size
+  end
+
+  def considering
+    @considering ||= build_column(filter.with(engagement_status: "considering"))
+  end
+
+  def on_deck
+    @on_deck ||= build_column(filter.with(engagement_status: "on_deck"))
+  end
+
+  def doing
+    @doing ||= build_column(filter.with(engagement_status: "doing"))
+  end
+
+  def closed
+    @closed ||= if filter.indexed_by.stalled?
+      build_column(filter) { |cards| cards.recently_closed_first }
+    else
+      build_column(filter.with(indexed_by: "closed")) { |cards| cards.recently_closed_first }
+    end
+  end
+
+  def cache_key
+    ActiveSupport::Cache.expand_cache_key([considering, on_deck, doing, closed, Workflow.all, user_filtering])
+  end
+
+  private
+    def build_column(filter, &block)
+      cards = block ? yield(filter.cards) : filter.cards
+
+      Column.new(page: GearedPagination::Recordset.new(cards, per_page: page_size).page(1), user_filtering: user_filtering)
+    end
+end
diff --git a/app/models/cards/columns/column.rb b/app/models/cards/columns/column.rb
new file mode 100644
index 000000000..91e0b3b0f
--- /dev/null
+++ b/app/models/cards/columns/column.rb
@@ -0,0 +1,18 @@
+class Cards::Columns::Column
+  attr_reader :page, :user_filtering
+
+  delegate :filter, to: :user_filtering
+
+  def initialize(page:, user_filtering:)
+    @page = page
+    @user_filtering = user_filtering
+  end
+
+  def cards
+    page.records
+  end
+
+  def cache_key
+    ActiveSupport::Cache.expand_cache_key([cards])
+  end
+end
diff --git a/app/views/cards/index.html.erb b/app/views/cards/index.html.erb
index 680535c0b..91602a3a6 100644
--- a/app/views/cards/index.html.erb
+++ b/app/views/cards/index.html.erb
@@ -42,5 +42,5 @@
         drop->drag-and-drop#drop
         dragend->drag-and-drop#dragEnd" } do %>
 
-  <%= render "cards/index/columns", user_filtering: @user_filtering, on_deck: @on_deck, considering: @considering, doing: @doing, closed: @closed, cache_key: @cache_key %>
+  <%= render "cards/index/columns", columns: @columns %>
 <% end %>
diff --git a/app/views/cards/index/_columns.html.erb b/app/views/cards/index/_columns.html.erb
index 9104145fc..b7b79aa67 100644
--- a/app/views/cards/index/_columns.html.erb
+++ b/app/views/cards/index/_columns.html.erb
@@ -1,13 +1,13 @@
-<% cache @cache_key do %>
+<% cache columns do %>
   <%= turbo_frame_tag :card_columns do %>
     <div class="card-columns" data-controller="related-element" data-related-element-highlight-class="cards--related">
-      <% unless user_filtering.only_closed? %>
-        <%= render "cards/index/engagement/on_deck", user_filtering: user_filtering, **on_deck.to_h %>
-        <%= render "cards/index/engagement/considering", user_filtering: user_filtering, **considering.to_h %>
-        <%= render "cards/index/engagement/doing", user_filtering: user_filtering, **doing.to_h %>
+      <% unless columns.user_filtering.only_closed? %>
+        <%= render "cards/index/engagement/on_deck", column: columns.on_deck %>
+        <%= render "cards/index/engagement/considering", column: columns.considering %>
+        <%= render "cards/index/engagement/doing", column: columns.doing %>
       <% end %>
     </div>
 
-    <%= render "cards/index/engagement/closed", user_filtering: user_filtering, **closed.to_h %>
+    <%= render "cards/index/engagement/closed", column: columns.closed %>
   <% end %>
 <% end %>
diff --git a/app/views/cards/index/engagement/_closed.html.erb b/app/views/cards/index/engagement/_closed.html.erb
index ad323c67b..d268713dc 100644
--- a/app/views/cards/index/engagement/_closed.html.erb
+++ b/app/views/cards/index/engagement/_closed.html.erb
@@ -1,15 +1,15 @@
 <section id="closed-cards" class="cards cards--closed pad-block">
   <h2 class="cards__heading"><span class="cards__filter btn non-clickable borderless">Recently closed</span></h2>
 
-  <% if page.used? %>
-    <%= render partial: "cards/display/preview", collection: page.records, as: :card, cached: true %>
+  <% if column.page.used? %>
+    <%= render partial: "cards/display/preview", collection: column.cards, as: :card, cached: true %>
 
-    <% unless page.last? %>
+    <% unless column.page.last? %>
       <div class="full-width">
-        <%= cards_next_page_link "closed-cards", page: page, filter: filter, fetch_on_visible: true %>
+        <%= cards_next_page_link "closed-cards", page: column.page, filter: column.filter, fetch_on_visible: true %>
       </div>
     <% end %>
   <% else %>
-    <p class="txt-normal translucent"><%= user_filtering.any? ? "No matches" : "Nothing here" %></p>
+    <p class="txt-normal translucent"><%= column.user_filtering.any? ? "No matches" : "Nothing here" %></p>
   <% end %>
 </section>
diff --git a/app/views/cards/index/engagement/_considering.html.erb b/app/views/cards/index/engagement/_considering.html.erb
index 44a80c0e5..6a2b60b39 100644
--- a/app/views/cards/index/engagement/_considering.html.erb
+++ b/app/views/cards/index/engagement/_considering.html.erb
@@ -5,13 +5,13 @@
     data-related-element-group-value="card-columns"
     data-action="mouseover->related-element#highlight mouseout->related-element#unhighlight">
   <div class="cards__decoration"></div>
-  <%= render "cards/index/add_card_button", user_filtering: user_filtering %>
-  <% if page.used? %>
-    <%= render partial: "cards/display/preview", collection: page.records, as: :card, locals: { draggable: true }, cached: true %>
+  <%= render "cards/index/add_card_button", user_filtering: column.user_filtering %>
+  <% if column.page.used? %>
+    <%= render partial: "cards/display/preview", collection: column.cards, as: :card, locals: { draggable: true }, cached: true %>
 
-    <% unless page.last? %>
+    <% unless column.page.last? %>
       <div class="full-width">
-        <%= cards_next_page_link "considering-cards", page: page, filter: filter %>
+        <%= cards_next_page_link "considering-cards", page: column.page, filter: column.filter %>
       </div>
     <% end %>
   <% end %>
diff --git a/app/views/cards/index/engagement/_doing.html.erb b/app/views/cards/index/engagement/_doing.html.erb
index 9dded775d..07e315065 100644
--- a/app/views/cards/index/engagement/_doing.html.erb
+++ b/app/views/cards/index/engagement/_doing.html.erb
@@ -4,23 +4,23 @@
     data-related-element-target="related"
     data-related-element-group-value="card-columns"
     data-action="mouseover->related-element#highlight mouseout->related-element#unhighlight">
-  <% if workflow = filter.single_workflow %>
-    <%= render "cards/index/workflow_filter", workflow: workflow, filter: filter %>
+  <% if workflow = column.filter.single_workflow %>
+    <%= render "cards/index/workflow_filter", workflow: workflow, filter: column.filter %>
   <% else %>
     <h2 class="cards__heading">
       Working On
     </h2>
   <% end %>
 
-    <% if page.used? %>
-      <%= render partial: "cards/display/preview", collection: page.records, as: :card, locals: { draggable: true }, cached: ->(card) { card.cache_invalidation_parts.for_preview } %>
+    <% if column.page.used? %>
+      <%= render partial: "cards/display/preview", collection: column.cards, as: :card, locals: { draggable: true }, cached: ->(card) { card.cache_invalidation_parts.for_preview } %>
 
-      <% unless page.last? %>
+      <% unless column.page.last? %>
         <div class="full-width">
-          <%= cards_next_page_link "doing-cards", page: page, filter: filter %>
+          <%= cards_next_page_link "doing-cards", page: column.page, filter: column.filter %>
         </div>
       <% end %>
     <% else %>
-      <p class="txt-normal translucent"><%= user_filtering.any? ? "No matches" : "Nothing here" %></p>
+      <p class="txt-normal translucent"><%= column.user_filtering.any? ? "No matches" : "Nothing here" %></p>
     <% end %>
 </section>
diff --git a/app/views/cards/index/engagement/_on_deck.html.erb b/app/views/cards/index/engagement/_on_deck.html.erb
index c56871f7c..0da1cc695 100644
--- a/app/views/cards/index/engagement/_on_deck.html.erb
+++ b/app/views/cards/index/engagement/_on_deck.html.erb
@@ -6,15 +6,15 @@
     data-action="mouseover->related-element#highlight mouseout->related-element#unhighlight">
   <h2 class="cards__heading"><span class="cards__filter btn non-clickable borderless">On Deck</span></h2>
 
-  <% if page.used? %>
-    <%= render partial: "cards/display/preview", collection: page.records, as: :card, locals: { draggable: true }, cached: true %>
+  <% if column.page.used? %>
+    <%= render partial: "cards/display/preview", collection: column.cards, as: :card, locals: { draggable: true }, cached: true %>
 
-    <% unless page.last? %>
+    <% unless column.page.last? %>
       <div class="full-width">
-        <%= cards_next_page_link "on-deck-cards", page: page, filter: filter %>
+        <%= cards_next_page_link "on-deck-cards", page: column.page, filter: column.filter %>
       </div>
     <% end %>
   <% else %>
-    <p class="txt-normal translucent"><%= user_filtering.any? ? "No matches" : "Nothing here" %></p>
+    <p class="txt-normal translucent"><%= column.user_filtering.any? ? "No matches" : "Nothing here" %></p>
   <% end %>
 </section>