refactors controllers to use the new method signature and request object #19
Conversation
WalkthroughUpdated the composer dependency for neuron-php/mvc to 0.9.* and refactored controller public methods across the CMS to accept a single Changes
Sequence Diagram(s)sequenceDiagram
participant Router
participant Request
participant Controller
note over Router,Controller: OLD (pre-change)
Router->>Controller: invoke(array $params, ?Request $request)
Controller->>Controller: read $params['id'] / use globals ($_POST)
note over Router,Controller: NEW (post-change)
Router->>Request: create & setRouteParameters()
Router->>Controller: invoke(Request $request)
Controller->>Request: getRouteParameter('id')
Request-->>Controller: id value
Controller->>Request: post('field')
Request-->>Controller: field value
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
src/Cms/Controllers/Admin/Posts.php (1)
160-200: Complete the refactoring by using$request->post()instead of$_POST.The
store()method still uses direct$_POSTaccess on lines 172-179, which is inconsistent with the refactoring pattern used in theupdate()method (lines 283-290) and other controllers across the codebase.Apply this diff:
try { // Get form data - $title = $_POST['title'] ?? ''; - $slug = $_POST['slug'] ?? ''; - $content = $_POST['content'] ?? ''; - $excerpt = $_POST['excerpt'] ?? ''; - $featuredImage = $_POST['featured_image'] ?? ''; - $status = $_POST['status'] ?? Post::STATUS_DRAFT; - $categoryIds = $_POST['categories'] ?? []; - $tagNames = $_POST['tags'] ?? ''; + $title = $request->post( 'title', '' ); + $slug = $request->post( 'slug', '' ); + $content = $request->post( 'content', '' ); + $excerpt = $request->post( 'excerpt', '' ); + $featuredImage = $request->post( 'featured_image', '' ); + $status = $request->post( 'status', Post::STATUS_DRAFT ); + $categoryIds = $request->post( 'categories', [] ); + $tagNames = $request->post( 'tags', '' );src/Cms/Controllers/Blog.php (1)
116-138: Fix undefined variable$parameters.Line 118 references an undefined variable
$parameters. This should be updated to use$request->getRouteParameter('author')to match the refactoring pattern used in the other methods.Apply this diff:
public function author( Request $request ): string { - $authorName = $parameters['author'] ?? ''; + $authorName = $request->getRouteParameter( 'author', '' );src/Cms/Controllers/Auth/PasswordReset.php (2)
83-126: Fix incorrect redirect route name.Line 100 redirects to
'admin_posts', which appears to be a copy-paste error. For a password reset request failure, it should redirect to the forgot password form or use a header redirect as done on line 124.Apply this diff:
// Validate input if( empty( $email ) || !filter_var( $email, FILTER_VALIDATE_EMAIL ) ) { - $this->redirect( 'admin_posts', [], ['error', 'Please enter a valid email address.'] ); + $this->redirect( 'forgot_password', [], ['error', 'Please enter a valid email address.'] ); }
183-226: Fix incorrect redirect route name.Line 189 redirects to
'admin_posts', which appears to be a copy-paste error. For CSRF token validation failure in password reset, it should redirect to the forgot password form.Apply this diff:
// Validate CSRF token $csrfToken = $request->post( 'csrf_token', '' ); if( !$this->_csrfManager->validate( $csrfToken ) ) { - $this->redirect( 'admin_posts', [], ['error', 'Invalid CSRF token.'] ); + $this->redirect( 'forgot_password', [], ['error', 'Invalid CSRF token.'] ); }src/Cms/Controllers/Auth/Login.php (1)
92-127: Complete the refactoring by using$request->post()instead of$_POST.Line 121 still uses direct
$_POSTaccess for the redirect URL, which is inconsistent with the refactoring pattern. The method already uses$request->post()for credentials on lines 103-105.Apply this diff:
// Successful login - redirect to intended URL or dashboard $defaultRedirect = $this->urlFor( 'admin_dashboard' ) ?? '/admin/dashboard'; - $requestedRedirect = $_POST['redirect_url'] ?? $defaultRedirect; + $requestedRedirect = $request->post( 'redirect_url', $defaultRedirect ); $redirectUrl = $this->isValidRedirectUrl( $requestedRedirect )src/Cms/Controllers/Admin/Users.php (1)
187-220: Complete the refactoring by using$request->post()instead of$_POST.Lines 208-209 still use direct
$_POSTaccess for the 'role' and 'password' fields, which is inconsistent with the refactoring pattern used for username and email on lines 197-198.Apply this diff:
if( $username === '' || $email === '' ) { $this->redirect( 'admin_users_edit', ['id' => $id], ['error', 'Username and email are required'] ); } - $role = $_POST['role'] ?? $user->getRole(); - $password = $_POST['password'] ?? null; + $role = $request->post( 'role', $user->getRole() ); + $password = $request->post( 'password', null );
♻️ Duplicate comments (1)
src/Cms/Controllers/Admin/Categories.php (1)
85-85: Verify if the Request parameter is required by the framework.Similar to the
indexmethod, the$requestparameter is not used in the method body. This might be intentional for framework API consistency.
🧹 Nitpick comments (4)
src/Cms/Controllers/Admin/Profile.php (1)
85-102: LGTM! Request-based input handling implemented correctly.The migration from
$_POSTto$request->post()is properly implemented for all form fields.Minor formatting note: Line 99 has inconsistent spacing with two spaces before the empty string default value.
If you'd like to fix the spacing:
- $timezone = $request->post( 'timezone', '' ); + $timezone = $request->post( 'timezone', '' );src/Cms/Controllers/Member/Profile.php (1)
85-103: LGTM! Request-based input handling implemented correctly.The migration from
$_POSTto$request->post()is properly implemented for all form fields.Minor formatting note: Line 99 has inconsistent spacing with two spaces before the empty string default value.
If you'd like to fix the spacing:
- $email = $request->post( 'email', '' ); + $email = $request->post( 'email', '' );src/Cms/Controllers/Admin/Tags.php (2)
130-165: Minor: Clean up extra blank line in docblock.Line 130 has an unnecessary blank line in the docblock.
Apply this diff:
/** * Show edit tag form - * * @param Request|null $request
207-225: Minor: Clean up extra blank line in docblock.Line 207 has an unnecessary blank line in the docblock.
Apply this diff:
/** * Delete tag - * @param Request $request
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
composer.json(1 hunks)src/Cms/Controllers/Admin/Categories.php(7 hunks)src/Cms/Controllers/Admin/Dashboard.php(2 hunks)src/Cms/Controllers/Admin/Posts.php(11 hunks)src/Cms/Controllers/Admin/Profile.php(4 hunks)src/Cms/Controllers/Admin/Tags.php(7 hunks)src/Cms/Controllers/Admin/Users.php(7 hunks)src/Cms/Controllers/Auth/Login.php(5 hunks)src/Cms/Controllers/Auth/PasswordReset.php(7 hunks)src/Cms/Controllers/Blog.php(10 hunks)src/Cms/Controllers/Content.php(5 hunks)src/Cms/Controllers/Home.php(2 hunks)src/Cms/Controllers/Member/Dashboard.php(2 hunks)src/Cms/Controllers/Member/Profile.php(4 hunks)src/Cms/Controllers/Member/Registration.php(6 hunks)tests/Cms/BlogControllerTest.php(4 hunks)tests/Cms/ContentControllerTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (16)
src/Cms/Controllers/Admin/Posts.php (3)
src/Cms/Controllers/Admin/Categories.php (4)
create(85-106)store(114-129)edit(137-167)update(175-198)src/Cms/Controllers/Admin/Tags.php (4)
create(75-96)store(104-126)edit(135-165)update(174-203)src/Cms/Controllers/Admin/Users.php (4)
create(89-110)store(118-140)edit(149-178)update(187-220)
tests/Cms/ContentControllerTest.php (1)
src/Cms/Controllers/Content.php (1)
markdown(201-214)
src/Cms/Controllers/Member/Profile.php (1)
src/Cms/Controllers/Admin/Profile.php (2)
edit(50-81)update(89-136)
src/Cms/Controllers/Admin/Dashboard.php (7)
src/Cms/Controllers/Admin/Categories.php (1)
index(53-77)src/Cms/Controllers/Admin/Posts.php (1)
index(79-118)src/Cms/Controllers/Admin/Tags.php (1)
index(43-67)src/Cms/Controllers/Admin/Users.php (1)
index(56-81)src/Cms/Controllers/Blog.php (1)
index(45-64)src/Cms/Controllers/Home.php (1)
index(27-48)src/Cms/Controllers/Member/Dashboard.php (1)
index(37-63)
src/Cms/Controllers/Home.php (6)
src/Cms/Controllers/Admin/Categories.php (1)
index(53-77)src/Cms/Controllers/Admin/Dashboard.php (1)
index(38-66)src/Cms/Controllers/Admin/Posts.php (1)
index(79-118)src/Cms/Controllers/Admin/Tags.php (1)
index(43-67)src/Cms/Controllers/Admin/Users.php (1)
index(56-81)src/Cms/Controllers/Member/Dashboard.php (1)
index(37-63)
src/Cms/Controllers/Blog.php (4)
src/Cms/Repositories/DatabasePostRepository.php (3)
DatabasePostRepository(22-534)getPublished(307-310)findBySlug(58-71)src/Cms/Repositories/DatabaseCategoryRepository.php (2)
DatabaseCategoryRepository(19-229)findBySlug(50-58)src/Cms/Repositories/DatabaseTagRepository.php (2)
DatabaseTagRepository(19-204)findBySlug(50-58)src/Cms/Repositories/IPostRepository.php (2)
getPublished(83-83)findBySlug(22-22)
tests/Cms/BlogControllerTest.php (1)
src/Cms/Controllers/Blog.php (4)
index(45-64)show(73-107)tag(148-179)category(188-219)
src/Cms/Controllers/Member/Dashboard.php (2)
src/Cms/Controllers/Admin/Categories.php (1)
index(53-77)src/Cms/Controllers/Admin/Dashboard.php (1)
index(38-66)
src/Cms/Controllers/Admin/Users.php (5)
src/Cms/Controllers/Admin/Categories.php (6)
index(53-77)create(85-106)store(114-129)edit(137-167)update(175-198)destroy(206-219)src/Cms/Controllers/Admin/Posts.php (6)
index(79-118)create(126-152)store(160-200)edit(208-249)update(257-311)destroy(318-350)src/Cms/Controllers/Admin/Tags.php (6)
index(43-67)create(75-96)store(104-126)edit(135-165)update(174-203)destroy(212-225)src/Cms/Controllers/Admin/Profile.php (2)
edit(50-81)update(89-136)src/Cms/Controllers/Content.php (1)
redirect(240-251)
src/Cms/Controllers/Auth/PasswordReset.php (4)
src/Cms/Auth/PasswordResetManager.php (2)
requestReset(75-106)resetPassword(136-169)src/Cms/Controllers/Content.php (2)
redirect(240-251)redirectToUrl(260-270)src/Cms/Auth/SessionManager.php (1)
get(108-112)src/Cms/Auth/CsrfTokenManager.php (1)
validate(49-60)
src/Cms/Controllers/Admin/Tags.php (1)
src/Cms/Controllers/Admin/Categories.php (6)
index(53-77)create(85-106)store(114-129)edit(137-167)update(175-198)destroy(206-219)
src/Cms/Controllers/Auth/Login.php (3)
src/Cms/Auth/AuthManager.php (1)
check(145-160)src/Cms/Controllers/Content.php (1)
redirect(240-251)src/Cms/Auth/CsrfTokenManager.php (1)
validate(49-60)
src/Cms/Controllers/Admin/Categories.php (4)
src/Cms/Controllers/Admin/Dashboard.php (1)
index(38-66)src/Cms/Controllers/Admin/Posts.php (5)
index(79-118)store(160-200)edit(208-249)update(257-311)destroy(318-350)src/Cms/Controllers/Admin/Tags.php (5)
index(43-67)store(104-126)edit(135-165)update(174-203)destroy(212-225)src/Cms/Controllers/Admin/Users.php (5)
index(56-81)store(118-140)edit(149-178)update(187-220)destroy(229-249)
src/Cms/Controllers/Member/Registration.php (4)
src/Cms/Auth/CsrfTokenManager.php (1)
validate(49-60)src/Cms/Controllers/Content.php (1)
redirect(240-251)src/Cms/Auth/SessionManager.php (1)
get(108-112)src/Cms/Auth/EmailVerificationManager.php (1)
resendVerification(173-190)
src/Cms/Controllers/Admin/Profile.php (3)
src/Cms/Controllers/Admin/Posts.php (2)
edit(208-249)update(257-311)src/Cms/Controllers/Admin/Users.php (2)
edit(149-178)update(187-220)src/Cms/Controllers/Member/Profile.php (2)
edit(50-81)update(90-137)
src/Cms/Controllers/Content.php (1)
src/Cms/Auth/SessionManager.php (1)
SessionManager(13-188)
🪛 PHPMD (2.15.0)
src/Cms/Controllers/Admin/Posts.php
79-79: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
126-126: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
160-160: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Member/Profile.php
50-50: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/Dashboard.php
38-38: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Home.php
27-27: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Blog.php
45-45: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
116-116: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
228-228: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Member/Dashboard.php
37-37: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/Users.php
56-56: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
89-89: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Auth/PasswordReset.php
56-56: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/Tags.php
43-43: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
75-75: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Auth/Login.php
134-134: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/Categories.php
53-53: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
85-85: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Member/Registration.php
67-67: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
163-163: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
src/Cms/Controllers/Admin/Profile.php
50-50: Avoid unused parameters such as '$request'. (undefined)
(UnusedFormalParameter)
🔇 Additional comments (54)
composer.json (1)
15-15: LGTM! Dependency update aligns with the refactor.The version bump to neuron-php/mvc 0.9.* is necessary to support the Request-based controller signatures introduced throughout this PR.
tests/Cms/ContentControllerTest.php (2)
7-7: LGTM! Request import added for new test pattern.
168-170: LGTM! Test correctly updated for Request-based signature.The test properly creates a Request object, sets route parameters, and passes it to the
markdown()method, matching the new controller signature.src/Cms/Controllers/Member/Dashboard.php (2)
7-9: LGTM! Imports added for Request-based signature.
32-37: LGTM! Method signature updated consistently with the refactor.The signature change from
array $parameterstoRequest $requestis necessary for framework consistency, even though this display-only method doesn't access request data. The static analysis warning about the unused parameter is a false positive.src/Cms/Controllers/Admin/Dashboard.php (2)
8-10: LGTM! Imports added for Request-based signature.
34-38: LGTM! Method signature updated consistently with the refactor.The signature change is necessary for framework consistency. The static analysis warning about the unused parameter is a false positive for this display-only method.
src/Cms/Controllers/Admin/Profile.php (2)
12-12: LGTM! Request import added.
46-50: LGTM! Method signature updated consistently.The signature change is necessary for framework consistency. The static analysis warning about the unused parameter is a false positive for this display-only method.
src/Cms/Controllers/Content.php (4)
50-56: LGTM! Imports added for Request-based API.
195-213: LGTM! Request-based implementation is correct.The
markdown()method has been properly refactored to accept a Request object and extract the route parameter usinggetRouteParameter('page').
247-247: LGTM! Minor formatting improvement.
267-267: LGTM! Minor formatting improvement.src/Cms/Controllers/Home.php (3)
4-7: LGTM! Imports added for explicit usage.The Registry import makes the usage more explicit and the NotFound import prepares for potential exception handling.
23-27: LGTM! Method signature updated consistently.The signature change is necessary for framework consistency. The static analysis warning about the unused parameter is a false positive for this display-only method.
31-31: LGTM! Explicit Registry usage.src/Cms/Controllers/Member/Profile.php (2)
11-11: LGTM! Request import added.
45-50: LGTM! Method signature updated consistently.The signature change is necessary for framework consistency. The static analysis warning about the unused parameter is a false positive for this display-only method.
tests/Cms/BlogControllerTest.php (1)
270-270: LGTM - Test correctly updated for new Request signature.The test properly constructs and passes a Request object to the index method.
src/Cms/Controllers/Member/Registration.php (4)
67-101: LGTM - Registration form rendering correctly updated.The method signature has been properly updated to accept a Request object, maintaining consistency with the framework's new API pattern.
108-154: LGTM - Form processing correctly migrated to Request object.All form data retrieval has been properly updated to use
$request->post()instead of direct superglobal access.
186-262: LGTM - Email verification correctly uses Request object.Query string parameter retrieval via
$request->get('token')is correct and follows the new pattern.
270-311: LGTM - Resend verification properly refactored.Form data and CSRF token retrieval correctly updated to use the Request object.
src/Cms/Controllers/Admin/Posts.php (5)
79-118: LGTM - Index method correctly updated.The method signature and implementation properly use the Request object pattern.
126-152: LGTM - Create form method correctly updated.Method signature properly updated to accept Request object for consistency with the framework.
208-249: LGTM - Edit method correctly refactored.Route parameter retrieval via
$request->getRouteParameter('id')is correct and consistent with the refactoring pattern.
257-311: LGTM - Update method correctly refactored.Route parameter and form data retrieval properly use the Request object methods.
318-350: LGTM - Destroy method correctly refactored.Route parameter retrieval via
$request->getRouteParameter('id')follows the correct pattern.src/Cms/Controllers/Blog.php (5)
45-64: LGTM - Index method correctly refactored.The method signature and implementation properly use the Request object pattern.
73-107: LGTM - Show method correctly refactored.Route parameter retrieval via
$request->getRouteParameter('slug')is correct and consistent.
148-179: LGTM - Tag method correctly refactored.Route parameter retrieval and repository usage follow the correct pattern.
188-219: LGTM - Category method correctly refactored.Route parameter retrieval and repository usage follow the correct pattern.
228-271: LGTM - Feed method signature updated correctly.The method signature has been properly updated with the NoReturn attribute and Request parameter.
src/Cms/Controllers/Admin/Tags.php (6)
43-67: LGTM - Index method correctly updated.The method signature properly accepts a Request object for framework consistency.
75-96: LGTM - Create method correctly updated.Method signature properly updated to accept Request object.
104-126: LGTM - Store method correctly refactored.Form data retrieval properly uses
$request->post().
135-165: LGTM - Edit method correctly refactored.Route parameter retrieval via
$request->getRouteParameter('id')follows the correct pattern.
174-203: LGTM - Update method correctly refactored.Route parameter and form data retrieval properly use the Request object.
212-225: LGTM - Destroy method correctly refactored.Route parameter retrieval follows the correct pattern.
src/Cms/Controllers/Auth/PasswordReset.php (2)
56-75: LGTM - Form rendering correctly updated.Method signature properly updated to accept Request object.
135-176: LGTM - Reset form correctly refactored.Query parameter retrieval via
$request->get('token')follows the correct pattern.src/Cms/Controllers/Auth/Login.php (3)
53-84: LGTM - Login form rendering correctly updated.Query parameter retrieval via
$request->get()and redirect validation are properly implemented.
134-138: LGTM - Logout method signature correctly updated.Method signature properly updated to accept Request object for framework consistency.
147-176: LGTM - Redirect validation modernized.The use of
str_contains()on line 170 is a good modernization from the deprecatedstrpos()pattern.src/Cms/Controllers/Admin/Users.php (5)
56-81: LGTM - Index method correctly updated.Method signature properly updated to accept Request object.
89-110: LGTM - Create method correctly updated.Method signature properly updated to accept Request object.
118-140: LGTM - Store method correctly refactored.All form data retrieval properly uses
$request->post().
149-178: LGTM - Edit method correctly refactored.Route parameter retrieval via
$request->getRouteParameter('id')follows the correct pattern.
229-249: LGTM - Destroy method correctly refactored.Route parameter retrieval follows the correct pattern.
src/Cms/Controllers/Admin/Categories.php (5)
114-129: LGTM! Correct usage of Request object.The method correctly uses
$request->post()to extract form data, and the implementation is consistent with similar methods in other controllers.
137-167: LGTM! Route parameter correctly extracted from Request.The method properly uses
$request->getRouteParameter('id')to extract the category ID, consistent with the refactor pattern seen in other controllers.
175-198: LGTM! Request object used correctly for both route parameters and form data.The method properly uses both
$request->getRouteParameter('id')and$request->post()to access route parameters and form data respectively. Implementation is consistent with the refactor pattern.
206-219: LGTM! Route parameter correctly extracted from Request.The method properly uses
$request->getRouteParameter('id')to extract the category ID for deletion. Implementation is consistent with the refactor pattern seen across other controllers.
53-53: No action required — the Request parameter is a framework design pattern.The consistent signature across all Admin controllers (
Dashboard,Users,Tags,Posts,Categories) confirms this is intentional framework design. All public controller methods receive theRequestobject, even if certain methods don't actively use it. Theindex()andcreate()methods don't require request data, whilestore(),edit(),update(), anddestroy()do—and they correctly utilize it. This aligns with the framework's standard controller API.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Parameters were moved into the request object and removed from the controller methods. All get, post and parameters access is now provided by the request object. Controller methods were updated to use the new method signature and access to the various data elements has been updated.
Summary by CodeRabbit
Refactor
Bug Fixes
Tests
Chores