Skip to content

Commit

Permalink
Fixing issue where defining a pattern for :action would create an opt…
Browse files Browse the repository at this point in the history
…ional route parameter that wouldn't respect its pattern.

Added the default action value only when a pattern isn't defined.
Added tests to CakeRoute and Router to cover this case.
Fixes #1197
  • Loading branch information
markstory committed Oct 15, 2010
1 parent 025090f commit 67874bd
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 1 deletion.
5 changes: 4 additions & 1 deletion cake/libs/router.php
Expand Up @@ -269,7 +269,10 @@ function connect($route, $defaults = array(), $options = array()) {
$self->__prefixes[] = $defaults['prefix'];
$self->__prefixes = array_keys(array_flip($self->__prefixes));
}
$defaults += array('action' => 'index', 'plugin' => null);
$defaults += array('plugin' => null);
if (empty($options['action'])) {
$defaults += array('action' => 'index');
}
$routeClass = 'CakeRoute';
if (isset($options['routeClass'])) {
$routeClass = $options['routeClass'];
Expand Down
64 changes: 64 additions & 0 deletions cake/tests/cases/libs/router.test.php
Expand Up @@ -1694,6 +1694,45 @@ function testParsingWithTrailingPeriodAndParseExtensions() {
$this->assertEqual($result['pass'][0], 'something. . .', 'Period was chopped off %s');
}

/**
* test that patterns work for :action
*
* @return void
*/
function testParsingWithPatternOnAction() {
Router::reload();
Router::connect(
'/blog/:action/*',
array('controller' => 'blog_posts'),
array('action' => 'other|actions')
);
$result = Router::parse('/blog/other');
$expected = array(
'plugin' => null,
'controller' => 'blog_posts',
'action' => 'other',
'pass' => array(),
'named' => array()
);
$this->assertEqual($expected, $result);

$result = Router::parse('/blog/foobar');
$expected = array(
'plugin' => null,
'controller' => 'blog',
'action' => 'foobar',
'pass' => array(),
'named' => array()
);
$this->assertEqual($expected, $result);

$result = Router::url(array('controller' => 'blog_posts', 'action' => 'foo'));
$this->assertEqual('/blog_posts/foo', $result);

$result = Router::url(array('controller' => 'blog_posts', 'action' => 'actions'));
$this->assertEqual('/blog/actions', $result);
}

/**
* testParsingWithPrefixes method
*
Expand Down Expand Up @@ -2489,6 +2528,31 @@ function testMatchWithPatterns() {
$this->assertFalse($result);
}

/**
* test that patterns work for :action
*
* @return void
*/
function testPatternOnAction() {
$route =& new CakeRoute(
'/blog/:action/*',
array('controller' => 'blog_posts'),
array('action' => 'other|actions')
);
$result = $route->match(array('controller' => 'blog_posts', 'action' => 'foo'));
$this->assertFalse($result);

$result = $route->match(array('controller' => 'blog_posts', 'action' => 'actions'));
$this->assertTrue($result);

$result = $route->parse('/blog/other');
$expected = array('controller' => 'blog_posts', 'action' => 'other', 'pass' => array(), 'named' => array());
$this->assertEqual($expected, $result);

$result = $route->parse('/blog/foobar');
$this->assertFalse($result);
}

/**
* test persistParams ability to persist parameters from $params and remove params.
*
Expand Down

0 comments on commit 67874bd

Please sign in to comment.