Skip to content

Revert validation of album artists separator - #998

Merged
robbevp merged 1 commit into
mainfrom
fix/separator-validations
Aug 29, 2026
Merged

Revert validation of album artists separator#998
robbevp merged 1 commit into
mainfrom
fix/separator-validations

Conversation

@robbevp

@robbevp robbevp commented Aug 29, 2026

Copy link
Copy Markdown
Member

This reverts a small part of #722, where I moved the separator to the album artist model. This worked inside the model tests, but does not work in the controller since when updating (since we fully replace the album artists, during the validation album.album_artists.size is 0.

There are still two underlying issues that are not solved here:

  • If we provide an invalid album artist, rails will raise an ActiveRecord::RecordNotSaved error and not render a body in the form that we expect. (I suspect this is the case for all associations that get replaced in our controllers: album labels, track artists, ...)
  • We do properly handle this for create, but in that case the errors only contain { model: 'album', attribute: 'album_artists', type: 'invalid' }. So we have no context about which of the album artists is invalid and just what the issue is.
    I suspect that both of these issues where already present before Return errors as json objects instead of strings #722 - they are a bit trickier to solve, so I already wanted to get this quick fix out
  • I've added tests relevant to my changes.

@robbevp robbevp self-assigned this Aug 29, 2026
@robbevp
robbevp requested a review from chvp as a code owner August 29, 2026 12:56
@robbevp robbevp added the bug Something isn't working label Aug 29, 2026
Comment thread test/controllers/albums_controller_test.rb Outdated
This reverts a small part of #722, where I moved the separator to the album artist model. This worked inside the model tests, but does not work in the controller since when updating (since we fully replace the album artists, during the validation `album.album_artists.size` is 0.
@robbevp
robbevp force-pushed the fix/separator-validations branch from ac58e48 to adfc3ec Compare August 29, 2026 15:05
@robbevp
robbevp enabled auto-merge August 29, 2026 15:07
@robbevp
robbevp added this pull request to the merge queue Aug 29, 2026
Merged via the queue into main with commit 509e716 Aug 29, 2026
4 checks passed
@robbevp
robbevp deleted the fix/separator-validations branch August 29, 2026 20:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants