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/adjustments_controller.rb b/app/controllers/adjustments_controller.rb index 7366ee861e..03398c645a 100644 --- a/app/controllers/adjustments_controller.rb +++ b/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("
".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("
".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/audits_controller.rb b/app/controllers/audits_controller.rb index 22f8826d2e..252b3f1f9c 100644 --- a/app/controllers/audits_controller.rb +++ b/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 diff --git a/app/controllers/distributions_controller.rb b/app/controllers/distributions_controller.rb index fc65dfbcfd..854c632b9d 100644 --- a/app/controllers/distributions_controller.rb +++ b/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 diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index f86616a191..676d41cad2 100644 --- a/app/controllers/donations_controller.rb +++ b/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,9 +78,7 @@ 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" @@ -88,8 +86,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 615e786716..2acb5b8831 100644 --- a/app/controllers/purchases_controller.rb +++ b/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,10 +50,7 @@ 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" @@ -61,8 +58,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 66750acc2d..bc2f843fc5 100644 --- a/app/controllers/transfers_controller.rb +++ b/app/controllers/transfers_controller.rb @@ -12,25 +12,22 @@ 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 @@ -38,8 +35,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 @@ -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)) diff --git a/app/models/adjustment.rb b/app/models/adjustment.rb index d19f411e93..9169b66e69 100644 --- a/app/models/adjustment.rb +++ b/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 diff --git a/app/models/concerns/itemizable.rb b/app/models/concerns/itemizable.rb index e40f67012d..63bd29853c 100644 --- a/app/models/concerns/itemizable.rb +++ b/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? diff --git a/app/models/distribution.rb b/app/models/distribution.rb index 5e42bf0330..e8531b7bcb 100644 --- a/app/models/distribution.rb +++ b/app/models/distribution.rb @@ -49,6 +49,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.each(&: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/donation.rb b/app/models/donation.rb index 8ee2e37178..86bffaffaa 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]}'" }, @@ -91,16 +90,25 @@ 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) + def replace_increase!(new_donation_params) + old_data = to_a + 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) + 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) + # Apply the new changes to the storage location inventory + 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 + rescue ActiveRecord::RecordInvalid + false end def remove(item) @@ -122,24 +130,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 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/item.rb b/app/models/item.rb index 4a2f0093e9..35e45822d9 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 e32ed917d5..1bf34b0b20 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,22 +50,6 @@ def purchased_from_view vendor.nil? ? purchased_from : vendor.business_name end - def remove_inventory - storage_location.remove!(self) - 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 @@ -78,14 +61,26 @@ def remove(item) line_item&.destroy 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 + def replace_increase!(new_purchase_params) + old_data = to_a + 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) + 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) + # Apply the new changes to the storage location inventory + 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 + rescue ActiveRecord::RecordInvalid + false end def self.csv_export_headers @@ -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/app/models/storage_location.rb b/app/models/storage_location.rb index 08e36bbb13..86300596cd 100644 --- a/app/models/storage_location.rb +++ b/app/models/storage_location.rb @@ -71,89 +71,7 @@ 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) do |inv_item| - inv_item.quantity = 0 - end - inventory_item.quantity += begin - line_item.quantity - rescue StandardError - 0 - end - inventory_item.save - log[line_item.item_id] = "+#{line_item.quantity}" - end - log - end - - def remove!(donation_or_purchase) - log = {} - donation_or_purchase.line_items.each do |line_item| - inventory_item = InventoryItem.find_by(storage_location: id, item_id: line_item.item_id) - if (inventory_item.quantity - line_item.quantity) <= 0 - 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 - - def adjust_from_past!(donation_or_purchase, previous_line_item_values) - donation_or_purchase.line_items.each do |line_item| - 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 durring the updated/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 - - def distribute!(distribution) - updated_quantities = {} - insufficient_items = [] - distribution.line_items.each do |line_item| - inventory_item = inventory_items.find_by(item: line_item.item) - 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 - - unless insufficient_items.empty? - raise Errors::InsufficientAllotment.new( - "Distribution line_items exceed the available inventory", - insufficient_items - ) - end - - 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) @@ -162,105 +80,91 @@ 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") - 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 - # Ex: move 500 size "2" diapers from main warehouse to overflow warehouse because insufficient space in main warehouse - def move_inventory!(transfer) - 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? + # FIXME: After this is stable, revisit how we do logging + def increase_inventory(itemizable_array) + itemizable_array = itemizable_array.to_a - 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 - else - item_validator.add_insufficiency(line_item.item, - inventory_item.quantity, - line_item.quantity) - 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 + Item.reactivate(itemizable_array.map { |item_hash| item_hash[:item_id] }) + itemizable_array.each do |item_hash| + # 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].to_i) + # Record in the log that this has occurred + log[item_hash[:item_id]] = "+#{item_hash[:quantity]}" end - - raise item_validator unless item_validator.satisfied? - - update_inventory_inventory_items(updated_quantities) + # log could be pulled from dirty AR stuff? + # Save the final changes -- does this need to occur here? + save + # return log + log 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 - def adjust!(adjustment) - updated_quantities = {} - item_validator = Errors::InsufficientAllotment.new("Adjustment exceeds the available inventory") + # TODO: re-evaluate this for optimization + def decrease_inventory(itemizable_array) + itemizable_array = itemizable_array.to_a - 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 + # 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_hash| + # Locate the storage box for the item, or create an empty storage box + 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_hash[:quantity] + + # Otherwise, we need to record that there was insufficient inventory on-hand + insufficient_items << { + item_id: item_hash[:item_id], + item_name: item_hash[:name], + quantity_on_hand: inventory_item.quantity, + quantity_requested: item_hash[:quantity] + } end - raise item_validator unless item_validator.satisfied? - - update_inventory_inventory_items(updated_quantities) - end - - def reclaim!(distribution) - ActiveRecord::Base.transaction do - distribution.line_items.each do |line_item| - if line_item.item.nil? || !line_item.item.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 + # 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 items exceed the available inventory", + insufficient_items + ) end - end - - 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 + # Re-run through the items in the moving box again + 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_hash[:item_id]) + # Reduce the inventory box quantity + inventory_item.decrement!(:quantity, item_hash[:quantity]) + # Record in the log that this has occurred + log[item_hash[:item_id]] = "-#{item_hash[:quantity]}" end - rescue ActiveRecord::RecordInvalid - false + # log could be pulled from dirty AR stuff + save! + # return log + log end def self.csv_export_headers @@ -270,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/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" } %> diff --git a/config/routes.rb b/config/routes.rb index e0764daac5..b2d4a56a53 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -69,9 +69,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/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/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 diff --git a/spec/factories/adjustments.rb b/spec/factories/adjustments.rb index c5b2c62558..9af6865d6e 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/features/adjustment_spec.rb b/spec/features/adjustment_spec.rb index 8edd3ac5c0..832f9ca0ad 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,23 @@ 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 + 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("items exceed the available inventory") end scenario "User can filter the #index by storage location" do @@ -48,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/features/distribution_spec.rb b/spec/features/distribution_spec.rb index 422342321e..2f647f0bee 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" @@ -139,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 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/adjustment_spec.rb b/spec/models/adjustment_spec.rb index 827712179d..9e407f8912 100644 --- a/spec/models/adjustment_spec.rb +++ b/spec/models/adjustment_spec.rb @@ -38,6 +38,58 @@ 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) + 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 describe "nested line item attributes" do diff --git a/spec/models/distribution_spec.rb b/spec/models/distribution_spec.rb index 53168261cf..e166ed342c 100644 --- a/spec/models/distribution_spec.rb +++ b/spec/models/distribution_spec.rb @@ -101,5 +101,16 @@ expect(distribution.line_items.first.quantity).to eq 15 end end + + describe "#replace_distribution!" do + 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/donation_spec.rb b/spec/models/donation_spec.rb index 42643ce3f5..05405bfee7 100644 --- a/spec/models/donation_spec.rb +++ b/spec/models/donation_spec.rb @@ -97,61 +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 "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) } @@ -173,21 +126,53 @@ 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) expect(donation.money_raised).to eq(100) end end + + 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) } + + context "changing the donation" do + let(:attributes) { { line_items_attributes: { "0": { item_id: subject.line_items.first.item_id, quantity: 2 } } } } + + 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 + + context "when adding an item that has been previously deleted" do + let!(:inactive_item) { create(:item, active: false) } + let(:attributes) { { line_items_attributes: { "0": { 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 describe "SOURCES" do 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 diff --git a/spec/models/purchase_spec.rb b/spec/models/purchase_spec.rb index d2442d01f7..4ae67c0095 100644 --- a/spec/models/purchase_spec.rb +++ b/spec/models/purchase_spec.rb @@ -70,61 +70,14 @@ 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 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) } @@ -143,12 +96,37 @@ end end - describe "remove_inventory" do - it "removes inventory from the right storage location when purchase deleted" do - purchase = create(:purchase, :with_items) + describe "replace_increase!" do + 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 + attributes = { line_items_attributes: { "0": { item_id: subject.line_items.first.item_id, quantity: 2 } } } + expect do + subject.replace_increase!(attributes) + storage_location.reload + 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 + attributes = { line_items_attributes: {} } expect do - purchase.remove_inventory - end.to change { purchase.storage_location.size }.by(-purchase.total_quantity) + 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: { "0": { 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 diff --git a/spec/models/storage_location_spec.rb b/spec/models/storage_location_spec.rb index 9ff8c96f0d..9f281a6392 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 @@ -37,6 +35,78 @@ 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 + expect do + subject.increase_inventory(donation.to_a) + end.to change { subject.size }.by(66) + end + 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) } + + it "creates those new inventory items in the storage location" do + expect do + subject.increase_inventory(donation_with_new_items.to_a) + end.to change { subject.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 + 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) + 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 + storage_location.decrease_inventory(distribution.to_a) + 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 do + storage_location.decrease_inventory(distribution_but_too_much.to_a).errors + 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 + begin + storage_location.decrease_inventory(distribution.to_a) + rescue Errors::InsufficientAllotment + end + storage_location.reload + expect(storage_location.size).to eq(starting_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) @@ -75,131 +145,13 @@ 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.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) } - let(:donation) { create(:donation, :with_items, item_quantity: 10) } - - context "with_donations" do - before(:each) do - storage_location.intake!(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.intake!(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 "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 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 @@ -207,88 +159,14 @@ 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 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) - 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 "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, diff --git a/spec/support/itemizable_shared_example.rb b/spec/support/itemizable_shared_example.rb index f9c827271f..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.line_items_quantities - line_item.update!(quantity: 5) - expect do - storage_location.adjust_from_past!(subject, previous_quantities) - end.to change { inventory_item.reload.quantity }.by(-5) - end - end end end diff --git a/spec/system/adjustment_system_spec.rb b/spec/system/adjustment_system_spec.rb index cac3f1ddec..88f3331a19 100644 --- a/spec/system/adjustment_system_spec.rb +++ b/spec/system/adjustment_system_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/i) end it "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/i) end it "can filter the #index by storage location" do diff --git a/spec/system/transfer_system_spec.rb b/spec/system/transfer_system_spec.rb index 6994649028..c3126afb95 100644 --- a/spec/system/transfer_system_spec.rb +++ b/spec/system/transfer_system_spec.rb @@ -17,7 +17,26 @@ 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 + + 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 it "can filter the #index by storage location both from and to as a user" do