From e9a085f8488f041c1b2bab153f4cd384df0c962f Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Wed, 15 Jan 2025 16:27:33 -0600 Subject: [PATCH 1/5] Simplify filters --- app/controllers/bubbles_controller.rb | 2 +- app/controllers/filters_controller.rb | 4 +- app/models/filter.rb | 16 +++--- app/models/filter/fields.rb | 23 +++++---- app/models/filter/params.rb | 50 ++++++++++-------- app/models/filter/resources.rb | 2 +- app/models/filter/summarized.rb | 6 +-- app/views/bubbles/_filters.html.erb | 30 +++++------ app/views/bubbles/index.html.erb | 4 +- app/views/bubbles/list/_bubble.html.erb | 2 +- app/views/filters/_filter.html.erb | 4 +- test/controllers/bubbles_controller_test.rb | 2 +- test/controllers/filters_controller_test.rb | 6 +-- test/models/filter_test.rb | 57 +++++++-------------- 14 files changed, 100 insertions(+), 108 deletions(-) diff --git a/app/controllers/bubbles_controller.rb b/app/controllers/bubbles_controller.rb index 8aa6850d8..8409b53ca 100644 --- a/app/controllers/bubbles_controller.rb +++ b/app/controllers/bubbles_controller.rb @@ -34,7 +34,7 @@ class BubblesController < ApplicationController private def set_filter - @filter = Current.user.filters.build params.permit(*Filter::PERMITTED_PARAMS) + @filter = Current.user.filters.from_params params.permit(*Filter::PERMITTED_PARAMS) end def set_bubble diff --git a/app/controllers/filters_controller.rb b/app/controllers/filters_controller.rb index e39456b18..c0523fa0d 100644 --- a/app/controllers/filters_controller.rb +++ b/app/controllers/filters_controller.rb @@ -3,7 +3,7 @@ class FiltersController < ApplicationController def create @filter = Current.user.filters.remember filter_params - redirect_to bubbles_path(@filter.to_params) + redirect_to bubbles_path(@filter.as_params) end def destroy @@ -24,7 +24,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.as_params) end end end diff --git a/app/models/filter.rb b/app/models/filter.rb index 64fc6d183..9bcf8b485 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -5,10 +5,14 @@ class Filter < ApplicationRecord has_one :account, through: :creator class << self + def from_params(params) + find_by_params(params) || build(params) + end + def remember(attrs) create!(attrs) rescue ActiveRecord::RecordNotUnique - find_by!(params_digest: digest_params(attrs)).tap(&:touch) + find_by_params(attrs).tap(&:touch) end def digest_params(params) @@ -16,15 +20,11 @@ class Filter < ApplicationRecord end end - def empty? - as_params.blank? - end - def bubbles @bubbles ||= begin result = creator.accessible_bubbles.indexed_by(indexed_by) result = result.active unless indexed_by.popped? - result = result.unassigned if assignments.unassigned? + result = result.unassigned if assignment_status.unassigned? result = result.assigned_to(assignees.ids) if assignees.present? result = result.assigned_by(assigners.ids) if assigners.present? result = result.in_bucket(buckets.ids) if buckets.present? @@ -37,6 +37,10 @@ class Filter < ApplicationRecord end end + def empty? + self.class.normalize_params(as_params).blank? + end + def cacheable? buckets.exists? end diff --git a/app/models/filter/fields.rb b/app/models/filter/fields.rb index 93faed6c7..af4dd4311 100644 --- a/app/models/filter/fields.rb +++ b/app/models/filter/fields.rb @@ -3,36 +3,39 @@ module Filter::Fields INDEXES = %w[ most_discussed most_boosted newest oldest popped ] + delegate :default_value?, to: :class + class_methods do def default_fields { indexed_by: "most_active" } end + + def default_value?(key, value) + default_fields[key.to_sym].eql?(value) + end end included do - store_accessor :fields, :indexed_by, :assignments, :terms + store_accessor :fields, :assignment_status, :indexed_by, :terms + + def assignment_status + super.to_s.inquiry + end def indexed_by (super || default_indexed_by).inquiry end - def assignments - super.to_s.inquiry - end - def terms Array(super) end end def default_indexed_by - default_fields[:indexed_by] + self.class.default_fields[:indexed_by] end def default_indexed_by? - indexed_by == default_indexed_by + default_value?(:indexed_by, indexed_by) end - - private - delegate :default_fields, to: :class, private: true end diff --git a/app/models/filter/params.rb b/app/models/filter/params.rb index 8938840eb..91970d5b9 100644 --- a/app/models/filter/params.rb +++ b/app/models/filter/params.rb @@ -1,34 +1,44 @@ module Filter::Params extend ActiveSupport::Concern - PERMITTED_PARAMS = [ :indexed_by, :assignments, bucket_ids: [], assignee_ids: [], assigner_ids: [], tag_ids: [], terms: [] ] + PERMITTED_PARAMS = [ :assignment_status, :indexed_by, assignee_ids: [], assigner_ids: [], bucket_ids: [], tag_ids: [], terms: [] ] + + class_methods do + def find_by_params(params) + find_by params_digest: digest_params(params) + end + + def digest_params(params) + Digest::MD5.hexdigest normalize_params(params).to_json + end + + def normalize_params(params) + params.to_h.compact_blank.reject(&method(:default_value?)).sort + end + end included do before_save { self.params_digest = self.class.digest_params(as_params) } end + # +as_params+ uses `resource#ids` instead of `#resource_ids` + # because the latter won't work on unpersisted filters. def as_params - @as_params ||= { - terms: terms, - tag_ids: tags.ids, - indexed_by: indexed_by, - bucket_ids: buckets.ids, - assignments: assignments, - assignee_ids: assignees.ids, - assigner_ids: assigners.ids - }.reject { |k, v| default_fields[k] == v }.compact_blank + {}.tap do |params| + params[:indexed_by] = indexed_by + params[:assignment_status] = assignment_status + params[:terms] = terms + params[:tag_ids] = tags.ids + params[:bucket_ids] = buckets.ids + params[:assignee_ids] = assignees.ids + params[:assigner_ids] = assigners.ids + end.compact_blank.reject(&method(:default_value?)) end - def to_params - ActionController::Parameters.new(as_params).permit(*PERMITTED_PARAMS).tap do |params| - params[:filter_id] = id if persisted? - end - end - - def params_without(key, value) - to_params.tap do |params| - params[key].delete(value) if params[key].is_a?(Array) - params.delete(key) if params[key] == value + def as_params_without(key, value) + as_params.tap do |params| + params[key].delete(value.to_i) if params[key].is_a?(Array) + params.delete(key) if params[key] == value.to_s end end end diff --git a/app/models/filter/resources.rb b/app/models/filter/resources.rb index bde1a30d1..537c52c35 100644 --- a/app/models/filter/resources.rb +++ b/app/models/filter/resources.rb @@ -17,6 +17,6 @@ module Filter::Resources end def buckets - creator.buckets.where id: bucket_ids + creator.buckets.where id: super.ids end end diff --git a/app/models/filter/summarized.rb b/app/models/filter/summarized.rb index b912967c0..fe65518d4 100644 --- a/app/models/filter/summarized.rb +++ b/app/models/filter/summarized.rb @@ -3,10 +3,6 @@ module Filter::Summarized [ index_summary, tag_summary, assignee_summary, assigner_summary, terms_summary ].compact.to_sentence + " #{bucket_summary}" end - def plain_summary - summary.remove(/<\/?mark>/) - end - private def index_summary indexed_by.humanize @@ -21,7 +17,7 @@ module Filter::Summarized def assignee_summary if assignees.any? "assigned to #{assignees.pluck(:name).to_choice_sentence}" - elsif assignments.unassigned? + elsif assignment_status.unassigned? "assigned to no one" end end diff --git a/app/views/bubbles/_filters.html.erb b/app/views/bubbles/_filters.html.erb index 75f3693b4..1538e22a2 100644 --- a/app/views/bubbles/_filters.html.erb +++ b/app/views/bubbles/_filters.html.erb @@ -17,32 +17,32 @@
- <%= filter_chip_tag filter.indexed_by.humanize, filter.params_without(:indexed_by, filter.indexed_by) unless filter.default_indexed_by? %> + <%= filter_chip_tag filter.indexed_by.humanize, filter.as_params_without(:indexed_by, filter.indexed_by) unless filter.default_indexed_by? %> <% filter.tags.each do |tag| %> - <%= filter_chip_tag tag.hashtag, filter.params_without(:tag_ids, tag.id) %> + <%= filter_chip_tag tag.hashtag, filter.as_params_without(:tag_ids, tag.id) %> <% end %> <% filter.assignees.each do |assignee| %> - <%= filter_chip_tag "for #{assignee.name}", filter.params_without(:assignee_ids, assignee.id) %> + <%= filter_chip_tag "for #{assignee.name}", filter.as_params_without(:assignee_ids, assignee.id) %> <% end %> - <% if filter.assignments.present? %> - <%= filter_chip_tag filter.assignments.humanize, filter.params_without(:assignments, filter.assignments) %> + <% if filter.assignment_status.present? %> + <%= filter_chip_tag filter.assignment_status.humanize, filter.as_params_without(:assignment_status, filter.assignment_status) %> <% end %> <% filter.assigners.each do |assigner| %> - <%= filter_chip_tag "by #{assigner.name}", filter.params_without(:assigner_ids, assigner.id) %> + <%= filter_chip_tag "by #{assigner.name}", filter.as_params_without(:assigner_ids, assigner.id) %> <% end %> <% if filter.buckets.many? %> <% filter.buckets.each do |bucket| %> - <%= filter_chip_tag "in #{bucket.name}", filter.params_without(:bucket_ids, bucket.id) %> + <%= filter_chip_tag "in #{bucket.name}", filter.as_params_without(:bucket_ids, bucket.id) %> <% end %> <% end %> <% filter.terms.each do |term| %> - <%= filter_chip_tag %Q("#{term}"), filter.params_without(:terms, term) %> + <%= filter_chip_tag %Q("#{term}"), filter.as_params_without(:terms, term) %> <% end %>
@@ -67,7 +67,7 @@
<%= form_tag bubbles_path, class: "flex align-center gap input input--actor txt-small fill-white border-radius", method: :get do %> - <% @filter.to_params.each do |key, value| %> + <% @filter.as_params.each do |key, value| %> <%= filter_hidden_field_tag key, value %> <% end %> @@ -140,8 +140,8 @@
  • Assigned to…
  • - <%= label_tag "assignments_unassigned", class: "btn filter__button" do %> - <%= check_box_tag "assignments", "unassigned", filter.assignments.unassigned?, id: "assignments_unassigned", + <%= label_tag "assignment_status_unassigned", class: "btn filter__button" do %> + <%= check_box_tag "assignment_status", "unassigned", filter.assignment_status.unassigned?, id: "assignment_status_unassigned", hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignee_ids[],assigner_ids[]" } %> No one <%= image_tag "close.svg", aria: { hidden: true }, size: 24 %> @@ -150,7 +150,7 @@
  • <%= label_tag "assignee_ids_me", class: "btn filter__button" do %> <%= check_box_tag "assignee_ids[]", Current.user.id, filter.assignees.include?(Current.user), id: "assignee_ids_me", - hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignments" } %> + hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignment_status" } %> Me <%= image_tag "close.svg", aria: { hidden: true }, size: 24 %> <% end %> @@ -159,7 +159,7 @@
  • <%= label_tag "assignee_ids_#{user.id}", user.name, class: "btn filter__button" do %> <%= check_box_tag "assignee_ids[]", user.id, filter.assignees.include?(user), id: "assignee_ids_#{user.id}", - hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignments" } %> + hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignment_status" } %> <%= user.name %> <%= image_tag "close.svg", aria: { hidden: true }, size: 24 %> <% end %> @@ -172,7 +172,7 @@
  • <%= label_tag "assigner_ids_me", class: "btn filter__button" do %> <%= check_box_tag "assigner_ids[]", Current.user.id, filter.assigners.include?(Current.user), id: "assigner_ids_me", - hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignments" } %> + hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignment_status" } %> Me <%= image_tag "close.svg", aria: { hidden: true }, size: 24 %> <% end %> @@ -181,7 +181,7 @@
  • <%= label_tag "assigner_ids_#{user.id}", class: "btn filter__button" do %> <%= check_box_tag "assigner_ids[]", user.id, filter.assigners.include?(user), id: "assigner_ids_#{user.id}", - hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignments" } %> + hidden: true, data: { action: "filter-form#clearCategory", filter_form_name_param: "assignment_status" } %> <%= user.name %> <%= image_tag "close.svg", aria: { hidden: true }, size: 24 %> <% end %> diff --git a/app/views/bubbles/index.html.erb b/app/views/bubbles/index.html.erb index 66ca2ff76..814530d10 100644 --- a/app/views/bubbles/index.html.erb +++ b/app/views/bubbles/index.html.erb @@ -1,4 +1,4 @@ -<% @page_title = @filter.plain_summary %> +<% @page_title = @filter.summary %> <% turbo_refreshes_with method: :morph, scroll: :preserve %> <%= render "filters/broadcasts", filter: @filter %> @@ -25,7 +25,7 @@ <% end %> -
    +
    <% if @bubbles.any? %> <%= render partial: "bubbles/bubble", collection: @bubbles.limit(10), cached: true %> <% else %> diff --git a/app/views/bubbles/list/_bubble.html.erb b/app/views/bubbles/list/_bubble.html.erb index cb514292c..68588b64b 100644 --- a/app/views/bubbles/list/_bubble.html.erb +++ b/app/views/bubbles/list/_bubble.html.erb @@ -16,7 +16,7 @@
    <% bubble.assignees.each do |assignee| %> - <%= link_to bubbles_path(@filter.to_params.merge(assignee_ids: [ assignee.id ])), class: "btn avatar txt-x-small" do %> + <%= link_to bubbles_path(@filter.as_params.merge(assignee_ids: [ assignee.id ])), class: "btn avatar txt-x-small" do %> <%= avatar_image_tag assignee, loading: :lazy %> <% end %> <% end %> diff --git a/app/views/filters/_filter.html.erb b/app/views/filters/_filter.html.erb index b19b749ea..5b782fdf7 100644 --- a/app/views/filters/_filter.html.erb +++ b/app/views/filters/_filter.html.erb @@ -1,6 +1,6 @@ <% cache_if filter.cacheable?, filter do %>
  • - <%= link_to bubbles_path(**filter.to_params), class: "border border-radius margin-block-end-half windshield__container flex justify-center align-center position-relative" do %> + <%= link_to bubbles_path(**filter.as_params), class: "border border-radius margin-block-end-half windshield__container flex justify-center align-center position-relative" do %>
    <% filter.bubbles.ordered_by_activity.limit(10).each do |bubble| %>
    @@ -11,7 +11,7 @@ <% end %>
    - <%= link_to bubbles_path(**filter.to_params), class: "txt-ink" do %> + <%= link_to bubbles_path(**filter.as_params), class: "txt-ink" do %> <%= image_tag "filter.svg", aria: { hidden: true }, size: 30, class: "flex-inline", style: "vertical-align: bottom" %> <%= filter.summary %> <% end %> <%= button_to filter_path(filter), method: :delete, class: "btn txt-small btn--negative bucket__button", data: { turbo_confirm: "Are you sure you want to delete this filter?" } do %> diff --git a/test/controllers/bubbles_controller_test.rb b/test/controllers/bubbles_controller_test.rb index 4f1d0733e..106f53ca2 100644 --- a/test/controllers/bubbles_controller_test.rb +++ b/test/controllers/bubbles_controller_test.rb @@ -11,7 +11,7 @@ class BubblesControllerTest < ActionDispatch::IntegrationTest end test "filtered index" do - get bubbles_url(filters(:jz_assignments).to_params.merge(term: "haggis")) + get bubbles_url(filters(:jz_assignments).as_params.merge(term: "haggis")) assert_response :success end diff --git a/test/controllers/filters_controller_test.rb b/test/controllers/filters_controller_test.rb index 8e422fae2..1562686ea 100644 --- a/test/controllers/filters_controller_test.rb +++ b/test/controllers/filters_controller_test.rb @@ -9,16 +9,16 @@ class FiltersControllerTest < ActionDispatch::IntegrationTest assert_difference "users(:david).filters.count", +1 do post filters_url, params: { indexed_by: "popped", - assignments: "unassigned", + assignment_status: "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) + assert_redirected_to bubbles_path(Filter.last.as_params) filter = Filter.last assert_predicate filter.indexed_by, :popped? - assert_predicate filter.assignments, :unassigned? + assert_predicate filter.assignment_status, :unassigned? assert_equal [ tags(:mobile) ], filter.tags assert_equal [ users(:jz) ], filter.assignees assert_equal [ buckets(:writebook) ], filter.buckets diff --git a/test/models/filter_test.rb b/test/models/filter_test.rb index 51fec13af..762b699a4 100644 --- a/test/models/filter_test.rb +++ b/test/models/filter_test.rb @@ -1,16 +1,6 @@ require "test_helper" class FilterTest < ActiveSupport::TestCase - test "remembering" do - assert_difference "users(:david).filters.count", +1 do - filter = users(:david).filters.remember(indexed_by: "most_boosted", tag_ids: [ tags(:mobile).id ]) - - assert_changes "filter.reload.updated_at" do - assert_equal filter, users(:david).filters.remember(indexed_by: "most_boosted", tag_ids: [ tags(:mobile).id ]) - end - end - end - test "bubbles" do Current.set session: sessions(:david) do @new_bucket = accounts("37s").buckets.create! name: "Inaccessible Bucket" @@ -28,7 +18,7 @@ class FilterTest < ActiveSupport::TestCase filter = users(:david).filters.new assigner_ids: [ users(:david).id ], tag_ids: [ tags(:mobile).id ] assert_equal [ bubbles(:layout) ], filter.bubbles - filter = users(:david).filters.new assignments: "unassigned", bucket_ids: [ @new_bucket.id ] + filter = users(:david).filters.new assignment_status: "unassigned", bucket_ids: [ @new_bucket.id ] assert_equal [ @new_bubble ], filter.bubbles filter = users(:david).filters.new terms: [ "haggis" ] @@ -48,37 +38,30 @@ class FilterTest < ActiveSupport::TestCase assert_empty users(:david).filters.new(bucket_ids: [ buckets(:writebook).id ]).bubbles 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 + test "remembering equivalent filters" do + assert_difference "users(:david).filters.count", +1 do + filter = users(:david).filters.remember(indexed_by: "most_active", assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ]) + + assert_changes "filter.reload.updated_at" do + assert_equal filter, users(:david).filters.remember(assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ]) + end + end end - test "param sanitization" do + test "turning into params" do filter = users(:david).filters.new indexed_by: "most_active", tag_ids: "", assignee_ids: [ users(:jz).id ], bucket_ids: [ buckets(:writebook).id ] expected = { assignee_ids: [ users(:jz).id ], bucket_ids: [ buckets(:writebook).id ] } assert_equal expected, filter.as_params end - test "cacheable" do + test "cacheability" do assert_not filters(:jz_assignments).cacheable? assert users(:david).filters.create!(bucket_ids: [ buckets(:writebook).id ]).cacheable? end - test "default fields" do - assert_equal "most_active", users(:david).filters.new.indexed_by - end - - test "indexed by" do - assert_predicate users(:david).filters.new(indexed_by: "most_discussed").indexed_by, :most_discussed? - end - - test "assignments" do - assert_predicate users(:david).filters.new(assignments: "unassigned").assignments, :unassigned? - end - test "terms" do - assert_empty users(:david).filters.new.terms - assert_includes users(:david).filters.new(terms: [ "haggis" ]).terms, "haggis" + assert_equal [], users(:david).filters.new.terms + assert_equal [ "haggis" ], users(:david).filters.new(terms: [ "haggis" ]).terms end test "resource removal" do @@ -92,7 +75,7 @@ class FilterTest < ActiveSupport::TestCase assert_changes "filter.reload.updated_at" do tags(:mobile).destroy! end - assert_nil Filter.find(filter.id).as_params["tag_ids"] # can't reload because as_params is memoized + assert_nil filter.reload.as_params[:tag_ids] assert_changes "Filter.exists?(filter.id)" do buckets(:writebook).destroy! @@ -112,23 +95,19 @@ class FilterTest < ActiveSupport::TestCase 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 - test "params without a key-value pair" do filter = users(:david).filters.new indexed_by: "most_discussed", assignee_ids: [ users(:jz).id, users(:kevin).id ] expected = { indexed_by: "most_discussed", assignee_ids: [ users(:kevin).id ] } - assert_equal expected.stringify_keys, filter.params_without(:assignee_ids, users(:jz).id).to_h + assert_equal expected, filter.as_params_without(:assignee_ids, users(:jz).id).to_h expected = { assignee_ids: [ users(:jz).id, users(:kevin).id ] } - assert_equal expected.stringify_keys, filter.params_without(:indexed_by, "most_discussed").to_h + assert_equal expected, filter.as_params_without(:indexed_by, "most_discussed").to_h expected = { indexed_by: "most_discussed", assignee_ids: [ users(:jz).id, users(:kevin).id ] } - assert_equal expected.stringify_keys, filter.params_without(:indexed_by, "most_active").to_h + assert_equal expected, filter.as_params_without(:indexed_by, "most_active").to_h expected = { indexed_by: "most_discussed", assignee_ids: [ users(:jz).id, users(:kevin).id ] } - assert_equal expected.stringify_keys, filter.params_without(:assignee_ids, users(:david).id).to_h + assert_equal expected, filter.as_params_without(:assignee_ids, users(:david).id).to_h end end From 19874dcc2aa8033c356e685ba1439034b2de0366 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Wed, 15 Jan 2025 16:41:25 -0600 Subject: [PATCH 2/5] Test creating an equivalent filter with a different user --- test/models/filter_test.rb | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/test/models/filter_test.rb b/test/models/filter_test.rb index 762b699a4..5f7722384 100644 --- a/test/models/filter_test.rb +++ b/test/models/filter_test.rb @@ -39,7 +39,7 @@ class FilterTest < ActiveSupport::TestCase end test "remembering equivalent filters" do - assert_difference "users(:david).filters.count", +1 do + assert_difference "Filter.count", +1 do filter = users(:david).filters.remember(indexed_by: "most_active", assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ]) assert_changes "filter.reload.updated_at" do @@ -48,6 +48,13 @@ class FilterTest < ActiveSupport::TestCase end end + test "remembering equivalent filters for different users" do + assert_difference "Filter.count", +2 do + users(:david).filters.remember(assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ]) + users(:kevin).filters.remember(assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ]) + end + end + test "turning into params" do filter = users(:david).filters.new indexed_by: "most_active", tag_ids: "", assignee_ids: [ users(:jz).id ], bucket_ids: [ buckets(:writebook).id ] expected = { assignee_ids: [ users(:jz).id ], bucket_ids: [ buckets(:writebook).id ] } From 8d7542943188d6ad46861f0e1de3de215e304ccd Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Wed, 15 Jan 2025 16:45:15 -0600 Subject: [PATCH 3/5] Remove old digest_params method --- app/models/filter.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/filter.rb b/app/models/filter.rb index 9bcf8b485..73b03c9c4 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -14,10 +14,6 @@ class Filter < ApplicationRecord rescue ActiveRecord::RecordNotUnique find_by_params(attrs).tap(&:touch) end - - def digest_params(params) - Digest::MD5.hexdigest params.to_h.sort.to_json - end end def bubbles From 4441dc7cd84142a080983e5759e565a1db1bcb31 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Wed, 15 Jan 2025 16:55:57 -0600 Subject: [PATCH 4/5] Test passing params in different order --- test/models/filter_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/models/filter_test.rb b/test/models/filter_test.rb index 5f7722384..c653551dd 100644 --- a/test/models/filter_test.rb +++ b/test/models/filter_test.rb @@ -43,7 +43,7 @@ class FilterTest < ActiveSupport::TestCase filter = users(:david).filters.remember(indexed_by: "most_active", assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ]) assert_changes "filter.reload.updated_at" do - assert_equal filter, users(:david).filters.remember(assignment_status: "unassigned", tag_ids: [ tags(:mobile).id ]) + assert_equal filter, users(:david).filters.remember(tag_ids: [ tags(:mobile).id ], assignment_status: "unassigned") end end end From 5ac9063a84132be1b94f88e476d7886eaa72fb62 Mon Sep 17 00:00:00 2001 From: Jose Farias Date: Wed, 15 Jan 2025 16:57:02 -0600 Subject: [PATCH 5/5] default_fields -> default_values --- app/models/filter/fields.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/filter/fields.rb b/app/models/filter/fields.rb index af4dd4311..9c2c212b0 100644 --- a/app/models/filter/fields.rb +++ b/app/models/filter/fields.rb @@ -6,12 +6,12 @@ module Filter::Fields delegate :default_value?, to: :class class_methods do - def default_fields + def default_values { indexed_by: "most_active" } end def default_value?(key, value) - default_fields[key.to_sym].eql?(value) + default_values[key.to_sym].eql?(value) end end @@ -32,7 +32,7 @@ module Filter::Fields end def default_indexed_by - self.class.default_fields[:indexed_by] + self.class.default_values[:indexed_by] end def default_indexed_by?