Comments and changes to this ticket

• 

  Mark Story April 2nd, 2010 @ 09:54 AM

  • → Tag changed from “incorrect url, paginatorhelper” to “paginatorhelper, plugins, rfc, router, routing”
  • → Milestone set to “1.3.0”

  This is actually a feature, and is part of the 'plugin shortcut' routes. That when a plugin contains a controller with the same name the route can be just the plugin name. So if you have a users plugin with a users controller /users/users/index can be expressed as /users/index and still work. This isn't something I'm personally overly fond of, and perhaps we can consider removing it in the future, as it is a bit unexpected.

• 

  thatcode April 2nd, 2010 @ 09:57 AM

  I think you've got my bug the wrong way round. The bug is the extra /index in the url.

  I understand the feature, and personally I like it.

  Paginator->sort however does not understand this feature.

  I expect it to produce /plugin_name/action/paginator:var, but it actually produces /plugin_name/action/index/paginator:var as it incorrectly views the action name as a controller name, and thus adds index as the action.

• 

  Mark Story April 2nd, 2010 @ 10:09 AM

  • → Tag changed from “paginatorhelper, plugins, rfc, router, routing” to “defect, paginatorhelper, plugins, router”
  • → Assigned user set to “Mark Story”

  Oh my mistake. Yeah the extra index should not be there.

• 

  Mark Story April 2nd, 2010 @ 12:44 PM

  • → State changed from “new” to “hold”

  I was not able to reproduce your issue. I added tests in [ffb5c365787b804fe66d949b4d63a308af2e92ba] and [d058234c1c466f6d62c5f962e0af1c68118a1053]. I also used bake to generate a plugin, and didn't get any additional parameter leakage there either. Some more information on how to reproduce your issue would be helpful.

• 

  thatcode April 2nd, 2010 @ 01:09 PM

  Just updated to RC3, still happens.

  Start from default app.

  CREATE TABLE `tests` (
    `id` int(11) NOT NULL AUTO_INCREMENT,
    PRIMARY KEY (`id`)
  )
  

  app/plugins/tests/tests_app_controller.php
  

  <?php
  class TestsAppController extends AppController {
  }
  

  app/plugins/tests/tests_app_model.php
  

  <?php
  class TestsAppModel extends AppModel {
  }
  

  app/plugins/tests/controllers/tests_controller.php
  

  <?php
  class TestsController extends TestsAppController {
  function some() {
  $this->set('tests', $this->paginate());
  }
  }
  

  app/plugins/tests/views/tests/some.ctp
  

  <?php echo $this->Paginator->sort('id');
  

  HTML produced by this call:
  

  <a href="/tests/some/index/page:0/sort:id/direction:asc">Id</a>
  

  HTML expected:
  

  <a href="/tests/some/page:0/sort:id/direction:asc">Id</a>
  

  I'll tar the plugin and get it up so you can download it.

• 

  thatcode April 2nd, 2010 @ 01:11 PM

  http://up.thatcode.com/tests.tar.gz

• 

  Mark Story April 4th, 2010 @ 03:27 PM

  Thanks for the additional information, I was able to reproduce it, but investigating it further has turned up the stack of turtles that the plugin shortcuts 'provide'. The errant index parameter is caused by parameter mangling in the Dispatcher. That in of itself is fixable, but there are several other edge cases that make me inclined to just remove the plugin shortcut routes. They require a pile of hacks, and don't always work correctly. My inclination is to just remove the feature, and inform people that wish to take advantage of short cut routes to create their own custom routes.

• 

  Mark Story April 7th, 2010 @ 11:17 PM

  • → State changed from “hold” to “resolved”

  So the consensus was to remove the offending plugin shortcut routes. They were troublesome and unpredictable, and often magically broke or shifted around. This ticket was the straw that broke the camels back :). For additional reference consider the following:

  app/controllers/foo_controller.php with a bar() method
  app/plugins/foo/foo_controller.php with a bar() method
  

  Going to the url /foo/bar will call the app/controller class. However, when making a link with array('plugin' => 'foo', 'controller' => 'foo', 'action' => 'bar') you will get the string '/foo/bar'. Making the plugin controller inaccessible. To further compound the issue, there is this:

  app/plugins/foo/foo_controller.php with a bar() method
  app/plugins/foo/bar_controller.php with an index() method
  

  Now going to /foo/bar is totally ambiguous as it could go to two places, and one will always be inaccessible. Combine this with the previous example, and one route could go to 3 different places. This is not really something that is maintainable or sensical in the long term. So what has been done is plugin shortcut routes have been removed and a new /:plugin route had been created. This route will map to array('plugin' => $plugin, 'controller' => $plugin, 'action' => 'index'). Any other parameters will expand the url into a longer plugin url.

  Of course you can use routes to maintain the old urls if these changes adversely affect your application.

• 

  thatcode April 9th, 2010 @ 03:28 AM

  Hi.

  Despite liking this feature I do understand the reason to remove it. However, for a 'drop in and work' plugin there's now no way to define a 'home' controller as you don't have access to the main routes.php file from the plugin. I know there's a few tickets about having a config file for plugins, and would like to suggest a similar effect for routes - given less priority than the main routes.php thus allowing it to be over-ridden, but still there.

  Obviously all plugins could just say 'add this to your routes file' but I think this is common enough to be worthwhile Cake magic.

• 

  Mark Story April 9th, 2010 @ 09:43 PM

  The reason to remove it, is that it was a source of ambiguity and broken functionality. The code that provided this feature was a hack. Basically it tried to load controllers with different permutations of the parameters until it ran out of options. This creates additional overhead, for an already defective feature.

  There still is a 'home' route for a plugin /:plugin still maps to an index() method on a controller with the same name as the plugin.