add support for various artists album

app/controllers/albums_controller.rb

@@ -13,7 +13,6 @@ def index
 
   def show
     @songs = @album.songs
-
     AttachAlbumImageFromDiscogsJob.perform_later(@album.id) if @album.need_attach_from_discogs?
   end
 

app/controllers/application_controller.rb

@@ -5,7 +5,7 @@ class ApplicationController < ActionController::Base
 
   before_action :find_current_user
 
-  rescue_from ActiveRecord::RecordNotFound do |exception|
+  rescue_from ActiveRecord::RecordNotFound, BlackCandyError::NotFound do |exception|
     respond_to do |format|
       format.js { head :not_found }
       format.json { head :not_found }

app/controllers/artists/albums_controller.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class Artists::AlbumsController < ApplicationController
+  before_action :require_login
+  before_action :find_artist
+  before_action :find_albums
+
+  def index
+    raise BlackCandyError::NotFound if @albums.empty?
+  end
+
+  private
+    def find_albums
+      @albums = @artist.albums
+    end
+
+    def find_artist
+      @artist = Artist.find(params[:artist_id])
+    end
+end

app/controllers/artists/appears_on_albums_controller.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class Artists::AppearsOnAlbumsController < Artists::AlbumsController
+  private
+    def find_albums
+      @albums = @artist.appears_on_albums
+    end
+end

app/controllers/artists_controller.rb

@@ -1,6 +1,8 @@
 # frozen_string_literal: true
 
 class ArtistsController < ApplicationController
+  ALBUMS_COUNT = 10
+
   include Pagy::Backend
 
   before_action :require_login
@@ -12,7 +14,8 @@ def index
 
   def show
     @artist = Artist.find(params[:id])
-    @pagy, @albums = pagy_countless(@artist.albums)
+    @albums_pagy, @albums = pagy_countless(@artist.albums, items: ALBUMS_COUNT)
+    @appears_on_albums_pagy, @appears_on_albums = pagy_countless(@artist.appears_on_albums, items: ALBUMS_COUNT)
 
     AttachArtistImageFromDiscogsJob.perform_later(@artist.id) if @artist.need_attach_from_discogs?
   end

app/lib/black_candy_error.rb

@@ -2,5 +2,6 @@
 
 module BlackCandyError
   class Forbidden < StandardError; end
+  class NotFound < StandardError; end
   class InvalidFilePath < StandardError; end
 end

app/models/artist.rb

@@ -4,14 +4,17 @@ class Artist < ApplicationRecord
   include Searchable
 
   has_many :albums, dependent: :destroy
-  has_many :songs, dependent: :destroy
+  has_many :songs
 
   mount_uploader :image, ImageUploader
 
   search_by :name
 
   def title
-    is_unknown? ? I18n.t('text.unknown_artist') : name
+    return I18n.t('text.various_artists') if is_various?
+    return I18n.t('text.unknown_artist') if is_unknown?
+
+    name
   end
 
   def has_image?
@@ -22,6 +25,14 @@ def is_unknown?
     name.blank?
   end
 
+  def all_albums
+    Album.joins(:songs).where('albums.artist_id = ? OR songs.artist_id = ?', id, id).distinct
+  end
+
+  def appears_on_albums
+    Album.joins(:songs).where('albums.artist_id != ? AND songs.artist_id = ?', id, id).distinct
+  end
+
   def need_attach_from_discogs?
     Setting.discogs_token.present? && !has_image? && !is_unknown?
   end

app/models/media.rb

@@ -9,7 +9,13 @@ def initialize(file_path)
 
   def attach
     artist = Artist.find_or_create_by(name: file_info[:artist_name])
-    album = Album.find_or_create_by(artist: artist, name: file_info[:album_name])
+
+    if various_artists?
+      various_artist = Artist.find_or_create_by(is_various: true)
+      album = Album.find_or_create_by(artist: various_artist, name: file_info[:album_name])
+    else
+      album = Album.find_or_create_by(artist: artist, name: file_info[:album_name])
+    end
 
     # Attach image from file to the album.
     AttachAlbumImageFromFileJob.perform_later(album.id, file_info[:file_path]) unless album.has_image?
@@ -25,6 +31,11 @@ def song_info
     file_info.slice(:name, :tracknum, :length, :file_path)
   end
 
+  def various_artists?
+    albumartist = file_info[:albumartist_name]
+    albumartist.present? && (albumartist.downcase == 'various artists' || albumartist != file_info[:artist_name])
+  end
+
   class << self
     def sync
       media_hashes = MediaFile.file_paths.map do |file_path|
@@ -46,7 +57,7 @@ def clean_up(media_hashes)
 
         # Clean up no content albums and artist.
         Album.left_outer_joins(:songs).where('songs.id is null').destroy_all
