Merge pull request #73 from Prommah/develop

Fix Verification.contain bug & other cleanliness

app/controllers/matches_controller.rb

@@ -6,7 +6,7 @@ class MatchesController < ApplicationController
   end
 
   def show
-    @ownpred = @match.predictions.first conditions: {user_id: cuser.id} if cuser
+    @ownpred = @match.predictions.first conditions: { user_id: cuser.id } if cuser
     @newpred = @match.predictions.build
   end
 
@@ -17,8 +17,9 @@ class MatchesController < ApplicationController
   end
 
   def admin
-    @matches = Match.active.includes(:contest, :contester1, :contester2, :map1, :map2, :referee).all.group_by {|t| t.week.to_s }.to_a.reverse
-    render :layout => "full"
+    @matches = Match.active.includes(:contest, :contester1, :contester2, :map1, :map2, :referee)
+               .all.group_by { |t| t.week.to_s }.to_a.reverse
+    render layout: "full"
   end
 
   def extra
@@ -39,7 +40,7 @@ class MatchesController < ApplicationController
 
     if @match.save
       flash[:notice] = t(:matches_create)
-      redirect_to controller: 'contests', action: 'edit', id: @match.contest
+      redirect_to edit_contest_path(@match.contest)
     else
       render :new
     end
@@ -49,11 +50,11 @@ class MatchesController < ApplicationController
     raise AccessError unless @match.can_update? cuser, params[:match]
     if params[:match][:matchers_attributes]
       params[:match][:matchers_attributes].each do |key, matcher|
-        matcher['_destroy'] = matcher['_destroy'] == "keep" ? false : true
-        if matcher['user_id'] == ""
+        matcher["_destroy"] = matcher["_destroy"] == "keep" ? false : true
+        if matcher["user_id"] == ""
           params[:match][:matchers_attributes].delete key
-        elsif matcher['user_id'].to_i == 0
-          matcher['user_id'] = User.find_by_username(matcher['user_id']).id
+        elsif matcher["user_id"].to_i == 0
+          matcher["user_id"] = User.find_by_username(matcher["user_id"]).id
         end
       end
     end
@@ -70,10 +71,15 @@ class MatchesController < ApplicationController
           end
         end
       end
+    else
+      if URI(request.referer).path == match_ref_path(@match)
+        ref
+        render :ref
       else
         render :edit
       end
     end
+  end
 
   def hltv
     raise AccessError unless @match.can_update? cuser, [:hltv]
@@ -90,13 +96,13 @@ class MatchesController < ApplicationController
       @match.hltv_stop
       flash[:notice] = t(:hltv_stopped)
     end
-    redirect_to action: 'show'
+    redirect_to action: "show"
   end
 
   def destroy
     raise AccessError unless @match.can_destroy? cuser
     @match.destroy
-    redirect_to controller: 'contests', action: 'edit', id: @match.contest
+    redirect_to edit_contest_path(@match.contest)
   end
 
   private

app/models/match.rb

@@ -193,7 +193,7 @@ class Match < ActiveRecord::Base
   end
 
   def send_notifications
-    Profile.all(include: :user, conditions: "notify_any_match = 1").find_each do |p|
+    Profile.includes(:user).where(notify_any_match: 1).find_each do |p|
       Notifications.match p.user, self if p.user
     end
     contester2.team.teamers.active.each do |teamer|

app/views/matches/show.html.erb

@@ -117,9 +117,9 @@
     </div>
   <% end %>
 
-  <% if cuser and @match.can_update? cuser, referee_id: cuser.id %>
+  <% if cuser and @match.can_update? cuser, [:report] %>
     <div class="referee">
-      <%= link_to "Referee Admin", { controller: :matches, action: 'ref', id: @match }, class: 'button' %>
+      <%= link_to "Referee Admin", match_ref_path(@match), class: 'button' %>
     </div>
   <% end %>
 </div>

config/routes.rb

@@ -1,6 +1,6 @@
 Ensl::Application.routes.draw do
   %w(403 404 422 500).each do |code|
-    get code, to: 'errors#show', code: code
+    get code, to: "errors#show", code: code
   end
 
   namespace :api do
@@ -17,12 +17,12 @@ Ensl::Application.routes.draw do
     resources :versions
   end
 
-  match 'contests/del_map'
-  match 'contests/scores'
-  match 'contests/historical', to: "contests#historical"
+  match "contests/del_map"
+  match "contests/scores"
+  match "contests/historical", to: "contests#historical"
 
   resources :contests do
-    get 'current', on: :collection
+    get "current", on: :collection
   end
 
   resources :log_events
@@ -30,7 +30,7 @@ Ensl::Application.routes.draw do
   resources :options
   resources :polls
 
-  match 'comments/quote'
+  match "comments/quote"
 
   resources :comments
   resources :shoutmsgs
@@ -43,8 +43,8 @@ Ensl::Application.routes.draw do
   resources :forumers
   resources :topics
 
-  match 'forums/up'
-  match 'forums/down'
+  match "forums/up"
+  match "forums/down"
 
   resources :forums
   resources :users
@@ -55,9 +55,10 @@ Ensl::Application.routes.draw do
   resources :servers
   resources :predictions
   resources :rounds
-  resources :matches do |m|
+
+  get "matches/ref/:id" => "matches#ref", as: :match_ref
+  resources :matches do
     get :admin, to: "matches#admin", on: :collection
-    get :ref, to: "matches#ref"
   end
 
   resources :maps
@@ -74,65 +75,65 @@ Ensl::Application.routes.draw do
   resources :tweets
   resources :issues
 
-  match 'posts/quote'
+  match "posts/quote"
 
   resources :posts
   resources :brackets
 
-  match 'about/action'
-  match 'about/staff'
-  match 'about/statistics'
+  match "about/action"
+  match "about/staff"
+  match "about/statistics"
 
-  match 'refresh', to: "application#refresh"
-  match 'search', to: "application#search"
+  match "refresh", to: "application#refresh"
+  match "search", to: "application#search"
 
-  match 'news', to: "articles#news_index"
-  match 'news/archive', to: "articles#news_archive"
-  match 'news/admin', to: "articles#admin"
-  match 'articles/cleanup'
+  match "news", to: "articles#news_index"
+  match "news/archive", to: "articles#news_archive"
+  match "news/admin", to: "articles#admin"
+  match "articles/cleanup"
 
-  match 'data_files/admin'
-  match 'data_files/addFile'
-  match 'data_files/delFile'
-  match 'data_files/trash'
+  match "data_files/admin"
+  match "data_files/addFile"
+  match "data_files/delFile"
+  match "data_files/trash"
 
-  match 'contesters/recalc'
+  match "contesters/recalc"
 
-  match 'directories', to: "directories#show", id: 1
+  match "directories", to: "directories#show", id: 1
 
-  match 'gathers/refresh'
-  match 'gathers/latest/:game', to: "gathers#latest", via: :get
-  match 'gather', to: "gathers#latest", game: "ns2", via: :get
+  match "gathers/refresh"
+  match "gathers/latest/:game", to: "gathers#latest", via: :get
+  match "gather", to: "gathers#latest", game: "ns2", via: :get
 
-  match 'gatherers/:id/status', to: "gatherers#status", via: :post
+  match "gatherers/:id/status", to: "gatherers#status", via: :post
 
-  match 'groups/addUser'
-  match 'groups/delUser'
+  match "groups/addUser"
+  match "groups/delUser"
 
-  match 'movies/download'
-  match 'movies/preview'
-  match 'movies/snapshot'
+  match "movies/download"
+  match "movies/preview"
+  match "movies/snapshot"
 
-  match 'plugin/user'
+  match "plugin/user"
 
-  match 'users/forgot'
-  match 'users/recover'
-  match 'users/agenda'
-  match 'users/logout'
-  match 'users/login'
+  match "users/forgot"
+  match "users/recover"
+  match "users/agenda"
+  match "users/logout"
+  match "users/login"
 
-  match 'users/agenda'
-  match 'users/login'
-  match 'users/logout'
-  match 'users/popup'
-  match 'users/forgot', to: "users#forgot"
+  match "users/agenda"
+  match "users/login"
+  match "users/logout"
+  match "users/popup"
+  match "users/forgot", to: "users#forgot"
 
-  match 'votes/create'
+  match "votes/create"
 
-  match ':controller/:action', requirements: { action: /A-Za-z/ }
-  match ':controller/:action/:id'
-  match ':controller/:action/:id.:format'
-  match ':controller/:action/:id/:id2'
+  match ":controller/:action", requirements: { action: /A-Za-z/ }
+  match ":controller/:action/:id"
+  match ":controller/:action/:id.:format"
+  match ":controller/:action/:id/:id2"
 
-  match 'teamers/replace', to: 'teamers#replace', as: 'teamers_replace'
+  match "teamers/replace", to: "teamers#replace", as: "teamers_replace"
 end

lib/verification.rb

@@ -54,8 +54,9 @@ module Verification
   end
 
   # TODO: rikki?
-  # Returns true if params (or its keys) are a subset of filter
+  # Returns true if params (or its keys if a hash) are a subset of filter
   def self.contain(params, filter)
-    ((params.instance_of?(Array) ? params : params.keys) - filter).empty?
+    keys = (params.instance_of?(Array) ? params : params.keys).map(&:to_sym)
+    (keys - filter).empty?
   end
 end

spec/controllers/api/v1/maps_controller_spec.rb

@@ -1,19 +1,19 @@
-require 'spec_helper'
+require "spec_helper"
 
 describe Api::V1::MapsController do
   before do
-    request.accept = 'application/json'
+    request.accept = "application/json"
   end
 
-  describe '#index' do
+  describe "#index" do
     let!(:map) { create :map }
 
-    it 'returns a list of maps' do
+    it "returns a list of maps" do
       get :index
       expect(response).to be_success
-      expect(json['maps'].length).to eq(1)
-      json_map = json['maps'][0]
-      expect(json_map['id']).to eq(map.id)
+      expect(json["maps"].length).to eq(1)
+      json_map = json["maps"][0]
+      expect(json_map["id"]).to eq(map.id)
     end
   end
 end

spec/controllers/api/v1/servers_controller_spec.rb

@@ -1,20 +1,20 @@
-require 'spec_helper'
+require "spec_helper"
 
 describe Api::V1::ServersController do
   before do
-    request.accept = 'application/json'
+    request.accept = "application/json"
   end
 
-  describe '#index' do
+  describe "#index" do
     let!(:server) { create :server, :active }
     let!(:inactive_server) { create :server, :inactive }
 
-    it 'returns a list of servers' do
+    it "returns a list of servers" do
       get :index
       expect(response).to be_success
-      expect(json['servers'].length).to eq(1)
-      json_server = json['servers'][0]
-      expect(json_server['id']).to eq(server.id)
+      expect(json["servers"].length).to eq(1)
+      json_server = json["servers"][0]
+      expect(json_server["id"]).to eq(server.id)
     end
   end
 end

spec/controllers/plugin/plugin_controller_spec.rb

@@ -1,9 +1,9 @@
-require 'spec_helper'
+require "spec_helper"
 
 describe PluginController do
   render_views
 
-  describe '#user' do
+  describe "#user" do
     before do
       create :group, :donors
       create :group, :champions
@@ -11,14 +11,14 @@ describe PluginController do
 
     let!(:user) { create :user_with_team }
 
-    it 'returns user data' do
+    it "returns user data" do
       get :user, id: user.steamid
       expect(response).to be_success
       expect(response.body).to include(user.username)
     end
 
-    it 'definitely does not return IP address' do
-      last_ip = '127.0.0.1'
+    it "definitely does not return IP address" do
+      last_ip = "127.0.0.1"
       user.lastip = last_ip
       user.save!
       get :user, id: user.steamid

spec/factories/article.rb

@@ -1,6 +1,6 @@
 FactoryGirl.define do
   factory :article do
     sequence(:title) { |n| "Article #{n}" }
-    sequence(:text)  { |n| (0..100).map{ (0...8).map { (65 + rand(26)).chr }.join }.join(" ") }
+    sequence(:text)  { (0..100).map { (0...8).map { (65 + rand(26)).chr }.join }.join(" ") }
   end
 end

spec/factories/ban.rb

@@ -1,7 +1,7 @@
 FactoryGirl.define do
   factory :ban do
     ban_type Ban::TYPE_SITE
-    expiry Date.today + 1
+    expiry Time.now.utc.to_date + 1
     # Hack because of the awkward way bans are created (requires user_name)
     before(:create) do |ban|
       if ban.user.nil?

spec/factories/contest.rb

@@ -0,0 +1,10 @@
+FactoryGirl.define do
+  factory :contest do
+    sequence(:name) { |n| "Contest ##{n}" }
+
+    start Date.yesterday
+    self.end Date.tomorrow
+    status Contest::STATUS_PROGRESS
+    default_time "12:00:00"
+  end
+end

spec/factories/contester.rb

@@ -0,0 +1,8 @@
+FactoryGirl.define do
+  factory :contester do
+    contest
+    team do
+      create(:user_with_team).team
+    end
+  end
+end

spec/factories/group.rb

@@ -3,6 +3,8 @@ FactoryGirl.define do
     sequence(:id) { |n| n + 100 } # Preserve first 100
     sequence(:name) { |n| "Group#{n}" }
     association :founder, factory: :user
+
+    initialize_with { Group.find_or_create_by_id(id) }
   end
 
   trait :admin do
@@ -10,6 +12,11 @@ FactoryGirl.define do
     id Group::ADMINS
   end
 
