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

$this->form->create(...) generates a RoutingException: 'No parameter match found for URL' insides sub modules (libraries) #374

Closed
Ciaro opened this issue Mar 11, 2012 · 20 comments
Labels

Comments

@Ciaro
Copy link
Contributor

Ciaro commented Mar 11, 2012

$this->form->create(...) seems to generate a RoutingException in libraries (sub modules), which is very weird...

Uncaught exception 'lithium\net\http\RoutingException' with message 'No parameter match found for URL ('controller' => 'li3_docs.ApiBrowser', 'action' => 'index', 'library' => 'li3_docs')

How to reproduce:
Try adding $this->form->create(null); to any of the views of the library 'li3_docs'.

@nateabele
Copy link
Member

The library is being double-listed. Try removing the library key or changing controller to 'ApiBrowser'.

@Ciaro
Copy link
Contributor Author

Ciaro commented Mar 11, 2012

Without that key lithium will think the route is part of the main app, and try to render the templates and layout found in app/views/* (instead of for example app/libraries/li3_docs/views/*).

@Ciaro
Copy link
Contributor Author

Ciaro commented Mar 12, 2012

unset($url['library']); in form::create() seems to fix this. I will commit a pull request tomorrow with some unit tests to cover it.

@Ciaro
Copy link
Contributor Author

Ciaro commented Mar 17, 2012

This issue has been reported before, I somehow missed that. (@see #321)

@JoelLarson
Copy link

I managed to get around this issue by doing this in

/config/bootstrap/routes.php
Router::connect(
    '{:controller}/{:action}/{:args}',
    array('locale' => true),
    array('persist' => array('locale', 'controller'))
);

I'm not positive if it's a proper fix, but it removed the error for the moment.

EDIT:

I'd also like to note that this error was triggered when I enabled the g11n.php bootstrap and had a $this->form->create() in my layout.

Another Edit:

It turns out that the Route above fixed my " / " route, but failed with my " /admin/* " route. From what I'm digging through, I think the cause is in the Form library not being able to handle the request params properly.

@JoelLarson
Copy link

Okay, my half asleep brain has come to the conclusion that this problem would be fixed by allowing the Router to specify optional params rather than just required params. I think this would fix it because then the 'locale' param (in my case) would trigger the Route with the locale option and route it back to itself with the param removed and the global param set.

@marcghorayeb
Copy link
Member

@JoelLarson You can already have optional parameters with setting their default value to null in the second argument of router::connect iirc?

@nateabele
Copy link
Member

I think the actual solution here would be to make sure that enabling g11n.php also enables an if-block'd continuation route for locales, similar to this one:

Router::connect(
    '/{:locale:' . join('|', array_keys(Environment::get('locales'))) . '}/{:args}',
    array(),
    array('continue' => true)
);

Or maybe it's just always there in the default routing.

@DavidPersson Thoughts?

@mariuswilms
Copy link
Member

I like the idea of having a continuation-route wrapped in an if-block. I'll work on this :) We already have several routes that get connected only under certain conditions.

@ghost ghost assigned mariuswilms Apr 30, 2012
@nateabele
Copy link
Member

Really only the test ones, I think. The thing with locale routes, though, is that if you never use a locale key, it never gets triggered anyway, but yeah, I guess it's fine. One less route to hit if you're not using G11n.

mariuswilms added a commit to UnionOfRAD/framework that referenced this issue May 1, 2012
@Ciaro
Copy link
Contributor Author

Ciaro commented May 1, 2012

Ok, I'm clearly missing something here?

What does localized routing have to do with this issue :s?

@JoelLarson
Copy link

I agree. This is a patch to the underline issue. I was hoping my comments would help point someone into the right direction.

The routing issue is from a route param being set and the Router not knowing what to do with it.

Unfortunately I'm brand new to Lithium and am not sure how it all works in the core yet. :s

@mariuswilms mariuswilms reopened this May 1, 2012
@mariuswilms
Copy link
Member

Reviewing this discussion thread I get the feeling that @Ciaro and @JoelLarson are seeing two distinct issues. The latter being addressed by the default localized route. Reopening until submitted PR can be merged.

@Ciaro
Copy link
Contributor Author

Ciaro commented May 1, 2012

Here's the underlying issue: #388

So I'm closing this issue in favor of that pr.

@Ciaro Ciaro closed this as completed May 1, 2012
@mariuswilms
Copy link
Member

As long as no PR is merged/patch applied so that the issue reported here is solved. I think it's best to leave this ticket open. There's information in here that isn't attached to the PR.

Both issue and PR are linked together so we're reminded of this open ticket when working on the PR.

@mariuswilms mariuswilms reopened this May 1, 2012
@Ciaro
Copy link
Contributor Author

Ciaro commented May 1, 2012

Same for me, I just tought you'd like to reduce the number of open issue (since you basically can mark this one as duplicate of) ;)

@mariuswilms
Copy link
Member

I definitely like to reduce the number of tickets :) - It's true another possibility would be to see this as a dupe. However it's best to leave it like it is now and clarify our view upon the conceptual relationship between PR and "normal" tickets later.

@Ciaro
Copy link
Contributor Author

Ciaro commented May 1, 2012

Sure. Keep up the good work!

@nateabele
Copy link
Member

Yup, sorry, my brain got sidetracked with the introduction of G11n into the conversation. :-) There are still other (but possibly closely related) issues here.

@nateabele
Copy link
Member

Okay, closing this because $this->form->create(null) works fine from any li3_docs view, with or without g11n.php. Also, #388 has been resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants