Merge pull request #48 from turniere/ticket/TURNIERE-226

Fix Playoff Matches ending without a winner

app/controllers/matches_controller.rb

@@ -13,22 +13,37 @@ class MatchesController < ApplicationController
   # PATCH/PUT /matches/1
   def update
     new_state = match_params['state']
-    if @match.update(match_params)
+
-      if new_state == 'finished'
+    Match.transaction do
-        result = PopulateMatchBelowAndSave.call(match: @match) unless @match.group_match?
+      if @match.update(match_params)
-        unless result.success?
+        handle_match_end if new_state == 'finished'
-          render json: { error: 'Moving Team one stage down failed' }, status: :unprocessable_entity
+
-          return
+        render json: @match
-        end
+      else
+        render json: @match.errors, status: :unprocessable_entity
+        raise ActiveRecord::Rollback
       end
-      render json: @match
-    else
-      render json: @match.errors, status: :unprocessable_entity
     end
   end
 
   private
 
+  def handle_match_end
+    return if @match.group_match?
+
+    if @match.winner.nil?
+      render json: { error: 'Stopping undecided Matches isn\'t allowed in playoff stage' },
+             status: :unprocessable_entity
+      raise ActiveRecord::Rollback
+    end
+
+    return if PopulateMatchBelowAndSave.call(match: @match).success?
+
+    render json: { error: 'Moving Team one stage down failed' },
+           status: :unprocessable_entity
+    raise ActiveRecord::Rollback
+  end
+
   def validate_params
     case match_params['state']
     when 'in_progress'

spec/controllers/matches_controller_spec.rb

@@ -8,6 +8,7 @@ RSpec.describe MatchesController, type: :controller do
     @amount_of_stages = 2
     @tournament = create(:stage_tournament, stage_count: @amount_of_stages)
     @running_playoff_match = @tournament.stages.find_by(level: @amount_of_stages).matches.first
+    @not_ready_playoff_match = create(:running_playoff_match, state: :not_ready)
     @match.match_scores = create_pair(:match_score)
   end
 
@@ -27,98 +28,201 @@ RSpec.describe MatchesController, type: :controller do
   end
 
   describe 'POST #update' do
-    let(:valid_update) do
+    context 'on a running playoff match' do
-      {
+      let(:valid_update) do
-        state: 'in_progress'
+        {
-      }
+          state: 'in_progress'
-    end
+        }
-
-    let(:invalid_update) do
-      {
-        state: 'finished'
-      }
-    end
-
-    context 'as owner' do
-      before(:each) do
-        apply_authentication_headers_for @match.owner
       end
 
-      context 'with valid params' do
+      let(:invalid_update) do
-        it 'updates the match' do
+        {
-          put :update, params: { id: @match.to_param }.merge(valid_update)
+          state: 'finished'
-          @match.reload
+        }
-          expect(response).to be_successful
-          expect(@match.state).to eq(valid_update[:state])
-        end
-
-        it 'renders a response with the updated match' do
-          put :update, params: { id: @match.to_param }.merge(valid_update)
-          expect(response).to be_successful
-          body = deserialize_response response
-          expect(body[:state]).to eq(valid_update[:state])
-        end
-
-        context 'on a running playoff match' do
-          let(:finished) do
-            {
-              state: 'finished'
-            }
-          end
-
-          before(:each) do
-            apply_authentication_headers_for @running_playoff_match.owner
-          end
-
-          before do
-            @running_playoff_match.match_scores.each_with_index do |ms, i|
-              ms.points = i
-              ms.save!
-            end
-            put :update, params: { id: @running_playoff_match.to_param }.merge(finished)
-            @running_playoff_match.reload
-          end
-
-          it 'updates the matches status' do
-            expect(response).to be_successful
-            expect(@running_playoff_match.state).to eq(finished[:state])
-          end
-
-          describe 'updates the match below' do
-            before do
-              @match_below = @tournament.stages.find_by(level: @amount_of_stages - 1).matches
-                                        .find_by(position: @running_playoff_match.position / 2).reload
-            end
-
-            it 'with the right teams' do
-              expect(@running_playoff_match.winner).to be_a(Team)
-              expect(@match_below.teams).to include(@running_playoff_match.winner)
-            end
-
-            it 'with the right status' do
-              expect(@match_below.state).to eq('not_ready')
-            end
-          end
-        end
       end
 
-      context 'with invalid params' do
+      let(:senseless_update) do
-        it 'renders an unprocessable entity response' do
+        {
-          put :update, params: { id: @match.to_param }.merge(invalid_update)
+          state: 'not_ready'
-          expect(response).to have_http_status(:unprocessable_entity)
+        }
-        end
       end
-    end
 
-    context 'as another user' do
+      context 'as owner' do
-      context 'with valid params' do
         before(:each) do
-          apply_authentication_headers_for create(:user)
+          apply_authentication_headers_for @match.owner
         end
 
-        it 'renders a forbidden error response' do
+        context 'with valid params' do
-          put :update, params: { id: @match.to_param }.merge(valid_update)
+          it 'updates the match' do
-          expect(response).to have_http_status(:forbidden)
+            put :update, params: { id: @match.to_param }.merge(valid_update)
+            @match.reload
+            expect(response).to be_successful
+            expect(@match.state).to eq(valid_update[:state])
+          end
+
+          it 'renders a response with the updated match' do
+            put :update, params: { id: @match.to_param }.merge(valid_update)
+            expect(response).to be_successful
+            body = deserialize_response response
+            expect(body[:state]).to eq(valid_update[:state])
+          end
+
+          context 'on a running playoff match' do
+            let(:finished) do
+              {
+                state: 'finished'
+              }
+            end
+
+            before(:each) do
+              apply_authentication_headers_for @running_playoff_match.owner
+            end
+
+            context 'match update succeeds' do
+              context 'on a decided match' do
+                before do
+                  @running_playoff_match.match_scores.each_with_index do |ms, i|
+                    ms.points = i
+                    ms.save!
+                  end
+                  put :update, params: { id: @running_playoff_match.to_param }.merge(finished)
+                  @running_playoff_match.reload
+                end
+
+                it 'updates the matches status' do
+                  expect(response).to be_successful
+                  expect(@running_playoff_match.state).to eq(finished[:state])
+                end
+
+                describe 'updates the match below' do
+                  before do
+                    @match_below = @tournament.stages.find_by(level: @amount_of_stages - 1).matches
+                                              .find_by(position: @running_playoff_match.position / 2).reload
+                  end
+
+                  it 'with the right teams' do
+                    expect(@running_playoff_match.winner).to be_a(Team)
+                    expect(@match_below.teams).to include(@running_playoff_match.winner)
+                  end
+
+                  it 'with the right status' do
+                    expect(@match_below.state).to eq('not_ready')
+                  end
+                end
+              end
+
+              context 'on an undecided match' do
+                before do
+                  @running_playoff_match.match_scores.each do |ms|
+                    ms.points = 42
+                    ms.save!
+                  end
+                  put :update, params: { id: @running_playoff_match.to_param }.merge(finished)
+                  @running_playoff_match.reload
+                end
+
+                it 'returns an unprocessable entity response' do
+                  expect(response).to have_http_status(:unprocessable_entity)
+                end
+
+                it 'doesn\'t change the matches status' do
+                  expect(@running_playoff_match.state).to eq('in_progress')
+                end
+
+                describe 'doesn\'t update the match below' do
+                  before do
+                    @match_below = @tournament.stages.find_by(level: @amount_of_stages - 1).matches
+                                              .find_by(position: @running_playoff_match.position / 2).reload
+                  end
+
+                  it 'teams' do
+                    expect(@match_below.teams.empty?).to be(true)
+                  end
+
+                  it 'status' do
+                    expect(@match_below.state).to eq('not_ready')
+                  end
+                end
+              end
+            end
+
+            context 'match update fails' do
+              before do
+                allow_any_instance_of(Match)
+                  .to receive(:update)
+                  .and_return(false)
+              end
+
+              it 'returns unprocessable entity' do
+                put :update, params: { id: @running_playoff_match.to_param }.merge(finished)
+                expect(response).to have_http_status(:unprocessable_entity)
+              end
+            end
+
+            context 'PopulateMatchBelowAndSave fails' do
+              before do
+                expect(PopulateMatchBelowAndSave).to receive(:call).once.with(match: @running_playoff_match)
+                                                                   .and_return(context)
+              end
+
+              context 'when unsuccessful' do
+                let(:context) { double(:context, success?: false) }
+
+                it 'returns unprocessable entity' do
+                  put :update, params: { id: @running_playoff_match.to_param }.merge(finished)
+                  expect(response).to have_http_status(:unprocessable_entity)
+                end
+              end
+            end
+          end
+        end
+
+        context 'with invalid params' do
+          it 'renders an unprocessable entity response' do
+            put :update, params: { id: @match.to_param }.merge(invalid_update)
+            expect(response).to have_http_status(:unprocessable_entity)
+          end
+        end
+
+        context 'with senseless params' do
+          it 'renders an unprocessable entity response' do
+            put :update, params: { id: @match.to_param }.merge(senseless_update)
+            expect(response).to have_http_status(:unprocessable_entity)
+          end
+        end
+      end
+
+      context 'as another user' do
+        context 'with valid params' do
+          before(:each) do
+            apply_authentication_headers_for create(:user)
+          end
+
+          it 'renders a forbidden error response' do
+            put :update, params: { id: @match.to_param }.merge(valid_update)
+            expect(response).to have_http_status(:forbidden)
+          end
+        end
+      end
+    end
+
+    context 'on a playoff match that isn\'t ready yet' do
+      let(:invalid_update) do
+        {
+          state: 'in_progress'
+        }
+      end
+
+      context 'as owner' do
+        before(:each) do
+          apply_authentication_headers_for @not_ready_playoff_match.owner
+        end
+
+        context 'with invalid params' do
+          it 'renders an unprocessable entity response' do
+            put :update, params: { id: @not_ready_playoff_match.to_param }.merge(invalid_update)
+            expect(response).to have_http_status(:unprocessable_entity)
+          end
         end
       end
     end

spec/factories/matches.rb

@@ -15,7 +15,7 @@ FactoryBot.define do
     end
 
     factory :empty_prepared_playoff_match do
-      state { :not_started }
+      state { :not_ready }
     end
 
     factory :decided_playoff_match do