-        Artist.left_outer_joins(:songs).where('songs.id is null').destroy_all
+        Artist.left_outer_joins(:songs, :albums).where('songs.album_id is null').where('albums.id is null').destroy_all
       end
   end
 end

app/models/media_file.rb

@@ -40,6 +40,7 @@ def get_tag_info(file_path)
           name: tag.title.presence || File.basename(file_path),
           album_name: tag.album.presence,
           artist_name: tag.artist.presence,
+          albumartist_name: tag.albumartist.presence,
           tracknum: tag.track,
           length: tag.duration
         }

app/views/artists/_albums.html.erb

@@ -0,0 +1,17 @@
+<% if albums.present? %>
+  <div class='cards'>
+    <div class='heading heading--expand display__space-between display__full-width display__align-center'>
+      <div class='heading__body'>
+        <div class='heading__body__text heading__body__text--main'>
+          <%= title %>
+        </div>
+      </div>
+      <% if local_assigns[:all_albums_link] && all_albums_link.present? %>
+        <%= link_to t('text.see_all'), all_albums_link %>
+      <% end %>
+    </div>
+  </div>
+  <div class='cards'>
+    <%= render partial: 'albums/album', collection: albums, cached: true %>
+  </div>
+<% end %>

app/views/artists/_info.html.erb

@@ -0,0 +1,18 @@
+<% cache artist do %>
+  <div class='cards'>
+    <div class='heading heading--expand'>
+      <div class='heading__image'>
+        <%= image_tag image_url_for(artist), class: 'image' %>
+      </div>
+      <div class='heading__body'>
+        <div class='heading__body__text heading__body__text--main'><%= artist.title %></div>
+        <div class='heading__body__text'>
+          <span><%= artist.all_albums.size %> <%= t('label.albums') %></span>
+          <span>,</span>
+          <span><%= artist.songs.size %> <%= t('label.songs')%></span>
+        </div>
+      </div>
+    </div>
+  </div>
+  <div class='line'></div>
+<% end %>

app/views/artists/albums/index.html.erb

@@ -0,0 +1,2 @@
+<%= render partial: 'artists/info', locals: { artist: @artist } %>
+<%= render partial: 'artists/albums', locals: { title: t('label.albums'), albums: @albums }  %>

app/views/artists/appears_on_albums/index.html.erb

@@ -0,0 +1,2 @@
+<%= render partial: 'artists/info', locals: { artist: @artist } %>
+<%= render partial: 'artists/albums', locals: { title: t('label.appears_on'), albums: @albums }  %>

app/views/artists/show.html.erb

@@ -1,28 +1,11 @@
-<% cache @artist do %>
-  <div class='cards'>
-    <div class='heading heading--expand'>
-      <div class='heading__image'>
-        <%= image_tag image_url_for(@artist), class: 'image' %>
-      </div>
-      <div class='heading__body'>
-        <div class='heading__body__text heading__body__text--main'><%= @artist.title %></div>
-        <div class='heading__body__text'>
-          <span><%= @albums.size %> <%= t('label.albums') %></span>
-          <span>,</span>
-          <span><%= @artist.songs.size %> <%= t('label.songs')%></span>
-        </div>
-      </div>
-    </div>
-  </div>
-  <div class='line'></div>
-<% end %>
-<div data-controller='infinite-scroll' data-infinite-scroll-next-url='<%= pagy_next_url(@pagy) %>'>
-  <div class='cards' id='js-albums-content'>
-    <%= render partial: 'albums/album', collection: @albums, cached: true %>
-  </div>
-  <div data-target='infinite-scroll.trigger' class='display__justify-align-center'>
-    <% if @pagy.next %>
-      <%= loader_tag %>
-    <% end %>
-  </div>
-</div>
+<%= render partial: 'info', locals: { artist: @artist} %>
+
+<%= render partial: 'albums', locals: {
+  title: t('label.albums'),
+  albums: @albums,
+  all_albums_link: @albums_pagy.next.present? ? artist_albums_path(@artist) : '' } %>
+
+<%= render partial: 'albums', locals: {
+  title: t('label.appears_on'),
+  albums: @appears_on_albums,
+  all_albums_link: @appears_on_albums_pagy.next.present? ? artist_appears_on_albums_path(@artist) : '' }  %>

config/locales/en.yml

@@ -52,6 +52,7 @@ en:
     integration: 'Integration'
     close: 'Close'
     more: 'More'
+    appears_on: 'Appears On'
   error:
     login: 'Wrong email or password'
     media_path_blank: 'Media path is not exist'
@@ -75,6 +76,7 @@ en:
     add_user: 'Add user'
     create_user: 'Create User'
     media_path: 'Media path'
