Merge pull request #69 from Prommah/develop

Tidying & fixes

app/controllers/matches_controller.rb

@@ -24,12 +24,6 @@ class MatchesController < ApplicationController
   def extra
   end
 
-  def score
-    raise AccessError unless @match.can_update? cuser, [:matchers_attributes]
-    @contester = @match.contester1.team.is_leader?(cuser) ? @match.contester1 : @match.contester2
-    @n = 0
-  end
-
   def ref
     raise AccessError unless @match.can_update? cuser, [:report]
     @n = 0

app/models/match.rb

@@ -38,119 +38,118 @@ class Match < ActiveRecord::Base
   attr_accessor :lineup, :method, :motm_name, :friendly
   attr_protected :id, :updated_at, :created_at, :diff, :points1, :points2
 
-  has_many :matchers, :dependent => :destroy
+  has_many :matchers, dependent: :destroy
-  has_many :users, :through => :matchers
+  has_many :users, through: :matchers
-  has_many :predictions, :dependent => :destroy
+  has_many :predictions, dependent: :destroy
-  has_many :comments, :as => :commentable, :order => "created_at", :dependent => :destroy
+  has_many :comments, as: :commentable, order: "created_at", dependent: :destroy
   belongs_to :challenge
   belongs_to :contest
-  belongs_to :contester1, :class_name => "Contester", :include => 'team'
+  belongs_to :contester1, class_name: "Contester", include: "team"
-  belongs_to :contester2, :class_name => "Contester", :include => 'team'
+  belongs_to :contester2, class_name: "Contester", include: "team"
-  belongs_to :map1, :class_name => "Map"
+  belongs_to :map1, class_name: "Map"
-  belongs_to :map2, :class_name => "Map"
+  belongs_to :map2, class_name: "Map"
   belongs_to :server
-  belongs_to :referee, :class_name => "User"
+  belongs_to :referee, class_name: "User"
-  belongs_to :motm, :class_name => "User"
+  belongs_to :motm, class_name: "User"
-  belongs_to :demo, :class_name => "DataFile"
+  belongs_to :demo, class_name: "DataFile"
   belongs_to :week
-  belongs_to :hltv, :class_name => "Server"
+  belongs_to :hltv, class_name: "Server"
-  belongs_to :stream, :class_name => "Movie"
+  belongs_to :stream, class_name: "Movie"
-  belongs_to :caster, :class_name => "User"
+  belongs_to :caster, class_name: "User"
 
-  scope :future, :conditions => ["match_time > UTC_TIMESTAMP()"]
+  scope :future, conditions: ["match_time > UTC_TIMESTAMP()"]
-  scope :future5, :conditions => ["match_time > UTC_TIMESTAMP()"], :limit => 5
+  scope :future5, conditions: ["match_time > UTC_TIMESTAMP()"], limit: 5
-  scope :finished, :conditions => ["score1 != 0 OR score2 != 0"]
+  scope :finished, conditions: ["score1 != 0 OR score2 != 0"]
-  scope :realfinished, :conditions => ["score1 IS NOT NULL AND score2 IS NOT NULL"]
+  scope :realfinished, conditions: ["score1 IS NOT NULL AND score2 IS NOT NULL"]
-  scope :unfinished, :conditions => ["score1 IS NULL AND score2 IS NULL"]
+  scope :unfinished, conditions: ["score1 IS NULL AND score2 IS NULL"]
-  scope :unreffed, :conditions => ["referee_id IS NULL"]
+  scope :unreffed, conditions: ["referee_id IS NULL"]
-  scope :ordered, :order => "match_time DESC"
+  scope :ordered, order: "match_time DESC"
-  scope :chrono, :order => "match_time ASC"
+  scope :chrono, order: "match_time ASC"
-  scope :recent, :limit => "8"
+  scope :recent, limit: "8"
-  scope :bigrecent, :limit => "50"
+  scope :bigrecent, limit: "50"
-  scope :active, :conditions => ["contest_id IN (?)", Contest.active]
+  scope :active, conditions: ["contest_id IN (?)", Contest.active]
   scope :on_day,
-    lambda { |day| {
+        ->(day) { where("match_time > ? and match_time < ?", day.beginning_of_day, day.end_of_day) }
-    :conditions => ["match_time > ? and match_time < ?", day.beginning_of_day, day.end_of_day]} }
   scope :on_week,
