cleaned up movie list policy

app/Http/Controllers/MovieListController.php

@@ -8,7 +8,6 @@ use App\Http\Resources\MovieListResource;
 use App\Interfaces\MovieDbInterface;
 use App\Models\Movie;
 use App\Models\MovieList;
-use App\Models\Role;
 use App\Models\User;
 use Illuminate\Http\JsonResponse;
 use Illuminate\Http\Request;
@@ -34,18 +33,18 @@ class MovieListController extends Controller
     /**
      * Store a newly created resource in storage.
      */
-    public function store(CreateMovieListRequest $request)
+    public function store(CreateMovieListRequest $request): MovieListResource
     {
         $this->authorize('create', MovieList::class);
 
         $validated = $request->validated();
         $movieList = MovieList::create([
             ...$validated,
-            'owner' => auth()->id(),
+            'owner' => Auth::user()->id,
             'slug' => Str::slug($validated['name']),
         ]);
 
-        return response()->json($movieList, 201);
+        return MovieListResource::make($movieList);
     }
 
     /**
@@ -77,12 +76,13 @@ class MovieListController extends Controller
         $this->authorize('delete', $movieList);
         $movieList->delete();
 
-        return response()->json(['message', 'Movie list deleted successfully'], 204);
+        return response()->json(['message' => 'Movie list deleted successfully'], 204);
     }
 
     public function addMovie(MovieDbInterface $movieDb, Request $request, MovieList $movieList): MovieListResource
     {
-        $this->authorize('update', $movieList);
+        $this->authorize('editMovies', $movieList);
+
         $movieResult = $movieDb->find($request->input('movie')['imdbId'], ['type' => 'imdb']);
         $movie = Movie::where('imdb_id', $movieResult->imdbId)->first();
 
@@ -94,7 +94,7 @@ class MovieListController extends Controller
 
     public function removeMovie(MovieList $movieList, Movie $movie): MovieListResource
     {
-        $this->authorize('update', $movieList);
+        $this->authorize('editMovies', $movieList);
 
         $movieList->movies()->detach($movie);
         $movieList->load('movies');
@@ -104,13 +104,13 @@ class MovieListController extends Controller
 
     public function updateCollaboratorRole(Request $request, MovieList $movieList, User $collaborator): MovieListResource|JsonResponse
     {
+        $this->authorize('update', $movieList);
         $request->validate([
             'role_id' => 'required|exists:roles,id',
         ]);
 
-        $adminRole = Role::query()->where('name', 'ADMIN')->first()?->id;
-        if (Auth::id() !== $movieList->owner && ! Auth::user()->hasRole($movieList, $adminRole)) {
-            return response()->json(['message' => 'Unauthorized'], 403);
+        if (Auth::id() === $collaborator->getKey()) {
+            return response()->json(['message' => 'Cannot edit own role'], 422);
         }
 
         $movieList->collaborators()->updateExistingPivot($collaborator->getKey(), [

app/Models/MovieList.php

@@ -27,7 +27,7 @@ class MovieList extends Model
         return $this->belongsToMany(Movie::class);
     }
 
-    public function getUserRole($userId): string
+    public function getUserRole($userId): ?string
     {
         $roleId = $this->collaborators()
             ->where('user_id', $userId)

app/Models/User.php

@@ -15,6 +15,10 @@ class User extends Authenticatable
     /** @use HasFactory<\Database\Factories\UserFactory> */
     use HasFactory, Notifiable;
 
+    private static $adminRoleId = null;
+
+    private static $editorRoleId = null;
+
     /**
      * The attributes that are mass assignable.
      *
@@ -45,10 +49,33 @@ class User extends Authenticatable
         return $this->hasMany(MovieList::class, 'owner');
     }
 
-    public function hasRole(MovieList $movieList, int $role): bool
+    public function isListEditor(MovieList $movieList): bool
+    {
+        self::$editorRoleId = Role::query()
+            ->where('name', 'EDITOR')
+            ->value('id');
+
+        return $this->isListAdmin($movieList) || $this->hasRole($movieList->getKey(), self::$editorRoleId);
+    }
+
+    public function isListAdmin(MovieList $movieList): bool
+    {
+        self::$adminRoleId = Role::query()
+            ->where('name', 'ADMIN')
+            ->value('id');
+
+        return $this->isListOwner($movieList) || $this->hasRole($movieList->getKey(), self::$adminRoleId);
+    }
+
+    public function isListOwner(MovieList $movieList): bool
+    {
+        return $this->getKey() === $movieList->owner;
+    }
+
+    public function hasRole(int $movieListId, int $role): bool
     {
         return $this->sharedLists()
-            ->wherePivot('movie_list_id', $movieList->id)
+            ->wherePivot('movie_list_id', $movieListId)
             ->wherePivot('role_id', $role)
             ->exists();
     }
@@ -60,6 +87,13 @@ class User extends Authenticatable
             ->withTimestamps();
     }
 
+    public function roles(): BelongsToMany
+    {
+        return $this->belongsToMany(Role::class, 'movie_list_user')
+            ->withPivot('role_id')
+            ->withTimestamps();
+    }
+
     /**
      * Get the attributes that should be cast.
      *

app/Policies/MovieListPolicy.php

@@ -22,29 +22,23 @@ class MovieListPolicy
 
     public function view(User $user, MovieList $movieList): bool
     {
-        if ($movieList->owner === $user->getKey() || $movieList->isPublic || $user->sharedLists->contains($movieList)) {
-            return true;
-        }
-
-        return false;
-    }
-
-    public function update(User $user, MovieList $movieList): bool
-    {
-
-        if ($movieList->owner === $user->getKey()) {
-            return true;
-        }
-
-        return false;
+        return $movieList->is_public
+            || $user->isListOwner($movieList)
+            || $user->sharedLists->contains($movieList);
     }
 
     public function delete(User $user, MovieList $movieList): bool
     {
-        if ($movieList->owner === $user->getKey()) {
-            return true;
-        }
+        return $user->isListOwner($movieList);
+    }
 
-        return false;
+    public function editMovies(User $user, MovieList $movieList): bool
+    {
+        return $user->isListEditor($movieList);
+    }
+
+    public function update(User $user, MovieList $movieList): bool
+    {
+        return $user->isListAdmin($movieList);
     }
 }

routes/api.php

@@ -10,7 +10,6 @@ use Illuminate\Support\Facades\Route;
 // Public auth routes
 Route::post('/register', [AuthController::class, 'register'])->name('auth.register');
 Route::post('/login', [AuthController::class, 'login'])->name('auth.login');
-Route::post('/reset-password', [AuthController::class, 'resetPassword'])->name('auth.reset-password');
 Route::post('/forgot-password', [AuthController::class, 'forgotPassword'])->name('auth.forgot-password');
 Route::get('/invitations/{token}/accept', [InvitationController::class, 'accept'])->name('invitations.accept');
 Route::get('/invitations/{token}/decline', [InvitationController::class, 'decline'])->name('invitations.decline');
@@ -18,6 +17,7 @@ Route::get('/invitations/{token}/decline', [InvitationController::class, 'declin
 // Authenticated routes
 Route::middleware('auth:sanctum')->group(function () {
     Route::post('/logout', [AuthController::class, 'logout'])->name('auth.logout');
+    Route::post('/reset-password', [AuthController::class, 'resetPassword'])->name('auth.reset-password');
 
     // Invitations
     Route::post('/invitations', [InvitationController::class, 'store'])->name('invitations.store');