Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[router] Does not take additional GET parameter in account if it has a default value. #18035

Closed
Alsatian67 opened this issue Mar 6, 2016 · 7 comments

Comments

@Alsatian67
Copy link

In a Controller, I have two actions :

    /**
     * @Route("/search", name="search_with_default", defaults={"p"=1})
     */
    public function searchWithDefaultAction(Request $request)
    {
        return $this->render('default/index.html.twig');
    }

    /**
     * @Route("/search", name="search_without_default")
     */
    public function searchWithoutDefaultAction(Request $request)
    {
        return $this->render('default/index.html.twig');
    }

In twig, I generate routes to these actions with an additional GET parameter 'p' :

    {{ path('search_with_default',{'p':2}) }}<br/>
    {{ path('search_without_default',{'p':2}) }}

Result :

/app_dev.php/search
/app_dev.php/search?p=2 

The first one, totally ignore the value assigned on 'p'.

Bug reproduced here :

https://github.com/Alsatian67/symfony-standard

(index page shows the two paths)

@Alsatian67 Alsatian67 changed the title [router] Bug: does not take additional GET parameter in account if it has a default value. [router] Does not take additional GET parameter in account if it has a default value. Mar 7, 2016
@xabbuh
Copy link
Member

xabbuh commented Mar 7, 2016

Does this only happen if both routes are configured for the same path?

@Alsatian67
Copy link
Author

No, that's just to show the bug. It is always so.

simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 7, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 8, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 8, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 8, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 8, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 8, 2016
@hhamon
Copy link
Contributor

hhamon commented Mar 9, 2016

I would say that's a normal behavior. The second argument of the path function is meant to replace the placeholders in the url patterns. So if the url doesn't have a {placeholder} but you have a default hardcoded value set, then it's taken. The extra GET parameters are only added for parameters / placeholders that are not defined in the route definition.

@Alsatian67
Copy link
Author

I don't think it's normal.

Default means value when not defined.
So I can't immagine the default is overwriting the submitted one (as extra or not).

And when I enter an url with ?p=2 manually, the controller receives 2.

So this can not be intended by url generating and not when it retrives the parameter value.

simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
simonhayre added a commit to simonhayre/symfony that referenced this issue Mar 11, 2016
@weaverryan
Copy link
Member

It's weird because defaults is meant really to fill in the {wildcard} values. So the whole example looks strange - why add a defaults and then want to have query parameters added?

That being said, I would agree it's a bug - though really edge-case. The intention is that if there is no wildcard to fill in, the extra parameters are added as query parameters. In this case, the first route has no p wildcard, so I would also expect it to be added as a query parameter.

But again, it's weird :). You would fetch the p defaults value in a different way than the p query parameter: they're two different things entirely.

Btw @Alsatian67 thanks for the very clear report and fork - it's a weird situation, but very easy to understand!

@Tobion
Copy link
Member

Tobion commented Mar 13, 2016

Duplicate of #10940

@Tobion Tobion closed this as completed Mar 13, 2016
@Alsatian67
Copy link
Author

@weaverryan

Let me explain my usage :

It's a search page with many optional parameters :

?page=1&name=&sortby=&date=&category= ...

Each of them are optional and mostly not used.

So I don't like a route like /{page}/{name}/{sortby} ...

But I want to set defaults like page = 1, name = "", sortby = "name"

;)

fabpot added a commit that referenced this issue Apr 15, 2016
…lt (Tobion)

This PR was merged into the 2.3 branch.

Discussion
----------

[Routing] add query param if value is different from default

| Q             | A
| ------------- | ---
| Branch?       | 2.3
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | most likely not
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | #10940, #18111, #18035
| License       | MIT
| Doc PR        | -

Commits
-------

1ef2edf [Routing] add query param if value is different from default
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants