Merge pull request #1132 from basecamp/invalidation-cache

Fix several caching issues when viewing card permas

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

app/controllers/cards_controller.rb

@@ -1,7 +1,7 @@
 require "ostruct"
 
 class CardsController < ApplicationController
-  include 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
+    fresh_when etag: @columns
   end
 
   def create
@@ -26,7 +22,7 @@ class CardsController < ApplicationController
   end
 
   def show
-    fresh_when @card
+    fresh_when etag: @card.cache_invalidation_parts.for_perma
   end
 
   def edit

app/controllers/concerns/collections/columns_scoped.rb

@@ -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

app/helpers/cards_helper.rb

@@ -65,8 +65,4 @@ module CardsHelper
     title << "assigned to #{card.assignees.map(&:name).to_sentence}" if card.assignees.any?
     title.join(" ")
   end
-
-  def cacheable_preview_parts_for(card, *options)
-    [ card, card.workflow, card.collection.entropy_configuration, card.collection.publication, *options ]
-  end
 end

app/models/card.rb

@@ -1,5 +1,5 @@
 class Card < ApplicationRecord
-  include Assignable, Attachments, Closeable, Colored, Engageable, Entropic, Eventable,
+  include Assignable, Attachments, Cacheable, Closeable, Colored, Engageable, Entropic, Eventable,
     Golden, Mentions, Multistep, Pinnable, Promptable, Readable, Searchable,
     Staged, Stallable, Statuses, Taggable, Watchable
 

app/models/card/cacheable.rb

@@ -0,0 +1,23 @@
+module Card::Cacheable
+  extend ActiveSupport::Concern
+
+  def cache_invalidation_parts
+    @cache_invalidation_parts ||= InvalidationParts.new(self)
+  end
+
+  class InvalidationParts
+    attr_reader :card
+
+    def initialize(card)
+      @card = card
+    end
+
+    def for_perma(*other)
+      [ card, Workflow.all, User.all, Tag.all, *other ]
+    end
+
+    def for_preview(*other)
+      [ card, card.workflow, Workflow.all, card.collection.entropy_configuration, card.collection.publication, *other ]
+    end
+  end
+end

app/models/card/staged.rb

@@ -7,10 +7,7 @@ module Card::Staged
     before_create :assign_initial_stage
 
     scope :in_stage, ->(stage) { where stage: stage }
-  end
-
-  def workflow
-    stage&.workflow
+    delegate :workflow, to: :collection, allow_nil: true
   end
 
   def staged?

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

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

app/views/cards/_container.html.erb

@@ -1,4 +1,4 @@
-<% cache card do %>
+<% cache card.cache_invalidation_parts.for_perma do %>
   <section id="<%= dom_id(card, :card_container) %>"
     class="card-perma" style="--card-color: <%= card.color %>;">
     <%= render "cards/container/engagement", card: card %>

app/views/cards/columns/_column.html.erb

@@ -0,0 +1 @@
+<%= render partial: "cards/display/preview", collection: column.cards, as: :card, locals: { draggable: draggable }, cached: ->(card) { card.cache_invalidation_parts.for_preview } %>

app/views/cards/display/_preview.html.erb

@@ -1,43 +1,41 @@
 <% draggable = local_assigns.fetch(:draggable, false) && card.published? %>
 
-<% cache cacheable_preview_parts_for(card, draggable) do %>
-  <%= card_article_tag card, class: "card", draggable: draggable, data: { id: card.id, drag_and_drop_target: "item" } do %>
-    <div class="flex flex-column flex-item-grow max-inline-size">
-      <header class="card__header">
-        <%= render "cards/display/preview/collection", card: card %>
-        <%= render "cards/display/preview/tags", card: card %>
-        <%= render "cards/display/preview/steps", card: card %>
-        <%= icon_tag "attachment", class: "card__attachments-indicator translucent txt-x-small" if card.has_attachments? %>
+<%= card_article_tag card, class: "card", draggable: draggable, data: { id: card.id, drag_and_drop_target: "item" } do %>
+  <div class="flex flex-column flex-item-grow max-inline-size">
+    <header class="card__header">
+      <%= render "cards/display/preview/collection", card: card %>
+      <%= render "cards/display/preview/tags", card: card %>
+      <%= render "cards/display/preview/steps", card: card %>
+      <%= icon_tag "attachment", class: "card__attachments-indicator translucent txt-x-small" if card.has_attachments? %>
 
-        <% if card.staged? && card.doing? %>
+      <% if card.staged? && card.doing? %>
           <span class="btn justify-start workflow-stage workflow-stage--current txt-uppercase min-width max-width">
             <span class="overflow-ellipsis "><%= card.stage.name %></span>
           </span>
-        <% end %>
-      </header>
+      <% end %>
+    </header>
 
-      <div class="card__body justify-space-between">
-        <div class="card__content">
-          <h3 class="card__title overflow-line-clamp">
-            <%= card.title %>
-          </h3>
-        </div>
-
-        <%= render "cards/display/preview/stages", card: card if card.doing? %>
+    <div class="card__body justify-space-between">
+      <div class="card__content">
+        <h3 class="card__title overflow-line-clamp">
+          <%= card.title %>
+        </h3>
       </div>
+
+      <%= render "cards/display/preview/stages", card: card if card.doing? %>
     </div>
+  </div>
 
-    <footer class="card__footer">
-      <%= render "cards/display/preview/meta", card: card %>
-      <%= render "cards/display/common/background", card: card %>
-    </footer>
+  <footer class="card__footer">
+    <%= render "cards/display/preview/meta", card: card %>
+    <%= render "cards/display/common/background", card: card %>
+  </footer>
 
-    <%= link_to card_path(card), draggable: false, class: "card__link", title: card_title_tag(card), data: { action: "dialog#close", turbo_frame: "_top" } do %>
-      <span class="for-screen-reader"><%= card.title %></span>
-    <% end %>
+  <%= link_to card_path(card), draggable: false, class: "card__link", title: card_title_tag(card), data: { action: "dialog#close", turbo_frame: "_top" } do %>
+    <span class="for-screen-reader"><%= card.title %></span>
+  <% end %>
 
-    <% if card.entropic? %>
-      <%= render "cards/display/preview/bubble", card: card %>
-    <% end %>
+  <% if card.entropic? %>
+    <%= render "cards/display/preview/bubble", card: card %>
   <% end %>
 <% end %>

app/views/cards/display/_public_preview.html.erb

@@ -1,4 +1,4 @@
-<% cache cacheable_preview_parts_for(card) do %>
+<% cache card.cache_invalidation_parts.for_preview do %>
   <%= card_article_tag card, class: "card" do %>
     <div class="flex gap">
       <div class="flex flex-column flex-item-grow">

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 %>

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 %>

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 "cards/columns/column", column: column, draggable: false %>
 
-    <% 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>

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 "cards/columns/column", column: column, draggable: 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 %>

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) { cacheable_preview_parts_for(card) } %>
+    <% if column.page.used? %>
+      <%= render "cards/columns/column", column: column, draggable: true %>
 
-      <% 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>

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 "cards/columns/column", column: column, draggable: 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>

app/views/public/collections/show/_doing.html.erb

@@ -4,7 +4,7 @@
   </h2>
 
   <% if page.used? %>
-    <%= render partial: "cards/display/public_preview", collection: page.records, as: :card, cached: ->(card) { cacheable_preview_parts_for(card) } %>
+    <%= render partial: "cards/display/public_preview", collection: page.records, as: :card, cached: ->(card) { card.cache_invalidation_parts.for_preview } %>
 
     <% unless page.last? %>
       <div class="full-width">