diff --git a/app/controllers/albums_controller.rb b/app/controllers/albums_controller.rb index 2cd99827..800ff72f 100644 --- a/app/controllers/albums_controller.rb +++ b/app/controllers/albums_controller.rb @@ -29,7 +29,7 @@ def create if @album.save render json: transform_album_for_json(@album), status: :created else - render json: @album.errors, status: :unprocessable_content + render json: transform_errors_for_json(@album), status: :unprocessable_content end end @@ -37,12 +37,12 @@ def update if @album.update(transformed_attributes) render json: transform_album_for_json(@album), status: :ok else - render json: @album.errors, status: :unprocessable_content + render json: transform_errors_for_json(@album), status: :unprocessable_content end end def destroy - render json: @album.errors, status: :unprocessable_content unless @album.destroy + render json: transform_errors_for_json(@album), status: :unprocessable_content unless @album.destroy end def destroy_empty @@ -51,7 +51,7 @@ def destroy_empty end def merge - render json: @album.errors, status: :unprocessable_content unless @album.merge(Album.find(params.expect(:source_id))) + render json: transform_errors_for_json(@album), status: :unprocessable_content unless @album.merge(Album.find(params.expect(:source_id))) end private diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index cfcfb67d..99a0a963 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -2,6 +2,9 @@ class ApplicationController < ActionController::API include Pundit::Authorization include ActionController::HttpAuthentication::Token::ControllerMethods + # This map only includes the type of validation errors that we could have inside the app + ERROR_TYPE_MAP = { blank: :required, taken: :not_unique }.freeze + etag { params[:page] } etag { params[:per_page] } @@ -42,6 +45,15 @@ def stale?(scope:, **) super(etag:, **) end + def transform_error_for_json(error) + { attribute: error.attribute, type: ERROR_TYPE_MAP[error.type] } + end + + # This method expects an instance of a class that includes `ActiveModel::Errors` + def transform_errors_for_json(object) + { errors: object.errors.errors.map { transform_error_for_json(it) } } + end + private def authenticate_user @@ -52,16 +64,11 @@ def authenticate_user end def user_not_authorized(exc) - policy_name = exc.policy.class.to_s.underscore - status = current_user.present? ? :forbidden : :unauthorized - render json: { status => [I18n.t("#{policy_name}.#{exc.query}", - scope: 'pundit', - default: :default)] }, - status: + render json: { errors: [{ policy: exc.policy.class.to_s.underscore, type: status, action: exc.query }] }, status: end def model_not_found(exc) - render json: { not_found: ["#{exc.model.pluralize.downcase}.not-found"] }, status: :not_found + render json: { errors: [{ model: exc.model.downcase, type: :not_found }] }, status: :not_found end end diff --git a/app/controllers/artists_controller.rb b/app/controllers/artists_controller.rb index a53736b0..b5af9a98 100644 --- a/app/controllers/artists_controller.rb +++ b/app/controllers/artists_controller.rb @@ -26,7 +26,7 @@ def create if @artist.save render json: transform_artist_for_json(@artist), status: :created else - render json: @artist.errors, status: :unprocessable_content + render json: transform_errors_for_json(@artist), status: :unprocessable_content end end @@ -34,12 +34,12 @@ def update if @artist.update(transformed_attributes) render json: transform_artist_for_json(@artist), status: :ok else - render json: @artist.errors, status: :unprocessable_content + render json: transform_errors_for_json(@artist), status: :unprocessable_content end end def destroy - render json: @artist.errors, status: :unprocessable_content unless @artist.destroy + render json: transform_errors_for_json(@artist), status: :unprocessable_content unless @artist.destroy end def destroy_empty @@ -51,7 +51,7 @@ def destroy_empty end def merge - render json: @artist.errors, status: :unprocessable_content unless @artist.merge(Artist.find(params.expect(:source_id))) + render json: transform_errors_for_json(@artist), status: :unprocessable_content unless @artist.merge(Artist.find(params.expect(:source_id))) end private diff --git a/app/controllers/auth_tokens_controller.rb b/app/controllers/auth_tokens_controller.rb index 294fb671..f646ec8d 100644 --- a/app/controllers/auth_tokens_controller.rb +++ b/app/controllers/auth_tokens_controller.rb @@ -19,8 +19,7 @@ def create user = User.find_by(name: params[:name]) unless user.try(:authenticate, params[:password]) - render json: { unauthorized: [I18n.t('auth_tokens.create.wrong_credentials')] }, - status: :unauthorized + render json: { errors: [{ attribute: :base, type: :wrong_credentials }] }, status: :unauthorized return end @@ -33,12 +32,12 @@ def create if @auth_token.save render json: transform_auth_token_for_json_with_token(@auth_token), status: :created else - render json: @auth_token.errors, status: :unprocessable_content + render json: transform_errors_for_json(@auth_token), status: :unprocessable_content end end def destroy - render json: @auth_token.errors, status: :unprocessable_content unless @auth_token.destroy + render json: transform_errors_for_json(@auth_token), status: :unprocessable_content unless @auth_token.destroy end private diff --git a/app/controllers/codec_conversions_controller.rb b/app/controllers/codec_conversions_controller.rb index f0af3e32..dd0f169c 100644 --- a/app/controllers/codec_conversions_controller.rb +++ b/app/controllers/codec_conversions_controller.rb @@ -23,7 +23,7 @@ def create if @codec_conversion.save render json: transform_codec_conversion_for_json(@codec_conversion), status: :created else - render json: @codec_conversion.errors, status: :unprocessable_content + render json: transform_errors_for_json(@codec_conversion), status: :unprocessable_content end end @@ -31,12 +31,12 @@ def update if @codec_conversion.update(permitted_attributes(CodecConversion)) render json: transform_codec_conversion_for_json(@codec_conversion), status: :ok else - render json: @codec_conversion.errors, status: :unprocessable_content + render json: transform_errors_for_json(@codec_conversion), status: :unprocessable_content end end def destroy - render json: @codec_conversion.errors, status: :unprocessable_content unless @codec_conversion.destroy + render json: transform_errors_for_json(@codec_conversion), status: :unprocessable_content unless @codec_conversion.destroy end private diff --git a/app/controllers/codecs_controller.rb b/app/controllers/codecs_controller.rb index 4b5ea0d2..b25ad3ad 100644 --- a/app/controllers/codecs_controller.rb +++ b/app/controllers/codecs_controller.rb @@ -21,7 +21,7 @@ def create if @codec.save render json: transform_codec_for_json(@codec), status: :created else - render json: @codec.errors, status: :unprocessable_content + render json: transform_errors_for_json(@codec), status: :unprocessable_content end end @@ -29,12 +29,12 @@ def update if @codec.update(permitted_attributes(@codec)) render json: transform_codec_for_json(@codec), status: :ok else - render json: @codec.errors, status: :unprocessable_content + render json: transform_errors_for_json(@codec), status: :unprocessable_content end end def destroy - render json: @codec.errors, status: :unprocessable_content unless @codec.destroy + render json: transform_errors_for_json(@codec), status: :unprocessable_content unless @codec.destroy end private diff --git a/app/controllers/cover_filenames_controller.rb b/app/controllers/cover_filenames_controller.rb index 2cedc366..a5b06daf 100644 --- a/app/controllers/cover_filenames_controller.rb +++ b/app/controllers/cover_filenames_controller.rb @@ -21,12 +21,12 @@ def create if @cover_filename.save render json: transform_cover_filename_for_json(@cover_filename), status: :created else - render json: @cover_filename.errors, status: :unprocessable_content + render json: transform_errors_for_json(@cover_filename), status: :unprocessable_content end end def destroy - render json: @cover_filename.errors, status: :unprocessable_content unless @cover_filename.destroy + render json: transform_errors_for_json(@cover_filename), status: :unprocessable_content unless @cover_filename.destroy end private diff --git a/app/controllers/genres_controller.rb b/app/controllers/genres_controller.rb index 395476ca..b75b113d 100644 --- a/app/controllers/genres_controller.rb +++ b/app/controllers/genres_controller.rb @@ -21,7 +21,7 @@ def create if @genre.save render json: transform_genre_for_json(@genre), status: :created else - render json: @genre.errors, status: :unprocessable_content + render json: transform_errors_for_json(@genre), status: :unprocessable_content end end @@ -29,12 +29,12 @@ def update if @genre.update(permitted_attributes(@genre)) render json: transform_genre_for_json(@genre), status: :ok else - render json: @genre.errors, status: :unprocessable_content + render json: transform_errors_for_json(@genre), status: :unprocessable_content end end def destroy - render json: @genre.errors, status: :unprocessable_content unless @genre.destroy + render json: transform_errors_for_json(@genre), status: :unprocessable_content unless @genre.destroy end def destroy_empty diff --git a/app/controllers/image_types_controller.rb b/app/controllers/image_types_controller.rb index 4fa7fbd7..d9d71d0a 100644 --- a/app/controllers/image_types_controller.rb +++ b/app/controllers/image_types_controller.rb @@ -21,7 +21,7 @@ def create if @image_type.save render json: transform_image_type_for_json(@image_type), status: :created else - render json: @image_type.errors, status: :unprocessable_content + render json: transform_errors_for_json(@image_type), status: :unprocessable_content end end @@ -29,12 +29,12 @@ def update if @image_type.update(permitted_attributes(@image_type)) render json: transform_image_type_for_json(@image_type), status: :ok else - render json: @image_type.errors, status: :unprocessable_content + render json: transform_errors_for_json(@image_type), status: :unprocessable_content end end def destroy - render json: @image_type.errors, status: :unprocessable_content unless @image_type.destroy + render json: transform_errors_for_json(@image_type), status: :unprocessable_content unless @image_type.destroy end private diff --git a/app/controllers/labels_controller.rb b/app/controllers/labels_controller.rb index 7ff4e58c..37383420 100644 --- a/app/controllers/labels_controller.rb +++ b/app/controllers/labels_controller.rb @@ -21,7 +21,7 @@ def create if @label.save render json: transform_label_for_json(@label), status: :created else - render json: @label.errors, status: :unprocessable_content + render json: transform_errors_for_json(@label), status: :unprocessable_content end end @@ -29,12 +29,12 @@ def update if @label.update(permitted_attributes(@label)) render json: transform_label_for_json(@label), status: :ok else - render json: @label.errors, status: :unprocessable_content + render json: transform_errors_for_json(@label), status: :unprocessable_content end end def destroy - render json: @label.errors, status: :unprocessable_content unless @label.destroy + render json: transform_errors_for_json(@label), status: :unprocessable_content unless @label.destroy end def destroy_empty diff --git a/app/controllers/locations_controller.rb b/app/controllers/locations_controller.rb index 427aa940..0f2a0fb0 100644 --- a/app/controllers/locations_controller.rb +++ b/app/controllers/locations_controller.rb @@ -21,12 +21,12 @@ def create if @location.save render json: transform_location_for_json(@location), status: :created else - render json: @location.errors, status: :unprocessable_content + render json: transform_errors_for_json(@location), status: :unprocessable_content end end def destroy - render json: @location.errors, status: :unprocessable_content unless @location.destroy + render json: transform_errors_for_json(@location), status: :unprocessable_content unless @location.destroy end private diff --git a/app/controllers/playlists_controller.rb b/app/controllers/playlists_controller.rb index 84effa70..588b3cfc 100644 --- a/app/controllers/playlists_controller.rb +++ b/app/controllers/playlists_controller.rb @@ -21,7 +21,7 @@ def create if @playlist.save render json: transform_playlist_for_json(@playlist), status: :created else - render json: @playlist.errors, status: :unprocessable_content + render json: transform_errors_for_json(@playlist), status: :unprocessable_content end end @@ -29,18 +29,18 @@ def update if @playlist.update(permitted_attributes(@playlist)) render json: transform_playlist_for_json(@playlist), status: :ok else - render json: @playlist.errors, status: :unprocessable_content + render json: transform_errors_for_json(@playlist), status: :unprocessable_content end end def destroy - render json: @playlist.errors, status: :unprocessable_content unless @playlist.destroy + render json: transform_errors_for_json(@playlist), status: :unprocessable_content unless @playlist.destroy end def add_item @item = @playlist.items.create(permitted_attributes(@playlist)) - render json: @item.errors, status: :unprocessable_content unless @item.save + render json: transform_errors_for_json(@item), status: :unprocessable_content unless @item.save end private diff --git a/app/controllers/plays_controller.rb b/app/controllers/plays_controller.rb index 52ef0b69..8036bc2c 100644 --- a/app/controllers/plays_controller.rb +++ b/app/controllers/plays_controller.rb @@ -20,7 +20,7 @@ def create if @play.save render json: transform_play_for_json(@play), status: :created else - render json: @play.errors, status: :unprocessable_content + render json: transform_errors_for_json(@play), status: :unprocessable_content end end diff --git a/app/controllers/tracks_controller.rb b/app/controllers/tracks_controller.rb index 0ff5c5cd..fea7f5c3 100644 --- a/app/controllers/tracks_controller.rb +++ b/app/controllers/tracks_controller.rb @@ -26,7 +26,7 @@ def create if @track.save render json: transform_track_for_json(@track), status: :created else - render json: @track.errors, status: :unprocessable_content + render json: transform_errors_for_json(@track), status: :unprocessable_content end end @@ -34,12 +34,12 @@ def update if @track.update(transformed_attributes) render json: transform_track_for_json(@track), status: :ok else - render json: @track.errors, status: :unprocessable_content + render json: transform_errors_for_json(@track), status: :unprocessable_content end end def destroy - render json: @track.errors, status: :unprocessable_content unless @track.destroy + render json: transform_errors_for_json(@track), status: :unprocessable_content unless @track.destroy end def destroy_empty diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index cba20173..df5efbdd 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -21,7 +21,7 @@ def create if @user.save render json: transform_user_for_json(@user), status: :created else - render json: @user.errors, status: :unprocessable_content + render json: transform_errors_for_json(@user), status: :unprocessable_content end end @@ -29,20 +29,19 @@ def update if @user == current_user && params[:user][:password].present? && !@user.try(:authenticate, params[:user][:current_password]) - render json: { unauthorized: [I18n.t('users.current_password_is_incorrect')] }, - status: :unauthorized + render json: { errors: [{ attribute: :base, type: :incorrect_password }] }, status: :unauthorized return end if @user.update(permitted_attributes(@user)) render json: transform_user_for_json(@user), status: :ok else - render json: @user.errors, status: :unprocessable_content + render json: transform_errors_for_json(@user), status: :unprocessable_content end end def destroy - render json: @user.errors, status: :unprocessable_content unless @user.destroy + render json: transform_errors_for_json(@user), status: :unprocessable_content unless @user.destroy end private diff --git a/config/locales/en.yml b/config/locales/en.yml deleted file mode 100644 index 7126d8a8..00000000 --- a/config/locales/en.yml +++ /dev/null @@ -1,130 +0,0 @@ -# Files in the config/locales directory are used for internationalization -# and are automatically loaded by Rails. If you want to use locales other -# than English, add the necessary files in this directory. -# -# To use the locales, use `I18n.t`: -# -# I18n.t 'hello' -# -# In views, this is aliased to just `t`: -# -# <%= t('hello') %> -# -# To use a different locale, set it with `I18n.locale`: -# -# I18n.locale = :es -# -# This would use the information in config/locales/es.yml. -# -# The following keys must be escaped otherwise they will not be retrieved by -# the default I18n backend: -# -# true, false, on, off, yes, no -# -# Instead, surround them with single quotes. -# -# en: -# 'true': 'foo' -# -# To learn more, please read the Rails Internationalization guide -# available at http://guides.rubyonrails.org/i18n.html. - -en: - activerecord: - errors: - models: - album: - attributes: - title: - blank: "albums.title-blank" - album_artists: - attributes: - name: - blank: "aa.name-blank" - order: - blank: "aa.order-blank" - album_label: - attributes: - catalogue_number: - blank: "al.cat-blank" - artist: - attributes: - name: - blank: "artists.name-blank" - codec: - attributes: - mimetype: - blank: "codec.mime-blank" - extension: - blank: "codec.ext-blank" - taken: "codec.ext-taken" - codec_conversion: - attributes: - name: - blank: "codecconv.nane-blank" - taken: "codecconv.name-taken" - ffmpeg_params: - blank: "codecconv.ffmpeg-blank" - resulting_codec: - blank: "codecconv.result-blank" - genre: - attributes: - name: - blank: "genre.name-blank" - taken: "genre.name-taken" - image_type: - attributes: - mimetype: - blank: "image.mime-blank" - extension: - blank: "image.ext-blank" - taken: "image.ext-taken" - label: - attributes: - name: - blank: "label.name-blank" - location: - attributes: - path: - blank: "location.path-blank" - taken: "location.path-taken" - play: - attributes: - played_at: - blank: "play.played-at-blank" - track: - attributes: - title: - blank: "tracks.title-blank" - number: - blank: "tracks.number-blank" - track_artist: - attributes: - artist: - blank: "ta.artist-blank" - name: - blank: "ta.name-blank" - taken: "ta.name-taken" - order: - blank: "ta.order-blank" - role: - blank: "ta.role-blank" - track: - blank: "ta.track-blank" - user: - attributes: - name: - blank: "user.name-blank" - password_confirmation: - confirmation: "user.password-confirmation" - password_digest: - blank: "user.password-blank" - permission: - blank: "user.permission-blank" - auth_tokens: - create: - wrong_credentials: 'user.wrong-credentials' - users: - current_password_is_incorrect: 'user.current-password-incorrect' - pundit: - default: 'not-authorized' diff --git a/test/controllers/albums_controller_test.rb b/test/controllers/albums_controller_test.rb index 62ef41df..8c4b94eb 100644 --- a/test/controllers/albums_controller_test.rb +++ b/test/controllers/albums_controller_test.rb @@ -39,6 +39,7 @@ class AlbumsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'album_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should create album for moderator' do @@ -74,6 +75,7 @@ class AlbumsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'title', 'type' => 'required' } end test 'should create dependent album_labels' do @@ -153,9 +155,8 @@ class AlbumsControllerTest < ActionDispatch::IntegrationTest patch album_url(@album), params: { album: { release: @album.release, title: '' } } assert_response :unprocessable_content - @album.reload - - assert_not_equal '', @album.title + assert_not_equal '', @album.reload.title + assert_includes response.parsed_body['errors'], { 'attribute' => 'title', 'type' => 'required' } end test 'should clear review comment' do @@ -229,6 +230,7 @@ class AlbumsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'album_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy album for moderator' do @@ -251,6 +253,7 @@ class AlbumsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'album_policy', 'type' => 'forbidden', 'action' => 'destroy_empty?' } end test 'should destroy empty albums for moderator' do @@ -279,6 +282,7 @@ class AlbumsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'album_policy', 'type' => 'forbidden', 'action' => 'merge?' } end test 'should merge albums for moderator' do diff --git a/test/controllers/artists_controller_test.rb b/test/controllers/artists_controller_test.rb index 6a701e59..7eb3c270 100644 --- a/test/controllers/artists_controller_test.rb +++ b/test/controllers/artists_controller_test.rb @@ -39,6 +39,7 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'artist_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should create artist for moderator' do @@ -69,6 +70,7 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should show artist' do @@ -91,9 +93,8 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest patch artist_url(@artist), params: { artist: { name: '' } } assert_response :unprocessable_content - @artist.reload - - assert_not_equal '', @artist.name + assert_not_equal '', @artist.reload.name + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should not update artist metadata for user' do @@ -176,6 +177,7 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'artist_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy artist for moderator' do @@ -198,6 +200,7 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'artist_policy', 'type' => 'forbidden', 'action' => 'destroy_empty?' } end test 'should destroy empty artists for moderator (track_artist)' do @@ -248,6 +251,7 @@ class ArtistsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'artist_policy', 'type' => 'forbidden', 'action' => 'merge?' } end test 'should merge artists for moderator' do diff --git a/test/controllers/auth_tokens_controller_test.rb b/test/controllers/auth_tokens_controller_test.rb index 942f8f48..7456ba71 100644 --- a/test/controllers/auth_tokens_controller_test.rb +++ b/test/controllers/auth_tokens_controller_test.rb @@ -63,6 +63,7 @@ class AuthTokensControllerTest < ActionDispatch::IntegrationTest end assert_response :unauthorized + assert_includes response.parsed_body['errors'], { 'attribute' => 'base', 'type' => 'wrong_credentials' } end test 'should not create auth_token without user_agent' do @@ -74,6 +75,7 @@ class AuthTokensControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'user_agent', 'type' => 'required' } end test 'should show auth_token' do diff --git a/test/controllers/codec_conversions_controller_test.rb b/test/controllers/codec_conversions_controller_test.rb index 3691eaeb..62ee6ad6 100644 --- a/test/controllers/codec_conversions_controller_test.rb +++ b/test/controllers/codec_conversions_controller_test.rb @@ -52,6 +52,7 @@ class CodecConversionsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'codec_conversion_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should create codec_conversion for moderator' do @@ -90,6 +91,7 @@ class CodecConversionsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'not_unique' } end test 'should not create codec_conversion with empty ffmpeg_params' do @@ -103,6 +105,7 @@ class CodecConversionsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'ffmpeg_params', 'type' => 'required' } end test 'should not create codec_conversion with empty name' do @@ -116,6 +119,7 @@ class CodecConversionsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should not create codec_conversion with non-existing resulting_codec' do @@ -130,6 +134,7 @@ class CodecConversionsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'resulting_codec', 'type' => 'required' } end test 'should create codec_conversion for admin' do @@ -160,6 +165,7 @@ class CodecConversionsControllerTest < ActionDispatch::IntegrationTest } } assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'codec_conversion_policy', 'type' => 'forbidden', 'action' => 'update?' } end test 'should update codec_conversion for moderator' do @@ -175,11 +181,13 @@ class CodecConversionsControllerTest < ActionDispatch::IntegrationTest test 'should not update codec_conversion to empty name' do sign_in_as(create(:moderator)) - patch codec_conversion_url(@codec_conversion), params: { codec_conversion: { - name: '' - } } + + assert_no_changes '@codec_conversion.reload.name' do + patch codec_conversion_url(@codec_conversion), params: { codec_conversion: { name: '' } } + end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should update codec_conversion for admin' do @@ -199,6 +207,7 @@ class CodecConversionsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'codec_conversion_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy codec_conversion for moderator' do diff --git a/test/controllers/codecs_controller_test.rb b/test/controllers/codecs_controller_test.rb index 5a47c906..b636a072 100644 --- a/test/controllers/codecs_controller_test.rb +++ b/test/controllers/codecs_controller_test.rb @@ -40,6 +40,7 @@ class CodecsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'codec_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should not create codec with missing extension' do @@ -50,16 +51,19 @@ class CodecsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'extension', 'type' => 'required' } end test 'should not create codec with missing mimetype' do sign_in_as(create(:moderator)) codec = build(:codec) + assert_difference('Codec.count', 0) do post codecs_url, params: { codec: { extension: codec.extension } } end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'mimetype', 'type' => 'required' } end test 'should create codec for moderator' do @@ -92,13 +96,18 @@ class CodecsControllerTest < ActionDispatch::IntegrationTest patch codec_url(@codec), params: { codec: { mimetype: @codec.mimetype } } assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'codec_policy', 'type' => 'forbidden', 'action' => 'update?' } end test 'should not update codec when clearing mimetype' do sign_in_as(create(:moderator)) - patch codec_url(@codec), params: { codec: { mimetype: '' } } + + assert_no_changes '@codec.reload.mimetype' do + patch codec_url(@codec), params: { codec: { mimetype: '' } } + end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'mimetype', 'type' => 'required' } end test 'should update codec for moderator' do @@ -121,6 +130,7 @@ class CodecsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'codec_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy codec for moderator' do diff --git a/test/controllers/cover_filenames_controller_test.rb b/test/controllers/cover_filenames_controller_test.rb index 40b5a0c6..51da2a42 100644 --- a/test/controllers/cover_filenames_controller_test.rb +++ b/test/controllers/cover_filenames_controller_test.rb @@ -10,6 +10,7 @@ class CoverFilenamesControllerTest < ActionDispatch::IntegrationTest get cover_filenames_url assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'cover_filename_policy', 'type' => 'forbidden', 'action' => 'index?' } end test 'should get index for moderator' do @@ -59,6 +60,7 @@ class CoverFilenamesControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'cover_filename_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should not create cover_filename with empty filename' do @@ -68,6 +70,7 @@ class CoverFilenamesControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'filename', 'type' => 'required' } end test 'should create cover_filename for moderator' do @@ -94,6 +97,7 @@ class CoverFilenamesControllerTest < ActionDispatch::IntegrationTest get cover_filename_url(@cover_filename) assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'cover_filename_policy', 'type' => 'forbidden', 'action' => 'show?' } end test 'should show cover_filename for moderator' do @@ -116,6 +120,7 @@ class CoverFilenamesControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'cover_filename_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy cover_filename for moderator' do diff --git a/test/controllers/genres_controller_test.rb b/test/controllers/genres_controller_test.rb index 8597d9fd..09be283d 100644 --- a/test/controllers/genres_controller_test.rb +++ b/test/controllers/genres_controller_test.rb @@ -40,6 +40,7 @@ class GenresControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'genre_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should not create genre with empty name' do @@ -49,6 +50,7 @@ class GenresControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should create genre for moderator' do @@ -81,6 +83,7 @@ class GenresControllerTest < ActionDispatch::IntegrationTest patch genre_url(@genre), params: { genre: { name: @genre.name } } assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'genre_policy', 'type' => 'forbidden', 'action' => 'update?' } end test 'should not update genre to empty name' do @@ -88,9 +91,8 @@ class GenresControllerTest < ActionDispatch::IntegrationTest patch genre_url(@genre), params: { genre: { name: '' } } assert_response :unprocessable_content - @genre.reload - - assert_not_equal '', @genre.name + assert_not_equal '', @genre.reload.name + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should update genre for moderator' do @@ -113,6 +115,7 @@ class GenresControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'genre_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy genre for moderator' do @@ -139,6 +142,7 @@ class GenresControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'genre_policy', 'type' => 'forbidden', 'action' => 'destroy_empty?' } end test 'should destroy empty genres for moderator' do @@ -179,6 +183,7 @@ class GenresControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'genre_policy', 'type' => 'forbidden', 'action' => 'merge?' } end test 'should merge genres for moderator' do diff --git a/test/controllers/image_types_controller_test.rb b/test/controllers/image_types_controller_test.rb index 8f4b6f94..430bdb48 100644 --- a/test/controllers/image_types_controller_test.rb +++ b/test/controllers/image_types_controller_test.rb @@ -40,6 +40,7 @@ class ImageTypesControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'image_type_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should not create image_type without extension' do @@ -50,6 +51,7 @@ class ImageTypesControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'extension', 'type' => 'required' } end test 'should not create image_type without mimetype' do @@ -60,6 +62,7 @@ class ImageTypesControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'mimetype', 'type' => 'required' } end test 'should create image_type for moderator' do @@ -92,6 +95,7 @@ class ImageTypesControllerTest < ActionDispatch::IntegrationTest patch image_type_url(@image_type), params: { image_type: { mimetype: @image_type.mimetype } } assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'image_type_policy', 'type' => 'forbidden', 'action' => 'update?' } end test 'should not update image_type to empty mimetype' do @@ -99,9 +103,8 @@ class ImageTypesControllerTest < ActionDispatch::IntegrationTest patch image_type_url(@image_type), params: { image_type: { mimetype: '' } } assert_response :unprocessable_content - @image_type.reload - - assert_not_equal '', @image_type.mimetype + assert_not_equal '', @image_type.reload.mimetype + assert_includes response.parsed_body['errors'], { 'attribute' => 'mimetype', 'type' => 'required' } end test 'should update image_type for moderator' do @@ -124,6 +127,7 @@ class ImageTypesControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'image_type_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy image_type for moderator' do diff --git a/test/controllers/labels_controller_test.rb b/test/controllers/labels_controller_test.rb index a097fb74..b95e5d83 100644 --- a/test/controllers/labels_controller_test.rb +++ b/test/controllers/labels_controller_test.rb @@ -40,6 +40,7 @@ class LabelsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'label_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should not create label with empty name' do @@ -50,6 +51,7 @@ class LabelsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should create label for moderator' do @@ -82,6 +84,7 @@ class LabelsControllerTest < ActionDispatch::IntegrationTest patch label_url(@label), params: { label: { name: @label.name } } assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'label_policy', 'type' => 'forbidden', 'action' => 'update?' } end test 'should not update label to empty name' do @@ -89,9 +92,8 @@ class LabelsControllerTest < ActionDispatch::IntegrationTest patch label_url(@label), params: { label: { name: '' } } assert_response :unprocessable_content - @label.reload - - assert_not_equal '', @label.name + assert_not_equal '', @label.reload.name + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should update label for moderator' do @@ -114,6 +116,7 @@ class LabelsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'label_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy label for moderator' do @@ -140,6 +143,7 @@ class LabelsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'label_policy', 'type' => 'forbidden', 'action' => 'destroy_empty?' } end test 'should destroy empty labels for moderator' do @@ -180,6 +184,7 @@ class LabelsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'label_policy', 'type' => 'forbidden', 'action' => 'merge?' } end test 'should merge labels for moderator' do diff --git a/test/controllers/locations_controller_test.rb b/test/controllers/locations_controller_test.rb index e6f92f44..c1c8dd65 100644 --- a/test/controllers/locations_controller_test.rb +++ b/test/controllers/locations_controller_test.rb @@ -10,6 +10,7 @@ class LocationsControllerTest < ActionDispatch::IntegrationTest get locations_url assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'location_policy', 'type' => 'forbidden', 'action' => 'index?' } end test 'should get index for moderator' do @@ -59,6 +60,7 @@ class LocationsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'location_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should not create location with empty path' do @@ -68,6 +70,7 @@ class LocationsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'path', 'type' => 'required' } end test 'should create location for moderator' do @@ -94,6 +97,7 @@ class LocationsControllerTest < ActionDispatch::IntegrationTest get location_url(@location) assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'location_policy', 'type' => 'forbidden', 'action' => 'show?' } end test 'should show location for moderator' do @@ -116,6 +120,7 @@ class LocationsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'location_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy location for moderator' do diff --git a/test/controllers/playlists_controller_test.rb b/test/controllers/playlists_controller_test.rb index 276c80af..f0576c9b 100644 --- a/test/controllers/playlists_controller_test.rb +++ b/test/controllers/playlists_controller_test.rb @@ -66,6 +66,7 @@ class PlaylistsControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should create personal playlist for current user if specified' do @@ -97,9 +98,12 @@ class PlaylistsControllerTest < ActionDispatch::IntegrationTest end test 'should not update playlist with empty name' do - patch playlist_url(@playlist), params: { playlist: { name: '' } } + assert_no_changes '@playlist.reload.name' do + patch playlist_url(@playlist), params: { playlist: { name: '' } } + end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should create playlist items during update' do @@ -121,6 +125,7 @@ class PlaylistsControllerTest < ActionDispatch::IntegrationTest patch playlist_url(@playlist), params: { playlist: { name: 'My playlist' } } assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'playlist_policy', 'type' => 'forbidden', 'action' => 'update?' } end test 'should not update secret playlist for different user' do @@ -129,6 +134,7 @@ class PlaylistsControllerTest < ActionDispatch::IntegrationTest patch playlist_url(@playlist), params: { playlist: { name: 'My playlist' } } assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'playlist_policy', 'type' => 'forbidden', 'action' => 'update?' } end test 'should destroy shared playlist for user' do @@ -147,6 +153,7 @@ class PlaylistsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'playlist_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should not destroy secret playlist for different user' do @@ -157,6 +164,7 @@ class PlaylistsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'playlist_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should not add item if no user' do @@ -169,6 +177,7 @@ class PlaylistsControllerTest < ActionDispatch::IntegrationTest end assert_response :unauthorized + assert_includes response.parsed_body['errors'], { 'policy' => 'playlist_policy', 'type' => 'unauthorized', 'action' => 'add_item?' } end test 'should add item in shared playlist' do @@ -204,5 +213,6 @@ class PlaylistsControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'playlist_policy', 'type' => 'forbidden', 'action' => 'add_item?' } end end diff --git a/test/controllers/plays_controller_test.rb b/test/controllers/plays_controller_test.rb index 9f8b3d32..93f90827 100644 --- a/test/controllers/plays_controller_test.rb +++ b/test/controllers/plays_controller_test.rb @@ -75,6 +75,7 @@ class PlaysControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'played_at', 'type' => 'required' } end test 'should get stats and not return play stats for other users' do diff --git a/test/controllers/rescans_controller_test.rb b/test/controllers/rescans_controller_test.rb index 50d21ca8..9a6688dd 100644 --- a/test/controllers/rescans_controller_test.rb +++ b/test/controllers/rescans_controller_test.rb @@ -12,6 +12,7 @@ class RescansControllerTest < ActionDispatch::IntegrationTest get rescans_url assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'rescan_runner_policy', 'type' => 'forbidden', 'action' => 'index?' } end test 'should get index for moderator' do @@ -58,6 +59,7 @@ class RescansControllerTest < ActionDispatch::IntegrationTest get rescan_url(@runner) assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'rescan_runner_policy', 'type' => 'forbidden', 'action' => 'show?' } end test 'should get show for moderator' do @@ -71,6 +73,7 @@ class RescansControllerTest < ActionDispatch::IntegrationTest post rescan_url(@runner) assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'rescan_runner_policy', 'type' => 'forbidden', 'action' => 'start?' } end test 'should start rescan' do @@ -91,6 +94,7 @@ class RescansControllerTest < ActionDispatch::IntegrationTest post rescans_url assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'rescan_runner_policy', 'type' => 'forbidden', 'action' => 'start_all?' } end test 'should start all rescans' do diff --git a/test/controllers/tracks_controller_test.rb b/test/controllers/tracks_controller_test.rb index aea0e41e..d131c8ca 100644 --- a/test/controllers/tracks_controller_test.rb +++ b/test/controllers/tracks_controller_test.rb @@ -63,6 +63,7 @@ class TracksControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'track_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should not create track without title' do @@ -72,6 +73,7 @@ class TracksControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'title', 'type' => 'required' } end test 'should not create track without album_id' do @@ -81,6 +83,7 @@ class TracksControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'album', 'type' => 'required' } end test 'should create track for moderator' do @@ -156,9 +159,8 @@ class TracksControllerTest < ActionDispatch::IntegrationTest patch track_url(@track), params: { track: { title: '' } } assert_response :unprocessable_content - @track.reload - - assert_not_equal '', @track.title + assert_not_equal '', @track.reload.title + assert_includes response.parsed_body['errors'], { 'attribute' => 'title', 'type' => 'required' } end test 'should clear review comment' do @@ -187,6 +189,7 @@ class TracksControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'track_policy', 'type' => 'forbidden', 'action' => 'destroy?' } end test 'should destroy track for moderator' do @@ -205,6 +208,7 @@ class TracksControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'track_policy', 'type' => 'forbidden', 'action' => 'destroy_empty?' } end test 'should destroy empty tracks for moderator' do @@ -231,6 +235,7 @@ class TracksControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'track_policy', 'type' => 'forbidden', 'action' => 'merge?' } end test 'should merge tracks for moderator' do @@ -256,6 +261,7 @@ class TracksControllerTest < ActionDispatch::IntegrationTest get audio_track_url(create(:track)) assert_response :not_found + assert_includes response.parsed_body['errors'], { 'model' => 'audio', 'type' => 'not_found' } end test 'should return not_found and destroy audio if file is missing ' do @@ -266,6 +272,7 @@ class TracksControllerTest < ActionDispatch::IntegrationTest end assert_response :not_found + assert_includes response.parsed_body['errors'], { 'model' => 'audio', 'type' => 'not_found' } end test 'should serve audio to user' do @@ -306,6 +313,7 @@ class TracksControllerAudioTest < ActionDispatch::IntegrationTest get audio_track_url(track, codec_conversion_id: 0) assert_response :not_found + assert_includes response.parsed_body['errors'], { 'model' => 'codec_conversion', 'type' => 'not_found' } end test 'should create transcoded_item if codec_conversion is present' do diff --git a/test/controllers/users_controller_test.rb b/test/controllers/users_controller_test.rb index b66db3d1..84bb7527 100644 --- a/test/controllers/users_controller_test.rb +++ b/test/controllers/users_controller_test.rb @@ -40,6 +40,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest end assert_response :forbidden + assert_includes response.parsed_body['errors'], { 'policy' => 'user_policy', 'type' => 'forbidden', 'action' => 'create?' } end test 'should not create user without name' do @@ -50,6 +51,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest end assert_response :unprocessable_content + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should create user for admin' do @@ -78,6 +80,7 @@ class UsersControllerTest < ActionDispatch::IntegrationTest patch user_url(@user), params: { user: { password: 'new password', current_password: 'not correct' } } assert_response :unauthorized + assert_includes response.parsed_body['errors'], { 'attribute' => 'base', 'type' => 'incorrect_password' } end test 'should update password with current password for current user' do @@ -98,9 +101,8 @@ class UsersControllerTest < ActionDispatch::IntegrationTest patch user_url(@user), params: { user: { name: '' } } assert_response :unprocessable_content - @user.reload - - assert_not_equal '', @user.name + assert_not_equal '', @user.reload.name + assert_includes response.parsed_body['errors'], { 'attribute' => 'name', 'type' => 'required' } end test 'should not update own permission if not admin' do