Fixes #473 Refactor inventory changing methods mkd/amh

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"

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)

app/controllers/adjustments_controller.rb

@@ -1,7 +1,5 @@
 # Provides limited CRUD for Adjustments, which are the way that Diaper Banks fix incorrect inventory totals at Storage Locations
 class AdjustmentsController < ApplicationController
-  before_action :set_adjustment, only: %i(show destroy)
-
   # GET /adjustments
   # GET /adjustments.json
   def index
@@ -13,44 +11,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?
-      @adjustment.storage_location.adjust!(@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

app/controllers/audits_controller.rb

@@ -34,7 +34,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

app/controllers/distributions_controller.rb

@@ -19,16 +19,14 @@ def print
     end
   end
 
-  # TODO: This should be renamed :destroy, because that's what it does. I think the name is vestigial from a time when it didn't actually destroy the object
-  def reclaim
+  def destroy
     ActiveRecord::Base.transaction do
-      @distribution_id = params[:id]
-      distribution = Distribution.find(params[:id])
-      distribution.storage_location.reclaim!(distribution)
+      distribution = current_organization.distributions.find(params[:id])
+      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
 
@@ -44,31 +42,25 @@ def index
 
   def create
     @distribution = Distribution.new(distribution_params.merge(organization: current_organization))
+    @storage_locations = current_organization.storage_locations
 
     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
       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
@@ -109,7 +101,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
 

app/controllers/donations_controller.rb

@@ -41,20 +41,20 @@ 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
 
   def create
-    @donation = Donation.new(donation_params.merge(organization: current_organization))
+    @donation = current_organization.donations.new(donation_params)
     if @donation.save
-      @donation.storage_location.intake! @donation
+      @donation.storage_location.increase_inventory @donation
       redirect_to donations_path
     else
       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
@@ -78,18 +78,21 @@ def show
 
   def update
     @donation = Donation.find(params[:id])
-    previous_quantities = @donation.line_items_quantities
-    if @donation.update(donation_params)
-      @donation.storage_location.adjust_from_past!(@donation, previous_quantities)
+    if @donation.replace_increase!(donation_params)
       redirect_to donations_path
     else
       render "edit"
     end
   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
 

app/controllers/purchases_controller.rb

@@ -20,13 +20,13 @@ 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
       @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
@@ -50,19 +50,21 @@ def show
 
   def update
     @purchase = current_organization.purchases.find(params[:id])
-    @purchase.changed?
-    previous_quantities = @purchase.line_items_quantities
-    if @purchase.update(purchase_params)
-      @purchase.storage_location.adjust_from_past!(@purchase, previous_quantities)
+    if @purchase.replace_increase!(purchase_params)
       redirect_to purchases_path
     else
       render "edit"
     end
   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
 

app/controllers/transfers_controller.rb

@@ -12,34 +12,30 @@ def create
     @transfer = current_organization.transfers.new(transfer_params)
 
     if @transfer.valid?
-      @transfer.from.move_inventory!(@transfer)
-
-      if @transfer.save
-        redirect_to transfers_path, notice: "Transfer was successfully created."
-      else
-        flash[:error] = "There was an error, try again?"
-        render :new
+      ActiveRecord::Base.transaction do
+        @transfer.save
+        @transfer.from.decrease_inventory @transfer
+        @transfer.to.increase_inventory @transfer
       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
 
   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
@@ -50,6 +46,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))

app/models/adjustment.rb

@@ -30,6 +30,14 @@ def self.storage_locations_adjusted_for(organization)
     includes(:storage_location).where(organization_id: organization.id).collect(&:storage_location)
   end
 
+  def split_difference
+    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]
+  end
+
   def self.csv_export_headers
     ["Created", "Organization", "Storage Location", "Comment", "Changes"]
   end

app/models/concerns/itemizable.rb

@@ -21,11 +21,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`
@@ -67,19 +67,25 @@ def total
     validates_associated :line_items
   end
 
-  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
-  end
-
   def value_per_itemizable
     line_items.sum(&:value_per_line_item)
   end
 
+  def to_a
+    line_items.map do |l|
+      # 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
+
   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?