Similar to #1801356: Entity reference autocomplete using routes it might be the time now to implement routing for certain pieces of core. User autocomplete is certainly a good place to start.

CommentFileSizeAuthor
#17 interdiff.txt917 bytesdawehner
#17 drupal-1903684-17.patch9.34 KBdawehner
#13 interdiff.txt760 bytesdawehner
#13 drupal-1903684-13.patch9.45 KBdawehner
#11 interdiff.txt2.8 KBdawehner
#11 drupal-1903684-11.patch9.5 KBdawehner
#9 drupal-1903684-9.patch9.55 KBdawehner
#5 drupal-1903684-5.patch5.96 KBdawehner
#1 drupal-1903684-1.patch5.57 KBdawehner
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Active » Needs review
FileSize
drupal-1903684-1.patch5.57 KB

This does not work yet, because drupal_valid_path() is runned for the actual UI, though a review is highly welcomened.

Status: Needs review » Needs work

The last submitted patch, drupal-1903684-1.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review

We certainly don't want to have all the logic in the controller, so what is the route to go?

* Add a UserAutocomplete helper class, which get's it's stuff injected and returns the actual matches? The router controller just takes care about routing json? This seems to be for me the best approach.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Issue tags: +WSCCI

Add tag.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Title: User complete using routes » User autocomplete using routes
FileSize
drupal-1903684-5.patch5.96 KB

Let's see.

Status: Needs review » Needs work

The last submitted patch, drupal-1903684-5.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented

I'm not 100% sure what the best way to do that would be. I guess the actual page controller should be in the container, so it's requirements are injected,
but it seems to be that we should have another class, which contain the logic of how to query for the usernames.

Should this second class be used directly or should that also provide a service?

Crell’s picture

Crell CreditAttribution: Crell commented

See my most recent comments on #1801356: Entity reference autocomplete using routes for answers to all of @dawehner's questions. :-)

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
drupal-1903684-9.patch9.55 KB

Thanks for the really good guidance, I hope this does it as expected.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs review » Needs work

As with the sister issue, the request should be passed to the controller method, not to the constructor. Otherwise this looks fine.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
drupal-1903684-11.patch9.5 KB
interdiff.txt2.8 KB

There we go.

Status: Needs review » Needs work

The last submitted patch, drupal-1903684-11.patch, failed testing.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
drupal-1903684-13.patch9.45 KB
interdiff.txt760 bytes

Same problem as on the taxonomy issue.

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Needs review » Reviewed & tested by the community

If bot likes, I likes.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented 
+++ b/core/modules/user/lib/Drupal/user/UserAutocomplete.phpundefined
@@ -0,0 +1,81 @@
+    debug($string);

I think this is by mistake

+++ b/core/modules/user/lib/Drupal/user/UserAutocomplete.phpundefined
@@ -0,0 +1,81 @@
+        debug($string);

same here

Crell’s picture

Crell CreditAttribution: Crell commented
Status: Reviewed & tested by the community » Needs work

That's what I get for reviewing code while I have a cold. Good catch, jibran.

dawehner’s picture

dawehner
Primary language German
CreditAttribution: dawehner commented
Status: Needs work » Needs review
FileSize
drupal-1903684-17.patch9.34 KB
interdiff.txt917 bytes

Get well soon!

I should have seen that.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Status: Needs review » Reviewed & tested by the community

RTBC as per #14

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Looks good.

Committed to 8.x., I will push once tesbot has caught up a bit.

Automatically closed -- issue fixed for 2 weeks with no activity.