feat: make song edit/deletion plus-aware

2024-11-24 13:13:05 +00:00 · 2024-01-06 12:31:50 +01:00 · 2024-01-06 12:31:50 +01:00 · a8c78adf65
commit a8c78adf65
parent acc3374ee2
24 changed files with 97 additions and 51 deletions
--- a/app/Helpers.php
+++ b/app/Helpers.php
@ -71,3 +71,10 @@ function attempt_unless($condition, callable $callback, bool $log = true): mixed
 {
    return !value($condition) ? attempt($callback, $log) : null;
 }
+
+if (!function_exists('test_path')) {
+    function test_path(string $path = ''): string
+    {
+        return base_path('tests' . DIRECTORY_SEPARATOR . ltrim($path, DIRECTORY_SEPARATOR));
+    }
+}
--- a/app/Http/Controllers/API/SettingController.php
+++ b/app/Http/Controllers/API/SettingController.php
@ -7,6 +7,7 @@ use App\Http\Requests\API\SettingRequest;
 use App\Models\Setting;
 use App\Models\User;
 use App\Services\MediaScanner;
+use App\Values\ScanConfiguration;
 use Illuminate\Contracts\Auth\Authenticatable;

 class SettingController extends Controller
@ -22,7 +23,7 @@ class SettingController extends Controller

        Setting::set('media_path', rtrim(trim($request->media_path), '/'));

-        $this->mediaSyncService->scan($this->user, makePublic: true);
+        $this->mediaSyncService->scan(ScanConfiguration::make(owner: $this->user, makePublic: true));

        return response()->noContent();
    }
--- a/app/Http/Controllers/API/SongController.php
+++ b/app/Http/Controllers/API/SongController.php
@ -50,8 +50,6 @@ class SongController extends Controller

    public function update(SongUpdateRequest $request)
    {
-        $this->authorize('admin', $this->user);
-
        $updatedSongs = $this->songService->updateSongs($request->songs, SongUpdateData::fromRequest($request));
        $albums = $this->albumRepository->getMany($updatedSongs->pluck('album_id')->toArray());

@ -72,8 +70,6 @@ class SongController extends Controller

    public function destroy(DeleteSongsRequest $request)
    {
-        $this->authorize('admin', $this->user);
-
        $this->songService->deleteSongs($request->songs);

        return response()->noContent();
--- a/app/Http/Requests/API/DeleteSongsRequest.php
+++ b/app/Http/Requests/API/DeleteSongsRequest.php
@ -2,6 +2,9 @@

 namespace App\Http\Requests\API;

+use App\Facades\License;
+use App\Models\Song;
+
 /** @property-read array<string> $songs */
 class DeleteSongsRequest extends Request
 {
@ -12,4 +15,16 @@ class DeleteSongsRequest extends Request
            'songs' => 'required|array|exists:songs,id',
        ];
    }
+
+    public function authorize(): bool
+    {
+        if (License::isCommunity()) {
+            return $this->user()->is_admin;
+        }
+
+        return Song::query()
+            ->whereIn('id', $this->songs)
+            ->get()
+            ->every(fn (Song $song): bool => $song->owner_id === $this->user()->id);
+    }
 }
--- a/app/Http/Requests/API/SongUpdateRequest.php
+++ b/app/Http/Requests/API/SongUpdateRequest.php
@ -2,6 +2,9 @@

 namespace App\Http\Requests\API;

+use App\Facades\License;
+use App\Models\Song;
+
 /**
 * @property array<string> $songs
 * @property array<mixed> $data
@ -13,7 +16,19 @@ class SongUpdateRequest extends Request
    {
        return [
            'data' => 'required|array',
-            'songs' => 'required|array',
+            'songs' => 'required|array|exists:songs,id',
        ];
    }
+
+    public function authorize(): bool
+    {
+        if (License::isCommunity()) {
+            return $this->user()->is_admin;
+        }
+
+        return Song::query()
+            ->whereIn('id', $this->songs)
+            ->get()
+            ->every(fn (Song $song): bool => $song->owner_id === $this->user()->id);
+    }
 }
--- a/app/Services/CommunityLicenseService.php
+++ b/app/Services/CommunityLicenseService.php
@ -2,8 +2,15 @@

 namespace App\Services;

+use App\Services\ApiClients\LemonSqueezyApiClient;
+
 class CommunityLicenseService extends LicenseService
 {
+    public function __construct(LemonSqueezyApiClient $client)
+    {
+        parent::__construct($client, config('app.key'));
+    }
+
    public function isPlus(): bool
    {
        return false;
--- a/app/Services/FileScanner.php
+++ b/app/Services/FileScanner.php
@ -48,7 +48,7 @@ class FileScanner
        return $this;
    }

-    public function getFileScanInformation(): ?SongScanInformation
+    public function getScanInformation(): ?SongScanInformation
    {
        $raw = $this->getID3->analyze($this->filePath);
        $this->syncError = Arr::get($raw, 'error.0') ?: (Arr::get($raw, 'playtime_seconds') ? null : 'Empty file');
@ -71,7 +71,7 @@ class FileScanner
            return ScanResult::skipped($this->filePath);
        }

-        $info = $this->getFileScanInformation()?->toArray();
+        $info = $this->getScanInformation()?->toArray();

        if (!$info) {
            return ScanResult::error($this->filePath, $this->syncError);
--- a/resources/assets/js/components/song/SongContextMenu.vue
+++ b/resources/assets/js/components/song/SongContextMenu.vue
@ -42,7 +42,7 @@
      <li class="separator" />
    </template>

-    <li v-if="isAdmin" @click="openEditForm">Edit…</li>
+    <li v-if="canModify" @click="openEditForm">Edit…</li>
    <li v-if="allowsDownload" @click="download">Download</li>
    <li v-if="onlyOneSongSelected" @click="copyUrl">Copy Shareable URL</li>

@ -51,7 +51,7 @@
      <li @click="removeFromPlaylist">Remove from Playlist</li>
    </template>

-    <template v-if="isAdmin">
+    <template v-if="canModify">
      <li class="separator" />
      <li @click="deleteFromFilesystem">Delete from Filesystem</li>
    </template>
@ -67,6 +67,7 @@ import {
  useAuthorization,
  useContextMenu,
  useDialogBox,
+  useLicense,
  useMessageToaster,
  usePlaylistManagement,
  useRouter,
@ -76,9 +77,10 @@ import {
 const { toastSuccess } = useMessageToaster()
 const { showConfirmDialog } = useDialogBox()
 const { go, getRouteParam, isCurrentScreen } = useRouter()
-const { isAdmin } = useAuthorization()
+const { isAdmin, currentUser } = useAuthorization()
 const { base, ContextMenuBase, open, close, trigger } = useContextMenu()
 const { removeSongsFromPlaylist } = usePlaylistManagement()
+const { isKoelPlus } = useLicense()

 const songs = ref<Song[]>([])

@ -96,6 +98,11 @@ const allowsDownload = toRef(commonStore.state, 'allows_download')
 const queue = toRef(queueStore.state, 'songs')
 const currentSong = toRef(queueStore, 'current')

+const canModify = computed(() => {
+  if (isKoelPlus.value) return songs.value.every(song => song.owner_id === currentUser.value?.id)
+  return isAdmin.value
+})
+
 const onlyOneSongSelected = computed(() => songs.value.length === 1)
 const firstSongPlaying = computed(() => songs.value.length ? songs.value[0].playback_state === 'Playing' : false)
 const normalPlaylists = computed(() => playlists.value.filter(playlist => !playlist.is_smart))
--- a/tests/Feature/SettingTest.php
+++ b/tests/Feature/SettingTest.php
@ -24,7 +24,7 @@ class SettingTest extends TestCase
        /** @var User $admin */
        $admin = User::factory()->admin()->create();

-        $this->mediaSyncService->shouldReceive('sync')->once()
+        $this->mediaSyncService->shouldReceive('scan')->once()
            ->andReturn(ScanResultCollection::create());

        $this->putAs('/api/settings', ['media_path' => __DIR__], $admin)
--- a/tests/Feature/UploadTest.php
+++ b/tests/Feature/UploadTest.php
@ -19,7 +19,7 @@ class UploadTest extends TestCase
    {
        parent::setUp();

-        $this->file = UploadedFile::fromFile(__DIR__ . '/../songs/full.mp3', 'song.mp3');
+        $this->file = UploadedFile::fromFile(test_path('songs/full.mp3'), 'song.mp3');
    }

    public function testUnauthorizedPost(): void
--- a/tests/Integration/Models/SongZipArchiveTest.php
+++ b/tests/Integration/Models/SongZipArchiveTest.php
@ -11,7 +11,7 @@ class SongZipArchiveTest extends TestCase
    public function testAddSongIntoArchive(): void
    {
        /** @var Song $song */
-        $song = Song::factory()->create(['path' => realpath(__DIR__ . '/../../songs/full.mp3')]);
+        $song = Song::factory()->create(['path' => test_path('songs/full.mp3')]);

        $songZipArchive = new SongZipArchive();
        $songZipArchive->addSong($song);
@ -23,8 +23,8 @@ class SongZipArchiveTest extends TestCase
    public function testAddMultipleSongsIntoArchive(): void
    {
        $songs = collect([
-            Song::factory()->create(['path' => realpath(__DIR__ . '/../../songs/full.mp3')]),
-            Song::factory()->create(['path' => realpath(__DIR__ . '/../../songs/lorem.mp3')]),
+            Song::factory()->create(['path' => test_path('songs/full.mp3')]),
+            Song::factory()->create(['path' => test_path('songs/lorem.mp3')]),
        ]);

        $songZipArchive = new SongZipArchive();
--- a/tests/Integration/Services/ApplicationInformationServiceTest.php
+++ b/tests/Integration/Services/ApplicationInformationServiceTest.php
@ -18,7 +18,7 @@ class ApplicationInformationServiceTest extends TestCase
        $latestVersion = 'v1.1.2';

        $mock = new MockHandler([
-            new Response(200, [], File::get(__DIR__ . '../../../blobs/github-tags.json')),
+            new Response(200, [], File::get(test_path('blobs/github-tags.json'))),
        ]);

        $client = new Client(['handler' => HandlerStack::create($mock)]);
--- a/tests/Integration/Services/FileScannerTest.php
+++ b/tests/Integration/Services/FileScannerTest.php
@ -20,7 +20,7 @@ class FileScannerTest extends TestCase

    public function testGetFileInfo(): void
    {
-        $info = $this->scanner->setFile(__DIR__ . '/../../songs/full.mp3')->getFileScanInformation();
+        $info = $this->scanner->setFile(test_path('songs/full.mp3'))->getScanInformation();

        $expectedData = [
            'artist' => 'Koel',
@ -30,7 +30,7 @@ class FileScannerTest extends TestCase
            'disc' => 3,
            'lyrics' => "Foo\rbar",
            'cover' => [
-                'data' => File::get(__DIR__ . '/../../blobs/cover.png'),
+                'data' => File::get(test_path('blobs/cover.png')),
                'image_mime' => 'image/png',
                'image_width' => 512,
                'image_height' => 512,
@ -39,8 +39,8 @@ class FileScannerTest extends TestCase
                'description' => '',
                'datalength' => 7627,
            ],
-            'path' => realpath(__DIR__ . '/../../songs/full.mp3'),
-            'mtime' => filemtime(__DIR__ . '/../../songs/full.mp3'),
+            'path' => test_path('songs/full.mp3'),
+            'mtime' => filemtime(test_path('songs/full.mp3')),
            'albumartist' => '',
        ];

@ -50,8 +50,8 @@ class FileScannerTest extends TestCase

    public function testGetFileInfoVorbisCommentsFlac(): void
    {
-        $flacPath = __DIR__ . '/../../songs/full-vorbis-comments.flac';
-        $info = $this->scanner->setFile($flacPath)->getFileScanInformation();
+        $flacPath = test_path('songs/full-vorbis-comments.flac');
+        $info = $this->scanner->setFile($flacPath)->getScanInformation();

        $expectedData = [
            'artist' => 'Koel',
@ -62,7 +62,7 @@ class FileScannerTest extends TestCase
            'disc' => 3,
            'lyrics' => "Foo\r\nbar",
            'cover' => [
-                'data' => File::get(__DIR__ . '/../../blobs/cover.png'),
+                'data' => File::get(test_path('blobs/cover.png')),
                'image_mime' => 'image/png',
                'image_width' => 512,
                'image_height' => 512,
@ -79,9 +79,9 @@ class FileScannerTest extends TestCase

    public function testSongWithoutTitleHasFileNameAsTitle(): void
    {
-        $this->scanner->setFile(__DIR__ . '/../../songs/blank.mp3');
+        $this->scanner->setFile(test_path('songs/blank.mp3'));

-        self::assertSame('blank', $this->scanner->getFileScanInformation()->title);
+        self::assertSame('blank', $this->scanner->getScanInformation()->title);
    }

    public function testIgnoreLrcFileIfEmbeddedLyricsAvailable(): void
@ -89,10 +89,10 @@ class FileScannerTest extends TestCase
        $base = sys_get_temp_dir() . '/' . Str::uuid();
        $mediaFile = $base . '.mp3';
        $lrcFile = $base . '.lrc';
-        copy(__DIR__ . '/../../songs/full.mp3', $mediaFile);
-        copy(__DIR__ . '/../../blobs/simple.lrc', $lrcFile);
+        File::copy(test_path('songs/full.mp3'), $mediaFile);
+        File::copy(test_path('blobs/simple.lrc'), $lrcFile);

-        self::assertSame("Foo\rbar", $this->scanner->setFile($mediaFile)->getFileScanInformation()->lyrics);
+        self::assertSame("Foo\rbar", $this->scanner->setFile($mediaFile)->getScanInformation()->lyrics);
    }

    public function testReadLrcFileIfEmbeddedLyricsNotAvailable(): void
@ -100,10 +100,10 @@ class FileScannerTest extends TestCase
        $base = sys_get_temp_dir() . '/' . Str::uuid();
        $mediaFile = $base . '.mp3';
        $lrcFile = $base . '.lrc';
-        copy(__DIR__ . '/../../songs/blank.mp3', $mediaFile);
-        copy(__DIR__ . '/../../blobs/simple.lrc', $lrcFile);
+        File::copy(test_path('songs/blank.mp3'), $mediaFile);
+        File::copy(test_path('blobs/simple.lrc'), $lrcFile);

-        $info = $this->scanner->setFile($mediaFile)->getFileScanInformation();
+        $info = $this->scanner->setFile($mediaFile)->getScanInformation();

        self::assertSame("Line 1\nLine 2\nLine 3", $info->lyrics);
    }
--- a/tests/Integration/Services/MediaMetadataServiceTest.php
+++ b/tests/Integration/Services/MediaMetadataServiceTest.php
@ -18,7 +18,7 @@ class MediaMetadataServiceTest extends TestCase

    public function testGetAlbumThumbnailUrl(): void
    {
-        File::copy(__DIR__ . '/../../blobs/cover.png', album_cover_path('album-cover-for-thumbnail-test.jpg'));
+        File::copy(test_path('blobs/cover.png'), album_cover_path('album-cover-for-thumbnail-test.jpg'));

        /** @var Album $album */
        $album = Album::factory()->create(['cover' => 'album-cover-for-thumbnail-test.jpg']);
--- a/tests/Integration/Services/MediaScannerTest.php
+++ b/tests/Integration/Services/MediaScannerTest.php
@ -281,7 +281,7 @@ class MediaScannerTest extends TestCase

        /** @var FileScanner $fileScanner */
        $fileScanner = app(FileScanner::class);
-        $info = $fileScanner->setFile($path)->getFileScanInformation();
+        $info = $fileScanner->setFile($path)->getScanInformation();

        self::assertSame('佐倉綾音 Unknown', $info->artistName);
        self::assertSame('小岩井こ Random', $info->albumName);
--- a/tests/Integration/Services/UploadServiceTest.php
+++ b/tests/Integration/Services/UploadServiceTest.php
@ -51,7 +51,7 @@ class UploadServiceTest extends TestCase
        /** @var User $user */
        $user = User::factory()->create();

-        $song = $this->service->handleUploadedFile(UploadedFile::fromFile(__DIR__ . '/../../songs/full.mp3'), $user);
+        $song = $this->service->handleUploadedFile(UploadedFile::fromFile(test_path('songs/full.mp3')), $user);

        self::assertSame($song->owner_id, $user->id);
        self::assertSame(public_path("sandbox/media/__KOEL_UPLOADS_\${$user->id}__/full.mp3"), $song->path);
--- a/tests/Traits/CreatesApplication.php
+++ b/tests/Traits/CreatesApplication.php
@ -10,18 +10,18 @@ use Illuminate\Support\Facades\DB;

 trait CreatesApplication
 {
-    protected string $mediaPath = __DIR__ . '/../songs';
+    protected string $mediaPath;
    private Kernel $artisan;
    protected string $baseUrl = 'http://localhost';
    public static bool $migrated = false;

    public function createApplication(): Application
    {
-        $this->mediaPath = realpath($this->mediaPath);
-
        /** @var Application $app */
        $app = require __DIR__ . '/../../bootstrap/app.php';

+        $this->mediaPath = test_path('songs');
+
        /** @var Kernel $artisan */
        $artisan = $app->make(Artisan::class);

--- a/tests/Unit/Listeners/WriteSyncLogTest.php
+++ b/tests/Unit/Listeners/WriteSyncLogTest.php
@ -40,7 +40,7 @@ class WriteSyncLogTest extends TestCase

        self::assertStringEqualsFile(
            storage_path('logs/sync-20210102-123456.log'),
-            file_get_contents(__DIR__ . '/../../blobs/sync-log-all.log')
+            File::get(test_path('blobs/sync-log-all.log'))
        );
    }

@ -52,7 +52,7 @@ class WriteSyncLogTest extends TestCase

        self::assertStringEqualsFile(
            storage_path('logs/sync-20210102-123456.log'),
-            file_get_contents(__DIR__ . '/../../blobs/sync-log-error.log')
+            File::get(test_path('blobs/sync-log-error.log'))
        );
    }

--- a/tests/Unit/Models/ArtistTest.php
+++ b/tests/Unit/Models/ArtistTest.php
@ -41,7 +41,7 @@ class ArtistTest extends TestCase

    public function testArtistsWithNameInUtf16EncodingAreRetrievedCorrectly(): void
    {
-        $name = File::get(__DIR__ . '../../../blobs/utf16');
+        $name = File::get(test_path('blobs/utf16'));
        $artist = Artist::getOrCreate($name);

        self::assertTrue(Artist::getOrCreate($name)->is($artist));
--- a/tests/Unit/Services/ApiClients/LastfmClientTest.php
+++ b/tests/Unit/Services/ApiClients/LastfmClientTest.php
@ -14,9 +14,7 @@ class LastfmClientTest extends TestCase
 {
    public function testGetSessionKey(): void
    {
-        $mock = new MockHandler([
-            new Response(200, [], File::get(__DIR__ . '/../../../blobs/lastfm/session-key.json')),
-        ]);
+        $mock = new MockHandler([new Response(200, [], File::get(test_path('blobs/lastfm/session-key.json')))]);

        $client = new LastfmClient(new GuzzleHttpClient(['handler' => HandlerStack::create($mock)]));

--- a/tests/Unit/Services/LastfmServiceTest.php
+++ b/tests/Unit/Services/LastfmServiceTest.php
@ -40,7 +40,7 @@ class LastfmServiceTest extends TestCase
        $this->client->shouldReceive('get')
            ->with('?method=artist.getInfo&autocorrect=1&artist=foo&format=json')
            ->once()
-            ->andReturn(json_decode(File::get(__DIR__ . '/../../blobs/lastfm/artist.json')));
+            ->andReturn(json_decode(File::get(test_path('blobs/lastfm/artist.json'))));

        $info = $this->service->getArtistInformation($artist);

@ -62,7 +62,7 @@ class LastfmServiceTest extends TestCase
        $this->client->shouldReceive('get')
            ->with('?method=artist.getInfo&autocorrect=1&artist=bar&format=json')
            ->once()
-            ->andReturn(json_decode(File::get(__DIR__ . '/../../blobs/lastfm/artist-notfound.json')));
+            ->andReturn(json_decode(test_path('blobs/lastfm/artist-notfound.json')));

        self::assertNull($this->service->getArtistInformation($artist));
    }
@ -75,7 +75,7 @@ class LastfmServiceTest extends TestCase
        $this->client->shouldReceive('get')
            ->with('?method=album.getInfo&autocorrect=1&album=foo&artist=bar&format=json')
            ->once()
-            ->andReturn(json_decode(File::get(__DIR__ . '/../../blobs/lastfm/album.json')));
+            ->andReturn(json_decode(File::get(test_path('blobs/lastfm/album.json'))));

        $info = $this->service->getAlbumInformation($album);

@ -109,7 +109,7 @@ class LastfmServiceTest extends TestCase
        $this->client->shouldReceive('get')
            ->with('?method=album.getInfo&autocorrect=1&album=foo&artist=bar&format=json')
            ->once()
-            ->andReturn(json_decode(File::get(__DIR__ . '/../../blobs/lastfm/album-notfound.json')));
+            ->andReturn(json_decode(File::get(test_path('blobs/lastfm/album-notfound.json'))));

        self::assertNull($this->service->getAlbumInformation($album));
    }
--- a/tests/Unit/Services/SimpleLrcReaderTest.php
+++ b/tests/Unit/Services/SimpleLrcReaderTest.php
@ -23,7 +23,7 @@ class SimpleLrcReaderTest extends TestCase
        $base = sys_get_temp_dir() . '/' . Str::uuid();
        $lrcFile = $base . '.lrc';

-        File::copy(__DIR__ . '/../../blobs/simple.lrc', $lrcFile);
+        File::copy(test_path('blobs/simple.lrc'), $lrcFile);

        self::assertSame("Line 1\nLine 2\nLine 3", $this->reader->tryReadForMediaFile($base . '.mp3'));
        File::delete($lrcFile);
--- a/tests/Unit/Services/SpotifyServiceTest.php
+++ b/tests/Unit/Services/SpotifyServiceTest.php
@ -77,7 +77,7 @@ class SpotifyServiceTest extends TestCase
    /** @return array<mixed> */
    private static function parseFixture(string $name): array
    {
-        return json_decode(File::get(__DIR__ . '/../../blobs/spotify/' . $name), true);
+        return json_decode(File::get(test_path("blobs/spotify/$name")), true);
    }

    protected function tearDown(): void
--- a/tests/Unit/Services/YouTubeServiceTest.php
+++ b/tests/Unit/Services/YouTubeServiceTest.php
@ -21,7 +21,7 @@ class YouTubeServiceTest extends TestCase

        $client->shouldReceive('get')
            ->with('search?part=snippet&type=video&maxResults=10&pageToken=my-token&q=Foo+Bar')
-            ->andReturn(json_decode(File::get(__DIR__ . '/../../blobs/youtube/search.json')));
+            ->andReturn(json_decode(File::get(test_path('blobs/youtube/search.json'))));

        $service = new YouTubeService($client, app(Repository::class));
        $response = $service->searchVideosRelatedToSong($song, 'my-token');