diff --git a/app/controllers/bubbles_controller.rb b/app/controllers/bubbles_controller.rb index 6d9306e7a..1b9591008 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: params.permit(*Filter::KNOWN_PARAMS) + @filter = Current.user.filters.build params.permit(*Filter::KNOWN_PARAMS) end def set_bubble diff --git a/app/controllers/buckets_controller.rb b/app/controllers/buckets_controller.rb index 8320b0735..320882381 100644 --- a/app/controllers/buckets_controller.rb +++ b/app/controllers/buckets_controller.rb @@ -12,7 +12,7 @@ class BucketsController < ApplicationController def create @bucket = Current.account.buckets.create! bucket_params - redirect_to bubbles_path(bucket_ids: @bucket) + redirect_to bubbles_path(bucket_ids: [ @bucket ]) end def edit @@ -24,7 +24,7 @@ class BucketsController < ApplicationController @bucket.update! bucket_params @bucket.accesses.revise granted: grantees, revoked: revokees - redirect_to bubbles_path(bucket_ids: @bucket) + redirect_to bubbles_path(bucket_ids: [ @bucket ]) end def destroy diff --git a/app/controllers/filters_controller.rb b/app/controllers/filters_controller.rb index 7683ce0c5..cc5ee531d 100644 --- a/app/controllers/filters_controller.rb +++ b/app/controllers/filters_controller.rb @@ -2,7 +2,7 @@ class FiltersController < ApplicationController before_action :set_filter, only: :destroy def create - @filter = Current.user.filters.create_or_find_by!(params: filter_params).tap(&:touch) + @filter = Current.user.filters.create_or_find_by_params!(filter_params).tap(&:touch) redirect_to bubbles_path(@filter.to_params) end diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index e4e02db13..677e43fc0 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -5,8 +5,8 @@ module FiltersHelper end def buckets_filter_text(filter) - if filter.buckets.any? - filter.buckets.pluck(:name).to_choice_sentence + if filter.buckets.present? + filter.buckets.map(&:name).to_choice_sentence else "all projects" end @@ -14,7 +14,7 @@ module FiltersHelper def assignments_filter_text(filter) if filter.assignees.present? - "assigned to #{filter.assignees.pluck(:name).to_choice_sentence}" + "assigned to #{filter.assignees.map(&:name).to_choice_sentence}" elsif filter.assignments.unassigned? "assigned to no one" else diff --git a/app/models/bucket.rb b/app/models/bucket.rb index f2476b761..3f986e0fb 100644 --- a/app/models/bucket.rb +++ b/app/models/bucket.rb @@ -1,4 +1,5 @@ class Bucket < ApplicationRecord + include Filterable include Accessible belongs_to :account diff --git a/app/models/concerns/filterable.rb b/app/models/concerns/filterable.rb new file mode 100644 index 000000000..81656b42c --- /dev/null +++ b/app/models/concerns/filterable.rb @@ -0,0 +1,17 @@ +module Filterable + extend ActiveSupport::Concern + + included do + has_and_belongs_to_many :filters + + after_update { filters.touch_all } + before_destroy :remove_from_filters + end + + private + def remove_from_filters + filters.each do |filter| + filter.resource_removed kind: self.class.name.downcase, id: id + end + end +end diff --git a/app/models/filter.rb b/app/models/filter.rb index b20a47d03..9c012911a 100644 --- a/app/models/filter.rb +++ b/app/models/filter.rb @@ -1,35 +1,65 @@ class Filter < ApplicationRecord - include Assignments, Buckets, Indexes, Summarized, Tags + include Summarized - KNOWN_PARAMS = %i[ indexed_by bucket_ids assignments tag_ids ] + KNOWN_PARAMS = [ :indexed_by, :assignments, bucket_ids: [], assignee_ids: [], tag_ids: [] ] + INDEXES = %w[ most_active most_discussed most_boosted newest oldest popped ] 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 + filter.save! + filter + rescue ActiveRecord::RecordNotUnique + find_by! params: filter.params + end end def bubbles @bubbles ||= begin result = creator.accessible_bubbles.indexed_by(indexed_by) result = result.active unless indexed_by.popped? - result = result.in_bucket(buckets) if buckets.present? - result = result.tagged_with(tags) if tags.present? result = result.unassigned if assignments.unassigned? - result = result.assigned_to(assignees) 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 - ActionController::Parameters.new(params).permit(*KNOWN_PARAMS).tap do |params| - params[:filter_id] = id if persisted? + 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 @@ -42,12 +72,28 @@ class Filter < ApplicationRecord 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 bucket_default? - non_default_params.keys == %w[ bucket_ids ] && buckets.one? + 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/assignments.rb b/app/models/filter/assignments.rb deleted file mode 100644 index 43bcb90ad..000000000 --- a/app/models/filter/assignments.rb +++ /dev/null @@ -1,9 +0,0 @@ -module Filter::Assignments - def assignments - params["assignments"].to_s.inquiry - end - - def assignees - @assignees ||= account.users.where id: assignments.split(",") - end -end diff --git a/app/models/filter/buckets.rb b/app/models/filter/buckets.rb deleted file mode 100644 index e444c8d1a..000000000 --- a/app/models/filter/buckets.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Filter::Buckets - extend ActiveSupport::Concern - - included do - store_accessor :params, :bucket_ids - end - - def buckets - @buckets ||= account.buckets.where id: bucket_ids.to_s.split(",") - end -end diff --git a/app/models/filter/indexes.rb b/app/models/filter/indexes.rb deleted file mode 100644 index 746b8dec1..000000000 --- a/app/models/filter/indexes.rb +++ /dev/null @@ -1,7 +0,0 @@ -module Filter::Indexes - INDEXES = %w[ most_active most_discussed most_boosted newest oldest popped ] - - def indexed_by - (params["indexed_by"] || self.class.default_params["indexed_by"]).inquiry - end -end diff --git a/app/models/filter/tags.rb b/app/models/filter/tags.rb deleted file mode 100644 index 4b457b461..000000000 --- a/app/models/filter/tags.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Filter::Tags - extend ActiveSupport::Concern - - included do - store_accessor :params, :tag_ids - end - - def tags - @tags ||= account.tags.where id: tag_ids.to_s.split(",") - end -end diff --git a/app/models/tag.rb b/app/models/tag.rb index fb8db226d..c4cd4209e 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -1,4 +1,6 @@ class Tag < ApplicationRecord + include Filterable + belongs_to :account has_many :taggings, dependent: :destroy diff --git a/app/views/bubbles/filters/_assignments.html.erb b/app/views/bubbles/filters/_assignments.html.erb index f3a4df950..fb893ebb4 100644 --- a/app/views/bubbles/filters/_assignments.html.erb +++ b/app/views/bubbles/filters/_assignments.html.erb @@ -5,16 +5,16 @@ -
  • <%= link_to "No one", bubbles_path(filter.to_params.merge(assignments: :unassigned)), class: "filter__button" %>
    • +
  • <%= link_to "No one", bubbles_path(filter.to_params.merge(assignments: :unassigned, assignee_ids: [])), class: "filter__button" %>
    • <% Current.account.users.active.order(:name).each do |user| %> -
  • <%= link_to user.name, bubbles_path(filter.to_params.merge(assignments: user.id)) %>
    • +
  • <%= link_to user.name, bubbles_path(filter.to_params.merge(assignments: nil, assignee_ids: [ user.id ])) %>
    • <% end %>     - <% if filter.assignments.present? %> - <%= link_to bubbles_path(filter.to_params.without(:assignments)), class: "btn", style: "font-size: 0.4em;" do %> + <% if [ filter.assignees, filter.assignments ].any?(&:present?) %> + <%= link_to bubbles_path(filter.to_params.without(:assignments, :assignee_ids)), class: "btn", style: "font-size: 0.4em;" do %> <%= image_tag "remove.svg", aria: { hidden: true }, size: 24 %> Clear <% end %> diff --git a/app/views/bubbles/filters/_bookmark.html.erb b/app/views/bubbles/filters/_bookmark.html.erb index c38cf4e63..5b7b962dc 100644 --- a/app/views/bubbles/filters/_bookmark.html.erb +++ b/app/views/bubbles/filters/_bookmark.html.erb @@ -5,9 +5,27 @@ <% end %> <% else %> <%= form_with url: filters_path, method: :post do %> - <% Filter::KNOWN_PARAMS.each do |param| %> - <%= hidden_field_tag param, params[param] if params[param].present? %> + <%= hidden_field_tag :indexed_by, params[:indexed_by] if params[:indexed_by].present? %> + <%= hidden_field_tag :assignments, params[:assignments] if params[:assignments].present? %> + + <% if values = params[:bucket_ids].presence %> + <% values.each do |value| %> + <%= hidden_field_tag "bucket_ids[]", value, id: nil %> + <% end %> <% end %> + + <% if values = params[:tag_ids].presence %> + <% values.each do |value| %> + <%= hidden_field_tag "tag_ids[]", value, id: nil %> + <% end %> + <% end %> + + <% if values = params[:assignee_ids].presence %> + <% values.each do |value| %> + <%= hidden_field_tag "assignee_ids[]", value, id: nil %> + <% end %> + <% end %> + <%= button_tag type: :submit, class: "btn txt-small borderless" do %> <%= image_tag "bookmark-outline.svg", aria: { hidden: true }, size: 24 %> Save this filter diff --git a/app/views/bubbles/filters/_buckets.html.erb b/app/views/bubbles/filters/_buckets.html.erb index 33130a767..d5537aa07 100644 --- a/app/views/bubbles/filters/_buckets.html.erb +++ b/app/views/bubbles/filters/_buckets.html.erb @@ -8,7 +8,7 @@
  • <%= link_to "all projects", bubbles_path(filter.to_params.merge(bucket_ids: nil)), class: "filter__button" %>
    • <% Current.user.buckets.each do |bucket| %> -
  • <%= link_to bucket.name, bubbles_path(filter.to_params.merge(bucket_ids: bucket.id)), class: "filter__button" %>
    • +
  • <%= link_to bucket.name, bubbles_path(filter.to_params.merge(bucket_ids: [ bucket.id ])), class: "filter__button" %>
    • <% end %> diff --git a/app/views/bubbles/filters/_tags.html.erb b/app/views/bubbles/filters/_tags.html.erb index e520a761c..7972e7e2f 100644 --- a/app/views/bubbles/filters/_tags.html.erb +++ b/app/views/bubbles/filters/_tags.html.erb @@ -6,7 +6,7 @@ <% Current.account.tags.order(:title).each do |tag| %> -
  • <%= link_to tag.title, bubbles_path(filter.to_params.merge(tag_ids: tag.id)) %>
    • +
  • <%= link_to tag.title, bubbles_path(filter.to_params.merge(tag_ids: [ tag.id ])) %>
    • <% end %>     diff --git a/app/views/buckets/_bucket.html.erb b/app/views/buckets/_bucket.html.erb index 4ed607d43..fc270bc8d 100644 --- a/app/views/buckets/_bucket.html.erb +++ b/app/views/buckets/_bucket.html.erb @@ -1,6 +1,6 @@ <% cache bucket do %>
  • - <%= link_to bubbles_path(bucket_ids: bucket), class: "windshield__container flex justify-center align-center position-relative" do %> + <%= link_to bubbles_path(bucket_ids: [ bucket ]), class: "windshield__container flex justify-center align-center position-relative" do %>
    <% bucket.bubbles.ordered_by_activity.limit(10).each do |bubble| %>
    @@ -14,7 +14,7 @@ <% end %>
    - <%= link_to bubbles_path(bucket_ids: bucket), class: "txt-ink flex flex-column" do %> + <%= link_to bubbles_path(bucket_ids: [ bucket ]), class: "txt-ink flex flex-column" do %> In <%= bucket.name %> <% end %>
    diff --git a/db/migrate/20241105224305_create_filter_join_tables.rb b/db/migrate/20241105224305_create_filter_join_tables.rb new file mode 100644 index 000000000..31a0bc697 --- /dev/null +++ b/db/migrate/20241105224305_create_filter_join_tables.rb @@ -0,0 +1,18 @@ +class CreateFilterJoinTables < ActiveRecord::Migration[8.0] + def change + create_join_table :filters, :tags do |t| + t.index :filter_id + t.index :tag_id + end + + create_join_table :filters, :buckets do |t| + t.index :filter_id + t.index :bucket_id + end + + create_join_table :filters, :assignees do |t| + t.index :filter_id + t.index :assignee_id + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 7bad74467..f77d399a4 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.0].define(version: 2024_11_05_181312) do +ActiveRecord::Schema[8.0].define(version: 2024_11_05_224305) do create_table "accesses", force: :cascade do |t| t.integer "bucket_id", null: false t.integer "user_id", null: false @@ -58,6 +58,13 @@ ActiveRecord::Schema[8.0].define(version: 2024_11_05_181312) do t.index ["blob_id", "variation_digest"], name: "index_active_storage_variant_records_uniqueness", unique: true end + create_table "assignees_filters", id: false, force: :cascade do |t| + t.integer "filter_id", null: false + t.integer "assignee_id", null: false + t.index ["assignee_id"], name: "index_assignees_filters_on_assignee_id" + t.index ["filter_id"], name: "index_assignees_filters_on_filter_id" + end + create_table "assignments", force: :cascade do |t| t.integer "assignee_id", null: false t.integer "bubble_id", null: false @@ -92,6 +99,13 @@ ActiveRecord::Schema[8.0].define(version: 2024_11_05_181312) do t.index ["creator_id"], name: "index_buckets_on_creator_id" end + create_table "buckets_filters", id: false, force: :cascade do |t| + t.integer "filter_id", null: false + t.integer "bucket_id", null: false + t.index ["bucket_id"], name: "index_buckets_filters_on_bucket_id" + t.index ["filter_id"], name: "index_buckets_filters_on_filter_id" + end + create_table "comments", force: :cascade do |t| t.text "body", null: false t.integer "creator_id", null: false @@ -123,6 +137,13 @@ ActiveRecord::Schema[8.0].define(version: 2024_11_05_181312) do t.index ["creator_id", "params"], name: "index_filters_on_creator_id_and_params", unique: true end + create_table "filters_tags", id: false, force: :cascade do |t| + t.integer "filter_id", null: false + t.integer "tag_id", null: false + t.index ["filter_id"], name: "index_filters_tags_on_filter_id" + t.index ["tag_id"], name: "index_filters_tags_on_tag_id" + end + create_table "messages", force: :cascade do |t| t.integer "bubble_id", null: false t.string "messageable_type", null: false diff --git a/test/controllers/buckets_controller_test.rb b/test/controllers/buckets_controller_test.rb index 7bbce07c5..306d745e3 100644 --- a/test/controllers/buckets_controller_test.rb +++ b/test/controllers/buckets_controller_test.rb @@ -21,7 +21,7 @@ class BucketsControllerTest < ActionDispatch::IntegrationTest end bucket = Bucket.last - assert_redirected_to bubbles_path(bucket_ids: bucket.id) + assert_redirected_to bubbles_path(bucket_ids: [ bucket ]) assert_includes bucket.users, users(:kevin) assert_equal "Remodel Punch List", bucket.name end @@ -34,7 +34,7 @@ class BucketsControllerTest < ActionDispatch::IntegrationTest test "update" do patch bucket_url(buckets(:writebook)), params: { bucket: { name: "Writebook bugs" }, user_ids: users(:david, :jz).pluck(:id) } - assert_redirected_to bubbles_path(bucket_ids: buckets(:writebook)) + assert_redirected_to bubbles_path(bucket_ids: [ buckets(:writebook) ]) assert_equal "Writebook bugs", buckets(:writebook).reload.name assert_equal users(:david, :jz), buckets(:writebook).users end diff --git a/test/fixtures/filters.yml b/test/fixtures/filters.yml index 5fc0d61fc..0835844a6 100644 --- a/test/fixtures/filters.yml +++ b/test/fixtures/filters.yml @@ -1,3 +1,5 @@ jz_assignments: creator: david - params: <%= { indexed_by: :most_discussed, assignments: ActiveRecord::FixtureSet.identify(:jz).to_s, tag_ids: ActiveRecord::FixtureSet.identify(:mobile).to_s }.to_json %> + params: <%= { indexed_by: :most_discussed }.to_json %> + tags: mobile + assignees: jz