From fc67e16c4d60c28479a6476b6578e4f066875740 Mon Sep 17 00:00:00 2001
From: Michael Dworken <michael.dworken@framebridge.com>
Date: Sun, 17 Feb 2019 13:50:07 -0500
Subject: [PATCH 01/32] review current models and refactor confusing sections

---
 app/models/storage_location.rb | 90 ++++++++++++++++++++++++----------
 1 file changed, 64 insertions(+), 26 deletions(-)

diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index ebe8012fcb..7ad954f8a9 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -77,25 +77,23 @@ def intake!(adjustment)
     log = {}
     adjustment.line_items.each do |line_item|
       inventory_item = InventoryItem.find_or_create_by(storage_location_id: id,
-                                                       item_id: line_item.item_id) do |inv_item|
-        inv_item.quantity = 0
-      end
-      inventory_item.quantity += begin
-                                   line_item.quantity
-                                 rescue StandardError
-                                   0
-                                 end
+                                                       item_id: line_item.item_id)
+      inventory_item.quantity += line_item&.quantity || 0
       inventory_item.save
       log[line_item.item_id] = "+#{line_item.quantity}"
     end
     log
   end
 
-  def remove!(donation_or_purchase)
+  # NOTE: This knows too much about donations/purchases
+  # NOTE: Can we do logging better? It seems like a side effect
+  def remove!(itemizable)
     log = {}
-    donation_or_purchase.line_items.each do |line_item|
+    itemizable.line_items.each do |line_item|
       inventory_item = InventoryItem.find_by(storage_location: id, item_id: line_item.item_id)
+      # NOTE: Code smell - are we sure we want to allow this to be destroyed if < 0?
       if (inventory_item.quantity - line_item.quantity) <= 0
+        # NOTE: Instead of deleting, maybe we could leave it at 0, and hide it in the UI instead
         inventory_item.destroy
       else
         inventory_item.update(quantity: inventory_item.quantity - line_item.quantity)
@@ -105,8 +103,10 @@ def remove!(donation_or_purchase)
     log
   end
 
-  def adjust_from_past!(donation_or_purchase, previous_line_item_values)
-    donation_or_purchase.line_items.each do |line_item|
+  # NOTE: Make this code clearer in its intent -- needs more context
+  def adjust_from_past!(itemizable, previous_line_item_values)
+    itemizable.line_items.each do |line_item|
+      # NOTE: Can't we do an association lookup, instead of going all the way up to the model?
       inventory_item = InventoryItem.find_or_create_by(storage_location_id: id, item_id: line_item.item_id)
       # If the item wasn't deleted by the user, then it will be present to be deleted
       # here, and delete returns the item as a return value.
@@ -118,7 +118,7 @@ def adjust_from_past!(donation_or_purchase, previous_line_item_values)
       inventory_item.destroy! if inventory_item.quantity.zero?
     end
     # Update storage for line items that are no longer persisted because they
-    # were removed durring the updated/delete process.
+    # were removed during the update/delete process.
     previous_line_item_values.values.each do |value|
       inventory_item = InventoryItem.find_or_create_by(storage_location_id: id, item_id: value.item_id)
       inventory_item.decrement!(:quantity, value.quantity)
@@ -126,11 +126,16 @@ def adjust_from_past!(donation_or_purchase, previous_line_item_values)
     end
   end
 
-  def distribute!(distribution)
+  # This is the "subtract inventory method"
+  # NOTE: This is not returning a log object
+  def distribute!(itemizable)
+    # This is passed to update_inventory_inventory_items
     updated_quantities = {}
+    # Used in the exception return value
     insufficient_items = []
-    distribution.line_items.each do |line_item|
+    itemizable.line_items.each do |line_item|
       inventory_item = inventory_items.find_by(item: line_item.item)
+      # NOTE: If the distribution isn't able to find the inventory item, it continues
       next if inventory_item.nil?
 
       if inventory_item.quantity >= line_item.quantity
@@ -146,16 +151,19 @@ def distribute!(distribution)
       end
     end
 
+    # NOTE: Could this be handled by a validation instead?
     unless insufficient_items.empty?
       raise Errors::InsufficientAllotment.new(
-        "Distribution line_items exceed the available inventory",
+        "#{itemizable.class.name} line_items exceed the available inventory",
         insufficient_items
       )
     end
 
+    # NOTE: This executes the transaction to actually change the data
     update_inventory_inventory_items(updated_quantities)
   end
 
+  # NOTE: We should generalize this elsewhere -- Importable concern?
   def self.import_csv(csv, organization)
     csv.each do |row|
       loc = StorageLocation.new(row.to_hash)
@@ -164,6 +172,7 @@ def self.import_csv(csv, organization)
     end
   end
 
+  # NOTE: We should generalize this elsewhere -- Importable concern?
   def self.import_inventory(filename, org, loc)
     current_org = Organization.find(org)
     adjustment = current_org.adjustments.create(storage_location_id: loc.to_i, comment: "Starting Inventory")
@@ -176,34 +185,42 @@ def self.import_inventory(filename, org, loc)
 
   # Used to move inventory between StorageLocations; reflects items being physically moved
   # Ex: move 500 size "2" diapers from main warehouse to overflow warehouse because insufficient space in main warehouse
+  # This could all be moved over to the `Transfer` model
   def move_inventory!(transfer)
+    # Contains the total deltas from/to for all the InventoryItem records that are being changed
     updated_quantities = {}
     item_validator = Errors::InsufficientAllotment.new("Transfer items exceeds \
                                                         the available inventory")
     transfer.line_items.each do |line_item|
-      inventory_item = inventory_items.find_by(item: line_item.item)
-      new_inventory_item = transfer.to.inventory_items.find_or_create_by(item: line_item.item)
-      next if inventory_item.nil? || inventory_item.quantity.zero?
-
-      if inventory_item.quantity >= line_item.quantity
-        updated_quantities[inventory_item.id] = (updated_quantities[inventory_item.id] ||
-                                                 inventory_item.quantity) - line_item.quantity
-        updated_quantities[new_inventory_item.id] = (updated_quantities[new_inventory_item.id] ||
-          new_inventory_item.quantity) + line_item.quantity
+      # NOTE: We should do it this way more
+      from_inventory_item = inventory_items.find_by(item: line_item.item)
+      # NOTE: Initialize the inventory item on the destination storage location; "to" = "a storage location"
+      to_inventory_item = transfer.to.inventory_items.find_or_create_by(item: line_item.item)
+      # NOTE: this is for if the transfer includes inventory items that are nonexistent / zero, we can't transfer them
+      # maybe we could do this as a validation, or at the model level instead?
+      next if from_inventory_item.nil? || from_inventory_item.quantity.zero?
+
+      if from_inventory_item.quantity >= line_item.quantity
+        # NOTE: this is subtracting the inventory found on each line item, from the running total of inventory available at the source location
+        updated_quantities[from_inventory_item.id] = (updated_quantities[from_inventory_item.id] || from_inventory_item.quantity) - line_item.quantity
+        # NOTE: this is adding the inventory found to the new destination at the new storage location
+        updated_quantities[to_inventory_item.id] = (updated_quantities[to_inventory_item.id] || to_inventory_item.quantity) + line_item.quantity
       else
         item_validator.add_insufficiency(line_item.item,
-                                         inventory_item.quantity,
+                                         from_inventory_item.quantity,
                                          line_item.quantity)
       end
     end
 
     raise item_validator unless item_validator.satisfied?
 
+    # NOTE: Run the transaction
     update_inventory_inventory_items(updated_quantities)
   end
 
   # Used to adjust inventory at a StorageLocation to reflect reality
   # Ex: we thought we had 200 size "5" diapers, but we actually have 180 size "5" diapers
+  # NOTE: Has too much knowledge about adjustments -- should be moved to `Adjustment`
   def adjust!(adjustment)
     updated_quantities = {}
     item_validator = Errors::InsufficientAllotment.new("Adjustment exceeds the available inventory")
@@ -227,10 +244,13 @@ def adjust!(adjustment)
     update_inventory_inventory_items(updated_quantities)
   end
 
+  # NOTE: This reverses a distribution
+  # NOTE: Too much knowledge about distribution
   def reclaim!(distribution)
     ActiveRecord::Base.transaction do
       distribution.line_items.each do |line_item|
         if line_item.item.nil? || !line_item.item.active?
+          # If the item was previously hidden (inactive), make it active
           Item.unscoped.find(line_item.item_id).update(active: true)
           line_item.reload
         end
@@ -240,6 +260,7 @@ def reclaim!(distribution)
     end
   end
 
+  # NOTE: This has WAY too much knowledge of distribution
   def update_distribution!(distribution, new_distribution_params)
     ActiveRecord::Base.transaction do
       distribution.line_items.each do |line_item|
@@ -265,6 +286,23 @@ def update_distribution!(distribution, new_distribution_params)
     false
   end
 
+  def increase_inventory(itemizable)
+    itemizable.line_items.each do |line_item|
+      inventory_item = inventory_items.find_or_create_by!(item: line_item.item)
+      inventory_item.increment(:quantity, line_item.quantity)
+    end
+  end
+
+  end
+
+  def decrease_inventory(itemizable)
+    itemizble.line_items.each do |line_item|
+      # NOTE: Handle the ActiveRecord::RecordNotFound exception
+      inventory_item = inventory_items.find_by!(item: line_item.item)
+      inventory_item.decrement(:quantity, line_item.quantity)
+    end
+  end
+
   def self.csv_export_headers
     ["Name", "Address", "Total Inventory"]
   end

From c065f1f5fdef20c82ae289378b041caca805c41d Mon Sep 17 00:00:00 2001
From: Michael Dworken <michael.dworken@framebridge.com>
Date: Sun, 17 Feb 2019 14:25:10 -0500
Subject: [PATCH 02/32] Improve increase_inventory decrease_inventory method

---
 app/models/storage_location.rb | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 7ad954f8a9..28f150d78e 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -31,6 +31,8 @@ class StorageLocation < ApplicationRecord
                           dependent: :destroy
 
   validates :name, :address, :organization, presence: true
+  # InventoryItem knows it should never be negative, we can leverage this
+  validates_associated :inventory_items
 
   geocoded_by :address
   after_validation :geocode, if: ->(obj) { obj.address.present? && obj.address_changed? }
@@ -286,23 +288,30 @@ def update_distribution!(distribution, new_distribution_params)
     false
   end
 
+  # FIXME: After this is stable, revisit how we do logging
   def increase_inventory(itemizable)
     itemizable.line_items.each do |line_item|
       inventory_item = inventory_items.find_or_create_by!(item: line_item.item)
       inventory_item.increment(:quantity, line_item.quantity)
     end
-  end
-
+    # log could be pulled from dirty AR stuff
+    self.save!
+    # return log
   end
 
   def decrease_inventory(itemizable)
-    itemizble.line_items.each do |line_item|
-      # NOTE: Handle the ActiveRecord::RecordNotFound exception
-      inventory_item = inventory_items.find_by!(item: line_item.item)
+    itemizable.line_items.each do |line_item|
+      # Raise AR:RNF if it fails to find it
+      inventory_item = inventory_items.find_by(item: line_item.item) || inventory_items.build
+      # Attempt to reduce the inventory box quantity
       inventory_item.decrement(:quantity, line_item.quantity)
     end
+    # log could be pulled from dirty AR stuff
+    self.save!
+    # return log
   end
 
+
   def self.csv_export_headers
     ["Name", "Address", "Total Inventory"]
   end

From dda8bfaa3d7a3785cdf467ea4f91e23df5312cd0 Mon Sep 17 00:00:00 2001
From: Michael Dworken <michael.dworken@framebridge.com>
Date: Sun, 17 Feb 2019 15:09:01 -0500
Subject: [PATCH 03/32] WIP - push up to diagnose failing tests

---
 spec/models/storage_location_spec.rb | 45 ++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)

diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index ca58498980..7a82cbf77c 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -37,6 +37,51 @@
   end
 
   context "Methods >" do
+    #subject { create(:storage_location, :with_items, organization: organization) }
+    xdescribe "increase_inventory" do
+      #subject { create(:storage_location, :with_items, organization: organization) }
+      let(:donation) { create(:donation, :with_items, item_quantity: 66, organization: organization) }
+      it "increases inventory quantities from an itemizable object" do
+        expect {
+          subject.increase_inventory(donation)
+        }.to change{subject.size}.by(66)
+      end
+
+      context "when providing a new item that does not yet exist" do
+        let(:mystery_item) { create(:item, organization: organization) }
+        let(:donation_with_new_items) { create(:donation, organization: organization, line_items: build(:line_item, quantity: 10, item: mystery_item))  }
+        it "creates those new inventory items in the storage location" do
+          expect {
+            subject.increase_inventory(donation_with_new_items)
+          }.to change{subject.inventory_items.count}.by(1)
+        end
+      end
+    end
+
+    xdescribe "decrease_inventory" do
+      let(:distribution) { create(:distribution, :with_items, item_quantity: 66) }
+      it "decreases inventory quantities from an itemizable object" do
+        expect {
+          subject.decrease_inventory(distribution)
+        }.to change{subject.size}.by(-66)
+      end
+
+      context "when there is insufficient inventory available" do
+        let(:distribution_but_too_much) { create(:distribution, line_items: build(:line_item, quantity: 1000, item: subject.inventory_items.first.item, organization: organization) ) }
+        it "gives informative errors" do
+
+          ap subject.decrease_inventory(distribution_but_too_much).errors
+
+        end
+
+        it "does not change inventory quantities if there is an error" do
+          expect {
+            subject.decrease_inventory(distribution)
+          }.not_to change{subject.size}
+        end
+      end
+    end
+
     describe "StorageLocation.item_total" do
       it "gathers the final total of a single item across all inventories" do
         item = create(:item)

From ad162e53243f93913624c74c46b00f0ad6044863 Mon Sep 17 00:00:00 2001
From: Aaron Hill <armahillo@gmail.com>
Date: Mon, 18 Feb 2019 00:18:47 -0500
Subject: [PATCH 04/32] All specs pass again.

---
 app/models/storage_location.rb       | 57 +++++++++++++++++++++++++---
 db/schema.rb                         | 56 +++++++++++++--------------
 spec/models/storage_location_spec.rb | 48 +++++++++++++----------
 3 files changed, 108 insertions(+), 53 deletions(-)

diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 28f150d78e..9667b17b08 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -31,8 +31,6 @@ class StorageLocation < ApplicationRecord
                           dependent: :destroy
 
   validates :name, :address, :organization, presence: true
-  # InventoryItem knows it should never be negative, we can leverage this
-  validates_associated :inventory_items
 
   geocoded_by :address
   after_validation :geocode, if: ->(obj) { obj.address.present? && obj.address_changed? }
@@ -292,25 +290,72 @@ def update_distribution!(distribution, new_distribution_params)
   def increase_inventory(itemizable)
     itemizable.line_items.each do |line_item|
       inventory_item = inventory_items.find_or_create_by!(item: line_item.item)
-      inventory_item.increment(:quantity, line_item.quantity)
+      inventory_item.increment!(:quantity, line_item.quantity)
     end
     # log could be pulled from dirty AR stuff
-    self.save!
+    self.save
     # return log
   end
 
+  # TODO: re-evaluate this for optimization
   def decrease_inventory(itemizable)
+    insufficient_items = []
     itemizable.line_items.each do |line_item|
-      # Raise AR:RNF if it fails to find it
       inventory_item = inventory_items.find_by(item: line_item.item) || inventory_items.build
+    
+      if inventory_item.quantity < line_item.quantity
+        insufficient_items << {
+          item_id: line_item.item.id,
+          item_name: line_item.item.name,
+          quantity_on_hand: inventory_item.quantity,
+          quantity_requested: line_item.quantity
+        }
+      end
+    end
+
+    # NOTE: Could this be handled by a validation instead?
+    unless insufficient_items.empty?
+      raise Errors::InsufficientAllotment.new(
+        "Requested #{itemizable.class.name} items exceed the available inventory",
+        insufficient_items
+      )
+    end
+
+
+    itemizable.line_items.each do |line_item|
+      # Raise AR:RNF if it fails to find it
+      inventory_item = inventory_items.find_by(item: line_item.item)
       # Attempt to reduce the inventory box quantity
-      inventory_item.decrement(:quantity, line_item.quantity)
+      inventory_item.decrement!(:quantity, line_item.quantity)
     end
     # log could be pulled from dirty AR stuff
     self.save!
     # return log
   end
+=begin
+def distribute!(itemizable)
+    # This is passed to update_inventory_inventory_items
+    updated_quantities = {}
+    # Used in the exception return value
+    insufficient_items = []
+    itemizable.line_items.each do |line_item|
+      inventory_item = inventory_items.find_by(item: line_item.item)
+      # NOTE: If the distribution isn't able to find the inventory item, it continues
+      next if inventory_item.nil?
 
+      if inventory_item.quantity >= line_item.quantity
+        updated_quantities[inventory_item.id] = (updated_quantities[inventory_item.id] ||
+                                                 inventory_item.quantity) - line_item.quantity
+      else
+        insufficient_items << {
+          item_id: line_item.item.id,
+          item_name: line_item.item.name,
+          quantity_on_hand: inventory_item.quantity,
+          quantity_requested: line_item.quantity
+        }
+      end
+    end
+=end
 
   def self.csv_export_headers
     ["Name", "Address", "Total Inventory"]
diff --git a/db/schema.rb b/db/schema.rb
index 46941c34d9..73444737a1 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -46,7 +46,7 @@
     t.index ["key"], name: "index_active_storage_blobs_on_key", unique: true
   end
 
-  create_table "adjustments", id: :serial, force: :cascade do |t|
+  create_table "adjustments", force: :cascade do |t|
     t.integer "organization_id"
     t.integer "storage_location_id"
     t.text "comment"
@@ -70,7 +70,7 @@
     t.index ["user_id"], name: "index_audits_on_user_id"
   end
 
-  create_table "barcode_items", id: :serial, force: :cascade do |t|
+  create_table "barcode_items", force: :cascade do |t|
     t.string "value"
     t.integer "barcodeable_id"
     t.integer "quantity"
@@ -94,7 +94,7 @@
     t.string "partner_key"
   end
 
-  create_table "diaper_drive_participants", id: :serial, force: :cascade do |t|
+  create_table "diaper_drive_participants", force: :cascade do |t|
     t.string "contact_name"
     t.string "email"
     t.string "phone"
@@ -109,10 +109,10 @@
     t.index ["latitude", "longitude"], name: "index_diaper_drive_participants_on_latitude_and_longitude"
   end
 
-  create_table "distributions", id: :serial, force: :cascade do |t|
+  create_table "distributions", force: :cascade do |t|
     t.text "comment"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer "storage_location_id"
     t.integer "partner_id"
     t.integer "organization_id"
@@ -123,11 +123,11 @@
     t.index ["storage_location_id"], name: "index_distributions_on_storage_location_id"
   end
 
-  create_table "donation_sites", id: :serial, force: :cascade do |t|
+  create_table "donation_sites", force: :cascade do |t|
     t.string "name"
     t.string "address"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer "organization_id"
     t.float "latitude"
     t.float "longitude"
@@ -135,11 +135,11 @@
     t.index ["organization_id"], name: "index_donation_sites_on_organization_id"
   end
 
-  create_table "donations", id: :serial, force: :cascade do |t|
+  create_table "donations", force: :cascade do |t|
     t.string "source"
     t.integer "donation_site_id"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer "storage_location_id"
     t.text "comment"
     t.integer "organization_id"
@@ -176,19 +176,19 @@
     t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true
   end
 
-  create_table "inventory_items", id: :serial, force: :cascade do |t|
+  create_table "inventory_items", force: :cascade do |t|
     t.integer "storage_location_id"
     t.integer "item_id"
     t.integer "quantity", default: 0
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
   end
 
-  create_table "items", id: :serial, force: :cascade do |t|
+  create_table "items", force: :cascade do |t|
     t.string "name"
     t.string "category"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer "barcode_count"
     t.integer "organization_id"
     t.boolean "active", default: true
@@ -198,7 +198,7 @@
     t.index ["partner_key"], name: "index_items_on_partner_key"
   end
 
-  create_table "line_items", id: :serial, force: :cascade do |t|
+  create_table "line_items", force: :cascade do |t|
     t.integer "quantity"
     t.integer "item_id"
     t.integer "itemizable_id"
@@ -208,7 +208,7 @@
     t.index ["itemizable_id", "itemizable_type"], name: "index_line_items_on_itemizable_id_and_itemizable_type"
   end
 
-  create_table "organizations", id: :serial, force: :cascade do |t|
+  create_table "organizations", force: :cascade do |t|
     t.string "name"
     t.string "short_name"
     t.string "email"
@@ -226,11 +226,11 @@
     t.index ["short_name"], name: "index_organizations_on_short_name"
   end
 
-  create_table "partners", id: :serial, force: :cascade do |t|
+  create_table "partners", force: :cascade do |t|
     t.string "name"
     t.string "email"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer "organization_id"
     t.string "status"
     t.index ["organization_id"], name: "index_partners_on_organization_id"
@@ -263,11 +263,11 @@
     t.index ["partner_id"], name: "index_requests_on_partner_id"
   end
 
-  create_table "storage_locations", id: :serial, force: :cascade do |t|
+  create_table "storage_locations", force: :cascade do |t|
     t.string "name"
     t.string "address"
-    t.datetime "created_at", null: false
-    t.datetime "updated_at", null: false
+    t.datetime "created_at"
+    t.datetime "updated_at"
     t.integer "organization_id"
     t.float "latitude"
     t.float "longitude"
@@ -275,7 +275,7 @@
     t.index ["organization_id"], name: "index_storage_locations_on_organization_id"
   end
 
-  create_table "transfers", id: :serial, force: :cascade do |t|
+  create_table "transfers", force: :cascade do |t|
     t.integer "from_id"
     t.integer "to_id"
     t.string "comment"
@@ -285,7 +285,7 @@
     t.index ["organization_id"], name: "index_transfers_on_organization_id"
   end
 
-  create_table "users", id: :serial, force: :cascade do |t|
+  create_table "users", force: :cascade do |t|
     t.string "email", default: "", null: false
     t.string "encrypted_password", default: "", null: false
     t.string "reset_password_token"
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index 7a82cbf77c..585c9bbec3 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -37,47 +37,57 @@
   end
 
   context "Methods >" do
-    #subject { create(:storage_location, :with_items, organization: organization) }
-    xdescribe "increase_inventory" do
-      #subject { create(:storage_location, :with_items, organization: organization) }
+    describe "increase_inventory" do
       let(:donation) { create(:donation, :with_items, item_quantity: 66, organization: organization) }
       it "increases inventory quantities from an itemizable object" do
+        item = create(:item)
+        storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
         expect {
-          subject.increase_inventory(donation)
-        }.to change{subject.size}.by(66)
+          storage_location.increase_inventory(donation)
+        }.to change{storage_location.size}.by(66)
       end
 
       context "when providing a new item that does not yet exist" do
         let(:mystery_item) { create(:item, organization: organization) }
-        let(:donation_with_new_items) { create(:donation, organization: organization, line_items: build(:line_item, quantity: 10, item: mystery_item))  }
+        let(:donation_with_new_items) { create(:donation, :with_items, organization: organization, item_quantity: 10, item: mystery_item)  }
         it "creates those new inventory items in the storage location" do
