diff --git a/app/controllers/concerns/importable.rb b/app/controllers/concerns/importable.rb index 90152f41e4..488614efac 100644 --- a/app/controllers/concerns/importable.rb +++ b/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 diff --git a/app/models/concerns/provideable.rb b/app/models/concerns/provideable.rb index 55380639f8..56a64620c1 100644 --- a/app/models/concerns/provideable.rb +++ b/app/models/concerns/provideable.rb @@ -19,6 +19,7 @@ def self.import_csv(csv, organization) loc.save! end + [] end def self.csv_export_headers diff --git a/app/models/donation_site.rb b/app/models/donation_site.rb index 95195b5d1a..99ddd71d97 100644 --- a/app/models/donation_site.rb +++ b/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 diff --git a/app/models/partner.rb b/app/models/partner.rb index 9ac073d186..4d7175e1dd 100644 --- a/app/models/partner.rb +++ b/app/models/partner.rb @@ -158,6 +158,7 @@ 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| @@ -165,7 +166,11 @@ def self.import_csv(csv, organization_id) 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 diff --git a/app/models/storage_location.rb b/app/models/storage_location.rb index 53fcdc3d25..3c7f7a2f5f 100644 --- a/app/models/storage_location.rb +++ b/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? diff --git a/app/services/partner_create_service.rb b/app/services/partner_create_service.rb index 0028155e32..6b81bfae59 100644 --- a/app/services/partner_create_service.rb +++ b/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 diff --git a/spec/fixtures/files/partners_with_invalid_email.csv b/spec/fixtures/files/partners_with_invalid_email.csv new file mode 100644 index 0000000000..e78f35d9dc --- /dev/null +++ b/spec/fixtures/files/partners_with_invalid_email.csv @@ -0,0 +1,4 @@ +name,email +Partner 1,partner1@example.com +Partner 2,this is not an email address +Partner 3,partner3@example.com \ No newline at end of file diff --git a/spec/requests/partners_requests_spec.rb b/spec/requests/partners_requests_spec.rb index 4acd3cb63a..9084b6251d 100644 --- a/spec/requests/partners_requests_spec.rb +++ b/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