Provide a GET query parameter default argument plugin

This appears to be working well so far, with the exception of a minor form issue found below.

To test, I used a D8 install profile I've been maintaining that sets up a content structure with various relatioships. Then, I created content with relationships and attempted to filter based on those parameters. You can see the exported view in the profile repository.

There was an issue with the contextual filter having a required form field, causing other filter options to fail even when this filter option wasn't selected. To address this, I removed the required attribute, but there may be a better solution to that particular issue (see attached patch).

I ran into an issue with regard to multiple argument filtering behavior, but based on the QueryParameter plugin architecture, I believe this issue may lie outside the plugin referenced in this patch. To validate this, I built a separate View that did not use the QueryParameter plugin, and replicated the issue. Currently, d.o issue search is giving me a WSOD, so I can't verify if this issue is known already. Once I can verify, I'll follow-up here with a link to a related or new issue.

It would be great to have some unit test coverage ...

1. +++ b/core/modules/views/lib/Drupal/views/Plugin/views/argument_default/QueryParameter.php
   @@ -0,0 +1,110 @@
   +  /**
   +   * The request stack.
   +   *
   +   * @var \Symfony\Component\HttpFoundation\RequestStack
   +   */
   +  protected $requestStack;
   

   <3, but I wonder whether we should get the request from the view object itself instead. This would allow us here to safe at least a couple of lines.

2. +++ b/core/modules/views/lib/Drupal/views/Plugin/views/argument_default/QueryParameter.php
   @@ -0,0 +1,110 @@
   +      '#title' => t('Query parameter'),
   ...
   +      '#title' => t('Fallback value'),
   ...
   +      '#title' => t('Multiple values'),
   +      '#description' => t('Conjuction to use when handling multiple values. E.g. "?value[0]=a&value[1]=b".'),
   

   We can use $this->t() always ...

3. +++ b/core/modules/views/lib/Drupal/views/Plugin/views/argument_default/QueryParameter.php
   @@ -0,0 +1,110 @@
   +      '#description' => t('The GET query parameter to use.'),
   ...
   +      '#description' => t('The fallback value to use when the above GET query parameter is not present.'),
   

   Is this really always about GET, ... note that we might have AJAX request, crazy exposed forms/VBO and what not.

dawehner, I was wondering the same thing about your third point. I don't know enough about how the request handling works, but if it's parsing out any request parameters, whether they be GET or POST, then it makes sense to clarify those descriptions. I would anticipate users wanting that capability, regardless.

Added some nice little unit test

I would argue that https://api.drupal.org/api/drupal/core%21modules%21views%21lib%21Drupal%... should inject the current request into the view executable.

Well I don't have a checkout of drupal by hand.

Opened up a critical for that: #2261181: Always pass in the request into the view executable

So for now, #6 is fine and kind of orthogonal to the request issue.

Is it true that arguments so far were admin only? So they were considered safe. If so, are we introducing a security weakness / hole here by changing that assumption?

@chx
Can you be more detailed? Views has argument validation so we can sure that things are always valid.

Ah, never mind, I get it. Arguments themselves were always user supplied so they weren't treated as safe. And this just changes the defaults but the defaults don't get any special treatment, they just get loaded into arguments which are treated unsafe.

Good to go.

Reroll for #2247991: [May 27] Move all module code from …/lib/Drupal/… to …/src/… for PSR-4.

Committed 742fe61 and pushed to 8.x. Thanks!

diff --git a/core/modules/views/src/Plugin/views/argument_default/QueryParameter.php b/core/modules/views/src/Plugin/views/argument_default/QueryParameter.php
index e6aa0b4..d8eb1ab 100644
--- a/core/modules/views/src/Plugin/views/argument_default/QueryParameter.php
+++ b/core/modules/views/src/Plugin/views/argument_default/QueryParameter.php
@@ -7,9 +7,6 @@

 namespace Drupal\views\Plugin\views\argument_default;

-use Symfony\Component\DependencyInjection\ContainerInterface;
-use Symfony\Component\HttpFoundation\RequestStack;
-
 /**
  * A query parameter argument default handler.
  *
@@ -72,8 +69,8 @@ public function getArgument() {
     if ($current_request->query->has($this->options['query_param'])) {
       $param = $current_request->query->get($this->options['query_param']);
       if (is_array($param)) {
-        $conjuction = ($this->options['multiple'] == 'and') ? ',' : '+';
-        $param = implode($conjuction, $param);
+        $conjunction = ($this->options['multiple'] == 'and') ? ',' : '+';
+        $param = implode($conjunction, $param);
       }

       return $param;

Fixed on commit. The code change is just a spelling mistake.