+  trait :caster do
+    name "Shoutcasters"
+    id Group::CASTERS
+  end
+
   trait :champions do
     name "Champions"
     id Group::CHAMPIONS
@@ -24,4 +31,9 @@ FactoryGirl.define do
     name "Gather Moderator"
     id Group::GATHER_MODERATORS
   end
+
+  trait :ref do
+    name "Referees"
+    id Group::REFEREES
+  end
 end

spec/factories/match.rb

@@ -0,0 +1,13 @@
+FactoryGirl.define do
+  factory :match do
+    contest
+    contester1 do
+      create(:contester, contest: contest)
+    end
+    contester2 do
+      create(:contester, contest: contest)
+    end
+
+    match_time 1.hour.from_now
+  end
+end

spec/factories/user.rb

@@ -20,6 +20,20 @@ FactoryGirl.define do
       end
     end
 
+    trait :caster do
+      after(:create) do |user|
+        group = create(:group, :caster)
+        create :grouper, user: user, group: group
+      end
+    end
+
+    trait :ref do
+      after(:create) do |user|
+        group = create(:group, :ref)
+        create :grouper, user: user, group: group
+      end
+    end
+
     trait :chris do
       steamid "0:1:58097444"
     end

spec/features/shoutbox/shoutbox_spec.rb

@@ -1,33 +1,33 @@
-require 'spec_helper'
+require "spec_helper"
 
-feature 'Shoutbox', js: true do
+feature "Shoutbox", js: true do
   let!(:user) { create :user }
 
   background do
     sign_in_as user
   end
 
-	scenario 'creating a valid shout' do
+  scenario "creating a valid shout" do
     visit root_path
-		shout = rand(100000).to_s
-		fill_in 'shoutbox_text', with: shout
-		click_button 'Shout!'
+    shout = rand(100_000).to_s
+    fill_in "shoutbox_text", with: shout
+    click_button "Shout!"
     expect(page).to have_content(shout)
   end
 
-	scenario 'enter more than 100 characters' do
+  scenario "enter more than 100 characters" do
     valid_shout = 100.times.map { "a" }.join
     invalid_shout = 101.times.map { "a" }.join
     visit root_path
     expect(page).to_not have_content("Maximum shout length exceeded")
-		fill_in 'shoutbox_text', with: invalid_shout
+    fill_in "shoutbox_text", with: invalid_shout
     expect(page).to have_content("Maximum shout length exceeded")
-		fill_in 'shoutbox_text', with: valid_shout
+    fill_in "shoutbox_text", with: valid_shout
     expect(page).to_not have_content("Maximum shout length exceeded")
   end
 
-	scenario 'creating shout while banned' do
-		Ban.create! ban_type: Ban::TYPE_MUTE, expiry: Time.now + 10.days, user_name: user.username
+  scenario "creating shout while banned" do
+    Ban.create! ban_type: Ban::TYPE_MUTE, expiry: Time.now.utc + 10.days, user_name: user.username
     visit root_path
     expect(find("#sidebar")).to have_content "You have been muted."
   end

spec/features/users/case_insensitive_login_spec.rb

@@ -1,6 +1,6 @@
-require 'spec_helper'
+require "spec_helper"
 
-feature 'Case insensitive login', js: :true do
+feature "Case insensitive login", js: :true do
   let(:username) { "CaSe_InSeNsItIvE" }
   let(:password) { "passwordABC123" }
   let!(:user) { create(:user, username: username, raw_password: password) }
@@ -9,23 +9,23 @@ feature 'Case insensitive login', js: :true do
     visit root_path
   end
 
-  feature 'when a user with mixed-case username signs in' do
-    scenario 'with a matching case allows the user to sign in' do
+  feature "when a user with mixed-case username signs in" do
+    scenario "with a matching case allows the user to sign in" do
       fill_login_form(username)
       click_button submit(:user, :login)
 
-      expect(page).to have_content(I18n.t('login_successful'))
+      expect(page).to have_content(I18n.t("login_successful"))
 
       within user_status do
         expect(page).to have_content(account_link)
       end
     end
 
-    scenario 'with a non-matching case allows the user to sign in' do
+    scenario "with a non-matching case allows the user to sign in" do
       fill_login_form("CASE_INSENSITIVE")
       click_button submit(:user, :login)
 
-      expect(page).to have_content(I18n.t('login_successful'))
+      expect(page).to have_content(I18n.t("login_successful"))
 
       within user_status do
         expect(page).to have_content(account_link)
@@ -39,6 +39,6 @@ feature 'Case insensitive login', js: :true do
   end
 
   def account_link
