Add tests

app/controllers/bubbles_controller.rb

@@ -11,10 +11,6 @@ class BubblesController < ApplicationController
     @bubbles = @bubbles.mentioning(params[:term]) if params[:term].present?
   end
 
-  def new
-    @bubble = @bucket.bubbles.build
-  end
-
   def create
     @bubble = @bucket.bubbles.create!
     redirect_to @bubble

app/controllers/filters_controller.rb

@@ -1,8 +1,8 @@
 class FiltersController < ApplicationController
-  before_action :set_filter, only: :destroy
+  before_action :set_filter, :remember_params, only: :destroy
 
   def create
-    @filter = Current.user.filters.create_or_find_by_params!(filter_params).tap(&:touch)
+    @filter = Current.user.filters.idempotent_create!(filter_params).tap(&:touch)
     redirect_to bubbles_path(@filter.to_params)
   end
 
@@ -16,6 +16,10 @@ class FiltersController < ApplicationController
       @filter = Current.user.filters.find params[:id]
     end
 
+    def remember_params
+      @filter_params = @filter.to_params
+    end
+
     def filter_params
       params.permit(*Filter::KNOWN_PARAMS).compact_blank
     end
@@ -24,7 +28,7 @@ class FiltersController < ApplicationController
       if request.referer == root_url
         redirect_to root_path
       else
-        redirect_to bubbles_path(@filter.to_params)
+        redirect_to bubbles_path(@filter_params)
       end
     end
 end

app/helpers/filters_helper.rb

@@ -1,9 +1,4 @@
 module FiltersHelper
-  def bubble_filters_heading(filter, &)
-    tag.h1 class: "txt-large flex align-center gap-half",
-      style: token_list("margin-inline-end: calc(var(--btn-size) / -2);": filter.savable?), &
-  end
-
   def buckets_filter_text(filter)
     if filter.buckets.present?
       filter.buckets.map(&:name).to_choice_sentence

app/models/concerns/filterable.rb

@@ -10,8 +10,6 @@ module Filterable
 
   private
     def remove_from_filters
-      filters.each do |filter|
-        filter.resource_removed kind: self.class.name.downcase, id: id
-      end
+      filters.each { |filter| filter.resource_removed self }
     end
 end

app/models/filter.rb

@@ -1,34 +1,17 @@
 class Filter < ApplicationRecord
-  include Summarized
-
-  KNOWN_PARAMS = [ :indexed_by, :assignments, bucket_ids: [], assignee_ids: [], tag_ids: [] ]
-  INDEXES = %w[ most_active most_discussed most_boosted newest oldest popped ]
+  include Params, Resources, Summarized
 
   belongs_to :creator, class_name: "User", default: -> { Current.user }
 
   has_one :account, through: :creator
 
-  has_and_belongs_to_many :tags
-  has_and_belongs_to_many :buckets
-  has_and_belongs_to_many :assignees, class_name: "User", join_table: "assignees_filters", association_foreign_key: "assignee_id"
-
-  before_validation :denormalize_resource_ids
-  before_validation :remove_default_params
-
-  store_accessor :params, :indexed_by
-  store_accessor :params, :assignments
-
   class << self
-    def default_params
-      { "indexed_by" => "most_active" }
-    end
-
-    def create_or_find_by_params!(params)
-      filter = new params
+    def idempotent_create!(attrs)
+      filter = new(attrs)
       filter.save!
       filter
     rescue ActiveRecord::RecordNotUnique
-      find_by! params: filter.params
+      find_by! params: filter.params # possible thanks to denormalized params
     end
   end
 
@@ -37,33 +20,13 @@ class Filter < ApplicationRecord
       result = creator.accessible_bubbles.indexed_by(indexed_by)
       result = result.active unless indexed_by.popped?
       result = result.unassigned if assignments.unassigned?
+      result = result.assigned_to(assignees.ids) if assignees.present?
       result = result.in_bucket(buckets.ids) if buckets.present?
       result = result.tagged_with(tags.ids) if tags.present?
-      result = result.assigned_to(assignees.ids) if assignees.present?
       result
     end
   end
 
-  def to_params
-    params.merge(tag_ids: tags.ids, assignee_ids: assignees.ids, bucket_ids: buckets.ids).then do |params|
-      ActionController::Parameters.new(params).permit(*KNOWN_PARAMS).tap do |params|
-        params[:filter_id] = id if persisted?
-      end
-    end
-  end
-
-  def indexed_by
-    (params["indexed_by"] || self.class.default_params["indexed_by"]).inquiry
-  end
-
-  def assignments
-    params["assignments"].to_s.inquiry
-  end
-
-  def savable?
-    !bucket_default?
-  end
-
   def cacheable?
     buckets.exists?
   end
@@ -71,29 +34,4 @@ class Filter < ApplicationRecord
   def cache_key
     ActiveSupport::Cache.expand_cache_key buckets.cache_key_with_version, super
   end
-
-  def resource_removed(kind:, id:)
-    params["#{kind}_ids"] = Array(params["#{kind}_ids"]).without(id).presence
-    non_default_params.blank? ? destroy : touch
-  end
-
-  private
-    def remove_default_params
-      self[:params] = non_default_params.compact_blank
-    end
-
-    def non_default_params
-      params.reject { |k, v| self.class.default_params[k] == v }
-    end
-
-    # `denormalize_resource_ids` stores ids in the params column to enforce uniqueness
-    def denormalize_resource_ids
-      params[:bucket_ids] = buckets.ids.presence
-      params[:tag_ids] = tags.ids.presence
-      params[:assignee_ids] = assignees.ids.presence
-    end
-
-    def bucket_default?
-      non_default_params.keys == %w[ bucket_ids ] && buckets.one?
-    end
 end

app/models/filter/params.rb

@@ -0,0 +1,51 @@
+module Filter::Params
+  extend ActiveSupport::Concern
+
+  KNOWN_PARAMS = [ :indexed_by, :assignments, bucket_ids: [], assignee_ids: [], tag_ids: [] ]
+  INDEXES = %w[ most_active most_discussed most_boosted newest oldest popped ]
+
+  class_methods do
+    def default_params
+      { "indexed_by" => "most_active" }
+    end
+  end
+
+  included do
+    before_validation :sanitize_params
+  end
+
+  def to_params
+    params.merge(tag_ids: tags.ids.presence, assignee_ids: assignees.ids.presence, bucket_ids: buckets.ids.presence).then do |params|
+      ActionController::Parameters.new(params).permit(*KNOWN_PARAMS).tap do |params|
+        params[:filter_id] = id if persisted?
+      end
+    end
+  end
+
+  def assignments=(value)
+    params["assignments"] = value
+  end
+
+  def assignments
+    params["assignments"].to_s.inquiry
+  end
+
+  def indexed_by=(value)
+    params["indexed_by"] = value
+  end
+
+  def indexed_by
+    (params["indexed_by"] || self.class.default_params["indexed_by"]).inquiry
+  end
+
+  private
+    def sanitize_params
+      denormalize_resource_ids
+      self.params = non_default_params
+      params.compact_blank!
+    end
+
+    def non_default_params
+      params.reject { |k, v| self.class.default_params[k] == v }
+    end
+end

app/models/filter/resources.rb

@@ -0,0 +1,24 @@
+module Filter::Resources
+  extend ActiveSupport::Concern
+
+  included do
+    has_and_belongs_to_many :tags
+    has_and_belongs_to_many :buckets
+    has_and_belongs_to_many :assignees, class_name: "User", join_table: "assignees_filters", association_foreign_key: "assignee_id"
+  end
+
+  def resource_removed(resource)
+    kind = resource.class.model_name.plural
+    send("#{kind}=", send(kind).without(resource))
+    sanitize_params
+    params.blank? ? destroy! : save!
+  end
+
+  private
+    # `denormalize_resource_ids` stores ids in the params column to enforce uniqueness with a db constraint.
+    def denormalize_resource_ids
+      params["bucket_ids"] = buckets.ids
+      params["tag_ids"] = tags.ids
+      params["assignee_ids"] = assignees.ids
+    end
+end

app/views/bubbles/_filters.html.erb

@@ -1,11 +1,11 @@
 <div class="flex flex-column gap-half align-center">
