Skip to content

[Refactor:System] PHP: switch route loader#12935

Merged
bmcutler merged 3 commits into
mainfrom
bandaid-fix-php-lint
Jun 20, 2026
Merged

[Refactor:System] PHP: switch route loader#12935
bmcutler merged 3 commits into
mainfrom
bandaid-fix-php-lint

Conversation

@roye2

@roye2 roye2 commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

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:

image

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.

@codecov

codecov Bot commented Jun 19, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 21.50%. Comparing base (7073f7c) to head (a2d3f31).

Additional details and impacted files

Impacted file tree graph

@@            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           
Flag Coverage Δ
autograder 21.32% <ø> (ø)
js 2.01% <ø> (ø)
migrator 100.00% <ø> (ø)
php 20.54% <100.00%> (ø)
python_submitty_utils 80.08% <ø> (ø)
submitty_daemon_jobs 91.13% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@roye2 roye2 changed the title [Refactor:System] Switch php from AnnotatedRouteLoader to AttributeRouteLoader (fixes php lint) [Refactor:System] PHP: AnnotatedRouteLoader to AttributeRouteLoader Jun 19, 2026
@roye2 roye2 changed the title [Refactor:System] PHP: AnnotatedRouteLoader to AttributeRouteLoader [Refactor:System] PHP: switch php route loader Jun 19, 2026
@roye2 roye2 changed the title [Refactor:System] PHP: switch php route loader [Refactor:System] PHP: switch route loader Jun 19, 2026
Comment thread site/app/libraries/routers/AttributeRouteLoader.php Outdated
Comment thread site/app/libraries/routers/AttributeRouteLoader.php Outdated
@github-project-automation github-project-automation Bot moved this from Seeking Reviewer to Work in Progress in Submitty Development Jun 19, 2026

@cjreed121 cjreed121 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Functionally this looks good but have not tested locally. I see no reason it wouldn't work though.

@github-project-automation github-project-automation Bot moved this from Work in Progress to Awaiting Maintainer Review in Submitty Development Jun 19, 2026
@roye2

roye2 commented Jun 20, 2026

Copy link
Copy Markdown
Contributor Author

@cjreed121 Thanks for the review! I updated the code to match your requested changes.

@bmcutler
bmcutler merged commit 653ad80 into main Jun 20, 2026
48 of 52 checks passed
@bmcutler
bmcutler deleted the bandaid-fix-php-lint branch June 20, 2026 03:22
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants