Conversation
- Implement `BlockUser` and `UnBlockUser` actions. - Add `BlockedUsers` model, migration, and factory. - Introduce `BlockController` with endpoints to block/unblock users. - Update `ForYouFeedQuery` and `FollowingFeedQuery` to exclude posts from blocked users. - Add tests for blocking and unblocking users.
WalkthroughAdds a user-blocking feature: new BlockedUser model, migration and factory; BlockUser and UnBlockUser actions; BlockController and API routes; feed queries (ForYou and Following) updated to exclude blocked users; ForYouFeedController now accepts CurrentUser; comprehensive tests added. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as API Router
participant BC as BlockController
participant ABlock as BlockUser Action
participant AUnblock as UnBlockUser Action
participant DB as DB (blocked_users)
Client->>API: POST /blocks/{user}
API->>BC: store(#[CurrentUser] loggedIn, user)
BC->>ABlock: handle(loggedIn, user)
ABlock->>DB: firstOrCreate({user_id, blocked_user_id})
DB-->>ABlock: BlockedUser
ABlock-->>BC: void
BC-->>Client: 201 Created
Client->>API: DELETE /blocks/{user}
API->>BC: destroy(#[CurrentUser] loggedIn, user)
BC->>AUnblock: handle(loggedIn, user)
AUnblock->>DB: delete where({user_id, blocked_user_id})
DB-->>AUnblock: rows affected
AUnblock-->>BC: void
BC-->>Client: 204 No Content
sequenceDiagram
autonumber
actor User
participant C as ForYou/Following Controller
participant Q as FeedQuery
participant DB as DB
User->>C: GET /feed
C->>Q: new FeedQuery(currentUser?)
Q->>DB: SELECT posts WHERE ... AND user_id NOT IN (SELECT blocked_user_id FROM blocked_users WHERE user_id = currentUser.id)
DB-->>Q: posts
Q-->>C: Collection<Post>
C-->>User: JSON posts
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 2
🧹 Nitpick comments (6)
app/Queries/FollowingFeedQuery.php (1)
31-31: Consider removing trailing blank line.Line 31 appears to be an unintentional blank line before the
->latest()call.Apply this diff:
->where('user_id', $this->user->id)) ->with(['user', 'likes']) - ->latest('updated_at');routes/api.php (1)
47-48: Consider importing BlockController for consistency.The routes use the fully qualified class name
App\Http\Controllers\BlockController::class, while other controllers in this file are imported at the top. For consistency, consider adding an import statement.Add this import at the top of the file:
+use App\Http\Controllers\BlockController; use App\Http\Controllers\FollowController;Then update the routes:
- Route::post('/blocks/{user}', [App\Http\Controllers\BlockController::class, 'store'])->name('blocks.store'); - Route::delete('/blocks/{user}', [App\Http\Controllers\BlockController::class, 'destroy'])->name('blocks.destroy'); + Route::post('/blocks/{user}', [BlockController::class, 'store'])->name('blocks.store'); + Route::delete('/blocks/{user}', [BlockController::class, 'destroy'])->name('blocks.destroy');app/Actions/UnBlockUser.php (2)
12-12: Consider renaming parameter for clarity.The parameter
$toBlockis semantically confusing in an unblock context. Consider renaming it to$toUnblockor$userToUnblockfor better readability.Apply this diff:
- public function handle(User $user, User $toBlock): void + public function handle(User $user, User $toUnblock): void { BlockedUser::query() ->where([ 'user_id' => $user->id, - 'blocked_user_id' => $toBlock->id, + 'blocked_user_id' => $toUnblock->id, ]) ->delete(); }
14-20: Remove unnecessary blank line.Line 14 contains an unnecessary blank line before the query.
Apply this diff:
public function handle(User $user, User $toBlock): void { - BlockedUser::query() ->where([database/migrations/2025_10_03_144125_create_blocked_users_table.php (1)
13-19: Consider adding onDelete cascade behavior.The foreign keys lack explicit
onDelete()clauses. When a user is deleted, orphaned records inblocked_usersmay remain. Consider adding->onDelete('cascade')to both foreign keys to automatically clean up blocks when users are removed.Apply this diff:
Schema::create('blocked_users', function (Blueprint $table): void { $table->id(); - $table->foreignId('user_id')->constrained(); - $table->foreignId('blocked_user_id')->constrained('users'); + $table->foreignId('user_id')->constrained()->onDelete('cascade'); + $table->foreignId('blocked_user_id')->constrained('users')->onDelete('cascade'); $table->unique(['user_id', 'blocked_user_id']); $table->timestamps(); });app/Http/Controllers/BlockController.php (1)
15-35: Consider preventing self-blocking.While not critical, you may want to add validation to prevent users from blocking themselves. This could be implemented either in the controller or in the BlockUser action.
Example validation in the action:
public function handle(User $user, User $toBlock): void { if ($user->id === $toBlock->id) { throw new \InvalidArgumentException('Users cannot block themselves'); } BlockedUser::query()->firstOrCreate([ 'user_id' => $user->id, 'blocked_user_id' => $toBlock->id, ]); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
app/Actions/BlockUser.php(1 hunks)app/Actions/UnBlockUser.php(1 hunks)app/Http/Controllers/BlockController.php(1 hunks)app/Http/Controllers/ForYouFeedController.php(2 hunks)app/Models/BlockedUser.php(1 hunks)app/Models/User.php(1 hunks)app/Queries/FollowingFeedQuery.php(2 hunks)app/Queries/ForYouFeedQuery.php(1 hunks)database/factories/BlockedUserFactory.php(1 hunks)database/migrations/2025_10_03_144125_create_blocked_users_table.php(1 hunks)routes/api.php(1 hunks)tests/Feature/Http/BlockControllerTest.php(1 hunks)tests/Unit/Actions/BlockUserTest.php(1 hunks)tests/Unit/Actions/UnBlockUserTest.php(1 hunks)tests/Unit/Queries/FollowingFeedQueryTest.php(1 hunks)tests/Unit/Queries/ForYouFeedQueryTest.php(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (15)
app/Actions/BlockUser.php (3)
app/Models/BlockedUser.php (2)
BlockedUser(12-28)user(18-21)app/Models/User.php (1)
User(27-98)app/Actions/UnBlockUser.php (1)
handle(12-21)
tests/Unit/Actions/UnBlockUserTest.php (4)
app/Models/User.php (3)
User(27-98)blockedUsers(86-89)blockedByUsers(94-97)app/Models/BlockedUser.php (1)
user(18-21)app/Actions/BlockUser.php (2)
BlockUser(10-19)handle(12-18)app/Actions/UnBlockUser.php (2)
handle(12-21)UnBlockUser(10-22)
app/Actions/UnBlockUser.php (3)
app/Models/BlockedUser.php (2)
BlockedUser(12-28)user(18-21)app/Models/User.php (1)
User(27-98)app/Actions/BlockUser.php (1)
handle(12-18)
app/Models/BlockedUser.php (2)
database/factories/BlockedUserFactory.php (1)
BlockedUserFactory(15-28)app/Models/User.php (1)
User(27-98)
tests/Unit/Queries/ForYouFeedQueryTest.php (4)
app/Models/BlockedUser.php (2)
user(18-21)blockedUser(24-27)app/Models/User.php (1)
User(27-98)app/Queries/ForYouFeedQuery.php (2)
ForYouFeedQuery(12-31)builder(21-30)app/Actions/BlockUser.php (2)
BlockUser(10-19)handle(12-18)
tests/Unit/Actions/BlockUserTest.php (3)
app/Models/User.php (3)
User(27-98)blockedUsers(86-89)blockedByUsers(94-97)app/Models/BlockedUser.php (3)
user(18-21)blockedUser(24-27)BlockedUser(12-28)app/Actions/BlockUser.php (2)
BlockUser(10-19)handle(12-18)
app/Queries/FollowingFeedQuery.php (1)
app/Models/BlockedUser.php (1)
user(18-21)
app/Queries/ForYouFeedQuery.php (4)
app/Models/User.php (1)
User(27-98)app/Queries/FollowingFeedQuery.php (2)
__construct(14-16)builder(21-33)app/Models/BlockedUser.php (1)
user(18-21)app/Models/Post.php (1)
Post(24-44)
app/Models/User.php (1)
app/Models/BlockedUser.php (1)
BlockedUser(12-28)
app/Http/Controllers/BlockController.php (4)
app/Actions/BlockUser.php (2)
BlockUser(10-19)handle(12-18)app/Actions/UnBlockUser.php (2)
UnBlockUser(10-22)handle(12-21)app/Models/User.php (1)
User(27-98)app/Models/BlockedUser.php (1)
user(18-21)
routes/api.php (1)
app/Http/Controllers/BlockController.php (1)
BlockController(13-36)
tests/Feature/Http/BlockControllerTest.php (2)
app/Models/User.php (2)
User(27-98)blockedUsers(86-89)app/Models/BlockedUser.php (2)
user(18-21)BlockedUser(12-28)
database/factories/BlockedUserFactory.php (2)
app/Models/BlockedUser.php (1)
BlockedUser(12-28)app/Models/User.php (1)
User(27-98)
app/Http/Controllers/ForYouFeedController.php (3)
app/Models/User.php (1)
User(27-98)app/Queries/ForYouFeedQuery.php (1)
ForYouFeedQuery(12-31)app/Http/Controllers/FollowingFeedController.php (1)
__invoke(18-31)
tests/Unit/Queries/FollowingFeedQueryTest.php (6)
app/Models/BlockedUser.php (2)
user(18-21)blockedUser(24-27)app/Models/User.php (2)
User(27-98)posts(54-57)app/Actions/FollowUser.php (1)
FollowUser(9-17)app/Actions/BlockUser.php (2)
handle(12-18)BlockUser(10-19)app/Models/Post.php (1)
Post(24-44)app/Queries/FollowingFeedQuery.php (2)
FollowingFeedQuery(12-34)builder(21-33)
🔇 Additional comments (18)
app/Models/User.php (1)
83-97: LGTM! Blocking relationships are well-structured.The
blockedUsers()andblockedByUsers()relationships are correctly implemented usingHasManywith appropriate foreign keys. The bidirectional setup enables efficient querying from both perspectives (who a user blocks and who blocks them).app/Queries/FollowingFeedQuery.php (1)
27-29: Blocking filter correctly excludes blocked users.The
whereNotInsubquery efficiently filters out posts from users blocked by the current user. The implementation aligns with Laravel best practices and uses proper type hints.tests/Unit/Actions/BlockUserTest.php (1)
7-42: LGTM! Comprehensive test coverage for BlockUser action.The tests properly verify:
- Blocking creates the relationship from both perspectives
- Re-blocking is idempotent (no duplicates)
- BlockedUser model relationships return correct user references
tests/Unit/Actions/UnBlockUserTest.php (1)
7-32: LGTM! UnBlockUser tests are well-structured.The tests verify:
- Unblocking removes the relationship completely
- Unblocking when no block exists is a safe no-op
app/Actions/BlockUser.php (1)
14-17: Configure mass assignment for BlockedUser model.app/Models/BlockedUser.phpdefines neither$fillablenor$guarded, so it relies on Laravel’s default. Explicitly addprotected $fillable = ['user_id', 'blocked_user_id'];or
protected $guarded = [];to allow
firstOrCreateto work as intended.app/Queries/ForYouFeedQuery.php (2)
14-17: LGTM!The constructor correctly accepts an optional User to support both authenticated and unauthenticated scenarios.
25-28: LGTM!The blocking filter is correctly implemented using a subquery pattern that mirrors FollowingFeedQuery. The use of
when()ensures the filter only applies when a user is authenticated, and the null-safe operator guards against edge cases.app/Http/Controllers/ForYouFeedController.php (1)
18-20: LGTM!The controller correctly injects the optional current user and passes it to ForYouFeedQuery, enabling blocking logic while supporting unauthenticated access.
tests/Feature/Http/BlockControllerTest.php (3)
8-19: LGTM!The test correctly verifies that a user can block another user, with proper authentication and assertion on the relationship count.
21-38: LGTM!The test properly seeds a block, authenticates, invokes the unblock endpoint, and verifies cleanup using
refresh()to reload the model state.
40-55: Idempotent blocking returns 201.The test expects HTTP 201 when blocking an already-blocked user. This is consistent with
firstOrCreatebehavior in the BlockUser action, treating the operation as idempotent. However, REST conventions sometimes use 200 for operations that don't create new resources. The current behavior is acceptable but worth noting.tests/Unit/Queries/ForYouFeedQueryTest.php (4)
10-17: LGTM!The test correctly instantiates ForYouFeedQuery with a User, aligning with the updated constructor signature.
19-31: LGTM!The test properly verifies post ordering and now passes the required User parameter to ForYouFeedQuery.
33-39: LGTM!The empty collection test is updated to work with the new constructor.
41-59: LGTM!Comprehensive test for blocking functionality. The test correctly sets up the blocking relationship, creates posts from both blocked and non-blocked users, and verifies that only the non-blocked user's post appears in the feed.
app/Http/Controllers/BlockController.php (2)
15-24: LGTM!The store method correctly delegates to the BlockUser action and returns HTTP 201. The idempotent behavior (via
firstOrCreatein the action) is appropriate.
26-35: LGTM!The destroy method properly delegates to UnBlockUser and returns HTTP 204 (No Content), which is the standard response for successful DELETE operations.
database/factories/BlockedUserFactory.php (1)
15-27: LGTM!The factory correctly defines all required attributes for BlockedUser, using User factories for the foreign key relationships. The use of
Carbon::now()for timestamps is appropriate for test data.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
app/Models/BlockedUser.php (1)
1-11: Add explicit import for User model.While PHP resolves
User::classcorrectly because both models share theApp\Modelsnamespace, Laravel convention is to explicitly import all referenced classes for clarity and consistency with the rest of the codebase.Apply this diff:
namespace App\Models; use Database\Factories\BlockedUserFactory; use Illuminate\Database\Eloquent\Factories\HasFactory; use Illuminate\Database\Eloquent\Model; use Illuminate\Database\Eloquent\Relations\BelongsTo; +use App\Models\User;
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/Models/BlockedUser.php(1 hunks)tests/Unit/Queries/FollowingFeedQueryTest.php(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Unit/Queries/FollowingFeedQueryTest.php
🧰 Additional context used
🧬 Code graph analysis (1)
app/Models/BlockedUser.php (2)
database/factories/BlockedUserFactory.php (1)
BlockedUserFactory(15-28)app/Models/User.php (1)
User(27-98)
🔇 Additional comments (3)
app/Models/BlockedUser.php (3)
12-15: LGTM!The class declaration and factory trait usage follow Laravel conventions correctly.
17-21: LGTM! Previous issue fixed.The method name case sensitivity issue has been corrected. The relationship now correctly calls
belongsTo()with lowercase 'b'.
23-27: LGTM!The
blockedUser()relationship is correctly defined with an explicit foreign key, and the type hint is accurate.
| final class BlockedUser extends Model | ||
| { | ||
| /** @use HasFactory<BlockedUserFactory> */ | ||
| use HasFactory; | ||
|
|
||
| /** @return BelongsTo<User, $this> */ | ||
| public function user(): BelongsTo | ||
| { | ||
| return $this->belongsTo(User::class); | ||
| } | ||
|
|
||
| /** @return BelongsTo<User, $this> */ | ||
| public function blockedUser(): BelongsTo | ||
| { | ||
| return $this->belongsTo(User::class, 'blocked_user_id'); | ||
| } | ||
| } |
There was a problem hiding this comment.
Add $fillable to prevent MassAssignmentException.
The model lacks $fillable or $guarded protection. The BlockUser action uses firstOrCreate() with user_id and blocked_user_id, which requires these fields to be mass-assignable. Without this configuration, the action will throw a MassAssignmentException at runtime.
Apply this diff:
final class BlockedUser extends Model
{
/** @use HasFactory<BlockedUserFactory> */
use HasFactory;
+ /**
+ * @var list<string>
+ */
+ protected $fillable = [
+ 'user_id',
+ 'blocked_user_id',
+ ];
+
/** @return BelongsTo<User, $this> */
public function user(): BelongsTo📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| final class BlockedUser extends Model | |
| { | |
| /** @use HasFactory<BlockedUserFactory> */ | |
| use HasFactory; | |
| /** @return BelongsTo<User, $this> */ | |
| public function user(): BelongsTo | |
| { | |
| return $this->belongsTo(User::class); | |
| } | |
| /** @return BelongsTo<User, $this> */ | |
| public function blockedUser(): BelongsTo | |
| { | |
| return $this->belongsTo(User::class, 'blocked_user_id'); | |
| } | |
| } | |
| final class BlockedUser extends Model | |
| { | |
| /** @use HasFactory<BlockedUserFactory> */ | |
| use HasFactory; | |
| /** | |
| * @var list<string> | |
| */ | |
| protected $fillable = [ | |
| 'user_id', | |
| 'blocked_user_id', | |
| ]; | |
| /** @return BelongsTo<User, $this> */ | |
| public function user(): BelongsTo | |
| { | |
| return $this->belongsTo(User::class); | |
| } | |
| /** @return BelongsTo<User, $this> */ | |
| public function blockedUser(): BelongsTo | |
| { | |
| return $this->belongsTo(User::class, 'blocked_user_id'); | |
| } | |
| } |
🤖 Prompt for AI Agents
In app/Models/BlockedUser.php around lines 12 to 28, the model lacks
mass-assignment protection so firstOrCreate() will throw
MassAssignmentException; add a protected $fillable property (e.g. ['user_id',
'blocked_user_id']) or alternatively set protected $guarded = [] to allow these
attributes to be mass-assignable; place the property near the top of the class
(before the relations) and ensure it is declared protected.
There was a problem hiding this comment.
Model::unguard(); already at AppServiceProvider.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Model::unguard() is called in AppServiceProvider
rg -n "Model::unguard\(\)" app/Providers/AppServiceProvider.php
Length of output: 28
@hasansoyalan, you're absolutely right! I've confirmed that Model::unguard() is called in AppServiceProvider.php (line 19), which disables mass assignment protection globally. This means the $fillable property is unnecessary for the BlockedUser model.
I apologize for the false positive—the model works correctly as-is with your project's configuration. Thank you for the clarification! 🐰
✏️ Learnings added
Learnt from: hasansoyalan
PR: supo-dev/api#10
File: app/Models/BlockedUser.php:12-28
Timestamp: 2025-10-03T17:22:21.246Z
Learning: In the supo-dev/api repository, Model::unguard() is called globally in AppServiceProvider.php, so models do not require $fillable or $guarded properties for mass assignment protection.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
Yey @nunomaduro if you like this pr i can fix conflicts. |
1 participant
Added user blocking functionality,
BlockUserandUnBlockUseractions.BlockedUsersmodel, migration, and factory.BlockControllerwith endpoints to block/unblock users.ForYouFeedQueryandFollowingFeedQueryto exclude posts from blocked users.Please let me know if any tweaks needed.
Thanks.
Summary by CodeRabbit
New Features
Tests