From 708bbbbbd81d0ff9bf230a227c4d9ef62fcfacd1 Mon Sep 17 00:00:00 2001 From: Jimmy Li Date: Fri, 12 Jul 2024 01:04:52 +0000 Subject: [PATCH] GREEN keeps all entered information after invalid form submission * Also keeps submitted incorrect quantity --- app/controllers/donations_controller.rb | 17 ++++++++++-- app/views/donations/_donation_form.html.erb | 27 ++++++++++++++----- app/views/donations/edit.html.erb | 8 +++--- spec/controllers/donations_controller_spec.rb | 2 +- spec/requests/donations_requests_spec.rb | 2 +- 5 files changed, 41 insertions(+), 15 deletions(-) diff --git a/app/controllers/donations_controller.rb b/app/controllers/donations_controller.rb index c6c2966fc8..bb289af0d7 100644 --- a/app/controllers/donations_controller.rb +++ b/app/controllers/donations_controller.rb @@ -77,14 +77,27 @@ 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}" - redirect_back(fallback_location: edit_donation_path) + 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)) + line_items = [] + donation_params[:line_items_attributes].values.each { |attr| + attr.delete(:_destroy) + line_items.push(attr) + } + @previous_input.line_items.build(line_items) + + render "edit", status: :conflict end def destroy @@ -116,7 +129,7 @@ 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 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 0c6acb6f7b..b062bcfd44 100644 --- a/spec/controllers/donations_controller_spec.rb +++ b/spec/controllers/donations_controller_spec.rb @@ -119,7 +119,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).to redirect_to(edit_donation_path(donation)) + 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 diff --git a/spec/requests/donations_requests_spec.rb b/spec/requests/donations_requests_spec.rb index 3ae88b9acc..2d7bde9dd9 100644 --- a/spec/requests/donations_requests_spec.rb +++ b/spec/requests/donations_requests_spec.rb @@ -223,7 +223,7 @@ 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 by 99 - current quantity is 10 for Brightbloom Seed") + 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