From 5a2b7c5abd9a73ec96c6c44a4f97c8706f5e0cfa Mon Sep 17 00:00:00 2001
From: Jorge Manrubia <jorge@37signals.com>
Date: Thu, 3 Apr 2025 13:26:16 +0200
Subject: [PATCH] Rework buckets index using proper queries and tidy up

---
 app/controllers/bubbles_controller.rb         |  6 +-
 app/models/filter.rb                          |  6 +-
 app/models/filter/fields.rb                   |  8 ++
 app/views/bubbles/index.html.erb              | 80 +------------------
 .../bubbles/index/_workflow_filter.html.erb   | 41 ++++++++++
 .../index/bubbles/_considering.html.erb       | 10 +++
 .../bubbles/index/bubbles/_doing.html.erb     | 15 ++++
 .../bubbles/index/bubbles/_popped.html.erb    | 12 +++
 8 files changed, 98 insertions(+), 80 deletions(-)
 create mode 100644 app/views/bubbles/index/_workflow_filter.html.erb
 create mode 100644 app/views/bubbles/index/bubbles/_considering.html.erb
 create mode 100644 app/views/bubbles/index/bubbles/_doing.html.erb
 create mode 100644 app/views/bubbles/index/bubbles/_popped.html.erb

diff --git a/app/controllers/bubbles_controller.rb b/app/controllers/bubbles_controller.rb
index 7864ffe31..d64f76a38 100644
--- a/app/controllers/bubbles_controller.rb
+++ b/app/controllers/bubbles_controller.rb
@@ -9,10 +9,12 @@ class BubblesController < ApplicationController
 
   DISPLAY_COUNT_OPTIONS = [ 6, 12, 18, 24 ].freeze
   DEFAULT_DISPLAY_COUNT = 6
+  RECENTLY_POPPED_LIMIT = 100
 
   def index
-    @bubbles = @filter.bubbles.published_or_drafted_by(Current.user)
-    @display_count = display_count
+    @considering_bubbles = @filter.bubbles.considering
+    @doing_bubbles = @filter.bubbles.doing
+    @popped_bubbles = @filter.with(indexed_by: "popped").bubbles.limit(RECENTLY_POPPED_LIMIT)
   end
 
   def create
diff --git a/app/models/filter.rb b/app/models/filter.rb
index 1c94485f4..d3d7df9c2 100644
--- a/app/models/filter.rb
+++ b/app/models/filter.rb
@@ -19,7 +19,7 @@ class Filter < ApplicationRecord
   def bubbles
     @bubbles ||= begin
       result = creator.accessible_bubbles.indexed_by(indexed_by)
-      result = result.active unless indexed_by.popped?
+      result = indexed_by.popped? ? result.popped : result.active
       result = result.unassigned if assignment_status.unassigned?
       result = result.assigned_to(assignees.ids) if assignees.present?
       result = result.where(creator_id: creators.ids) if creators.present?
@@ -42,6 +42,10 @@ class Filter < ApplicationRecord
     buckets.first if buckets.one?
   end
 
+  def single_workflow
+    buckets.first.workflow if buckets.pluck(:workflow_id).uniq.one?
+  end
+
   def cacheable?
     buckets.exists?
   end
diff --git a/app/models/filter/fields.rb b/app/models/filter/fields.rb
index 37880f230..a9d7ab28b 100644
--- a/app/models/filter/fields.rb
+++ b/app/models/filter/fields.rb
@@ -31,6 +31,14 @@ module Filter::Fields
     end
   end
 
+  def with(**fields)
+    dup.tap do |filter|
+      fields.each do |key, value|
+        filter.public_send("#{key}=", value)
+      end
+    end
+  end
+
   def default_indexed_by
     self.class.default_values[:indexed_by]
   end
diff --git a/app/views/bubbles/index.html.erb b/app/views/bubbles/index.html.erb
index b25d51842..026f80100 100644
--- a/app/views/bubbles/index.html.erb
+++ b/app/views/bubbles/index.html.erb
@@ -7,82 +7,8 @@
 <% end %>
 
 <div class="cards--columns">