+          item = create(:item)
+          storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
           expect {
-            subject.increase_inventory(donation_with_new_items)
-          }.to change{subject.inventory_items.count}.by(1)
+            storage_location.increase_inventory(donation_with_new_items)
+          }.to change{storage_location.inventory_items.count}.by(1)
         end
       end
     end
 
-    xdescribe "decrease_inventory" do
-      let(:distribution) { create(:distribution, :with_items, item_quantity: 66) }
+    describe "decrease_inventory" do
+      let(:item) { create(:item) }
+      let(:distribution) { create(:distribution, :with_items, item: item, item_quantity: 66) }
       it "decreases inventory quantities from an itemizable object" do
+        storage_location = create(:storage_location, :with_items, item_quantity: 100, item: item, organization: @organization)        
         expect {
-          subject.decrease_inventory(distribution)
-        }.to change{subject.size}.by(-66)
+          storage_location.decrease_inventory(distribution)
+        }.to change{storage_location.size}.by(-66)
       end
 
       context "when there is insufficient inventory available" do
-        let(:distribution_but_too_much) { create(:distribution, line_items: build(:line_item, quantity: 1000, item: subject.inventory_items.first.item, organization: organization) ) }
+        let(:distribution_but_too_much) { create(:distribution, :with_items, item: item, item_quantity: 9001) }
         it "gives informative errors" do
-
-          ap subject.decrease_inventory(distribution_but_too_much).errors
-
+          storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
+          expect {
+            storage_location.decrease_inventory(distribution_but_too_much).errors
+          }.to raise_error(Errors::InsufficientAllotment)
         end
 
         it "does not change inventory quantities if there is an error" do
-          expect {
-            subject.decrease_inventory(distribution)
-          }.not_to change{subject.size}
+          storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
+          starting_size  = storage_location.size
+          begin
+            storage_location.decrease_inventory(distribution)
+          rescue Errors::InsufficientAllotment
+          end
+          storage_location.reload
+          expect(storage_location.size).to eq(starting_size)
         end
       end
     end

From 7e9a1d96cbc3d42affc7112b8f1eee642e388746 Mon Sep 17 00:00:00 2001
From: Aaron Hill <armahillo@gmail.com>
Date: Mon, 18 Feb 2019 00:20:34 -0500
Subject: [PATCH 05/32] Rubocop fixes

---
 app/models/storage_location.rb       | 23 +++++++++++------------
 spec/models/storage_location_spec.rb | 22 +++++++++++-----------
 2 files changed, 22 insertions(+), 23 deletions(-)

diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 9667b17b08..9e53f4f956 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -293,7 +293,7 @@ def increase_inventory(itemizable)
       inventory_item.increment!(:quantity, line_item.quantity)
     end
     # log could be pulled from dirty AR stuff
-    self.save
+    save
     # return log
   end
 
@@ -302,15 +302,15 @@ def decrease_inventory(itemizable)
     insufficient_items = []
     itemizable.line_items.each do |line_item|
       inventory_item = inventory_items.find_by(item: line_item.item) || inventory_items.build
-    
-      if inventory_item.quantity < line_item.quantity
-        insufficient_items << {
-          item_id: line_item.item.id,
-          item_name: line_item.item.name,
-          quantity_on_hand: inventory_item.quantity,
-          quantity_requested: line_item.quantity
-        }
-      end
+
+      next unless inventory_item.quantity < line_item.quantity
+
+      insufficient_items << {
+        item_id: line_item.item.id,
+        item_name: line_item.item.name,
+        quantity_on_hand: inventory_item.quantity,
+        quantity_requested: line_item.quantity
+      }
     end
 
     # NOTE: Could this be handled by a validation instead?
@@ -321,7 +321,6 @@ def decrease_inventory(itemizable)
       )
     end
 
-
     itemizable.line_items.each do |line_item|
       # Raise AR:RNF if it fails to find it
       inventory_item = inventory_items.find_by(item: line_item.item)
@@ -329,7 +328,7 @@ def decrease_inventory(itemizable)
       inventory_item.decrement!(:quantity, line_item.quantity)
     end
     # log could be pulled from dirty AR stuff
-    self.save!
+    save!
     # return log
   end
 =begin
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index 585c9bbec3..3ca4d16449 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -42,20 +42,20 @@
       it "increases inventory quantities from an itemizable object" do
         item = create(:item)
         storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
-        expect {
+        expect do
           storage_location.increase_inventory(donation)
-        }.to change{storage_location.size}.by(66)
+        end.to change { storage_location.size }.by(66)
       end
 
       context "when providing a new item that does not yet exist" do
         let(:mystery_item) { create(:item, organization: organization) }
-        let(:donation_with_new_items) { create(:donation, :with_items, organization: organization, item_quantity: 10, item: mystery_item)  }
+        let(:donation_with_new_items) { create(:donation, :with_items, organization: organization, item_quantity: 10, item: mystery_item) }
         it "creates those new inventory items in the storage location" do
           item = create(:item)
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
-          expect {
+          expect do
             storage_location.increase_inventory(donation_with_new_items)
-          }.to change{storage_location.inventory_items.count}.by(1)
+          end.to change { storage_location.inventory_items.count }.by(1)
         end
       end
     end
@@ -64,24 +64,24 @@
       let(:item) { create(:item) }
       let(:distribution) { create(:distribution, :with_items, item: item, item_quantity: 66) }
       it "decreases inventory quantities from an itemizable object" do
-        storage_location = create(:storage_location, :with_items, item_quantity: 100, item: item, organization: @organization)        
-        expect {
+        storage_location = create(:storage_location, :with_items, item_quantity: 100, item: item, organization: @organization)
+        expect do
           storage_location.decrease_inventory(distribution)
-        }.to change{storage_location.size}.by(-66)
+        end.to change { storage_location.size }.by(-66)
       end
 
       context "when there is insufficient inventory available" do
         let(:distribution_but_too_much) { create(:distribution, :with_items, item: item, item_quantity: 9001) }
         it "gives informative errors" do
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
-          expect {
+          expect do
             storage_location.decrease_inventory(distribution_but_too_much).errors
-          }.to raise_error(Errors::InsufficientAllotment)
+          end.to raise_error(Errors::InsufficientAllotment)
         end
 
         it "does not change inventory quantities if there is an error" do
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
-          starting_size  = storage_location.size
+          starting_size = storage_location.size
           begin
             storage_location.decrease_inventory(distribution)
           rescue Errors::InsufficientAllotment

From 4ed08fa38e0b41b25878aaad662c0bfc96b297ff Mon Sep 17 00:00:00 2001
From: Aaron Hill <armahillo@gmail.com>
Date: Mon, 18 Feb 2019 00:57:47 -0500
Subject: [PATCH 06/32] Swaps out a few methods with the new methods. Created a
 test that spotlights an issue with how Adjustments are done now.

---
 app/controllers/adjustments_controller.rb   |  3 ++-
 app/controllers/distributions_controller.rb | 28 ++++++++-------------
 app/controllers/donations_controller.rb     |  2 +-
 app/controllers/purchases_controller.rb     |  2 +-
 app/controllers/transfers_controller.rb     |  5 +++-
 app/models/purchase.rb                      |  2 +-
 app/models/storage_location.rb              |  6 +++++
 spec/features/adjustment_spec.rb            | 16 ++++++++++++
 8 files changed, 42 insertions(+), 22 deletions(-)

diff --git a/app/controllers/adjustments_controller.rb b/app/controllers/adjustments_controller.rb
index aaa4b40556..3470dd5153 100644
--- a/app/controllers/adjustments_controller.rb
+++ b/app/controllers/adjustments_controller.rb
@@ -29,7 +29,8 @@ def create
     @items = current_organization.items.alphabetized
 
     if @adjustment.valid?
-      @adjustment.storage_location.adjust!(@adjustment)
+      # We use negative quantities for reduction
+      @adjustment.storage_location.increase_inventory @adjustment
 
       if @adjustment.save
         redirect_to adjustment_path(@adjustment), notice: "Adjustment was successfully created."
diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb
index 101cecab3d..c192ad3ee2 100644
--- a/app/controllers/distributions_controller.rb
+++ b/app/controllers/distributions_controller.rb
@@ -40,24 +40,18 @@ def create
     @distribution = Distribution.new(distribution_params.merge(organization: current_organization))
 
     if @distribution.valid?
-      if params[:commit] == "Preview Distribution"
-        @distribution.line_items.combine!
-        @line_items = @distribution.line_items
-        render :show
+      @distribution.storage_location.decrease_inventory @distribution
+
+      if @distribution.save
+        update_request(params[:distribution][:request_attributes], @distribution.id)
+
+        send_notification(current_organization, @distribution)
+        flash[:notice] = "Distribution created!"
+        session[:created_distribution_id] = @distribution.id
+        redirect_to distributions_path
       else
-        @distribution.storage_location.distribute!(@distribution)
-
-        if @distribution.save
-          update_request(params[:distribution][:request_attributes], @distribution.id)
-
-          send_notification(current_organization.id, @distribution.id)
-          flash[:notice] = "Distribution created!"
-          session[:created_distribution_id] = @distribution.id
-          redirect_to distributions_path
-        else
-          flash[:error] = "There was an error, try again?"
-          render :new
-        end
+        flash[:error] = "There was an error, try again?"
+        render :new
       end
     else
       @storage_locations = current_organization.storage_locations
diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb
index 0c5ab9bf22..2688d313e5 100644
--- a/app/controllers/donations_controller.rb
+++ b/app/controllers/donations_controller.rb
@@ -69,7 +69,7 @@ def scale_intake
   def create
     @donation = Donation.new(donation_params.merge(organization: current_organization))
     if @donation.save
-      @donation.storage_location.intake! @donation
+      @donation.storage_location.increase_inventory @donation
       redirect_to donations_path
     else
       load_form_collections
diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb
index e864d5d835..0d0913e5cc 100644
--- a/app/controllers/purchases_controller.rb
+++ b/app/controllers/purchases_controller.rb
@@ -19,7 +19,7 @@ def index
   def create
     @purchase = current_organization.purchases.new(purchase_params)
     if @purchase.save
-      @purchase.storage_location.intake! @purchase
+      @purchase.storage_location.increase_inventory @purchase
       redirect_to purchases_path
     else
       load_form_collections
diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb
index 3fd79d2625..8bac2c25c9 100644
--- a/app/controllers/transfers_controller.rb
+++ b/app/controllers/transfers_controller.rb
@@ -11,7 +11,10 @@ def create
     @transfer = current_organization.transfers.new(transfer_params)
 
     if @transfer.valid?
-      @transfer.from.move_inventory!(@transfer)
+      ActiveRecord::Base.transaction do
+        @transfer.from.decrease_inventory @transfer
+        @transfer.to.increase_inventory @transfer
+      end
 
       if @transfer.save
         redirect_to transfers_path, notice: "Transfer was successfully created."
diff --git a/app/models/purchase.rb b/app/models/purchase.rb
index e32ed917d5..37a049bb6b 100644
--- a/app/models/purchase.rb
+++ b/app/models/purchase.rb
@@ -52,7 +52,7 @@ def purchased_from_view
   end
 
   def remove_inventory
-    storage_location.remove!(self)
+    storage_location.decrease_inventory self
   end
 
   def track(item, quantity)
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 9e53f4f956..59d3c7defb 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -288,18 +288,22 @@ def update_distribution!(distribution, new_distribution_params)
 
   # FIXME: After this is stable, revisit how we do logging
   def increase_inventory(itemizable)
+    log = {}
     itemizable.line_items.each do |line_item|
       inventory_item = inventory_items.find_or_create_by!(item: line_item.item)
       inventory_item.increment!(:quantity, line_item.quantity)
+      log[line_item.item_id] = "+#{line_item.quantity}"
     end
     # log could be pulled from dirty AR stuff
     save
     # return log
+    log
   end
 
   # TODO: re-evaluate this for optimization
   def decrease_inventory(itemizable)
     insufficient_items = []
+    log = {}
     itemizable.line_items.each do |line_item|
       inventory_item = inventory_items.find_by(item: line_item.item) || inventory_items.build
 
@@ -326,10 +330,12 @@ def decrease_inventory(itemizable)
       inventory_item = inventory_items.find_by(item: line_item.item)
       # Attempt to reduce the inventory box quantity
       inventory_item.decrement!(:quantity, line_item.quantity)
+      log[line_item.item_id] = "-#{line_item.quantity}"
     end
     # log could be pulled from dirty AR stuff
     save!
     # return log
+    log
   end
 =begin
 def distribute!(itemizable)
diff --git a/spec/features/adjustment_spec.rb b/spec/features/adjustment_spec.rb
index 8edd3ac5c0..98075aeedd 100644
--- a/spec/features/adjustment_spec.rb
+++ b/spec/features/adjustment_spec.rb
@@ -36,6 +36,22 @@
     expect(page).to have_content("Adjustment was successfully created")
   end
 
+  fscenario "User is informed politely that they're adjusting way too hard", js: true do
+    sub_quantity = -9001
+    storage_location = create(:storage_location, :with_items, item_quantity: 10, organization: @organization)
+    visit url_prefix + "/adjustments"
+    click_on "New Adjustment"
+    select storage_location.name, from: "From storage location"
+    fill_in "Comment", with: "something"
+    select Item.last.name, from: "adjustment_line_items_attributes_0_item_id"
+    fill_in "adjustment_line_items_attributes_0_quantity", with: sub_quantity.to_s
+
+    expect do
+      click_button "Save"
+    end.not_to change { storage_location.size }
+    expect(page).to have_content("Adjustment was successfully created")
+  end
+
   scenario "User can filter the #index by storage location" do
     storage_location = create(:storage_location, name: "here", organization: @organization)
     storage_location2 = create(:storage_location, name: "there", organization: @organization)

From 82e9662dfc707e524ffda26d7c52043f61b992d9 Mon Sep 17 00:00:00 2001
From: Aaron Hill <armahillo@gmail.com>
Date: Sun, 31 Mar 2019 11:04:49 -0700
Subject: [PATCH 07/32] Distribution now uses increase_inventory, reclaim! has
 been renamed to just plain ol' destroy. Removed a few unnecessary database
 hits in a controller spec. Tests have not yet been resolved.

---
 app/controllers/distributions_controller.rb   | 11 ++---
 app/models/storage_location.rb                | 49 +++++++++++--------
 .../distributions/_distribution_row.html.erb  |  3 +-
 config/routes.rb                              |  3 +-
 .../distributions_controller_spec.rb          | 28 ++++-------
 5 files changed, 44 insertions(+), 50 deletions(-)

diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb
index c192ad3ee2..1fa48e3c4b 100644
--- a/app/controllers/distributions_controller.rb
+++ b/app/controllers/distributions_controller.rb
@@ -14,15 +14,14 @@ def print
     end
   end
 
-  def reclaim
+  def destroy
     ActiveRecord::Base.transaction do
-      @distribution_id = params[:id]
       distribution = Distribution.find(params[:id])
-      distribution.storage_location.reclaim!(distribution)
+      distribution.storage_location.increase_inventory(distribution)
       distribution.destroy!
     end
 
-    flash[:notice] = "Distribution #{@distribution_id} has been reclaimed!"
+    flash[:notice] = "Distribution #{params[:id]} has been reclaimed!"
     redirect_to distributions_path
   end
 
@@ -38,6 +37,7 @@ def index
 
   def create
     @distribution = Distribution.new(distribution_params.merge(organization: current_organization))
+    @storage_locations = current_organization.storage_locations
 
     if @distribution.valid?
       @distribution.storage_location.decrease_inventory @distribution
@@ -54,9 +54,8 @@ def create
         render :new
       end
     else
-      @storage_locations = current_organization.storage_locations
       flash[:error] = "An error occurred, try again?"
-      logger.error "failed to save distribution: #{@distribution.errors.full_messages}"
+      logger.error "[!] DistributionsController#create failed to save distribution: #{@distribution.errors.full_messages}"
       render :new
     end
   rescue Errors::InsufficientAllotment => ex
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 59d3c7defb..4374e0982b 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -244,22 +244,6 @@ def adjust!(adjustment)
     update_inventory_inventory_items(updated_quantities)
   end
 
-  # NOTE: This reverses a distribution
-  # NOTE: Too much knowledge about distribution
-  def reclaim!(distribution)
-    ActiveRecord::Base.transaction do
-      distribution.line_items.each do |line_item|
-        if line_item.item.nil? || !line_item.item.active?
-          # If the item was previously hidden (inactive), make it active
-          Item.unscoped.find(line_item.item_id).update(active: true)
-          line_item.reload
-        end
-        inventory_item = inventory_items.find_by(item: line_item.item)
-        inventory_item.update!(quantity: inventory_item.quantity + line_item.quantity)
-      end
-    end
-  end
-
   # NOTE: This has WAY too much knowledge of distribution
   def update_distribution!(distribution, new_distribution_params)
     ActiveRecord::Base.transaction do
@@ -288,13 +272,24 @@ def update_distribution!(distribution, new_distribution_params)
 
   # FIXME: After this is stable, revisit how we do logging
   def increase_inventory(itemizable)
+    # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
+    # Iterate through each of the line-items in the moving box
     itemizable.line_items.each do |line_item|
+      if line_item.item.nil? || !line_item.item.active?
+        # If the item was previously hidden (inactive), make it active
+        Item.unscoped.find(line_item.item_id).update(active: true)
+        line_item.reload
+      end
+      # Locate the storage box for the item, or create a new storage box for it
       inventory_item = inventory_items.find_or_create_by!(item: line_item.item)
+      # Increase the quantity-on-record for that item
       inventory_item.increment!(:quantity, line_item.quantity)
+      # Record in the log that this has occurred
       log[line_item.item_id] = "+#{line_item.quantity}"
     end
-    # log could be pulled from dirty AR stuff
+    # log could be pulled from dirty AR stuff?
+    # Save the final changes -- does this need to occur here?
     save
     # return log
     log
@@ -302,13 +297,18 @@ def increase_inventory(itemizable)
 
   # TODO: re-evaluate this for optimization
   def decrease_inventory(itemizable)
-    insufficient_items = []
+    # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
+    # This tracks items that have insufficient inventory counts to be reduced as much
+    insufficient_items = []
+    # Iterate through each of the line-items in the moving box
     itemizable.line_items.each do |line_item|
+      # Locate the storage box for the item, or create an empty storage box
       inventory_item = inventory_items.find_by(item: line_item.item) || inventory_items.build
-
+      # If we've got sufficient inventory in the storage box to fill the moving box, then continue
       next unless inventory_item.quantity < line_item.quantity
 
