diff --git a/app/models/partner.rb b/app/models/partner.rb index 4d7175e1dd..2e7b5ab77b 100644 --- a/app/models/partner.rb +++ b/app/models/partner.rb @@ -48,8 +48,7 @@ class Partner < ApplicationRecord validates :organization, presence: true validates :name, presence: true, uniqueness: { scope: :organization } - validates :email, presence: true, uniqueness: { case_sensitive: false }, - format: { with: URI::MailTo::EMAIL_REGEXP, on: :create } + validates :email, presence: true, uniqueness: { case_sensitive: false }, format: { with: URI::MailTo::EMAIL_REGEXP } validates :quota, numericality: { greater_than_or_equal_to: 0 }, allow_blank: true diff --git a/app/models/partners/profile.rb b/app/models/partners/profile.rb index 5c2de0381f..b2806db5bf 100644 --- a/app/models/partners/profile.rb +++ b/app/models/partners/profile.rb @@ -158,14 +158,14 @@ def pick_up_email_addresses emails = split_pick_up_emails if emails.size > 3 - errors.add(:pick_up_email, "There can't be more than three pick up email addresses.") + errors.add(:pick_up_email, "can't have more than three email addresses") nil end if emails.uniq.size != emails.size - errors.add(:pick_up_email, "There should not be repeated email addresses.") + errors.add(:pick_up_email, "should not have repeated email addresses") end emails.each do |e| - errors.add(:pick_up_email, "Invalid email address for '#{e}'.") unless e.match? URI::MailTo::EMAIL_REGEXP + errors.add(:pick_up_email, "is invalid") unless e.match? URI::MailTo::EMAIL_REGEXP end end end diff --git a/app/services/organization_update_service.rb b/app/services/organization_update_service.rb index d49e650acf..9ebd27e4a8 100644 --- a/app/services/organization_update_service.rb +++ b/app/services/organization_update_service.rb @@ -30,7 +30,7 @@ def update(organization, params) result = organization.update(org_params) return false unless result - update_partner_flags(organization) + return false unless update_partner_flags(organization) true end @@ -46,6 +46,9 @@ def update_partner_flags(organization) next if organization.send(field) organization.partners.each do |partner| partner.profile.update!(field => organization.send(field)) + rescue ActiveRecord::RecordInvalid => e + organization.errors.add(:base, "Profile for partner '#{e.record.partner.name}' had error(s) preventing the organization from being saved. #{e.message}") + return false end end end diff --git a/app/services/partner_profile_update_service.rb b/app/services/partner_profile_update_service.rb index 3fa2b6bbf7..7098599b80 100644 --- a/app/services/partner_profile_update_service.rb +++ b/app/services/partner_profile_update_service.rb @@ -18,6 +18,8 @@ def call @profile.served_areas.destroy_all @profile.attributes = @profile_params @profile.save!(context: :edit) + else + @error = "Partner '#{@partner.name}' had error(s) preventing the profile from being updated: #{@partner.errors.full_messages.join(", ")}" end end end diff --git a/spec/services/organization_update_service_spec.rb b/spec/services/organization_update_service_spec.rb index def193ac96..5943bc71a0 100644 --- a/spec/services/organization_update_service_spec.rb +++ b/spec/services/organization_update_service_spec.rb @@ -88,6 +88,52 @@ end end end + + context "when partner is invalid" do + let(:params) { {name: "New organization"} } + + before do + organization.update!(enable_individual_requests: false) + # Want to have an invalid email on purpose + # rubocop:disable Rails::SkipsModelValidations + partner_one.update_columns(email: "not/an_email") + # rubocop:enable Rails::SkipsModelValidations + end + + it "updates the organization and returns true" do + expect(described_class.update(organization, params)).to eq(true) + expect(organization.name).to eq("New organization") + end + end + + context "when partner profile is invalid" do + let(:profile) { partner_one.profile } + let(:params) { {name: "New organization"} } + + before do + organization.update!(enable_individual_requests: false) + # Want to have an invalid email on purpose + # rubocop:disable Rails::SkipsModelValidations + profile.update_columns(pick_up_email: "not/an/email") + # rubocop:enable Rails::SkipsModelValidations + end + + it "returns false" do + expect(described_class.update(organization, params)).to eq(false) + end + + it "adds an error message to the organization" do + described_class.update(organization, params) + expect(organization.errors.full_messages).to include( + "Profile for partner '#{partner_one.name}' had error(s) preventing the organization from being saved. Validation failed: Pick up email is invalid" + ) + end + + it "updates the organization regardless" do # because updating the organization takes place before updating partner profiles + described_class.update(organization, params) + expect(organization.name).to eq("New organization") + end + end end end diff --git a/spec/services/partner_profile_update_service_spec.rb b/spec/services/partner_profile_update_service_spec.rb index a0dccd7668..25b2446bc5 100644 --- a/spec/services/partner_profile_update_service_spec.rb +++ b/spec/services/partner_profile_update_service_spec.rb @@ -43,6 +43,7 @@ end end end + context "when served area client shares pre-exist" do let!(:original_served_area_1) { create(:partners_served_area, partner_profile: profile, county: county_1, client_share: 51) } let!(:original_served_area_2) { create(:partners_served_area, partner_profile: profile, county: county_2, client_share: 49) } @@ -93,5 +94,29 @@ end end end + + context "when the partner has invalid attributes" do + let(:partner) { profile.partner } + + before do + # Want to have an invalid email on purpose + # rubocop:disable Rails::SkipsModelValidations + partner.update_columns(email: "not/an_email") + # rubocop:enable Rails::SkipsModelValidations + end + + it "returns failure" do + result = PartnerProfileUpdateService.new(partner, partner_params, basic_correct_attributes).call + expect(result.success?).to eq(false) + expect(result.error.to_s).to include("Partner '#{partner.name}' had error(s) preventing the profile from being updated: Email is invalid") + end + + it "doesn't update the partner profile" do + old_pick_up_email = profile.pick_up_email + valid_profile_params = {pick_up_email: "new_email@test.com "} + PartnerProfileUpdateService.new(profile.partner, partner_params, valid_profile_params).call + expect(profile.pick_up_email).to eq(old_pick_up_email) + end + end end end