From 2b7d0edf74f736d2796c3e4b8c42cdb00e2f12ae Mon Sep 17 00:00:00 2001
From: Malaber <32635600+Malaber@users.noreply.github.com>
Date: Wed, 29 May 2019 15:40:07 +0200
Subject: [PATCH 1/8] Correct state of empty_prepared_playoff_match factory

---
 spec/factories/matches.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/spec/factories/matches.rb b/spec/factories/matches.rb
index b200839..5c8a42e 100644
--- a/spec/factories/matches.rb
+++ b/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

From 68535b89fc40ba4abe930e66d31147316a54d33b Mon Sep 17 00:00:00 2001
From: Malaber <32635600+Malaber@users.noreply.github.com>
Date: Wed, 29 May 2019 15:57:25 +0200
Subject: [PATCH 2/8] Prevent matches from being stopped without a winner in
 playoffs

---
 app/controllers/matches_controller.rb       |  5 ++
 spec/controllers/matches_controller_spec.rb | 77 +++++++++++++++------
 2 files changed, 60 insertions(+), 22 deletions(-)

diff --git a/app/controllers/matches_controller.rb b/app/controllers/matches_controller.rb
index d64c60d..b978852 100644
--- a/app/controllers/matches_controller.rb
+++ b/app/controllers/matches_controller.rb
@@ -13,6 +13,11 @@ class MatchesController < ApplicationController
   # PATCH/PUT /matches/1
   def update
     new_state = match_params['state']
+    if new_state == 'finished' && @match.current_leading_team.nil?
+      render json: { error: 'Stopping undecided Matches isn\'t allowed in playoff stage' },
+             status: :unprocessable_entity
+      return
+    end
     if @match.update(match_params)
       if new_state == 'finished'
         result = PopulateMatchBelowAndSave.call(match: @match) unless @match.group_match?
diff --git a/spec/controllers/matches_controller_spec.rb b/spec/controllers/matches_controller_spec.rb
index 7606be1..48c73a2 100644
--- a/spec/controllers/matches_controller_spec.rb
+++ b/spec/controllers/matches_controller_spec.rb
@@ -70,33 +70,66 @@ RSpec.describe MatchesController, type: :controller 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
+          context 'on a decided match' do
             before do
-              @match_below = @tournament.stages.find_by(level: @amount_of_stages - 1).matches
-                                        .find_by(position: @running_playoff_match.position / 2).reload
+              @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 'with the right teams' do
-              expect(@running_playoff_match.winner).to be_a(Team)
-              expect(@match_below.teams).to include(@running_playoff_match.winner)
+            it 'updates the matches status' do
+              expect(response).to be_successful
+              expect(@running_playoff_match.state).to eq(finished[:state])
             end
 
-            it 'with the right status' do
-              expect(@match_below.state).to eq('not_ready')
+            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 a 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 unprocessable entity' do
+              expect(response).to have_http_status(:unprocessable_entity)
+              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

From ab33ec157da4d48f73f558be1d24fcdbc3b833b3 Mon Sep 17 00:00:00 2001
From: Malaber <32635600+Malaber@users.noreply.github.com>
Date: Wed, 29 May 2019 18:14:04 +0200
Subject: [PATCH 3/8] Split test methods for failed stopping of matches

---
 spec/controllers/matches_controller_spec.rb | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/spec/controllers/matches_controller_spec.rb b/spec/controllers/matches_controller_spec.rb
index 48c73a2..57b920e 100644
--- a/spec/controllers/matches_controller_spec.rb
+++ b/spec/controllers/matches_controller_spec.rb
@@ -114,6 +114,9 @@ RSpec.describe MatchesController, type: :controller do
 
             it 'returns unprocessable entity' 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
 

From c1b2b72ca3c967d1893f42f2c44031a86eccd915 Mon Sep 17 00:00:00 2001
From: Malaber <32635600+Malaber@users.noreply.github.com>
Date: Wed, 29 May 2019 18:30:10 +0200
Subject: [PATCH 4/8] Simplify controller code

Matches update now gets rolled back via a Transaction
---
 app/controllers/matches_controller.rb | 40 +++++++++++++++++----------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/app/controllers/matches_controller.rb b/app/controllers/matches_controller.rb
index b978852..ca5275d 100644
--- a/app/controllers/matches_controller.rb
+++ b/app/controllers/matches_controller.rb
@@ -13,27 +13,37 @@ class MatchesController < ApplicationController
   # PATCH/PUT /matches/1
   def update
     new_state = match_params['state']
-    if new_state == 'finished' && @match.current_leading_team.nil?
-      render json: { error: 'Stopping undecided Matches isn\'t allowed in playoff stage' },
-             status: :unprocessable_entity
-      return
-    end
-    if @match.update(match_params)
-      if new_state == 'finished'
-        result = PopulateMatchBelowAndSave.call(match: @match) unless @match.group_match?
-        unless result.success?
-          render json: { error: 'Moving Team one stage down failed' }, status: :unprocessable_entity
-          return
-        end
+
+    Match.transaction do
+      if @match.update(match_params)
+        handle_match_end if new_state == 'finished'
+
+        render json: @match
+      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'

