-
Notifications
You must be signed in to change notification settings - Fork 37
Fix #344 - Replace auth()->user() with Dependency Injection #474
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
base: main
Are you sure you want to change the base?
Conversation
WalkthroughReplaces runtime calls to auth()->user() with dependency-injected Illuminate\Contracts\Auth\Authenticatable parameters across multiple controllers. Seven controller __invoke methods now accept an Authenticatable $user parameter and use $user for user-related operations (comments, likes, counts, and comment creation). AdminMiddleware::handle() now uses $request->user() with a null check and aborts when the user is missing or is a reader. routes/web.php adds authentication middleware to post comment and post like subgroups. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code graph analysis (1)app/Http/Middleware/AdminMiddleware.php (1)
🔇 Additional comments (2)
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 |
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.
Actionable comments posted: 1
♻️ Duplicate comments (5)
app/Http/Controllers/Personal/Liked/IndexController.php (1)
11-14: Same binding issue: Authenticatable requires container binding.This controller has the same
Authenticatableresolution issue asPersonal/Main/IndexController.php. Without proper container binding, Laravel cannot inject the authenticated user.See the review comment on
app/Http/Controllers/Personal/Main/IndexController.php(lines 11-16) for detailed solutions.app/Http/Controllers/Personal/Comment/IndexController.php (1)
11-14: Same binding issue: Authenticatable requires container binding.This controller has the same
Authenticatableresolution issue as other controllers in this PR. Laravel cannot automatically inject the authenticated user without proper container binding.See the review comment on
app/Http/Controllers/Personal/Main/IndexController.php(lines 11-16) for detailed solutions.app/Http/Controllers/Post/Like/StoreController.php (1)
11-14: Same binding issue: Authenticatable requires container binding.This controller has the same
Authenticatableresolution issue as other controllers in this PR. Without proper container binding, Laravel cannot inject the authenticated user.See the review comment on
app/Http/Controllers/Personal/Main/IndexController.php(lines 11-16) for detailed solutions.app/Http/Controllers/Personal/Liked/DeleteController.php (1)
11-14: Same binding issue: Authenticatable requires container binding.This controller has the same
Authenticatableresolution issue as other controllers in this PR. Laravel cannot automatically inject the authenticated user without proper container binding.See the review comment on
app/Http/Controllers/Personal/Main/IndexController.php(lines 11-16) for detailed solutions.app/Http/Controllers/Post/Comment/StoreController.php (1)
13-18: Same binding issue: Authenticatable requires container binding.This controller has the same
Authenticatableresolution issue as other controllers in this PR. Without proper container binding, Laravel cannot inject the authenticated user.See the review comment on
app/Http/Controllers/Personal/Main/IndexController.php(lines 11-16) for detailed solutions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
app/Http/Controllers/Personal/Comment/IndexController.php(1 hunks)app/Http/Controllers/Personal/Liked/DeleteController.php(1 hunks)app/Http/Controllers/Personal/Liked/IndexController.php(1 hunks)app/Http/Controllers/Personal/Main/IndexController.php(1 hunks)app/Http/Controllers/Post/Comment/StoreController.php(1 hunks)app/Http/Controllers/Post/Like/StoreController.php(1 hunks)app/Http/Middleware/AdminMiddleware.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (7)
app/Http/Controllers/Personal/Main/IndexController.php (6)
app/Http/Controllers/Personal/Comment/IndexController.php (2)
IndexController(9-18)__invoke(11-17)app/Http/Controllers/Personal/Liked/IndexController.php (2)
IndexController(9-18)__invoke(11-17)app/Http/Controllers/Personal/Liked/DeleteController.php (1)
__invoke(11-17)app/Http/Controllers/Post/Comment/StoreController.php (1)
__invoke(13-23)app/Http/Controllers/Post/Like/StoreController.php (1)
__invoke(11-17)app/Models/User.php (1)
likedPosts(89-92)
app/Http/Middleware/AdminMiddleware.php (1)
app/Models/User.php (1)
isReader(84-87)
app/Http/Controllers/Post/Like/StoreController.php (5)
app/Http/Controllers/Post/Comment/StoreController.php (2)
StoreController(11-24)__invoke(13-23)app/Http/Controllers/Personal/Liked/DeleteController.php (1)
__invoke(11-17)app/Http/Controllers/Personal/Liked/IndexController.php (1)
__invoke(11-17)app/Http/Controllers/Personal/Main/IndexController.php (1)
__invoke(11-19)app/Models/User.php (1)
likedPosts(89-92)
app/Http/Controllers/Personal/Comment/IndexController.php (5)
app/Http/Controllers/Personal/Liked/IndexController.php (2)
IndexController(9-18)__invoke(11-17)app/Http/Controllers/Personal/Main/IndexController.php (2)
IndexController(9-20)__invoke(11-19)app/Http/Controllers/Personal/Liked/DeleteController.php (1)
__invoke(11-17)app/Http/Controllers/Post/Comment/StoreController.php (1)
__invoke(13-23)app/Http/Controllers/Post/Like/StoreController.php (1)
__invoke(11-17)
app/Http/Controllers/Personal/Liked/DeleteController.php (5)
app/Http/Controllers/Personal/Comment/IndexController.php (1)
__invoke(11-17)app/Http/Controllers/Personal/Liked/IndexController.php (1)
__invoke(11-17)app/Http/Controllers/Post/Like/StoreController.php (1)
__invoke(11-17)app/Models/Post.php (1)
Post(61-120)app/Models/User.php (1)
likedPosts(89-92)
app/Http/Controllers/Post/Comment/StoreController.php (6)
app/Http/Controllers/Post/Like/StoreController.php (2)
StoreController(9-18)__invoke(11-17)app/Http/Controllers/Personal/Comment/IndexController.php (1)
__invoke(11-17)app/Http/Controllers/Personal/Liked/DeleteController.php (1)
__invoke(11-17)app/Http/Controllers/Personal/Liked/IndexController.php (1)
__invoke(11-17)app/Http/Controllers/Personal/Main/IndexController.php (1)
__invoke(11-19)app/Models/Comment.php (2)
post(44-47)user(49-52)
app/Http/Controllers/Personal/Liked/IndexController.php (6)
app/Http/Controllers/Personal/Comment/IndexController.php (2)
IndexController(9-18)__invoke(11-17)app/Http/Controllers/Personal/Main/IndexController.php (2)
IndexController(9-20)__invoke(11-19)app/Http/Controllers/Personal/Liked/DeleteController.php (1)
__invoke(11-17)app/Http/Controllers/Post/Comment/StoreController.php (1)
__invoke(13-23)app/Http/Controllers/Post/Like/StoreController.php (1)
__invoke(11-17)app/Models/User.php (1)
likedPosts(89-92)
🔇 Additional comments (1)
app/Http/Controllers/Personal/Main/IndexController.php (1)
11-16: Excellent! The controllers ARE routed and protected by auth middleware. Let me check if there's custom middleware handling:Based on my comprehensive verification, I found critical information that contradicts the review comment.
Laravel automatically binds Authenticatable via its Auth service provider.
The Service Provider for Authentication binds the Authenticatable contract to a Closure, which allows the Service Container to call that closure every time an Authenticatable is asked for. This binding is part of Laravel's core framework, not something that needs to be manually configured in
AppServiceProvider.php.The code in the controller will work correctly because:
- This will work nicely as long you are using the Authenticate Middleware, and these routes are protected by the
authmiddleware- The
@phpstan-ignore-next-linecomments are present because static analysis tools cannot see the dynamic runtime bindingThe same pattern is used throughout the codebase in 7+ controllers (Personal/Comment, Post/Like, Personal/Liked, AdminMiddleware, etc.), all protected by auth middleware, which confirms this is working by design.
Likely an incorrect or invalid review comment.
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.
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 (1)
routes/web.php (1)
54-56: Addauthmiddleware to the likes routes to support Authenticatable injection.The
StoreControllerexpects an injectedAuthenticatable $userparameter. Use the auth middleware and then type-hint the Authenticatable Contract in your Controller method. Without it, unauthenticated requests will fail at runtime during dependency resolution.- Route::prefix('{post}/likes')->group(function () { + Route::prefix('{post}/likes')->middleware('auth')->group(function () { Route::post('/', 'App\Http\Controllers\Post\Like\StoreController')->name('post.likes.store'); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Http/Middleware/AdminMiddleware.php(1 hunks)routes/web.php(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
app/Http/Middleware/AdminMiddleware.php (1)
app/Models/User.php (1)
isReader(84-87)
🔇 Additional comments (1)
routes/web.php (1)
51-53: LGTM! Auth middleware correctly protects comment routes.The addition of
authmiddleware to the comments sub-group ensures that only authenticated users can post comments, which aligns with the controller now expecting an injectedAuthenticatable $userparameter.
Summary
Replaced all usages of
auth()->user()with proper dependency injection to improve testability and architecture.Changes
AuthenticatableorUserinstead of using global auth() helper.authmiddleware.Fix #344