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

fix: spark routes shows 404 error when using regex #6279

Merged
merged 2 commits into from
Jul 21, 2022

Conversation

kenjis
Copy link
Member

@kenjis kenjis commented Jul 19, 2022

Description
Fixes #6276

How to Test

$routes->get('detail/([a-z0-9]{40})', 'User::detail/$1');

Before:

ERROR: 404
Can't find a route for 'get: detail/([a-z0-9]{40})'.

After:

+--------+-----------------------+------------------------------------------+----------------+---------------+
| Method | Route                 | Handler                                  | Before Filters | After Filters |
+--------+-----------------------+------------------------------------------+----------------+---------------+
| GET    | /                     | \App\Controllers\Home::index             |                | toolbar       |
| GET    | detail/([a-z0-9]{40}) | \App\Controllers\User::detail/$1         | <unknown>      | <unknown>     |
| CLI    | ci(.*)                | \CodeIgniter\CLI\CommandRunner::index/$1 |                |               |
+--------+-----------------------+------------------------------------------+----------------+---------------+

Checklist:

  • Securely signed commits
  • Component(s) with PHPDoc blocks, only if necessary or adds value
  • Unit testing, with >80% coverage
  • User guide updated
  • Conforms to style guide

@kenjis kenjis added the bug Verified issues on the current code behavior or pull requests that will fix them label Jul 19, 2022
@kenjis kenjis changed the title fix: spark routes show 404 error when using regex fix: spark routes shows 404 error when using regex Jul 19, 2022
@iRedds
Copy link
Collaborator

iRedds commented Jul 19, 2022

I think it would make more sense to catch the exception in FilterFilder::find()

@kenjis kenjis force-pushed the fix-spark-routes-404-error branch from a01ff93 to b4f4a2c Compare July 19, 2022 10:11
@kenjis kenjis force-pushed the fix-spark-routes-404-error branch from b4f4a2c to d89bca7 Compare July 19, 2022 10:11
@kenjis
Copy link
Member Author

kenjis commented Jul 19, 2022

@iRedds Thanks. I was confusing FilterFinder with Filters.

Copy link
Member

@MGatner MGatner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure how this fixes the issue but if it does then I'm good with it.

@kenjis
Copy link
Member Author

kenjis commented Jul 20, 2022

This error is because of the current limitation of the functionality.
We need a real route (without regex) to find its filters, but we can't get a real route from the regex route.
So it tries to find filters for the route detail/([a-z0-9]{40}), but there is no such a route.

@kenjis kenjis merged commit 3800c5d into codeigniter4:develop Jul 21, 2022
@kenjis kenjis deleted the fix-spark-routes-404-error branch July 21, 2022 02:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Verified issues on the current code behavior or pull requests that will fix them
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: spark routes show error when using custom placeholder (regex)
4 participants