[Refactor:System] PHP: switch route loader#12935
Merged
Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12935 +/- ##
=========================================
Coverage 21.50% 21.50%
Complexity 9896 9896
=========================================
Files 268 268
Lines 36968 36968
Branches 495 495
=========================================
Hits 7951 7951
Misses 28526 28526
Partials 491 491
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
cjreed121
requested changes
Jun 19, 2026
cjreed121
approved these changes
Jun 19, 2026
cjreed121
left a comment
Member
There was a problem hiding this comment.
Functionally this looks good but have not tested locally. I see no reason it wouldn't work though.
Contributor
Author
|
@cjreed121 Thanks for the review! I updated the code to match your requested changes. |
bmcutler
pushed a commit
to Submitty/submitty.github.io
that referenced
this pull request
Jul 6, 2026
### Why is this Change Important & Necessary? The documentation concerning Routes was outdated. The old syntax of `@Route(...)` is deprecated and in fact will no longer work with the recent update to the Route object we include from Symfony in [PR #12935](Submitty/Submitty#12935). ### New Additions Instances of `@Route(...)` have been replaced with `#[Route(...)]` to match what will be expected from developers when writing PHP code. ### Screenshots #### Before 1. Controller page: <img width="1620" height="951" alt="image" src="https://github.com/user-attachments/assets/56bd84aa-f0b3-4d36-830d-c4eb84db370f" /> 2. Excerpt from Router page: <img width="1650" height="951" alt="image" src="https://github.com/user-attachments/assets/3e0d70db-0517-4689-b3ce-fa35475c18db" /> 3. Excerpt from Routes and Responses page: <img width="1650" height="951" alt="image" src="https://github.com/user-attachments/assets/2b041778-698f-41f2-9bff-2cdad0b06277" /> #### After 1. Updated Controller page: <img width="1620" height="951" alt="image" src="https://github.com/user-attachments/assets/ad1a7dd4-530d-4cbf-b62c-98f81536a37a" /> 2. Updated Router page: <img width="1650" height="951" alt="image" src="https://github.com/user-attachments/assets/6f1cf287-8bc8-4818-b9e3-eb25bd490e3c" /> 3. Updated Routes and Responses page: <img width="1650" height="951" alt="image" src="https://github.com/user-attachments/assets/a7fe9db4-3feb-4ed6-819f-0099068da5b5" /> ### Other information Unfortunately, the markdown renderer thinks the #[Route(...)] syntax is a comment due to the leading #. If at some point GitHub pages updates their markdown renderer to allow for PHP 8 syntax highlighting then the Routes will appear correctly as uncommented. Example of how it is now: <img width="437" height="319" alt="image" src="https://github.com/user-attachments/assets/5daab045-878e-4ef2-a165-faca83feeea4" /> Here is how it actually looks in the code: <img width="503" height="321" alt="image" src="https://github.com/user-attachments/assets/87e875d8-c2cd-4ee8-82fe-5ad8107531a5" /> Notice how the routes are not italicized. This is not really a big deal but it kind of annoys me that the routes look like they are comments. However, I think that is better than rendering these code blocks as plain text or a different language than php (both of which would make the route appear 'correctly'). Also, the example given of UserController on the Controller / View pages is code that no longer exists in the submitty codebase. It is still a good example of how to write our php code but at some point we should probably update the example to use real code.
This file contains hidden or 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
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.
Why is this Change Important & Necessary?
When we fixed the php errors in CI, we updated symfony/routing to 6.4.13. This updated php lint to say that our AnnotatedRouteLoader function was deprecated, and to switch to using AttributeRouteLoader. For example:
What is the New Behavior?
A new file called AttributeRouteLoader.php has been added to represent the class. (Github is showing that as the edit / retitle of the old file.)
Edits to WebRouter.php have been made to support the switch.
What steps should a reviewer take to reproduce or test the bug or new feature?
Test general functionality of the site. To be honest, I am not sure about how significant this change is or whether it will break anything, so looking for an experience reviewer.
Automated Testing & Documentation
This PR is not documented by testing.
Other information
Is this a breaking change? I am not sure.
This is not a bandaid fix, but I thought it would be when I titled the branch :). It's just a regular fix.