diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index 041abf9fa0..a925ab6c41 100644 --- a/app/controllers/donations_controller.rb +++ b/app/controllers/donations_controller.rb @@ -90,14 +90,22 @@ def show def update @donation = Donation.find(params[:id]) + @original_source = @donation.source ItemizableUpdateService.call(itemizable: @donation, params: donation_params, type: :increase, event_class: DonationEvent) + flash.clear + flash[:notice] = "Donation updated!" redirect_to donations_path rescue => e flash[:alert] = "Error updating donation: #{e.message}" - render "edit" + load_form_collections + # calling new(donation_params) triggers a validation error if line_item quantity is invalid + @previous_input = Donation.new(donation_params.except(:line_items_attributes)) + @previous_input.line_items.build(donation_params[:line_items_attributes].values) + + render "edit", status: :conflict end def destroy @@ -129,14 +137,14 @@ def clean_donation_money_raised params[:donation][:money_raised] = money_raised.gsub(/[$,.]/, "") if money_raised money_raised_in_dollars = params[:donation][:money_raised_in_dollars] - params[:donation][:money_raised] = money_raised_in_dollars.gsub(/[$,]/, "").to_d * 100 if money_raised_in_dollars + params[:donation][:money_raised] = (money_raised_in_dollars.gsub(/[$,]/, "").to_d * 100).to_s if money_raised_in_dollars end def donation_params strip_unnecessary_params clean_donation_money_raised params = compact_line_items - params.require(:donation).permit(:source, :comment, :storage_location_id, :money_raised, :issued_at, :donation_site_id, :product_drive_id, :product_drive_participant_id, :manufacturer_id, line_items_attributes: %i(id item_id quantity _destroy)).merge(organization: current_organization) + params.require(:donation).permit(:source, :comment, :storage_location_id, :money_raised, :issued_at, :donation_site_id, :product_drive_id, :product_drive_participant_id, :manufacturer_id, line_items_attributes: %i(id item_id quantity)).merge(organization: current_organization) end def donation_item_params diff --git a/app/views/donations/_donation_form.html.erb b/app/views/donations/_donation_form.html.erb index 295be85271..c7de6c4921 100644 --- a/app/views/donations/_donation_form.html.erb +++ b/app/views/donations/_donation_form.html.erb @@ -6,6 +6,8 @@
<%= f.input :source, collection: Donation::SOURCES.values, + selected: donation_form.source, + include_blank: true, label: "Source", error: "What effort or initiative did this donation come from?", wrapper: :input_group %> @@ -16,6 +18,8 @@
<%= f.association :donation_site, collection: @donation_sites, + selected: donation_form.donation_site_id, + include_blank: true, label: "Donation Site", error: "Where was this donation dropped off?", wrapper: :input_group %> @@ -25,6 +29,8 @@
<%= f.association :product_drive, collection: @product_drives, + selected: donation_form.product_drive_id, + include_blank: true, label_method: lambda { |x| "#{x.try(:name) }" }, label: "Product Drive", error: "Which product drive was this from?", @@ -33,6 +39,8 @@
<%= f.association :product_drive_participant, collection: @product_drive_participants, + selected: donation_form.product_drive_participant_id, + include_blank: true, label_method: lambda { |x| "#{x.try(:business_name) }" }, label: "Product Drive Participant", error: "Which product drive participant was this from?", @@ -44,7 +52,8 @@
<%= f.association :manufacturer, collection: @manufacturers, - prompt: "Choose one...", + selected: donation_form.manufacturer_id, + include_blank: true, label_method: lambda { |x| "#{x.try(:name) }" }, label: "Manufacturer", error: "Which Manufacturer was this from?", @@ -58,7 +67,7 @@ collection: @storage_locations, label: "Storage Location", error: "Where is it being stored?", - selected: f.object.storage_location&.id || current_organization.intake_location, + selected: donation_form.storage_location&.id || current_organization.intake_location, include_blank: true, wrapper: :input_group %>
@@ -68,15 +77,18 @@
<%= f.input :money_raised_in_dollars, as: :money_raised_in_dollars, wrapper: :input_group do %> - <%= f.input_field :money_raised_in_dollars, class: "form-control" %> + <%= f.input_field :money_raised_in_dollars, + class: "form-control", + value: donation_form.money_raised_in_dollars %> <% end %>
- <%= f.input :comment, - wrapper: :input_group %> + <%= f.input :comment, wrapper: :input_group do %> + <%= f.text_area :comment, value: donation_form.comment, cols: 90, rows: 2, class: "text optional form-control" %> + <% end %>
@@ -86,7 +98,8 @@ label: "Issued on", as: :date, html5: true, - wrapper: :input_group %> + wrapper: :input_group, + input_html: { value: donation_form.issued_at } %>
@@ -94,7 +107,7 @@ Items in this donation
- <%= render 'line_items/line_item_fields', form: f %> + <%= render 'line_items/line_item_fields', form: f, object: donation_form.line_items %>
diff --git a/app/views/donations/edit.html.erb b/app/views/donations/edit.html.erb index b7718e256a..6abc0d8eb9 100644 --- a/app/views/donations/edit.html.erb +++ b/app/views/donations/edit.html.erb @@ -2,10 +2,10 @@
- <% content_for :title, "Edit - Donations - #{@donation.source} - #{current_organization.name}" %> + <% content_for :title, "Edit - Donations - #{@original_source ? @original_source : @donation.source} - #{current_organization.name}" %>

Editing Donation - from <%= @donation.source %> + from <%= @original_source ? @original_source : @donation.source %>

@@ -16,7 +16,7 @@
  • <%= link_to "Donations", donations_path %>
  • - Editing <%= @donation.source %> on <%= @donation.created_at.to_fs(:distribution_date) %>
    • + Editing <%= @original_source ? @original_source : @donation.source %> on <%= @donation.created_at.to_fs(:distribution_date) %>
    @@ -39,7 +39,7 @@
    - <%= render partial: "donation_form", object: @donation %> + <%= render partial: "donation_form", object: @previous_input ? @previous_input : @donation %> <%= link_to "Add vendor", new_vendor_path, {:remote => true, "data-bs-toggle" => "modal", "data-bs-target" => "#modal-window", id: "new_vendor", "style" => "display: none;"} %>
    diff --git a/spec/controllers/donations_controller_spec.rb b/spec/controllers/donations_controller_spec.rb index 64a1c7c92f..a42080161a 100644 --- a/spec/controllers/donations_controller_spec.rb +++ b/spec/controllers/donations_controller_spec.rb @@ -58,7 +58,6 @@ line_item = donation.line_items.first line_item_params = { "0" => { - "_destroy" => "false", item_id: line_item.item_id, quantity: "15", id: line_item.id @@ -82,7 +81,6 @@ line_item = donation.line_items.first line_item_params = { "0" => { - "_destroy" => "false", item_id: line_item.item_id, quantity: "8", id: line_item.id @@ -111,7 +109,6 @@ line_item = donation.line_items.first line_item_params = { "0" => { - "_destroy" => "false", item_id: line_item.item_id, quantity: "1", id: line_item.id @@ -119,7 +116,7 @@ } donation_params = { source: donation.source, storage_location: new_storage_location, line_items_attributes: line_item_params } put :update, params: { id: donation.id, donation: donation_params } - expect(response).not_to redirect_to(anything) + expect(response).not_to redirect_to(edit_donation_path(donation)) expect(original_storage_location.size).to eq 5 expect(new_storage_location.size).to eq 0 expect(donation.reload.line_items.first.quantity).to eq 10 @@ -127,24 +124,19 @@ end describe "when removing a line item" do - it "updates storage invetory item quantity correctly" do - donation = create(:donation, :with_items, item_quantity: 10) - line_item = donation.line_items.first - line_item_params = { - "0" => { - "_destroy" => "true", - item_id: line_item.item_id, - id: line_item.id - } - } - donation_params = { source: donation.source, line_items_attributes: line_item_params } + it "updates storage inventory item quantity correctly" do + item_id = 1 + item_quantity = 10 + donation = create(:donation, :with_items, item: create(:item, id: item_id), item_quantity: item_quantity) + # if all line items including blanks are deleted line_items_attributes parameter is not sent + donation_params = { source: donation.source } expect do put :update, params: { id: donation.id, donation: donation_params } - end.to change { donation.storage_location.inventory_items.first.quantity }.by(-10) + end.to change { donation.storage_location.inventory_items.first.quantity }.by(-1 * item_quantity) .and change { View::Inventory.new(donation.organization_id) - .quantity_for(storage_location: donation.storage_location_id, item_id: line_item.item_id) - }.by(-10) + .quantity_for(storage_location: donation.storage_location_id, item_id: item_id) + }.by(-1 * item_quantity) end end end diff --git a/spec/factories/donations.rb b/spec/factories/donations.rb index a248c76b6e..7791bba25b 100644 --- a/spec/factories/donations.rb +++ b/spec/factories/donations.rb @@ -42,15 +42,15 @@ end trait :with_items do + transient do + item_quantity { 100 } + item { nil } + end storage_location do create :storage_location, :with_items, item: item || create(:item, value_in_cents: 100), organization: organization end - transient do - item_quantity { 100 } - item { nil } - end after(:build) do |donation, evaluator| event_item = View::Inventory.new(donation.organization_id) diff --git a/spec/requests/donations_requests_spec.rb b/spec/requests/donations_requests_spec.rb index 59e90164bf..963a5244c6 100644 --- a/spec/requests/donations_requests_spec.rb +++ b/spec/requests/donations_requests_spec.rb @@ -192,5 +192,83 @@ expect(response.body).to_not include("you’ll need to make an adjustment to the inventory as well.") end end + + # Bug fix - Issue #4172 + context "when donated items are distributed to less than donated amount " \ + "and you edit the donation to less than distributed amount" do + it "shows a warning and displays original names and amounts and info the user entered" do + item_name = "Brightbloom Seed" + item = create(:item, organization: organization, name: item_name) + storage_location = create(:storage_location, :with_items, item: item, item_quantity: 0, organization: organization) + extra_item_name = "Extra Item" + extra_item = create(:item, organization: organization, name: extra_item_name) + item_to_delete_name = "Item To Delete" + item_to_delete = create(:item, organization: organization, name: item_to_delete_name) + TestInventory.create_inventory(organization, { storage_location.id => { item.id => 0, extra_item.id => 1 } }) + original_quantity = 100 + original_source = Donation::SOURCES[:manufacturer] + original_date = DateTime.new(2024) + donation = build(:manufacturer_donation, issued_at: original_date, organization: organization, storage_location: storage_location, source: original_source) + donation.line_items << build(:line_item, item: item, quantity: original_quantity) + donation.line_items << build(:line_item, item: item_to_delete, quantity: 1) + + DonationCreateService.call(donation) + + # simulate distribution by setting item inventory to 10 + TestInventory.create_inventory(organization, { storage_location.id => { item.id => 10, extra_item.id => 1 } }) + + edited_source = Donation::SOURCES[:product_drive] + edited_source_drive_name = "Test Product Drive" + edited_source_drive = create(:product_drive, name: edited_source_drive_name, organization: organization) + edited_source_drive_participant_business_name = "Test Business Name" + edited_source_drive_participant = create(:product_drive_participant, business_name: edited_source_drive_participant_business_name, organization: organization) + edited_storage_location_name = "Test Storage" + edited_storage_location = create(:storage_location, name: edited_storage_location_name, organization: organization) + edited_money = 10.0 + edited_comment = "New test comment" + edited_date = "2019-01-01" + extra_quantity = 1 + edited_quantity = 1 + + # deleted item is removed from line_items_attributes entirely + edited_donation = { + source: edited_source, + product_drive_id: edited_source_drive.id, + product_drive_participant_id: edited_source_drive_participant.id, + storage_location_id: edited_storage_location.id, + money_raised_in_dollars: edited_money, + comment: edited_comment, + issued_at: edited_date, + line_items_attributes: { + "0": { item_id: item.id, quantity: edited_quantity }, + "1": { item_id: extra_item.id, quantity: extra_quantity } + } + } + + put donation_path(id: donation.id, donation: edited_donation) + + if Event.read_events?(organization) + expect(flash[:alert]).to include("Error updating donation: Could not reduce quantity") + else # TODO remove this branch when switching to events + expect(flash[:alert]).to include("Error updating donation: Requested items exceed the available inventory") + end + + expect(response.body).to include("Edit - Donations - #{original_source}") + expect(response.body).to include("Editing Donation\n from #{original_source}") + expect(response.body).to include("
  • \n Editing #{original_source}") + expect(response.body).to include("") + expect(response.body).to include("") + expect(response.body).to include("") + expect(response.body).to include("") + expect(response.body).to include(edited_comment) + expect(response.body).to include("value=\"#{edited_money}\" type=\"text\" name=\"donation[money_raised_in_dollars]") + expect(response.body).to include(edited_date) + expect(response.body).to include("") + expect(response.body).to include("value=\"#{edited_quantity}\" name=\"donation[line_items_attributes][0][quantity]") + expect(response.body).to include("") + expect(response.body).to include("value=\"#{extra_quantity}\" name=\"donation[line_items_attributes][1][quantity]") + expect(response.body).not_to include("") + end + end end end