4486: address pickup email issues

[DariusPirvulescu]

Resolves #4486

Description

This PR adds validation for Partner::Profile.pick_up_email. This field is a plain string and the lack of validation caused messy data.

The pick_up_email field should accept multiple comma-separated email addresses, not more than 3, and with optional whitespace between the addresses.
The validation checks this and if every email is in a valid format (using URI::MailTo::EMAIL_REGEXP).

Type of change

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

• added tests for the new pick_up_email model validation in profile_spec.rb
• • bundle exec rspec spec/models/partners/profile_spec.rb or bundle exec rspec spec/models/partners/profile_spec.rb:170
• updated the distribution_mailer_spec.rb to handle multiple pick_up_email addresses
• • bundle exec rspec spec/mailers/distribution_mailer_spec.rb

I am not experienced with testing and I could definitely use some help, any suggestion is more than welcome.

Screenshots

[dorner]

Non-blocking comment. Thanks for the contribution!

[dorner]

@cielf did you want to test?

[cielf]

@cielf did you want to test?

Yeah, I think I should.

[cielf]

@DariusPirvulescu I entered "[email protected]. Call first" when updating a partner profile. It saved it, with no error. That doesn't seem right to me. Similarly "Call first [email protected]"

[cielf]

[Note: I have not yet done the tests against prod that we need to do before merging.]

[DariusPirvulescu]

Thanks for testing.
I added more tests, and changed the code. Now it will fail validation for pick_up_email like: "Call first [email protected]" or "[email protected]. Call first".

I'm wondering if you would want me to modify the input component, maybe change "Person's -> Persons" or add a tooltip informing users to comma separate email addresses ([email protected], [email protected])

[cielf]

We should put something about the format.... Maybe just add "(comma-separated if multiple addresses) to the label?

[DariusPirvulescu]

I modified the label and added a placeholder

LGTM

[cielf]

It looks pretty good to me. There's one edge case that I want to check if it exists in the wile (waiting for approval with a bad email address), which will hold this up a bit. (edit we do have one. so need to test with that prod data)

[cielf]

Hrm. If you already have a bad email it will let you approve (Approving changes the partner record, not the profile record, so it's not re-saving that).

However, there's only one case that has been saved with bad email and is waiting for approval. We can probably live with that -- I can reach out to the bank. @dorner -- are you good with that, or should we make the change?

[dorner]

I think the bank should be able to fix it if it's just one case.

[cielf]

(nods) There are other ones that are bad -- just not in that status.