4745: Disable deactivate button for storage locations not deactivatable (#4775)

* 4745: Disable deactivate button for storage locations not deactivatable

* Remove @inventory_item_totals instance variable from storage locations controller

Author: coalest

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

app/views/storage_locations/_storage_location_row.html.erb

@@ -3,15 +3,16 @@
   <td><%= storage_location.address %></td>
   <td><%= storage_location.square_footage %></td>
   <td><%= storage_location.warehouse_type %></td>
-  <td><%= @inventory ? @inventory.quantity_for(storage_location: storage_location.id) : storage_location.size %></td>
+  <td><%= @inventory.quantity_for(storage_location: storage_location.id) %></td>
   <td><%= number_to_currency(storage_location.inventory_total_value_in_dollars(@inventory)) %></td>
   <td class="text-right">
     <%= view_button_to storage_location %>
     <%= edit_button_to edit_storage_location_path(storage_location) %>
     <% 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.quantity_for(storage_location: storage_location.id).zero? } %>
     <% end %>
   </td>
 </tr>

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

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