diff --git a/app/controllers/albums_controller.rb b/app/controllers/albums_controller.rb index 9c95b550..1b96cc0d 100644 --- a/app/controllers/albums_controller.rb +++ b/app/controllers/albums_controller.rb @@ -86,8 +86,18 @@ def transformed_attributes end if attributes[:album_artists].present? - attributes[:album_artists] = attributes[:album_artists].map do |aa, _i| - AlbumArtist.new(artist_id: aa[:artist_id], name: aa[:name], separator: aa[:separator], order: aa[:order] || 0) + prev_aa = @album&.album_artists.to_a + attributes[:album_artists_attributes] = attributes.delete(:album_artists).map do |aa| + existing = prev_aa.find { it.artist_id == aa[:artist_id] } + unless existing.nil? + prev_aa.delete(existing) + aa.merge!({ id: existing.id }) + end + aa + end + prev_aa.each do |aa| + attributes[:album_artists_attributes].push({ id: aa.id, _destroy: true }) + attributes[:album_artists_attributes].last.permit! end end diff --git a/app/models/album.rb b/app/models/album.rb index 1834cea5..73044058 100644 --- a/app/models/album.rb +++ b/app/models/album.rb @@ -37,6 +37,8 @@ class Album < ApplicationRecord has_many :playlists, through: :playlist_items, source: :playlist belongs_to :image, optional: true, dependent: :destroy + accepts_nested_attributes_for :album_artists, allow_destroy: true + before_validation :normalize_artist_order validates :title, presence: true diff --git a/test/controllers/albums_controller_test.rb b/test/controllers/albums_controller_test.rb index 616be92c..c0aa8cb7 100644 --- a/test/controllers/albums_controller_test.rb +++ b/test/controllers/albums_controller_test.rb @@ -130,6 +130,31 @@ class AlbumsControllerTest < ActionDispatch::IntegrationTest assert_response :created end + test 'should not create album and render errors if invalid' do + sign_in_as create(:moderator) + attributes = attributes_for(:album) + + assert_no_difference 'Album.count' do + post albums_url, params: { album: { release: attributes[:release], title: '' } } + end + + assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'model' => 'album', 'attribute' => 'title', 'type' => 'blank' } + end + + test 'should not create album and render errors if album artists are invalid' do + sign_in_as create(:moderator) + artist = create(:artist) + attributes = attributes_for(:album) + + assert_no_difference 'Album.count' do + post albums_url, params: { album: attributes.merge({ album_artists: [{ artist_id: artist.id, name: '', order: 1 }] }) } + end + + assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'model' => 'album', 'attribute' => 'album_artists.name', 'type' => 'blank' } + end + test 'should show album' do get album_url(@album) @@ -197,6 +222,44 @@ class AlbumsControllerTest < ActionDispatch::IntegrationTest assert_response :success end + test 'should update album and album artists' do + sign_in_as create(:moderator) + album_artist = create(:album_artist, album: @album, separator: nil) + + assert_no_difference '@album.album_artists.reload.count' do + assert_changes 'album_artist.reload.name', to: 'New name' do + patch album_url(@album), params: { album: { album_artists: [{ artist_id: album_artist.artist_id, name: 'New name' }] } } + end + end + + assert_response :success + end + + test 'should not update album and render errors if invalid' do + sign_in_as create(:moderator) + album = create(:album) + + assert_no_difference 'Album.count' do + patch album_url(album), params: { album: { title: '' } } + end + + assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'model' => 'album', 'attribute' => 'title', 'type' => 'blank' } + end + + test 'should not update album and render errors if album artists are invalid' do + sign_in_as create(:moderator) + album = create(:album) + artist = create(:artist) + + assert_no_difference 'Album.count' do + patch album_url(album), params: { album: { album_artists: [{ artist_id: artist.id, name: '', order: 1 }] } } + end + + assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'model' => 'album', 'attribute' => 'album_artists.name', 'type' => 'blank' } + end + test 'should destroy previous image when image is replaced' do sign_in_as create(:moderator) album = create(:album, :with_image)