-    lambda { |time| {
+        lambda { |time|
-    :conditions => ["match_time > ? and match_time < ?", time.beginning_of_week, time.end_of_week]} }
+          where("match_time > ? and match_time < ?", time.beginning_of_week, time.end_of_week)
+        }
   scope :of_contester,
-    lambda { |contester| {
+        ->contester { where("contester1_id = ? OR contester2_id = ?", contester.id, contester.id) }
-    :conditions => ["contester1_id = ? OR contester2_id = ?", contester.id, contester.id]} }
   scope :of_user,
-    lambda { |user| {
+        ->user { includes(:matchers).where("matchers.user_id = ?", user.id) }
-    :include => :matchers,
-    :conditions => ["matchers.user_id = ?",  user.id]} }
   scope :of_team,
-    lambda { |team| {
+        lambda { |team|
-    :include => {:contester1 => :team,
+          includes(contester1: :team, contester2: :team)
-                 :contester2 => :team}, :conditions => ["teams.id = ? OR teams_contesters.id = ?", team.id, team.id]} }
+            .where("teams.id = ? OR teams_contesters.id = ?", team.id, team.id)
+        }
   scope :of_userteam,
-    lambda { |user, team| {
+        lambda { |user, team|
-    :include => {:matchers => {:contester => :team}},
+          includes(matchers: { contester: :team })
-    :conditions => ["teams.id = ? AND matchers.user_id = ?", team.id, user.id]} }
+            .where("teams.id = ? AND matchers.user_id = ?", team.id, user.id)
+        }
   scope :within_time,
-    lambda { |from, to| {
+        ->(from, to) { where("match_time > ? AND match_time < ?", from.utc, to.utc) }
-    :conditions => ["match_time > ? AND match_time < ?", from.utc, to.utc]} }
   scope :around,
-    lambda { |time| {
+        lambda { |time|
-    :conditions => ["match_time > ? AND match_time < ?",  time.ago(MATCH_LENGTH).utc, time.ago(-MATCH_LENGTH).utc]} }
+          where("match_time > ? AND match_time < ?",
+                time.ago(MATCH_LENGTH).utc, time.ago(-MATCH_LENGTH).utc)
+        }
   scope :after,
-    lambda { |time| {
+        ->time { where("match_time > ? AND match_time < ?", time.utc, time.ago(-MATCH_LENGTH).utc) }
-    :conditions => ["match_time > ? AND match_time < ?",  time.utc, time.ago(-MATCH_LENGTH).utc]} }
   scope :map_stats,
-    :select => "map1_id, COUNT(*) as num, maps.name",
+        select: "map1_id, COUNT(*) as num, maps.name",
-    :joins => "LEFT JOIN maps ON maps.id = map1_id",
+        joins: "LEFT JOIN maps ON maps.id = map1_id",
-    :group => "map1_id",
+        group: "map1_id",
-    :having => "map1_id is not null",
+        having: "map1_id is not null",
-    :order => "num DESC"
+        order: "num DESC"
   scope :year_stats,
-    :select => "id, DATE_FORMAT(match_time, '%Y') as year, COUNT(*) as num",
+        select: "id, DATE_FORMAT(match_time, '%Y') as year, COUNT(*) as num",
-    :conditions => "match_time > '2000-01-01 01:01:01'",
+        conditions: "match_time > '2000-01-01 01:01:01'",
-    :group => "year",
+        group: "year",
-    :order => "num DESC"
+        order: "num DESC"
   scope :month_stats,
-    :select => "id, DATE_FORMAT(match_time, '%m') as month_n,
+        select: "id, DATE_FORMAT(match_time, '%m') as month_n,
         DATE_FORMAT(match_time, '%M') as month,
         COUNT(*) as num",
-    :conditions => "match_time > '2000-01-01 01:01:01'",
+        conditions: "match_time > '2000-01-01 01:01:01'",
-    :group => "month",
+        group: "month",
-    :order => "month_n"
+        order: "month_n"
 
