Currently the email_registration module only contains procedural code.

Functions like email_registration_unique_username and email_registration_cleanup_username could be moved to a separate service.

The patch attached, moves these 2 functions to a separate service.

CommentFileSizeAuthor
#8 move_username_functions-2831238-8.patch7.3 KBJeroenT
#6 move_username_functions-2831238-6.patch6.7 KBJeroenT
#6 interdiff-2831238-2-6.txt592 bytesJeroenT
#4 interdiff.txt10.16 KBJeroenT
#4 move_username_functions-2831238-3.patch9.81 KBJeroenT
#2 move_username_functions_to_OO.patch6.44 KBJeroenT
move_username_functions_to_OO.patch6.44 KBJeroenT
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JeroenT created an issue. See original summary.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
FileSize
move_username_functions_to_OO.patch6.44 KB

Re-uploading patch to run tests.

Status: Needs review » Needs work

The last submitted patch, 2: move_username_functions_to_OO.patch, failed testing.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Status: Needs work » Needs review
FileSize
move_username_functions-2831238-3.patch9.81 KB
interdiff.txt10.16 KB

Fixed coding standards + added email_registration.services.yml file.

Status: Needs review » Needs work

The last submitted patch, 4: move_username_functions-2831238-3.patch, failed testing.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Status: Needs work » Needs review
FileSize
interdiff-2831238-2-6.txt592 bytes
move_username_functions-2831238-6.patch6.7 KB

Status: Needs review » Needs work

The last submitted patch, 6: move_username_functions-2831238-6.patch, failed testing.

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Status: Needs work » Needs review
FileSize
move_username_functions-2831238-8.patch7.3 KB
andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented

Idea looks good except name for service

+++ b/email_registration.services.yml
@@ -0,0 +1,4 @@
+  email_registration.username_helper:
+    class: Drupal\email_registration\UsernameHelper

helper really confusion and tells nothing about what this file for

JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented
Status: Needs review » Needs work
JeroenT’s picture

JeroenT
Primary language Dutch
Location 🇧🇪
CreditAttribution: JeroenT at iO commented

@andypost,

Any suggestion for the classname?

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Related issues: +#2402445: Implement object oriented form alters

not yet( maybe better to split it more because clean-up can be a static class member
but this will just add more cpu usage for autoloading

So overall only db query could be in service but here's #551626: slow query in email_registration_user_login_validate

btw This conversion may have sense in terms of #657472: Add setting to allow users to login with email address or username but everything in hooks that means procedural...

So after thinking about it I'd better postponed that until #2402445: Implement object oriented form alters

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Needs work » Postponed

yep, let's postpone on OOP way for forms

andypost’s picture

andypost
he/him
Primary language Russian
CreditAttribution: andypost as a volunteer and at Skilld commented
Status: Postponed » Needs work
Related issues: +#3021912: Plan for creating email registration 8.x-1.0 final stable

As release 1.0 on a way, and we getting rid of legacy, it makes sense again

Grevil’s picture

Grevil CreditAttribution: Grevil at DROWL.de commented
Title: Move username functions to OO code » [2.x] Move username functions to OO code
Priority: Normal » Minor
Grevil’s picture

Grevil CreditAttribution: Grevil at DROWL.de commented
Version: 8.x-1.x-dev » 2.x-dev