Refactor player enliting

Author: cveneziani

app/controllers/games_controller.rb

@@ -1,10 +1,9 @@
 class GamesController < ApplicationController
-  before_action :set_game,  only: [:show]
-  before_action :join_game, only: [:show], unless: :player_in_game?
+  before_action :set_game, only: [:show]
 
   def show
     @opponent ||= @game.players.where.not(id: current_player&.id).first || NilPlayer.new
-    return render FinishedGame.template_path if @game.state == 'finished'
+    render game_template || :show
   end
 
   def create
@@ -14,14 +13,20 @@ def create
 
   private
 
-  def join_game
-    return if @game.state == 'finished'
-    enlist  = Players::Enlist.new(game: @game, player: current_player)
+  def game_params
+    params.require(:game).permit(:name)
+  end
 
-    if enlist.game_full?
-      return render FullGame.template_path
-    else
-      return render AwaitingOpponentGame.template_path
+  def game_template
+    case @game.state
+    when 'awaiting_opponent'
+      return if player_in_game?
+      AwaitingOpponentGame.template_path
+    when 'running'
+      return if player_in_game?
+      FullGame.template_path
+    when 'finished'
+      FinishedGame.template_path
     end
   end
 
@@ -33,8 +38,4 @@ def set_game
     @game = Game.find_by(slug: params[:id].parameterize)
     redirect_to root_path unless @game
   end
-
-  def game_params
-    params.require(:game).permit(:name)
-  end
 end

app/controllers/opponent_controller.rb

@@ -2,21 +2,22 @@ class OpponentController < ApplicationController
   before_action :set_game,  only: [:create]
 
   def create
-    enlist = Players::Enlist.new(game: @game, player: current_player)
-    return redirect_to game_path(@game) if enlist.game_full?
-
-    payload = enlist.assign_game_to_player!
-    set_current_player(enlist.player)
-    ActionCable.server.broadcast "game_#{current_player.game_id}", payload
+    @enlist = Players::Enlist.new(game: @game, player: current_player)
+    enlist_player if @enlist.available?
 
     redirect_to game_path(@game)
   end
 
   private
 
+  def enlist_player
+    payload = @enlist.assign_game_to_player!
+    set_current_player(@enlist.player)
+    ActionCable.server.broadcast "game_#{current_player.game_id}", payload
+  end
+
   def set_game
     @game = Game.find_by(slug: params[:game_id].parameterize)
-    return redirect_to root_path        unless @game
-    return redirect_to game_path(@game) if @game.state != 'awaiting_opponent'
+    return redirect_to root_path unless @game
   end
 end

app/dispatches/players/enlist.rb

@@ -1,7 +1,5 @@
 module Players
   class Enlist
-    MAX_PLAYERS = 2
-
     private
     attr_reader :game
 
@@ -15,13 +13,13 @@ def initialize(game:, player:)
 
     def assign_game_to_player!
       @player = Player.create!(nickname: nickname, game: game, life: Player::LIFE)
-      @game.update(state: 'running')
+      game.update(state: 'running')
 
       return successful_payload
     end
 
-    def game_full?
-      game.players.count >= MAX_PLAYERS
+    def available?
+      game.state == 'awaiting_opponent' && !game.full?
     end
 
     private

app/models/game.rb

@@ -3,12 +3,18 @@
 # - running:  the game is started
 # - finished: game over. A player has destroyed his ennemy mothership
 class Game < ApplicationRecord
+  MAX_PLAYERS = 2
+
+  has_one :loser,  -> { where(won: false ) }, class_name: 'Player'
+  has_one :winner, -> { where(won: true ) },  class_name: 'Player'
+
   has_many :players, dependent: :destroy
   has_many :ships,   through: :players
   has_many :words,   dependent: :destroy
 
-  has_one :loser,  -> { where(won: false ) }, class_name: 'Player'
-  has_one :winner, -> { where(won: true ) },  class_name: 'Player'
+  def full?
+    players.count >= MAX_PLAYERS
+  end
 
   def to_param
     slug

spec/models/game_spec.rb

@@ -5,4 +5,40 @@
 
   it { is_expected.to have_one(:loser).class_name(Player).conditions(won: false) }
   it { is_expected.to have_one(:winner).class_name(Player).conditions(won: true) }
+
+  it { is_expected.to have_many(:players) }
+  it { is_expected.to have_many(:ships) }
+  it { is_expected.to have_many(:words) }
+
+  describe '#full?' do
+    before :each do
+      game.save!
+    end
+
+    context 'when there is no player' do
+      it 'returns false' do
+        expect(game.full?).to eq(false)
+      end
+    end
+
+    context 'when there is one player' do
+      before :each do
+        game.players.create!
+      end
+
+      it 'returns false' do
+        expect(game.full?).to eq(false)
+      end
+    end
+
+    context 'when there are two players' do
+      before :each do
+        2.times { game.players.create! }
+      end
+
+      it 'returns true' do
+        expect(game.full?).to eq(true)
+      end
+    end
+  end
 end

spec/requests/games_spec.rb

@@ -4,34 +4,40 @@
   let(:game) { Game.create!(name: 'Starship Battle', slug: 'starship-battle') }
 
   describe "GET show" do
-    context "url is not parameterized" do
-      before :each do
-        get URI::encode("/games/#{game.name}")
-      end
-
-      it { expect(Game.count).to eq(1) }
-    end
-
     context "player is signed in" do
       let(:player) { Player.create! }
 
       before :each do
         allow_any_instance_of(ApplicationController).to receive(:current_player).and_return(player)
       end
 
+      context "url is not parameterized" do
+        before :each do
+          game.update(state: 'awaiting_opponent')
+          allow_any_instance_of(ApplicationController).to receive(:current_player).and_return(player)
+          get URI::encode("/games/#{game.name}")
+        end
+
+        it { expect(Game.count).to eq(1) }
+      end
+
       context "game is full" do
-        before { 2.times { Player.create(game: game) } }
+        before :each do
+          game.update(state: 'running')
+          2.times { Player.create(game: game) }
+          get "/games/#{game.to_param}"
+        end
 
-        before { get "/games/#{game.to_param}" }
 
         it { expect(response.body).to include("already full") }
         it { expect(player.reload.game).to_not eq(game) }
       end
 
-      context  "player joined the game and refresh the page" do
+      context "player joined the game and refresh the page" do
         let!(:opponent) { Player.create(game: game) }
 
         before :each do
+          game.update(state: 'running')
           player.update(game: game)
 
           get "/games/#{game.to_param}"
@@ -45,6 +51,7 @@
         let(:other_game) { Game.create!(name: 'Other game', slug: 'other-game') }
 
         before :each do
+          other_game.update(state: 'awaiting_opponent')
           player.update(game: game, nickname: "Rico", life: 3)
           get "/games/#{other_game.to_param}"
         end
@@ -84,9 +91,11 @@
       end
 
       context "game is already full" do
-        before { 2.times { Player.create!(game: game) } }
-
-        before { get "/games/#{game.to_param}" }
+        before :each do
+          game.update(state: 'running')
+          2.times { Player.create!(game: game) }
+          get "/games/#{game.to_param}"
+        end
 
         it { expect(response).to have_http_status(200) }
         it { expect(Player.count).to eq(2) }