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

Comments

dawehner’s picture

dawehner
Primary language German
commented
Status: Active » Needs review
StatusFileSize
new 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
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
commented
Issue tags: +WSCCI

Add tag.

dawehner’s picture

dawehner
Primary language German
commented
Title: User complete using routes » User autocomplete using routes
StatusFileSize
new 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
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 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
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1903684-9.patch9.55 KB

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

Crell’s picture

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
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1903684-11.patch9.5 KB
new 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
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1903684-13.patch9.45 KB
new interdiff.txt760 bytes

Same problem as on the taxonomy issue.

Crell’s picture

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 Toronto, Canada
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 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
commented
Status: Needs work » Needs review
StatusFileSize
new drupal-1903684-17.patch9.34 KB
new interdiff.txt917 bytes

Get well soon!

I should have seen that.

jibran’s picture

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

RTBC as per #14

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
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.