Skip to content

Standardize callbacks for user secondary emails

What does this MR do?

This MR removes before_validation callbacks for user's secondary emails: public email and commit email. Those callbacks kept the validations from kicking-in - user would not be shown an error message when trying to change secondary emails to a not verified email address.

Another before_validation was introduced instead - reset_secondary_emails - which is called when primary email address is changed, and makes sure, that secondary emails are either verified addresses or are not set at all.

This makes code cleaner, but there is still some awkwardness left resulting from user having a primary email in user.email and also emails as user.emails, while user.email is not necessarily one of user.emails. Changing that requires another MR.

This MR also makes few implementation details methods, not used outside of class, private.

Notes:

  • there are also before_save callbacks for public email and commit email and I have left them in place, as Users::UpdateService enables skipping validations and that is used in few places in the codebase.

In general I would avoid skipping validations, but I don't have enough context here, maybe there is a good reason to do it in those places.

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Edited by Dan Jensen

Merge request reports

Loading