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

Exclude method "middleware" from discovered routes #30

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

LWlook
Copy link

@LWlook LWlook commented Jul 14, 2024

Description

In Laravel 11, an interface has been added that allows middleware to be defined via a method. This package, which automatically detects routes in controllers, should exclude this new method since it returns all middleware classes.

Changes

  • Updated the route detection logic to exclude the new middleware method introduced in Laravel 11.

Reason

The new method in Laravel 11's controllers that returns all middleware classes should not be included in the automatic route detection, as it does not define actual routes.

Additional Notes

Please review the changes and provide any feedback or suggestions for improvement.

Thank you!

@freekmurze
Copy link
Member

Thanks! Could you add a test for this to prove that it works correctly?

@LWlook
Copy link
Author

LWlook commented Jul 15, 2024

Hi, yes I can add some tests for this feature.

But I cannot fix the fail check from PHPStan, is that okey?

@freekmurze
Copy link
Member

Only adding a test is ok!

@spatie-bot
Copy link

Dear contributor,

because this pull request seems to be inactive for quite some time now, I've automatically closed it. If you feel this pull request deserves some attention from my human colleagues feel free to reopen it.

@spatie-bot spatie-bot closed this Nov 18, 2024
@LWlook
Copy link
Author

LWlook commented Dec 18, 2024

Sorry it took me a while, but I added test cases. @freekmurze

Could you open the pull request again, or should I make a new?

@freekmurze freekmurze reopened this Dec 23, 2024
@freekmurze
Copy link
Member

Reopened this one

@LWlook
Copy link
Author

LWlook commented Dec 23, 2024

I have added the test cases, should now be able to be merged.

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