From d81119bf6490d105ee9a726c42c05acd6a15aadd Mon Sep 17 00:00:00 2001 From: Justin Edmund Date: Mon, 10 Feb 2025 05:29:51 -0800 Subject: [PATCH] Refactor GridSummonsController and add tests --- .../api/v1/grid_summons_controller.rb | 262 +++++++++++++----- .../grid_characters_controller_spec.rb | 213 ++++++++++++++ 2 files changed, 410 insertions(+), 65 deletions(-) create mode 100644 spec/requests/grid_characters_controller_spec.rb diff --git a/app/controllers/api/v1/grid_summons_controller.rb b/app/controllers/api/v1/grid_summons_controller.rb index 4fd6e6e..08450c6 100644 --- a/app/controllers/api/v1/grid_summons_controller.rb +++ b/app/controllers/api/v1/grid_summons_controller.rb @@ -2,6 +2,11 @@ module Api module V1 + ## + # Controller responsible for managing grid summons for a party. + # Provides actions to create, update, and destroy grid summons. + # + # @note All actions assume that the request has been authorized. class GridSummonsController < Api::V1::ApiController attr_reader :party, :incoming_summon @@ -10,56 +15,85 @@ module Api before_action :find_incoming_summon, only: :create before_action :authorize, only: %i[create update update_uncap_level update_quick_summon destroy] + ## + # Creates a new grid summon. + # + # This method builds a new grid summon using the permitted parameters merged + # with the party and summon IDs. It ensures that the `uncap_level` is set to the + # maximum allowed level if not provided. Depending on validation, it will either save + # the summon, handle conflict resolution, or render a validation error response. + # + # @return [void] def create - # Create the GridSummon with the desired parameters - summon = GridSummon.new - summon.attributes = summon_params.merge(party_id: party.id, summon_id: incoming_summon.id) - summon.uncap_level = max_uncap_level(summon) if summon.uncap_level.nil? + # Build a new grid summon using permitted parameters merged with party and summon IDs. + # Then, using `tap`, ensure that the uncap_level is set by using the max_uncap_level helper + # if it hasn't already been provided. + grid_summon = build_grid_summon.tap do |gs| + gs.uncap_level ||= max_uncap_level(gs) + end - if summon.validate - save_summon(summon) + # If the grid summon is valid (i.e. it passes all validations), then save it normally. + if grid_summon.valid? + save_summon(grid_summon) + # If it is invalid due to a conflict error, handle the conflict resolution flow. + elsif conflict_error?(grid_summon) + handle_conflict(grid_summon) + # If there's some other kind of validation error, render the validation error response back to the client. else - handle_conflict(summon) + render_validation_error_response(grid_summon) end end + ## + # Updates an existing grid summon. + # + # Updates the grid summon attributes using permitted parameters. If the update is successful, + # it renders the updated grid summon view; otherwise, it renders a validation error response. + # + # @return [void] def update @summon.attributes = summon_params return render json: GridSummonBlueprint.render(@summon, view: :nested, root: :grid_summon) if @summon.save - render_validation_error_response(@character) + render_validation_error_response(@summon) end + ## + # Updates the uncap level and transcendence step of a grid summon. + # + # This action recalculates the maximum allowed uncap level based on the summon attributes + # and applies business logic to adjust the uncap level and transcendence step accordingly. + # On success, it renders the updated grid summon view; otherwise, it renders a validation error response. + # + # @return [void] def update_uncap_level summon = @summon.summon - max_uncap_level = max_uncap_level(summon) + max_level = max_uncap_level(summon) - greater_than_max_uncap = summon_params[:uncap_level].to_i > max_uncap_level - can_be_transcended = summon.transcendence && summon_params[:transcendence_step] && summon_params[:transcendence_step]&.to_i&.positive? + greater_than_max_uncap = summon_params[:uncap_level].to_i > max_level + can_be_transcended = summon.transcendence && + summon_params[:transcendence_step].present? && + summon_params[:transcendence_step].to_i.positive? - uncap_level = if greater_than_max_uncap || can_be_transcended - max_uncap_level - else - summon_params[:uncap_level] - end + new_uncap_level = greater_than_max_uncap || can_be_transcended ? max_level : summon_params[:uncap_level] + new_transcendence_step = summon.transcendence && summon_params[:transcendence_step].present? ? summon_params[:transcendence_step] : 0 - transcendence_step = if summon.transcendence && summon_params[:transcendence_step] - summon_params[:transcendence_step] - else - 0 - end - - @summon.update!( - uncap_level: uncap_level, - transcendence_step: transcendence_step - ) - - return unless @summon.persisted? - - render json: GridSummonBlueprint.render(@summon, view: :nested, root: :grid_summon) + if @summon.update(uncap_level: new_uncap_level, transcendence_step: new_transcendence_step) + render json: GridSummonBlueprint.render(@summon, view: :nested, root: :grid_summon) + else + render_validation_error_response(@summon) + end end + ## + # Updates the quick summon status for a grid summon. + # + # If the grid summon is in positions 4, 5, or 6, no update is performed. + # Otherwise, it disables quick summon for all other summons in the party, + # updates the current summon, and renders the updated list of summons. + # + # @return [void] def update_quick_summon return if [4, 5, 6].include?(@summon.position) @@ -78,6 +112,31 @@ module Api render json: GridSummonBlueprint.render(summons, view: :nested, root: :summons) end + # + # Destroys a grid summon. + # + # Finds the grid summon by ID. If not found, renders a not-found response. + # If the current user is not authorized to perform the deletion, renders an unauthorized response. + # On successful destruction, renders the destroyed grid summon view. + # + # @return [void] + def destroy + grid_summon = GridSummon.find_by('id = ?', params[:id]) + + return render_not_found_response('grid_summon') if grid_summon.nil? + return render_unauthorized_response if grid_summon.party.user != current_user + + render json: GridSummonBlueprint.render(grid_summon, view: :destroyed), status: :ok if grid_summon.destroy + end + + ## + # Saves the provided grid summon. + # + # If an existing grid summon is found at the specified position for the party, it is replaced. + # On successful save, renders the grid summon view with a created status. + # + # @param summon [GridSummon] The grid summon instance to be saved. + # @return [void] def save_summon(summon) if (grid_summon = GridSummon.where( party_id: party.id, @@ -92,6 +151,15 @@ module Api render json: output, status: :created end + ## + # Handles conflict resolution for a grid summon. + # + # If a conflict is detected and the conflicting summon matches the incoming summon, + # the method updates the conflicting summon’s position with the new position. + # On a successful update, renders the updated grid summon view. + # + # @param summon [GridSummon] The grid summon instance that encountered a conflict. + # @return [void] def handle_conflict(summon) conflict_summon = summon.conflicts(party) return unless conflict_summon.summon.id == incoming_summon.id @@ -105,14 +173,103 @@ module Api render json: output end - def destroy - summon = GridSummon.find_by('id = ?', params[:id]) - render_unauthorized_response if summon.party.user != current_user - return render json: GridSummonBlueprint.render(summon, view: :destroyed) if summon.destroy - end - private + ## + # Finds the party based on the provided party_id parameter. + # + # Sets the @party instance variable and renders an unauthorized response if the current + # user is not the owner of the party. + # + # @return [void] + def find_party + # BUG: I can create grid weapons even when I'm not logged in on an authenticated party + @party = Party.find(summon_params[:party_id]) + render_unauthorized_response if current_user && (party.user != current_user) + end + + ## + # Finds the incoming summon based on the provided parameters. + # + # Sets the @incoming_summon instance variable. + # + # @return [void] + def find_incoming_summon + @incoming_summon = Summon.find_by(id: summon_params[:summon_id]) + end + + ## + # Finds and sets the grid summon for update actions. + # + # Sets the @summon instance variable using the provided id parameter. + # + # @return [void] + def set + @summon = GridSummon.find_by('id = ?', summon_params[:id]) + end + + ## + # Authorizes the current request based on the party or grid summon ownership and edit key. + # + # Checks if the current user is authorized to create or update the party or grid summon. + # Renders an unauthorized response if the authorization fails. + # + # @return [void] + def authorize + # Create + unauthorized_create = @party && (@party.user != current_user || @party.edit_key != edit_key) + unauthorized_update = @summon && @summon.party && (@summon.party.user != current_user || @summon.party.edit_key != edit_key) + + render_unauthorized_response if unauthorized_create || unauthorized_update + end + + ## + # Builds a new GridSummon instance using permitted parameters. + # + # Merges the party id and the incoming summon id into the parameters. + # + # @return [GridSummon] A new grid summon instance. + def build_grid_summon + GridSummon.new(summon_params.merge(party_id: party.id, summon_id: incoming_summon.id)) + end + + ## + # Checks whether the grid summon error is solely due to a conflict. + # + # Verifies if the errors on the :series attribute include the specific conflict message + # and confirms that a conflict exists for the current party. + # + # @param grid_summon [GridSummon] The grid summon instance to check. + # @return [Boolean] True if the error is due solely to a conflict, false otherwise. + def conflict_error?(grid_summon) + grid_summon.errors[:series].include?('must not conflict with existing summons') && + grid_summon.conflicts(party).present? + end + + ## + # Renders the grid summon view with additional metadata. + # + # @param grid_summon [GridSummon] The grid summon instance to render. + # @param conflict_position [Integer, nil] The position of a conflicting summon, if applicable. + # @return [String] The rendered grid summon view as JSON. + def render_grid_summon_view(grid_summon, conflict_position = nil) + GridSummonBlueprint.render(grid_summon, + view: :nested, + root: :grid_summon, + meta: { replaced: conflict_position }) + end + + ## + # Determines the maximum uncap level for a given summon. + # + # The maximum uncap level is determined based on the attributes of the summon: + # - Returns 4 if the summon has FLB but not ULB and is not transcended. + # - Returns 5 if the summon has ULB and is not transcended. + # - Returns 6 if the summon has transcendence. + # - Otherwise, returns 3. + # + # @param summon [Summon] The summon for which to determine the maximum uncap level. + # @return [Integer] The maximum uncap level. def max_uncap_level(summon) if summon.flb && !summon.ulb && !summon.transcendence 4 @@ -125,35 +282,10 @@ module Api end end - def find_incoming_summon - @incoming_summon = Summon.find_by(id: summon_params[:summon_id]) - end - - def find_party - # BUG: I can create grid weapons even when I'm not logged in on an authenticated party - @party = Party.find(summon_params[:party_id]) - render_unauthorized_response if current_user && (party.user != current_user) - end - - def render_grid_summon_view(grid_summon, conflict_position = nil) - GridSummonBlueprint.render(grid_summon, view: :nested, - root: :grid_summon, - meta: { replaced: conflict_position }) - end - - def authorize - # Create - unauthorized_create = @party && (@party.user != current_user || @party.edit_key != edit_key) - unauthorized_update = @summon && @summon.party && (@summon.party.user != current_user || @summon.party.edit_key != edit_key) - - render_unauthorized_response if unauthorized_create || unauthorized_update - end - - def set - @summon = GridSummon.find_by('id = ?', summon_params[:id]) - end - - # Specify whitelisted properties that can be modified. + ## + # Defines and permits the whitelisted parameters for a grid summon. + # + # @return [ActionController::Parameters] The permitted parameters. def summon_params params.require(:summon).permit(:id, :party_id, :summon_id, :position, :main, :friend, :quick_summon, :uncap_level, :transcendence_step) diff --git a/spec/requests/grid_characters_controller_spec.rb b/spec/requests/grid_characters_controller_spec.rb new file mode 100644 index 0000000..25f2c33 --- /dev/null +++ b/spec/requests/grid_characters_controller_spec.rb @@ -0,0 +1,213 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe 'GridCharacters API', type: :request do + let(:user) { create(:user) } + let(:party) { create(:party, user: user, edit_key: 'secret') } + # Use canonical records seeded into your DB. + # For example, assume Rosamia (granblue_id "3040087000") and Seofon (granblue_id "3040036000") + let(:rosamia) { Character.find_by(granblue_id: '3040087000') } + let(:seofon) { Character.find_by(granblue_id: '3040036000') } + let(:access_token) do + Doorkeeper::AccessToken.create!( + resource_owner_id: user.id, + expires_in: 30.days, + scopes: 'public' + ) + end + let(:headers) do + { + 'Authorization' => "Bearer #{access_token.token}", + 'Content-Type' => 'application/json', + 'X-Edit-Key' => 'secret' + } + end + + describe 'POST /api/v1/characters (create)' do + context 'when creating a grid character with a unique canonical character (e.g. Seofon)' do + let(:valid_params) do + { + character: { + party_id: party.id, + character_id: seofon.id, + position: 0, + uncap_level: 3, + transcendence_step: 0, + rings: [ + { modifier: 'A', strength: 1 }, + { modifier: 'B', strength: 2 } + ] + } + } + end + + it 'creates the grid character and returns status created' do + expect do + post '/api/v1/characters', params: valid_params.to_json, headers: headers + end.to change(GridCharacter, :count).by(1) + expect(response).to have_http_status(:created) + json_response = JSON.parse(response.body) + expect(json_response).to include('position' => 0) + end + end + + context 'when attempting to add a duplicate canonical character (e.g. Rosamia)' do + before do + # Create an initial grid character for Rosamia. + GridCharacter.create!( + party_id: party.id, + character_id: rosamia.id, + position: 1, + uncap_level: 3, + transcendence_step: 0 + ) + end + + let(:duplicate_params) do + { + character: { + party_id: party.id, + character_id: rosamia.id, # same canonical character + position: 2, + uncap_level: 3, + transcendence_step: 0 + } + } + end + + it 'detects the conflict and returns a conflict resolution view without adding a duplicate' do + # Here we simulate conflict resolution via the resolve endpoint. + expect do + post '/api/v1/characters/resolve', + params: { resolve: { position: 2, incoming: rosamia.id, conflicting: [GridCharacter.last.id] } }.to_json, + headers: headers + end.to change(GridCharacter, :count).by(0) + expect(response).to have_http_status(:created) + json_response = JSON.parse(response.body) + expect(json_response).to include('position' => 2) + end + end + end + + describe 'PUT /api/v1/characters/:id (update)' do + before do + @grid_character = GridCharacter.create!( + party_id: party.id, + character_id: rosamia.id, + position: 1, + uncap_level: 3, + transcendence_step: 0 + ) + end + + let(:update_params) do + { + character: { + id: @grid_character.id, + party_id: party.id, + character_id: rosamia.id, + position: 1, + uncap_level: 4, + transcendence_step: 0, + rings: [ + { modifier: 'C', strength: 3 } + ] + } + } + end + + it 'updates the grid character and returns the updated record' do + put "/api/v1/grid_characters/#{@grid_character.id}", params: update_params.to_json, headers: headers + expect(response).to have_http_status(:ok) + + json_response = JSON.parse(response.body) + expect(json_response).to include('uncap_level' => 4) + end + end + + describe 'POST /api/v1/characters/update_uncap (update uncap level)' do + context 'for a character that does NOT allow transcendence (e.g. Rosamia)' do + before do + @grid_character = GridCharacter.create!( + party_id: party.id, + character_id: rosamia.id, + position: 2, + uncap_level: 2, + transcendence_step: 0 + ) + end + + let(:update_uncap_params) do + { + character: { + id: @grid_character.id, + party_id: party.id, + character_id: rosamia.id, + uncap_level: 3, + transcendence_step: 0 + } + } + end + + it 'caps the uncap level at 4 for a character with flb true' do + post '/api/v1/characters/update_uncap', params: update_uncap_params.to_json, headers: headers + expect(response).to have_http_status(:ok) + + json_response = JSON.parse(response.body) + expect(json_response['grid_character']).to include('uncap_level' => 3) + end + end + + context 'for a character that allows transcendence (e.g. Seofon)' do + before do + # For Seofon, the "transcendence" behavior is enabled by its ulb flag. + @grid_character = GridCharacter.create!( + party_id: party.id, + character_id: seofon.id, + position: 2, + uncap_level: 5, + transcendence_step: 0 + ) + end + + let(:update_uncap_params) do + { + character: { + id: @grid_character.id, + party_id: party.id, + character_id: seofon.id, + uncap_level: 5, + transcendence_step: 1 + } + } + end + + it 'updates the uncap level to 6 when the character supports transcendence via ulb' do + post '/api/v1/characters/update_uncap', params: update_uncap_params.to_json, headers: headers + expect(response).to have_http_status(:ok) + + json_response = JSON.parse(response.body) + expect(json_response['grid_character']).to include('uncap_level' => 5, 'transcendence_step' => 1) + end + end + end + + describe 'DELETE /api/v1/characters (destroy)' do + before do + @grid_character = GridCharacter.create!( + party_id: party.id, + character_id: rosamia.id, + position: 4, + uncap_level: 3, + transcendence_step: 0 + ) + end + + it 'destroys the grid character and returns a destroyed view' do + expect do + delete '/api/v1/characters', params: { id: @grid_character.id }.to_json, headers: headers + end.to change(GridCharacter, :count).by(-1) + expect(response).to have_http_status(:ok) + end + end +end