Published albums must have at least one track

Publishing an album with no tracks doesn't make much sense and causes
exceptions like this one [1], because the album has no preview to point
the audio element at.

[1]: https://app.rollbar.com/a/gofreerange/fix/item/jam/49

Fixes #148.

app/controllers/albums_controller.rb

@@ -27,8 +27,11 @@ def update
   end
 
   def publish
-    @album.publish
-    AlbumMailer.with(album: @album).published.deliver_later
+    if @album.publish
+      AlbumMailer.with(album: @album).published.deliver_later
+    else
+      flash[:alert] = "Errors prohibited this album from being saved: #{@album.errors.full_messages.to_sentence}"
+    end
     redirect_to artist_album_url(@album.artist, @album)
   end
 

app/models/album.rb

@@ -16,6 +16,7 @@ class Album < ApplicationRecord
   validates :title, presence: true
   validates :price, presence: true, numericality: true
   validates :released_on, comparison: { less_than_or_equal_to: Time.zone.today, allow_blank: true }
+  validates :number_of_tracks, comparison: { greater_than: 0 }, if: :published?
   validates(
     :cover,
     attached: { message: 'file cannot be missing' },
@@ -45,11 +46,12 @@ def pending
   end
 
   def publish
-    published!
-    update!(first_published_on: Time.current) if first_published_on.blank?
-
-    ZipDownloadJob.perform_later(self, format: :mp3v0)
-    ZipDownloadJob.perform_later(self, format: :flac)
+    saved = update(publication_status: :published, first_published_on: first_published_on || Time.current)
+    if saved
+      ZipDownloadJob.perform_later(self, format: :mp3v0)
+      ZipDownloadJob.perform_later(self, format: :flac)
+    end
+    saved
   end
 
   def unpublish
@@ -60,6 +62,10 @@ def released_on
     super || first_published_on
   end
 
+  def number_of_tracks
+    tracks.length
+  end
+
   def metadata_or_cover_changed?
     title_previously_changed? || attachment_changes['cover'].present?
   end

test/controllers/albums_controller_test.rb

@@ -4,7 +4,7 @@
 
 class AlbumsControllerTestSignedInAsAdmin < ActionDispatch::IntegrationTest
   def setup
-    @album = create(:album)
+    @album = create(:album, :with_tracks)
     @user = create(:user, admin: true)
     log_in_as(@user)
   end
@@ -118,6 +118,26 @@ def setup
     end
   end
 
+  test '#publish does not succeed if album has validation errors' do
+    @album.tracks.destroy_all
+    patch publish_artist_album_url(@album.artist, @album)
+    assert_not @album.reload.published?
+  end
+
+  test '#publish redirects back to album page with error message if album has validation errors' do
+    @album.tracks.destroy_all
+    patch publish_artist_album_url(@album.artist, @album)
+    assert_redirected_to artist_album_url(@album.artist, @album)
+    assert_equal 'Errors prohibited this album from being saved: Number of tracks must be greater than 0', flash[:alert]
+  end
+
+  test '#publish does not send email to artist if album has validation errors' do
+    @album.tracks.destroy_all
+    assert_enqueued_emails 0 do
+      patch publish_artist_album_url(@album.artist, @album)
+    end
+  end
+
   test '#unpublish' do
     patch unpublish_artist_album_url(@album.artist, @album)
     assert_redirected_to artist_album_url(@album.artist, @album)
@@ -128,7 +148,7 @@ def setup
 
 class AlbumsControllerTestSignedInAsArtist < ActionDispatch::IntegrationTest
   setup do
-    @album = create(:album)
+    @album = create(:published_album)
     user = create(:user)
     user.artists << @album.artist
     log_in_as(user)

test/factories/album.rb

@@ -44,7 +44,7 @@
     publication_status { :pending }
   end
 
-  factory :published_album, parent: :album do
+  factory :published_album, parent: :album, traits: %i[with_tracks] do
     publication_status { :published }
   end
 end

test/models/album_test.rb

@@ -95,7 +95,7 @@ class AlbumTest < ActiveSupport::TestCase
   end
 
   test 'publish enqueues ZipDownloadJob to prepare mp3v0 download' do
-    album = create(:unpublished_album)
+    album = create(:unpublished_album, :with_tracks)
 
     args_matcher = ->(job_args) { job_args[1][:format] == :mp3v0 }
     assert_enqueued_with(job: ZipDownloadJob, args: args_matcher) do
@@ -104,7 +104,7 @@ class AlbumTest < ActiveSupport::TestCase
   end
 
   test 'publish enqueues ZipDownloadJob to prepare flac download' do
-    album = create(:unpublished_album)
+    album = create(:unpublished_album, :with_tracks)
 
     args_matcher = ->(job_args) { job_args[1][:format] == :flac }
     assert_enqueued_with(job: ZipDownloadJob, args: args_matcher) do
@@ -223,4 +223,15 @@ class AlbumTest < ActiveSupport::TestCase
       assert_includes album.errors[:released_on], "must be less than or equal to #{Time.zone.today}"
     end
   end
+
+  test 'is valid if not published and has no tracks' do
+    album = build(:unpublished_album, tracks: [])
+    assert album.valid?
+  end
+
+  test 'is not valid if published and has no tracks' do
+    album = build(:album, tracks: [], publication_status: :published)
+    assert_not album.valid?
+    assert_includes album.errors[:number_of_tracks], 'must be greater than 0'
+  end
 end

test/system/publishing_an_album_test.rb

@@ -4,7 +4,7 @@
 
 class PublishingAnAlbumTest < ApplicationSystemTestCase
   setup do
-    @album = create(:unpublished_album)
+    @album = create(:unpublished_album, :with_tracks)
     user = create(:user)
     user.artists << @album.artist
     log_in_as(user)