From d03ceeffa4bab135ac320a34f63680a8770331a1 Mon Sep 17 00:00:00 2001
From: Malaber <32635600+Malaber@users.noreply.github.com>
Date: Tue, 4 Jun 2019 19:03:13 +0200
Subject: [PATCH 5/8] Test returning unprocessable entity response

We now test sending a senseless state and also what happens when the
match.update method fails for some reason.
---
 spec/controllers/matches_controller_spec.rb | 268 +++++++++++---------
 1 file changed, 149 insertions(+), 119 deletions(-)

diff --git a/spec/controllers/matches_controller_spec.rb b/spec/controllers/matches_controller_spec.rb
index 57b920e..f0a4fa4 100644
--- a/spec/controllers/matches_controller_spec.rb
+++ b/spec/controllers/matches_controller_spec.rb
@@ -27,134 +27,164 @@ RSpec.describe MatchesController, type: :controller do
   end
 
   describe 'POST #update' do
-    let(:valid_update) do
-      {
-        state: 'in_progress'
-      }
-    end
-
-    let(:invalid_update) do
-      {
-        state: 'finished'
-      }
-    end
-
-    context 'as owner' do
-      before(:each) do
-        apply_authentication_headers_for @match.owner
+    context 'on a running playoff match' do
+      let(:valid_update) do
+        {
+          state: 'in_progress'
+        }
       end
 
-      context 'with valid params' do
-        it 'updates the match' do
-          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 '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 a 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 unprocessable entity' 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
+      let(:invalid_update) do
+        {
+          state: 'finished'
+        }
       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
+      let(:senseless_update) do
+        {
+          state: 'not_ready'
+        }
       end
-    end
 
-    context 'as another user' do
-      context 'with valid params' do
+      context 'as owner' do
         before(:each) do
-          apply_authentication_headers_for create(:user)
+          apply_authentication_headers_for @match.owner
         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)
+        context 'with valid params' do
+          it 'updates the match' do
+            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 a 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 unprocessable entity' 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
+          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

From 41b8bdce776cfe44320af2c76452bfa39fd73620 Mon Sep 17 00:00:00 2001
From: Malaber <32635600+Malaber@users.noreply.github.com>
Date: Tue, 4 Jun 2019 19:04:12 +0200
Subject: [PATCH 6/8] Test unprocessable entity response on a match that is not
 ready

Unprocessable entity is returned when you try to start a match that is
not ready yet.
---
 spec/controllers/matches_controller_spec.rb | 22 +++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/spec/controllers/matches_controller_spec.rb b/spec/controllers/matches_controller_spec.rb
index f0a4fa4..f1721cd 100644
--- a/spec/controllers/matches_controller_spec.rb
+++ b/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
 
@@ -188,5 +189,26 @@ RSpec.describe MatchesController, type: :controller do
         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
   end
 end

From 9c9a8562652fb15757cd433ff923b781a40de503 Mon Sep 17 00:00:00 2001
From: Malaber <32635600+Malaber@users.noreply.github.com>
Date: Tue, 4 Jun 2019 23:39:34 +0200
Subject: [PATCH 7/8] Test returning unprocessable entity when error occurs in
 match population

---
 spec/controllers/matches_controller_spec.rb | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/spec/controllers/matches_controller_spec.rb b/spec/controllers/matches_controller_spec.rb
index f1721cd..0825e8e 100644
--- a/spec/controllers/matches_controller_spec.rb
+++ b/spec/controllers/matches_controller_spec.rb
@@ -158,6 +158,22 @@ RSpec.describe MatchesController, type: :controller do
                 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
 

From 5d47ae54267d7e12348d5dd22dd703fc06083bb4 Mon Sep 17 00:00:00 2001
From: Malaber <32635600+Malaber@users.noreply.github.com>
Date: Wed, 5 Jun 2019 10:53:18 +0200
Subject: [PATCH 8/8] Fix two typos

---
 spec/controllers/matches_controller_spec.rb | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/spec/controllers/matches_controller_spec.rb b/spec/controllers/matches_controller_spec.rb
index 0825e8e..0078b61 100644
--- a/spec/controllers/matches_controller_spec.rb
+++ b/spec/controllers/matches_controller_spec.rb
@@ -111,7 +111,7 @@ RSpec.describe MatchesController, type: :controller do
                 end
               end
 
-              context 'on a undecided match' do
+              context 'on an undecided match' do
                 before do
                   @running_playoff_match.match_scores.each do |ms|
                     ms.points = 42
@@ -121,7 +121,7 @@ RSpec.describe MatchesController, type: :controller do
                   @running_playoff_match.reload
                 end
 
-                it 'returns unprocessable entity' do
+                it 'returns an unprocessable entity response' do
                   expect(response).to have_http_status(:unprocessable_entity)
                 end