-  validates_presence_of :contester1, :contester2, :contest
+  validates :contester1, :contester2, :contest, presence: true
-  validates_format_of [:score1, :score2], :with => /\A[0-9]\z/, :allow_nil => true
+  validates :score1, :score2, format: /\A[0-9]\z/, allow_nil: true
-  validates_length_of :report, :maximum => 64000, :allow_blank => true
+  validates :report, length: { maximum: 64_000 }, allow_blank: true
 
   before_create :set_hltv
   after_create :send_notifications
-  before_save :set_motm, :if => Proc.new {|match| match.motm_name and !match.motm_name.empty?}
+  before_save :set_motm, if: proc { |match| match.motm_name && !match.motm_name.empty? }
-  before_update :reset_contest, :if => Proc.new {|match| match.score1_changed? or match.score2_changed?}
+  before_update :reset_contest, if: proc { |match| match.score1_changed? || match.score2_changed? }
   before_destroy :reset_contest
-  after_save :recalculate, :if => Proc.new {|match| match.score1_changed? or match.score2_changed?}
+  after_save :recalculate, if: proc { |match| match.score1_changed? || match.score2_changed? }
-  after_save :set_predictions, :if => Proc.new {|match| match.score1_changed? or match.score2_changed?}
+  after_save :set_predictions, if: proc { |match| match.score1_changed? || match.score2_changed? }
 
-  accepts_nested_attributes_for :matchers, :allow_destroy => true
+  accepts_nested_attributes_for :matchers, allow_destroy: true
 
   def to_s
     contester1.to_s + " vs " + contester2.to_s
   end
 
   def score_color
-    return "black" if score1.nil? or score2.nil? or contester1.nil? or contester2.nil?
+    return "black" if score1.nil? || score2.nil? || contester1.nil? || contester2.nil?
     return "yellow" if score1 == score2
-    return "green" if contester1.team == friendly and score1 > score2
+    return "green" if contester1.team == friendly && score1 > score2
-    return "green" if contester2.team == friendly and score2 > score1
+    return "green" if contester2.team == friendly && score2 > score1
-    return "red" if contester1.team == friendly and score1 < score2
+    return "red" if contester1.team == friendly && score1 < score2
-    "red" if contester2.team == friendly and score2 < score1
+    "red" if contester2.team == friendly && score2 < score1
   end
 
-  def preds contester
+  def preds(contester)
-    perc = Prediction.count(:conditions => ["match_id = ? AND score#{contester} > 2", id])
+    perc = Prediction.count(conditions: ["match_id = ? AND score#{contester} > 2", id])
-    perc != 0 ? (perc/predictions.count.to_f*100).round : 0
+    perc != 0 ? (perc / predictions.count.to_f * 100).round : 0
   end
 
-  def mercs contester
+  def mercs(contester)
-    matchers.all :conditions => {:merc => true, :contester_id => contester.id}
+    matchers.all conditions: { merc: true, contester_id: contester.id }
   end
 
   def get_hltv
@@ -158,18 +157,18 @@ class Match < ActiveRecord::Base
   end
 
   def demo_name
-    Verification.uncrap(contest.short_name + "-" + self.id.to_s + "_" + contester1.to_s + "-vs-" + contester2.to_s)
+    Verification.uncrap("#{contest.short_name}-#{id}_#{contester1}-vs-#{contester2}")
   end
 
   def team1_lineup
-    matchers.all(:conditions => {:contester_id => contester1_id})
+    matchers.all(conditions: { contester_id: contester1_id })
   end
 
   def team2_lineup
-    matchers.all(:conditions => {:contester_id => contester2_id})
+    matchers.all(conditions: { contester_id: contester2_id })
   end
 
-  def get_friendly param = nil
+  def get_friendly(param = nil)
     if param.nil?
       friendly == contester1.team ? contester1 : contester2
     elsif param == :score
@@ -179,7 +178,7 @@ class Match < ActiveRecord::Base
     end
   end
 
-  def get_opponent param = nil
+  def get_opponent(param = nil)
     if param.nil?
       friendly == contester1.team ? contester2 : contester1
     elsif param == :score
@@ -194,7 +193,7 @@ class Match < ActiveRecord::Base
   end
 
   def send_notifications
-    Profile.all(:include => :user, :conditions => "notify_any_match = 1").each do |p|
+    Profile.all(include: :user, conditions: "notify_any_match = 1").find_each do |p|
       Notifications.match p.user, self if p.user
     end
     contester2.team.teamers.active.each do |teamer|
