-
Notifications
You must be signed in to change notification settings - Fork 1
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
III-4896 Add PSR router #1102
Merged
Merged
III-4896 Add PSR router #1102
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It still works for all routes, also those matched by the new router. And if we move it now it won't work anymore for routes matched by the Silex router. So we need to move all the routes to the new router first
…Provider Better to stay consistent, and we will refactor the error handling in a later ticket
Otherwise routes that are not registered in the new router yet will not work with CORS anymore
LucWollants
approved these changes
Aug 23, 2022
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the test for path rewrites!
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Added
Changed
GetDetailRequestHandler
for events and places on the new router as a proof of conceptLegacyPathRewriter
classLegacyRoutesServiceProvider
toCatchAllRouteServiceProvider
as it is also used to dispatch requests to the new PSR router (until we can by-pass the Silex router completely once all routes, middlewares etc are moved)Ticket: https://jira.uitdatabank.be/browse/III-4896
All acceptance tests are green, and the CORS requests still work in the UI, also for event details. I also tested the OPTIONS request via curl like this (make sure to change the event URL):
Notes:
handle
method as a second array value. I opened an issue for this in the router's repo: Feature request: Treat Route$handler
that implements RequestHandlerInterface as callable thephpleague/route#324