Skip to content

Commit

Permalink
Rename controller option to path.
Browse files Browse the repository at this point in the history
After merging the changes from #10488 I started writing the
documentation and noticed that most of the other RouteBuilder methods
use 'path' and not 'controller' as an option. Given that changing the
path was the original intent, I thought that might be a more consistent
option name. Furthermore, the previous changes would result in awkward
reverse routing scenarios where the routing parameter would not match
the controller/model name, and instead match the resource name. e.g.
`resources('BlogPosts', ['controller' => 'articles'])` would use
`blog_post_id` as the routing parameter instead of `article_id` which
better matches the conventions used elsewhere in the framework.
  • Loading branch information
markstory committed Apr 11, 2017
1 parent 1151f5d commit abe68a6
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 12 deletions.
20 changes: 12 additions & 8 deletions src/Routing/RouteBuilder.php
Expand Up @@ -302,7 +302,9 @@ public function namePrefix($value = null)
* make sure that your mapped methods are also in the 'only' list.
* - 'prefix' - Define a routing prefix for the resource controller. If the current scope
* defines a prefix, this prefix will be appended to it.
* - 'connectOptions' – Custom options for connecting the routes.
* - 'connectOptions' - Custom options for connecting the routes.
* - 'path' - Change the path so it doesn't match the resource name. E.g ArticlesController
* is available at `/posts`
*
* @param string $name A controller name to connect resource routes for.
* @param array|callable $options Options to use when generating REST routes, or a callback.
Expand All @@ -324,7 +326,7 @@ public function resources($name, $options = [], $callback = null)
'actions' => [],
'map' => [],
'prefix' => null,
'controller' => $name
'path' => null,
];

foreach ($options['map'] as $k => $mapped) {
Expand All @@ -337,8 +339,10 @@ public function resources($name, $options = [], $callback = null)
}

$connectOptions = $options['connectOptions'];
$method = $options['inflect'];
$urlName = Inflector::$method($name);
if (empty($options['path'])) {
$method = $options['inflect'];
$options['path'] = Inflector::$method($name);
}
$resourceMap = array_merge(static::$_resourceMap, $options['map']);

$only = (array)$options['only'];
Expand All @@ -364,9 +368,9 @@ public function resources($name, $options = [], $callback = null)
$action = $options['actions'][$method];
}

$url = '/' . implode('/', array_filter([$urlName, $params['path']]));
$url = '/' . implode('/', array_filter([$options['path'], $params['path']]));
$params = [
'controller' => $options['controller'],
'controller' => $name,
'action' => $action,
'_method' => $params['method'],
];
Expand All @@ -382,8 +386,8 @@ public function resources($name, $options = [], $callback = null)
}

if (is_callable($callback)) {
$idName = Inflector::singularize(str_replace('-', '_', $urlName)) . '_id';
$path = '/' . $urlName . '/:' . $idName;
$idName = Inflector::singularize(Inflector::underscore($name)) . '_id';
$path = '/' . $options['path'] . '/:' . $idName;
$this->scope($path, [], $callback);
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/Routing/Router.php
Expand Up @@ -280,6 +280,8 @@ public static function redirect($route, $url, $options = [])
* - 'actions' - Override the method names used for connecting actions.
* - 'map' - Additional resource routes that should be connected. If you define 'only' and 'map',
* make sure that your mapped methods are also in the 'only' list.
* - 'path' - Change the path so it doesn't match the resource name. E.g ArticlesController
* is available at `/posts`
*
* @param string|array $controller A controller name or array of controller names (i.e. "Posts" or "ListItems")
* @param array $options Options to use when generating REST routes
Expand Down
17 changes: 13 additions & 4 deletions tests/TestCase/Routing/RouteBuilderTest.php
Expand Up @@ -409,16 +409,25 @@ public function testResources()
}

/**
* Test connecting resources with a controller
* Test connecting resources with a path
*
* @return void
*/
public function testResourcesController()
public function testResourcesPathOption()
{
$routes = new RouteBuilder($this->collection, '/api');
$routes->resources('Articles', ['controller' => 'Posts']);
$routes->resources('Articles', ['path' => 'posts'], function ($routes) {
$routes->resources('Comments');
});
$all = $this->collection->routes();
$this->assertEquals('Posts', $all[0]->defaults['controller']);
$this->assertEquals('Articles', $all[0]->defaults['controller']);
$this->assertEquals('/api/posts', $all[0]->template);
$this->assertEquals('/api/posts/:id', $all[2]->template);
$this->assertEquals(
'/api/posts/:article_id/comments',
$all[6]->template,
'parameter name should reflect resource name'
);
}

/**
Expand Down

0 comments on commit abe68a6

Please sign in to comment.