@@ -218,10 +217,11 @@ class Match < ActiveRecord::Base
     contest.recalculate
   end
 
-  #Since ladders are broken anyway, they are not handled here
+  # Since ladders are broken anyway, they are not handled here
   def reset_contest
-    return if score1_was.nil? or score2_was.nil?
+    return if score1_was.nil? || score2_was.nil?
-    return if contest.contest_type == Contest::TYPE_LEAGUE and !contester2.active or !contester1.active
+    return if contest.contest_type == Contest::TYPE_LEAGUE &&
+              !contester2.active || !contester1.active
 
     if score1_was == score2_was
       contester1.draw = contester1.draw - 1
@@ -235,16 +235,17 @@ class Match < ActiveRecord::Base
     end
 
     unless contest.contest_type == Contest::TYPE_BRACKET
-      contester1.score = contester1.score-score1_was
+      contester1.score = contester1.score - score1_was
-      contester2.score = contester2.score-score2_was
+      contester2.score = contester2.score - score2_was
       contester1.save!
       contester2.save!
     end
   end
 
   def recalculate
-    return if score1.nil? or score2.nil?
+    return if score1.nil? || score2.nil?
-    return if contest.contest_type == Contest::TYPE_LEAGUE and !contester2.active or !contester1.active
+    return if contest.contest_type == Contest::TYPE_LEAGUE &&
+              !contester2.active || !contester1.active
 
     if score1 == score2
       contester1.draw = contester1.draw + 1
@@ -263,7 +264,7 @@ class Match < ActiveRecord::Base
       contester2.trend = Contester::TREND_UP
     end
 
-    self.diff = diff ? diff : (contester2.score-contester1.score)
+    self.diff = diff ? diff : (contester2.score - contester1.score)
 
     if contest.contest_type == Contest::TYPE_LADDER
       self.points1 = contest.elo_score score1, score2, diff
@@ -274,23 +275,24 @@ class Match < ActiveRecord::Base
     end
 
     if contest.contest_type == Contest::TYPE_LADDER
-      contester1.extra = contester1.extra + contest.modulus_base/10
+      contester1.extra = contester1.extra + contest.modulus_base / 10
-      contester2.extra = contester2.extra + contest.modulus_base/10
+      contester2.extra = contester2.extra + contest.modulus_base / 10
     end
 
     unless contest.contest_type == Contest::TYPE_BRACKET
-      contester1.score = contester1.score+points1 < 0 ? 0 : contester1.score+points1
+      contester1.score = contester1.score + points1 < 0 ? 0 : contester1.score + points1
-      contester2.score = contester2.score+points2 < 0 ? 0 : contester2.score+points2
+      contester2.score = contester2.score + points2 < 0 ? 0 : contester2.score + points2
       contester1.save!
       contester2.save!
     end
   end
 
-  def hltv_record addr, pwd
+  def hltv_record(addr, pwd)
-    if (match_time - MATCH_LENGTH*10) > DateTime.now or (match_time + MATCH_LENGTH*10) < DateTime.now
+    if (match_time - MATCH_LENGTH * 10) > DateTime.now.utc ||
+       (match_time + MATCH_LENGTH * 10) < DateTime.now.utc
       raise Error, I18n.t(:hltv_request_20)
     end
-    if hltv and hltv.recording
+    if hltv && hltv.recording
       raise Error, I18n.t(:hltv_already) + hltv.addr
     end
     unless get_hltv
@@ -304,47 +306,55 @@ class Match < ActiveRecord::Base
     hltv.save!
   end
 
-  def hltv_move addr, pwd
+  def hltv_move(addr, pwd)
-    raise Error, I18n.t(:hltv_notset) if hltv.nil? or hltv.recording.nil?
+    raise Error, I18n.t(:hltv_notset) if hltv.nil? || hltv.recording.nil?
     Server.move hltv.reservation, addr, pwd
   end
 
   def hltv_stop
-    raise Error, I18n.t(:hltv_notset) if hltv.nil? or hltv.recording.nil?
+    raise Error, I18n.t(:hltv_notset) if hltv.nil? || hltv.recording.nil?
     Server.stop hltv.reservation
   end
 