-  <%= bubble_filters_heading filter do %>
+  <h1 class="txt-large flex align-center gap-half">
     <%= render "bubbles/filters/index", filter: filter %>
     in
     <%= render "bubbles/filters/buckets", filter: filter %>
 
-    <%= render "bubbles/filters/bookmark", filter: filter if filter.savable? %>
-  <% end %>
+    <%= render "bubbles/filters/bookmark", filter: filter %>
+  </h1>
 
   <h2 class="txt-medium flex align-center justify-center gap-half">
     <%= filter.tags.present? ? "Tagged" : "with" %>

test/controllers/bubbles_controller_test.rb

@@ -1,7 +1,52 @@
 require "test_helper"
 
 class BubblesControllerTest < ActionDispatch::IntegrationTest
-  # test "the truth" do
-  #   assert true
-  # end
+  setup do
+    sign_in_as :kevin
+  end
+
+  test "index" do
+    get bubbles_url
+    assert_response :success
+  end
+
+  test "filtered index" do
+    get bubbles_url(filters(:jz_assignments).to_params.merge(term: "haggis"))
+    assert_response :success
+  end
+
+  test "create" do
+    assert_difference "Bubble.count", 1 do
+      post bucket_bubbles_url(buckets(:writebook))
+    end
+    assert_redirected_to bucket_bubble_url(buckets(:writebook), Bubble.last)
+  end
+
+  test "show" do
+    get bucket_bubble_url(buckets(:writebook), bubbles(:logo))
+    assert_response :success
+  end
+
+  test "edit" do
+    get edit_bucket_bubble_url(buckets(:writebook), bubbles(:logo))
+    assert_response :success
+  end
+
+  test "update" do
+    patch bucket_bubble_url(buckets(:writebook), bubbles(:logo)), params: {
+      bubble: {
+        title: "Logo needs to change",
+        color: "#000000",
+        due_on: 1.week.from_now,
+        image: fixture_file_upload("moon.jpg", "image/jpeg"),
+        tag_ids: [ tags(:mobile).id ] } }
+    assert_redirected_to bucket_bubble_url(buckets(:writebook), bubbles(:logo))
+
+    bubble = bubbles(:logo).reload
+    assert_equal "Logo needs to change", bubble.title
+    assert_equal "#000000", bubble.color
+    assert_equal 1.week.from_now.to_date, bubble.due_on
+    assert_equal "moon.jpg", bubble.image.filename.to_s
+    assert_equal [ tags(:mobile) ], bubble.tags
+  end
 end

test/controllers/filters_controller_test.rb

@@ -0,0 +1,34 @@
+require "test_helper"
+
+class FiltersControllerTest < ActionDispatch::IntegrationTest
+  setup do
+    sign_in_as :david
+  end
+
+  test "create" do
+    assert_difference "Filter.count", +1 do
+      post filters_url, params: {
+        indexed_by: "popped",
+        assignments: "unassigned",
+        tag_ids: [ tags(:mobile).id ],
+        assignee_ids: [ users(:jz).id ],
+        bucket_ids: [ buckets(:writebook).id ] }
+    end
+    assert_redirected_to bubbles_path(Filter.last.to_params)
+
+    filter = Filter.last
+    assert_predicate filter.indexed_by, :popped?
+    assert_predicate filter.assignments, :unassigned?
+    assert_equal [ tags(:mobile) ], filter.tags
+    assert_equal [ users(:jz) ], filter.assignees
+    assert_equal [ buckets(:writebook) ], filter.buckets
+  end
+
+  test "destroy" do
+    params = filters(:jz_assignments).to_params
+    assert_difference "Filter.count", -1 do
+      delete filter_url(filters(:jz_assignments))
+    end
+    assert_redirected_to bubbles_path(params)
+  end
+end

test/fixtures/filters.yml

@@ -1,5 +1,5 @@
 jz_assignments:
   creator: david
-  params: <%= { indexed_by: :most_discussed }.to_json %>
+  params: <%= { indexed_by: :most_discussed, tag_ids: [ ActiveRecord::FixtureSet.identify(:mobile) ], assignee_ids: [ ActiveRecord::FixtureSet.identify(:jz) ] }.to_json %>
   tags: mobile
   assignees: jz