-  <section class="cards" style="align-items: center; border-inline-end: 1px solid var(--color-subtle);">
-    <h2 class="txt-align-center flex-inline center txt-uppercase txt-medium margin-none-block-start margin-block-end">
-      Considering
-    </h2>
-    <% if (unstaged_bubbles = Filter.from_params(@filter.as_params.except(:stage_ids)).tap { |f| f.creator = Current.user }.bubbles.where(stage: nil)).any? %>
-      <%= render partial: "bubbles/card_mini", collection: unstaged_bubbles, as: :bubble, cached: true %>
-    <% else %>
-      <p class="txt-medium translucent">Nothing here</p>
-    <% end %>
-  </section>
-  <section class="cards gap position-relative" style="align-items: center; view-transition-name: cards-container;" data-controller="bubble-size" data-action="turbo:morph@window->bubble-size#resize">
-    <% if workflow = @filter.buckets.first&.workflow %>
-      <div class="card-column__filter flex-inline center position-relative" data-controller="dialog" data-action="keydown.esc->dialog#close click@document->dialog#closeOnClickOutside">
-        <button class="txt-align-center input input--select borderless center txt-uppercase txt-medium margin-none-block-start margin-block-end" data-action="click->dialog#open:stop">
-          <strong class="overflow-ellipsis">
-            <% if @filter.stages.any? %>
-              <%= @filter.stages.map(&:name).to_sentence %>
-            <% else %>
-              Doing
-            <% end %>
-          </strong>
-        </button>
-
-        <dialog class="events__popup popup panel flex-column align-start gap-half fill-white shadow"
-            aria-label="In stage…" aria-description="In stage…"
-            data-dialog-target="dialog" data-action="turbo:before-cache@document->dialog#close">
-          <strong class="popup__title margin-block-start-half pad-inline-half">In stage(s)…</strong>
-          <%= form_with url: bubbles_path, method: :get, class: "flex flex-column full-width popup__list",
-                data: { controller: "form" } do |form| %>
-            <% @filter.as_params.except(:stage_ids).each do |key, value| %>
-              <%= filter_hidden_field_tag key, value %>
-            <% end %>
-
-            <%= link_to bubbles_path(@filter.as_params.except(:stage_ids)), class: "btn popup__item" do %>
-              <span class="overflow-ellipsis">All stages</span>
-            <% end %>
-
-            <% workflow.stages.each do |stage| %>
-              <div class="btn popup__item">
-                <%= form.check_box :stage_ids, {
-                  multiple: true,
-                  checked: @filter.stages.include?(stage),
-                  data: { action: "change->form#submit" },
-                  include_hidden: false,
-                  }, stage.id %>
-
-                <%= form.label :stage_ids, stage.name, for: form.field_id(:stage_ids, stage.id), class: "overflow-ellipsis" %>
-                <%= icon_tag "check", size: 18, class: "checked flex-item-justify-end" %>
-              </div>
-            <% end %>
-          <% end %>
-        </dialog>
-      </div>
-    <% else %>
-      <h2 class="txt-align-center flex-inline center txt-uppercase txt-medium margin-none-block-start margin-block-end">
-        Doing
-      </h2>
-    <% end %>
-
-    <% if (staged_bubbles = @bubbles.where.not(stage: nil)).any? %>
-      <%= render partial: "bubbles/card_preview", collection: staged_bubbles, as: :bubble, cached: true %>
-    <% else %>
-      <p class="txt-medium translucent">Nothing here</p>
-    <% end %>
-  </section>
+  <%= render "bubbles/index/bubbles/considering", bubbles: @considering_bubbles %>
+  <%= render "bubbles/index/bubbles/doing", bubbles: @doing_bubbles, filter: @filter %>
 </div>
 
