Problem/Motivation

In issue #3223988: Add an option to include url parameters in the request_path a small error slipped in. 

$url = Url::fromRoute(
  '<current>',
  ['query' => \Drupal::request()->query->all()],
  ['absolute' => TRUE]
)->toString();

This makes sense:
array $route_parameters: (optional) An associative array of route parameter names and values.
Now the query string will be:
https://domain.com/page?query=[parameter=value]

Proposed resolution

replace
['query' => \Drupal::request()->query->all()]
with
[\Drupal::request()->query->all()],

CommentFileSizeAuthor
#2 open_readspeaker-rework_add_query_parameters-3247605-2.patch583 bytesjoshahubbers

Issue fork open_readspeaker-3247605

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

JoshaHubbers created an issue. See original summary.

joshahubbers’s picture

joshahubbers commented
StatusFileSize
new open_readspeaker-rework_add_query_parameters-3247605-2.patch583 bytes

This patch fixes it.

Only thing is that if there are no query parameters, there is still a '?' added to the url. I could not fix this nicely. Replacing the empty array with null will generate a parameter '?0'.

joshahubbers’s picture

joshahubbers commented
Issue summary: View changes

sunlix made their first commit to this issue’s fork.

sunlix opened merge request !4

sunlix’s picture

sunlix
Primary language English
Location Wesel
commented

@JoshaHubbers

thank a lot for your elaboration.

I have added an issue-fork and opened a merge-request. Can check the change?

Proposed resolution

replace

    $url = Url::fromRoute(
      '<current>',
      ['query' => \Drupal::request()->query->all()],
      ['absolute' => TRUE]
    )->toString();

with

    $url = Url::fromRoute(
      '<current>',
      \Drupal::request()->query->all(),
      ['absolute' => TRUE]
    )->toString();

\Drupal::request()->query->all() already return a key/value based array. No need to wrap that into another array.
My bad. I misread the documentation on Drupal\Core\Url :-)
By that change the Drupal internals handles empty arrays correctly so no unnessercary ? is added.

sunlix’s picture

sunlix
Primary language English
Location Wesel
commented
Status: Active » Needs review

  • sunlix committed d4838db on 8.x-1.x 
    Issue #3247605 by sunlix, JoshaHubbers: Rework: Add an option to include...
sunlix’s picture

sunlix
Primary language English
Location Wesel
commented
Status: Needs review » Fixed
joshahubbers’s picture

joshahubbers commented

Sorry for my late reaction, but thanx for your fast fix. ;-)

I wrapped \Drupal::request()->query->all() in [] because I was not entirely shure this would work if there were no parameters present. But I suppose you tested that case ;-).

Status: Fixed » Closed (fixed)

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