Feature tags step 1 (#4884)

coalest · web-flow · commit 7daa2dcfeedb · 2025-01-28T21:02:29.000-05:00
* Fix model spec generation error

* Add tags and taggings model/concern

* Add uniqueness constraint on taggings and fix indexes

* Add tagging related associations and scopes

* Add tag and tagging model specs

* Add tag filtering to product drives index page

* Clean up by_tags scope

* Fix factory_bot linting error

* Merge with main

* Add ability to tag product drives

* Remove WIP controller

* Fix factory bot lint error

* Fix missing tags after validation failure

* Add specs

* Fix styling of product drive modal

* Add tags_for_display method to Taggable concern

* Fix name in tag factory

* Refactor ProductDrivesController#update to be more consistent

* Move by_type scope to Tag model

* Move by_type scope spec to tag model

* Order tags alphabetically in dropdowns

* Move organization_id to tags table

* Fix linting

* Refactor filter by tags to shared example

* Remove unnecessary scope

* Fix wip changes

* Add type column to tags table

* Rename by_tags scope parameter to tag_names

* Fix missing tags on failure bug

* Fix lost tags on product drive update failure

* Add request specs for preserving tags on validation failure

* Set selectOnClose config value to true for product drive tags

* Fix new tag disappearance on validation failure
diff --git a/Gemfile b/Gemfile
@@ -155,6 +155,8 @@ group :development, :test do
   # Rails add-on for static analysis.
   gem 'rubocop-performance'
   gem "rubocop-rails", "~> 2.25.1"
+  # More concise test ("should") matchers
+  gem "shoulda-matchers", "~> 6.2"
   # Default rules for Rubocop.
   gem "standard", "~> 1.40"
   gem "standard-rails"
@@ -197,8 +199,6 @@ group :test do
   gem "rails-controller-testing"
   # Show code coverage.
   gem 'simplecov'
-  # More concise test ("should") matchers
-  gem 'shoulda-matchers', '~> 6.2'
   # Mock HTTP requests and ensure they are not called during tests.
   gem "webmock", "~> 3.24"
   # Interface capybara to chrome headless
diff --git a/app/assets/stylesheets/application.scss b/app/assets/stylesheets/application.scss
@@ -188,6 +188,12 @@ select.selectpicker + .dropdown-toggle::after {
   background-color: #8282df
 }
 
+// Fix select2 tagging search box to be inline with tags.
+// This is the default for select2 but adminLTE overwrites it.
+.select2-search.select2-search--inline {
+  display: inline-block;
+}
+
 div.low_priority_warning {
   margin: 5px;
   text-align: center;
diff --git a/app/controllers/product_drives_controller.rb b/app/controllers/product_drives_controller.rb
@@ -1,12 +1,13 @@
 class ProductDrivesController < ApplicationController
   include Importable
   before_action :set_product_drive, only: [:show, :edit, :update, :destroy]
+  before_action :set_tags, only: [:index, :new, :edit]
 
   def index
     setup_date_range_picker
     @product_drives = current_organization
                      .product_drives
-                     .includes(donations: {line_items: :item})
+                     .includes(:tags, donations: {line_items: :item})
                      .class_filter(filter_params)
                      .within_date_range(@selected_date_range)
                      .order(start_date: :desc)
@@ -15,6 +16,7 @@ def index
     @item_categories = current_organization.item_categories
     @selected_name_filter = filter_params[:by_name]
     @selected_item_category = filter_params[:by_item_category_id]
+    @selected_tags = filter_params[:by_tags]
 
     respond_to do |format|
       format.html
@@ -32,12 +34,14 @@ def index
   # GET /product_drives/1.json
 
   def create
-    @product_drive = current_organization.product_drives.new(product_drive_params.merge(organization: current_organization))
+    @product_drive = current_organization.product_drives.new(product_drive_params)
+    @product_drive.tags = tags_from_params
     respond_to do |format|
       if @product_drive.save
         format.html { redirect_to product_drives_path, notice: "New product drive added!" }
         format.js
       else
+        set_tags
         flash.now[:error] = "Something didn't work quite right -- try again?"
         format.html { render action: :new }
         format.js { render template: "product_drives/new_modal" }
@@ -66,10 +70,11 @@ def show
 
   def update
     @product_drive = current_organization.product_drives.find(params[:id])
+    @product_drive.tags = tags_from_params
     if @product_drive.update(product_drive_params)
       redirect_to product_drives_path, notice: "#{@product_drive.name} updated!"
-
     else
+      set_tags
       flash.now[:error] = "Something didn't work quite right -- try again?"
       render action: :edit
     end
@@ -90,9 +95,24 @@ def set_product_drive
     @product_drive_info = ProductDrive.find(params[:id])
   end
 
+  def set_tags
+    @tags = current_organization.product_drive_tags.alphabetized.select(:id, :name)
+  end
+
   def product_drive_params
     params.require(:product_drive)
-          .permit(:name, :start_date, :end_date, :virtual)
+      .permit(:name, :start_date, :end_date, :virtual, tags: [])
+      .except(:tags)
+  end
+
+  def tags_from_params
+    tag_names = params[:product_drive][:tags]
+    return [] if tag_names.blank?
+
+    tag_names
+      .compact_blank
+      .uniq
+      .map { |name| Tag.find_or_create_by(name:, type: "ProductDrive", organization: current_organization) }
   end
 
   def date_range_filter
@@ -105,6 +125,6 @@ def date_range_filter
     def filter_params
     return {} unless params.key?(:filters)
 
-    params.require(:filters).permit(:by_name, :by_item_category_id)
+    params.require(:filters).permit(:by_name, :by_item_category_id, :by_tags)
   end
 end
diff --git a/app/models/concerns/taggable.rb b/app/models/concerns/taggable.rb
@@ -0,0 +1,16 @@
+module Taggable
+  extend ActiveSupport::Concern
+
+  included do
+    has_many :taggings, as: :taggable, dependent: :destroy
+    has_many :tags, through: :taggings, source: :tag
+
+    scope :by_tags, ->(tag_names) { left_joins(:tags).where(tags: {name: tag_names}) }
+
+    accepts_nested_attributes_for :taggings, :tags
+
+    def tags_for_display
+      tags.map(&:name).sort.join(", ")
+    end
+  end
+end
diff --git a/app/models/organization.rb b/app/models/organization.rb
@@ -71,6 +71,9 @@ class Organization < ApplicationRecord
     has_many :purchases
     has_many :requests
     has_many :storage_locations
+    has_many :tags
+    has_many :product_drive_tags, -> { by_type("ProductDrive") },
+      class_name: "Tag", inverse_of: false
     has_many :inventory_items, through: :storage_locations
     has_many :kits
     has_many :transfers
diff --git a/app/models/product_drive.rb b/app/models/product_drive.rb
@@ -16,6 +16,7 @@ class ProductDrive < ApplicationRecord
   has_paper_trail
   belongs_to :organization, optional: true
   include Filterable
+  include Taggable
 
   scope :by_name, ->(name_filter) { where(name: name_filter) }
   scope :by_item_category_id, ->(item_category_id) {
@@ -44,7 +45,7 @@ def end_date_is_bigger_of_end_date
     return if start_date.nil? || end_date.nil?
 
     if end_date < start_date
-      errors.add(:end_date, 'End date must be after the start date')
+      errors.add(:end_date, 'must be after the start date')
     end
   end
 
diff --git a/app/models/tag.rb b/app/models/tag.rb
@@ -0,0 +1,23 @@
+# == Schema Information
+#
+# Table name: tags
+#
+#  id              :bigint           not null, primary key
+#  name            :string(256)      not null
+#  type            :string           not null
+#  created_at      :datetime         not null
+#  updated_at      :datetime         not null
+#  organization_id :bigint           not null
+#
+class Tag < ApplicationRecord
+  self.inheritance_column = nil
+
+  has_many :taggings, dependent: :destroy
+  belongs_to :organization
+
+  scope :alphabetized, -> { order(:name) }
+  scope :by_type, ->(type) { where(type: type) }
+
+  validates :name, presence: true, length: {maximum: 256}
+  validates :name, uniqueness: {scope: [:type, :organization_id]}
+end
diff --git a/app/models/tagging.rb b/app/models/tagging.rb
@@ -0,0 +1,17 @@
+# == Schema Information
+#
+# Table name: taggings
+#
+#  id            :bigint           not null, primary key
+#  taggable_type :string           not null
+#  created_at    :datetime         not null
+#  updated_at    :datetime         not null
+#  tag_id        :bigint           not null
+#  taggable_id   :bigint           not null
+#
+class Tagging < ApplicationRecord
+  belongs_to :tag
+  belongs_to :taggable, polymorphic: true
+
+  validates :tag_id, uniqueness: {scope: :taggable, message: "has already been applied"}
+end
diff --git a/app/views/product_drives/_form.html.erb b/app/views/product_drives/_form.html.erb
@@ -24,7 +24,18 @@
                             as: :date,
                             html5: true %>
               </div>
-
+              <div class="form-group">
+                <%= f.input :tags,
+                  collection: @tags,
+                  value_method: :name,
+                  multiple: true,
+                  selected: @product_drive.tags.map(&:name),
+                  wrapper: :input_group,
+                  input_html: {
+                    "data-controller": "select2",
+                    "data-select2-config-value": '{"selectOnClose": "true", "width": "100%", "placeholder":"Add a tag", "tags": "true", "tokenSeparators": [",", "\t"]}'
+                  } %>
+              </div>
               <div class="form-group">
                 <%= f.input :virtual, label: "Product Drive is Virtual?", wrapper: :input_group do %>
                   <%= f.check_box :virtual, {class: "input-group-text", id: "virtual"}, "true" %>
diff --git a/app/views/product_drives/index.html.erb b/app/views/product_drives/index.html.erb
@@ -43,6 +43,9 @@
                 <div class="form-group col-lg-3 col-md-4 col-sm-6 col-xs-12">
                   <%= filter_select(label: "Filter by item category", scope: :by_item_category_id, collection: @item_categories, selected: @selected_item_category) %>
                 </div>
+                <div class="form-group col-lg-3 col-md-4 col-sm-6 col-xs-12">
+                  <%= filter_select(label: "Filter by tag", scope: :by_tags, key: :name, collection: @tags, selected: @selected_tags) %>
+                </div>
                 <div class="form-group col-lg-3 col-md-4 col-sm-6 col-xs-12">
                   <%= label_tag "Date Range" %>
                   <%= render partial: "shared/date_range_picker", locals: {css_class: "form-control"} %>
@@ -90,6 +93,7 @@
                 <th>Product Drive Name</th>
                 <th>Start Date</th>
                 <th>End Date</th>
+                <th>Tags</th>
                 <th>Held Virtually?</th>
                 <th class="numeric">Quantity of Items</th>
                 <th class="numeric">Variety of Items</th>
@@ -103,6 +107,7 @@
                   <td><%= product_drive.name %></td>
                   <td><%= product_drive.start_date.strftime("%m-%d-%Y") %></td>
                   <td><%= product_drive.end_date&.strftime("%m-%d-%Y") %></td>
+                  <td><%= product_drive.tags_for_display %></td>
                   <td><%= is_virtual(product_drive: product_drive) %></td>
                   <td class="text-right"><%= product_drive.donation_quantity_by_date(selected_range, @selected_item_category) %></td>
                   <td class="text-right"><%= product_drive.distinct_items_count_by_date(selected_range, @selected_item_category) %></td>
diff --git a/app/views/product_drives/show.html.erb b/app/views/product_drives/show.html.erb
@@ -32,13 +32,15 @@
               <thead>
               <tr>
                 <th>Name</th>
+                <th>Tags</th>
                 <th>Start Date</th>
                 <th>End Date</th>
               </tr>
               </thead>
               <tbody>
               <tr>
               <td><%= @product_drive.name %></td>
+              <td><%= @product_drive.tags_for_display %></td>
               <td class="text-left" class="date"><%= @product_drive.start_date.strftime("%m-%d-%Y") %></td>
               <td class="text-left"><%= @product_drive.end_date&.strftime("%m-%d-%Y") %></td>
               </tr>
diff --git a/db/migrate/20241221180952_create_tags.rb b/db/migrate/20241221180952_create_tags.rb
@@ -0,0 +1,13 @@
+class CreateTags < ActiveRecord::Migration[7.2]
+  def change
+    create_table :tags do |t|
+      t.string :name, limit: 256, null: false
+      t.string :type, null: false
+      t.belongs_to :organization, null: false, foreign_key: true
+
+      t.index [:type, :organization_id, :name], unique: true
+
+      t.timestamps
+    end
+  end
+end
diff --git a/db/migrate/20241221181633_create_taggings.rb b/db/migrate/20241221181633_create_taggings.rb
@@ -0,0 +1,11 @@
+class CreateTaggings < ActiveRecord::Migration[7.2]
+  def change
+    create_table :taggings do |t|
+      t.belongs_to :tag, null: false, foreign_key: true
+      t.references :taggable, polymorphic: true, null: false, index: false
+      t.index [:taggable_type, :taggable_id, :tag_id], unique: true
+
+      t.timestamps
+    end
+  end
+end
diff --git a/db/schema.rb b/db/schema.rb
@@ -745,6 +745,26 @@
     t.index ["organization_id"], name: "index_storage_locations_on_organization_id"
   end
 
+  create_table "taggings", force: :cascade do |t|
+    t.bigint "tag_id", null: false
+    t.string "taggable_type", null: false
+    t.bigint "taggable_id", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["tag_id"], name: "index_taggings_on_tag_id"
+    t.index ["taggable_type", "taggable_id", "tag_id"], name: "index_taggings_on_taggable_type_and_taggable_id_and_tag_id", unique: true
+  end
+
+  create_table "tags", force: :cascade do |t|
+    t.string "name", limit: 256, null: false
+    t.string "type", null: false
+    t.bigint "organization_id", null: false
+    t.datetime "created_at", null: false
+    t.datetime "updated_at", null: false
+    t.index ["organization_id"], name: "index_tags_on_organization_id"
+    t.index ["type", "organization_id", "name"], name: "index_tags_on_type_and_organization_id_and_name", unique: true
+  end
+
   create_table "transfers", id: :serial, force: :cascade do |t|
     t.integer "from_id"
     t.integer "to_id"
@@ -877,6 +897,8 @@
   add_foreign_key "requests", "distributions"
   add_foreign_key "requests", "organizations"
   add_foreign_key "requests", "partners"
+  add_foreign_key "taggings", "tags"
+  add_foreign_key "tags", "organizations"
   add_foreign_key "units", "organizations"
   add_foreign_key "users", "users_roles", column: "last_role_id", on_delete: :nullify
 end
diff --git a/spec/factories/taggings.rb b/spec/factories/taggings.rb
@@ -0,0 +1,17 @@
+# == Schema Information
+#
+# Table name: taggings
+#
+#  id            :bigint           not null, primary key
+#  taggable_type :string           not null
+#  created_at    :datetime         not null
+#  updated_at    :datetime         not null
+#  tag_id        :bigint           not null
+#  taggable_id   :bigint           not null
+#
+FactoryBot.define do
+  factory :tagging do
+    tag { create(:tag) }
+    taggable { create(:product_drive) }
+  end
+end
diff --git a/spec/factories/tags.rb b/spec/factories/tags.rb
@@ -0,0 +1,18 @@
+# == Schema Information
+#
+# Table name: tags
+#
+#  id              :bigint           not null, primary key
+#  name            :string(256)      not null
+#  type            :string           not null
+#  created_at      :datetime         not null
+#  updated_at      :datetime         not null
+#  organization_id :bigint           not null
+#
+FactoryBot.define do
+  factory :tag do
+    organization { Organization.try(:first) || create(:organization) }
+    sequence(:name) { |n| "Tag #{n}" }
+    type { "ProductDrive" }
+  end
+end
diff --git a/spec/models/organization_spec.rb b/spec/models/organization_spec.rb
@@ -87,6 +87,7 @@
 
   context "Associations >" do
     it { should have_many(:item_categories) }
+    it { should have_many(:product_drive_tags) }
     it { should belong_to(:ndbn_member).class_name("NDBNMember").optional }
 
     describe 'users' do
diff --git a/spec/models/product_drive_spec.rb b/spec/models/product_drive_spec.rb
@@ -76,6 +76,7 @@
     let!(:donation) { create(:product_drive_donation, product_drive_participant: product_drive_participant) }
     let!(:product_drive2) { create(:product_drive) }
     let!(:donation2) { create(:donation, :with_items, item_quantity: 7, product_drive: product_drive2, product_drive_participant: product_drive_participant) }
+    let(:tag) { create(:tag, name: "Foo tag", organization:) }
 
     subject { create(:product_drive) }
 
@@ -89,6 +90,12 @@
       is_expected.to have_many(:product_drive_participants).through(:donations)
       expect(subject.product_drive_participants).to include(donation.product_drive_participant)
     end
+
+    it "has_many tags" do
+      subject.tags << tag
+
+      expect(subject.tags).to include(tag)
+    end
   end
 
   describe "distinct_items" do
diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb
diff --git a/spec/models/tagging_spec.rb b/spec/models/tagging_spec.rb
diff --git a/spec/requests/product_drives_requests_spec.rb b/spec/requests/product_drives_requests_spec.rb
diff --git a/spec/support/tag_filter_shared_example.rb b/spec/support/tag_filter_shared_example.rb
