Display errors taking place during import partner process (#4707)

* Remove duplicate validation errors form PartnerCreateService

* Allow Importable to display error message based on each model's import_csv implementation

* Add request tests

* Display import errors with correct flash styling

* Update test

* Update logic for 'import_csv' methods

* Refactor code

Author: jp524

app/controllers/concerns/importable.rb

@@ -27,8 +27,12 @@ def import_csv
       data = File.read(params[:file].path, encoding: "BOM|UTF-8")
       csv = CSV.parse(data, headers: true).reject { |row| row.to_hash.values.any?(&:nil?) }
       if csv.count.positive? && csv.first.headers.all? { |header| !header.nil? }
-        resource_model.import_csv(csv, current_organization.id)
-        flash[:notice] = "#{resource_model_humanized} were imported successfully!"
+        errors = resource_model.import_csv(csv, current_organization.id)
+        if errors.empty?
+          flash[:notice] = "#{resource_model_humanized} were imported successfully!"
+        else
+          flash[:error] = "The following #{resource_model_humanized} did not import successfully:\n#{errors.join("\n")}"
+        end
       else
         flash[:error] = "Check headers in file!"
       end

app/models/concerns/provideable.rb

@@ -19,6 +19,7 @@ def self.import_csv(csv, organization)
 
         loc.save!
       end
+      []
     end
 
     def self.csv_export_headers

app/models/donation_site.rb

@@ -46,6 +46,7 @@ def self.import_csv(csv, organization)
       loc.organization_id = organization
       loc.save!
     end
+    []
   end
 
   def self.csv_export_headers

app/models/partner.rb

@@ -158,14 +158,19 @@ def approvable?
 
   # better to extract this outside of the model
   def self.import_csv(csv, organization_id)
+    errors = []
     organization = Organization.find(organization_id)
 
     csv.each do |row|
       hash_rows = Hash[row.to_hash.map { |k, v| [k.downcase, v] }]
 
       svc = PartnerCreateService.new(organization: organization, partner_attrs: hash_rows)
       svc.call
+      if svc.errors.present?
+        errors << "#{svc.partner.name}: #{svc.partner.errors.full_messages.to_sentence}"
+      end
     end
+    errors
   end
 
   def self.csv_export_headers

app/models/storage_location.rb

@@ -121,6 +121,7 @@ def self.import_csv(csv, organization)
       loc.organization_id = organization
       loc.save!
     end
+    []
   end
 
   # NOTE: We should generalize this elsewhere -- Importable concern?

app/services/partner_create_service.rb

@@ -11,27 +11,27 @@ def initialize(organization:, partner_attrs:)
   def call
     @partner = organization.partners.build(partner_attrs)
 
-    unless @partner.valid?
+    if @partner.valid?
+      ActiveRecord::Base.transaction do
+        @partner.save!
+
+        Partners::Profile.create!({
+                                    partner_id: @partner.id,
+                                    name: @partner.name,
+                                    enable_child_based_requests: organization.enable_child_based_requests,
+                                    enable_individual_requests: organization.enable_individual_requests,
+                                    enable_quantity_based_requests: organization.enable_quantity_based_requests
+                                  })
+      rescue StandardError => e
+        errors.add(:base, e.message)
+        raise ActiveRecord::Rollback
+      end
+    else
       @partner.errors.each do |error|
         errors.add(error.attribute, error.message)
       end
     end
 
-    ActiveRecord::Base.transaction do
-      @partner.save!
-
-      Partners::Profile.create!({
-                                  partner_id: @partner.id,
-                                  name: @partner.name,
-                                  enable_child_based_requests: organization.enable_child_based_requests,
-                                  enable_individual_requests: organization.enable_individual_requests,
-                                  enable_quantity_based_requests: organization.enable_quantity_based_requests
-                                })
-    rescue StandardError => e
-      errors.add(:base, e.message)
-      raise ActiveRecord::Rollback
-    end
-
     self
   end
 

spec/fixtures/files/partners_with_invalid_email.csv

@@ -0,0 +1,4 @@
+name,email
+Partner 1,[email protected]
+Partner 2,this is not an email address
+Partner 3,[email protected]

spec/requests/partners_requests_spec.rb

@@ -222,6 +222,26 @@
         expect(response).to have_error "Check headers in file!"
       end
     end
+
+    context "csv file with invalid email address" do
+      let(:file) { fixture_file_upload("partners_with_invalid_email.csv", "text/csv") }
+      subject { post import_csv_partners_path, params: { file: file } }
+
+      it "invokes .import_csv" do
+        expect(model_class).to respond_to(:import_csv).with(2).arguments
+      end
+
+      it "redirects to :index" do
+        subject
+        expect(response).to be_redirect
+      end
+
+      it "presents a flash notice message displaying the import errors" do
+        subject
+        expect(response).to have_error(/The following #{model_class.name.underscore.humanize.pluralize} did not import successfully:/)
+        expect(response).to have_error(/Partner 2: Email is invalid/)
+      end
+    end
   end
 
   describe "POST #create" do