-  def can_create? cuser
+  def can_create?(cuser)
-    cuser and cuser.admin?
+    cuser && cuser.admin?
   end
 
-  def can_update? cuser, params = {}
+  def can_update?(cuser, params = {})
     return false unless cuser
     return true if cuser.admin?
-    return true if cuser.caster? and Verification.contain params, [:caster_id] \
+
-      and (params[:caster_id] && params[:caster_id].to_i == cuser.id && caster_id.blank?) \
+    if cuser.ref?
-      or (params[:caster_id].blank? && caster_id == cuser.id)
+      if referee == cuser
-    return true if cuser.ref?  and Verification.contain params, [:referee_id] \
+        return true if Verification.contain(params,
-      and (params[:referee_id] && params[:referee_id].to_i == cuser.id  && referee_id.blank?) \
+                                            [:score1, :score2, :forfeit, :report, :demo_id,
-      or (params[:referee_id].blank? && referee_id == cuser.id)
+                                             :motm_name, :matchers_attributes, :server_id])
-    return true if cuser.ref? and referee == cuser \
+        return true if Verification.contain(params, [:hltv]) && !demo
-      and Verification.contain params, [:score1, :score2, :forfeit, :report, :demo_id, :motm_name, :matchers_attributes, :server_id]
+      end
-    return true if match_time.past? and !score1 and !score2 and !forfeit \
+      if Verification.contain(params, [:referee_id])
-      and Verification.contain(params, [:score1, :score2]) \
+        return true if (params[:referee_id].to_i == cuser.id && referee_id.blank?) ||
-      and (contester1.team.is_leader? cuser or contester2.team.is_leader? cuser)
+                       (params[:referee_id].blank? && referee_id == cuser.id)
-    return true if match_time.today? \
+      end
-      and Verification.contain(params, [:stream_id]) \
+    end
-      and (contester1.team.is_leader? cuser or contester2.team.is_leader? cuser)
+
-    return true if match_time.past? \
+    if contester1.team.is_leader?(cuser) || contester2.team.is_leader?(cuser)
-      and Verification.contain(params, [:matchers_attributes]) \
+      if match_time.past?
-      and (contester1.team.is_leader? cuser or contester2.team.is_leader? cuser)
+        return true if Verification.contain(params, [:score1, :score2]) &&
-    return true if (cuser.ref? and referee == cuser) \
+                       !score1 && !score2 && !forfeit
-      and Verification.contain(params, [:hltv]) \
+        return true if Verification.contain(params, [:matchers_attributes])
-      and !demo
+      end
+      return true if match_time.today? && Verification.contain(params, [:stream_id])
+    end
+
+    if cuser.caster? && Verification.contain(params, [:caster_id])
+      return true if (params[:caster_id].to_i == cuser.id && caster_id.blank?) ||
+                     (params[:caster_id].blank? && caster_id == cuser.id)
+    end
+
     false
   end
 
-  def can_destroy? cuser
+  def can_destroy?(cuser)
-    cuser and cuser.admin?
+    cuser && cuser.admin?
   end
 end

app/views/matches/admin.html.haml

@@ -34,7 +34,7 @@
           - elsif match.can_update?(cuser, {:caster_id => cuser.id})
             = form_for match do |f|
               - if cuser.admin?
-                = f.collection_select :caster_id, Group.casters, :id, :username, {:include_blank => true}, {:class => "autosubmit"}
+                = f.collection_select :caster_id, User.casters, :id, :username, {:include_blank => true}, {:class => "autosubmit"}
               - else
                 = f.hidden_field :caster_id, {:value => cuser.id}
                 = link_to "Take!", "#form_submit"

app/views/matches/score.html.erb

@@ -1,37 +0,0 @@
-<h2>
-  <%= namelink @match.contester1.team %> vs <%= namelink @match.contester2.team %>
-</h2>
-
-<div class="wide box">
-  <h3>
-    Lineup
-  </h3>
-  <%= form_for @match do |f| %>
-    <%= render :partial => "lineup", :locals => {:contester => @contester} %>
-    <p>
-      <%= f.submit "Save" %>
-    </p>
-  <% end %>
-</div>
-
-<% if @match.can_update? cuser, [:score1, :score2] %>
-  <div class="wide box">
-    <%= form_for(@match) do |f| %>
-      <h3>
-        Scoring
-      </h3>
-
-      <%= f.error_messages %>
-      <p>
-        <%= f.label :score1, "Score" %><br />
-        <%= f.text_field :score1, :size => 1 %>
-        <%= f.text_field :score2, :size => 1 %>
-      </p>
-      <p>
-        <%= f.submit 'Submit' %>
-      </p>
-    <% end %>
-  </div>
-<% end %>
-
-<%= link_to "Back", @match %>

