From f6028f829a4a31973e6d31da51495f0849374cf7 Mon Sep 17 00:00:00 2001 From: Cory Streiff Date: Tue, 12 Nov 2024 14:28:32 +0100 Subject: [PATCH 1/2] 4745: Disable deactivate button for storage locations not deactivatable --- .../storage_locations_controller.rb | 7 ++--- .../_storage_location_row.html.erb | 5 ++-- .../storage_locations_requests_spec.rb | 30 ++++++++++++++++++- spec/system/storage_location_system_spec.rb | 3 +- 4 files changed, 36 insertions(+), 9 deletions(-) diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index e2c5d20bdf..44b0336d1e 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -14,7 +14,7 @@ def index @items = StorageLocation.items_inventoried(current_organization, @inventory) @include_inactive_storage_locations = params[:include_inactive_storage_locations].present? @storage_locations = current_organization.storage_locations.alphabetized - if @inventory && filter_params[:containing].present? + if filter_params[:containing].present? containing_ids = @inventory.storage_locations.keys.select do |sl| @inventory.quantity_for(item_id: filter_params[:containing], storage_location: sl).positive? end @@ -23,9 +23,8 @@ def index @storage_locations = @storage_locations.class_filter(filter_params) end - unless @include_inactive_storage_locations - @storage_locations = @storage_locations.kept - end + @storage_locations = @storage_locations.kept unless @include_inactive_storage_locations + @inventory_item_totals = @storage_locations.to_h { |loc| [loc.id, @inventory.quantity_for(storage_location: loc.id)] } respond_to do |format| format.html diff --git a/app/views/storage_locations/_storage_location_row.html.erb b/app/views/storage_locations/_storage_location_row.html.erb index 2e8e645852..6198421c8d 100644 --- a/app/views/storage_locations/_storage_location_row.html.erb +++ b/app/views/storage_locations/_storage_location_row.html.erb @@ -3,7 +3,7 @@ <%= storage_location.address %> <%= storage_location.square_footage %> <%= storage_location.warehouse_type %> - <%= @inventory ? @inventory.quantity_for(storage_location: storage_location.id) : storage_location.size %> + <%= @inventory_item_totals[storage_location.id] %> <%= number_to_currency(storage_location.inventory_total_value_in_dollars(@inventory)) %> <%= view_button_to storage_location %> @@ -11,7 +11,8 @@ <% if storage_location.discarded? %> <%= reactivate_button_to storage_location_reactivate_path(storage_location), { confirm: confirm_reactivate_msg(storage_location.name) } %> <% else %> - <%= deactivate_button_to storage_location_deactivate_path(storage_location), { confirm: confirm_deactivate_msg(storage_location.name) } %> + <%= deactivate_button_to storage_location_deactivate_path(storage_location), + { confirm: confirm_deactivate_msg(storage_location.name), enabled: @inventory_item_totals[storage_location.id].zero? } %> <% end %> diff --git a/spec/requests/storage_locations_requests_spec.rb b/spec/requests/storage_locations_requests_spec.rb index 0067969f54..648da84f14 100644 --- a/spec/requests/storage_locations_requests_spec.rb +++ b/spec/requests/storage_locations_requests_spec.rb @@ -9,7 +9,12 @@ end describe "GET #index" do - before { create(:storage_location, name: "Test Storage Location", address: "123 Donation Site Way", warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) } + let!(:storage_location) do + create(:storage_location, + name: "Test Storage Location", + address: "123 Donation Site Way", + warehouse_type: StorageLocation::WAREHOUSE_TYPES.first) + end context "html" do let(:response_format) { 'html' } @@ -34,6 +39,29 @@ end end end + + context "with empty storage location" do + it "shows a deactivate button" do + get storage_locations_path(format: response_format) + page = Nokogiri::HTML(response.body) + deactivate_link = page.at_css("a[href='#{storage_location_deactivate_path(storage_location)}']") + expect(deactivate_link.attr("class")).not_to match(/disabled/) + end + end + + context "with nonempty storage location" do + before do + TestInventory.create_inventory(storage_location.organization, + { storage_location.id => { create(:item, name: "A").id => 1 } }) + end + + it "shows a disabled deactivate button" do + get storage_locations_path(format: response_format) + page = Nokogiri::HTML(response.body) + deactivate_link = page.at_css("a[href='#{storage_location_deactivate_path(storage_location)}']") + expect(deactivate_link.attr("class")).to match(/disabled/) + end + end end context "csv" do diff --git a/spec/system/storage_location_system_spec.rb b/spec/system/storage_location_system_spec.rb index 15b460f483..c579901546 100644 --- a/spec/system/storage_location_system_spec.rb +++ b/spec/system/storage_location_system_spec.rb @@ -134,8 +134,7 @@ location1 = create(:storage_location, :with_items) visit subject - expect(accept_confirm { click_on "Deactivate", match: :first }).to include "Are you sure you want to deactivate #{location1.name}" - expect(page.find(".alert")).to have_content "Cannot deactivate storage location containing inventory items with non-zero quantities" + expect(page).to have_link('Deactivate', class: "disabled", href: "/storage_locations/#{location1.id}/deactivate") end it "Allows user to deactivate and reactivate storage locations" do From f5ccc08b3a0abb2d2b461a6cd2de3601cc43a21c Mon Sep 17 00:00:00 2001 From: Cory Streiff Date: Tue, 19 Nov 2024 12:53:26 +0100 Subject: [PATCH 2/2] Remove @inventory_item_totals instance variable from storage locations controller --- app/controllers/storage_locations_controller.rb | 5 +++-- app/views/storage_locations/_storage_location_row.html.erb | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/app/controllers/storage_locations_controller.rb b/app/controllers/storage_locations_controller.rb index 44b0336d1e..2fb3a6cc64 100644 --- a/app/controllers/storage_locations_controller.rb +++ b/app/controllers/storage_locations_controller.rb @@ -23,8 +23,9 @@ def index @storage_locations = @storage_locations.class_filter(filter_params) end - @storage_locations = @storage_locations.kept unless @include_inactive_storage_locations - @inventory_item_totals = @storage_locations.to_h { |loc| [loc.id, @inventory.quantity_for(storage_location: loc.id)] } + unless @include_inactive_storage_locations + @storage_locations = @storage_locations.kept + end respond_to do |format| format.html diff --git a/app/views/storage_locations/_storage_location_row.html.erb b/app/views/storage_locations/_storage_location_row.html.erb index 6198421c8d..70721d2c28 100644 --- a/app/views/storage_locations/_storage_location_row.html.erb +++ b/app/views/storage_locations/_storage_location_row.html.erb @@ -3,7 +3,7 @@ <%= storage_location.address %> <%= storage_location.square_footage %> <%= storage_location.warehouse_type %> - <%= @inventory_item_totals[storage_location.id] %> + <%= @inventory.quantity_for(storage_location: storage_location.id) %> <%= number_to_currency(storage_location.inventory_total_value_in_dollars(@inventory)) %> <%= view_button_to storage_location %> @@ -12,7 +12,7 @@ <%= reactivate_button_to storage_location_reactivate_path(storage_location), { confirm: confirm_reactivate_msg(storage_location.name) } %> <% else %> <%= deactivate_button_to storage_location_deactivate_path(storage_location), - { confirm: confirm_deactivate_msg(storage_location.name), enabled: @inventory_item_totals[storage_location.id].zero? } %> + { confirm: confirm_deactivate_msg(storage_location.name), enabled: @inventory.quantity_for(storage_location: storage_location.id).zero? } %> <% end %>