+      # Otherwise, we need to record that there was insufficient inventory on-hand
       insufficient_items << {
         item_id: line_item.item.id,
         item_name: line_item.item.name,
@@ -318,18 +318,25 @@ def decrease_inventory(itemizable)
     end
 
     # NOTE: Could this be handled by a validation instead?
+    # If we found any insufficiencies
     unless insufficient_items.empty?
+      # Raise this custom error with information about each of the items that showed insufficient
+      # This bails out of the method!
       raise Errors::InsufficientAllotment.new(
         "Requested #{itemizable.class.name} items exceed the available inventory",
         insufficient_items
       )
     end
 
+    # Re-run through the items in the moving box again
     itemizable.line_items.each do |line_item|
-      # Raise AR:RNF if it fails to find it
+      # Look for the moving box for this item -- we know there is sufficient quantity this time
+      # Raise AR:RNF if it fails to find it -- though that seems moot since it would have been
+      # captured by the previous block.
       inventory_item = inventory_items.find_by(item: line_item.item)
-      # Attempt to reduce the inventory box quantity
+      # Reduce the inventory box quantity
       inventory_item.decrement!(:quantity, line_item.quantity)
+      # Record in the log that this has occurred
       log[line_item.item_id] = "-#{line_item.quantity}"
     end
     # log could be pulled from dirty AR stuff
diff --git a/app/views/distributions/_distribution_row.html.erb b/app/views/distributions/_distribution_row.html.erb
index 8f8de5e015..1a9b46e868 100644
--- a/app/views/distributions/_distribution_row.html.erb
+++ b/app/views/distributions/_distribution_row.html.erb
@@ -8,10 +8,9 @@
     <%= view_button_to distribution_path(distribution_row) %>
     <%= edit_button_to edit_distribution_path(distribution_row) %>
     <%= print_button_to print_distribution_path(distribution_row, format: :pdf) %>
-    <%= delete_button_to reclaim_distribution_path(distribution_row),
+    <%= delete_button_to distribution_path(distribution_row),
           { confirm: "Are you sure you want to reclaim this distribution to #{distribution_row.partner.name}?",
             text: "Reclaim",
-            method: "post",
             icon: "undo" } 
     %>
   </td>
diff --git a/config/routes.rb b/config/routes.rb
index 373b8dfe10..5d9af793e3 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -65,9 +65,8 @@
       end
     end
 
-    resources :distributions, except: %i(destroy) do
+    resources :distributions do
       get :print, on: :member
-      post :reclaim, on: :member
       collection do
         get :pick_ups
       end
diff --git a/spec/controllers/distributions_controller_spec.rb b/spec/controllers/distributions_controller_spec.rb
index 08ae845085..d1d0ab103d 100644
--- a/spec/controllers/distributions_controller_spec.rb
+++ b/spec/controllers/distributions_controller_spec.rb
@@ -1,7 +1,10 @@
 RSpec.describe DistributionsController, type: :controller do
+  let(:distribution) { crate(:distribution) }
   let(:default_params) do
     { organization_id: @organization.to_param }
   end
+  let(:storage_location) { create(:storage_location) }
+  let(:partner) { create(:partner) }
 
   context "While signed in" do
     before do
@@ -9,14 +12,7 @@
     end
 
     describe "GET #print" do
-      subject { get :print, params: default_params.merge(id: create(:distribution).id) }
-      it "returns http success" do
-        expect(subject).to be_successful
-      end
-    end
-
-    describe "GET #reclaim" do
-      subject { get :index, params: default_params.merge(organization_id: @organization, id: create(:distribution).id) }
+      subject { get :print, params: default_params.merge(id: distribution.id) }
       it "returns http success" do
         expect(subject).to be_successful
       end
@@ -31,16 +27,13 @@
 
     describe "POST #create" do
       it "redirects to #index on success" do
-        i = create(:storage_location)
-        p = create(:partner)
-
-        expect(i).to be_valid
-        expect(p).to be_valid
+        expect(storage_location).to be_valid
+        expect(partner).to be_valid
         expect(Flipper).to receive(:enabled?).with(:email_active).and_return(true)
 
         jobs_count = PartnerMailerJob.jobs.count
 
-        post :create, params: default_params.merge(distribution: { storage_location_id: i.id, partner_id: p.id })
+        post :create, params: default_params.merge(distribution: { storage_location_id: storage_location.id, partner_id: partner.id })
         expect(response).to have_http_status(:redirect)
 
         expect(response).to redirect_to(distributions_path)
@@ -62,7 +55,7 @@
     end
 
     describe "GET #show" do
-      subject { get :show, params: default_params.merge(id: create(:distribution).id) }
+      subject { get :show, params: default_params.merge(id: distribution.id) }
       it "returns http success" do
         expect(subject).to be_successful
       end
@@ -74,9 +67,6 @@
     end
 
     describe "POST #update" do
-      let(:location) { create(:storage_location) }
-      let(:partner) { create(:partner) }
-
       let(:distribution) { create(:distribution, partner: partner) }
       let(:issued_at) { distribution.issued_at }
       let(:distribution_params) do
@@ -84,7 +74,7 @@
           id: distribution.id,
           distribution: {
             partner_id: partner.id,
-            storage_location_id: location.id,
+            storage_location_id: storage_location.id,
             'issued_at(1i)' => issued_at.to_date.year,
             'issued_at(2i)' => issued_at.to_date.month,
             'issued_at(3i)' => issued_at.to_date.day

From 28919ab866c8c4f18338fd2355b8b3ffaebb8a3c Mon Sep 17 00:00:00 2001
From: Aaron Hill <armahillo@gmail.com>
Date: Sun, 31 Mar 2019 11:21:29 -0700
Subject: [PATCH 08/32] Wraps #create in a transaction. Extracts
 remove_inventory from Donation; all is handled at controller level now. Tests
 have not yet been run. Purchases, Transfers, Adjustments are also all
 implemented.

---
 app/controllers/adjustments_controller.rb     | 34 +++++++++----------
 app/controllers/distributions_controller.rb   |  2 +-
 app/controllers/donations_controller.rb       | 13 ++++---
 app/controllers/purchases_controller.rb       | 11 ++++--
 app/controllers/transfers_controller.rb       | 23 ++++++-------
 app/models/adjustment.rb                      | 21 ++++++++++++
 app/models/concerns/itemizable.rb             | 24 ++++++++++---
 app/models/donation.rb                        |  5 ---
 app/models/purchase.rb                        |  7 +---
 .../distributions_controller_spec.rb          |  2 +-
 spec/controllers/donations_controller_spec.rb |  7 ++--
 spec/features/adjustment_spec.rb              |  2 +-
 spec/models/adjustment_spec.rb                | 17 ++++++++++
 spec/models/donation_spec.rb                  |  9 -----
 spec/support/itemizable_shared_example.rb     | 30 ++++++++++++++++
 15 files changed, 139 insertions(+), 68 deletions(-)

diff --git a/app/controllers/adjustments_controller.rb b/app/controllers/adjustments_controller.rb
index 3470dd5153..9fe508c28d 100644
--- a/app/controllers/adjustments_controller.rb
+++ b/app/controllers/adjustments_controller.rb
@@ -1,6 +1,4 @@
 class AdjustmentsController < ApplicationController
-  before_action :set_adjustment, only: %i(show destroy)
-
   # GET /adjustments
   # GET /adjustments.json
   def index
@@ -12,45 +10,45 @@ def index
 
   # GET /adjustments/1
   # GET /adjustments/1.json
-  def show; end
+  def show
+    @adjustment = current_organization.adjustments.find(params[:id])
+  end
 
   # GET /adjustments/new
   def new
     @adjustment = current_organization.adjustments.new
     @adjustment.line_items.build
-    @storage_locations = current_organization.storage_locations
-    @items = current_organization.items.alphabetized
+    load_form_collections
   end
 
   # POST /adjustments
   def create
     @adjustment = current_organization.adjustments.new(adjustment_params)
-    @storage_locations = current_organization.storage_locations
-    @items = current_organization.items.alphabetized
-
-    if @adjustment.valid?
-      # We use negative quantities for reduction
-      @adjustment.storage_location.increase_inventory @adjustment
 
-      if @adjustment.save
-        redirect_to adjustment_path(@adjustment), notice: "Adjustment was successfully created."
-      else
-        flash[:error] = @adjustment.errors.collect { |model, message| "#{model}: " + message }.join("<br />".html_safe)
-        render :new
+    if @adjustment.valid? && @adjustment.save
+      increasing_adjustment, decreasing_adjustment = @adjustment.split_difference
+      ActiveRecord::Base.transaction do
+        @adjustment.storage_location.increase_inventory increasing_adjustment
+        @adjustment.storage_location.decrease_inventory decreasing_adjustment
       end
+
+      redirect_to adjustment_path(@adjustment), notice: "Adjustment was successful."
     else
       flash[:error] = @adjustment.errors.collect { |model, message| "#{model}: " + message }.join("<br />".html_safe)
+      load_form_collections
       render :new
     end
   rescue Errors::InsufficientAllotment => ex
     flash[:error] = ex.message
+    load_form_collections
     render :new
   end
 
   private
 
-  def set_adjustment
-    @adjustment = current_organization.adjustments.find(params[:id])
+  def load_form_collections
+    @storage_locations = current_organization.storage_locations
+    @items = current_organization.items.alphabetized
   end
 
   def adjustment_params
diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb
index 1fa48e3c4b..85027a2e85 100644
--- a/app/controllers/distributions_controller.rb
+++ b/app/controllers/distributions_controller.rb
@@ -16,7 +16,7 @@ def print
 
   def destroy
     ActiveRecord::Base.transaction do
-      distribution = Distribution.find(params[:id])
+      distribution = current_organization.distributions.find(params[:id])
       distribution.storage_location.increase_inventory(distribution)
       distribution.destroy!
     end
diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb
index 2688d313e5..b4f12258f2 100644
--- a/app/controllers/donations_controller.rb
+++ b/app/controllers/donations_controller.rb
@@ -67,7 +67,7 @@ def scale_intake
   end
 
   def create
-    @donation = Donation.new(donation_params.merge(organization: current_organization))
+    @donation = current_organization.donations.new(donation_params)
     if @donation.save
       @donation.storage_location.increase_inventory @donation
       redirect_to donations_path
@@ -75,7 +75,7 @@ def create
       load_form_collections
       @donation.line_items.build if @donation.line_items.count.zero?
       flash[:error] = "There was an error starting this donation, try again?"
-      Rails.logger.error "ERROR: #{@donation.errors}"
+      Rails.logger.error "[!] DonationsController#create Error: #{@donation.errors}"
       render action: :new
     end
   end
@@ -109,8 +109,13 @@ def update
   end
 
   def destroy
-    @donation = current_organization.donations.includes(:line_items, storage_location: :inventory_items).find(params[:id])
-    @donation.destroy
+    ActiveRecord::Base.transaction do
+      donation = current_organization.donations.find(params[:id])
+      donation.storage_location.decrease_inventory(donation)
+      donation.destroy!
+    end
+
+    flash[:notice] = "Donation #{params[:id]} has been removed!"
     redirect_to donations_path
   end
 
diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb
index 0d0913e5cc..dbd1dadbf5 100644
--- a/app/controllers/purchases_controller.rb
+++ b/app/controllers/purchases_controller.rb
@@ -25,7 +25,7 @@ def create
       load_form_collections
       @purchase.line_items.build if @purchase.line_items.count.zero?
       flash[:error] = "There was an error starting this purchase, try again?"
-      Rails.logger.error "ERROR: #{@purchase.errors}"
+      Rails.logger.error "[!] PurchasesController#create ERROR: #{@purchase.errors}"
       render action: :new
     end
   end
@@ -60,8 +60,13 @@ def update
   end
 
   def destroy
-    @purchase = current_organization.purchases.includes(:line_items, storage_location: :inventory_items).find(params[:id])
-    @purchase.destroy
+    ActiveRecord::Base.transaction do
+      purchase = current_organization.purchases.find(params[:id])
+      purchase.storage_location.decrease_inventory(purchase)
+      purchase.destroy!
+    end
+
+    flash[:notice] = "Purchase #{params[:id]} has been removed!"
     redirect_to purchases_path
   end
 
diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb
index 8bac2c25c9..905e0288db 100644
--- a/app/controllers/transfers_controller.rb
+++ b/app/controllers/transfers_controller.rb
@@ -10,29 +10,22 @@ def index
   def create
     @transfer = current_organization.transfers.new(transfer_params)
 
-    if @transfer.valid?
+    if @transfer.valid? && @transfer.save
       ActiveRecord::Base.transaction do
         @transfer.from.decrease_inventory @transfer
         @transfer.to.increase_inventory @transfer
       end
 
-      if @transfer.save
-        redirect_to transfers_path, notice: "Transfer was successfully created."
-      else
-        flash[:error] = "There was an error, try again?"
-        render :new
-      end
+      redirect_to transfers_path, notice: "#{@transfer.line_items.total} items have been transferred from #{@transfer.from.name} to #{@transfer.to.name}!"
     else
       flash[:error] = "There was an error creating the transfer"
-      @storage_locations = current_organization.storage_locations.alphabetized
-      @items = current_organization.items.alphabetized
+      load_form_collections
       @transfer.line_items.build if @transfer.line_items.empty?
       render :new
     end
   rescue Errors::InsufficientAllotment => ex
     flash[:error] = ex.message
-    @storage_locations = current_organization.storage_locations.alphabetized
-    @items = current_organization.items.alphabetized
+    load_form_collections
     @transfer.line_items.build if @transfer.line_items.empty?
     render :new
   end
@@ -40,8 +33,7 @@ def create
   def new
     @transfer = current_organization.transfers.new
     @transfer.line_items.build
-    @storage_locations = current_organization.storage_locations.alphabetized
-    @items = current_organization.items.alphabetized
+    load_form_collections
   end
 
   def show
@@ -52,6 +44,11 @@ def show
 
   private
 
+  def load_form_collections
+    @storage_locations = current_organization.storage_locations.alphabetized
+    @items = current_organization.items.alphabetized
+  end
+
   def transfer_params
     params.require(:transfer).permit(:from_id, :to_id, :comment,
                                      line_items_attributes: %i(item_id quantity _destroy))
diff --git a/app/models/adjustment.rb b/app/models/adjustment.rb
index 9ecc06e8f3..988acc4d0d 100644
--- a/app/models/adjustment.rb
+++ b/app/models/adjustment.rb
@@ -30,6 +30,27 @@ def self.storage_locations_adjusted_for(organization)
     includes(:storage_location).where(organization_id: organization.id).collect(&:storage_location)
   end
 
+  def split_difference
+    # Adjustments are weird. They'll have positive AND negative values, instead
+    # of just one or the other. Negative values mean "decrease", positive values
+    # mean "increase". Because Storage location only needs an Itemizable duck,
+    # We're going to create two temporary copies, one for increasing and one
+    # for decreasing.
+
+    increasing_adjustment = ::Adjustment.new
+    decreasing_adjustment = ::Adjustment.new
+
+    line_items.replicate_to(increasing_adjustment) do |line_item|
+      next if line_item.quantity.negative?
+    end
+
+    line_items.replicate_to(decreasing_adjustment) do |line_item|
+      next if line_item.quantity.positive?
+    end
+
+    [increasing_adjustment, decreasing_adjustment]
+  end
+
   def self.csv_export_headers
     ["Created", "Organization", "Storage Location", "Comment", "Changes"]
   end
diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index d15a22ba82..e23662fe17 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -12,6 +12,22 @@ module Itemizable
     end
 
     has_many :line_items, as: :itemizable, inverse_of: :itemizable do
+      def replicate_to(itemizable)
+        raise ArgumentError unless itemizable.respond_to?(:line_items)
+
+        all_line_items = if block_given?
+                           collect do |line_item|
+                             yield(line_item)
+                             line_item
+                           end.compact
+                         else
+                           collect
+                         end
+
+        all_line_items.each { |li| itemizable.line_items << li.dup }
+        itemizable
+      end
+
       def combine!
         # Bail if there's nothing
         return if size.zero?
@@ -19,11 +35,11 @@ def combine!
         # First we'll collect all the line_items that are used
         combined = {}
         parent_id = first.itemizable_id
-        each do |i|
-          next unless i.valid?
+        each do |line_item|
+          next unless line_item.valid?
 
-          combined[i.item_id] ||= 0
-          combined[i.item_id] += i.quantity
+          combined[line_item.item_id] ||= 0
+          combined[line_item.item_id] += line_item.quantity
         end
         # Delete all the existing ones in this association -- this
         # method aliases to `delete_all`
diff --git a/app/models/donation.rb b/app/models/donation.rb
index 99f9e9f0f1..ec3719c473 100644
--- a/app/models/donation.rb
+++ b/app/models/donation.rb
@@ -43,7 +43,6 @@ class Donation < ApplicationRecord
   }
 
   before_create :combine_duplicates
-  before_destroy :remove_inventory
 
   validates :donation_site, presence:
     { message: "must be specified since you chose '#{SOURCES[:donation_site]}'" },
@@ -136,10 +135,6 @@ def update_quantity(quantity, item)
     line_item.save
   end
 
-  def remove_inventory
-    storage_location.remove!(self)
-  end
-
   def self.csv_export_headers
     ["Source", "Date", "Donation Site", "Storage Location", "Quantity of Items", "Variety of Items"]
   end
diff --git a/app/models/purchase.rb b/app/models/purchase.rb
index 37a049bb6b..2576ee94f2 100644
--- a/app/models/purchase.rb
+++ b/app/models/purchase.rb
@@ -39,7 +39,6 @@ class Purchase < ApplicationRecord
   }
 
   before_create :combine_duplicates
-  before_destroy :remove_inventory
 
   validates :amount_spent, numericality: { greater_than: 0 }
 
@@ -51,10 +50,6 @@ def purchased_from_view
     vendor.nil? ? purchased_from : vendor.business_name
   end
 
-  def remove_inventory
-    storage_location.decrease_inventory self
-  end
-
   def track(item, quantity)
     if contains_item_id?(item.id)
       update_quantity(quantity, item)
@@ -105,7 +100,7 @@ def csv_export_attributes
   private
 
   def combine_duplicates
-    Rails.logger.info "Combining!"
+    Rails.logger.info "[!] Purchase.combine_duplicates: Combining!"
     line_items.combine!
   end
 end
diff --git a/spec/controllers/distributions_controller_spec.rb b/spec/controllers/distributions_controller_spec.rb
index d1d0ab103d..7bdd2e4e81 100644
--- a/spec/controllers/distributions_controller_spec.rb
+++ b/spec/controllers/distributions_controller_spec.rb
@@ -1,5 +1,5 @@
 RSpec.describe DistributionsController, type: :controller do
-  let(:distribution) { crate(:distribution) }
+  let(:distribution) { create(:distribution) }
   let(:default_params) do
     { organization_id: @organization.to_param }
   end
diff --git a/spec/controllers/donations_controller_spec.rb b/spec/controllers/donations_controller_spec.rb
index b65931911d..a1c16d43ea 100644
--- a/spec/controllers/donations_controller_spec.rb
+++ b/spec/controllers/donations_controller_spec.rb
@@ -4,6 +4,7 @@
   end
 
   context "While signed in >" do
+    let(:donation) { create(:donation, organization: @organization) }
     before do
       sign_in(@user)
     end
@@ -105,21 +106,21 @@
     end
 
     describe "GET #edit" do
-      subject { get :edit, params: default_params.merge(id: create(:donation)) }
+      subject { get :edit, params: default_params.merge(id: donation.id) }
       it "returns http success" do
         expect(subject).to be_successful
       end
     end
 
     describe "GET #show" do
-      subject { get :show, params: default_params.merge(id: create(:donation, organization: @organization)) }
+      subject { get :show, params: default_params.merge(id: donation.id) }
       it "returns http success" do
         expect(subject).to be_successful
       end
     end
 
     describe "DELETE #destroy" do
-      subject { delete :destroy, params: default_params.merge(id: create(:donation, organization: @organization)) }
+      subject { delete :destroy, params: default_params.merge(id: donation.id) }
       it "redirects to the index" do
         expect(subject).to redirect_to(donations_path)
       end
diff --git a/spec/features/adjustment_spec.rb b/spec/features/adjustment_spec.rb
index 98075aeedd..d4e22cfb7f 100644
--- a/spec/features/adjustment_spec.rb
+++ b/spec/features/adjustment_spec.rb
@@ -36,7 +36,7 @@
     expect(page).to have_content("Adjustment was successfully created")
   end
 
-  fscenario "User is informed politely that they're adjusting way too hard", js: true do
+  scenario "User is informed politely that they're adjusting way too hard", js: true do
     sub_quantity = -9001
     storage_location = create(:storage_location, :with_items, item_quantity: 10, organization: @organization)
     visit url_prefix + "/adjustments"
diff --git a/spec/models/adjustment_spec.rb b/spec/models/adjustment_spec.rb
index 4d114fe51c..c324c3d0a5 100644
--- a/spec/models/adjustment_spec.rb
+++ b/spec/models/adjustment_spec.rb
@@ -38,6 +38,23 @@
       create(:adjustment, storage_location: storage_location4, organization: storage_location4.organization)
       expect(Adjustment.storage_locations_adjusted_for(@organization).to_a).to match_array([storage_location1, storage_location2])
     end
+
+    describe "split_difference" do
+      it "returns two adjustment objects" do
+        item = create(:item)
+        storage_location = create(:storage_location, :with_items, item: item, item_quantity: 10)
+        storage_location.inventory_items << create(:inventory_item, item: create(:item), quantity: 10)
+        adjustment = create(:adjustment,
+                            storage_location: storage_location,
+                            line_items_attributes: [
+                              { item_id: storage_location.items.first.id, quantity: 10 },
+                              { item_id: storage_location.items.last.id, quantity: -5 }
+                            ])
+        pos, neg = adjustment.split_difference
+        expect(pos.line_items.size).to eq(1)
+        expect(neg.line_items.size).to eq(1)
+      end
+    end
   end
 
   describe "nested line item attributes" do
diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb
index d271da7be1..5771c978c2 100644
--- a/spec/models/donation_spec.rb
+++ b/spec/models/donation_spec.rb
@@ -173,15 +173,6 @@
       end
     end
 
-    describe "remove_inventory" do
-      it "removes inventory from the right storage location when donation is destroyed" do
-        donation = create(:donation, :with_items)
-        expect do
-          donation.destroy
-        end.to change { donation.storage_location.size }.by(-donation.total_quantity)
-      end
-    end
-
     describe "money_raised" do
       it "tracks the money raised in a donation" do
         donation = create(:donation, :with_items, money_raised: 100)
diff --git a/spec/support/itemizable_shared_example.rb b/spec/support/itemizable_shared_example.rb
index af1f1e8b2d..718cd92ba2 100644
--- a/spec/support/itemizable_shared_example.rb
+++ b/spec/support/itemizable_shared_example.rb
@@ -6,6 +6,36 @@
   end
 
   context ".line_items" do
+    describe "replicate_to" do
+      it "copies the line_items to the provided Itemizable object" do
+        f = build(model_f)
+        line_item = build(:line_item, item: item, quantity: 10)
+        f.line_items << line_item
+        g = build(model_f)
+        f.line_items.replicate_to(g)
+        expect(f.line_items.to_a.first.item_id).to eq(g.line_items.to_a.first.item_id)
+      end
+
+      it "raises an ArgumentError if the object provided isn't Itemizable" do
+        expect { build(model_f).line_items.replicate_to(Object.new) }.to raise_error(ArgumentError)
+      end
+
+      it "allows a block to be passed to transform the line_items first" do
+        other_item = create(:item, name: item.name + "-other")
+        f = create(model_f)
+        a_line_item = build(:line_item, item: item, quantity: 5)
+        another_line_item = build(:line_item, item: other_item, quantity: 50)
+        f.line_items << a_line_item
+        f.line_items << another_line_item
+        g = build(model_f)
+        f.line_items.replicate_to(g) do |line_item|
+          next if line_item.quantity > 10
+        end
+
+        expect(f.line_items.to_a.first.item_id).to eq(g.line_items.to_a.first.item_id)
+      end
+    end
+
     describe "combine!" do
       it "combines multiple line_items with the same item_id into a single record" do
         storage_location = create(:storage_location, :with_items, item: item, organization: @organization)

From 74147b2cdd547d312579762ea4d997875c0865d5 Mon Sep 17 00:00:00 2001
From: Michael Dworken <michael.dworken@framebridge.com>
Date: Tue, 30 Apr 2019 21:40:57 -0400
Subject: [PATCH 09/32] Simplify split_difference method

---
 app/models/adjustment.rb                  | 36 +++++++++++------------
 app/models/concerns/itemizable.rb         | 15 ----------
 spec/support/itemizable_shared_example.rb | 31 +------------------
 3 files changed, 19 insertions(+), 63 deletions(-)

diff --git a/app/models/adjustment.rb b/app/models/adjustment.rb
index 988acc4d0d..1576c783d3 100644
--- a/app/models/adjustment.rb
+++ b/app/models/adjustment.rb
@@ -31,24 +31,8 @@ def self.storage_locations_adjusted_for(organization)
   end
 
   def split_difference
-    # Adjustments are weird. They'll have positive AND negative values, instead
-    # of just one or the other. Negative values mean "decrease", positive values
-    # mean "increase". Because Storage location only needs an Itemizable duck,
-    # We're going to create two temporary copies, one for increasing and one
-    # for decreasing.
-
-    increasing_adjustment = ::Adjustment.new
-    decreasing_adjustment = ::Adjustment.new
-
-    line_items.replicate_to(increasing_adjustment) do |line_item|
-      next if line_item.quantity.negative?
-    end
-
-    line_items.replicate_to(decreasing_adjustment) do |line_item|
-      next if line_item.quantity.positive?
-    end
-
-    [increasing_adjustment, decreasing_adjustment]
+    pre_adjustment = line_items.partition {|line_item| line_item.quantity.negative?}
+    pre_adjustment.map {|pre_adjustment| Adjustment.new(line_items: pre_adjustment)  }
   end
 
   def self.csv_export_headers
@@ -74,4 +58,20 @@ def storage_locations_belong_to_organization
       errors.add :storage_location, "storage location must belong to organization"
     end
   end
+
+  def replicate_to(itemizable)
+    raise ArgumentError unless itemizable.respond_to?(:line_items)
+
+    all_line_items = if block_given?
+                       collect do |line_item|
+                         yield(line_item)
+                         line_item
+                       end.compact
+                     else
+                       collect
+                     end
+
+    all_line_items.each { |li| itemizable.line_items << li.dup }
+    itemizable
+  end
 end
diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index e23662fe17..6b1d41a68c 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -12,21 +12,6 @@ module Itemizable
     end
 
     has_many :line_items, as: :itemizable, inverse_of: :itemizable do
-      def replicate_to(itemizable)
-        raise ArgumentError unless itemizable.respond_to?(:line_items)
-
-        all_line_items = if block_given?
-                           collect do |line_item|
-                             yield(line_item)
-                             line_item
-                           end.compact
-                         else
-                           collect
-                         end
-
-        all_line_items.each { |li| itemizable.line_items << li.dup }
-        itemizable
-      end
 
       def combine!
         # Bail if there's nothing
diff --git a/spec/support/itemizable_shared_example.rb b/spec/support/itemizable_shared_example.rb
index 718cd92ba2..295a57cb07 100644
--- a/spec/support/itemizable_shared_example.rb
+++ b/spec/support/itemizable_shared_example.rb
@@ -6,36 +6,7 @@
   end
 
   context ".line_items" do
