From c5780c2da3dde5f713af086f473e3e0aacfb5f22 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Fri, 14 Jun 2019 18:02:47 +0200 Subject: [PATCH 01/17] Implement function to calculate the difference in points --- app/models/group_score.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/app/models/group_score.rb b/app/models/group_score.rb index d7686d3..077b081 100644 --- a/app/models/group_score.rb +++ b/app/models/group_score.rb @@ -3,4 +3,8 @@ class GroupScore < ApplicationRecord belongs_to :team belongs_to :group + + def difference_in_points + scored_points - received_points + end end From 88cbba440d05c293d75167c41add9d633785517d Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Fri, 14 Jun 2019 18:03:40 +0200 Subject: [PATCH 02/17] Implement methods to get teams sorted by their group scores --- app/services/group_stage_service.rb | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/app/services/group_stage_service.rb b/app/services/group_stage_service.rb index 5e92a1d..c8f7181 100644 --- a/app/services/group_stage_service.rb +++ b/app/services/group_stage_service.rb @@ -55,5 +55,32 @@ class GroupStageService end changed_group_scores end + + # Returns a list of the teams in the group sorted by their group_points, difference_in_points, scored_points + # + # @param group Group the group to get the teams from + # @return [Array] of teams + def teams_sorted_by_group_scores(group) + group.teams.sort_by do |t| + [group.group_scores.find_by(team: t).group_points, + group.group_scores.find_by(team: t).difference_in_points, + group.group_scores.find_by(team: t).scored_points] + end + end + + # Returns all teams advancing to playoff stage from given group stage + # They are ordered in such a way, that PlayoffStageService will correctly match the teams + # + # @param group_stage GroupStage the group stage to get all advancing teams from + # @return [Array] the teams advancing from that group stage + def get_advancing_teams(group_stage) + advancing_teams = [] + group_winners = group_stage.groups.map(&method(:teams_sorted_by_group_scores)) + (group_stage.tournament.instant_finalists_amount + group_stage.tournament.intermediate_round_participants_amount) + .times do |i| + advancing_teams << group_winners[i % group_stage.groups.size].shift + end + advancing_teams + end end end From bc0c7fddc3f5db1658d170fb8f3cbde3c189b3d5 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Fri, 14 Jun 2019 18:04:31 +0200 Subject: [PATCH 03/17] Make match position randomizable --- app/interactors/add_playoffs_to_tournament.rb | 3 ++- app/services/playoff_stage_service.rb | 7 ++++--- spec/services/playoff_stage_service_spec.rb | 6 +++--- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/app/interactors/add_playoffs_to_tournament.rb b/app/interactors/add_playoffs_to_tournament.rb index ce530cb..51b1d18 100644 --- a/app/interactors/add_playoffs_to_tournament.rb +++ b/app/interactors/add_playoffs_to_tournament.rb @@ -6,7 +6,8 @@ class AddPlayoffsToTournament def call tournament = context.tournament context.fail! if tournament.stages.size > 1 - if (playoff_stages = PlayoffStageService.generate_playoff_stages_from_tournament(tournament)) + if (playoff_stages = PlayoffStageService.generate_playoff_stages_from_tournament(tournament, + context.randomize_matches)) if tournament.stages.empty? tournament.stages = playoff_stages else diff --git a/app/services/playoff_stage_service.rb b/app/services/playoff_stage_service.rb index 11b28b1..effafd8 100644 --- a/app/services/playoff_stage_service.rb +++ b/app/services/playoff_stage_service.rb @@ -6,11 +6,12 @@ class PlayoffStageService # # @param teams [Array] The teams to generate the playoff stages with # @return [Array] the generated playoff stages - def generate_playoff_stages(teams) + def generate_playoff_stages(teams, randomize_matches) playoffs = [] stage_count = calculate_required_stage_count(teams.size) # initial_matches are the matches in the first stage; this is the only stage filled with teams from the start on initial_matches = MatchService.generate_matches(teams) + initial_matches = initial_matches.shuffle.each_with_index { |m, i| m.position = i } if randomize_matches initial_stage = Stage.new level: stage_count - 1, matches: initial_matches initial_stage.state = :intermediate_stage unless initial_stage.matches.find(&:single_team?).nil? playoffs << initial_stage @@ -24,8 +25,8 @@ class PlayoffStageService # # @param tournament [Tournament] The tournament to generate the playoff stages from # @return [Array] the generated playoff stages - def generate_playoff_stages_from_tournament(tournament) - generate_playoff_stages tournament.teams + def generate_playoff_stages_from_tournament(tournament, randomize_matches) + generate_playoff_stages(tournament.teams, randomize_matches) end # Generates given number of empty stages diff --git a/spec/services/playoff_stage_service_spec.rb b/spec/services/playoff_stage_service_spec.rb index 97fdfb7..ee12487 100644 --- a/spec/services/playoff_stage_service_spec.rb +++ b/spec/services/playoff_stage_service_spec.rb @@ -70,7 +70,7 @@ RSpec.describe PlayoffStageService do amount_of_teams = parameters[:team_size] expected_amount_of_playoff_stages = parameters[:expected_amount_of_playoff_stages] teams = build_list(:team, amount_of_teams) - stages = PlayoffStageService.generate_playoff_stages(teams) + stages = PlayoffStageService.generate_playoff_stages(teams, false) expect(stages.size).to eq(expected_amount_of_playoff_stages) stages.each_index do |i| stage = stages[i] @@ -82,7 +82,7 @@ RSpec.describe PlayoffStageService do describe 'number of teams isn\'t a power of two' do let(:generated_stages) do - PlayoffStageService.generate_playoff_stages(create_list(:team, 12)) + PlayoffStageService.generate_playoff_stages(create_list(:team, 12), false) end let(:intermediate_stage) do @@ -102,7 +102,7 @@ RSpec.describe PlayoffStageService do describe 'number of teams is a power of two' do let(:generated_stages) do - PlayoffStageService.generate_playoff_stages(create_list(:team, 16)) + PlayoffStageService.generate_playoff_stages(create_list(:team, 16), false) end it 'generates only normal playoff_stage state stages' do From 68efd3caae7a2328ea1fc353a6bcf47f21453c89 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Fri, 14 Jun 2019 18:04:46 +0200 Subject: [PATCH 04/17] Add factory for a finished_playoff_match --- spec/factories/matches.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/factories/matches.rb b/spec/factories/matches.rb index cd0543f..22711e7 100644 --- a/spec/factories/matches.rb +++ b/spec/factories/matches.rb @@ -12,6 +12,9 @@ FactoryBot.define do match.match_scores = create_list(:match_score, evaluator.match_scores_count) end state { :in_progress } + factory :finished_playoff_match do + state { :finished } + end end factory :single_team_match do From 4e72d015ff2d7b8a13c08a7a33dc0733a8084fc4 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Sun, 16 Jun 2019 17:31:34 +0200 Subject: [PATCH 05/17] Implement method to check if stage is over --- app/models/stage.rb | 10 ++++++++ spec/models/stage_spec.rb | 54 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/app/models/stage.rb b/app/models/stage.rb index f29d661..8e4267f 100644 --- a/app/models/stage.rb +++ b/app/models/stage.rb @@ -18,4 +18,14 @@ class Stage < ApplicationRecord [] end end + + def over? + return matches.find { |m| m.state != 'finished' }.nil? unless matches.size.zero? + + unless groups.size.zero? && groups.map(&:matches).flatten.size.zero? + return groups.map(&:matches).flatten.find { |m| m.state != 'finished' }.nil? + end + + false + end end diff --git a/spec/models/stage_spec.rb b/spec/models/stage_spec.rb index ac1dc72..569fec2 100644 --- a/spec/models/stage_spec.rb +++ b/spec/models/stage_spec.rb @@ -40,4 +40,58 @@ RSpec.describe Stage, type: :model do end end end + + describe '#over?' do + context 'group stage' do + context 'with unfinished matches' do + it 'returns false' do + expect(create(:group_stage).over?).to eq(false) + end + end + + context 'with all matches finished' do + let(:finished_group_stage) do + group_stage = create(:group_stage) + group_stage.groups.map(&:matches).flatten.each do |m| + m.state = :finished + m.save! + end + group_stage + end + + it 'returns true' do + expect(finished_group_stage.over?).to eq(true) + end + end + end + + context 'playoff stage' do + context 'with unfinished matches' do + it 'returns false' do + expect(create(:playoff_stage).over?).to eq(false) + end + end + + context 'with all matches finished' do + let(:finished_playoff_stage) do + playoff_stage = create(:playoff_stage) + playoff_stage.matches.each do |m| + m.state = :finished + m.save! + end + playoff_stage + end + + it 'returns true' do + expect(finished_playoff_stage.over?).to eq(true) + end + end + end + + context 'empty stage' do + it 'returns false' do + expect(create(:stage).over?).to eq(false) + end + end + end end From 748ac18b35c5214143454c32420d50046948edd6 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Sun, 16 Jun 2019 17:34:39 +0200 Subject: [PATCH 06/17] Implement finished_group_match factory --- spec/factories/matches.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/factories/matches.rb b/spec/factories/matches.rb index 22711e7..6b9facd 100644 --- a/spec/factories/matches.rb +++ b/spec/factories/matches.rb @@ -56,6 +56,10 @@ FactoryBot.define do after(:create) do |match, evaluator| match.match_scores = create_list(:match_score, evaluator.match_scores_count) end + + factory :finished_group_match do + state { :finished } + end end factory :undecided_group_match do From 4925ea9d83aad3ad1de26edff903f5742638d7cd Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Mon, 17 Jun 2019 12:47:33 +0200 Subject: [PATCH 07/17] Make teams added to PlayoffStage configurable --- app/controllers/tournaments_controller.rb | 2 +- app/interactors/add_playoffs_to_tournament.rb | 3 +-- app/services/playoff_stage_service.rb | 8 -------- .../add_playoffs_to_tournament_interactor_spec.rb | 10 +++++----- 4 files changed, 7 insertions(+), 16 deletions(-) diff --git a/app/controllers/tournaments_controller.rb b/app/controllers/tournaments_controller.rb index 78bcfee..5d225b7 100644 --- a/app/controllers/tournaments_controller.rb +++ b/app/controllers/tournaments_controller.rb @@ -49,7 +49,7 @@ class TournamentsController < ApplicationController # associate provided teams with tournament tournament.teams = teams # add playoff stage to tournament - result = AddPlayoffsToTournamentAndSave.call(tournament: tournament) + result = AddPlayoffsToTournamentAndSave.call(tournament: tournament, teams: tournament.teams) end # validate tournament unless tournament.valid? diff --git a/app/interactors/add_playoffs_to_tournament.rb b/app/interactors/add_playoffs_to_tournament.rb index 51b1d18..9abecdf 100644 --- a/app/interactors/add_playoffs_to_tournament.rb +++ b/app/interactors/add_playoffs_to_tournament.rb @@ -6,8 +6,7 @@ class AddPlayoffsToTournament def call tournament = context.tournament context.fail! if tournament.stages.size > 1 - if (playoff_stages = PlayoffStageService.generate_playoff_stages_from_tournament(tournament, - context.randomize_matches)) + if (playoff_stages = PlayoffStageService.generate_playoff_stages(context.teams, context.randomize_matches)) if tournament.stages.empty? tournament.stages = playoff_stages else diff --git a/app/services/playoff_stage_service.rb b/app/services/playoff_stage_service.rb index effafd8..e12ae2c 100644 --- a/app/services/playoff_stage_service.rb +++ b/app/services/playoff_stage_service.rb @@ -21,14 +21,6 @@ class PlayoffStageService playoffs end - # Generates the playoff stage given the tournament - # - # @param tournament [Tournament] The tournament to generate the playoff stages from - # @return [Array] the generated playoff stages - def generate_playoff_stages_from_tournament(tournament, randomize_matches) - generate_playoff_stages(tournament.teams, randomize_matches) - end - # Generates given number of empty stages # # @param stage_count [Integer] number of stages to generate diff --git a/spec/interactors/add_playoffs_to_tournament_interactor_spec.rb b/spec/interactors/add_playoffs_to_tournament_interactor_spec.rb index 0ee2cbc..e1cebfc 100644 --- a/spec/interactors/add_playoffs_to_tournament_interactor_spec.rb +++ b/spec/interactors/add_playoffs_to_tournament_interactor_spec.rb @@ -2,15 +2,15 @@ RSpec.describe AddPlayoffsToTournament, type: :interactor do let(:group_stage_tournament_context) do - AddPlayoffsToTournament.call(tournament: @group_stage_tournament) + AddPlayoffsToTournament.call(tournament: @group_stage_tournament, teams: @group_stage_tournament.teams) end let(:playoff_stage_tournament_context) do - AddPlayoffsToTournament.call(tournament: @playoff_stage_tournament) + AddPlayoffsToTournament.call(tournament: @playoff_stage_tournament, teams: @playoff_stage_tournament.teams) end let(:full_tournament_context) do - AddPlayoffsToTournament.call(tournament: @full_tournament) + AddPlayoffsToTournament.call(tournament: @full_tournament, teams: @full_tournament.teams) end before do @@ -23,7 +23,7 @@ RSpec.describe AddPlayoffsToTournament, type: :interactor do context 'PlayoffStageService mocked' do before do expect(class_double('PlayoffStageService').as_stubbed_const(transfer_nested_constants: true)) - .to receive(:generate_playoff_stages_from_tournament) + .to receive(:generate_playoff_stages) .and_return(@stages) end @@ -53,7 +53,7 @@ RSpec.describe AddPlayoffsToTournament, type: :interactor do context 'playoff generation fails' do before do expect(class_double('PlayoffStageService').as_stubbed_const(transfer_nested_constants: true)) - .to receive(:generate_playoff_stages_from_tournament) + .to receive(:generate_playoff_stages) .and_return(nil) end From 15e2bd830f75f11c53aec40698bb2a6a07266bd3 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Mon, 17 Jun 2019 12:48:48 +0200 Subject: [PATCH 08/17] Randomize points in group_scores factory --- spec/factories/group_scores.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/spec/factories/group_scores.rb b/spec/factories/group_scores.rb index 558484a..0692ee6 100644 --- a/spec/factories/group_scores.rb +++ b/spec/factories/group_scores.rb @@ -4,5 +4,9 @@ FactoryBot.define do factory :group_score do team group + + group_points { rand 5 } + scored_points { rand 5 } + received_points { rand 5 } end end From 54ab1570b1e37b349d0996bdd204fb2116116536 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Mon, 17 Jun 2019 12:50:26 +0200 Subject: [PATCH 09/17] Sort teams more explicitly and in the right order --- app/services/group_stage_service.rb | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/app/services/group_stage_service.rb b/app/services/group_stage_service.rb index c8f7181..5b9f1ce 100644 --- a/app/services/group_stage_service.rb +++ b/app/services/group_stage_service.rb @@ -61,10 +61,13 @@ class GroupStageService # @param group Group the group to get the teams from # @return [Array] of teams def teams_sorted_by_group_scores(group) - group.teams.sort_by do |t| - [group.group_scores.find_by(team: t).group_points, - group.group_scores.find_by(team: t).difference_in_points, - group.group_scores.find_by(team: t).scored_points] + group.teams.sort do |a, b| + [group.group_scores.find_by(team: b).group_points, + group.group_scores.find_by(team: b).difference_in_points, + group.group_scores.find_by(team: b).scored_points] <=> + [group.group_scores.find_by(team: a).group_points, + group.group_scores.find_by(team: a).difference_in_points, + group.group_scores.find_by(team: a).scored_points] end end From 3dfeae8bf35727ec26a6742da4c29d1c1dfefdd2 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Mon, 17 Jun 2019 12:52:11 +0200 Subject: [PATCH 10/17] Test #teams_sorted_by_group_scores --- spec/services/group_stage_service_spec.rb | 34 +++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/spec/services/group_stage_service_spec.rb b/spec/services/group_stage_service_spec.rb index b5ebf06..8073ef2 100644 --- a/spec/services/group_stage_service_spec.rb +++ b/spec/services/group_stage_service_spec.rb @@ -156,4 +156,38 @@ RSpec.describe GroupStageService do it_should_behave_like 'only_return_group_scores' end end + + describe '#teams_sorted_by_group_scores' do + before do + @group_to_sort = create(:group, match_count: 10, match_factory: :filled_group_match) + end + + let(:sorted_teams) do + GroupStageService.teams_sorted_by_group_scores(@group_to_sort) + end + + let(:sorted_teams_grouped_by_group_points) do + sorted_teams.group_by { |t| @group_to_sort.group_scores.find_by(team: t).group_points }.values + end + + it 'sorts the teams after group_scores first' do + i = 0 + while i < (sorted_teams.size - 1) + expect(@group_to_sort.group_scores.find_by(team: sorted_teams[i]).group_points) + .to be >= @group_to_sort.group_scores.find_by(team: sorted_teams[i + 1]).group_points + i += 1 + end + end + + it 'sorts the teams after difference_in_points second' do + sorted_teams_grouped_by_group_points.each do |teams| + i = 0 + while i < (teams.size - 1) + expect(@group_to_sort.group_scores.find_by(team: teams[i]).difference_in_points) + .to be >= @group_to_sort.group_scores.find_by(team: teams[i + 1]).difference_in_points + i += 1 + end + end + end + end end From b28561043edca102cd2ca0e9cd5f0c8a6ac954c8 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Mon, 17 Jun 2019 13:10:42 +0200 Subject: [PATCH 11/17] Implement stages controller (GET UPDATE) --- app/controllers/stages_controller.rb | 62 +++++++++++++++++ config/routes.rb | 1 + spec/controllers/stages_controller_spec.rb | 77 ++++++++++++++++++++++ 3 files changed, 140 insertions(+) create mode 100644 app/controllers/stages_controller.rb create mode 100644 spec/controllers/stages_controller_spec.rb diff --git a/app/controllers/stages_controller.rb b/app/controllers/stages_controller.rb new file mode 100644 index 0000000..cdc5f1b --- /dev/null +++ b/app/controllers/stages_controller.rb @@ -0,0 +1,62 @@ +# frozen_string_literal: true + +class StagesController < ApplicationController + before_action :set_stage, only: %i[show update] + before_action :authenticate_user!, only: %i[update] + before_action -> { require_owner! @stage.owner }, only: %i[update] + + # GET /stages/1 + def show + render json: @stage, include: '**' + end + + # PUT /stages/1 + def update + if stage_params[:state] == 'finished' + unless @stage.state == 'in_progress' + render json: { error: 'Only running group stages can be finished' }, status: :unprocessable_entity + return + end + + Stage.transaction do + if @stage.update(stage_params) + handle_group_stage_end + + render json: @stage + else + render json: @stage.errors, status: :unprocessable_entity + raise ActiveRecord::Rollback + end + end + else + render json: { + error: 'The state attribute may only be changed to finished' + }, status: :unprocessable_entity + end + end + + private + + def handle_group_stage_end + unless @stage.over? + render json: { + error: 'Group Stage still has some matches that are not over yet. Finish them to generate playoffs' + }, status: :unprocessable_entity + raise ActiveRecord::Rollback + end + + return if AddPlayoffsToTournamentAndSave.call(tournament: @stage.tournament, + teams: GroupStageService.get_advancing_teams(@stage)).success? + + render json: { error: 'Generating group stage failed' }, status: :unprocessable_entity + raise ActiveRecord::Rollback + end + + def set_stage + @stage = Stage.find(params[:id]) + end + + def stage_params + params.slice(:state).permit! + end +end diff --git a/config/routes.rb b/config/routes.rb index d8f761a..61901bd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -9,6 +9,7 @@ Rails.application.routes.draw do resources :matches, only: %i[show update] do resources :bets, only: %i[index create] end + resources :stages, only: %i[show update] resources :teams, only: %i[show update] resources :tournaments do resources :statistics, only: %i[index] diff --git a/spec/controllers/stages_controller_spec.rb b/spec/controllers/stages_controller_spec.rb new file mode 100644 index 0000000..042db52 --- /dev/null +++ b/spec/controllers/stages_controller_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe StagesController, type: :controller do + before do + @stage = create(:playoff_stage) + end + + describe 'GET #show' do + it 'returns a success response' do + get :show, params: { id: @stage.to_param } + expect(response).to be_successful + end + + it 'should return the correct stage' do + get :show, params: { id: @stage.to_param } + body = deserialize_response response + expect(Stage.find_by(id: body[:id])).to eq(@stage) + expect(body[:level]).to eq(@stage.level) + expect(body[:state]).to eq(@stage.state) + end + end + + describe 'PUT #update' do + context 'group_stage with matches that are done' do + before do + @running_group_stage = create(:group_stage, match_factory: :finished_group_match) + end + + FINISHED = { state: 'finished' }.freeze + + it 'doesn\'t have any other stages besides it before update' do + expect(@running_group_stage.tournament.stages.size).to eq(1) + end + + context 'as owner' do + before(:each) do + apply_authentication_headers_for @running_group_stage.owner + end + + before do + put :update, params: { id: @running_group_stage.to_param }.merge(FINISHED) + @running_group_stage.reload + end + + it 'succeeds' do + expect(response).to be_successful + end + + it 'stops the stage' do + expect(@running_group_stage.state).to eq(FINISHED[:state]) + end + + it 'adds new stages to the tournament' do + expect(@running_group_stage.tournament.stages.size).to be > 1 + end + + it 'adds the right teams' do + expect(@running_group_stage.tournament.stages.max_by(&:level).teams) + .to match_array(GroupStageService.get_advancing_teams(@running_group_stage)) + end + end + + context 'as another user' do + before(:each) do + apply_authentication_headers_for create(:user) + end + + it 'returns an error' do + put :update, params: { id: @stage.to_param }.merge(FINISHED) + expect(response).to have_http_status(:forbidden) + end + end + end + end +end From 8873e7a95b69f25c8110152cecb5bb58ee674d55 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Tue, 18 Jun 2019 15:44:32 +0200 Subject: [PATCH 12/17] Test stage.teams returning [] on empty stage --- spec/models/stage_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/models/stage_spec.rb b/spec/models/stage_spec.rb index 569fec2..9fdd105 100644 --- a/spec/models/stage_spec.rb +++ b/spec/models/stage_spec.rb @@ -39,6 +39,12 @@ RSpec.describe Stage, type: :model do expect(teams).to match_array(@teams) end end + + context 'empty stage' do + it 'returns an empty Array' do + expect(create(:stage).teams).to match_array([]) + end + end end describe '#over?' do From fd8ff20ce8ad01282ee6b2c3d8960a56c95ece34 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Tue, 18 Jun 2019 16:06:25 +0200 Subject: [PATCH 13/17] Test for correct error on finishing already finished stages --- spec/controllers/stages_controller_spec.rb | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/spec/controllers/stages_controller_spec.rb b/spec/controllers/stages_controller_spec.rb index 042db52..773cd65 100644 --- a/spec/controllers/stages_controller_spec.rb +++ b/spec/controllers/stages_controller_spec.rb @@ -73,5 +73,27 @@ RSpec.describe StagesController, type: :controller do end end end + + context 'already finished group stage' do + let(:finished_group_stage) do + group_stage = create(:group_stage, match_factory: :finished_group_match) + group_stage.finished! + group_stage.save! + group_stage + end + + before do + apply_authentication_headers_for finished_group_stage.owner + put :update, params: { id: finished_group_stage.to_param }.merge(state: 'finished') + end + + it 'it returns unprocessable entity' do + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'returns the correct error' do + expect(deserialize_response(response)[:error]).to eq('Only running group stages can be finished') + end + end end end From 90d97962a3f6ea01943889b6c929d7fb0e5e1221 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Tue, 18 Jun 2019 16:12:18 +0200 Subject: [PATCH 14/17] Test trying to finish a group stage with unfinished matches --- spec/controllers/stages_controller_spec.rb | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/spec/controllers/stages_controller_spec.rb b/spec/controllers/stages_controller_spec.rb index 773cd65..61da200 100644 --- a/spec/controllers/stages_controller_spec.rb +++ b/spec/controllers/stages_controller_spec.rb @@ -74,6 +74,26 @@ RSpec.describe StagesController, type: :controller do end end + context 'trying to finish a group stage with unfinished matches' do + let(:group_stage) do + create(:group_stage) + end + + before do + apply_authentication_headers_for group_stage.owner + put :update, params: { id: group_stage.to_param }.merge(state: 'finished') + end + + it 'it returns unprocessable entity' do + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'returns the correct error' do + expect(deserialize_response(response)[:error]) + .to eq('Group Stage still has some matches that are not over yet. Finish them to generate playoffs') + end + end + context 'already finished group stage' do let(:finished_group_stage) do group_stage = create(:group_stage, match_factory: :finished_group_match) From fa9f47903b061a0c9c5bdee1f98bba2b0e64e9a9 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Tue, 18 Jun 2019 16:12:45 +0200 Subject: [PATCH 15/17] Test trying to change the state to something other than :finished --- spec/controllers/stages_controller_spec.rb | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/spec/controllers/stages_controller_spec.rb b/spec/controllers/stages_controller_spec.rb index 61da200..bc9eb7a 100644 --- a/spec/controllers/stages_controller_spec.rb +++ b/spec/controllers/stages_controller_spec.rb @@ -115,5 +115,24 @@ RSpec.describe StagesController, type: :controller do expect(deserialize_response(response)[:error]).to eq('Only running group stages can be finished') end end + + context 'trying to change the state to something other than :finished' do + let(:group_stage) do + create(:group_stage) + end + + before do + apply_authentication_headers_for group_stage.owner + put :update, params: { id: group_stage.to_param }.merge(state: 'in_progress') + end + + it 'it returns unprocessable entity' do + expect(response).to have_http_status(:unprocessable_entity) + end + + it 'returns the correct error' do + expect(deserialize_response(response)[:error]).to eq('The state attribute may only be changed to finished') + end + end end end From 945ab4981a7f03547651df055a1e62e9b0fd6890 Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Tue, 18 Jun 2019 22:23:02 +0200 Subject: [PATCH 16/17] Use let more effectively --- spec/controllers/stages_controller_spec.rb | 46 +++++++++++----------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/spec/controllers/stages_controller_spec.rb b/spec/controllers/stages_controller_spec.rb index bc9eb7a..a292400 100644 --- a/spec/controllers/stages_controller_spec.rb +++ b/spec/controllers/stages_controller_spec.rb @@ -3,45 +3,47 @@ require 'rails_helper' RSpec.describe StagesController, type: :controller do - before do - @stage = create(:playoff_stage) + let(:stage) do + create(:playoff_stage) end describe 'GET #show' do it 'returns a success response' do - get :show, params: { id: @stage.to_param } + get :show, params: { id: stage.to_param } expect(response).to be_successful end it 'should return the correct stage' do - get :show, params: { id: @stage.to_param } + get :show, params: { id: stage.to_param } body = deserialize_response response - expect(Stage.find_by(id: body[:id])).to eq(@stage) - expect(body[:level]).to eq(@stage.level) - expect(body[:state]).to eq(@stage.state) + expect(Stage.find_by(id: body[:id])).to eq(stage) + expect(body[:level]).to eq(stage.level) + expect(body[:state]).to eq(stage.state) end end describe 'PUT #update' do + let(:finished) do + { state: 'finished' } + end + context 'group_stage with matches that are done' do - before do - @running_group_stage = create(:group_stage, match_factory: :finished_group_match) + let(:running_group_stage) do + create(:group_stage, match_factory: :finished_group_match) end - FINISHED = { state: 'finished' }.freeze - it 'doesn\'t have any other stages besides it before update' do - expect(@running_group_stage.tournament.stages.size).to eq(1) + expect(running_group_stage.tournament.stages.size).to eq(1) end context 'as owner' do before(:each) do - apply_authentication_headers_for @running_group_stage.owner + apply_authentication_headers_for running_group_stage.owner end before do - put :update, params: { id: @running_group_stage.to_param }.merge(FINISHED) - @running_group_stage.reload + put :update, params: { id: running_group_stage.to_param }.merge(finished) + running_group_stage.reload end it 'succeeds' do @@ -49,16 +51,16 @@ RSpec.describe StagesController, type: :controller do end it 'stops the stage' do - expect(@running_group_stage.state).to eq(FINISHED[:state]) + expect(running_group_stage.state).to eq('finished') end it 'adds new stages to the tournament' do - expect(@running_group_stage.tournament.stages.size).to be > 1 + expect(running_group_stage.tournament.stages.size).to be > 1 end it 'adds the right teams' do - expect(@running_group_stage.tournament.stages.max_by(&:level).teams) - .to match_array(GroupStageService.get_advancing_teams(@running_group_stage)) + expect(running_group_stage.tournament.stages.max_by(&:level).teams) + .to match_array(GroupStageService.get_advancing_teams(running_group_stage)) end end @@ -68,7 +70,7 @@ RSpec.describe StagesController, type: :controller do end it 'returns an error' do - put :update, params: { id: @stage.to_param }.merge(FINISHED) + put :update, params: { id: stage.to_param }.merge(finished) expect(response).to have_http_status(:forbidden) end end @@ -81,7 +83,7 @@ RSpec.describe StagesController, type: :controller do before do apply_authentication_headers_for group_stage.owner - put :update, params: { id: group_stage.to_param }.merge(state: 'finished') + put :update, params: { id: group_stage.to_param }.merge(finished) end it 'it returns unprocessable entity' do @@ -104,7 +106,7 @@ RSpec.describe StagesController, type: :controller do before do apply_authentication_headers_for finished_group_stage.owner - put :update, params: { id: finished_group_stage.to_param }.merge(state: 'finished') + put :update, params: { id: finished_group_stage.to_param }.merge(finished) end it 'it returns unprocessable entity' do From a2691b9f88e12b7880063febdd45a8745694148e Mon Sep 17 00:00:00 2001 From: Malaber <32635600+Malaber@users.noreply.github.com> Date: Tue, 18 Jun 2019 22:31:52 +0200 Subject: [PATCH 17/17] Cache group_score in variable in sort function --- app/services/group_stage_service.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/app/services/group_stage_service.rb b/app/services/group_stage_service.rb index 5b9f1ce..6490980 100644 --- a/app/services/group_stage_service.rb +++ b/app/services/group_stage_service.rb @@ -62,12 +62,15 @@ class GroupStageService # @return [Array] of teams def teams_sorted_by_group_scores(group) group.teams.sort do |a, b| - [group.group_scores.find_by(team: b).group_points, - group.group_scores.find_by(team: b).difference_in_points, - group.group_scores.find_by(team: b).scored_points] <=> - [group.group_scores.find_by(team: a).group_points, - group.group_scores.find_by(team: a).difference_in_points, - group.group_scores.find_by(team: a).scored_points] + group_score_a = group.group_scores.find_by(team: a) + group_score_b = group.group_scores.find_by(team: b) + + [group_score_b.group_points, + group_score_b.difference_in_points, + group_score_b.scored_points] <=> + [group_score_a.group_points, + group_score_a.difference_in_points, + group_score_a.scored_points] end end