From 144b5cab5860e209339d05c95463b4660158bcbf Mon Sep 17 00:00:00 2001 From: Justin Edmund Date: Mon, 22 Sep 2025 02:51:50 -0700 Subject: [PATCH] Return proper REST response for deleting a party for more endpoints --- .../api/v1/favorites_controller.rb | 11 ++++-- .../api/v1/grid_characters_controller.rb | 8 +++- .../api/v1/grid_summons_controller.rb | 8 +++- .../api/v1/grid_weapons_controller.rb | 8 +++- app/controllers/api/v1/parties_controller.rb | 8 +++- .../concerns/party_authorization_concern.rb | 2 + .../api/v1/party_deletion_failed_error.rb | 38 +++++++++++++++++++ app/models/party.rb | 9 ++--- 8 files changed, 79 insertions(+), 13 deletions(-) create mode 100644 app/errors/api/v1/party_deletion_failed_error.rb diff --git a/app/controllers/api/v1/favorites_controller.rb b/app/controllers/api/v1/favorites_controller.rb index 4f5279b..4792213 100644 --- a/app/controllers/api/v1/favorites_controller.rb +++ b/app/controllers/api/v1/favorites_controller.rb @@ -31,10 +31,15 @@ module Api raise Api::V1::UnauthorizedError unless current_user @favorite = Favorite.where(user_id: current_user.id, party_id: favorite_params[:party_id]).first - render_not_found_response('favorite') unless @favorite + return render_not_found_response('favorite') unless @favorite - render_error("Couldn't delete favorite") unless Favorite.destroy(@favorite.id) - render json: FavoriteBlueprint.render(@favorite, root: :favorite, view: :destroyed) + if Favorite.destroy(@favorite.id) + render json: FavoriteBlueprint.render(@favorite, root: :favorite, view: :destroyed) + else + render_unprocessable_entity_response( + Api::V1::GranblueError.new("Couldn't delete favorite") + ) + end end private diff --git a/app/controllers/api/v1/grid_characters_controller.rb b/app/controllers/api/v1/grid_characters_controller.rb index dae8c9f..e5555f9 100644 --- a/app/controllers/api/v1/grid_characters_controller.rb +++ b/app/controllers/api/v1/grid_characters_controller.rb @@ -228,7 +228,13 @@ module Api return render_not_found_response('grid_character') if grid_character.nil? - render json: GridCharacterBlueprint.render(grid_character, view: :destroyed) if grid_character.destroy + if grid_character.destroy + render json: GridCharacterBlueprint.render(grid_character, view: :destroyed) + else + render_unprocessable_entity_response( + Api::V1::GranblueError.new(grid_character.errors.full_messages.join(', ')) + ) + end end private diff --git a/app/controllers/api/v1/grid_summons_controller.rb b/app/controllers/api/v1/grid_summons_controller.rb index 4d785bf..ab49093 100644 --- a/app/controllers/api/v1/grid_summons_controller.rb +++ b/app/controllers/api/v1/grid_summons_controller.rb @@ -220,7 +220,13 @@ module Api return render_not_found_response('grid_summon') if grid_summon.nil? - render json: GridSummonBlueprint.render(grid_summon, view: :destroyed), status: :ok if grid_summon.destroy + if grid_summon.destroy + render json: GridSummonBlueprint.render(grid_summon, view: :destroyed), status: :ok + else + render_unprocessable_entity_response( + Api::V1::GranblueError.new(grid_summon.errors.full_messages.join(', ')) + ) + end end ## diff --git a/app/controllers/api/v1/grid_weapons_controller.rb b/app/controllers/api/v1/grid_weapons_controller.rb index 75e3c4c..18d148f 100644 --- a/app/controllers/api/v1/grid_weapons_controller.rb +++ b/app/controllers/api/v1/grid_weapons_controller.rb @@ -212,7 +212,13 @@ module Api return render_not_found_response('grid_weapon') if grid_weapon.nil? - render json: GridWeaponBlueprint.render(grid_weapon, view: :destroyed), status: :ok if grid_weapon.destroy + if grid_weapon.destroy + render json: GridWeaponBlueprint.render(grid_weapon, view: :destroyed), status: :ok + else + render_unprocessable_entity_response( + Api::V1::GranblueError.new(grid_weapon.errors.full_messages.join(', ')) + ) + end end private diff --git a/app/controllers/api/v1/parties_controller.rb b/app/controllers/api/v1/parties_controller.rb index 6ec900d..e255d90 100644 --- a/app/controllers/api/v1/parties_controller.rb +++ b/app/controllers/api/v1/parties_controller.rb @@ -81,7 +81,13 @@ module Api # Deletes a party. def destroy - head :no_content if @party.destroy + if @party.destroy + head :no_content + else + render_unprocessable_entity_response( + Api::V1::PartyDeletionFailedError.new(@party.errors.full_messages) + ) + end end # Extended Party Actions diff --git a/app/controllers/concerns/party_authorization_concern.rb b/app/controllers/concerns/party_authorization_concern.rb index af72ce0..8d57277 100644 --- a/app/controllers/concerns/party_authorization_concern.rb +++ b/app/controllers/concerns/party_authorization_concern.rb @@ -5,6 +5,8 @@ module PartyAuthorizationConcern # Checks whether the current user (or provided edit key) is authorized to modify @party. def authorize_party! + return render_not_found_response('party') unless @party + if @party.user.present? render_unauthorized_response unless current_user.present? && @party.user == current_user else diff --git a/app/errors/api/v1/party_deletion_failed_error.rb b/app/errors/api/v1/party_deletion_failed_error.rb new file mode 100644 index 0000000..97a1b66 --- /dev/null +++ b/app/errors/api/v1/party_deletion_failed_error.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module Api + module V1 + class PartyDeletionFailedError < StandardError + attr_reader :errors + + def initialize(errors = []) + @errors = errors + super(message) + end + + def http_status + 422 + end + + def code + 'party_deletion_failed' + end + + def message + if @errors.any? + "Failed to delete party: #{@errors.join(', ')}" + else + 'Failed to delete party due to an unknown error' + end + end + + def to_hash + { + message: message, + code: code, + errors: @errors + } + end + end + end +end \ No newline at end of file diff --git a/app/models/party.rb b/app/models/party.rb index f15acff..27ae673 100644 --- a/app/models/party.rb +++ b/app/models/party.rb @@ -140,22 +140,19 @@ class Party < ApplicationRecord has_many :characters, foreign_key: 'party_id', class_name: 'GridCharacter', - counter_cache: true, - dependent: :destroy, + dependent: :delete_all, inverse_of: :party has_many :weapons, foreign_key: 'party_id', class_name: 'GridWeapon', - counter_cache: true, - dependent: :destroy, + dependent: :delete_all, inverse_of: :party has_many :summons, foreign_key: 'party_id', class_name: 'GridSummon', - counter_cache: true, - dependent: :destroy, + dependent: :delete_all, inverse_of: :party has_many :favorites, dependent: :destroy