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

[L5.5.13]: Resource route ordering #21384

Closed
joshbruce opened this issue Sep 25, 2017 · 6 comments
Closed

[L5.5.13]: Resource route ordering #21384

joshbruce opened this issue Sep 25, 2017 · 6 comments

Comments

@joshbruce
Copy link

  • Laravel Version: 5.5.13
  • PHP Version: 7.1
  • Database Driver & Version: MySQL

Description:

See also: #21258

Hmmmm...this seems to be an order of operations problem, which I might have noticed if route:list displayed them in the order they are interpreted. There might also be a better way to do what I'm trying to do.

  1. I have a resource.
  2. The index and show are publicly visible.
  3. Everything else is behind auth.

Steps To Reproduce:

// Handle the public
Route::resource('your-resource', YourResourceController::class, [
        'only' => [
            'index',
            'show'
        ]
    ])
    ->middleware(['web']);

// Put everything else behind a wall
Route::resource('your-resource, YourResourceController::class, [
        'except' => [
            'index',
            'show'
        ]
    ])
    ->middleware(['web', 'auth']);

Create (/create) ends up with show (/{your-resource}). route:list shows the following.

your-resource
your-resource
your-resource/create
your-resource/{your-resource}

Thereby hiding the "defect", which is that the interpretation order is really:

your-resource
your-resource
your-resource/{your-resource}
your-resource/create

Due to the ordering of the route blocks. Switching them fixes the problem:

// Put everything else behind a wall
Route::resource('your-resource, YourResourceController::class, [
        'except' => [
            'index',
            'show'
        ]
    ])
    ->middleware(['web', 'auth']);

// Handle the public
Route::resource('your-resource', YourResourceController::class, [
        'only' => [
            'index',
            'show'
        ]
    ])
    ->middleware(['web']);

Maybe route:list can be made to list the routes in literal interpretation order?

Anyhow, just my UX brain have a hiccup.

@devcircus
Copy link
Contributor

Yeah it seems like the list command just needs to be fixed to output the order correctly. If indeed the command should be responsible for that. It definitely makes sense for the "create" to be defined first.

@joshbruce
Copy link
Author

No doubt. That is the question. If there's a more logical way to get feedback on the order the routes are interpreted...then it should definitely be there. From a UX perspective, route:list was the first place I went when the route was doing what was not expected. My literal thought was: "I bet the parameterized route is being hit first; therefore, it's going to show() instead of create(). I'll run route:list to verify."

When it wasn't in that order, I didn't knwo where else to look (you know what I'm saying?); so, started writing this and said, "I wonder..." then flipped them, which changed the nature of this ticket. :)

@Dylan-DPC-zz
Copy link

I wouldn't consider this is a bug. More of a proposal that can be made in laravel/internals

@themsaid
Copy link
Member

You can use php artisan route:list --sort to get a sorted list.

@joshbruce
Copy link
Author

@Dylan-DPC: Yeah. Wasn't considering a bug either. Is there a different format to use or something more explicit I can do in the future to differentiate? First time posting.

@themsaid: Thanks! Didn't know that was a thing already.

@Dylan-DPC-zz
Copy link

@joshbruce say the framework states that something works in an X way but you get the result in a Y way you can consider it a bug. Also if you are not sure you can ask in the forums or pop up on the slack channel (can DM me if you wish, no problems)

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

4 participants