Merge pull request #325 from blackcandy-org/multi-disc-album

Fix #309, support multi-disc album

app/assets/stylesheets/components/_list.scss

@@ -19,6 +19,12 @@
   border-bottom: 1px solid var(--list-border-color);
 }
 
+.c-list__item--divider {
+  margin-top: spacing("small");
+  font-weight: bold;
+  text-transform: uppercase;
+}
+
 .c-list--border-none .c-list__item {
   border-bottom: none;
 }

app/controllers/albums_controller.rb

@@ -14,7 +14,7 @@ def index
   end
 
   def show
-    @songs = @album.songs.includes(:artist)
+    @groped_songs = @album.songs.includes(:artist).group_by(&:discnum)
     @album.attach_image_from_discogs
   end
 

app/models/album.rb

@@ -8,7 +8,7 @@ class Album < ApplicationRecord
 
   validates :name, uniqueness: {scope: :artist}
 
-  has_many :songs, -> { order(:tracknum) }, inverse_of: :album, dependent: :destroy
+  has_many :songs, -> { order(:discnum, :tracknum) }, inverse_of: :album, dependent: :destroy
   belongs_to :artist, touch: true
 
   search_by :name, associations: {artist: :name}

app/models/media.rb

@@ -76,7 +76,7 @@ def attach(file_info)
     end
 
     def song_info(file_info)
-      file_info.slice(:name, :tracknum, :duration, :file_path, :file_path_hash, :bit_depth).compact
+      file_info.slice(:name, :tracknum, :discnum, :duration, :file_path, :file_path_hash, :bit_depth).compact
     end
 
     def album_info(file_info)

app/models/media_file.rb

@@ -55,6 +55,7 @@ def get_tag_info(file_path)
         albumartist_name: tag.albumartist.presence,
         genre: tag.genre.presence,
         tracknum: tag.track,
+        discnum: tag.disc,
         duration: tag.duration.round,
         bit_depth: tag.bit_depth,
         image: extract_image_from(tag)

app/views/albums/show.html.erb

@@ -7,9 +7,9 @@
       <h1 class='c-card__body__title'><%= @album.title %></h1>
       <p class='c-card__body__text'><%= @album.artist.title %></p>
       <div class='c-card__body__text'>
-        <span><%= @songs.load.size %> <%= t("label.tracks") %></span>
+        <span><%= @album.songs.count %> <%= t("label.tracks") %></span>
         <span>,</span>
