Mirrors/koel
2
0
Fork 0
mirror of https://github.com/koel/koel synced 2024-11-10 06:34:14 +00:00
Code Issues Projects Releases Packages Wiki Activity

fix: update multiple songs duplicate values (#1607)

Browse source
This commit is contained in:
Phan An 2022-11-29 13:16:43 +01:00 committed by GitHub
parent 20bded3bca
commit 5441cfb5f3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 59 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

29
app/Services/SongService.php
View file

@ -23,13 +23,24 @@ class SongService
/** @return Collection|array<array-key, Song> */
public function updateSongs(array $ids, SongUpdateData $data): Collection
{
if (count($ids) === 1) {
// If we're only updating one song, an empty non-required should be converted to the default values.
// This allows the user to clear those fields.
$data->disc = $data->disc ?: 1;
$data->track = $data->track ?: 0;
$data->lyrics = $data->lyrics ?: '';
$data->year = $data->year ?: null;
$data->genre = $data->genre ?: '';
$data->albumArtistName = $data->albumArtistName ?: $data->artistName;
}
return DB::transaction(function () use ($ids, $data): Collection {
return collect($ids)->reduce(function (Collection $updated, string $id) use ($data): Collection {
/** @var Song|null $song */
$song = Song::with('album', 'album.artist', 'artist')->find($id);
if ($song) {
$updated->push($this->updateSong($song, $data));
$updated->push($this->updateSong($song, clone $data));
}
return $updated;
@ -39,15 +50,19 @@ class SongService
private function updateSong(Song $song, SongUpdateData $data): Song
{
// for non-nullable fields, if the provided data is empty, use the existing value
$data->albumName = $data->albumName ?: $song->album->name;
$data->artistName = $data->artistName ?: $song->artist->name;
$data->albumArtistName = $data->albumArtistName ?: $song->album_artist->name;
$data->title = $data->title ?: $song->title;
$data->lyrics = $data->lyrics ?: $song->lyrics;
$data->track = $data->track ?: $song->track;
$data->disc = $data->disc ?: $song->disc;
$data->genre = $data->genre ?: $song->genre;
$data->year = $data->year ?: $song->year;
// For nullable fields, use the existing value only if the provided data is explicitly null.
// This allows us to clear those fields.
$data->albumArtistName ??= $song->album_artist->name;
$data->lyrics ??= $song->lyrics;
$data->track ??= $song->track;
$data->disc ??= $song->disc;
$data->genre ??= $song->genre;
$data->year ??= $song->year;
$albumArtist = Artist::getOrCreate($data->albumArtistName);
$artist = Artist::getOrCreate($data->artistName);

2
app/Values/SongUpdateData.php
View file

@ -31,7 +31,7 @@ final class SongUpdateData implements Arrayable
track: (int) $request->input('data.track'),
disc: (int) $request->input('data.disc'),
genre: $request->input('data.genre'),
year: (int) $request->input('data.year') ?: null,
year: (int) $request->input('data.year'),
lyrics: $request->input('data.lyrics'),
);
}

39
tests/Feature/SongTest.php
View file

@ -93,15 +93,16 @@ class SongTest extends TestCase
$this->putAs('/api/songs', [
'songs' => $songIds,
'data' => [
'title' => 'foo',
'title' => null,
'artist_name' => 'John Cena',
'album_name' => 'One by One',
'lyrics' => 'bar',
'lyrics' => null,
'track' => 9999,
],
], $user)
->assertOk();
/** @var Collection|array<array-key, Song> $songs */
$songs = Song::query()->whereIn('id', $songIds)->get();
// All of these songs must now belong to a new album and artist set
@ -112,6 +113,13 @@ class SongTest extends TestCase
self::assertSame('John Cena', $songs[0]->artist->name);
self::assertSame($songs[0]->artist_id, $songs[1]->artist_id);
self::assertSame($songs[0]->artist_id, $songs[2]->artist_id);
self::assertNotSame($songs[0]->title, $songs[1]->title);
self::assertNotSame($songs[0]->lyrics, $songs[1]->lyrics);
self::assertSame(9999, $songs[0]->track);
self::assertSame(9999, $songs[1]->track);
self::assertSame(9999, $songs[2]->track);
}
public function testMultipleUpdateCreatingNewAlbumsAndArtists(): void
@ -160,7 +168,6 @@ class SongTest extends TestCase
/** @var Song $song */
$song = Song::query()->first();
$this->putAs('/api/songs', [
'songs' => [$song->id],
'data' => [
@ -196,6 +203,32 @@ class SongTest extends TestCase
self::assertTrue($album->artist->is($albumArtist));
}
public function testUpdateSingleSongWithEmptyTrackAndDisc(): void
{
/** @var User $user */
$user = User::factory()->admin()->create();
/** @var Song $song */
$song = Song::factory()->create([
'track' => 12,
'disc' => 2,
]);
$this->putAs('/api/songs', [
'songs' => [$song->id],
'data' => [
'track' => null,
'disc' => null,
],
], $user)
->assertOk();
$song->refresh();
self::assertSame(0, $song->track);
self::assertSame(1, $song->disc);
}
public function testDeletingByChunk(): void
{
self::assertNotSame(0, Song::query()->count());