+    various_artists: 'Various Artists'
     unknown_artist: 'Unknown Artist'
     unknown_album: 'Unknown Album'
     go_back: 'Go back'
@@ -89,3 +91,4 @@ en:
     expand_player: 'Expand player'
     collapse_player: 'Collapse player'
     personal: 'Personal'
+    see_all: 'See All'

config/routes.rb

@@ -4,7 +4,11 @@
   resource :session, only: [:new, :create, :destroy]
   resource :setting, only: [:show, :update]
 
-  resources :artists, only: [:index, :show]
+  resources :artists, only: [:index, :show] do
+    resources :albums, only: [:index], module: 'artists'
+    resources :appears_on_albums, only: [:index], module: 'artists'
+  end
+
   resources :stream, only: [:new]
   resources :transcoded_stream, only: [:new]
   resources :songs, only: [:index, :show]

db/migrate/20200721071945_add_is_various_to_artists.rb

@@ -0,0 +1,5 @@
+class AddIsVariousToArtists < ActiveRecord::Migration[6.0]
+  def change
+    add_column(:artists, :is_various, :boolean, default: false)
+  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.define(version: 2020_04_25_023906) do
+ActiveRecord::Schema.define(version: 2020_07_21_071945) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_trgm"
@@ -31,6 +31,7 @@
     t.string "image"
     t.datetime "created_at", precision: 6, null: false
     t.datetime "updated_at", precision: 6, null: false
+    t.boolean "is_various", default: false
     t.index ["name"], name: "index_artists_on_name", unique: true
   end
 

test/controllers/artists/albums_controller_test.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class Artists::AlbumsControllerTest < ActionDispatch::IntegrationTest
+  test 'should get index' do
+    assert_login_access(url: artist_albums_path(artists :artist1)) do
+      assert_response :success
+    end
+  end
+
+  test 'should redirect to not found page when artists have no albums' do
+    empty_artist = Artist.create
+
+    assert_login_access(url: artist_albums_path(empty_artist)) do
+      assert_response :not_found
+    end
+  end
+end

test/controllers/artists/appears_on_albums_controller_test.rb

@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+require 'test_helper'
+
+class Artists::AppearsOnAlbumsControllerTest < ActionDispatch::IntegrationTest
+  test 'should get index' do
+    assert_login_access(url: artist_appears_on_albums_path(artists :artist1)) do
+      assert_response :success
+    end
+  end
+
+  test 'should redirect to not found page when artists have no albums' do
+    assert artists(:artist2).appears_on_albums.empty?
+    assert_login_access(url: artist_appears_on_albums_path(artists :artist2)) do
+      assert_response :not_found
+    end
+  end
+end

test/fixtures/albums.yml

@@ -7,3 +7,6 @@ album2:
 album3:
   name: 'album3'
   artist: 'artist2'
+album4:
+  name: 'album4'
+  artist: 'various_artists'

test/fixtures/artists.yml

@@ -3,3 +3,6 @@ artist1:
 
 artist2:
   name: 'artist2'
+
+various_artists:
+  is_various: true

test/fixtures/songs.yml

@@ -61,3 +61,11 @@ wma_sample:
   md5_hash: 'fake_md5'
   artist: 'artist2'
   album: 'album3'
+
+various_artists_sample:
+  id: 9
+  name: 'various_artists_sample'
+  file_path: <%= Rails.root.join('test', 'fixtures', 'files', 'various_artists.mp3') %>
+  md5_hash: 'fake_md5'
+  artist: 'artist1'
+  album: 'album4'

test/models/artist_test.rb

@@ -6,4 +6,16 @@ class ArtistTest < ActiveSupport::TestCase
   test 'should have default title when name is empty' do
     assert_equal 'Unknown Artist', Artist.create(name: nil).title
   end
+
+  test 'should have default title when is various artist' do
+    assert_equal 'Various Artists', Artist.create(is_various: true).title
+  end
+
+  test 'should get all albums' do
+    assert_equal Album.where(name: %w(album1 album2 album4)).ids.sort, artists(:artist1).all_albums.ids.sort
+  end
+
+  test 'should get appears on albums' do
+    assert_equal Album.where(name: %w(album4)).ids.sort, artists(:artist1).appears_on_albums.ids.sort
+  end
 end

test/models/concerns/searchable_test.rb

@@ -41,7 +41,8 @@ class SearchableTest < ActiveSupport::TestCase
 
     album_serach_song_ids = Album.find_by_name('album1').songs.ids +
       Album.find_by_name('album2').songs.ids +
-      Album.find_by_name('album3').songs.ids
+      Album.find_by_name('album3').songs.ids +
+      Album.find_by_name('album4').songs.ids
 
     assert_equal album_serach_song_ids.sort, Song.search('album').ids.sort
   end