Merge pull request #95 from ruby-nord/bug/update-player-reload

Bug/update player reload

Author: cveneziani

app/assets/javascripts/battletype.js

@@ -6,6 +6,7 @@
 //= require battletype/combat_zone
 //= require battletype/dockyard
 //= require battletype/ship
+//= require battletype/player
 //= require battletype/life_o_meter
 
 (function () {
@@ -32,6 +33,9 @@
 
       this.mothership = Dockyard.launchMothership(this.$combatZone);
 
+      this.player   = Player.build(document.getElementById("current_player_nickname"));
+      this.opponent = Player.build(document.getElementById("opponent_nickname"));
+
       this.playerLifeOMeter   = LifeOMeter.activate(document.getElementById("life_player"));
       this.opponentLifeOMeter = LifeOMeter.activate(document.getElementById("life_opponent"));
 
@@ -118,9 +122,18 @@
       case "player_joined":
         if (payload.player_id != this.playerId) {
           var nickname = payload.nickname;
+          Battletype.opponent.nickname = payload.nickname;
+          this._logs.displayMessage(payload.code);
+        }
+        break;
+      case "player_nickname_changed":
+        var nickname = payload.nickname;
 
-          this.$combatZone.find("#opponent_nickname").text(nickname); // TODO tell-dont-ask
+        if (payload.player_id == this.playerId) {
+          Battletype.player.nickname = payload.nickname;
           this._logs.displayMessage(payload.code);
+        } else {
+          Battletype.opponent.nickname = payload.nickname;
         }
         break;
       }

app/assets/javascripts/battletype/logs.js

@@ -1,24 +1,26 @@
 (function () {
   var LOG_CODES_AND_TYPES_MAP = {
-    "failed_attack":      "attack",
-    "failed_bombing":     "attack",
-    "failed_defense":     "defense",
-    "game_won":           "game",
-    "player_joined":      "player",
-    "successful_attack":  "attack",
-    "successful_bombing": "bomb",
-    "successful_defense": "defense",
+    "failed_attack":           "attack",
+    "failed_bombing":          "attack",
+    "failed_defense":          "defense",
+    "game_won":                "game",
+    "player_joined":           "player",
+    "player_nickname_changed": "player",
+    "successful_attack":       "attack",
+    "successful_bombing":      "bomb",
+    "successful_defense":      "defense",
   };
 
   var LOG_CODES_AND_MESSAGES_MAP = {
-    "failed_attack":      "OOPS! Attack failed",
-    "failed_bombing":     "OOPS! Bombing failed",
-    "failed_defense":     "OOPS! Defense failed",
-    "game_won":           "Game finished!",
-    "player_joined":      "Opponent joined",
-    "successful_attack":  "New ship launched!",
-    "successful_bombing": "Mothership hit!",
-    "successful_defense": "Ship destroyed!",
+    "failed_attack":           "OOPS! Attack failed",
+    "failed_bombing":          "OOPS! Bombing failed",
+    "failed_defense":          "OOPS! Defense failed",
+    "game_won":                "Game finished!",
+    "player_joined":           "Opponent joined",
+    "player_nickname_changed": "Nickname changed",
+    "successful_attack":       "New ship launched!",
+    "successful_bombing":      "Mothership hit!",
+    "successful_defense":      "Ship destroyed!",
   };
 
   var ERROR_CODES_AND_MESSAGES_MAP = {

app/assets/javascripts/battletype/player.js

@@ -0,0 +1,21 @@
+(function () {
+  this.Player = {
+    _properties: {
+      nickname: {
+        get: function () { return this._nickname; },
+        set: function (nickname) {
+          this._nickname = nickname;
+
+          $(this)
+            .text(nickname)
+            .fadeIn("slow")
+            .blur();
+        }
+      }
+    },
+    build: function (node) {
+      var player = Object.defineProperties(node, this._properties);
+      return player;
+    }
+  };
+}).call(this);

app/controllers/players_controller.rb

@@ -2,18 +2,16 @@ class PlayersController < ApplicationController
   before_action :set_player
 
   def update
-    return render json: {error: "Cannot update another user"}, status: 401 if @player != current_player
+    return render json: { error: "Cannot update another user" }, status: 401 if @player != current_player
 
-    @player.update(player_params)
-    render json: {status: :ok}, status: 200
+    payload = Players::ChangeNickname.call(player: @player, nickname: params[:player][:nickname])
+    ActionCable.server.broadcast "game_#{current_player.game_id}", payload
+    head 200
   end
 
   private
+
   def set_player
     @player = Player.find(params[:id])
   end
-
-  def player_params
-    params.require(:player).permit(:nickname)
-  end
 end

app/dispatches/players/change_nickname.rb

@@ -0,0 +1,33 @@
+module Players
+  class ChangeNickname
+    private
+    attr_reader :nickname, :player
+
+    public
+
+    def self.call(player:, nickname:)
+      new(player: player, nickname: nickname).call
+    end
+
+    def initialize(player:, nickname:)
+      @nickname = nickname
+      @player   = player
+    end
+
+    def call
+      update_nickname
+
+      return successful_payload
+    end
+
+    private
+
+    def update_nickname
+      player.update(nickname: nickname)
+    end
+
+    def successful_payload
+      { code: "player_nickname_changed", player_id: @player.id, nickname:  @player.nickname }
+    end
+  end
+end

app/views/games/show.html.erb

@@ -19,9 +19,8 @@
         <li class="life_bar"><%= render "games/hud/life" %></li>
         <li class="life_bar"><%= render "games/hud/life" %></li>
       </ul>
-      <!-- <h1 class="player">Casimir</h1> -->
       <div id="current_player" data-player-id="<%= current_player.id %>" data-player-nickname="<%= current_player.nickname %>"></div>
-      <%= form_for(current_player, url: player_path(current_player), class: "player") do |f|%>
+      <%= form_for(current_player, url: player_path(current_player), data: { remote: true }, class: "player") do |f| %>
         <%= f.text_field :nickname, id: 'current_player_nickname' %>
       <% end %>
       <ul id="life_opponent">

spec/dispatches/players/change_nickname_spec.rb

@@ -0,0 +1,21 @@
+require "rails_helper"
+
+RSpec.describe "Players::ChangeNickname", type: :dispatch do
+  subject(:dispatch)  { Players::ChangeNickname }
+  let(:game)          { Game.create! }
+  let(:player)        { Player.create!(game: game, nickname: "Rico") }
+
+  describe ".call" do
+    it 'updates player nickname' do
+      expect { dispatch.call(player: player, nickname: "Zim") }.to change { player.nickname }.from("Rico").to("Zim")
+    end
+
+    it 'returns a payload' do
+      expect(dispatch.call(player: player, nickname: "Zim")).to be_instance_of(Hash)
+    end
+
+    it "returns payload with the new nickname of the player" do
+      expect(dispatch.call(player: player, nickname: "Zim")).to include(code: 'player_nickname_changed', player_id: player.id, nickname: "Zim")
+    end
+  end
+end

spec/requests/players_spec.rb

@@ -1,28 +1,44 @@
 require "rails_helper"
 
 RSpec.describe "Players", type: :request do
-  let(:player)          { Player.create!(nickname: "Rico") }
+  let(:player) { Player.create!(nickname: "Rico") }
 
   describe "PUT update" do
     context 'update current_player' do
       before :each do
         allow_any_instance_of(ApplicationController).to receive(:current_player).and_return(player)
       end
 
-      before { put "/players/#{player.id}", params: {player: {nickname: "Zim"}} }
-      it { expect(response).to have_http_status(200) }
-      it { expect(player.reload.nickname).to eq("Zim") }
-    end
-  end
+      it "returns 200 HTTP status" do
+        put "/players/#{player.id}", params: { player: { nickname: "Zim" }}
+        expect(response).to have_http_status(200)
+      end
 
-  context 'update another player' do
-    let(:other_player) { Player.create!(nickname: "Carmen") }
-    before :each do
-      allow_any_instance_of(ApplicationController).to receive(:current_player).and_return(other_player)
+      it "updates player nickname" do
+        put "/players/#{player.id}", params: { player: { nickname: "Zim" }}
+        expect(player.reload.nickname).to eq("Zim")
+      end
+
+      it 'broadcasts a player nickname updated payload' do
+        allow(ActionCable.server).to receive(:broadcast)
+        put "/players/#{player.id}", params: { player: { nickname: "Zim" }}
+        expect(ActionCable.server).to have_received(:broadcast).with(
+          anything,
+          code: 'player_nickname_changed', player_id: player.id, nickname: 'Zim'
+        )
+      end
     end
 
-    before { put "/players/#{player.id}", params: {player: {nickname: "Ace"}} }
-    it { expect(response).to have_http_status(401) }
-    it { expect(player.reload.nickname).to eq("Rico") }
+    context 'update another player' do
+      let(:other_player) { Player.create!(nickname: "Carmen") }
+
+      before :each do
+        allow_any_instance_of(ApplicationController).to receive(:current_player).and_return(other_player)
+        put "/players/#{player.id}", params: { player: { nickname: "Ace" }}
+      end
+
+      it { expect(response).to have_http_status(401) }
+      it { expect(player.reload.nickname).to eq("Rico") }
+    end
   end
-end
+end