ingeniasoftware / luthier-ci

Improved routing, middleware support, authentication tools and more for CodeIgniter 3 framework
https://luthier.ingenia.me/ci/en/
MIT License
150 stars 38 forks source link

It breaks other route when some method doesn't exist #11

Closed primapwd closed 6 years ago

primapwd commented 6 years ago

Hi, I have these routes

Route::get('/', 'app@index'); // method 'index()' exists in 'App' controller
Route::group('shop', function() {
   Route::get('/', 'shop@index'); // method 'index()' exists in 'Shop' controller
   Route::group('products', function() {
       Route::get('/', 'shop@products'); // method 'products()' doesn't exist on 'Shop' controller
   });
});

I can visit app.dev/shop/ properly. But whenever I visit app.dev/, it throws 404. I found that products() method doesn't exist on Shop controller and it breaks app.dev/ route. So I create products() on Shop controller.

And now I have these new routes:

Route::get('/', 'app@index'); // method 'index()' exists in 'App' controller
Route::group('shop', function() {
   Route::get('/', 'shop@index'); // method 'index()' exists in 'Shop' controller
   Route::group('products', function() {
       Route::get('/', 'shop@products'); // it exists now
       Route::get('/data', 'shop@data'); // this method doesn't exist
   });
});

Visit app.dev/ still works but visiting app.dev/shop/products/data not. It seems that your library only breaks when some of the same '/' route on different group doesn't have actual method.

andersonsalas commented 6 years ago

Hi,

It was a bug in the default_controller detection. The library was assuming that the / URL of your Route::get('/', 'shop@index'); route is the root path, without considering the group prefix.

Thanks for report it.

EDIT: version 0.2.1 released.

primapwd commented 6 years ago

Wow, thanks. And can I have route parameter inside group parameter like this?

Route::group('shop', function() {
   Route::get('{slug}', 'shop@index'); // doesn't work
   Route::get('/{slug}', 'shop@index'); // doesn't work too
});

When I tried, it throws exception

An uncaught Exception was encountered
Type: Luthier\Exception\RouteNotFoundException

Message: (null)

Filename: E:\xampp\htdocs\app\vendor\luthier\luthier\src\Route.php

Line Number: 649

Backtrace:

File: E:\xampp\htdocs\app\vendor\luthier\luthier\src\Functions.php
Line: 16
Function: getByName

File: E:\xampp\htdocs\app\application\controllers\Shop.php
Line: 43
Function: route

File: E:\xampp\htdocs\app\index.php
Line: 315
Function: require_once
andersonsalas commented 6 years ago

Yes, you can.

But that seems an error with the route() function,can you provide the code that is triggering that error?

primapwd commented 6 years ago

Ah, my bad. Yes, you're right. It has nothing to do with route parameter. This is the code that triggering error.

$data['url'] = route('api.shop.products.list', ['name' => $name]);

The route() function actually calling API Routes defined in api.php

Route::group('shop', function() {
    Route::get('{name}/products/', 'shop@productList')->name('api.shop.products.list');
});
andersonsalas commented 6 years ago

I'm glad of the problem got solved. +1

Closing.