-    describe "replicate_to" do
-      it "copies the line_items to the provided Itemizable object" do
-        f = build(model_f)
-        line_item = build(:line_item, item: item, quantity: 10)
-        f.line_items << line_item
-        g = build(model_f)
-        f.line_items.replicate_to(g)
-        expect(f.line_items.to_a.first.item_id).to eq(g.line_items.to_a.first.item_id)
-      end
-
-      it "raises an ArgumentError if the object provided isn't Itemizable" do
-        expect { build(model_f).line_items.replicate_to(Object.new) }.to raise_error(ArgumentError)
-      end
-
-      it "allows a block to be passed to transform the line_items first" do
-        other_item = create(:item, name: item.name + "-other")
-        f = create(model_f)
-        a_line_item = build(:line_item, item: item, quantity: 5)
-        another_line_item = build(:line_item, item: other_item, quantity: 50)
-        f.line_items << a_line_item
-        f.line_items << another_line_item
-        g = build(model_f)
-        f.line_items.replicate_to(g) do |line_item|
-          next if line_item.quantity > 10
-        end
-
-        expect(f.line_items.to_a.first.item_id).to eq(g.line_items.to_a.first.item_id)
-      end
-    end
-
+    
     describe "combine!" do
       it "combines multiple line_items with the same item_id into a single record" do
         storage_location = create(:storage_location, :with_items, item: item, organization: @organization)

From a276234ea0d8c33c6b72f82c2d8895a23074bcaa Mon Sep 17 00:00:00 2001
From: Michael Dworken <michael.dworken@framebridge.com>
Date: Tue, 30 Apr 2019 22:11:23 -0400
Subject: [PATCH 10/32] Get tests passing re: adjustments

---
 app/models/adjustment.rb         |  7 +++++--
 spec/features/adjustment_spec.rb |  6 +++---
 spec/models/adjustment_spec.rb   | 35 ++++++++++++++++++++++++++++++++
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/app/models/adjustment.rb b/app/models/adjustment.rb
index 1576c783d3..70c6bbd2fc 100644
--- a/app/models/adjustment.rb
+++ b/app/models/adjustment.rb
@@ -31,8 +31,11 @@ def self.storage_locations_adjusted_for(organization)
   end
 
   def split_difference
-    pre_adjustment = line_items.partition {|line_item| line_item.quantity.negative?}
-    pre_adjustment.map {|pre_adjustment| Adjustment.new(line_items: pre_adjustment)  }
+    pre_adjustment = line_items.partition {|line_item| line_item.quantity.positive?}
+    increasing_adjustment, decreasing_adjustment = pre_adjustment.map {|pre_adjustment| Adjustment.new(line_items: pre_adjustment)  }
+
+    decreasing_adjustment.line_items.each { |line_item| line_item.quantity *= -1 }
+    [increasing_adjustment, decreasing_adjustment]
   end
 
   def self.csv_export_headers
diff --git a/spec/features/adjustment_spec.rb b/spec/features/adjustment_spec.rb
index d4e22cfb7f..9ed8510626 100644
--- a/spec/features/adjustment_spec.rb
+++ b/spec/features/adjustment_spec.rb
@@ -17,7 +17,7 @@
     expect do
       click_button "Save"
     end.to change { storage_location.size }.by(add_quantity)
-    expect(page).to have_content("Adjustment was successfully created")
+    expect(page).to have_content("Adjustment was successful")
   end
 
   scenario "User can subtract an inventory adjustment at a storage location", js: true do
@@ -33,7 +33,7 @@
     expect do
       click_button "Save"
     end.to change { storage_location.size }.by(sub_quantity)
-    expect(page).to have_content("Adjustment was successfully created")
+    expect(page).to have_content("Adjustment was successful")
   end
 
   scenario "User is informed politely that they're adjusting way too hard", js: true do
@@ -49,7 +49,7 @@
     expect do
       click_button "Save"
     end.not_to change { storage_location.size }
-    expect(page).to have_content("Adjustment was successfully created")
+    expect(page).to have_content("Requested Adjustment items exceed the available inventory")
   end
 
   scenario "User can filter the #index by storage location" do
diff --git a/spec/models/adjustment_spec.rb b/spec/models/adjustment_spec.rb
index c324c3d0a5..efcf6508e7 100644
--- a/spec/models/adjustment_spec.rb
+++ b/spec/models/adjustment_spec.rb
@@ -53,6 +53,41 @@
         pos, neg = adjustment.split_difference
         expect(pos.line_items.size).to eq(1)
         expect(neg.line_items.size).to eq(1)
+        expect(pos.line_items.first.quantity).to eq(10)
+        expect(neg.line_items.first.quantity).to eq(5)
+      end
+
+      it "gracefully handles adjustments with only positive" do
+        item = create(:item)
+        storage_location = create(:storage_location, :with_items, item: item, item_quantity: 10)
+        storage_location.inventory_items << create(:inventory_item, item: create(:item), quantity: 10)
+        adjustment = create(:adjustment,
+                            storage_location: storage_location,
+                            line_items_attributes: [
+                              { item_id: storage_location.items.first.id, quantity: 10 },
+                              { item_id: storage_location.items.last.id, quantity: 5 }
+                            ])
+        pos, neg = adjustment.split_difference
+        expect(pos.line_items.size).to eq(2)
+        expect(pos.line_items.first.quantity).to eq(10)
+        expect(pos.line_items.last.quantity).to eq(5)
+        expect(neg.line_items).to be_empty
+      end
+      it "gracefully handles adjustments with only negative" do
+        item = create(:item)
+        storage_location = create(:storage_location, :with_items, item: item, item_quantity: 10)
+        storage_location.inventory_items << create(:inventory_item, item: create(:item), quantity: 10)
+        adjustment = create(:adjustment,
+                            storage_location: storage_location,
+                            line_items_attributes: [
+                              { item_id: storage_location.items.first.id, quantity: -10 },
+                              { item_id: storage_location.items.last.id, quantity: -5 }
+                            ])
+        pos, neg = adjustment.split_difference
+        expect(neg.line_items.size).to eq(2)
+        expect(neg.line_items.first.quantity).to eq(10)
+        expect(neg.line_items.last.quantity).to eq(5)
+        expect(pos.line_items).to be_empty
       end
     end
   end

From 53c7540075c13dd259509e940bb66e30f6d00a27 Mon Sep 17 00:00:00 2001
From: Michael Dworken <michael.dworken@framebridge.com>
Date: Tue, 30 Apr 2019 22:22:07 -0400
Subject: [PATCH 11/32] Fix remaining spec failures; remove specs for
 implementation details no longer present

---
 spec/features/transfer_spec.rb | 2 +-
 spec/models/purchase_spec.rb   | 9 ---------
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/spec/features/transfer_spec.rb b/spec/features/transfer_spec.rb
index bef2c9f830..9de8a2940f 100644
--- a/spec/features/transfer_spec.rb
+++ b/spec/features/transfer_spec.rb
@@ -17,7 +17,7 @@
       fill_in "transfer_line_items_attributes_0_quantity", with: "10"
       click_on "Save"
     end
-    expect(page).to have_content("Transfer was successfully created")
+    expect(page).to have_content("10 items have been transferred")
   end
 
   scenario "User can filter the #index by storage location both from and to" do
diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb
index d2442d01f7..a81f320246 100644
--- a/spec/models/purchase_spec.rb
+++ b/spec/models/purchase_spec.rb
@@ -142,14 +142,5 @@
         end.not_to change { purchase.line_items.count }
       end
     end
-
-    describe "remove_inventory" do
-      it "removes inventory from the right storage location when purchase deleted" do
-        purchase = create(:purchase, :with_items)
-        expect do
-          purchase.remove_inventory
-        end.to change { purchase.storage_location.size }.by(-purchase.total_quantity)
-      end
-    end
   end
 end

From 8e0b0e2c203eadc0e9df24d1b416076b52812896 Mon Sep 17 00:00:00 2001
From: Michael Dworken <michael.dworken@framebridge.com>
Date: Wed, 1 May 2019 00:21:40 -0400
Subject: [PATCH 12/32] Remove tests for methods no longer implemented

---
 spec/models/storage_location_spec.rb | 29 ----------------------------
 1 file changed, 29 deletions(-)

diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index 3ca4d16449..88c9bbe1cb 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -315,35 +315,6 @@
       end
     end
 
-    describe "reclaim!" do
-      it "adds distribution items back to storage location" do
-        storage_location = create :storage_location, :with_items, item_quantity: 300
-        distribution = create :distribution, :with_items, storage_location: storage_location, item_quantity: 50
-        storage_location.reclaim!(distribution)
-        expect(storage_location.inventory_items.first.quantity).to eq 350
-      end
-
-      it "re-activates items that were previously deleted" do
-        storage_location = create :storage_location, :with_items, item_quantity: 200
-        distribution = create :distribution, :with_items, storage_location: storage_location, item_quantity: 50
-        item = distribution.line_items.first.item
-        expect do
-          item.destroy
-        end.to change { item.active }.from(true).to(false)
-        expect do
-          storage_location.reclaim!(distribution)
-          item.reload
-        end.to change { item.active }.from(false).to(true)
-      end
-
-      it "does not destroy the distribution" do
-        storage_location = create :storage_location, :with_items, item_quantity: 300
-        distribution = create :distribution, :with_items, storage_location: storage_location, item_quantity: 50
-        storage_location.reclaim!(distribution)
-        expect(Distribution.find(distribution.id)).to eql distribution
-      end
-    end
-
     describe "geocode" do
       it "adds coordinates to the database" do
         storage_location = build(:storage_location,

From 6cf161ca5add9651337d0ecd84d5805c2b066972 Mon Sep 17 00:00:00 2001
From: Michael Dworken <michael.dworken@framebridge.com>
Date: Wed, 1 May 2019 10:04:03 -0400
Subject: [PATCH 13/32] Fix rubocop violations

---
 app/models/adjustment.rb                  | 4 ++--
 app/models/concerns/itemizable.rb         | 1 -
 spec/support/itemizable_shared_example.rb | 1 -
 3 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/app/models/adjustment.rb b/app/models/adjustment.rb
index 70c6bbd2fc..c9c439a5de 100644
--- a/app/models/adjustment.rb
+++ b/app/models/adjustment.rb
@@ -31,8 +31,8 @@ def self.storage_locations_adjusted_for(organization)
   end
 
   def split_difference
-    pre_adjustment = line_items.partition {|line_item| line_item.quantity.positive?}
-    increasing_adjustment, decreasing_adjustment = pre_adjustment.map {|pre_adjustment| Adjustment.new(line_items: pre_adjustment)  }
+    pre_adjustment = line_items.partition { |line_item| line_item.quantity.positive? }
+    increasing_adjustment, decreasing_adjustment = pre_adjustment.map { |adjustment| Adjustment.new(line_items: adjustment) }
 
     decreasing_adjustment.line_items.each { |line_item| line_item.quantity *= -1 }
     [increasing_adjustment, decreasing_adjustment]
diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index 6b1d41a68c..46208c3640 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -12,7 +12,6 @@ module Itemizable
     end
 
     has_many :line_items, as: :itemizable, inverse_of: :itemizable do
-
       def combine!
         # Bail if there's nothing
         return if size.zero?
diff --git a/spec/support/itemizable_shared_example.rb b/spec/support/itemizable_shared_example.rb
index a2a60d1b17..f9c827271f 100644
--- a/spec/support/itemizable_shared_example.rb
+++ b/spec/support/itemizable_shared_example.rb
@@ -6,7 +6,6 @@
   end
 
   context ".line_items" do
-    
     describe "combine!" do
       it "combines multiple line_items with the same item_id into a single record" do
         storage_location = create(:storage_location, :with_items, item: item, organization: @organization)

From 7c9a714097f6079cf5e8abde531a407aaa76b657 Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 11 May 2019 21:04:45 -0400
Subject: [PATCH 14/32] Incrementally steps towards itemizable_hash

---
 app/models/storage_location.rb | 45 ++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 18 deletions(-)

diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 510d659500..701f00ca9a 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -268,23 +268,29 @@ def update_distribution!(distribution, new_distribution_params)
     false
   end
 
+  # item.active?
+  # item_id
+  # quantity
   # FIXME: After this is stable, revisit how we do logging
   def increase_inventory(itemizable)
+    itemizable_array = itemizable.line_items.map do |l| 
+      { item_id: l.item_id, quantity: l.quantity, active: l.item.active }.with_indifferent_access
+    end
     # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
     # Iterate through each of the line-items in the moving box
-    itemizable.line_items.each do |line_item|
-      if line_item.item.nil? || !line_item.item.active?
+    itemizable_array.each do |item_hash|
+      unless item_hash[:active]
         # If the item was previously hidden (inactive), make it active
-        Item.unscoped.find(line_item.item_id).update(active: true)
-        line_item.reload
+        Item.unscoped.find(item_hash[:item_id]).update(active: true)
+        #line_item.reload
       end
       # Locate the storage box for the item, or create a new storage box for it
-      inventory_item = inventory_items.find_or_create_by!(item: line_item.item)
+      inventory_item = inventory_items.find_or_create_by!(item_id: item_hash[:item_id])
       # Increase the quantity-on-record for that item
-      inventory_item.increment!(:quantity, line_item.quantity)
+      inventory_item.increment!(:quantity, item_hash[:quantity])
       # Record in the log that this has occurred
-      log[line_item.item_id] = "+#{line_item.quantity}"
+      log[item_hash[:item_id]] = "+#{item_hash[:quantity]}"
     end
     # log could be pulled from dirty AR stuff?
     # Save the final changes -- does this need to occur here?
@@ -295,23 +301,26 @@ def increase_inventory(itemizable)
 
   # TODO: re-evaluate this for optimization
   def decrease_inventory(itemizable)
+    itemizable_array = itemizable.line_items.map do |l| 
+      { item_id: l.item_id, name: l.item.name, quantity: l.quantity, active: l.item.active }.with_indifferent_access
+    end
     # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
     # This tracks items that have insufficient inventory counts to be reduced as much
     insufficient_items = []
     # Iterate through each of the line-items in the moving box
-    itemizable.line_items.each do |line_item|
+    itemizable_array.each do |item|
       # Locate the storage box for the item, or create an empty storage box
-      inventory_item = inventory_items.find_by(item: line_item.item) || inventory_items.build
+      inventory_item = inventory_items.find_by(item_id: item[:item_id]) || inventory_items.build
       # If we've got sufficient inventory in the storage box to fill the moving box, then continue
-      next unless inventory_item.quantity < line_item.quantity
+      next unless inventory_item.quantity < item[:quantity]
 
       # Otherwise, we need to record that there was insufficient inventory on-hand
       insufficient_items << {
-        item_id: line_item.item.id,
-        item_name: line_item.item.name,
+        item_id: item[:item_id],
+        item_name: item[:name],
         quantity_on_hand: inventory_item.quantity,
-        quantity_requested: line_item.quantity
+        quantity_requested: item[:quantity]
       }
     end
 
@@ -321,21 +330,21 @@ def decrease_inventory(itemizable)
       # Raise this custom error with information about each of the items that showed insufficient
       # This bails out of the method!
       raise Errors::InsufficientAllotment.new(
-        "Requested #{itemizable.class.name} items exceed the available inventory",
+        "Requested items exceed the available inventory",
         insufficient_items
       )
     end
 
     # Re-run through the items in the moving box again
-    itemizable.line_items.each do |line_item|
+    itemizable_array.each do |item|
       # Look for the moving box for this item -- we know there is sufficient quantity this time
       # Raise AR:RNF if it fails to find it -- though that seems moot since it would have been
       # captured by the previous block.
-      inventory_item = inventory_items.find_by(item: line_item.item)
+      inventory_item = inventory_items.find_by(item_id: item[:item_id])
       # Reduce the inventory box quantity
-      inventory_item.decrement!(:quantity, line_item.quantity)
+      inventory_item.decrement!(:quantity, item[:quantity])
       # Record in the log that this has occurred
-      log[line_item.item_id] = "-#{line_item.quantity}"
+      log[item[:item_id]] = "-#{item[:quantity]}"
     end
     # log could be pulled from dirty AR stuff
     save!

From cca4a3456654700e55b7fa6677e6e739b170cb0d Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 11 May 2019 21:08:33 -0400
Subject: [PATCH 15/32] Adds Itemizable.to_a, adds guard clause to
 StorageLocation inventory change methods

---
 app/models/concerns/itemizable.rb |  6 ++++++
 app/models/storage_location.rb    | 13 +++++--------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index 46208c3640..4f9fe24b2f 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -76,6 +76,12 @@ def value_per_itemizable
     line_items.sum(&:value_per_line_item)
   end
 
+  def to_a
+    line_items.map do |l| 
+      { item_id: l.item_id, name: l.item.name, quantity: l.quantity, active: l.item.active }.with_indifferent_access
+    end
+  end
+
   private
 
   def line_item_items_quantity_is_positive
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 701f00ca9a..d50f4f3419 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -272,10 +272,9 @@ def update_distribution!(distribution, new_distribution_params)
   # item_id
   # quantity
   # FIXME: After this is stable, revisit how we do logging
-  def increase_inventory(itemizable)
-    itemizable_array = itemizable.line_items.map do |l| 
-      { item_id: l.item_id, quantity: l.quantity, active: l.item.active }.with_indifferent_access
-    end
+  def increase_inventory(itemizable_array)
+    itemizable_array = itemizable_array.is_a?(Array) ? itemizable_array : itemizable_array.to_a
+
     # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
     # Iterate through each of the line-items in the moving box
@@ -300,10 +299,8 @@ def increase_inventory(itemizable)
   end
 
   # TODO: re-evaluate this for optimization
-  def decrease_inventory(itemizable)
-    itemizable_array = itemizable.line_items.map do |l| 
-      { item_id: l.item_id, name: l.item.name, quantity: l.quantity, active: l.item.active }.with_indifferent_access
-    end
+  def decrease_inventory(itemizable_array)
+    itemizable_array = itemizable_array.is_a?(Array) ? itemizable_array : itemizable_array.to_a
     # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
     # This tracks items that have insufficient inventory counts to be reduced as much

From c3fe20bf142e32df88c3ee0fd514770e7de8fdce Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 11 May 2019 21:44:29 -0400
Subject: [PATCH 16/32] Removes intake\!; replaced with increase_inventory.

---
 app/controllers/donations_controller.rb |  2 +-
 app/models/concerns/itemizable.rb       |  2 +-
 app/models/storage_location.rb          | 20 ++--------
 spec/features/item_spec.rb              |  4 +-
 spec/models/storage_location_spec.rb    | 51 ++++++++-----------------
 5 files changed, 23 insertions(+), 56 deletions(-)

diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb
index 8cec090551..3b0fad315b 100644
--- a/app/controllers/donations_controller.rb
+++ b/app/controllers/donations_controller.rb
@@ -63,7 +63,7 @@ def scale_intake
                                 line_items_attributes: { "0" => { "item_id" => params["diaper_type"],
                                                                   "quantity" => params["number_of_diapers"],
                                                                   "_destroy" => "false" } })
-    @donation.storage_location.intake! @donation
+    @donation.storage_location.increase_inventory @donation
     render status: :ok, json: @donation.to_json
   end
 
diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index 4f9fe24b2f..9e9dc79bc2 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -77,7 +77,7 @@ def value_per_itemizable
   end
 
   def to_a
-    line_items.map do |l| 
+    line_items.map do |l|
       { item_id: l.item_id, name: l.item.name, quantity: l.quantity, active: l.item.active }.with_indifferent_access
     end
   end
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index d50f4f3419..0ef7929645 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -71,18 +71,6 @@ def to_csv
     end
   end
 
-  def intake!(adjustment)
-    log = {}
-    adjustment.line_items.each do |line_item|
-      inventory_item = InventoryItem.find_or_create_by(storage_location_id: id,
-                                                       item_id: line_item.item_id)
-      inventory_item.quantity += line_item&.quantity || 0
-      inventory_item.save
-      log[line_item.item_id] = "+#{line_item.quantity}"
-    end
-    log
-  end
-
   # NOTE: This knows too much about donations/purchases
   # NOTE: Can we do logging better? It seems like a side effect
   def remove!(itemizable)
@@ -174,11 +162,12 @@ def self.import_csv(csv, organization)
   def self.import_inventory(filename, org, loc)
     current_org = Organization.find(org)
     adjustment = current_org.adjustments.create(storage_location_id: loc.to_i, comment: "Starting Inventory")
-    CSV.parse(filename, headers: false) do |row|
+    # NOTE: this was originally headers: false; it may create buggy behavior
+    CSV.parse(filename, headers: true) do |row|
       adjustment.line_items
                 .create(quantity: row[0].to_i, item_id: current_org.items.find_by(name: row[1]))
     end
-    adjustment.storage_location.intake!(adjustment)
+    adjustment.storage_location.increase_inventory(adjustment)
   end
 
   # Used to move inventory between StorageLocations; reflects items being physically moved
@@ -268,9 +257,6 @@ def update_distribution!(distribution, new_distribution_params)
     false
   end
 
-  # item.active?
-  # item_id
-  # quantity
   # FIXME: After this is stable, revisit how we do logging
   def increase_inventory(itemizable_array)
     itemizable_array = itemizable_array.is_a?(Array) ? itemizable_array : itemizable_array.to_a
diff --git a/spec/features/item_spec.rb b/spec/features/item_spec.rb
index 7339992039..3b9f86ab68 100644
--- a/spec/features/item_spec.rb
+++ b/spec/features/item_spec.rb
@@ -88,8 +88,8 @@
     let(:donation_tampons) { create(:donation, :with_items, storage_location: storage, item_quantity: num_tampons_in_donation, item: item_tampons) }
     let(:donation_aux_tampons) { create(:donation, :with_items, storage_location: aux_storage, item_quantity: num_tampons_second_donation, item: item_tampons) }
     before do
-      storage.intake!(donation_tampons)
-      aux_storage.intake!(donation_aux_tampons)
+      storage.increase_inventory(donation_tampons)
+      aux_storage.increase_inventory(donation_aux_tampons)
       visit url_prefix + "/items"
     end
     # Consolidated these into one to reduce the setup/teardown
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index 88c9bbe1cb..e346078169 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -38,13 +38,15 @@
 
   context "Methods >" do
     describe "increase_inventory" do
-      let(:donation) { create(:donation, :with_items, item_quantity: 66, organization: organization) }
-      it "increases inventory quantities from an itemizable object" do
-        item = create(:item)
-        storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
-        expect do
-          storage_location.increase_inventory(donation)
-        end.to change { storage_location.size }.by(66)
+      context "With existing inventory" do
+        let(:donation) { create(:donation, :with_items, item_quantity: 66, organization: organization) }
+        it "increases inventory quantities from an itemizable object" do
+          item = create(:item)
+          storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
+          expect do
+            storage_location.increase_inventory(donation.to_a)
+          end.to change { storage_location.size }.by(66)
+        end
       end
 
       context "when providing a new item that does not yet exist" do
@@ -54,7 +56,7 @@
           item = create(:item)
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
           expect do
-            storage_location.increase_inventory(donation_with_new_items)
+            storage_location.increase_inventory(donation_with_new_items.to_a)
           end.to change { storage_location.inventory_items.count }.by(1)
         end
       end
@@ -66,7 +68,7 @@
       it "decreases inventory quantities from an itemizable object" do
         storage_location = create(:storage_location, :with_items, item_quantity: 100, item: item, organization: @organization)
         expect do
-          storage_location.decrease_inventory(distribution)
+          storage_location.decrease_inventory(distribution.to_a)
         end.to change { storage_location.size }.by(-66)
       end
 
@@ -75,7 +77,7 @@
         it "gives informative errors" do
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
           expect do
-            storage_location.decrease_inventory(distribution_but_too_much).errors
+            storage_location.decrease_inventory(distribution_but_too_much.to_a).errors
           end.to raise_error(Errors::InsufficientAllotment)
         end
 
@@ -83,7 +85,7 @@
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
           starting_size = storage_location.size
           begin
-            storage_location.decrease_inventory(distribution)
+            storage_location.decrease_inventory(distribution.to_a)
           rescue Errors::InsufficientAllotment
           end
           storage_location.reload
@@ -130,33 +132,12 @@
       end
     end
 
-    describe "intake!" do
-      it "adds items to a storage location even if none exist" do
-        storage_location = create(:storage_location)
-        donation = create(:donation, :with_items, item_quantity: 10)
-        expect do
-          storage_location.intake!(donation)
-          storage_location.items.reload
-        end.to change { storage_location.items.count }.by(1)
-        expect(storage_location.size).to eq(10)
-      end
-
-      it "adds items to the storage location total if that item already exists in inventory" do
-        storage_location = create(:storage_location, :with_items, item_quantity: 10)
-        donation = create(:donation, :with_items, item_quantity: 10, item: storage_location.inventory_items.first.item)
-        storage_location.intake!(donation)
-
-        expect(storage_location.inventory_items.count).to eq(1)
-        expect(storage_location.inventory_items.where(item_id: donation.line_items.first.item).first.quantity).to eq(20)
-      end
-    end
-
     describe "remove!" do
       let(:storage_location) { create(:storage_location) }
       let(:donation)         { create(:donation, :with_items, item_quantity: 10) }
 
       before(:each) do
-        storage_location.intake!(donation)
+        storage_location.increase_inventory(donation)
         storage_location.items.reload
 
         expect(storage_location.size).to eq(10)
@@ -179,7 +160,7 @@
 
       context "with_donations" do
         before(:each) do
-          storage_location.intake!(donation)
+          storage_location.increase_inventory(donation)
         end
 
         it "updates the quantity of items" do
@@ -204,7 +185,7 @@
       end
       context "With purchases" do
         before(:each) do
-          storage_location.intake!(purchase)
+          storage_location.increase_inventory(purchase)
           storage_location.items.reload
         end
 

From c4d8369f3c6e506117d4aee19c179f69d1de33fb Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 11 May 2019 21:55:11 -0400
Subject: [PATCH 17/32] Removes remove\! from StorageLocation

---
 app/models/storage_location.rb       | 20 +-------------------
 spec/models/storage_location_spec.rb | 21 ---------------------
 2 files changed, 1 insertion(+), 40 deletions(-)

diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 0ef7929645..c41092a647 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -71,24 +71,6 @@ def to_csv
     end
   end
 
-  # NOTE: This knows too much about donations/purchases
-  # NOTE: Can we do logging better? It seems like a side effect
-  def remove!(itemizable)
-    log = {}
-    itemizable.line_items.each do |line_item|
-      inventory_item = InventoryItem.find_by(storage_location: id, item_id: line_item.item_id)
-      # NOTE: Code smell - are we sure we want to allow this to be destroyed if < 0?
-      if (inventory_item.quantity - line_item.quantity) <= 0
-        # NOTE: Instead of deleting, maybe we could leave it at 0, and hide it in the UI instead
-        inventory_item.destroy
-      else
-        inventory_item.update(quantity: inventory_item.quantity - line_item.quantity)
-      end
-      log[line_item.item_id] = "-#{line_item.quantity}"
-    end
-    log
-  end
-
   # NOTE: Make this code clearer in its intent -- needs more context
   def adjust_from_past!(itemizable, previous_line_item_values)
     itemizable.line_items.each do |line_item|
@@ -268,7 +250,7 @@ def increase_inventory(itemizable_array)
       unless item_hash[:active]
         # If the item was previously hidden (inactive), make it active
         Item.unscoped.find(item_hash[:item_id]).update(active: true)
-        #line_item.reload
+        # line_item.reload
       end
       # Locate the storage box for the item, or create a new storage box for it
       inventory_item = inventory_items.find_or_create_by!(item_id: item_hash[:item_id])
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index e346078169..c5b775354e 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -132,27 +132,6 @@
       end
     end
 
-    describe "remove!" do
-      let(:storage_location) { create(:storage_location) }
-      let(:donation)         { create(:donation, :with_items, item_quantity: 10) }
-
-      before(:each) do
-        storage_location.increase_inventory(donation)
-        storage_location.items.reload
-
-        expect(storage_location.size).to eq(10)
-        expect(storage_location.items.count).to eq(1)
-      end
-
-      it "removes items from a storage location" do
-        expect do
-          storage_location.remove!(donation)
-        end.to change { storage_location.size }.by(-donation.total_quantity)
-                                               .and change { storage_location.inventory_items.size }.by(-donation.line_items.count)
-                                                                                                    .and change { InventoryItem.count }.by(-donation.line_items.count)
-      end
-    end
-
     describe "adjust_from_past!" do
       let(:storage_location) { create(:storage_location) }
       let(:purchase)         { create(:purchase, :with_items, item_quantity: 10) }

From 127b44122e7892539251584af7dc459773c2c12b Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 11 May 2019 22:05:31 -0400
Subject: [PATCH 18/32] Removes adjust\! method.

---
 app/controllers/audits_controller.rb |  6 +++++-
 app/models/storage_location.rb       | 26 --------------------------
 spec/models/storage_location_spec.rb | 20 --------------------
 3 files changed, 5 insertions(+), 47 deletions(-)

diff --git a/app/controllers/audits_controller.rb b/app/controllers/audits_controller.rb
index 57b81fac5e..b9ae164ee1 100644
--- a/app/controllers/audits_controller.rb
+++ b/app/controllers/audits_controller.rb
@@ -33,7 +33,11 @@ def finalize
       end
     end
 
-    @audit.adjustment.storage_location.adjust!(@audit.adjustment)
+    increasing_adjustment, decreasing_adjustment = @audit.adjustment.split_difference
+    ActiveRecord::Base.transaction do
+      @audit.storage_location.increase_inventory increasing_adjustment
+      @audit.storage_location.decrease_inventory decreasing_adjustment
+    end
     @audit.finalized!
     redirect_to audit_path(@audit), notice: "Audit is Finalized."
   end
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index c41092a647..c948116e1b 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -187,32 +187,6 @@ def move_inventory!(transfer)
     update_inventory_inventory_items(updated_quantities)
   end
 
-  # Used to adjust inventory at a StorageLocation to reflect reality
-  # Ex: we thought we had 200 size "5" diapers, but we actually have 180 size "5" diapers
-  # NOTE: Has too much knowledge about adjustments -- should be moved to `Adjustment`
-  def adjust!(adjustment)
-    updated_quantities = {}
-    item_validator = Errors::InsufficientAllotment.new("Adjustment exceeds the available inventory")
-
-    adjustment.line_items.each do |line_item|
-      inventory_item = inventory_items.find_by(item: line_item.item)
-      next if inventory_item.nil? || inventory_item.quantity.zero?
-
-      if (inventory_item.quantity + line_item.quantity) >= 0
-        updated_quantities[inventory_item.id] = (updated_quantities[inventory_item.id] ||
-                                                 inventory_item.quantity) + line_item.quantity
-      else
-        item_validator.add_insufficiency(line_item.item,
-                                         inventory_item.quantity,
-                                         line_item.quantity)
-      end
-    end
-
-    raise item_validator unless item_validator.satisfied?
-
-    update_inventory_inventory_items(updated_quantities)
-  end
-
   # NOTE: This has WAY too much knowledge of distribution
   def update_distribution!(distribution, new_distribution_params)
     ActiveRecord::Base.transaction do
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index c5b775354e..ab21c1b200 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -231,26 +231,6 @@
       end
     end
 
-    describe "adjust!" do
-      it "combines line item quantities with inventory amounts" do
-        storage_location = create :storage_location, :with_items, item_quantity: 300
-        adjustment = build :adjustment, :with_items, storage_location: storage_location, item_quantity: 50
-        storage_location.adjust!(adjustment)
-        expect(storage_location.inventory_items.first.quantity).to eq 350
-
-        adjustment2 = build :adjustment, :with_items, storage_location: storage_location, item_quantity: -50
-        storage_location.adjust!(adjustment2)
-        expect(storage_location.inventory_items.first.quantity).to eq 300
-      end
-      it "ensures that a user cannot adjust an inventory into the negative" do
-        storage_location = create :storage_location, :with_items, item_quantity: 300
-        adjustment = build :adjustment, :with_items, storage_location: storage_location, item_quantity: -301
-        expect do
-          storage_location.adjust!(adjustment)
-        end.to raise_error(Errors::InsufficientAllotment)
-      end
-    end
-
     describe "move_inventory!" do
       it "removes inventory from a storage location and adds them to another storage location" do
         item = create(:item)

From e3c3ecd1a56f01b3425d95fcab549a182ba8f476 Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 11 May 2019 22:56:30 -0400
Subject: [PATCH 19/32] Removes distribute\! and specs. Fixes two spec
 failures. Adds guard query to Itemizable.to_a

---
 app/models/concerns/itemizable.rb    |  4 +-
 app/models/storage_location.rb       | 61 ----------------------------
 spec/features/adjustment_spec.rb     |  2 +-
 spec/features/distribution_spec.rb   |  6 ++-
 spec/models/storage_location_spec.rb | 26 ------------
 5 files changed, 8 insertions(+), 91 deletions(-)

diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index 9e9dc79bc2..754b3480e4 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -78,7 +78,9 @@ def value_per_itemizable
 
   def to_a
     line_items.map do |l|
-      { item_id: l.item_id, name: l.item.name, quantity: l.quantity, active: l.item.active }.with_indifferent_access
+      # When the item isn't found, it's probably just inactive. This ensures it's available.
+      item = Item.unscoped.find(l.item_id)
+      { item_id: item.id, name: item.name, quantity: l.quantity, active: item.active }.with_indifferent_access
     end
   end
 
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index c948116e1b..33bc35a933 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -94,43 +94,6 @@ def adjust_from_past!(itemizable, previous_line_item_values)
     end
   end
 
-  # This is the "subtract inventory method"
-  # NOTE: This is not returning a log object
-  def distribute!(itemizable)
-    # This is passed to update_inventory_inventory_items
-    updated_quantities = {}
-    # Used in the exception return value
-    insufficient_items = []
-    itemizable.line_items.each do |line_item|
-      inventory_item = inventory_items.find_by(item: line_item.item)
-      # NOTE: If the distribution isn't able to find the inventory item, it continues
-      next if inventory_item.nil?
-
-      if inventory_item.quantity >= line_item.quantity
-        updated_quantities[inventory_item.id] = (updated_quantities[inventory_item.id] ||
-                                                 inventory_item.quantity) - line_item.quantity
-      else
-        insufficient_items << {
-          item_id: line_item.item.id,
-          item_name: line_item.item.name,
-          quantity_on_hand: inventory_item.quantity,
-          quantity_requested: line_item.quantity
-        }
-      end
-    end
-
-    # NOTE: Could this be handled by a validation instead?
-    unless insufficient_items.empty?
-      raise Errors::InsufficientAllotment.new(
-        "#{itemizable.class.name} line_items exceed the available inventory",
-        insufficient_items
-      )
-    end
-
-    # NOTE: This executes the transaction to actually change the data
-    update_inventory_inventory_items(updated_quantities)
-  end
-
   # NOTE: We should generalize this elsewhere -- Importable concern?
   def self.import_csv(csv, organization)
     csv.each do |row|
@@ -290,30 +253,6 @@ def decrease_inventory(itemizable_array)
     # return log
     log
   end
-=begin
-def distribute!(itemizable)
-    # This is passed to update_inventory_inventory_items
-    updated_quantities = {}
-    # Used in the exception return value
-    insufficient_items = []
-    itemizable.line_items.each do |line_item|
-      inventory_item = inventory_items.find_by(item: line_item.item)
-      # NOTE: If the distribution isn't able to find the inventory item, it continues
-      next if inventory_item.nil?
-
-      if inventory_item.quantity >= line_item.quantity
-        updated_quantities[inventory_item.id] = (updated_quantities[inventory_item.id] ||
-                                                 inventory_item.quantity) - line_item.quantity
-      else
-        insufficient_items << {
-          item_id: line_item.item.id,
-          item_name: line_item.item.name,
-          quantity_on_hand: inventory_item.quantity,
-          quantity_requested: line_item.quantity
-        }
-      end
-    end
-=end
 
   def self.csv_export_headers
     ["Name", "Address", "Total Inventory"]
diff --git a/spec/features/adjustment_spec.rb b/spec/features/adjustment_spec.rb
index 9ed8510626..bc653d4308 100644
--- a/spec/features/adjustment_spec.rb
+++ b/spec/features/adjustment_spec.rb
@@ -49,7 +49,7 @@
     expect do
       click_button "Save"
     end.not_to change { storage_location.size }
-    expect(page).to have_content("Requested Adjustment items exceed the available inventory")
+    expect(page).to have_content("items exceed the available inventory")
   end
 
   scenario "User can filter the #index by storage location" do
diff --git a/spec/features/distribution_spec.rb b/spec/features/distribution_spec.rb
index 761512b40b..0bf82c88cb 100644
--- a/spec/features/distribution_spec.rb
+++ b/spec/features/distribution_spec.rb
@@ -61,11 +61,13 @@
     end
 
     context "when one of the items has been 'deleted'" do
-      scenario "the user can still reclaim it and it reactivates the item" do
+      scenario "the user can still reclaim it and it reactivates the item", js: true do
         item = distribution.line_items.first.item
         item.destroy
         expect do
-          click_on "Reclaim"
+          accept_confirm do
+            click_on "Reclaim"
+          end
           page.find ".alert"
         end.to change { Distribution.count }.by(-1).and change { Item.count }.by(1)
         expect(page).to have_content "reclaimed"
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index ab21c1b200..5a5e480ac5 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -181,32 +181,6 @@
       end
     end
 
-    describe "distribute!" do
-      it "distrbutes items from storage location" do
-        storage_location = create :storage_location, :with_items, item_quantity: 300
-        distribution = build :distribution, :with_items, storage_location: storage_location, item_quantity: 50
-        storage_location.distribute!(distribution)
-        expect(storage_location.inventory_items.first.quantity).to eq 250
-      end
-
-      it "raises error when distribution exceeds storage location inventory" do
-        storage_location = create :storage_location, :with_items, item_quantity: 300
-        distribution = build :distribution, :with_items, storage_location: storage_location, item_quantity: 350
-        item = distribution.line_items.first.item
-        expect do
-          storage_location.distribute!(distribution)
-        end.to raise_error do |error|
-          expect(error).to be_a Errors::InsufficientAllotment
-          expect(error.insufficient_items).to include(
-            item_id: item.id,
-            item_name: item.name,
-            quantity_on_hand: 300,
-            quantity_requested: 350
-          )
-        end
-      end
-    end
-
     describe "import_csv" do
       it "imports storage locations from a csv file" do
         organization

From 26c9c86acaa575352ecf2adfe315837b33b8f1bc Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 11 May 2019 23:04:03 -0400
Subject: [PATCH 20/32] Removes move_inventory\! -- adds additional feature
 spec to ensure insufficient inventory condition is handled.

---
 app/models/storage_location.rb       | 35 ----------------------------
 spec/features/transfer_spec.rb       | 19 +++++++++++++++
 spec/models/storage_location_spec.rb | 24 -------------------
 3 files changed, 19 insertions(+), 59 deletions(-)

diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 33bc35a933..f6a40dba19 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -115,41 +115,6 @@ def self.import_inventory(filename, org, loc)
     adjustment.storage_location.increase_inventory(adjustment)
   end
 
-  # Used to move inventory between StorageLocations; reflects items being physically moved
-  # Ex: move 500 size "2" diapers from main warehouse to overflow warehouse because insufficient space in main warehouse
-  # This could all be moved over to the `Transfer` model
-  def move_inventory!(transfer)
-    # Contains the total deltas from/to for all the InventoryItem records that are being changed
-    updated_quantities = {}
-    item_validator = Errors::InsufficientAllotment.new("Transfer items exceeds \
-                                                        the available inventory")
-    transfer.line_items.each do |line_item|
-      # NOTE: We should do it this way more
-      from_inventory_item = inventory_items.find_by(item: line_item.item)
-      # NOTE: Initialize the inventory item on the destination storage location; "to" = "a storage location"
-      to_inventory_item = transfer.to.inventory_items.find_or_create_by(item: line_item.item)
-      # NOTE: this is for if the transfer includes inventory items that are nonexistent / zero, we can't transfer them
-      # maybe we could do this as a validation, or at the model level instead?
-      next if from_inventory_item.nil? || from_inventory_item.quantity.zero?
-
-      if from_inventory_item.quantity >= line_item.quantity
-        # NOTE: this is subtracting the inventory found on each line item, from the running total of inventory available at the source location
-        updated_quantities[from_inventory_item.id] = (updated_quantities[from_inventory_item.id] || from_inventory_item.quantity) - line_item.quantity
-        # NOTE: this is adding the inventory found to the new destination at the new storage location
-        updated_quantities[to_inventory_item.id] = (updated_quantities[to_inventory_item.id] || to_inventory_item.quantity) + line_item.quantity
-      else
-        item_validator.add_insufficiency(line_item.item,
-                                         from_inventory_item.quantity,
-                                         line_item.quantity)
-      end
-    end
-
-    raise item_validator unless item_validator.satisfied?
-
-    # NOTE: Run the transaction
-    update_inventory_inventory_items(updated_quantities)
-  end
-
   # NOTE: This has WAY too much knowledge of distribution
   def update_distribution!(distribution, new_distribution_params)
     ActiveRecord::Base.transaction do
diff --git a/spec/features/transfer_spec.rb b/spec/features/transfer_spec.rb
index 9de8a2940f..c037d41faa 100644
--- a/spec/features/transfer_spec.rb
+++ b/spec/features/transfer_spec.rb
@@ -20,6 +20,25 @@
     expect(page).to have_content("10 items have been transferred")
   end
 
+  context "when there's insufficient inventory at the origin to cover the move" do
+    let!(:from_storage_location) { create(:storage_location, :with_items, item_quantity: 10, name: "From me", organization: @organization) }
+    let!(:to_storage_location) { create(:storage_location, :with_items, name: "To me", organization: @organization) }
+
+    scenario "User can transfer an inventory from a storage location to another" do
+      visit url_prefix + "/transfers"
+      click_link "New Transfer"
+      within "form#new_transfer" do
+        select from_storage_location.name, from: "From storage location"
+        select to_storage_location.name, from: "To storage location"
+        fill_in "Comment", with: "something"
+        select from_storage_location.items.first.name, from: "transfer_line_items_attributes_0_item_id"
+        fill_in "transfer_line_items_attributes_0_quantity", with: "100"
+        click_on "Save"
+      end
+      expect(page).to have_content("exceed the available inventory")
+    end
+  end
+
   scenario "User can filter the #index by storage location both from and to" do
     from_storage_location = create(:storage_location, name: "here", organization: @organization)
     to_storage_location = create(:storage_location, name: "there", organization: @organization)
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index 5a5e480ac5..25c142e6a8 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -205,30 +205,6 @@
       end
     end
 
