Merge pull request #311 from blackcandy-org/dev

Unify error handling

app/controllers/api/v1/api_controller.rb

@@ -5,10 +5,6 @@ module V1
     class ApiController < ApplicationController
       skip_before_action :verify_authenticity_token
 
-      rescue_from ActiveRecord::RecordNotUnique do
-        render json: ApiError.new(:record_not_unique, t("error.already_in_playlist")), status: :bad_request
-      end
-
       private
 
       # If user already has logged in then authenticate with current session,

app/controllers/api/v1/authentications_controller.rb

@@ -10,24 +10,20 @@ def create
         session = UserSession.new(session_params.merge({remember_me: true}).to_h)
 
         if params[:with_session]
-          render_unauthorized and return unless session.save
+          raise BlackCandy::InvalidCredential unless session.save
         else
-          render_unauthorized and return unless session.valid?
+          raise BlackCandy::InvalidCredential unless session.valid?
         end
 
         @current_user = User.find_by(email: session_params[:email])
 
-        render_unauthorized and return unless @current_user.present?
+        raise BlackCandy::InvalidCredential unless @current_user.present?
 
         @current_user.regenerate_api_token if @current_user.api_token.blank?
       end
 
       private
 
-      def render_unauthorized
-        render(json: ApiError.new(:invalid_credential, t("error.login")), status: :unauthorized)
-      end
-
       def session_params
         params.require(:user_session).permit(:email, :password)
       end

app/controllers/api/v1/current_playlist/songs_controller.rb

@@ -35,6 +35,8 @@ def create
           current_song_position = @playlist.playlists_songs.find_by(song_id: params[:current_song_id])&.position.to_i
           @playlist.playlists_songs.create(song_id: @song.id, position: current_song_position + 1)
         end
+      rescue ActiveRecord::RecordNotUnique
+        raise BlackCandy::DuplicatePlaylistSong
       end
 
       private

app/controllers/api/v1/favorite_playlist/songs_controller.rb

@@ -8,6 +8,8 @@ class FavoritePlaylist::SongsController < ApiController
 
       def create
         @playlist.playlists_songs.create(song_id: @song.id, position: 1)
+      rescue ActiveRecord::RecordNotUnique
+        raise BlackCandy::DuplicatePlaylistSong
       end
 
       def destroy

app/controllers/application_controller.rb

@@ -9,14 +9,33 @@ class ApplicationController < ActionController::Base
   before_action :find_current_user
   before_action :require_login
 
-  rescue_from BlackCandy::Error::Forbidden do
+  rescue_from BlackCandy::Forbidden do |error|
     respond_to do |format|
-      format.js { head :forbidden }
-      format.json { head :forbidden }
+      format.json { render_json_error(error, :forbidden) }
       format.html { render template: "errors/forbidden", layout: "plain", status: :forbidden }
     end
   end
 
+  rescue_from BlackCandy::InvalidCredential do |error|
+    respond_to do |format|
+      format.json { render_json_error(error, :unauthorized) }
+      format.html { redirect_to new_session_path }
+    end
+  end
+
+  rescue_from BlackCandy::DuplicatePlaylistSong do |error|
+    respond_to do |format|
+      format.json { render_json_error(error, :bad_request) }
+    end
+  end
+
+  rescue_from ActiveRecord::RecordNotFound do |error|
+    respond_to do |format|
+      format.json { render_json_error(OpenStruct.new(type: "RecordNotFound", message: error.message), :not_found) }
+      format.html { render template: "errors/not_found", layout: "plain", status: :not_found }
+    end
+  end
+
   rescue_from ActionController::InvalidAuthenticityToken do
     logout_current_user
   end
@@ -76,7 +95,7 @@ def require_login
   end
 
   def require_admin
-    raise BlackCandy::Error::Forbidden if BlackCandy::Config.demo_mode? || !is_admin?
+    raise BlackCandy::Forbidden if BlackCandy::Config.demo_mode? || !is_admin?
   end
 
   def logout_current_user
