[BUG]: Organization editing raises 500 errors when there is a partner validation issue

[awwaiid]

Is there an existing issue for this?

• I have searched the existing issues

Current Behavior

When a bank saves their organization details (in My Organization -> Edit) we validate not only the organization, but also the Partners. Some partners have legacy data which is invalid, so the whole thing ends up with a 500 error.

For an example from bugsnag, ((direct link example)[https://app.bugsnag.com/ruby-for-good/human-essentials/errors/6703ebb0dab091357e9a2e6e]):

ActiveRecord::RecordInvalid 
organizations#update
Validation failed: Pick up email Invalid email address for '/'., Pick up email Invalid email address for 'example1@example.org/example2@otherexample.org'.

  | # github.com/rubyforgood/human-essentials/issues/3264
-- | --
  | next if organization.send(field)
  | organization.partners.each do \|partner\|
  | partner.profile.update!(field => organization.send(field))
  | end
  | end
  | end

So the error is that the partner has an invalid email address, since it is actually two email addresses separated by a slash, and we've increased our validation since the data was entered.

This is tricky to reproduce since new Partner data is now validated! So you'll need to force invalid data into a partner to test, and that's how you can do unit test as well.

Expected Behavior

Instead of getting a 500 error on saving the organization details with invalid partner data, show the user the validation error like we would for other organization-level validation errors.

So a flash message with the details of the validation issue.

Steps To Reproduce

• Force invalid partner email via the rails console, like partner.update_columns(email: 'not/an/email')
• Use the running UI to edit the bank that the partner is associated with
• Try to save
• Get a 500!!!

Environment

No response

Criteria for Completion

• User gets a flash error instead of a 500 when saving an organization with invalid partner data
• A unit or request spec that validates this behavior

Anything else?

No response

Code of Conduct

• I've read the Code of Conduct and understand my responsibilities as a member of the Ruby for Good community

[jp524]

I'd like to help with this!

[cielf]

Please do!

[jp524]

I was able to reproduce the issue by setting the partner's profile with an invalid email, like: partner.profile.update_columns(pick_up_email: 'not/an/email').

Setting the partner email to be invalid actually doesn't raise any errors. Currently we're only doing validation during the creation process, not when updating:

human-essentials/app/models/partner.rb

Lines 51 to 52 in 64d20c1

	validates :email, presence: true, uniqueness: { case_sensitive: false },
	format: { with: URI::MailTo::EMAIL_REGEXP, on: :create }

Is this intentional? If not, I'll go ahead and fix it as part of my PR.

[cielf]

Hey @jp524 -- For this problem I think you should be looking at the pickup person email, not the partner's email. So that's on the partner profile.

That being said, I can't think of a reason that we would want no validation check of the email on updating the partner.