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

[11.x] Add support for enums in whereIn route constraints #51121

Merged

Conversation

osbre
Copy link
Contributor

@osbre osbre commented Apr 18, 2024

Currently, passing an array of enum cases results in an exception:

Route::get('/posts/{type}')->whereIn('type', PostType::cases());

// Error: Object of class Illuminate\Tests\Routing\CategoryBackedEnum could not be converted to string

This change fixes it, allowing us to pass array of enums into whereIn.

Probably interchangeable with #51104

@taylorotwell
Copy link
Member

taylorotwell commented Apr 18, 2024

Fine with merging this, but note you could just type hint the Enum in your route parameter and you wouldn't need this whereIn call at all. 😅

@vanodevium
Copy link

@taylorotwell

It's undocumented feature

@taylorotwell taylorotwell merged commit 9c1b34c into laravel:11.x Apr 18, 2024
27 of 28 checks passed
@taylorotwell
Copy link
Member

@vanodevium https://laravel.com/docs/11.x/routing#implicit-enum-binding ?

@osbre
Copy link
Contributor Author

osbre commented Apr 18, 2024

@taylorotwell Unfortunately, it's not the same in certain cases. Perhaps later I can send a test update to further demonstrate why.

@vanodevium
Copy link

@taylorotwell

I'm sorry that I'm just now writing my comment.

I will try to explain my vision of the problem to you as simply as possible.

Let's imagine that we have the following routing configuration:

Category enum is taken from the documentation, values: [fruits, people]

Route::get('/{category}', function (Category $category) {
    dd("This is case without unpacking enum values into RegEx");
});

Route::get('/news', function () {
    dd("This is NEWS page");
});

According to the documentation, for this configuration I will never get to the news page, because the request will always fall into the first route, but enum does not have a news value, so it will be raise 404. Everything is as written in the documentation, I completely understand.

Of course, I can push the news to the top and everything will work as it should.

But the question is: why such behavior?
It's simple: because enum is not unpacked into a regular expression at the matching step, so anything gets into the resolver and shows a 404.

Ok, how to fix it?. Very easy:

Route::get('/{category}', function (Category $category) {
    dd("This is case with unpacking enum values into RegEx");
})->whereIn('category', Category::cases());

Route::get('/news', function () {
    dd("This is NEWS page");
});

In this version, everything that is not suitable for the enum will not get into the first route.
And I'll be happy to see news page as I want.


So maybe ->whereEnum('name', Enum::class) helper with automatic unpacking into regex is no so bad idea?

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

Successfully merging this pull request may close these issues.

3 participants