-    describe "move_inventory!" do
-      it "removes inventory from a storage location and adds them to another storage location" do
-        item = create(:item)
-        storage_location = create :storage_location, :with_items, item: item, item_quantity: 300
-        storage_location2 = create :storage_location, :with_items, item: item, item_quantity: 100
-        transfer = build :transfer, :with_items, item: item, item_quantity: 100,
-                                                 from: storage_location, to: storage_location2
-        storage_location.move_inventory!(transfer)
-        expect(storage_location.inventory_items.first.quantity).to eq 200
-        expect(storage_location2.inventory_items.first.quantity).to eq 200
-      end
-
-      it "raises error when distribution exceeds inventory in a storage facility" do
-        item = create(:item)
-        storage_location = create :storage_location, :with_items, item: item, item_quantity: 100
-        storage_location2 = create :storage_location, :with_items, item: item, item_quantity: 100
-        transfer = build :transfer, :with_items, item: item, item_quantity: 200,
-                                                 from_id: storage_location.id, to_id: storage_location2.id
-        expect do
-          storage_location.move_inventory!(transfer)
-        end.to raise_error(Errors::InsufficientAllotment)
-      end
-    end
-
     describe "geocode" do
       it "adds coordinates to the database" do
         storage_location = build(:storage_location,

From e20f1a6458243bd3e229a91551378c939e34733c Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sun, 12 May 2019 00:03:11 -0400
Subject: [PATCH 21/32] Moves update_distribution\! from StorageLocation to
 Distribution, renames to replace_distribution\! -- fixed a few specs as well.
 Adds pry-nav Gem.

---
 Gemfile                                     |  1 +
 Gemfile.lock                                |  3 ++
 app/controllers/distributions_controller.rb |  2 +-
 app/models/distribution.rb                  | 20 ++++++++++++
 app/models/storage_location.rb              | 35 ++-------------------
 spec/features/distribution_spec.rb          |  5 +--
 6 files changed, 31 insertions(+), 35 deletions(-)

diff --git a/Gemfile b/Gemfile
index ed047ed4b0..22a59f1342 100644
--- a/Gemfile
+++ b/Gemfile
@@ -51,6 +51,7 @@ group :development, :test do
   gem "awesome_print"
   gem "guard-rspec"
   gem "pry-rails"
+  gem "pry-nav"
   gem "rspec-rails", "~> 3.8"
   gem "terminal-notifier-guard"
   gem "terminal-notifier"
diff --git a/Gemfile.lock b/Gemfile.lock
index 44a9ffe320..a3b91597dd 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -275,6 +275,8 @@ GEM
     pry (0.12.2)
       coderay (~> 1.1.0)
       method_source (~> 0.9.0)
+    pry-nav (0.3.0)
+      pry (>= 0.9.10, < 0.13.0)
     pry-rails (0.3.9)
       pry (>= 0.10.4)
     public_suffix (3.0.3)
@@ -508,6 +510,7 @@ DEPENDENCIES
   paperclip
   pg (~> 1.1.3)
   prawn-rails
+  pry-nav
   pry-rails
   puma
   rails (~> 5.2.2)
diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb
index bbd2511ee3..405c94a430 100644
--- a/app/controllers/distributions_controller.rb
+++ b/app/controllers/distributions_controller.rb
@@ -96,7 +96,7 @@ def update
     # see examples: https://stackoverflow.com/questions/13605598/how-to-get-a-date-from-date-select-or-select-date-in-rails
     old_issued_at = distribution.issued_at
 
-    if distribution.storage_location.update_distribution!(distribution, distribution_params)
+    if distribution.replace_distribution!(distribution_params)
       @distribution = Distribution.includes(:line_items).includes(:storage_location).find(params[:id])
       @line_items = @distribution.line_items
 
diff --git a/app/models/distribution.rb b/app/models/distribution.rb
index 4092333ee1..aa64d7bc26 100644
--- a/app/models/distribution.rb
+++ b/app/models/distribution.rb
@@ -50,6 +50,26 @@ class Distribution < ApplicationRecord
 
   delegate :name, to: :partner, prefix: true
 
+  # TODO: kill me
+  def replace_distribution!(new_distribution_params)
+    ActiveRecord::Base.transaction do
+      # fixed_distribution_params = new_distribution_params["line_items_attributes"].to_h.values.reject { |f| f["item_id"].blank? && f["quantity"].blank? }
+      # Roll back distribution output by increasing storage location
+      storage_location.increase_inventory(to_a)
+      # Delete the line items -- they'll be replaced later
+      line_items.map(&:destroy!)
+      reload
+
+      # Replace the current distribution with the new parameters
+      update! new_distribution_params
+
+      # Apply the new changes to the storage location inventory
+      storage_location.decrease_inventory(to_a)
+    end
+  rescue ActiveRecord::RecordInvalid
+    false
+  end
+
   def distributed_at
     if is_midnight(issued_at)
       issued_at.to_s(:distribution_date)
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index f6a40dba19..ebf58d0c89 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -115,32 +115,6 @@ def self.import_inventory(filename, org, loc)
     adjustment.storage_location.increase_inventory(adjustment)
   end
 
-  # NOTE: This has WAY too much knowledge of distribution
-  def update_distribution!(distribution, new_distribution_params)
-    ActiveRecord::Base.transaction do
-      distribution.line_items.each do |line_item|
-        inventory_item = inventory_items.find_or_create_by!(item: line_item.item)
-        inventory_item.update!(quantity: (inventory_item.quantity || 0) + line_item.quantity)
-        line_item.destroy!
-      end
-      distribution = distribution.reload
-      distribution.update! new_distribution_params
-
-      distribution.line_items.each do |line_item|
-        inventory_item = inventory_items.find_by(item: line_item.item)
-        raise ActiveRecord::Rollback, "Failed to update distribution, please contact tech support if this problem persists" if inventory_item.nil?
-
-        if inventory_item.quantity == line_item.quantity # otherwise this would make the quantity 0 and an exception would be thrown
-          inventory_item.destroy!
-        else
-          inventory_item.update!(quantity: inventory_item.quantity - line_item.quantity)
-        end
-      end
-    end
-  rescue ActiveRecord::RecordInvalid
-    false
-  end
-
   # FIXME: After this is stable, revisit how we do logging
   def increase_inventory(itemizable_array)
     itemizable_array = itemizable_array.is_a?(Array) ? itemizable_array : itemizable_array.to_a
@@ -149,15 +123,12 @@ def increase_inventory(itemizable_array)
     log = {}
     # Iterate through each of the line-items in the moving box
     itemizable_array.each do |item_hash|
-      unless item_hash[:active]
-        # If the item was previously hidden (inactive), make it active
-        Item.unscoped.find(item_hash[:item_id]).update(active: true)
-        # line_item.reload
-      end
+      # TODO: make this an aggregate change
+      Item.unscoped.find(item_hash[:item_id]).update(active: true)
       # Locate the storage box for the item, or create a new storage box for it
       inventory_item = inventory_items.find_or_create_by!(item_id: item_hash[:item_id])
       # Increase the quantity-on-record for that item
-      inventory_item.increment!(:quantity, item_hash[:quantity])
+      inventory_item.increment!(:quantity, item_hash[:quantity].to_i)
       # Record in the log that this has occurred
       log[item_hash[:item_id]] = "+#{item_hash[:quantity]}"
     end
diff --git a/spec/features/distribution_spec.rb b/spec/features/distribution_spec.rb
index 0bf82c88cb..9433648acc 100644
--- a/spec/features/distribution_spec.rb
+++ b/spec/features/distribution_spec.rb
@@ -141,13 +141,14 @@
         expect(page).to have_content 13
       end
 
-      scenario "User creates a distribution from a donation then tries to make the quantity too big" do
+      scenario "User creates a distribution from a donation then tries to make the quantity too big", js: true do
         within "#edit_distribution_#{@distribution.to_param}" do
           first(".numeric").set 999_999
           click_on "Save"
         end
         expect(page).to have_no_content "Distribution updated!"
-        expect(page).to have_content "Distribution could not be updated!"
+        # NOTE: This is rendering the app/views/errors/insufficient.html.erb template
+        expect(page).to have_content /Insufficient/i
         expect(page).to have_no_content 999_999
         expect(Distribution.first.line_items.count).to eq 1
       end

From 64f2a9ba964f3b20d13f77b022114f97b616b6bd Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sun, 12 May 2019 00:29:45 -0400
Subject: [PATCH 22/32] Migrated adjust_from_past\! to Itemizable, renamed to
 replace_increase\!. line_items_quantities now just aliases for
 Itemizable#to_a.

---
 app/models/concerns/itemizable.rb         | 21 ++++++++++++---
 app/models/storage_location.rb            | 33 -----------------------
 spec/features/distribution_spec.rb        |  2 +-
 spec/support/itemizable_shared_example.rb |  4 +--
 4 files changed, 20 insertions(+), 40 deletions(-)

diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index 754b3480e4..cda6654ff4 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -65,11 +65,13 @@ def total
     validates_associated :line_items
   end
 
+  # TODO: Remove this
+  # app/controllers/purchases_controller.rb
+  # app/controllers/donations_controller.rb
+  # app/models/concerns/itemizable.rb
+  # spec/models/storage_location_spec.rb
   def line_items_quantities
-    line_items.inject(Hash.new) do |hash, line_item|
-      hash[line_item.id] = OpenStruct.new(quantity: line_item.quantity, item_id: line_item.item_id)
-      hash
-    end
+    to_a
   end
 
   def value_per_itemizable
@@ -84,6 +86,17 @@ def to_a
     end
   end
 
+  def replace_increase!(previous_line_item_values)
+    ActiveRecord::Base.transaction do
+      # Roll back distribution output by increasing storage location
+      storage_location.increase_inventory(to_a)
+      # Apply the new changes to the storage location inventory
+      storage_location.decrease_inventory(previous_line_item_values)
+    end
+  rescue ActiveRecord::RecordInvalid
+    false
+  end
+
   private
 
   def line_item_items_quantity_is_positive
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index ebf58d0c89..ab9fd30a9f 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -71,29 +71,6 @@ def to_csv
     end
   end
 
-  # NOTE: Make this code clearer in its intent -- needs more context
-  def adjust_from_past!(itemizable, previous_line_item_values)
-    itemizable.line_items.each do |line_item|
-      # NOTE: Can't we do an association lookup, instead of going all the way up to the model?
-      inventory_item = InventoryItem.find_or_create_by(storage_location_id: id, item_id: line_item.item_id)
-      # If the item wasn't deleted by the user, then it will be present to be deleted
-      # here, and delete returns the item as a return value.
-      if previous_line_item_value = previous_line_item_values.delete(line_item.id)
-        inventory_item.quantity += line_item.quantity
-        inventory_item.quantity -= previous_line_item_value.quantity
-        inventory_item.save!
-      end
-      inventory_item.destroy! if inventory_item.quantity.zero?
-    end
-    # Update storage for line items that are no longer persisted because they
-    # were removed during the update/delete process.
-    previous_line_item_values.values.each do |value|
-      inventory_item = InventoryItem.find_or_create_by(storage_location_id: id, item_id: value.item_id)
-      inventory_item.decrement!(:quantity, value.quantity)
-      inventory_item.destroy! if inventory_item.quantity.zero?
-    end
-  end
-
   # NOTE: We should generalize this elsewhere -- Importable concern?
   def self.import_csv(csv, organization)
     csv.each do |row|
@@ -197,14 +174,4 @@ def self.csv_export_headers
   def csv_export_attributes
     [name, address, size]
   end
-
-  private
-
-  def update_inventory_inventory_items(records)
-    ActiveRecord::Base.transaction do
-      records.each do |inventory_item_id, quantity|
-        InventoryItem.find(inventory_item_id).update(quantity: quantity)
-      end
-    end
-  end
 end
diff --git a/spec/features/distribution_spec.rb b/spec/features/distribution_spec.rb
index 9433648acc..9656a4264e 100644
--- a/spec/features/distribution_spec.rb
+++ b/spec/features/distribution_spec.rb
@@ -148,7 +148,7 @@
         end
         expect(page).to have_no_content "Distribution updated!"
         # NOTE: This is rendering the app/views/errors/insufficient.html.erb template
-        expect(page).to have_content /Insufficient/i
+        expect(page).to have_content(/Insufficient/i)
         expect(page).to have_no_content 999_999
         expect(Distribution.first.line_items.count).to eq 1
       end
diff --git a/spec/support/itemizable_shared_example.rb b/spec/support/itemizable_shared_example.rb
index f9c827271f..72c41be2dd 100644
--- a/spec/support/itemizable_shared_example.rb
+++ b/spec/support/itemizable_shared_example.rb
@@ -105,10 +105,10 @@
       it "updates storage location quantity by the correct amount" do
         line_item = subject.line_items.create(item_id: item.id, quantity: 10)
         inventory_item = create(:inventory_item, storage_location: storage_location, item_id: line_item.item_id)
-        previous_quantities = subject.line_items_quantities
+        previous_quantities = subject.to_a
         line_item.update!(quantity: 5)
         expect do
-          storage_location.adjust_from_past!(subject, previous_quantities)
+          subject.replace_increase!(previous_quantities)
         end.to change { inventory_item.reload.quantity }.by(-5)
       end
     end

From 82b6efce6db1b6176a22c10d1b847dce12850a2a Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sun, 12 May 2019 01:10:16 -0400
Subject: [PATCH 23/32] Replaces adjust_from_past\! with replace_increase\! in
 controllers. Fixes specs. Removes line_items_quantities entirely. Fixes a
 factory.

---
 app/controllers/donations_controller.rb   |  5 ++-
 app/controllers/purchases_controller.rb   |  5 ++-
 app/models/concerns/itemizable.rb         | 20 ---------
 app/models/donation.rb                    | 13 ++++++
 app/models/purchase.rb                    | 13 ++++++
 spec/factories/adjustments.rb             | 15 ++-----
 spec/models/donation_spec.rb              | 25 ++++++++++++
 spec/models/purchase_spec.rb              | 25 ++++++++++++
 spec/models/storage_location_spec.rb      | 49 -----------------------
 spec/support/itemizable_shared_example.rb | 15 -------
 10 files changed, 86 insertions(+), 99 deletions(-)

diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb
index 3b0fad315b..c65ed29c62 100644
--- a/app/controllers/donations_controller.rb
+++ b/app/controllers/donations_controller.rb
@@ -100,9 +100,10 @@ def show
 
   def update
     @donation = Donation.find(params[:id])
-    previous_quantities = @donation.line_items_quantities
+    previous_quantities = @donation.to_a
     if @donation.update(donation_params)
-      @donation.storage_location.adjust_from_past!(@donation, previous_quantities)
+      # TODO: Move update into replace_increase!
+      @donation.replace_increase!(@donation, previous_quantities)
       redirect_to donations_path
     else
       render "edit"
diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb
index dbd1dadbf5..f0f9b11741 100644
--- a/app/controllers/purchases_controller.rb
+++ b/app/controllers/purchases_controller.rb
@@ -50,9 +50,10 @@ def show
   def update
     @purchase = current_organization.purchases.find(params[:id])
     @purchase.changed?
-    previous_quantities = @purchase.line_items_quantities
+    previous_quantities = @purchase.to_a
     if @purchase.update(purchase_params)
-      @purchase.storage_location.adjust_from_past!(@purchase, previous_quantities)
+      # TODO: Revisit this. Move update into method.
+      @purchase.replace_increase!(previous_quantities)
       redirect_to purchases_path
     else
       render "edit"
diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index cda6654ff4..2b55021aa6 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -65,15 +65,6 @@ def total
     validates_associated :line_items
   end
 
-  # TODO: Remove this
-  # app/controllers/purchases_controller.rb
-  # app/controllers/donations_controller.rb
-  # app/models/concerns/itemizable.rb
-  # spec/models/storage_location_spec.rb
-  def line_items_quantities
-    to_a
-  end
-
   def value_per_itemizable
     line_items.sum(&:value_per_line_item)
   end
@@ -86,17 +77,6 @@ def to_a
     end
   end
 
-  def replace_increase!(previous_line_item_values)
-    ActiveRecord::Base.transaction do
-      # Roll back distribution output by increasing storage location
-      storage_location.increase_inventory(to_a)
-      # Apply the new changes to the storage location inventory
-      storage_location.decrease_inventory(previous_line_item_values)
-    end
-  rescue ActiveRecord::RecordInvalid
-    false
-  end
-
   private
 
   def line_item_items_quantity_is_positive
diff --git a/app/models/donation.rb b/app/models/donation.rb
index ec3719c473..2bc2b93057 100644
--- a/app/models/donation.rb
+++ b/app/models/donation.rb
@@ -102,6 +102,19 @@ def track(item, quantity)
     end
   end
 
+  def replace_increase!(previous_line_item_values)
+    ActiveRecord::Base.transaction do
+      # Roll back distribution output by increasing storage location
+      storage_location.increase_inventory(to_a)
+      # Apply the new changes to the storage location inventory
+      storage_location.decrease_inventory(previous_line_item_values)
+      # TODO: Discuss this -- *should* we be removing InventoryItems when they hit 0 count?
+      storage_location.inventory_items.where(quantity: 0).destroy_all
+    end
+  rescue ActiveRecord::RecordInvalid
+    false
+  end
+
   def remove(item)
     # doing this will handle either an id or an object
     item_id = item.to_i
diff --git a/app/models/purchase.rb b/app/models/purchase.rb
index 2576ee94f2..95d1394594 100644
--- a/app/models/purchase.rb
+++ b/app/models/purchase.rb
@@ -73,6 +73,19 @@ def remove(item)
     line_item&.destroy
   end
 
+  def replace_increase!(previous_line_item_values)
+    ActiveRecord::Base.transaction do
+      # Roll back distribution output by increasing storage location
+      storage_location.increase_inventory(to_a)
+      # Apply the new changes to the storage location inventory
+      storage_location.decrease_inventory(previous_line_item_values)
+      # TODO: Discuss this -- *should* we be removing InventoryItems when they hit 0 count?
+      storage_location.inventory_items.where(quantity: 0).destroy_all
+    end
+  rescue ActiveRecord::RecordInvalid
+    false
+  end
+
   # Use a negative quantity to subtract inventory
   def update_quantity(quantity, item)
     item_id = item.to_i
diff --git a/spec/factories/adjustments.rb b/spec/factories/adjustments.rb
index b1036d1e09..ce984e6a44 100644
--- a/spec/factories/adjustments.rb
+++ b/spec/factories/adjustments.rb
@@ -13,26 +13,19 @@
 FactoryBot.define do
   factory :adjustment do
     organization { Organization.try(:first) || create(:organization) }
-    storage_location { nil }
+    storage_location
     comment { "A comment" }
 
-    after(:build) do |instance, evaluator|
-      instance.storage_location = evaluator.storage_location || create(:storage_location, organization: instance.organization)
-    end
-
     trait :with_items do
+      storage_location { create :storage_location, :with_items, item: item || create(:item), organization: organization }
+
       transient do
         item_quantity { 100 }
         item { nil }
       end
 
       after(:build) do |instance, evaluator|
-        instance.storage_location ||= create(:storage_location, :with_items, item: evaluator.item, organization: instance.organization)
-        item = if evaluator.item.nil?
-                 instance.storage_location.inventory_items.first.item
-               else
-                 evaluator.item
-               end
+        item = evaluator.item || instance.storage_location.inventory_items.first.item
         instance.line_items << build(:line_item, quantity: evaluator.item_quantity, item: item)
       end
     end
diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb
index 5771c978c2..0ab9814ea0 100644
--- a/spec/models/donation_spec.rb
+++ b/spec/models/donation_spec.rb
@@ -181,6 +181,31 @@
     end
   end
 
+  describe "replace_increase!" do
+      let!(:storage_location) { create(:storage_location, :with_items, item_quantity: 10, organization: @organization) }
+      subject { create(:donation, :with_items, organization: @organization, item_quantity: 10, storage_location: storage_location) }
+
+      it "updates the quantity of items" do
+        previous_quantities = subject.to_a
+        subject.line_items.first.update(quantity: 5)
+        expect do
+          subject.replace_increase!(previous_quantities)
+          storage_location.reload
+        end.to change { storage_location.size }.by(-5)
+      end
+
+      it "removes the inventory item if the item's removal results in a 0 count" do
+          previous_quantities = subject.to_a
+          subject.line_items.first.update(quantity: 0)
+
+          expect do
+            subject.replace_increase!(previous_quantities)
+            storage_location.reload
+          end.to change { storage_location.inventory_items.size }.by(-1)
+                                                                 .and change { InventoryItem.count }.by(-1)
+      end
+    end
+
   describe "SOURCES" do
     it "is a hash that is referenceable by key to avoid 'magic strings'" do
       expect(Donation::SOURCES).to have_key(:diaper_drive)
diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb
index a81f320246..8b67a5d0d1 100644
--- a/spec/models/purchase_spec.rb
+++ b/spec/models/purchase_spec.rb
@@ -142,5 +142,30 @@
         end.not_to change { purchase.line_items.count }
       end
     end
+
+    describe "replace_increase!" do
+      let!(:storage_location) { create(:storage_location, :with_items, item_quantity: 10, organization: @organization) }
+      subject { create(:purchase, :with_items, organization: @organization, item_quantity: 10, storage_location: storage_location) }
+
+      it "updates the quantity of items" do
+        previous_quantities = subject.to_a
+        subject.line_items.first.update(quantity: 5)
+        expect do
+          subject.replace_increase!(previous_quantities)
+          storage_location.reload
+        end.to change { storage_location.size }.by(-5)
+      end
+
+      it "removes the inventory item if the item's removal results in a 0 count" do
+          previous_quantities = subject.to_a
+          subject.line_items.first.update(quantity: 0)
+
+          expect do
+            subject.replace_increase!(previous_quantities)
+            storage_location.reload
+          end.to change { storage_location.inventory_items.size }.by(-1)
+                                                                 .and change { InventoryItem.count }.by(-1)
+      end
+    end
   end
 end
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index 25c142e6a8..73d7819cff 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -132,55 +132,6 @@
       end
     end
 
-    describe "adjust_from_past!" do
-      let(:storage_location) { create(:storage_location) }
-      let(:purchase)         { create(:purchase, :with_items, item_quantity: 10) }
-      let(:donation)         { create(:donation, :with_items, item_quantity: 10) }
-
-      context "with_donations" do
-        before(:each) do
-          storage_location.increase_inventory(donation)
-        end
-
-        it "updates the quantity of items" do
-          previous_quantities = donation.line_items_quantities
-          donation.line_items.first.update(quantity: 5)
-          expect do
-            storage_location.adjust_from_past!(donation, previous_quantities)
-            storage_location.reload
-          end.to change { storage_location.size }.by(-5)
-        end
-
-        it "removes the inventory item from the DB if the item's removal results in a 0 count" do
-          previous_quantities = donation.line_items_quantities
-          donation.line_items.first.update(quantity: 0)
-
-          expect do
-            storage_location.adjust_from_past!(donation, previous_quantities)
-            storage_location.reload
-          end.to change { storage_location.inventory_items.size }.by(-1)
-                                                                 .and change { InventoryItem.count }.by(-1)
-        end
-      end
-      context "With purchases" do
-        before(:each) do
-          storage_location.increase_inventory(purchase)
-          storage_location.items.reload
-        end
-
-        it "removes the inventory item from the DB if the item's removal results in a 0 count" do
-          previous_quantities = purchase.line_items_quantities
-          purchase.line_items.first.update(quantity: 0)
-
-          expect do
-            storage_location.adjust_from_past!(purchase, previous_quantities)
-            storage_location.reload
-          end.to change { storage_location.inventory_items.size }.by(-1)
-                                                                 .and change { InventoryItem.count }.by(-1)
-        end
-      end
-    end
-
     describe "import_csv" do
       it "imports storage locations from a csv file" do
         organization
diff --git a/spec/support/itemizable_shared_example.rb b/spec/support/itemizable_shared_example.rb
index 72c41be2dd..67613582d1 100644
--- a/spec/support/itemizable_shared_example.rb
+++ b/spec/support/itemizable_shared_example.rb
@@ -97,20 +97,5 @@
         end.to change { subject.line_items.total }.by(10)
       end
     end
-
-    describe "updated line item quantity" do
-      subject { create(model_f, organization: @organization) }
-      let(:storage_location) { subject.storage_location }
-
-      it "updates storage location quantity by the correct amount" do
-        line_item = subject.line_items.create(item_id: item.id, quantity: 10)
-        inventory_item = create(:inventory_item, storage_location: storage_location, item_id: line_item.item_id)
-        previous_quantities = subject.to_a
-        line_item.update!(quantity: 5)
-        expect do
-          subject.replace_increase!(previous_quantities)
-        end.to change { inventory_item.reload.quantity }.by(-5)
-      end
-    end
   end
 end

From 3ac61512add20642c0e2f9ab3fbb1477fb358527 Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sun, 12 May 2019 01:24:46 -0400
Subject: [PATCH 24/32] Rubocop fixes. Removes 2nd argument from
 DonationsController replace_increase call

---
 app/controllers/donations_controller.rb |  2 +-
 spec/models/donation_spec.rb            | 38 ++++++++++++-------------
 spec/models/purchase_spec.rb            | 16 +++++------
 3 files changed, 28 insertions(+), 28 deletions(-)

diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb
index c65ed29c62..1ff5ad1044 100644
--- a/app/controllers/donations_controller.rb
+++ b/app/controllers/donations_controller.rb
@@ -103,7 +103,7 @@ def update
     previous_quantities = @donation.to_a
     if @donation.update(donation_params)
       # TODO: Move update into replace_increase!
-      @donation.replace_increase!(@donation, previous_quantities)
+      @donation.replace_increase!(previous_quantities)
       redirect_to donations_path
     else
       render "edit"
diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb
index 0ab9814ea0..4be83bc7d5 100644
--- a/spec/models/donation_spec.rb
+++ b/spec/models/donation_spec.rb
@@ -182,29 +182,29 @@
   end
 
   describe "replace_increase!" do
-      let!(:storage_location) { create(:storage_location, :with_items, item_quantity: 10, organization: @organization) }
-      subject { create(:donation, :with_items, organization: @organization, item_quantity: 10, storage_location: storage_location) }
+    let!(:storage_location) { create(:storage_location, :with_items, item_quantity: 10, organization: @organization) }
+    subject { create(:donation, :with_items, organization: @organization, item_quantity: 10, storage_location: storage_location) }
 
-      it "updates the quantity of items" do
-        previous_quantities = subject.to_a
-        subject.line_items.first.update(quantity: 5)
-        expect do
-          subject.replace_increase!(previous_quantities)
-          storage_location.reload
-        end.to change { storage_location.size }.by(-5)
-      end
+    it "updates the quantity of items" do
+      previous_quantities = subject.to_a
+      subject.line_items.first.update(quantity: 5)
+      expect do
+        subject.replace_increase!(previous_quantities)
+        storage_location.reload
+      end.to change { storage_location.size }.by(-5)
+    end
 
-      it "removes the inventory item if the item's removal results in a 0 count" do
-          previous_quantities = subject.to_a
-          subject.line_items.first.update(quantity: 0)
+    it "removes the inventory item if the item's removal results in a 0 count" do
+      previous_quantities = subject.to_a
+      subject.line_items.first.update(quantity: 0)
 
-          expect do
-            subject.replace_increase!(previous_quantities)
-            storage_location.reload
-          end.to change { storage_location.inventory_items.size }.by(-1)
-                                                                 .and change { InventoryItem.count }.by(-1)
-      end
+      expect do
+        subject.replace_increase!(previous_quantities)
+        storage_location.reload
+      end.to change { storage_location.inventory_items.size }.by(-1)
+                                                             .and change { InventoryItem.count }.by(-1)
     end
+  end
 
   describe "SOURCES" do
     it "is a hash that is referenceable by key to avoid 'magic strings'" do
diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb
index 8b67a5d0d1..d8624d6419 100644
--- a/spec/models/purchase_spec.rb
+++ b/spec/models/purchase_spec.rb
@@ -157,14 +157,14 @@
       end
 
       it "removes the inventory item if the item's removal results in a 0 count" do
-          previous_quantities = subject.to_a
-          subject.line_items.first.update(quantity: 0)
-
-          expect do
-            subject.replace_increase!(previous_quantities)
-            storage_location.reload
-          end.to change { storage_location.inventory_items.size }.by(-1)
-                                                                 .and change { InventoryItem.count }.by(-1)
+        previous_quantities = subject.to_a
+        subject.line_items.first.update(quantity: 0)
+
+        expect do
+          subject.replace_increase!(previous_quantities)
+          storage_location.reload
+        end.to change { storage_location.inventory_items.size }.by(-1)
+                                                               .and change { InventoryItem.count }.by(-1)
       end
     end
   end

From 9f331b0c85ae97c9bf2851c83990346584e375d1 Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Tue, 14 May 2019 22:59:12 -0400
Subject: [PATCH 25/32] Fixes replace_increase method for Donation and
 Purchase; fixes specs for that. Makes StorageLocation model spec use the
 global @organization instead of a local factory. Adds Item.reactivate for
 setting items back to active

---
 app/controllers/purchases_controller.rb |  5 +--
 app/models/donation.rb                  | 11 ++++-
 app/models/item.rb                      |  5 +++
 app/models/purchase.rb                  | 11 ++++-
 app/models/storage_location.rb          |  2 +-
 spec/models/distribution_spec.rb        |  4 ++
 spec/models/donation_spec.rb            | 56 ++++++++++++++++---------
 spec/models/purchase_spec.rb            | 29 ++++++++-----
 spec/models/storage_location_spec.rb    | 33 ++++++++++-----
 9 files changed, 107 insertions(+), 49 deletions(-)

diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb
index f0f9b11741..ba5263cb38 100644
--- a/app/controllers/purchases_controller.rb
+++ b/app/controllers/purchases_controller.rb
@@ -50,10 +50,7 @@ def show
   def update
     @purchase = current_organization.purchases.find(params[:id])
     @purchase.changed?
-    previous_quantities = @purchase.to_a
-    if @purchase.update(purchase_params)
-      # TODO: Revisit this. Move update into method.
-      @purchase.replace_increase!(previous_quantities)
+    if @purchase.replace_increase!(purchase_params)
       redirect_to purchases_path
     else
       render "edit"
diff --git a/app/models/donation.rb b/app/models/donation.rb
index 2bc2b93057..f42ed5f61d 100644
--- a/app/models/donation.rb
+++ b/app/models/donation.rb
@@ -102,12 +102,19 @@ def track(item, quantity)
     end
   end
 
-  def replace_increase!(previous_line_item_values)
+  def replace_increase!(new_donation_params)
+    old_data = to_a
+    item_ids = new_donation_params.values.flatten.map { |i| i[:item_id].to_i }
+
     ActiveRecord::Base.transaction do
+      line_items.map(&:destroy!)
+      reload
+      Item.reactivate(item_ids)
+      update! new_donation_params
       # Roll back distribution output by increasing storage location
       storage_location.increase_inventory(to_a)
       # Apply the new changes to the storage location inventory
-      storage_location.decrease_inventory(previous_line_item_values)
+      storage_location.decrease_inventory(old_data)
       # TODO: Discuss this -- *should* we be removing InventoryItems when they hit 0 count?
       storage_location.inventory_items.where(quantity: 0).destroy_all
     end
diff --git a/app/models/item.rb b/app/models/item.rb
index aa503e9578..906bf36f47 100644
--- a/app/models/item.rb
+++ b/app/models/item.rb
@@ -56,6 +56,11 @@ def self.barcodes_for(item)
     BarcodeItem.where("barcodeable_id = ?", item.id)
   end
 
+  def self.reactivate(item_ids)
+    item_ids = Array.wrap(item_ids)
+    Item.unscoped.where(id: item_ids).find_each { |item| item.update(active: true) }
+  end
+
   # Override `destroy` to ensure Item isn't accidentally destroyed
   # without first being disassociated with its historical presence
   def destroy
diff --git a/app/models/purchase.rb b/app/models/purchase.rb
index 95d1394594..73e381c527 100644
--- a/app/models/purchase.rb
+++ b/app/models/purchase.rb
@@ -73,12 +73,19 @@ def remove(item)
     line_item&.destroy
   end
 
-  def replace_increase!(previous_line_item_values)
+  def replace_increase!(new_purchase_params)
+    old_data = to_a
+    item_ids = new_purchase_params.values.flatten.map { |i| i[:item_id] }
+
     ActiveRecord::Base.transaction do
+      line_items.map(&:destroy!)
+      reload
+      Item.reactivate(item_ids)
+      update! new_purchase_params
       # Roll back distribution output by increasing storage location
       storage_location.increase_inventory(to_a)
       # Apply the new changes to the storage location inventory
-      storage_location.decrease_inventory(previous_line_item_values)
+      storage_location.decrease_inventory(old_data)
       # TODO: Discuss this -- *should* we be removing InventoryItems when they hit 0 count?
       storage_location.inventory_items.where(quantity: 0).destroy_all
     end
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index ab9fd30a9f..063dd9bf5b 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -101,7 +101,7 @@ def increase_inventory(itemizable_array)
     # Iterate through each of the line-items in the moving box
     itemizable_array.each do |item_hash|
       # TODO: make this an aggregate change
-      Item.unscoped.find(item_hash[:item_id]).update(active: true)
+      Item.reactivate(item_hash[:item_id])
       # Locate the storage box for the item, or create a new storage box for it
       inventory_item = inventory_items.find_or_create_by!(item_id: item_hash[:item_id])
       # Increase the quantity-on-record for that item
diff --git a/spec/models/distribution_spec.rb b/spec/models/distribution_spec.rb
index 94d4946fbd..1968b3d5b8 100644
--- a/spec/models/distribution_spec.rb
+++ b/spec/models/distribution_spec.rb
@@ -101,5 +101,9 @@
         expect(distribution.line_items.first.quantity).to eq 15
       end
     end
+
+    describe "#replace_distribution!" do
+      # TODO: Add these
+    end
   end
 end
diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb
index 4be83bc7d5..c6a273874f 100644
--- a/spec/models/donation_spec.rb
+++ b/spec/models/donation_spec.rb
@@ -179,30 +179,46 @@
         expect(donation.money_raised).to eq(100)
       end
     end
-  end
 
-  describe "replace_increase!" do
-    let!(:storage_location) { create(:storage_location, :with_items, item_quantity: 10, organization: @organization) }
-    subject { create(:donation, :with_items, organization: @organization, item_quantity: 10, storage_location: storage_location) }
+    describe "replace_increase!" do
+      let!(:storage_location) { create(:storage_location, :with_items, item_quantity: 5, organization: @organization) }
+      subject { create(:donation, :with_items, organization: @organization, item_quantity: 5, storage_location: storage_location) }
 
-    it "updates the quantity of items" do
-      previous_quantities = subject.to_a
-      subject.line_items.first.update(quantity: 5)
-      expect do
-        subject.replace_increase!(previous_quantities)
-        storage_location.reload
-      end.to change { storage_location.size }.by(-5)
-    end
+      context "changing the donation" do
+        let(:attributes) { { line_items_attributes: [{ item_id: subject.line_items.first.item_id, quantity: 2 }] } }
 
-    it "removes the inventory item if the item's removal results in a 0 count" do
-      previous_quantities = subject.to_a
-      subject.line_items.first.update(quantity: 0)
+        it "updates the quantity of items" do
+          expect do
+            subject.replace_increase!(attributes)
+            storage_location.reload
+          end.to change { storage_location.size }.by(-3)
+        end
+      end
 
-      expect do
-        subject.replace_increase!(previous_quantities)
-        storage_location.reload
-      end.to change { storage_location.inventory_items.size }.by(-1)
-                                                             .and change { InventoryItem.count }.by(-1)
+      context "when adding an item that has been previously deleted" do
+        let!(:inactive_item) { create(:item, active: false) }
+        let(:attributes) { { line_items_attributes: [{ item_id: inactive_item.to_param, quantity: 10 }] } }
+
+        it "re-creates the item" do
+          expect do
+            subject.replace_increase!(attributes)
+            storage_location.reload
+          end.to change { storage_location.size }.by(5) # We had 5 items of a different kind before, now we have 10
+                                                 .and change { Item.count }.by(1)
+        end
+      end
+
+      context "with empty line_items" do
+        let(:attributes) { { line_items_attributes: [{}] } }
+
+        it "removes the inventory item if the item's removal results in a 0 count" do
+          expect do
+            subject.replace_increase!(attributes)
+            storage_location.reload
+          end.to change { storage_location.inventory_items.size }.by(-1)
+                                                                 .and change { InventoryItem.count }.by(-1)
+        end
+      end
     end
   end
 
diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb
index d8624d6419..edae9c0d05 100644
--- a/spec/models/purchase_spec.rb
+++ b/spec/models/purchase_spec.rb
@@ -144,28 +144,37 @@
     end
 
     describe "replace_increase!" do
-      let!(:storage_location) { create(:storage_location, :with_items, item_quantity: 10, organization: @organization) }
-      subject { create(:purchase, :with_items, organization: @organization, item_quantity: 10, storage_location: storage_location) }
+      let!(:storage_location) { create(:storage_location, :with_items, item_quantity: 5, organization: @organization) }
+      subject { create(:purchase, :with_items, item_quantity: 5, storage_location: storage_location, organization: @organization) }
 
       it "updates the quantity of items" do
-        previous_quantities = subject.to_a
-        subject.line_items.first.update(quantity: 5)
+        attributes = { line_items_attributes: [{ item_id: subject.line_items.first.item_id, quantity: 2 }] }
         expect do
-          subject.replace_increase!(previous_quantities)
+          subject.replace_increase!(attributes)
           storage_location.reload
-        end.to change { storage_location.size }.by(-5)
+        end.to change { storage_location.size }.by(-3)
       end
 
       it "removes the inventory item if the item's removal results in a 0 count" do
-        previous_quantities = subject.to_a
-        subject.line_items.first.update(quantity: 0)
-
+        attributes = { line_items_attributes: [{}] }
         expect do
-          subject.replace_increase!(previous_quantities)
+          subject.replace_increase!(attributes)
           storage_location.reload
         end.to change { storage_location.inventory_items.size }.by(-1)
                                                                .and change { InventoryItem.count }.by(-1)
       end
+
+      context "when adding an item that has been previously deleted" do
+        let!(:inactive_item) { create(:item, active: false, organization: @organization) }
+        let(:attributes) { { line_items_attributes: [{ item_id: inactive_item.id, quantity: 10 }] } }
+        it "re-creates the item" do
+          expect do
+            subject.replace_increase!(attributes)
+            storage_location.reload
+          end.to change { storage_location.size }.by(5)
+                                                 .and change { Item.count }.by(1)
+        end
+      end
     end
   end
 end
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index 73d7819cff..038c4e445c 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -13,8 +13,6 @@
 #
 
 RSpec.describe StorageLocation, type: :model do
-  let(:organization) { create(:organization) }
-
   context "Validations >" do
     it "requires a name" do
       expect(build(:storage_location, name: nil)).not_to be_valid
@@ -39,7 +37,8 @@
   context "Methods >" do
     describe "increase_inventory" do
       context "With existing inventory" do
-        let(:donation) { create(:donation, :with_items, item_quantity: 66, organization: organization) }
+        let(:donation) { create(:donation, :with_items, item_quantity: 66, organization: @organization) }
+
         it "increases inventory quantities from an itemizable object" do
           item = create(:item)
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
@@ -50,8 +49,9 @@
       end
 
       context "when providing a new item that does not yet exist" do
-        let(:mystery_item) { create(:item, organization: organization) }
-        let(:donation_with_new_items) { create(:donation, :with_items, organization: organization, item_quantity: 10, item: mystery_item) }
+        let(:mystery_item) { create(:item, organization: @organization) }
+        let(:donation_with_new_items) { create(:donation, :with_items, organization: @organization, item_quantity: 10, item: mystery_item) }
+
         it "creates those new inventory items in the storage location" do
           item = create(:item)
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
@@ -60,11 +60,25 @@
           end.to change { storage_location.inventory_items.count }.by(1)
         end
       end
+
+      context "when increasing with an inactive item" do
+        let(:inactive_item) { create(:item, active: false, organization: @organization) }
+        let(:donation_with_inactive_item) { create(:donation, :with_items, organization: @organization, item_quantity: 10, item: inactive_item) }
+
+        it "re-activates the item as part of the creation process" do
+          storage_location = create(:storage_location, organization: @organization)
+          expect do
+            storage_location.increase_inventory(donation_with_inactive_item.to_a)
+          end.to change { storage_location.inventory_items.count }.by(1)
+                                                                  .and change { Item.count }.by(1)
+        end
+      end
     end
 
     describe "decrease_inventory" do
       let(:item) { create(:item) }
       let(:distribution) { create(:distribution, :with_items, item: item, item_quantity: 66) }
+
       it "decreases inventory quantities from an itemizable object" do
         storage_location = create(:storage_location, :with_items, item_quantity: 100, item: item, organization: @organization)
         expect do
@@ -74,6 +88,7 @@
 
       context "when there is insufficient inventory available" do
         let(:distribution_but_too_much) { create(:distribution, :with_items, item: item, item_quantity: 9001) }
+
         it "gives informative errors" do
           storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
           expect do
@@ -134,12 +149,11 @@
 
     describe "import_csv" do
       it "imports storage locations from a csv file" do
-        organization
         before_import = StorageLocation.count
         import_file_path = Rails.root.join("spec", "fixtures", "storage_locations.csv")
         data = File.read(import_file_path, encoding: "BOM|UTF-8")
         csv = CSV.parse(data, headers: true)
-        StorageLocation.import_csv(csv, organization.id)
+        StorageLocation.import_csv(csv, @organization.id)
         expect(StorageLocation.count).to eq before_import + 1
       end
     end
@@ -147,10 +161,9 @@
     describe "import_inventory" do
       it "imports storage locations from a csv file" do
         donations_count = Donation.count
-        organization
-        storage_location = create(:storage_location, organization_id: organization.id)
+        storage_location = create(:storage_location, organization_id: @organization.id)
         import_file_path = Rails.root.join("spec", "fixtures", "inventory.csv").read
-        StorageLocation.import_inventory(import_file_path, organization.id, storage_location.id)
+        StorageLocation.import_inventory(import_file_path, @organization.id, storage_location.id)
         expect(storage_location.size).to eq 14_842
         expect(donations_count).to eq Donation.count
       end

From b5179d1a47ebf3d161007a2896ae6ec7332d54d9 Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Tue, 14 May 2019 23:18:25 -0400
Subject: [PATCH 26/32] Removes track, contains_item_id, and update_quantity
 from both Donation and Purchase; neither is used anywhere anymore. Removes
 from specs as well.

---
 app/controllers/donations_controller.rb | 23 -------------
 app/models/donation.rb                  | 26 --------------
 app/models/purchase.rb                  | 22 ------------
 spec/models/donation_spec.rb            | 30 ----------------
 spec/models/purchase_spec.rb            | 46 -------------------------
 5 files changed, 147 deletions(-)

diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb
index 1ff5ad1044..cd2ad3e660 100644
--- a/app/controllers/donations_controller.rb
+++ b/app/controllers/donations_controller.rb
@@ -1,31 +1,8 @@
 class DonationsController < ApplicationController
-  # We load the resources in before_filters so that they are not re-loaded
-  # by Cancan, which won't use the correct methods.
-  # before_filter :simple_load, only: [:track, :remove_item, :edit, :update, :destroy]
-  # before_filter :eager_load_single, only: [:show]
-  # before_filter :load_collection, only: [:index]
-  # before_filter :load_new, only: [:new]
-
-  # Cancan authorization
-  # load_and_authorize_resource
-
   skip_before_action :verify_authenticity_token, only: %i(scale_intake scale)
   skip_before_action :authenticate_user!, only: %i(scale_intake scale)
   skip_before_action :authorize_user, only: %i(scale_intake scale)
 
-  #  def add_item
-  #    @donation = current_organization.donations.find(params[:id])
-  #    if (donation_item_params.has_key?(:barcode_id))
-  #      donation_item_params[:item_id] = BarcodeItem.find!(donation_item_params[:barcode_id]).item_id
-  #    end
-  #    @donation.track(donation_item_params[:item_id], donation_item_params[:quantity])
-  #  end
-
-  #  def remove_item
-  #    @donation = current_organization.donations.find(params[:id])
-  #    @donation.remove(donation_item_params[:item_id])
-  #  end
-
   def index
     @donations = current_organization.donations
                                      .includes(:line_items, :storage_location, :donation_site, :diaper_drive_participant)
diff --git a/app/models/donation.rb b/app/models/donation.rb
index f42ed5f61d..5bd737111e 100644
--- a/app/models/donation.rb
+++ b/app/models/donation.rb
@@ -90,18 +90,6 @@ def self.daily_quantities_by_source(start, stop)
                       .sum("line_items.quantity")
   end
 
-  # def self.total_received
-  #    self.includes(:line_items).map(&:total_quantity).reduce(0, :+)
-  #  end
-
-  def track(item, quantity)
-    if contains_item_id?(item.id)
-      update_quantity(quantity, item)
-    else
-      LineItem.create(itemizable: self, item_id: item.id, quantity: quantity)
-    end
-  end
-
   def replace_increase!(new_donation_params)
     old_data = to_a
     item_ids = new_donation_params.values.flatten.map { |i| i[:item_id].to_i }
@@ -141,20 +129,6 @@ def total_quantity
     line_items.sum(:quantity)
   end
 
-  def contains_item_id?(id)
-    line_items.find_by(item_id: id).present?
-  end
-
-  # Use a negative quantity to subtract inventory
-  def update_quantity(quantity, item)
-    item_id = item.to_i
-    line_item = line_items.find_by(item_id: item_id)
-    line_item.quantity += quantity
-    # Inventory can never be negative
-    line_item.quantity = 0 if line_item.quantity.negative?
-    line_item.save
-  end
-
   def self.csv_export_headers
     ["Source", "Date", "Donation Site", "Storage Location", "Quantity of Items", "Variety of Items"]
   end
diff --git a/app/models/purchase.rb b/app/models/purchase.rb
index 73e381c527..4934a62723 100644
--- a/app/models/purchase.rb
+++ b/app/models/purchase.rb
@@ -50,18 +50,6 @@ def purchased_from_view
     vendor.nil? ? purchased_from : vendor.business_name
   end
 
-  def track(item, quantity)
-    if contains_item_id?(item.id)
-      update_quantity(quantity, item)
-    else
-      LineItem.create(itemizable: self, item_id: item.id, quantity: quantity)
-    end
-  end
-
-  def contains_item_id?(id)
-    line_items.find_by(item_id: id).present?
-  end
-
   def total_quantity
     line_items.sum(:quantity)
   end
@@ -93,16 +81,6 @@ def replace_increase!(new_purchase_params)
     false
   end
 
-  # Use a negative quantity to subtract inventory
-  def update_quantity(quantity, item)
-    item_id = item.to_i
-    line_item = line_items.find_by(item_id: item_id)
-    line_item.quantity += quantity
-    # Inventory can never be negative
-    line_item.quantity = 0 if line_item.quantity.negative?
-    line_item.save
-  end
-
   def self.csv_export_headers
     ["Purchases from", "Storage Location", "Quantity of Items", "Variety of Items", "Amount spent"]
   end
diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb
index c6a273874f..08587fa919 100644
--- a/spec/models/donation_spec.rb
+++ b/spec/models/donation_spec.rb
@@ -122,36 +122,6 @@
       end
     end
 
-    describe "contains_item_id?" do
-      it "returns true if the item_id already exists" do
-        donation = create(:donation, :with_items)
-        expect(donation.contains_item_id?(donation.items.last.id)).to be_truthy
-      end
-    end
-
-    describe "update_quantity" do
-      let!(:donation) { create(:donation, :with_items) }
-      it "adds an additional quantity to the existing line_item" do
-        expect do
-          donation.update_quantity(1, donation.items.first)
-          donation.reload
-        end.to change { donation.line_items.first.quantity }.by(1)
-      end
-
-      it "can receive a negative quantity to subtract inventory" do
-        expect do
-          donation.update_quantity(-1, donation.items.first)
-        end.to change { donation.total_quantity }.by(-1)
-      end
-
-      it "works whether you give it an item or an id" do
-        expect do
-          donation.update_quantity(1, donation.items.first.id)
-          donation.reload
-        end.to change { donation.line_items.first.quantity }.by(1)
-      end
-    end
-
     describe "remove" do
       let!(:donation) { create(:donation, :with_items) }
 
diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb
index edae9c0d05..9832668c97 100644
--- a/spec/models/purchase_spec.rb
+++ b/spec/models/purchase_spec.rb
@@ -79,52 +79,6 @@
   end
 
   context "Methods >" do
-    describe "track" do
-      let!(:purchase) { create(:purchase) }
-      let!(:item) { create(:item) }
-
-      it "does not add a new line_item unnecessarily, updating existing line_item instead" do
-        item = create :item
-        purchase.track(item, 5)
-        expect do
-          purchase.track(item, 10)
-          purchase.reload
-        end.not_to change { purchase.line_items.count }
-
-        expect(purchase.line_items.first.quantity).to eq(15)
-      end
-    end
-
-    describe "contains_item_id?" do
-      it "returns true if the item_id already exists" do
-        purchase = create(:purchase, :with_items)
-        expect(purchase.contains_item_id?(purchase.items.first.id)).to be_truthy
-      end
-    end
-
-    describe "update_quantity" do
-      let!(:purchase) { create(:purchase, :with_items) }
-      it "adds an additional quantity to the existing line_item" do
-        expect do
-          purchase.update_quantity(1, purchase.items.first)
-          purchase.reload
-        end.to change { purchase.line_items.first.quantity }.by(1)
-      end
-
-      it "can receive a negative quantity to subtract inventory" do
-        expect do
-          purchase.update_quantity(-1, purchase.items.first)
-        end.to change { purchase.total_quantity }.by(-1)
-      end
-
-      it "works whether you give it an item or an id" do
-        expect do
-          purchase.update_quantity(1, purchase.items.first.id)
-          purchase.reload
-        end.to change { purchase.line_items.first.quantity }.by(1)
-      end
-    end
-
     describe "remove" do
       let!(:purchase) { create(:purchase, :with_items) }
 

From 61972e68efc2c065fe887caf63ad44cb368706f1 Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Tue, 14 May 2019 23:40:57 -0400
Subject: [PATCH 27/32] Donations|PurchasesController#update now uses
 replace_increase\! - specs updated.

---
 app/controllers/donations_controller.rb       | 5 +----
 app/controllers/purchases_controller.rb       | 1 -
 app/models/donation.rb                        | 3 ++-
 app/models/purchase.rb                        | 4 +++-
 spec/controllers/purchases_controller_spec.rb | 6 +++---
 5 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb
index cd2ad3e660..62bd5918be 100644
--- a/app/controllers/donations_controller.rb
+++ b/app/controllers/donations_controller.rb
@@ -77,10 +77,7 @@ def show
 
   def update
     @donation = Donation.find(params[:id])
-    previous_quantities = @donation.to_a
-    if @donation.update(donation_params)
-      # TODO: Move update into replace_increase!
-      @donation.replace_increase!(previous_quantities)
+    if @donation.replace_increase!(donation_params)
       redirect_to donations_path
     else
       render "edit"
diff --git a/app/controllers/purchases_controller.rb b/app/controllers/purchases_controller.rb
index ba5263cb38..0d10b93093 100644
--- a/app/controllers/purchases_controller.rb
+++ b/app/controllers/purchases_controller.rb
@@ -49,7 +49,6 @@ def show
 
   def update
     @purchase = current_organization.purchases.find(params[:id])
-    @purchase.changed?
     if @purchase.replace_increase!(purchase_params)
       redirect_to purchases_path
     else
diff --git a/app/models/donation.rb b/app/models/donation.rb
index 5bd737111e..316b65da50 100644
--- a/app/models/donation.rb
+++ b/app/models/donation.rb
@@ -92,12 +92,13 @@ def self.daily_quantities_by_source(start, stop)
 
   def replace_increase!(new_donation_params)
     old_data = to_a
-    item_ids = new_donation_params.values.flatten.map { |i| i[:item_id].to_i }
+    item_ids = Array.wrap(new_donation_params[:line_items_attributes]&.values).map { |i| i[:item_id].to_i }
 
     ActiveRecord::Base.transaction do
       line_items.map(&:destroy!)
       reload
       Item.reactivate(item_ids)
+      Array.wrap(new_donation_params[:line_items_attributes]&.values).map { |i| i.delete(:id) }
       update! new_donation_params
       # Roll back distribution output by increasing storage location
       storage_location.increase_inventory(to_a)
diff --git a/app/models/purchase.rb b/app/models/purchase.rb
index 4934a62723..ac6202b240 100644
--- a/app/models/purchase.rb
+++ b/app/models/purchase.rb
@@ -63,12 +63,14 @@ def remove(item)
 
   def replace_increase!(new_purchase_params)
     old_data = to_a
-    item_ids = new_purchase_params.values.flatten.map { |i| i[:item_id] }
+    item_ids = Array.wrap(new_purchase_params[:line_items_attributes]&.values).map { |i| i[:item_id].to_i }
 
     ActiveRecord::Base.transaction do
       line_items.map(&:destroy!)
       reload
       Item.reactivate(item_ids)
+      Array.wrap(new_purchase_params[:line_items_attributes]&.values).map { |i| i.delete(:id) }
+    
       update! new_purchase_params
       # Roll back distribution output by increasing storage location
       storage_location.increase_inventory(to_a)
diff --git a/spec/controllers/purchases_controller_spec.rb b/spec/controllers/purchases_controller_spec.rb
index e749061210..fbf9c87485 100644
--- a/spec/controllers/purchases_controller_spec.rb
+++ b/spec/controllers/purchases_controller_spec.rb
@@ -55,20 +55,20 @@
       end
 
       it "updates storage quantity correctly" do
-        purchase = create(:purchase, :with_items, item_quantity: 10)
+        purchase = create(:purchase, :with_items, item_quantity: 5)
         line_item = purchase.line_items.first
         line_item_params = {
           "0" => {
             "_destroy" => "false",
             item_id: line_item.item_id,
-            quantity: "5",
+            quantity: "10",
             id: line_item.id
           }
         }
         purchase_params = { source: "Purchase Site", line_items_attributes: line_item_params }
         expect do
           put :update, params: default_params.merge(id: purchase.id, purchase: purchase_params)
-        end.to change { purchase.storage_location.inventory_items.first.quantity }.by(-5)
+        end.to change { purchase.storage_location.inventory_items.first.quantity }.by(5)
       end
 
       describe "when removing a line item" do

From 04771364177c3b0a9e793da70c6538427513e31b Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Tue, 14 May 2019 23:57:35 -0400
Subject: [PATCH 28/32] Adds helper method to itemizable for when you want to
 pull the line_items_attributes from params. Removes a unnecessary spec. Fixes
 spec configuration for hashes in params for replace_increase tests.

---
 app/models/concerns/itemizable.rb |  5 +++++
 app/models/donation.rb            |  4 ++--
 app/models/purchase.rb            |  6 +++---
 spec/models/donation_spec.rb      | 21 ++-------------------
 spec/models/purchase_spec.rb      | 11 +++++------
 5 files changed, 17 insertions(+), 30 deletions(-)

diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb
index 2b55021aa6..aab0f77308 100644
--- a/app/models/concerns/itemizable.rb
+++ b/app/models/concerns/itemizable.rb
@@ -79,6 +79,11 @@ def to_a
 
   private
 
+  # From Controller parameters
+  def line_items_attributes(params)
+    Array.wrap(params[:line_items_attributes]&.values)
+  end
+
   def line_item_items_quantity_is_positive
     return if storage_location.nil?
 
diff --git a/app/models/donation.rb b/app/models/donation.rb
index 316b65da50..cc45363daf 100644
--- a/app/models/donation.rb
+++ b/app/models/donation.rb
@@ -92,13 +92,13 @@ def self.daily_quantities_by_source(start, stop)
 
   def replace_increase!(new_donation_params)
     old_data = to_a
-    item_ids = Array.wrap(new_donation_params[:line_items_attributes]&.values).map { |i| i[:item_id].to_i }
+    item_ids = line_items_attributes(new_donation_params).map { |i| i[:item_id].to_i }
 
     ActiveRecord::Base.transaction do
       line_items.map(&:destroy!)
       reload
       Item.reactivate(item_ids)
-      Array.wrap(new_donation_params[:line_items_attributes]&.values).map { |i| i.delete(:id) }
+      line_items_attributes(new_donation_params).map { |i| i.delete(:id) }
       update! new_donation_params
       # Roll back distribution output by increasing storage location
       storage_location.increase_inventory(to_a)
diff --git a/app/models/purchase.rb b/app/models/purchase.rb
index ac6202b240..1bf34b0b20 100644
--- a/app/models/purchase.rb
+++ b/app/models/purchase.rb
@@ -63,14 +63,14 @@ def remove(item)
 
   def replace_increase!(new_purchase_params)
     old_data = to_a
-    item_ids = Array.wrap(new_purchase_params[:line_items_attributes]&.values).map { |i| i[:item_id].to_i }
+    item_ids = line_items_attributes(new_purchase_params).map { |i| i[:item_id].to_i }
 
     ActiveRecord::Base.transaction do
       line_items.map(&:destroy!)
       reload
       Item.reactivate(item_ids)
-      Array.wrap(new_purchase_params[:line_items_attributes]&.values).map { |i| i.delete(:id) }
-    
+      line_items_attributes(new_purchase_params).map { |i| i.delete(:id) }
+
       update! new_purchase_params
       # Roll back distribution output by increasing storage location
       storage_location.increase_inventory(to_a)
diff --git a/spec/models/donation_spec.rb b/spec/models/donation_spec.rb
index 08587fa919..e7ca9ff20a 100644
--- a/spec/models/donation_spec.rb
+++ b/spec/models/donation_spec.rb
@@ -97,31 +97,14 @@
     describe "items >" do
       it "has_many" do
         donation = create(:donation)
-        item = create(:item)
-        # Using donation.track because it marshalls the HMT
-        donation.track(item, 1)
+        line_item = create(:line_item)
+        donation.line_items << line_item
         expect(donation.items.count).to eq(1)
       end
     end
   end
 
   context "Methods >" do
-    describe "track" do
-      let!(:donation) { create(:donation) }
-      let!(:item) { create(:item) }
-
-      it "does not add a new line_item unnecessarily, updating existing line_item instead" do
-        item = create :item
-        donation.track(item, 5)
-        expect do
-          donation.track(item, 10)
-          donation.reload
-        end.not_to change { donation.line_items.count }
-
-        expect(donation.line_items.first.quantity).to eq(15)
-      end
-    end
-
     describe "remove" do
       let!(:donation) { create(:donation, :with_items) }
 
diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb
index 9832668c97..4ae67c0095 100644
--- a/spec/models/purchase_spec.rb
+++ b/spec/models/purchase_spec.rb
@@ -70,9 +70,8 @@
     describe "items >" do
       it "has_many" do
         purchase = create(:purchase)
-        item = create(:item)
-        # Using purchase.track because it marshalls the HMT
-        purchase.track(item, 1)
+        line_item = create(:line_item)
+        purchase.line_items << line_item
         expect(purchase.items.count).to eq(1)
       end
     end
@@ -102,7 +101,7 @@
       subject { create(:purchase, :with_items, item_quantity: 5, storage_location: storage_location, organization: @organization) }
 
       it "updates the quantity of items" do
-        attributes = { line_items_attributes: [{ item_id: subject.line_items.first.item_id, quantity: 2 }] }
+        attributes = { line_items_attributes: { "0": { item_id: subject.line_items.first.item_id, quantity: 2 } } }
         expect do
           subject.replace_increase!(attributes)
           storage_location.reload
@@ -110,7 +109,7 @@
       end
 
       it "removes the inventory item if the item's removal results in a 0 count" do
-        attributes = { line_items_attributes: [{}] }
+        attributes = { line_items_attributes: {} }
         expect do
           subject.replace_increase!(attributes)
           storage_location.reload
@@ -120,7 +119,7 @@
 
       context "when adding an item that has been previously deleted" do
         let!(:inactive_item) { create(:item, active: false, organization: @organization) }
-        let(:attributes) { { line_items_attributes: [{ item_id: inactive_item.id, quantity: 10 }] } }
+        let(:attributes) { { line_items_attributes: { "0": { item_id: inactive_item.id, quantity: 10 } } } }
         it "re-creates the item" do
           expect do
             subject.replace_increase!(attributes)

From 3dc86a2929ec0165313627de8792b934e1594f0e Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Wed, 15 May 2019 00:02:20 -0400
Subject: [PATCH 29/32] Moves @transfer.save into the transaction in
 TransfersController

---
 app/controllers/transfers_controller.rb | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/app/controllers/transfers_controller.rb b/app/controllers/transfers_controller.rb
index 905e0288db..f78da2dfa1 100644
--- a/app/controllers/transfers_controller.rb
+++ b/app/controllers/transfers_controller.rb
@@ -10,8 +10,9 @@ def index
   def create
     @transfer = current_organization.transfers.new(transfer_params)
 
-    if @transfer.valid? && @transfer.save
+    if @transfer.valid?
       ActiveRecord::Base.transaction do
+        @transfer.save
         @transfer.from.decrease_inventory @transfer
         @transfer.to.increase_inventory @transfer
       end

From 81778ea584159f61fd5e7dc5ab662ad8ef373cf4 Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Fri, 17 May 2019 22:54:43 -0400
Subject: [PATCH 30/32] Adds some missing specs. Makes changes based on PR
 feedback.

---
 app/models/adjustment.rb         | 16 ------------
 app/models/distribution.rb       |  2 +-
 app/models/storage_location.rb   |  4 +--
 spec/models/distribution_spec.rb |  9 ++++++-
 spec/models/item_spec.rb         | 44 +++++++++++++++-----------------
 5 files changed, 31 insertions(+), 44 deletions(-)

diff --git a/app/models/adjustment.rb b/app/models/adjustment.rb
index 5b3d0ac9ec..9169b66e69 100644
--- a/app/models/adjustment.rb
+++ b/app/models/adjustment.rb
@@ -61,20 +61,4 @@ def storage_locations_belong_to_organization
       errors.add :storage_location, "storage location must belong to organization"
     end
   end
-
-  def replicate_to(itemizable)
-    raise ArgumentError unless itemizable.respond_to?(:line_items)
-
-    all_line_items = if block_given?
-                       collect do |line_item|
-                         yield(line_item)
-                         line_item
-                       end.compact
-                     else
-                       collect
-                     end
-
-    all_line_items.each { |li| itemizable.line_items << li.dup }
-    itemizable
-  end
 end
diff --git a/app/models/distribution.rb b/app/models/distribution.rb
index 7cc74db9a6..e8531b7bcb 100644
--- a/app/models/distribution.rb
+++ b/app/models/distribution.rb
@@ -56,7 +56,7 @@ def replace_distribution!(new_distribution_params)
       # Roll back distribution output by increasing storage location
       storage_location.increase_inventory(to_a)
       # Delete the line items -- they'll be replaced later
-      line_items.map(&:destroy!)
+      line_items.each(&:destroy!)
       reload
 
       # Replace the current distribution with the new parameters
diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 59687c65fb..2226bb5843 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -94,7 +94,7 @@ def self.import_inventory(filename, org, loc)
 
   # FIXME: After this is stable, revisit how we do logging
   def increase_inventory(itemizable_array)
-    itemizable_array = itemizable_array.is_a?(Array) ? itemizable_array : itemizable_array.to_a
+    itemizable_array = Array.wrap(itemizable_array)
 
     # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
@@ -118,7 +118,7 @@ def increase_inventory(itemizable_array)
 
   # TODO: re-evaluate this for optimization
   def decrease_inventory(itemizable_array)
-    itemizable_array = itemizable_array.is_a?(Array) ? itemizable_array : itemizable_array.to_a
+    itemizable_array = Array.wrap(itemizable_array)
     # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
     # This tracks items that have insufficient inventory counts to be reduced as much
diff --git a/spec/models/distribution_spec.rb b/spec/models/distribution_spec.rb
index ea050b84e7..e166ed342c 100644
--- a/spec/models/distribution_spec.rb
+++ b/spec/models/distribution_spec.rb
@@ -103,7 +103,14 @@
     end
 
     describe "#replace_distribution!" do
-      # TODO: Add these
+      subject { create(:distribution, :with_items, item_quantity: 10) }
+      let(:attributes) { { line_items_attributes: { "0": { item_id: subject.line_items.first.item_id, quantity: 2 } } } }
+
+      it "replaces a big distribution with a smaller one, resulting in increased stored quantities" do
+        expect do
+          subject.replace_distribution!(attributes)
+        end.to change { subject.storage_location.size }.by(8)
+      end
     end
   end
 end
diff --git a/spec/models/item_spec.rb b/spec/models/item_spec.rb
index 2966e70918..b63b29bb9d 100644
--- a/spec/models/item_spec.rb
+++ b/spec/models/item_spec.rb
@@ -143,29 +143,25 @@
         expect(item).not_to be_active
       end
     end
-  end
 
-  # context "Callbacks >" do
-  #   describe "when DIAPER_PARTNER_URL is present" do
-  #     let(:diaper_partner_url) { "http://diaper.partner.io" }
-  #     let(:callback_url) { "#{diaper_partner_url}/items" }
-  #
-  #     before do
-  #       stub_env "DIAPER_PARTNER_URL", diaper_partner_url
-  #       stub_env "DIAPER_PARTNER_SECRET_KEY", "secretkey123"
-  #       stub_request :post, callback_url
-  #     end
-  #
-  #     it "notifies the Diaper Partner app" do
-  #       item = create :item
-  #       headers = {
-  #         "Authorization" => /APIAuth diaperbase:.*/,
-  #         "Content-Type" => "application/x-www-form-urlencoded"
-  #       }
-  #       body = URI.encode_www_form item.attributes
-  #       expect(WebMock).to have_requested(:post, callback_url)
-  #         .with(headers: headers, body: body).once
-  #     end
-  #   end
-  # end
+    describe "#reactivate!" do
+      context "given an array of item ids" do
+        let(:item_array) { create_list(:item, 2, :inactive).collect(&:id) }
+        it "sets the active trait to true for all of them" do
+          expect do
+            Item.reactivate(item_array)
+          end.to change { Item.active.size }.by(item_array.size)
+        end
+      end
+
+      context "given a single item id" do
+        let(:item_id) { create(:item).id }
+        it "sets the active trait to true for that item" do
+          expect do
+            Item.reactivate(item_id)
+          end.to change { Item.active.size }.by(1)
+        end
+      end
+    end
+  end
 end

From 1286a021955f41914b154dbd70476fab013c0685 Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 18 May 2019 00:32:11 -0400
Subject: [PATCH 31/32] Renames a local var to make it more accurate. Extracts
 an aggregate query. Extracts some repetitive test setup. Fixes a bug in
 increase/decrease inventory where it wasn't running to_a

---
 app/models/storage_location.rb       | 28 ++++++++++++++--------------
 spec/features/adjustment_spec.rb     |  2 +-
 spec/models/storage_location_spec.rb | 20 +++++++++-----------
 3 files changed, 24 insertions(+), 26 deletions(-)

diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb
index 2226bb5843..86300596cd 100644
--- a/app/models/storage_location.rb
+++ b/app/models/storage_location.rb
@@ -94,14 +94,13 @@ def self.import_inventory(filename, org, loc)
 
   # FIXME: After this is stable, revisit how we do logging
   def increase_inventory(itemizable_array)
-    itemizable_array = Array.wrap(itemizable_array)
+    itemizable_array = itemizable_array.to_a
 
     # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
     # Iterate through each of the line-items in the moving box
+    Item.reactivate(itemizable_array.map { |item_hash| item_hash[:item_id] })
     itemizable_array.each do |item_hash|
-      # TODO: make this an aggregate change
-      Item.reactivate(item_hash[:item_id])
       # Locate the storage box for the item, or create a new storage box for it
       inventory_item = inventory_items.find_or_create_by!(item_id: item_hash[:item_id])
       # Increase the quantity-on-record for that item
@@ -118,24 +117,25 @@ def increase_inventory(itemizable_array)
 
   # TODO: re-evaluate this for optimization
   def decrease_inventory(itemizable_array)
-    itemizable_array = Array.wrap(itemizable_array)
+    itemizable_array = itemizable_array.to_a
+
     # This is, at least for now, how we log changes to the inventory made in this call
     log = {}
     # This tracks items that have insufficient inventory counts to be reduced as much
     insufficient_items = []
     # Iterate through each of the line-items in the moving box
-    itemizable_array.each do |item|
+    itemizable_array.each do |item_hash|
       # Locate the storage box for the item, or create an empty storage box
-      inventory_item = inventory_items.find_by(item_id: item[:item_id]) || inventory_items.build
+      inventory_item = inventory_items.find_by(item_id: item_hash[:item_id]) || inventory_items.build
       # If we've got sufficient inventory in the storage box to fill the moving box, then continue
-      next unless inventory_item.quantity < item[:quantity]
+      next unless inventory_item.quantity < item_hash[:quantity]
 
       # Otherwise, we need to record that there was insufficient inventory on-hand
       insufficient_items << {
-        item_id: item[:item_id],
-        item_name: item[:name],
+        item_id: item_hash[:item_id],
+        item_name: item_hash[:name],
         quantity_on_hand: inventory_item.quantity,
-        quantity_requested: item[:quantity]
+        quantity_requested: item_hash[:quantity]
       }
     end
 
@@ -151,15 +151,15 @@ def decrease_inventory(itemizable_array)
     end
 
     # Re-run through the items in the moving box again
-    itemizable_array.each do |item|
+    itemizable_array.each do |item_hash|
       # Look for the moving box for this item -- we know there is sufficient quantity this time
       # Raise AR:RNF if it fails to find it -- though that seems moot since it would have been
       # captured by the previous block.
-      inventory_item = inventory_items.find_by(item_id: item[:item_id])
+      inventory_item = inventory_items.find_by(item_id: item_hash[:item_id])
       # Reduce the inventory box quantity
-      inventory_item.decrement!(:quantity, item[:quantity])
+      inventory_item.decrement!(:quantity, item_hash[:quantity])
       # Record in the log that this has occurred
-      log[item[:item_id]] = "-#{item[:quantity]}"
+      log[item_hash[:item_id]] = "-#{item_hash[:quantity]}"
     end
     # log could be pulled from dirty AR stuff
     save!
diff --git a/spec/features/adjustment_spec.rb b/spec/features/adjustment_spec.rb
index bc653d4308..832f9ca0ad 100644
--- a/spec/features/adjustment_spec.rb
+++ b/spec/features/adjustment_spec.rb
@@ -64,4 +64,4 @@
 
     expect(page).to have_css("table tr", count: 2)
   end
-end
+end
\ No newline at end of file
diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index 11872b2c22..f5e66475bb 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -35,16 +35,17 @@
   end
 
   context "Methods >" do
+    let!(:item) { create(:item) }
+    subject { create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization) }
+
     describe "increase_inventory" do
       context "With existing inventory" do
         let(:donation) { create(:donation, :with_items, item_quantity: 66, organization: @organization) }
 
         it "increases inventory quantities from an itemizable object" do
-          item = create(:item)
-          storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
           expect do
-            storage_location.increase_inventory(donation.to_a)
-          end.to change { storage_location.size }.by(66)
+            subject.increase_inventory(donation.to_a)
+          end.to change { subject.size }.by(66)
         end
       end
 
@@ -53,11 +54,9 @@
         let(:donation_with_new_items) { create(:donation, :with_items, organization: @organization, item_quantity: 10, item: mystery_item) }
 
         it "creates those new inventory items in the storage location" do
-          item = create(:item)
-          storage_location = create(:storage_location, :with_items, item_quantity: 10, item: item, organization: @organization)
           expect do
-            storage_location.increase_inventory(donation_with_new_items.to_a)
-          end.to change { storage_location.inventory_items.count }.by(1)
+            subject.increase_inventory(donation_with_new_items.to_a)
+          end.to change { subject.inventory_items.count }.by(1)
         end
       end
 
@@ -66,10 +65,9 @@
         let(:donation_with_inactive_item) { create(:donation, :with_items, organization: @organization, item_quantity: 10, item: inactive_item) }
 
         it "re-activates the item as part of the creation process" do
-          storage_location = create(:storage_location, organization: @organization)
           expect do
-            storage_location.increase_inventory(donation_with_inactive_item.to_a)
-          end.to change { storage_location.inventory_items.count }.by(1)
+            subject.increase_inventory(donation_with_inactive_item.to_a)
+          end.to change { subject.inventory_items.count }.by(1)
                                                                   .and change { Item.count }.by(1)
         end
       end

From 7d8f5a761680633bcfd04a43a6aa0d550a8b43cc Mon Sep 17 00:00:00 2001
From: "Aaron (Home)" <armahillo@gmail.com>
Date: Sat, 18 May 2019 00:51:23 -0400
Subject: [PATCH 32/32] Rubocop would buy this error for a dollar.

---
 spec/models/storage_location_spec.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb
index f5e66475bb..9f281a6392 100644
--- a/spec/models/storage_location_spec.rb
+++ b/spec/models/storage_location_spec.rb
@@ -68,7 +68,7 @@
           expect do
             subject.increase_inventory(donation_with_inactive_item.to_a)
           end.to change { subject.inventory_items.count }.by(1)
-                                                                  .and change { Item.count }.by(1)
+                                                         .and change { Item.count }.by(1)
         end
       end
     end