Merge pull request #42 from cblanc/fix_users_api

Fix users api

app/controllers/api/v1/users_controller.rb

@@ -5,7 +5,7 @@ class Api::V1::UsersController < Api::V1::BaseController
 
   def show
     @user = User.find(params[:id])
-    @steam = SteamCondenser::Community::SteamId.from_steam_id("STEAM_#{@user.steamid}")
+    @steam = steam_profile @user
 
     render json: {
       id: @user.id,
@@ -15,8 +15,8 @@ class Api::V1::UsersController < Api::V1::BaseController
       avatar: @user.profile.avatar.url,
       admin: @user.admin?,
       steam: {
-        url: @steam.base_url, 
-        nickname: @steam.nickname
+        url: @steam.nil? ? nil : @steam.base_url,
+        nickname: @steam.nil? ? nil : @steam.nickname
       },
       bans: {
         gather: @user.banned?(Ban::TYPE_GATHER).present?,
@@ -28,4 +28,12 @@ class Api::V1::UsersController < Api::V1::BaseController
   rescue ActiveRecord::RecordNotFound
     raise ActionController::RoutingError.new('User Not Found')
   end
+
+  private
+
+  def steam_profile(user)
+    SteamCondenser::Community::SteamId.from_steam_id("STEAM_#{user.steamid}")
+  rescue SteamCondenser::Error
+    nil
+  end
 end

app/services/api/v1/users_collection.rb

@@ -29,7 +29,8 @@ class Api::V1::UsersCollection < Api::V1::Collection
       users_table[:steamid],
       teams_table[:name],
       teams_table[:tag],
-      teams_table[:logo]
+      teams_table[:logo],
+      users_table[:id]
     ]
   end
 
@@ -45,6 +46,7 @@ class Api::V1::UsersCollection < Api::V1::Collection
   def map_query
     execute_query.map do |row|
       {
+        id: row[5],
         username: row[0],
         steamid: row[1],
         team: {

spec/controllers/api/v1/users_controller_spec.rb

@@ -1,76 +1,87 @@
-require 'spec_helper'
+require "spec_helper"
 
 describe Api::V1::UsersController do
   before do
-    request.accept = 'application/json'
+    request.accept = "application/json"
   end
 
-  describe '#show' do
+  describe "#show" do
     before(:each) do
       @user = create :user, :chris
     end
 
-    it 'returns user data' do
+    it "returns user data" do
       get :show, id: @user.id
 
       expect(response).to be_success
-      expect(json['id']).to eq(@user.id)
-      expect(json['username']).to eq(@user.username)
-      expect(json['country']).to eq(@user.country)
-      expect(json['time_zone']).to eq(@user.time_zone)
-      expect(json['admin']).to eq(@user.admin?)
+      expect(json["id"]).to eq(@user.id)
+      expect(json["username"]).to eq(@user.username)
+      expect(json["country"]).to eq(@user.country)
+      expect(json["time_zone"]).to eq(@user.time_zone)
+      expect(json["admin"]).to eq(@user.admin?)
       expect(json).to have_key("steam")
-      expect(json['steam']).to have_key("url")
-      expect(json['steam']).to have_key("nickname")
-      expect(json['bans']['mute']).to eq(false)
-      expect(json['bans']['gather']).to eq(false)
-      expect(json['bans']['site']).to eq(false)
-      expect(json['team']).to be_nil
+      expect(json["steam"]).to have_key("url")
+      expect(json["steam"]).to have_key("nickname")
+      expect(json["bans"]["mute"]).to eq(false)
+      expect(json["bans"]["gather"]).to eq(false)
+      expect(json["bans"]["site"]).to eq(false)
+      expect(json["team"]).to be_nil
     end
 
-    it 'returns 404 if user does not exist' do
+    it "returns data for users with invalid steam ids" do
+      @user.steamid = "0:0:000"
+      @user.save!
+
+      get :show, id: @user.id
+
+      expect(response).to be_success
+      expect(json["steam"]["url"]).to be_nil
+      expect(json["steam"]["nickname"]).to be_nil
+    end
+
+    it "returns 404 if user does not exist" do
       expect {
         get :show, id: -1
       }.to raise_error(ActionController::RoutingError)
     end
 
-    it 'returns correct ban if user muted' do
+    it "returns correct ban if user muted" do
       create :ban, :mute, user: @user
       get :show, id: @user.id
       expect(response).to be_success
-      expect(json['bans']['mute']).to eq(true)
+      expect(json["bans"]["mute"]).to eq(true)
     end
 
-    it 'returns correct ban if user gather banned' do
+    it "returns correct ban if user gather banned" do
       create :ban, :gather, user: @user
       get :show, id: @user.id
       expect(response).to be_success
-      expect(json['bans']['gather']).to eq(true)
+      expect(json["bans"]["gather"]).to eq(true)
     end
 
-    it 'returns correct ban if user site banned' do
+    it "returns correct ban if user site banned" do
       create :ban, :site, user: @user
       get :show, id: @user.id
       expect(response).to be_success
-      expect(json['bans']['site']).to eq(true)
+      expect(json["bans"]["site"]).to eq(true)
     end
 
-    it 'returns team information' do
+    it "returns team information" do
       @user.destroy
       @user_with_team = create :user_with_team, :chris
       get :show, id: @user_with_team.id
       expect(response).to be_success
-      expect(json['team']['id']).to eq(@user_with_team.team.id)
-      expect(json['team']['name']).to eq(@user_with_team.team.name)
+      expect(json["team"]["id"]).to eq(@user_with_team.team.id)
+      expect(json["team"]["name"]).to eq(@user_with_team.team.name)
     end
   end
 
-  describe '#index' do
+  describe "#index" do
     before do
       5.times { create(:user_with_team) }
     end
 
-    it 'returns all users and associated teams' do
+    it "returns all users and associated teams" do
       users = User.all
 
       get :index
@@ -79,13 +90,13 @@ describe Api::V1::UsersController do
       expect(json["users"].size).to eq(users.size)
     end
 
-    it 'returns the excpected JSON keys' do
+    it "returns the excpected JSON keys" do
       get :index
-
       user_json = json["users"].first
       nested_team_json = user_json["team"]
 
       expect(user_json).to have_key("username")
+      expect(user_json).to have_key("id")
       expect(user_json).to have_key("steamid")
       expect(user_json).to have_key("team")
       expect(nested_team_json).to have_key("name")

spec/services/api/v1/users_collection_spec.rb

@@ -1,30 +1,30 @@
-require 'spec_helper'
+require "spec_helper"
 
 describe Api::V1::UsersCollection do
   let(:collection) { Api::V1::UsersCollection.new }
 
-  describe '#execute_query' do
-    describe 'when there are users with no teams' do
+  describe "#execute_query" do
+    describe "when there are users with no teams" do
       before do
         3.times { create(:user) }
       end
 
-      it 'returns 0 results' do
+      it "returns 0 results" do
         expect(collection.execute_query.size).to eq(0)
       end
     end
 
-    describe 'when there are some users with teams' do
+    describe "when there are some users with teams" do
       before do
         3.times { create(:user_with_team) }
       end
 
-      it 'returns 3 results' do
+      it "returns 3 results" do
         expect(collection.execute_query.size).to eq(3)
       end
 
-      it 'returns 5 columns' do
-        expect(collection.execute_query.first.size).to eq(5)
+      it "returns 6 columns" do
+        expect(collection.execute_query.first.size).to eq(6)
       end
     end
   end