-    'ACCOUNT'
+    "ACCOUNT"
   end
 end

spec/models/user_spec.rb

@@ -22,7 +22,7 @@
 #  salt         :string(255)
 #
 
-require 'spec_helper'
+require "spec_helper"
 
 describe User do
   let!(:user) { create :user }
@@ -34,7 +34,7 @@ describe User do
 
     it "returns true if user is banned" do
       Ban.create!(ban_type: Ban::TYPE_SITE,
-                  expiry: Time.now + 10.days,
+                  expiry: Time.now.utc + 10.days,
                   user_name: user.username)
 
       expect(user.banned?).to be_truthy
@@ -42,7 +42,7 @@ describe User do
 
     it "returns true for specific bans" do
       Ban.create!(ban_type: Ban::TYPE_MUTE,
-                  expiry: Time.now + 10.days,
+                  expiry: Time.now.utc + 10.days,
                   user_name: user.username)
 
       expect(user.banned? Ban::TYPE_MUTE).to be_truthy

spec/support/features/form_helpers.rb

@@ -2,7 +2,7 @@ module Features
   module FormHelpers
     def fill_form(model, hash)
       hash.each do |attribute, value|
-        fill_in attribute_translation(model, attribute), :with => value
+        fill_in attribute_translation(model, attribute), with: value
       end
     end
 

spec/support/features/server_helpers.rb

@@ -1,24 +1,24 @@
 module Features
   module ServerHelpers
     def test_server_creation_and_editing
-      dns = 'ServerDns.com'
-      ip = '192.168.1.1'
-      port = '8000'
-      password = 'secret'
-      name = 'MyNsServer'
-      description = 'My NS Server'
-      irc = '#some_channel'
+      dns = "ServerDns.com"
+      ip = "192.168.1.1"
+      port = "8000"
+      password = "secret"
+      name = "MyNsServer"
+      description = "My NS Server"
+      irc = "#some_channel"
 
       visit new_server_path
-      fill_in 'Dns', with: dns
-      fill_in 'server_ip', with: ip
-      fill_in 'server_port', with: port
-      fill_in 'Password', with: password
-      fill_in 'Name', with: name
-      fill_in 'Description', with: description
-      fill_in 'Irc', with: irc
-      check 'Available for officials?'
-      click_button 'Save'
+      fill_in "Dns", with: dns
+      fill_in "server_ip", with: ip
+      fill_in "server_port", with: port
+      fill_in "Password", with: password
+      fill_in "Name", with: name
+      fill_in "Description", with: description
+      fill_in "Irc", with: irc
+      check "Available for officials?"
+      click_button "Save"
 
       expect(page).to have_content(dns)
       expect(page).to have_content("#{ip}:#{port}")
@@ -26,17 +26,17 @@ module Features
       expect(page).to have_content(irc)
       expect(page).to have_content(description)
 
-      click_link 'Edit Server'
+      click_link "Edit Server"
 
-      fill_in 'Dns', with: "#{dns}2"
-      fill_in 'server_ip', with: "192.168.1.2"
-      fill_in 'server_port', with: "8001"
-      fill_in 'Password', with: "#{password}2"
-      fill_in 'Name', with: "#{name}2"
-      fill_in 'Description', with: "#{description}2"
-      fill_in 'Irc', with: "#{irc}2"
-      check 'Available for officials?'
-      click_button 'Save'
+      fill_in "Dns", with: "#{dns}2"
+      fill_in "server_ip", with: "192.168.1.2"
+      fill_in "server_port", with: "8001"
+      fill_in "Password", with: "#{password}2"
+      fill_in "Name", with: "#{name}2"
+      fill_in "Description", with: "#{description}2"
+      fill_in "Irc", with: "#{irc}2"
+      check "Available for officials?"
+      click_button "Save"
 
       expect(page).to have_content("192.168.1.2:8001")
       expect(page).to have_content("#{dns}2")

spec/support/features/session_helpers.rb

@@ -6,16 +6,16 @@ module Features
       fill_in "login_username", with: user.username
       fill_in "login_password", with: user.raw_password
 
-      click_button I18n.t('helpers.submit.user.login')
+      click_button I18n.t("helpers.submit.user.login")
     end
 
     def change_timezone_for(user, timezone)
       visit edit_user_path(user.id)
 
-      click_link I18n.t('profile.locals')
+      click_link I18n.t("profile.locals")
       find("option[value='#{timezone}']").select_option
 
-      click_button I18n.t('helpers.submit.user.update')
+      click_button I18n.t("helpers.submit.user.update")
     end
 
     def user_status