feat: rework playlist cover upload/remove (#1806)

.github/workflows/unit-frontend.yml

@@ -30,7 +30,7 @@ jobs:
     strategy:
       fail-fast: false
       matrix:
-        node-version: [ 18, 20, 21, 22 ]
+        node-version: [ 18 ]
     steps:
       - uses: actions/checkout@v2
       - uses: actions/setup-node@v2

app/Http/Controllers/API/PlaylistCoverController.php

@@ -0,0 +1,31 @@
+<?php
+
+namespace App\Http\Controllers\API;
+
+use App\Http\Controllers\Controller;
+use App\Http\Requests\API\PlaylistCoverUpdateRequest;
+use App\Models\Playlist;
+use App\Services\MediaMetadataService;
+
+class PlaylistCoverController extends Controller
+{
+    public function __construct(private readonly MediaMetadataService $mediaMetadataService)
+    {
+    }
+
+    public function update(PlaylistCoverUpdateRequest $request, Playlist $playlist)
+    {
+        $this->authorize('own', $playlist);
+        $this->mediaMetadataService->writePlaylistCover($playlist, $request->getFileContent());
+
+        return response()->json(['cover_url' => $playlist->cover]);
+    }
+
+    public function destroy(Playlist $playlist)
+    {
+        $this->authorize('own', $playlist);
+        $this->mediaMetadataService->deletePlaylistCover($playlist);
+
+        return response()->noContent();
+    }
+}

app/Http/Controllers/API/UploadPlaylistCoverController.php

@@ -1,22 +0,0 @@
-<?php
-
-namespace App\Http\Controllers\API;
-
-use App\Http\Controllers\Controller;
-use App\Http\Requests\API\UploadPlaylistCoverRequest;
-use App\Models\Playlist;
-use App\Services\MediaMetadataService;
-
-class UploadPlaylistCoverController extends Controller
-{
-    public function __invoke(
-        UploadPlaylistCoverRequest $request,
-        Playlist $playlist,
-        MediaMetadataService $mediaMetadataService
-    ) {
-        $this->authorize('own', $playlist);
-        $mediaMetadataService->writePlaylistCover($playlist, $request->getFileContent());
-
-        return response()->json(['cover_url' => $playlist->cover]);
-    }
-}

app/Http/Requests/API/PlaylistCoverUpdateRequest.php

@@ -3,7 +3,7 @@
 namespace App\Http\Requests\API;
 
 /** @property-read string $cover */
-class UploadPlaylistCoverRequest extends MediaImageUpdateRequest
+class PlaylistCoverUpdateRequest extends MediaImageUpdateRequest
 {
     protected function getImageFieldName(): string
     {

app/Services/MediaMetadataService.php

@@ -135,4 +135,12 @@ class MediaMetadataService
         File::delete($album->cover_path);
         File::delete($album->thumbnail_path);
     }
+
+    public function deletePlaylistCover(Playlist $playlist): void
+    {
+        if ($playlist->cover_path) {
+            File::delete($playlist->cover_path);
+            $playlist->update(['cover' => null]);
+        }
+    }
 }

resources/assets/js/components/playlist/PlaylistContextMenu.vue

@@ -6,10 +6,12 @@
     <template v-if="canShowCollaboration">
       <li class="separator" />
       <li @click="showCollaborationModal">Collaborate…</li>
-      <li class="separator" />
     </template>
-    <li v-if="canEditPlaylist" @click="edit">Edit…</li>
-    <li v-if="canEditPlaylist" @click="destroy">Delete</li>
+    <template v-if="canEditPlaylist">
+      <li class="separator" />
+      <li @click="edit">Edit…</li>
+      <li @click="destroy">Delete</li>
+    </template>
   </ContextMenu>
 </template>
 

resources/assets/js/components/screens/PlaylistScreen.spec.ts

@@ -59,6 +59,8 @@ new class extends UnitTestCase {
 
   private async renderComponent (songs: Playable[]) {
     playlist = playlist || factory('playlist')
+    this.be(factory('user', { id: playlist.user_id }))
+
     playlistStore.init([playlist])
     playlist.playables = songs
 

resources/assets/js/components/ui/PlaylistThumbnail.vue

@@ -1,15 +1,27 @@
 <template>
   <article
-    :class="{ droppable }"
     class="cover relative w-full aspect-square block rounded-md overflow-hidden bg-no-repeat bg-cover bg-center"
     data-testid="playlist-thumbnail"
-    @dragenter.prevent="onDragEnter"
-    @dragleave.prevent="onDragLeave"
-    @drop.prevent="onDrop"
-    @dragover.prevent
   >
-    <div class="pointer-events-none">
-      <slot />
+    <slot />
+    <div
+      v-if="canEditPlaylist"
+      class="absolute inset-0 w-full h-full bg-black bg-opacity-50 opacity-0 hover:opacity-100 transition-opacity duration-200
+             flex items-center justify-center"
+    >
+      <div class="border border-1.5 border-white/20 rounded-md overflow-hidden">
+        <button class="p-2 hover:bg-black/50" title="Upload cover image" @click.prevent="uploadCover">
+          <Icon :icon="faUpload" class="text-white text-xl" fixed-width />
+        </button>
+        <button
+          v-if="playlist.cover"
+          class="p-2 hover:bg-black/30"
+          title="Remove cover image"
+          @click.prevent="removeCover"
+        >
+          <Icon :icon="faTrash" class="text-white text-xl" fixed-width />
+        </button>
+      </div>
     </div>
   </article>
 </template>
@@ -18,74 +30,55 @@
 import { computed, ref, toRefs } from 'vue'
 import { defaultCover } from '@/utils'
 import { playlistStore } from '@/stores'
-import { useAuthorization, useErrorHandler, useFileReader, useKoelPlus } from '@/composables'
-import { acceptedImageTypes } from '@/config'
+import { useErrorHandler, useFileReader, useKoelPlus, usePolicies } from '@/composables'
+import { faTrash, faUpload } from '@fortawesome/free-solid-svg-icons'
 
 const props = defineProps<{ playlist: Playlist }>()
 const { playlist } = toRefs(props)
 
-const droppable = ref(false)
-
-const { isAdmin, currentUser } = useAuthorization()
+const { currentUserCan } = usePolicies()
 const { isPlus } = useKoelPlus()
 
+const canEditPlaylist = computed(() => currentUserCan.editPlaylist(playlist.value!))
 const backgroundImage = computed(() => `url(${playlist.value.cover || defaultCover})`)
 
-const allowsUpload = computed(() => (isAdmin.value || isPlus.value) && playlist.value.user_id === currentUser.value.id)
-const onDragEnter = () => (droppable.value = allowsUpload.value)
-const onDragLeave = () => (droppable.value = false)
+const uploadCover = () => {
+  const input = document.createElement('input')
+  input.setAttribute('type', 'file')
+  input.setAttribute('accept', 'image/*')
 
-const validImageDropEvent = (event: DragEvent) => {
-  if (!event.dataTransfer || !event.dataTransfer.items) {
-    return false
-  }
+  input.addEventListener('change', async () => {
+    const file = input.files?.[0]
 
-  if (event.dataTransfer.items.length !== 1) {
-    return false
-  }
+    if (!file) {
+      return
+    }
 
-  if (event.dataTransfer.items[0].kind !== 'file') {
-    return false
-  }
+    const backupImage = playlist.value.cover
 
-  return acceptedImageTypes.includes(event.dataTransfer.items[0].getAsFile()!.type)
+    try {
+      useFileReader().readAsDataUrl(file, async url => {
+        playlist.value!.cover = url
+        await playlistStore.uploadCover(playlist.value, url)
+        toastSuccess('Playlist cover updated.')
+      })
+    } catch (error: unknown) {
+      // restore the backup image
+      playlist.value.cover = backupImage
+      useErrorHandler().handleHttpError(error)
+    }
+  })
+
+  input.dispatchEvent(new MouseEvent('click'))
 }
 
-const onDrop = async (event: DragEvent) => {
-  droppable.value = false
-
-  if (!allowsUpload.value) {
-    return
-  }
-
-  if (!validImageDropEvent(event)) {
-    return
-  }
-
-  const backupImage = playlist.value.cover
-
-  try {
-    useFileReader().readAsDataUrl(event.dataTransfer!.files[0], async url => {
-      // Replace the image right away to create an "instant" effect
-      playlist.value.cover = url
-      await playlistStore.uploadCover(playlist.value, url)
-    })
-  } catch (error: unknown) {
-    // restore the backup image
-    playlist.value.cover = backupImage
-    useErrorHandler().handleHttpError(error)
-  }
-}
+const removeCover = async () => await playlistStore.removeCover(playlist.value)
 </script>
 
 <style lang="postcss" scoped>
 article {
   background-image: v-bind(backgroundImage);
 
-  &.droppable {
-    @apply border-2 border-dotted border-white brightness-50;
-  }
-
   .thumbnail-stack {
     @apply pointer-events-none;
   }

resources/assets/js/stores/playlistStore.ts

@@ -195,5 +195,10 @@ export const playlistStore = {
     this.byId(playlist.id)!.cover = playlist.cover
 
     return playlist.cover
+  },
+
+  async removeCover (playlist: Playlist) {
+    playlist.cover = null
+    await http.delete(`playlists/${playlist.id}/cover`)
   }
 }

routes/api.base.php

@@ -31,6 +31,7 @@ use App\Http\Controllers\API\PlaylistCollaboration\AcceptPlaylistCollaborationIn
 use App\Http\Controllers\API\PlaylistCollaboration\CreatePlaylistCollaborationTokenController;
 use App\Http\Controllers\API\PlaylistCollaboration\PlaylistCollaboratorController;
 use App\Http\Controllers\API\PlaylistController;
+use App\Http\Controllers\API\PlaylistCoverController;
 use App\Http\Controllers\API\PlaylistFolderController;
 use App\Http\Controllers\API\PlaylistFolderPlaylistController;
 use App\Http\Controllers\API\PlaylistSongController;
@@ -57,7 +58,6 @@ use App\Http\Controllers\API\UpdateUserPreferenceController;
 use App\Http\Controllers\API\UploadAlbumCoverController;
 use App\Http\Controllers\API\UploadArtistImageController;
 use App\Http\Controllers\API\UploadController;
-use App\Http\Controllers\API\UploadPlaylistCoverController;
 use App\Http\Controllers\API\UserController;
 use App\Http\Controllers\API\UserInvitationController;
 use App\Models\Song;
@@ -174,7 +174,8 @@ Route::prefix('api')->middleware('api')->group(static function (): void {
         Route::put('albums/{album}/cover', UploadAlbumCoverController::class);
         Route::get('albums/{album}/thumbnail', FetchAlbumThumbnailController::class);
         Route::put('artists/{artist}/image', UploadArtistImageController::class);
-        Route::put('playlists/{playlist}/cover', UploadPlaylistCoverController::class);
+        Route::put('playlists/{playlist}/cover', [PlaylistCoverController::class, 'update']);
+        Route::delete('playlists/{playlist}/cover', [PlaylistCoverController::class, 'destroy']);
         // deprecated routes
         Route::put('album/{album}/cover', UploadAlbumCoverController::class);
         Route::get('album/{album}/thumbnail', FetchAlbumThumbnailController::class);

tests/Feature/KoelPlus/PlaylistCoverTest.php

@@ -19,4 +19,15 @@ class PlaylistCoverTest extends PlusTestCase
         $this->putAs("api/playlists/$playlist->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], $collaborator)
             ->assertForbidden();
     }
+
+    public function testCollaboratorCannotDeleteCover(): void
+    {
+        /** @var Playlist $playlist */
+        $playlist = Playlist::factory()->create();
+        $collaborator = create_user();
+        $playlist->addCollaborator($collaborator);
+
+        $this->deleteAs("api/playlists/$playlist->id/cover", [], $collaborator)
+            ->assertForbidden();
+    }
 }

tests/Feature/PlaylistCoverTest.php

@@ -3,45 +3,54 @@
 namespace Tests\Feature;
 
 use App\Models\Playlist;
-use App\Services\MediaMetadataService;
-use Mockery;
-use Mockery\MockInterface;
 use Tests\TestCase;
 
 use function Tests\create_user;
+use function Tests\read_as_data_url;
+use function Tests\test_path;
 
 class PlaylistCoverTest extends TestCase
 {
-    private MockInterface|MediaMetadataService $mediaMetadataService;
-
-    public function setUp(): void
-    {
-        parent::setUp();
-
-        $this->mediaMetadataService = self::mock(MediaMetadataService::class);
-    }
-
     public function testUploadCover(): void
     {
         $playlist = Playlist::factory()->create();
         self::assertNull($playlist->cover);
 
-        $this->mediaMetadataService
-            ->shouldReceive('writePlaylistCover')
-            ->once()
-            ->with(Mockery::on(static fn (Playlist $target) => $target->is($playlist)), 'data:image/jpeg;base64,Rm9v');
-
-        $this->putAs("api/playlists/$playlist->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], $playlist->user)
+        $this->putAs(
+            "api/playlists/$playlist->id/cover",
+            ['cover' => read_as_data_url(test_path('blobs/cover.png'))],
+            $playlist->user
+        )
             ->assertOk();
+
+        self::assertNotNull($playlist->refresh()->cover);
     }
 
     public function testUploadCoverNotAllowedForNonOwner(): void
     {
         $playlist = Playlist::factory()->create();
 
-        $this->mediaMetadataService->shouldNotReceive('writePlaylistCover');
-
         $this->putAs("api/playlists/$playlist->id/cover", ['cover' => 'data:image/jpeg;base64,Rm9v'], create_user())
             ->assertForbidden();
     }
+
+    public function testDeleteCover(): void
+    {
+        $playlist = Playlist::factory()->create(['cover' => 'cover.jpg']);
+
+        $this->deleteAs("api/playlists/$playlist->id/cover", [], $playlist->user)
+            ->assertNoContent();
+
+        self::assertNull($playlist->refresh()->cover);
+    }
+
+    public function testNonOwnerCannotDeleteCover(): void
+    {
+        $playlist = Playlist::factory()->create(['cover' => 'cover.jpg']);
+
+        $this->deleteAs("api/playlists/$playlist->id/cover", [], create_user())
+            ->assertForbidden();
+
+        self::assertSame('cover.jpg', $playlist->refresh()->getRawOriginal('cover'));
+    }
 }