-        <span class='u-text-monospace'><%= format_duration(@songs.sum(:duration)) %></span>
+        <span class='u-text-monospace'><%= format_duration(@album.songs.sum(:duration)) %></span>
       </div>
       <div class='u-mt-large'>
         <%= button_to(
@@ -35,69 +35,75 @@
     </div>
   </div>
   <ul class='c-list'>
-    <% @songs.each do |song| %>
-      <li class='c-list__item' data-playlist-songs-target='item' data-song-id='<%= song.id %>' data-test-id='album_song'>
-        <div class='o-flex o-flex--justify-between o-flex--align-center'>
-          <%= button_to(
-                current_playlist_songs_path(song_id: song.id, should_play: true),
-                class: "c-button c-button--link u-w-100",
-                form_class: "o-flex__item--grow-1",
-                form: {
-                  data: {
-                    "delegated-action" => "turbo:submit-start->playlist-songs#checkBeforePlay playlist-songs-bridge#playSong",
-                    "turbo-frame" => "turbo-playlist",
-                    "disabled-on-native" => "true"
+    <% @groped_songs.each do |discnum, songs| %>
+      <% if @groped_songs.size > 1 %>
+        <li class='c-list__item c-list__item--divider'><%= t("label.disc", number: discnum) %></li>
+      <% end %>
+
+      <% songs.each do |song| %>
+        <li class='c-list__item' data-playlist-songs-target='item' data-song-id='<%= song.id %>' data-test-id='album_song'>
+          <div class='o-flex o-flex--justify-between o-flex--align-center'>
+            <%= button_to(
+                  current_playlist_songs_path(song_id: song.id, should_play: true),
+                  class: "c-button c-button--link u-w-100",
+                  form_class: "o-flex__item--grow-1",
+                  form: {
+                    data: {
+                      "delegated-action" => "turbo:submit-start->playlist-songs#checkBeforePlay playlist-songs-bridge#playSong",
+                      "turbo-frame" => "turbo-playlist",
+                      "disabled-on-native" => "true"
+                    }
                   }
-                }
-              ) do %>
-            <div class='o-flex o-flex--justify-between o-flex--align-center'>
-              <div>
-                <span class='u-mb-tiny' data-test-id='album_song_name'><%= song.name %></span>
-                <% if @album.artist.is_various? %>
-                  <%= song.artist.title %>
-                <% end %>
+                ) do %>
+              <div class='o-flex o-flex--justify-between o-flex--align-center'>
+                <div>
+                  <span class='u-mb-tiny' data-test-id='album_song_name'><%= song.name %></span>
+                  <% if @album.artist.is_various? %>
+                    <%= song.artist.title %>
+                  <% end %>
+                </div>
+                <div class='u-text-monospace u-mr-narrow'><%= format_duration(song.duration) %></div>
               </div>
-              <div class='u-text-monospace u-mr-narrow'><%= format_duration(song.duration) %></div>
-            </div>
-          <% end %>
+            <% end %>
 
-          <details class='c-dropdown' data-test-id='album_song_menu'>
-            <summary><%= icon_tag "more-vertical", size: "small", title: t("label.more") %></summary>
-            <div class='c-dropdown__list'>
-              <%= link_to(
-                    t("label.add_to_playlist"),
-                    dialog_playlists_path(song_id: song.id, referer_url: current_url),
-                    data: {turbo_frame: ("turbo-dialog" unless native_app?)},
-                    class: "c-dropdown__item"
-                  ) %>
-              <%= button_to(
-                    t("label.play_next"),
-                    current_playlist_songs_path(song_id: song.id),
-                    form_class: "c-dropdown__item",
-                    form: {
-                      data: {
-                        "turbo-frame" => "turbo-playlist",
-                        "delegated-action" => "turbo:submit-start->playlist-songs#checkBeforePlayNext playlist-songs-bridge#playNext",
-                        "disabled-on-native" => "true"
+            <details class='c-dropdown' data-test-id='album_song_menu'>
+              <summary><%= icon_tag "more-vertical", size: "small", title: t("label.more") %></summary>
+              <div class='c-dropdown__list'>
+                <%= link_to(
+                      t("label.add_to_playlist"),
+                      dialog_playlists_path(song_id: song.id, referer_url: current_url),
+                      data: {turbo_frame: ("turbo-dialog" unless native_app?)},
+                      class: "c-dropdown__item"
+                    ) %>
+                <%= button_to(
+                      t("label.play_next"),
+                      current_playlist_songs_path(song_id: song.id),
+                      form_class: "c-dropdown__item",
+                      form: {
+                        data: {
+                          "turbo-frame" => "turbo-playlist",
+                          "delegated-action" => "turbo:submit-start->playlist-songs#checkBeforePlayNext playlist-songs-bridge#playNext",
+                          "disabled-on-native" => "true"
+                        }
                       }
-                    }
-                  ) %>
-              <%= button_to(
-                    t("label.play_last"),
-                    current_playlist_songs_path(song_id: song.id, location: "last"),
-                    form_class: "c-dropdown__item",
-                    form: {
-                      data: {
-                        "turbo-frame" => "turbo-playlist",
-                        "delegated-action" => "playlist-songs-bridge#playLast",
-                        "disabled-on-native" => "true"
+                    ) %>
+                <%= button_to(
+                      t("label.play_last"),
+                      current_playlist_songs_path(song_id: song.id, location: "last"),
+                      form_class: "c-dropdown__item",
+                      form: {
+                        data: {
+                          "turbo-frame" => "turbo-playlist",
+                          "delegated-action" => "playlist-songs-bridge#playLast",
+                          "disabled-on-native" => "true"
+                        }
                       }
-                    }
-                  ) %>
-            </div>
-          </details>
-        </div>
-      </li>
+                    ) %>
+              </div>
+            </details>
+          </div>
+        </li>
+      <% end %>
     <% end %>
   </ul>
 </div>

config/locales/en.yml

@@ -108,6 +108,7 @@ en:
     play_next: 'Play Next'
     play_last: 'Play Last'
     search_results: 'Search Results'
+    disc: 'Disc %{number}'
   error:
     login: 'Wrong email or password'
     forbidden: "Sorry, you do not have permission to visit that"

db/migrate/20231207020650_add_discnum_to_songs.rb

@@ -0,0 +1,5 @@
+class AddDiscnumToSongs < ActiveRecord::Migration[7.1]
+  def change
+    add_column :songs, :discnum, :integer
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2023_11_02_065319) do
+ActiveRecord::Schema[7.1].define(version: 2023_12_07_020650) do
   create_table "albums", force: :cascade do |t|
     t.string "name"
     t.string "image"
@@ -77,6 +77,7 @@
     t.integer "artist_id"
     t.string "file_path_hash"
     t.integer "bit_depth"
+    t.integer "discnum"
     t.index ["album_id"], name: "index_songs_on_album_id"
     t.index ["artist_id"], name: "index_songs_on_artist_id"
     t.index ["file_path_hash"], name: "index_songs_on_file_path_hash"

test/models/album_test.rb

@@ -12,19 +12,19 @@ class AlbumTest < ActiveSupport::TestCase
     assert_equal "Unknown Album", Album.create(name: nil).title
   end
 
-  test "should order by tracknum for associated songs" do
+  test "should order by discnum and tracknum for associated songs" do
     artist = artists(:artist1)
     album = artist.albums.create
 
     album.songs.create!(
       [
-        {name: "test_song_1", file_path: "fake_path", file_path_hash: "fake_path_hash", md5_hash: "fake_md5", tracknum: 2, artist: artist},
-        {name: "test_song_2", file_path: "fake_path", file_path_hash: "fake_path_hash", md5_hash: "fake_md5", tracknum: 3, artist: artist},
-        {name: "test_song_3", file_path: "fake_path", file_path_hash: "fake_path_hash", md5_hash: "fake_md5", tracknum: 1, artist: artist}
+        {name: "test_song_1", file_path: "fake_path", file_path_hash: "fake_path_hash", md5_hash: "fake_md5", tracknum: 2, discnum: 2, artist: artist},
+        {name: "test_song_2", file_path: "fake_path", file_path_hash: "fake_path_hash", md5_hash: "fake_md5", tracknum: 3, discnum: 1, artist: artist},
+        {name: "test_song_3", file_path: "fake_path", file_path_hash: "fake_path_hash", md5_hash: "fake_md5", tracknum: 1, discnum: 1, artist: artist}
       ]
     )
 
-    assert_equal %w[test_song_3 test_song_1 test_song_2], album.songs.pluck(:name)
+    assert_equal %w[test_song_3 test_song_2 test_song_1], album.songs.pluck(:name)
   end
 
   test "should filter by genre" do

test/models/media_file_test.rb

@@ -70,6 +70,7 @@ class MediaFileTest < ActiveSupport::TestCase
     assert_equal 1984, tag_info[:year]
     assert_equal "Rock", tag_info[:genre]
     assert_equal cover_image_binary, tag_image_binary
+    assert_equal 0, tag_info[:discnum]
   end
 
   test "should get tag info from flac file" do
@@ -86,6 +87,7 @@ class MediaFileTest < ActiveSupport::TestCase
     assert_equal 1984, tag_info[:year]
     assert_equal "Rock", tag_info[:genre]
     assert_equal cover_image_binary, tag_image_binary
+    assert_equal 0, tag_info[:discnum]
   end
 
   test "should get tag info from ogg file" do
@@ -99,6 +101,7 @@ class MediaFileTest < ActiveSupport::TestCase
     assert_equal 8, tag_info[:duration]
     assert_equal 1984, tag_info[:year]
     assert_equal "Rock", tag_info[:genre]
+    assert_nil tag_info[:discnum]
   end
 
   test "should get tag info from wav file" do
@@ -115,6 +118,7 @@ class MediaFileTest < ActiveSupport::TestCase
     assert_equal 1984, tag_info[:year]
     assert_equal "Rock", tag_info[:genre]
     assert_equal cover_image_binary, tag_image_binary
+    assert_equal 0, tag_info[:discnum]
   end
 
   test "should get tag info from opus file" do
@@ -128,6 +132,7 @@ class MediaFileTest < ActiveSupport::TestCase
     assert_equal 8, tag_info[:duration]
     assert_equal 1984, tag_info[:year]
     assert_equal "Rock", tag_info[:genre]
+    assert_nil tag_info[:discnum]
   end
 
   test "should get tag info from m4a file" do
@@ -144,6 +149,7 @@ class MediaFileTest < ActiveSupport::TestCase
     assert_equal 1984, tag_info[:year]
     assert_equal "Rock", tag_info[:genre]
     assert_equal cover_image_binary, tag_image_binary
+    assert_nil tag_info[:discnum]
   end
 
   test "should get tag info from oga file" do
@@ -157,6 +163,7 @@ class MediaFileTest < ActiveSupport::TestCase
     assert_equal 8, tag_info[:duration]
     assert_equal 1984, tag_info[:year]
     assert_equal "Rock", tag_info[:genre]
+    assert_nil tag_info[:discnum]
   end
 
   test "should get tag info from wma file" do
@@ -170,6 +177,7 @@ class MediaFileTest < ActiveSupport::TestCase
     assert_equal 8, tag_info[:duration]
     assert_nil tag_info[:year]
     assert_nil tag_info[:genre]
+    assert_nil tag_info[:discnum]
   end
 
   test "should get md5 hash from file" do