Hello on my project D9 core version 9.5.9, we have optional email fields. Still, there was a problem when some people left emails like test@test.test so we decided to install the Advanced Email Validation module in order to check on valid email domains. But this unfortunately breaks the logic of our user creation process. We create a user with an optional email, but when we try to create a user without an email(field is not required) we get an error message about not valid email format when the field is left empty.
Install clean Drupal 9. Install the Advanced Email Validation module and add configurations on /admin/config/people/advanced-email-validation. Go to admin->people->create. Add the user but leave the email field empty. And u will get an error like showed on the screen.
I'm not very experienced with Drupal, but I've been added easy fix for this problem with my patch, will add it here.
| Comment | File | Size | Author |
|---|---|---|---|
| #11 | 3365661-11-test-only.patch | 951 bytes | darvanen |
| #6 | 3365661-6.patch | 903 bytes | sumit-k |
| #5 | 3365661-5.patch | 849 bytes | darvanen |
Comments
Comment #2
vadym.tseiko commentedI solved this issue with a small local patch, if someone will get in the same spot you can use it.
Comment #3
darvanenThis is great, thanks. You're right, we shouldn't be attempting to validate empty email addresses.
A unit test on the affected method would be sufficient for this I think and then I'll happily commit it.
Comment #4
darvanenOk so looking further into this, the validator's job is to validate a string as a valid email or not. So the response it is providing is correct. Where we need to fix this is in the user Constraint which triggers the validation - it should not be testing an empty result.
The thing is though, optional email addresses for Drupal users causes problems - like not being able to use the 'forgot password' system. What are you doing to make the email address optional?
Comment #5
darvanenThis still requires testing, but is a better solution to the issue, per #4.
Comment #6
sumit-k commentedI have verified that the shared patch #5 is functioning correctly. However, it would be beneficial to enhance readability and adhere to Drupal coding standards by using empty() instead of negating the assignment within the if condition.
To address this, I have prepared an updated patch that implements the recommended changes. Kindly find the attached patch file.
Comment #7
sumit-k commentedComment #8
vadym.tseiko commentedHi @darvanen, the user's email is optional because of the strange flow with content management inside the project. I'm agreed with you that better to use the email address and this is not the best flow, but I've been asked, to fix a problem of creating users with no email. And yes first I've added changes to the user Constraint too. But then I thought that maybe there can be other fields that have optional email(like forms with not required field email, leave contact information or something so I added this check to the advanced email validator) So if field is required it would not let to leave field be empty then. But I guess your approach is better. Anyway, thanks for your response I will replace my patch to patch #6.
Comment #9
darvanen@sumit-k can you reference where in the coding standards this is mentioned please? And did you test it out with a user that has an empty email address? Please be specific about what you did to manually test the patch.
@Vadym.Tseiko fair enough, requirements are strange beasts from time to time, I imagine you used a hook of some kind to alter the User entity and the login form?
For other optional fields to use this module they will need a special implementation - which would be the place to check whether the string is empty first.
I think what this needs is a test to ensure *our* constraint doesn't fail even if the required field one does.
Comment #10
darvanenOh and this is NW for tests.
Comment #11
darvanenHere we go, this should fail.
Comment #14
darvanenCommitted.