Inactivating vendors (#4959)

* initial hability to deactivate the vendor

* adds migration to set all existing vendors active to true

* adds reactivation to vendors index

* remove inactive vendors from new purchase form

* adds filter to vendors

* apply redirect suggestion

* refactor specs

* revert schema changes

* revert schema changes

* rubocop

app/controllers/purchases_controller.rb

@@ -101,7 +101,7 @@ def destroy
   def load_form_collections
     @storage_locations = current_organization.storage_locations.active.alphabetized
     @items = current_organization.items.active.alphabetized
-    @vendors = current_organization.vendors.alphabetized
+    @vendors = current_organization.vendors.active.alphabetized
   end
 
   def purchase_params

app/controllers/vendors_controller.rb

@@ -3,7 +3,14 @@ class VendorsController < ApplicationController
   include Importable
 
   def index
-    @vendors = current_organization.vendors.with_volumes.alphabetized
+    @vendors = current_organization
+      .vendors
+      .with_volumes
+      .alphabetized
+      .class_filter(filter_params)
+
+    @vendors = @vendors.active unless params[:include_inactive_vendors]
+    @include_inactive_vendors = params[:include_inactive_vendors]
 
     respond_to do |format|
       format.html
@@ -53,6 +60,34 @@ def update
     end
   end
 
+  def deactivate
+    vendor = current_organization.vendors.find(params[:id])
+
+    begin
+      vendor.deactivate!
+    rescue => e
+      flash[:error] = e.message
+      redirect_back(fallback_location: vendors_path)
+      return
+    end
+
+    redirect_to vendors_path, notice: "#{vendor.business_name} has been deactivated."
+  end
+
+  def reactivate
+    vendor = current_organization.vendors.find(params[:id])
+
+    begin
+      vendor.reactivate!
+    rescue => e
+      flash[:error] = e.message
+      redirect_back(fallback_location: vendors_path)
+      return
+    end
+
+    redirect_to vendors_path, notice: "#{vendor.business_name} has been reactivated."
+  end
+
   private
 
   def vendor_params
@@ -61,7 +96,9 @@ def vendor_params
   end
 
   helper_method \
-    def filter_params
-    {}
+    def filter_params(_parameters = nil)
+    return {} unless params.key?(:filters)
+
+    params.require(:filters).permit(:include_inactive_vendors)
   end
 end

app/models/vendor.rb

@@ -3,6 +3,7 @@
 # Table name: vendors
 #
 #  id              :bigint           not null, primary key
+#  active          :boolean          default(TRUE)
 #  address         :string
 #  business_name   :string
 #  comment         :string
@@ -20,16 +21,32 @@ class Vendor < ApplicationRecord
   has_paper_trail
   include Provideable
   include Geocodable
+  include Filterable
 
   has_many :purchases, inverse_of: :vendor, dependent: :destroy
 
   validates :business_name, presence: true
 
   scope :alphabetized, -> { order(:business_name) }
-
+  scope :active, -> { where(active: true) }
   scope :with_volumes, -> {
     left_joins(purchases: :line_items)
       .select("vendors.*, SUM(COALESCE(line_items.quantity, 0)) AS volume")
       .group(:id)
   }
+
+  def volume
+    LineItem.where(
+      itemizable_type: "Purchase",
+      itemizable_id: purchase_ids
+    ).sum(:quantity)
+  end
+
+  def deactivate!
+    update!(active: false)
+  end
+
+  def reactivate!
+    update!(active: true)
+  end
 end

app/views/vendors/_vendor_row.html.erb

@@ -7,5 +7,14 @@
   <td class="text-center">
     <%= view_button_to vendor_row %>
     <%= edit_button_to edit_vendor_path(vendor_row) %>
+    <% if vendor_row.active? %>
+      <%= deactivate_button_to deactivate_vendor_path(vendor_row),
+                             text: 'Deactivate',
+                             confirm: confirm_deactivate_msg(vendor_row.business_name) %>
+    <% else %>
+      <%= reactivate_button_to reactivate_vendor_path(vendor_row),
+                             text: 'Reactivate',
+                             confirm: confirm_reactivate_msg(vendor_row.business_name) %>
+    <% end %>
   </td>
 </tr>

app/views/vendors/index.html.erb

@@ -29,20 +29,38 @@
       <div class="col-md-12">
         <!-- jquery validation -->
         <div class="card card-primary">
-          <div class="card-footer">
-            <div class="pull-right">
-              <%= modal_button_to("#csvImportModal", { icon: "upload", text: "Import Vendors", size: "md"}) if @vendors.empty? %>
-              <%= download_button_to(vendors_path(format: :csv, filters: filter_params.merge(date_range: date_range_params)), {text: "Export Vendors", size: "md"}) if @vendors.any? %>
-              <%= new_button_to new_vendor_path, text: "New Vendor" %>            </div>
+          <div class="card-header">
+            <h3 class="card-title">Vendor Filter</h3>
           </div>
-        </div>
+          <!-- /.card-header -->
+          <!-- form start -->
+          <div class="card-body">
+            <%= form_tag(vendors_path, method: :get) do |f| %>
+              <div class="row">
+                <div class="form-group col-lg-3 col-md-4 col-sm-6 col-xs-12">
+                  <%= filter_checkbox(label: "Also include inactive vendors", scope: "include_inactive_vendors", selected: @include_inactive_vendors) %>
+                </div>
+              </div>
+              </div>
+              <div class="card-footer">
+                <%= filter_button %>
+                <%= clear_filter_button %>
+                <span class="float-right">
+                  <%= download_button_to(vendors_path(format: :csv, filters: filter_params.merge(date_range: date_range_params, include_inactive_vendors: @include_inactive_vendors)), {text: "Export Vendors", size: "md"}) if @vendors.any? %>
+                  <%= modal_button_to("#csvImportModal", { icon: "upload", text: "Import Vendors", size: "md"}) if @vendors.empty? %>
+                  <%= new_button_to new_vendor_path, text: "New Vendor" %>
+                </span>
+              </div>
+            <% end # form %>
+            </div>
         <!-- /.card -->
       </div>
-      <!--/.col (left) -->
     </div>
     <!-- /.row -->
   </div><!-- /.container-fluid -->
+</section>
 
+<section class="content">
   <div class="container-fluid">
     <div class="row">
       <div class="col-12">

config/routes.rb

@@ -173,6 +173,10 @@ def set_up_flipper
     collection do
       post :import_csv
     end
+    member do
+      put :deactivate
+      put :reactivate
+    end
   end
 
   resources :kits do

db/migrate/20250129154820_add_active_to_vendors.rb

@@ -0,0 +1,11 @@
+# Add active column to vendors to make it possible to delete vendors without actually deleting them
+class AddActiveToVendors < ActiveRecord::Migration[7.2]
+  def up
+    add_column :vendors, :active, :boolean
+    change_column_default :vendors, :active, true
+  end
+
+  def down
+    remove_column :vendors, :active
+  end
+end

db/migrate/20250201151720_set_existing_vendors_active.rb

@@ -0,0 +1,6 @@
+# Add active column to vendors to make it possible to delete vendors without actually deleting them
+class SetExistingVendorsActive < ActiveRecord::Migration[7.2]
+  def change
+    Vendor.update_all(active: true)
+  end
+end

db/schema.rb

@@ -10,8 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-
-ActiveRecord::Schema[7.2].define(version: 2025_01_29_015253) do
+ActiveRecord::Schema[7.2].define(version: 2025_02_01_151720) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 
@@ -856,6 +855,7 @@
     t.float "longitude"
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false
+    t.boolean "active", default: true
     t.index ["latitude", "longitude"], name: "index_vendors_on_latitude_and_longitude"
   end
 

spec/models/vendor_spec.rb

@@ -3,6 +3,7 @@
 # Table name: vendors
 #
 #  id              :bigint           not null, primary key
+#  active          :boolean          default(TRUE)
 #  address         :string
 #  business_name   :string
 #  comment         :string
@@ -37,6 +38,22 @@
         expect(subject.first.volume).to eq(10)
       end
     end
+
+    describe "deactivate!" do
+      it "deactivates the vendor" do
+        vendor = create(:vendor)
+        vendor.deactivate!
+        expect(vendor.active).to be(false)
+      end
+    end
+
+    describe "reactivate!" do
+      it "reactivates the vendor" do
+        vendor = create(:vendor, active: false)
+        vendor.reactivate!
+        expect(vendor.active).to be(true)
+      end
+    end
   end
 
   describe "versioning" do

spec/requests/purchases_requests_spec.rb

@@ -122,6 +122,11 @@
       it "should include the storage location name" do
         expect(subject.body).to include("Pawane Location")
       end
+
+      it 'does not show inactive vendors in the vendor dropdown' do
+        deactivated_vendor = create(:vendor, business_name: 'Deactivated Vendor', organization: organization, active: false)
+        expect(subject.body).not_to include(deactivated_vendor.business_name)
+      end
     end
 
     describe "POST#create" do

spec/requests/vendors_requests_spec.rb

@@ -16,9 +16,22 @@
       before { create(:vendor) }
 
       context "html" do
+        let!(:first_vendor) { create(:vendor, business_name: "Abc", organization: organization) }
+        let!(:deactivated_vendor) { create(:vendor, business_name: "Deactivated", organization: organization, active: false) }
+
         let(:response_format) { 'html' }
 
         it { is_expected.to be_successful }
+
+        it "should have only activated vendor names" do
+          subject
+          expect(response.body).to include(first_vendor.business_name)
+          expect(response.body).not_to include(deactivated_vendor.business_name)
+        end
+
+        it "should have a deactivate button for each active vendor" do
+          expect(subject.body.scan("Deactivate").count).to eq(2)
+        end
       end
 
       context "csv" do

spec/system/vendor_system_spec.rb

@@ -13,11 +13,41 @@
       @third = create(:vendor, business_name: "Cde")
       visit vendors_path
     end
+
     it "should have the vendor names in alphabetical order" do
       expect(page).to have_xpath("//table//tr", count: 4)
       expect(page.find(:xpath, "//table/tbody/tr[1]/td[1]")).to have_content(@first.business_name)
       expect(page.find(:xpath, "//table/tbody/tr[3]/td[1]")).to have_content(@third.business_name)
     end
+
+    it "should deactivate a vendor when the deactivate button is clicked" do
+      expect { click_link "Deactivate", match: :first }.to change { @first.reload.active }.to(false)
+    end
+
+    it "should reactivate a vendor when the reactivate button is clicked" do
+      expect { click_link "Deactivate", match: :first }.to change { @first.reload.active }.to(false)
+
+      check "include_inactive_vendors"
+      click_button "Filter"
+
+      expect { click_link "Reactivate", match: :first }.to change { @first.reload.active }.to(true)
+    end
+
+    context "When using the include_inactive_vendors filter" do
+      before(:each) do
+        @active_vendor = create(:vendor, business_name: "Active Vendor", active: true)
+        @inactive_vendor = create(:vendor, business_name: "Inactive Vendor", active: false)
+        visit vendors_path
+      end
+
+      it "shows inactive vendors when the filter is applied" do
+        check "include_inactive_vendors"
+        click_button "Filter"
+
+        expect(page).to have_content(@active_vendor.business_name)
+        expect(page).to have_content(@inactive_vendor.business_name)
+      end
+    end
   end
 
   context "when creating a new vendor" do