-<div class="margin-block">
-  <h2 class="txt-align-center flex-inline center txt-uppercase txt-medium margin-none">
-    Recently closed
-  </h2>
-  <section class="cards--grid">
-    <% if (popped_bubbles = Filter.from_params(@filter.as_params.except(:stage_ids).merge(indexed_by: "popped")).tap { |f| f.creator = Current.user }.bubbles).any? %>
-      <%= render partial: "bubbles/card_preview", collection: popped_bubbles, as: :bubble, cached: true %>
-    <% else %>
-      <p class="txt-medium translucent">Nothing here</p>
-    <% end %>
-  </section>
-</div>
+<%= render "bubbles/index/bubbles/popped", bubbles: @popped_bubbles, filter: @filter %>
diff --git a/app/views/bubbles/index/_workflow_filter.html.erb b/app/views/bubbles/index/_workflow_filter.html.erb
new file mode 100644
index 000000000..fd078736e
--- /dev/null
+++ b/app/views/bubbles/index/_workflow_filter.html.erb
@@ -0,0 +1,41 @@
+<div class="card-column__filter flex-inline center position-relative" data-controller="dialog" data-action="keydown.esc->dialog#close click@document->dialog#closeOnClickOutside">
+  <button class="txt-align-center input input--select borderless center txt-uppercase txt-medium margin-none-block-start margin-block-end" data-action="click->dialog#open:stop">
+    <strong class="overflow-ellipsis">
+      <% if filter.stages.any? %>
+        <%= filter.stages.map(&:name).to_sentence %>
+      <% else %>
+        Doing
+      <% end %>
+    </strong>
+  </button>
+
+  <dialog class="events__popup popup panel flex-column align-start gap-half fill-white shadow"
+    aria-label="In stage…" aria-description="In stage…"
+    data-dialog-target="dialog" data-action="turbo:before-cache@document->dialog#close">
+    <strong class="popup__title margin-block-start-half pad-inline-half">In stage(s)…</strong>
+    <%= form_with url: bubbles_path, method: :get, class: "flex flex-column full-width popup__list",
+          data: { controller: "form" } do |form| %>
+      <% filter.as_params.except(:stage_ids).each do |key, value| %>
+        <%= filter_hidden_field_tag key, value %>
+      <% end %>
+
+      <%= link_to bubbles_path(filter.as_params.except(:stage_ids)), class: "btn popup__item" do %>
+        <span class="overflow-ellipsis">All stages</span>
+      <% end %>
+
+      <% workflow.stages.each do |stage| %>
+        <div class="btn popup__item">
+          <%= form.check_box :stage_ids, {
+                multiple: true,
+                checked: filter.stages.include?(stage),
+                data: { action: "change->form#submit" },
+                include_hidden: false,
+          }, stage.id %>
+
+          <%= form.label :stage_ids, stage.name, for: form.field_id(:stage_ids, stage.id), class: "overflow-ellipsis" %>
+          <%= icon_tag "check", size: 18, class: "checked flex-item-justify-end" %>
+        </div>
+      <% end %>
+    <% end %>
+  </dialog>
+</div>
diff --git a/app/views/bubbles/index/bubbles/_considering.html.erb b/app/views/bubbles/index/bubbles/_considering.html.erb
new file mode 100644
index 000000000..8a119b420
--- /dev/null
+++ b/app/views/bubbles/index/bubbles/_considering.html.erb
@@ -0,0 +1,10 @@
+<section class="cards" style="align-items: center; border-inline-end: 1px solid var(--color-subtle);">
+  <h2 class="txt-align-center flex-inline center txt-uppercase txt-medium margin-none-block-start margin-block-end">
+    Considering
+  </h2>
+  <% if bubbles.any? %>
+    <%= render partial: "bubbles/card_mini", collection: bubbles, as: :bubble, cached: true %>
+  <% else %>
+    <p class="txt-medium translucent">Nothing here</p>
+  <% end %>
+</section>
diff --git a/app/views/bubbles/index/bubbles/_doing.html.erb b/app/views/bubbles/index/bubbles/_doing.html.erb
new file mode 100644
index 000000000..bae9b50c5
--- /dev/null
+++ b/app/views/bubbles/index/bubbles/_doing.html.erb
@@ -0,0 +1,15 @@
+<section class="cards gap position-relative" style="align-items: center; view-transition-name: cards-container;" data-controller="bubble-size" data-action="turbo:morph@window->bubble-size#resize">
+  <% if workflow = filter.single_workflow %>
+    <%= render "bubbles/index/workflow_filter", workflow: workflow, filter: filter %>
+  <% else %>
+    <h2 class="txt-align-center flex-inline center txt-uppercase txt-medium margin-none-block-start margin-block-end">
+      Doing
+    </h2>
+  <% end %>
+
+  <% if bubbles.any? %>
+    <%= render partial: "bubbles/card_preview", collection: bubbles, as: :bubble, cached: true %>
+  <% else %>
+    <p class="txt-medium translucent">Nothing here</p>
+  <% end %>
+</section>
diff --git a/app/views/bubbles/index/bubbles/_popped.html.erb b/app/views/bubbles/index/bubbles/_popped.html.erb
new file mode 100644
index 000000000..d31bf2500
--- /dev/null
+++ b/app/views/bubbles/index/bubbles/_popped.html.erb
@@ -0,0 +1,12 @@
+<div class="margin-block">
+  <h2 class="txt-align-center flex-inline center txt-uppercase txt-medium margin-none">
+    Recently closed
+  </h2>
+  <section class="cards--grid">
+    <% if bubbles.any? %>
+      <%= render partial: "bubbles/card_preview", collection: bubbles, as: :bubble, cached: true %>
+    <% else %>
+      <p class="txt-medium translucent">Nothing here</p>
+    <% end %>
+  </section>
+</div>