lib/verification.rb

@@ -1,63 +1,61 @@
 module Verification
-	def Verification.verify input
+  def self.verify(input)
     md5 = Digest::MD5.hexdigest("9WvcZ9hX" + input + "KF7L4luQ").upcase.split(//)
-		chars = ["A", "B", "C", "D", "E", "F"]
+    chars = %w[A B C D E F]
     nums = []
-		lastPos = md5[31].to_i
+    last_pos = md5[31].to_i
     result = ""
 
-		for i in 0..9
+    (0..9).each do |i|
       pos = md5[i].to_i
 
       if pos == 0
-				pos = lastPos ** (i % 4)
+        pos = last_pos**(i % 4)
       elsif (pos % 4) == 0
-				pos = pos * lastPos + i
+        pos = pos * last_pos + i
       elsif (pos % 3) == 0
-				pos = pos ** (i % 4)
+        pos **= (i % 4)
-			elsif (pos % 2) == 0
+      elsif pos.even?
         pos = pos * i + pos
       end
 
       pos = (pos > 31) ? (pos % 32) : pos
-			curChar = md5[31 - pos]
+      cur_char = md5[31 - pos]
-			curNum = curChar.to_i
+      cur_num = cur_char.to_i
 
-			if nums.include? curNum
+      if nums.include? cur_num
-				if curNum == 0
+        if cur_num == 0
-					curChar = chars[pos % 6]
+          cur_char = chars[pos % 6]
         else
-					curChar = (pos % 10).to_s
+          cur_char = (pos % 10).to_s
         end
-				curNum = curChar.to_i
+        cur_num = cur_char.to_i
       end
 
-			nums << curNum
+      nums << cur_num
-			result << curChar
+      result << cur_char
-			lastPos = pos
+      last_pos = pos
     end
 
-		return result
+    result
   end
 
-	def Verification.uncrap str
+  def self.uncrap(str)
     str.to_s.gsub(/[^A-Za-z0-9_\-]/, "")
   end
 
-	def Verification.random_string len
+  def self.random_string(len)
     chars = ("a".."z").to_a + ("A".."Z").to_a + ("0".."9").to_a
     str = ""
-		1.upto(len) do |i|
+    1.upto(len) do
-			str << chars[rand(chars.size-1)]
+      str << chars[rand(chars.size - 1)]
     end
-		return str
+    str
   end
 
   # TODO: rikki?
-	def Verification.contain params, filter
+  # Returns true if params (or its keys) are a subset of filter
-		(params.instance_of?(Array) ? params : params.keys).each do |key|
+  def self.contain(params, filter)
-			return false unless filter.include? key.to_sym
+    ((params.instance_of?(Array) ? params : params.keys) - filter).empty?
-		end
-		return true
   end
 end

spec/controllers/api/v1/users_controller_spec.rb

@@ -78,9 +78,23 @@ describe Api::V1::UsersController do
     end
 
     it "returns 404 if user does not exist" do
-      expect {
+      expect { get :show, id: -1 }.to raise_error(ActionController::RoutingError)
-        get :show, id: -1
+    end
-      }.to raise_error(ActionController::RoutingError)
+
+    it "returns 404 if user does not exist by steamid" do
+      expect { get :show, id: -1, format: "steamid" }.to raise_error(ActionController::RoutingError)
+    end
+
+    it "queries the steam condenser for an invalid steamid" do
+      @user.update_attribute(:steamid, "0:0:0")
+
+      get :show, id: @user.id
+
+      expect(response).to be_success
+      expect(json["steam"]).to_not be_nil
+      expect(json["steam"]["id"]).to eq(@user.steamid)
+      expect(json["steam"]["url"]).to be_nil
+      expect(json["steam"]["nickname"]).to be_nil
     end
 
     it "returns correct ban if user muted" do