@@ -93,4 +112,8 @@ def turbo_ios?
   def turbo_android?
     request.user_agent.to_s.match?(/Turbo Native Android/)
   end
+
+  def render_json_error(error, status)
+    render json: {type: error.type, message: error.message}, status: status
+  end
 end

app/controllers/concerns/playable.rb

@@ -9,7 +9,7 @@ module Playable
   end
 
   def play
-    raise BlackCandy::Error::Forbidden if @song_ids.blank?
+    raise BlackCandy::Forbidden if @song_ids.blank?
 
     @playlist = Current.user.current_playlist
     @playlist.replace(@song_ids)

app/controllers/users/settings_controller.rb

@@ -25,6 +25,6 @@ def find_user
   end
 
   def auth_user
-    raise BlackCandy::Error::Forbidden unless @user == Current.user
+    raise BlackCandy::Forbidden unless @user == Current.user
   end
 end

app/controllers/users_controller.rb

@@ -37,7 +37,7 @@ def update
 
   def destroy
     # Avoid user delete self
-    raise BlackCandy::Error::Forbidden if @user == Current.user
+    raise BlackCandy::Forbidden if @user == Current.user
 
     @user.destroy
     flash.now[:success] = t("success.delete")
@@ -50,8 +50,8 @@ def find_user
   end
 
   def auth_user
-    raise BlackCandy::Error::Forbidden if BlackCandy::Config.demo_mode?
-    raise BlackCandy::Error::Forbidden unless @user == Current.user || is_admin?
+    raise BlackCandy::Forbidden if BlackCandy::Config.demo_mode?
+    raise BlackCandy::Forbidden unless @user == Current.user || is_admin?
   end
 
   def user_params

config/application.rb

@@ -15,7 +15,7 @@
 require "rails/test_unit/railtie"
 
 require_relative "../lib/black_candy/config"
-require_relative "../lib/black_candy/error"
+require_relative "../lib/black_candy/errors"
 require_relative "../lib/black_candy/version"
 
 # Require the gems listed in Gemfile, including any gems

lib/black_candy/errors.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+module BlackCandy
+  class BaseError < StandardError
+    def type
+      self.class.name.split("::").last
+    end
+
+    def message
+      super
+    end
+  end
+
+  class Forbidden < BaseError
+    def message
+      I18n.t("error.forbidden")
+    end
+  end
+
+  class InvalidCredential < BaseError
+    def message
+      I18n.t("error.login")
+    end
+  end
+
+  class DuplicatePlaylistSong < BaseError
+    def message
+      I18n.t("error.already_in_playlist")
+    end
+  end
+end

test/controllers/api/v1/current_playlist/songs_controller_test.rb

@@ -69,7 +69,7 @@ class Api::V1::CurrentPlaylist::SongsControllerTest < ActionDispatch::Integratio
     response = @response.parsed_body
 
     assert_response :bad_request
-    assert_equal "RecordNotUnique", response["type"]
+    assert_equal "DuplicatePlaylistSong", response["type"]
     assert_not_empty response["message"]
   end
 end

test/controllers/api/v1/favorite_playlist/songs_controller_test.rb

@@ -33,11 +33,11 @@ class Api::V1::FavoritePlaylist::SongsControllerTest < ActionDispatch::Integrati
   end
 
   test "should not add song to playlist if already in playlist" do
-    post api_v1_favorite_playlist_songs_url, params: {song_id: 1}, headers: api_token_header(@user)
+    post api_v1_favorite_playlist_songs_url, params: {song_id: 1}, as: :json, headers: api_token_header(@user)
     response = @response.parsed_body
 
     assert_response :bad_request
-    assert_equal "RecordNotUnique", response["type"]
+    assert_equal "DuplicatePlaylistSong", response["type"]
     assert_not_empty response["message"]
   end
 end