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

Route naming and Groups #612

Closed
daylightstudio opened this issue Jul 16, 2017 · 3 comments
Closed

Route naming and Groups #612

daylightstudio opened this issue Jul 16, 2017 · 3 comments

Comments

@daylightstudio
Copy link
Contributor

daylightstudio commented Jul 16, 2017

I'm experiencing an issue with routing where the options in the route group (e.g. 'namespace') are being overwritten. For example, the following I would expect the URI admin/user/create to work:

$routes->group('admin', ['namespace' => 'Admin\Controllers'], function($routes)
{
	$routes->get('user/create', 'UserController::create', ['as' => 'user_create']);
}

However, because I added the ['as' => 'user_create'], it overwrites the default options and loses the 'namespace' option. I'd like to propose replacing the lines around 1120 in the RouteCollection::create method (https://github.com/bcit-ci/CodeIgniter4/blob/develop/system/Router/RouteCollection.php#L1120):

if (is_null($options))
{
	$options = $this->currentOptions;
}

with something like the following so that the current group options get merged with the passed options:

$options = array_merge((array)$this->currentOptions, (array)$options);
@lonnieezell
Copy link
Member

That's a good call. Care to submit a PR for that? :)

@daylightstudio
Copy link
Contributor Author

Yep... wanted to run by the solution first to make sure I wasn't missing something or if there was a preferred way of implementation. I'll send it in shortly.

@lonnieezell
Copy link
Member

Appreciate it. And that sounds good. That's what it was intended to do, but I obviously took a misstep :)

daylightstudio added a commit to daylightstudio/CodeIgniter4 that referenced this issue Jul 17, 2017
This fixes an issue with route groups and route options not merging correctly.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants