chore: some cleanups

app/Http/Controllers/API/ScrobbleController.php

@@ -14,7 +14,7 @@ class ScrobbleController extends Controller
     /** @param User $user */
     public function __invoke(ScrobbleRequest $request, Song $song, Authenticatable $user)
     {
-        if (!$song->artist->is_unknown && $user->connectedToLastfm()) {
+        if (!$song->artist->is_unknown && $user->connected_to_lastfm) {
             ScrobbleJob::dispatch($user, $song, $request->timestamp);
         }
 

app/Models/Playlist.php

@@ -4,6 +4,7 @@ namespace App\Models;
 
 use App\Casts\SmartPlaylistRulesCast;
 use App\Facades\License as LicenseFacade;
+use App\Models\Song as Playable;
 use App\Values\SmartPlaylistRuleGroupCollection;
 use Carbon\Carbon;
 use Illuminate\Database\Eloquent\Casts\Attribute;
@@ -16,7 +17,6 @@ use Illuminate\Support\Arr;
 use Illuminate\Support\Collection;
 use Illuminate\Support\Str;
 use Laravel\Scout\Searchable;
-use LogicException;
 
 /**
  * @property string $id
@@ -24,8 +24,7 @@ use LogicException;
  * @property bool $is_smart
  * @property int $user_id
  * @property User $user
- * @property Collection<array-key, Song> $songs
+ * @property Collection<array-key, Playable> $playables
- * @property array<string> $song_ids
  * @property ?SmartPlaylistRuleGroupCollection $rule_groups
  * @property ?SmartPlaylistRuleGroupCollection $rules
  * @property Carbon $created_at
@@ -61,9 +60,9 @@ class Playlist extends Model
         });
     }
 
-    public function songs(): BelongsToMany
+    public function playables(): BelongsToMany
     {
-        return $this->belongsToMany(Song::class)
+        return $this->belongsToMany(Playable::class)
             ->withTimestamps()
             ->withPivot('position')
             ->orderByPivot('position');
@@ -100,13 +99,6 @@ class Playlist extends Model
         return Attribute::get(fn () => $this->rules);
     }
 
-    public function songIds(): Attribute
-    {
-        throw_if($this->is_smart, new LogicException('Smart playlist contents are generated dynamically.'));
-
-        return Attribute::get(fn () => $this->songs->pluck('id')->all());
-    }
-
     protected function cover(): Attribute
     {
         return Attribute::get(static fn (?string $value): ?string => playlist_cover_url($value));
@@ -156,39 +148,39 @@ class Playlist extends Model
     }
 
     /**
-     * @param Collection|array<array-key, Song>|Song|array<string> $songs
+     * @param Collection|array<array-key, Playable>|Playable|array<string> $playables
      */
-    public function addPlayables(Collection|Song|array $songs, ?User $collaborator = null): void
+    public function addPlayables(Collection|Playable|array $playables, ?User $collaborator = null): void
     {
         $collaborator ??= $this->user;
-        $maxPosition = $this->songs()->getQuery()->max('position') ?? 0;
+        $maxPosition = $this->playables()->getQuery()->max('position') ?? 0;
 
-        if (!is_array($songs)) {
+        if (!is_array($playables)) {
-            $songs = Collection::wrap($songs)->pluck('id')->all();
+            $playables = Collection::wrap($playables)->pluck('id')->all();
         }
 
         $data = [];
 
-        foreach ($songs as $song) {
+        foreach ($playables as $playable) {
-            $data[$song] = [
+            $data[$playable] = [
                 'position' => ++$maxPosition,
                 'user_id' => $collaborator->id,
             ];
         }
 
-        $this->songs()->attach($data);
+        $this->playables()->attach($data);
     }
 
     /**
-     * @param Collection<array-key, Song>|Song|array<string> $songs
+     * @param Collection<array-key, Playable>|Playable|array<string> $playables
      */
-    public function removePlayables(Collection|Song|array $songs): void
+    public function removePlayables(Collection|Playable|array $playables): void
     {
-        if (!is_array($songs)) {
+        if (!is_array($playables)) {
-            $songs = Collection::wrap($songs)->pluck('id')->all();
+            $playables = Collection::wrap($playables)->pluck('id')->all();
         }
 
-        $this->songs()->detach($songs);
+        $this->playables()->detach($playables);
     }
 
     protected function isCollaborative(): Attribute

app/Models/User.php

@@ -20,27 +20,28 @@ use Laravel\Sanctum\HasApiTokens;
 use Laravel\Sanctum\PersonalAccessToken;
 
 /**
- * @property UserPreferences $preferences
- * @property int $id
- * @property bool $is_admin
- * @property string $name
- * @property string $email
- * @property string $password
- * @property-read bool $has_custom_avatar
- * @property-read string $avatar
- * @property Collection<array-key, Playlist> $playlists
- * @property Collection<array-key, PlaylistFolder> $playlist_folders
- * @property PersonalAccessToken $currentAccessToken
  * @property ?Carbon $invitation_accepted_at
+ * @property ?Carbon $invited_at
  * @property ?User $invitedBy
  * @property ?string $invitation_token
- * @property ?Carbon $invited_at
- * @property-read bool $is_prospect
  * @property Collection<array-key, Playlist> $collaboratedPlaylists
- * @property ?string $sso_provider
+ * @property Collection<array-key, Playlist> $playlists
- * @property ?string $sso_id
+ * @property Collection<array-key, PlaylistFolder> $playlist_folders
- * @property bool $is_sso
  * @property Collection<array-key, Podcast> $podcasts
+ * @property PersonalAccessToken $currentAccessToken
+ * @property UserPreferences $preferences
+ * @property bool $is_admin
+ * @property int $id
+ * @property string $email
+ * @property string $name
+ * @property string $password
+ * @property-read ?string $sso_id
+ * @property-read ?string $sso_provider
+ * @property-read bool $connected_to_lastfm Whether the user is connected to Last.fm
+ * @property-read bool $has_custom_avatar
+ * @property-read bool $is_prospect
+ * @property-read bool $is_sso
+ * @property-read string $avatar
  */
 class User extends Authenticatable
 {
@@ -126,11 +127,8 @@ class User extends Authenticatable
         return Attribute::get(fn (): bool => License::isPlus() && $this->sso_provider);
     }
 
-    /**
+    protected function connectedToLastfm(): Attribute
-     * Determine if the user is connected to Last.fm.
-     */
-    public function connectedToLastfm(): bool
     {
-        return (bool) $this->preferences->lastFmSessionKey;
+        return Attribute::get(fn (): bool => (bool) $this->preferences->lastFmSessionKey);
     }
 }

app/Services/PlaylistCollaborationService.php

@@ -65,7 +65,7 @@ class PlaylistCollaborationService
 
         DB::transaction(static function () use ($playlist, $user): void {
             $playlist->collaborators()->detach($user);
-            $playlist->songs()->wherePivot('user_id', $user->id)->detach();
+            $playlist->playables()->wherePivot('user_id', $user->id)->detach();
         });
     }
 }

app/Services/PlaylistService.php

@@ -95,7 +95,7 @@ class PlaylistService
             $playables = Collection::wrap($playables);
 
             $playlist->addPlayables(
-                $playables->filter(static fn ($song): bool => !$playlist->songs->contains($song)),
+                $playables->filter(static fn ($song): bool => !$playlist->playables->contains($song)),
                 $user
             );
 
@@ -119,7 +119,7 @@ class PlaylistService
 
     public function makePlaylistContentPublic(Playlist $playlist): void
     {
-        $playlist->songs()->where('is_public', false)->update(['is_public' => true]);
+        $playlist->playables()->where('is_public', false)->update(['is_public' => true]);
     }
 
     public function movePlayablesInPlaylist(Playlist $playlist, array $movingIds, string $target, string $type): void
@@ -128,11 +128,11 @@ class PlaylistService
         throw_if($playlist->is_smart, OperationNotApplicableForSmartPlaylistException::class);
 
         DB::transaction(static function () use ($playlist, $movingIds, $target, $type): void {
-            $targetPosition = $playlist->songs()->wherePivot('song_id', $target)->value('position');
+            $targetPosition = $playlist->playables()->wherePivot('song_id', $target)->value('position');
             $insertPosition = $type === 'before' ? $targetPosition : $targetPosition + 1;
 
             // create a "gap" for the moving songs by incrementing the position of the songs after the target
-            $playlist->songs()
+            $playlist->playables()
                 ->newPivotQuery()
                 ->where('position', $type === 'before' ? '>=' : '>', $targetPosition)
                 ->whereNotIn('song_id', $movingIds)
@@ -144,7 +144,7 @@ class PlaylistService
                 $values[$id] = ['position' => $insertPosition++];
             }
 
-            $playlist->songs()->syncWithoutDetaching($values);
+            $playlist->playables()->syncWithoutDetaching($values);
         });
     }
 }

tests/Feature/KoelPlus/PlaylistSongTest.php

@@ -58,7 +58,7 @@ class PlaylistSongTest extends PlusTestCase
             ->assertSuccessful();
 
         $playlist->refresh();
-        $songs->each(static fn (Song $song) => self::assertTrue($playlist->songs->contains($song)));
+        $songs->each(static fn (Song $song) => self::assertTrue($playlist->playables->contains($song)));
     }
 
     public function testCollaboratorCanRemoveSongs(): void
@@ -74,6 +74,6 @@ class PlaylistSongTest extends PlusTestCase
             ->assertSuccessful();
 
         $playlist->refresh();
-        $songs->each(static fn (Song $song) => self::assertFalse($playlist->songs->contains($song)));
+        $songs->each(static fn (Song $song) => self::assertFalse($playlist->playables->contains($song)));
     }
 }

tests/Feature/PlaylistSongTest.php

@@ -68,7 +68,7 @@ class PlaylistSongTest extends TestCase
         $this->postAs("api/playlists/$playlist->id/songs", ['songs' => $songs->pluck('id')->all()], $playlist->user)
             ->assertSuccessful();
 
-        self::assertEqualsCanonicalizing($songs->pluck('id')->all(), $playlist->songs->pluck('id')->all());
+        self::assertEqualsCanonicalizing($songs->pluck('id')->all(), $playlist->playables->pluck('id')->all());
     }
 
     public function testRemoveSongsFromPlaylist(): void
@@ -83,7 +83,7 @@ class PlaylistSongTest extends TestCase
 
         $playlist->addPlayables($toRemainSongs->merge($toBeRemovedSongs));
 
-        self::assertCount(7, $playlist->songs);
+        self::assertCount(7, $playlist->playables);
 
         $this->deleteAs(
             "api/playlists/$playlist->id/songs",
@@ -94,7 +94,7 @@ class PlaylistSongTest extends TestCase
 
         $playlist->refresh();
 
-        self::assertEqualsCanonicalizing($toRemainSongs->pluck('id')->all(), $playlist->songs->pluck('id')->all());
+        self::assertEqualsCanonicalizing($toRemainSongs->pluck('id')->all(), $playlist->playables->pluck('id')->all());
     }
 
     public function testNonOwnerCannotModifyPlaylist(): void

tests/Feature/PlaylistTest.php

@@ -43,7 +43,7 @@ class PlaylistTest extends TestCase
         self::assertSame('Foo Bar', $playlist->name);
         self::assertTrue($playlist->ownedBy($user));
         self::assertNull($playlist->getFolder());
-        self::assertEqualsCanonicalizing($songs->pluck('id')->all(), $playlist->songs->pluck('id')->all());
+        self::assertEqualsCanonicalizing($songs->pluck('id')->all(), $playlist->playables->pluck('id')->all());
     }
 
     public function testCreatingSmartPlaylist(): void

tests/Feature/SongPlayTest.php

@@ -75,7 +75,7 @@ class SongPlayTest extends TestCase
             ->shouldReceive('stream')
             ->once();
 
-        $this->get("play/$song->id/1/128?t=$token->audioToken")
+        $this->get("play/$song->id/1?t=$token->audioToken")
             ->assertOk();
     }
 }

tests/Integration/Services/PlaylistServiceTest.php

@@ -50,7 +50,7 @@ class PlaylistServiceTest extends TestCase
         self::assertSame('foo', $playlist->name);
         self::assertTrue($user->is($playlist->user));
         self::assertFalse($playlist->is_smart);
-        self::assertEqualsCanonicalizing($playlist->songs->pluck('id')->all(), $songs->pluck('id')->all());
+        self::assertEqualsCanonicalizing($playlist->playables->pluck('id')->all(), $songs->pluck('id')->all());
     }
 
     public function testCreateSmartPlaylist(): void
@@ -201,9 +201,9 @@ class PlaylistServiceTest extends TestCase
         $playlist->refresh();
 
         self::assertCount(2, $addedSongs);
-        self::assertCount(5, $playlist->songs);
+        self::assertCount(5, $playlist->playables);
         self::assertEqualsCanonicalizing($addedSongs->pluck('id')->all(), $songs->pluck('id')->all());
-        $songs->each(static fn (Song $song) => self::assertTrue($playlist->songs->contains($song)));
+        $songs->each(static fn (Song $song) => self::assertTrue($playlist->playables->contains($song)));
     }
 
     public function testAddEpisodesToPlaylist(): void
@@ -222,7 +222,7 @@ class PlaylistServiceTest extends TestCase
         $playlist->refresh();
 
         self::assertCount(2, $addedEpisodes);
-        self::assertCount(5, $playlist->songs);
+        self::assertCount(5, $playlist->playables);
         self::assertEqualsCanonicalizing($addedEpisodes->pluck('id')->all(), $episodes->pluck('id')->all());
     }
 
@@ -238,7 +238,7 @@ class PlaylistServiceTest extends TestCase
         $playlist->refresh();
 
         self::assertCount(4, $addedEpisodes);
-        self::assertCount(7, $playlist->songs);
+        self::assertCount(7, $playlist->playables);
         self::assertEqualsCanonicalizing($addedEpisodes->pluck('id')->all(), $playables->pluck('id')->all());
     }
 
@@ -268,7 +268,7 @@ class PlaylistServiceTest extends TestCase
 
         $this->service->makePlaylistContentPublic($playlist);
 
-        $playlist->songs->each(static fn (Song $song) => self::assertTrue($song->is_public));
+        $playlist->playables->each(static fn (Song $song) => self::assertTrue($song->is_public));
     }
 
     public function testMoveSongsInPlaylist(): void
@@ -282,17 +282,17 @@ class PlaylistServiceTest extends TestCase
         $playlist->addPlayables($songs);
 
         $this->service->movePlayablesInPlaylist($playlist, [$ids[2], $ids[3]], $ids[0], 'after');
-        self::assertSame([$ids[0], $ids[2], $ids[3], $ids[1]], $playlist->refresh()->songs->pluck('id')->all());
+        self::assertSame([$ids[0], $ids[2], $ids[3], $ids[1]], $playlist->refresh()->playables->pluck('id')->all());
 
         $this->service->movePlayablesInPlaylist($playlist, [$ids[0]], $ids[3], 'before');
-        self::assertSame([$ids[2], $ids[0], $ids[3], $ids[1]], $playlist->refresh()->songs->pluck('id')->all());
+        self::assertSame([$ids[2], $ids[0], $ids[3], $ids[1]], $playlist->refresh()->playables->pluck('id')->all());
 
         // move to the first position
         $this->service->movePlayablesInPlaylist($playlist, [$ids[0], $ids[1]], $ids[2], 'before');
-        self::assertSame([$ids[0], $ids[1], $ids[2], $ids[3]], $playlist->refresh()->songs->pluck('id')->all());
+        self::assertSame([$ids[0], $ids[1], $ids[2], $ids[3]], $playlist->refresh()->playables->pluck('id')->all());
 
         // move to the last position
         $this->service->movePlayablesInPlaylist($playlist, [$ids[0], $ids[1]], $ids[3], 'after');
-        self::assertSame([$ids[2], $ids[3], $ids[0], $ids[1]], $playlist->refresh()->songs->pluck('id')->all());
+        self::assertSame([$ids[2], $ids[3], $ids[0], $ids[1]], $playlist->refresh()->playables->pluck('id')->all());
     }
 }