I am porting this module as part of the CI&T/Acquia Hackathon in Ningbo today
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 2693349_4_port-email-verify.patch | 93.73 KB | adammalone |
| #2 | email_verify-d8.patch | 39.48 KB | adammalone |
I am porting this module as part of the CI&T/Acquia Hackathon in Ningbo today
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 2693349_4_port-email-verify.patch | 93.73 KB | adammalone |
| #2 | email_verify-d8.patch | 39.48 KB | adammalone |
Comments
Comment #2
adammaloneAttaching this patch for the upgrade to D8. A couple of differences remain for the time being:
Also, could a D8 branch be made so we can make patches against that directly.
Comment #3
oadaeh commentedI created an 8.x-2.x branch, based on the 7.x-2.x branch.
Several changes to the 7.x-1.x have been in the works in the 7.x-2.x branch for some time, and the last major one went in today. Most of the rest of them were committed 2 weeks before this issues was posted.
Since you didn't create this issue before starting work, I couldn't tell you that.
One of those changes was #2426859: Allow for more flexible form verification , which addresses your second point.
Comment #4
adammaloneNo problem, I've updated the latest patch to include anything updated between 7.x-1.x and 7.x-2.x. I've also replaced the way form fields are validated - I've added a new field widget to apply to fields instead.
Comment #5
amh5514 commentedI used git to download the 8.x-2.x branch and it appears to be the 7.x-2.x version.
Comment #7
oadaeh commented@amh5514: that is because the patch in #4 had not been applied to the 8.x-2.x branch.
However, I have just applied it.
I am not changing the status of the issue, because there have been no reviews of the code or functionality.
Comment #8
jurgenhaasThis is a great module and I'm glad to see that a port to D8 has started. I'm just wondering if you wanted to consider using a generic PHP package for the validation part as such an approach would help to get Drupal off the island? I was wondering around and found one that has 1+ million installs and is extensible. Something like that sounds like we could easily get a huge maintainer base and this module could focus on the Drupal integration. What do you think?
Edit: forgot to provide the link to that library: https://github.com/egulias/EmailValidator
Comment #9
ker688@gmail.com commentedComment #10
oadaeh commented@ker688@gmail.com are you actually working on completing the port?
I'm marking this as unassigned until I hear back, as it has been ~1.5 months with no update.
Comment #11
oadaeh commented@jurgenhaas That library is actually gets installed as part of a basic "composer install" for D8. I have not looked into it to see if it does the same sort of validation or not, but it is certainly worth investigating.
Comment #12
jurgenhaas@oadaeh thanks for pointing that out, I didn't realize. That's another reason I guess to build on top of that. Even if it doesn't do everything we need, the architecture is certainly extendable and from a maintenance point of view it would be smart to re-use something that's well maintained.
Comment #13
agoradesign commented@jurgenhaas as far as I have seen in a couple of minutes I've scanned through code and features of this module, I've seen that the D8 branch already calls the egulias library to do the syntactical check. Doing a plain DNS record check would also be possible (additional parameter to isValid()) call. Afaik the other checking stuff that this module does, by actually contacting the SMTP server, is not done by that library.
But I have seen that there are other libraries out there, that do these kinds of checks, e.g. https://github.com/zytzagoo/smtp-validate-email
Maybe we should have a look on this and use that instead of doing the check by custom coding? In theory, we should first check the built-in egulias library for a syntactical check incl DNS record check and afterwards use a SMTP validation based library additionally.
What do you think?
Comment #14
geek-merlinYes, #13 makes sense a lot.