diff --git a/app/controllers/bubbles_controller.rb b/app/controllers/bubbles_controller.rb index 1b9591008..7765be897 100644 --- a/app/controllers/bubbles_controller.rb +++ b/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 diff --git a/app/controllers/filters_controller.rb b/app/controllers/filters_controller.rb index cc5ee531d..9d9964e52 100644 --- a/app/controllers/filters_controller.rb +++ b/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 diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index 677e43fc0..05caac3d0 100644 --- a/app/helpers/filters_helper.rb +++ b/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 diff --git a/app/models/concerns/filterable.rb b/app/models/concerns/filterable.rb index 81656b42c..b3cb07c46 100644 --- a/app/models/concerns/filterable.rb +++ b/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 diff --git a/app/models/filter.rb b/app/models/filter.rb index 9c012911a..7ff26f981 100644 --- a/app/models/filter.rb +++ b/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 diff --git a/app/models/filter/params.rb b/app/models/filter/params.rb new file mode 100644 index 000000000..df2354ebd --- /dev/null +++ b/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 diff --git a/app/models/filter/resources.rb b/app/models/filter/resources.rb new file mode 100644 index 000000000..6f04ded0a --- /dev/null +++ b/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 diff --git a/app/views/bubbles/_filters.html.erb b/app/views/bubbles/_filters.html.erb index 0903cdda3..2efc451c8 100644 --- a/app/views/bubbles/_filters.html.erb +++ b/app/views/bubbles/_filters.html.erb @@ -1,11 +1,11 @@
- <%= bubble_filters_heading filter do %> +

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

<%= filter.tags.present? ? "Tagged" : "with" %> diff --git a/test/controllers/bubbles_controller_test.rb b/test/controllers/bubbles_controller_test.rb index cc5bc5526..d440a1dec 100644 --- a/test/controllers/bubbles_controller_test.rb +++ b/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 diff --git a/test/controllers/filters_controller_test.rb b/test/controllers/filters_controller_test.rb new file mode 100644 index 000000000..332b11d5e --- /dev/null +++ b/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 diff --git a/test/fixtures/files/.keep b/test/fixtures/files/.keep deleted file mode 100644 index e69de29bb..000000000 diff --git a/test/fixtures/files/moon.jpg b/test/fixtures/files/moon.jpg new file mode 100644 index 000000000..2ef418957 Binary files /dev/null and b/test/fixtures/files/moon.jpg differ diff --git a/test/fixtures/filters.yml b/test/fixtures/filters.yml index 0835844a6..7dfaa219f 100644 --- a/test/fixtures/filters.yml +++ b/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 diff --git a/test/models/bubble_test.rb b/test/models/bubble_test.rb index b2ab21c1b..11e50d926 100644 --- a/test/models/bubble_test.rb +++ b/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 diff --git a/test/models/filter_test.rb b/test/models/filter_test.rb new file mode 100644 index 000000000..f66466235 --- /dev/null +++ b/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 "Most discussed, tagged #Mobile, and assigned to JZ in all projects", 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