Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ekes’s picture

Component: user system » user.module

Two instances, both in user module, both in views default_argument.

ekes’s picture

All instances of this (and one of menu_get_object('node')) are in Drupal\user\Plugin\views\argument_default\User::getArgument

cf.

#1817630: Cleanup get_argument() on node/user default argument
#2028505: Provide the request object for ArgumentDefaultPluginBase::getArgument() as argument

ekes’s picture

Status: Active » Needs review
Issue tags: +VDC
FileSize
4.15 KB

The present getArgument(), returns the user object from any path defining user, or one with node and then the related author. This only makes sense in a non-page context (ie not for a conditional argument on a page.)
Attached patch does the same from the route.

It's confusing to say this comes from the URI now - that's more Raw - however the replaced 'route context' is probably not good for usability.

dawehner’s picture

It is great to see that there is some effort behind removing menu_get_object().

  1. +++ b/core/modules/user/lib/Drupal/user/Plugin/views/argument_default/User.php
    @@ -37,44 +79,24 @@ public function buildOptionsForm(&$form, &$form_state) {
    +    // If there is a user object in the current route.
    +    if ($this->request->attributes->has('user')) {
    +      $user = $this->request->attributes->get('user');
    +      return $user->id();
         }
    

    Maybe we should first check for the interface just in case maybe someone came to the idea that naming something user is a good idea.

  2. +++ b/core/modules/user/lib/Drupal/user/Plugin/views/argument_default/User.php
    @@ -37,44 +79,24 @@ public function buildOptionsForm(&$form, &$form_state) {
    +    if (!empty($this->options['user']) && $this->request->attributes->has('node')) {
    +      $node = $this->request->attributes->get('node');
    +      return $node->getAuthorId();
    

    Some with node then.

dawehner’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Novice

These two changes should be possible done by a novice.

ekes’s picture

Status: Needs work » Needs review
FileSize
4.28 KB
1.4 KB

Add patch with instance check.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for the final change.

ekes’s picture

Correcting the comment.

> - * Default argument plugin to extract a user via menu_get_object.
> + * Default argument plugin to extract a user from request.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: user_default_argument.2095961-08.patch, failed testing.

ekes’s picture

Seems there are issues with the testbot at the moment.

ekes’s picture

Status: Needs work » Needs review
ekes’s picture

Re-upload to force final test.

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Still like before

catch’s picture

Status: Reviewed & tested by the community » Fixed

I don't like that we replaced one magic string with another for getting router arguments, but that's not the fault of this patch. Committed/pushed to 8.x, thanks!

Status: Fixed » Closed (fixed)

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