test/models/bubble_test.rb

@@ -34,11 +34,42 @@ class BubbleTest < ActiveSupport::TestCase
 
   test "ordering by activity" do
     bubbles(:layout).update! boost_count: 1_000
-    assert_equal bubbles(:layout, :logo, :shipping, :text), Bubble.ordered_by_activity.to_a
+    assert_equal bubbles(:layout, :logo, :shipping, :text), Bubble.ordered_by_activity
   end
 
   test "ordering by comments" do
-    assert_equal bubbles(:logo, :layout, :shipping, :text), Bubble.ordered_by_comments.to_a
+    assert_equal bubbles(:logo, :layout, :shipping, :text), Bubble.ordered_by_comments
+  end
+
+  test "ordering by boosts" do
+    bubbles(:layout).update! boost_count: 1_000
+    assert_equal bubbles(:layout, :logo, :shipping, :text), Bubble.ordered_by_boosts
+  end
+
+  test "popped" do
+    assert_equal [ bubbles(:shipping) ], Bubble.popped
+  end
+
+  test "active" do
+    assert_equal bubbles(:logo, :layout, :text), Bubble.active
+  end
+
+  test "unassigned" do
+    assert_equal bubbles(:shipping, :text), Bubble.unassigned
+  end
+
+  test "assigned to" do
+    assert_equal bubbles(:logo, :layout), Bubble.assigned_to(users(:jz))
+  end
+
+  test "in bucket" do
+    new_bucket = accounts("37s").buckets.create! name: "New Bucket", creator: users(:david)
+    assert_equal bubbles(:logo, :shipping, :layout, :text), Bubble.in_bucket(buckets(:writebook))
+    assert_empty Bubble.in_bucket(new_bucket)
+  end
+
+  test "tagged with" do
+    assert_equal bubbles(:layout, :text), Bubble.tagged_with(tags(:mobile))
   end
 
   test "mentioning" do

test/models/filter_test.rb

@@ -0,0 +1,51 @@
+require "test_helper"
+
+class FilterTest < ActiveSupport::TestCase
+  test "idempotent creation" do
+    assert_difference "Filter.count", +1 do
+      filter = users(:david).filters.idempotent_create!(indexed_by: "most_boosted")
+      assert_equal filter, users(:david).filters.idempotent_create!(indexed_by: "most_boosted")
+    end
+  end
+
+  test "bubbles" do
+    Current.set session: sessions(:david) do
+      @inaccessible_bucket = accounts("37s").buckets.create! name: "Inaccessible Bucket"
+      @inaccessible_bubble = @inaccessible_bucket.bubbles.create!
+    end
+
+    assert_not_includes users(:kevin).filters.new.bubbles, @inaccessible_bubble
+  end
+
+  test "turning into params" do
+    expected = { indexed_by: "most_discussed", tag_ids: [ tags(:mobile).id ], assignee_ids: [ users(:jz).id ], filter_id: filters(:jz_assignments).id }
+    assert_equal expected.stringify_keys, filters(:jz_assignments).to_params.to_h
+  end
+
+  test "cacheable" do
+    assert_not filters(:jz_assignments).cacheable?
+    assert users(:david).filters.create!(bucket_ids: [ buckets(:writebook).id ]).cacheable?
+  end
+
+  test "resource removal" do
+    filter = users(:david).filters.create! tag_ids: [ tags(:mobile).id ], bucket_ids: [ buckets(:writebook).id ]
+    assert_includes filter.tags, tags(:mobile)
+    assert_includes filter.buckets, buckets(:writebook)
+
+    assert_changes "filter.reload.updated_at" do
+      tags(:mobile).destroy!
+    end
+
+    assert_changes "Filter.exists?(filter.id)" do
+      buckets(:writebook).destroy!
+    end
+  end
+
+  test "summary" do
+    assert_equal "<mark>Most discussed</mark>, tagged <mark>#Mobile</mark>, and assigned to <mark>JZ</mark> in <mark>all projects</mark>", filters(:jz_assignments).summary
+  end
+
+  test "plain summary" do
+    assert_equal "Most discussed, tagged #Mobile, and assigned to JZ in all projects", filters(:jz_assignments).plain_summary
+  end
+end