Description:
a cleaner, testable way to log out user after contest changed
Commit status:
[Not Reviewed]
References:
Comments:
0 Commit comments 0 Inline Comments
Unresolved TODOs:
There are no unresolved TODOs
Add another comment

r295:4f81b9ab5d77 - - 11 files changed: 170 inserted, 18 deleted

@@ -0,0 +1,9
1 + class AddForcedLogoutToUserContestStat < ActiveRecord::Migration
2 + def self.up
3 + add_column :user_contest_stats, :forced_logout, :boolean
4 + end
5 +
6 + def self.down
7 + remove_column :user_contest_stats, :forced_logout, :boolean
8 + end
9 + end
@@ -0,0 +1,73
1 + require 'delorean'
2 +
3 + require File.dirname(__FILE__) + '/../spec_helper'
4 + require File.dirname(__FILE__) + '/../config_spec_helper'
5 +
6 + describe UserAdminController, "when manage contest" do
7 +
8 + include ConfigSpecHelperMethods
9 +
10 + fixtures :users
11 + fixtures :problems
12 + fixtures :contests
13 + fixtures :roles
14 +
15 + def change_users_contest_to(user_login_list, contest, reset_timer=false)
16 + post_data = {
17 + :contest => {:id => contest.id},
18 + :operation => 'assign',
19 + :login_list => user_login_list
20 + }
21 + post_data[:reset_timer] = true if reset_timer
22 + post 'manage_contest', post_data, {:user_id => @admin_user.id}
23 + end
24 +
25 + before(:each) do
26 + @admin_user = users(:mary)
27 + @contest_b = contests(:contest_b)
28 + @james = users(:james)
29 + @jack = users(:jack)
30 +
31 + set_contest_time_limit('3:00')
32 + set_indv_contest_mode
33 + end
34 +
35 + it "should allow admin to see contest management page" do
36 + get 'contest_management', {}, {:user_id => @admin_user.id}
37 +
38 + response.should render_template 'user_admin/contest_management'
39 + end
40 +
41 + it "should change users' contest" do
42 + change_users_contest_to("james\njack", @contest_b)
43 + response.should redirect_to :action => 'contest_management'
44 +
45 + @james.contests(true).should include @contest_b
46 + @jack.contests(true).should_not include @contest_a
47 + end
48 +
49 + it "should reset users' timer when their contests change" do
50 + @james.update_start_time
51 +
52 + Delorean.time_travel_to(190.minutes.since) do
53 + @james.contest_finished?.should be_true
54 +
55 + change_users_contest_to("james", @contest_b, true)
56 +
57 + @james.contest_finished?.should be_false
58 + end
59 + end
60 +
61 + it "should set forced_logout flag for users when their contests change" do
62 + @james.update_start_time
63 +
64 + Delorean.time_travel_to(190.minutes.since) do
65 + @james.contest_finished?.should be_true
66 +
67 + change_users_contest_to("james", @contest_b, true)
68 +
69 + @james.contest_stat(true).forced_logout.should be_true
70 + end
71 + end
72 +
73 + end
@@ -26,22 +26,32
26 def authenticate
26 def authenticate
27 unless session[:user_id]
27 unless session[:user_id]
28 redirect_to :controller => 'main', :action => 'login'
28 redirect_to :controller => 'main', :action => 'login'
29 return false
29 return false
30 end
30 end
31
31
32 - #Configuration.reload
33 # check if run in single user mode
32 # check if run in single user mode
34 - if (Configuration[SINGLE_USER_MODE_CONF_KEY])
33 + if Configuration[SINGLE_USER_MODE_CONF_KEY]
35 user = User.find(session[:user_id])
34 user = User.find(session[:user_id])
36 if user==nil or (not user.admin?)
35 if user==nil or (not user.admin?)
37 redirect_to :controller => 'main', :action => 'login'
36 redirect_to :controller => 'main', :action => 'login'
38 return false
37 return false
39 end
38 end
39 + return true
40 end
40 end
41
41
42 + if Configuration.multicontests?
43 + user = User.find(session[:user_id])
44 + begin
45 + if user.contest_stat(true).forced_logout
46 + flash[:notice] = 'You have been automatically logged out.'
47 + redirect_to :controller => 'main', :action => 'index'
48 + end
49 + rescue
50 + end
51 + end
42 return true
52 return true
43 end
53 end
44
54
45 def authorization
55 def authorization
46 return false unless authenticate
56 return false unless authenticate
47 user = User.find(session[:user_id])
57 user = User.find(session[:user_id])
@@ -7,12 +7,24
7 end
7 end
8
8
9 def login
9 def login
10 if user = User.authenticate(params[:login], params[:password])
10 if user = User.authenticate(params[:login], params[:password])
11 session[:user_id] = user.id
11 session[:user_id] = user.id
12 session[:admin] = user.admin?
12 session[:admin] = user.admin?
13 +
14 + # clear forced logout flag for multicontests contest change
15 + if Configuration.multicontests?
16 + contest_stat = user.contest_stat
17 + if contest_stat.respond_to? :forced_logout
18 + if contest_stat.forced_logout
19 + contest_stat.forced_logout = false
20 + contest_stat.save
21 + end
22 + end
23 + end
24 +
13 redirect_to :controller => 'main', :action => 'list'
25 redirect_to :controller => 'main', :action => 'list'
14 else
26 else
15 flash[:notice] = 'Wrong password'
27 flash[:notice] = 'Wrong password'
16 redirect_to :controller => 'main', :action => 'login'
28 redirect_to :controller => 'main', :action => 'login'
17 end
29 end
18 end
30 end
@@ -340,10 +340,17
340 end
340 end
341
341
342 def reject_announcement_refresh_when_logged_out
342 def reject_announcement_refresh_when_logged_out
343 if not session[:user_id]
343 if not session[:user_id]
344 render :text => 'Access forbidden', :status => 403
344 render :text => 'Access forbidden', :status => 403
345 end
345 end
346 +
347 + if Configuration.multicontests?
348 + user = User.find(session[:user_id])
349 + if user.contest_stat.forced_logout
350 + render :text => 'Access forbidden', :status => 403
351 + end
352 + end
346 end
353 end
347
354
348 end
355 end
349
356
@@ -193,13 +193,13
193 if !lines or lines.blank?
193 if !lines or lines.blank?
194 flash[:notice] = 'You entered an empty list.'
194 flash[:notice] = 'You entered an empty list.'
195 redirect_to :action => 'contest_management' and return
195 redirect_to :action => 'contest_management' and return
196 end
196 end
197
197
198 note = []
198 note = []
199 - user_ids = {}
199 + users = []
200 lines.split("\n").each do |line|
200 lines.split("\n").each do |line|
201 user = User.find_by_login(line.chomp)
201 user = User.find_by_login(line.chomp)
202 if user
202 if user
203 if operation=='add'
203 if operation=='add'
204 if ! user.contests.include? contest
204 if ! user.contests.include? contest
205 user.contests << contest
205 user.contests << contest
@@ -207,21 +207,24
207 elsif operation=='remove'
207 elsif operation=='remove'
208 user.contests.delete(contest)
208 user.contests.delete(contest)
209 else
209 else
210 user.contests = [contest]
210 user.contests = [contest]
211 end
211 end
212
212
213 - user.contest_stat.destroy if params[:reset_timer]
213 + if params[:reset_timer]
214 + user.contest_stat.forced_logout = true
215 + user.contest_stat.reset_timer_and_save
216 + end
214
217
215 note << user.login
218 note << user.login
216 - user_ids[user.id] = true
219 + users << user
217 end
220 end
218 end
221 end
219
222
220 if params[:reset_timer]
223 if params[:reset_timer]
221 - logout_users(user_ids)
224 + logout_users(users)
222 end
225 end
223
226
224 flash[:notice] = 'User(s) ' + note.join(', ') +
227 flash[:notice] = 'User(s) ' + note.join(', ') +
225 ' were successfully modified. '
228 ' were successfully modified. '
226 redirect_to :action => 'contest_management'
229 redirect_to :action => 'contest_management'
227 end
230 end
@@ -328,16 +331,17
328 u.errors.each { |attr,msg| @import_log << "#{attr} - #{msg}\n" }
331 u.errors.each { |attr,msg| @import_log << "#{attr} - #{msg}\n" }
329 end
332 end
330 end
333 end
331
334
332 end
335 end
333
336
334 - def logout_users(user_ids)
337 + def logout_users(users)
335 - sessions = ActiveRecord::SessionStore::Session.find(:all, :conditions => ["updated_at >= ?", 60.minutes.ago])
338 + users.each do |user|
336 - sessions.each do |session|
339 + contest_stat = user.contest_stat(true)
337 - if user_ids.has_key? session.data[:user_id]
340 + if contest_stat and !contest_stat.forced_logout
338 - session.destroy
341 + contest_stat.forced_logout = true
342 + contest_stat.save
339 end
343 end
340 end
344 end
341 end
345 end
342
346
343 end
347 end
@@ -130,13 +130,13
130 return site.time_left
130 return site.time_left
131 elsif Configuration.indv_contest_mode?
131 elsif Configuration.indv_contest_mode?
132 time_limit = Configuration.contest_time_limit
132 time_limit = Configuration.contest_time_limit
133 if time_limit == nil
133 if time_limit == nil
134 return nil
134 return nil
135 end
135 end
136 - if contest_stat==nil
136 + if contest_stat==nil or contest_stat.started_at==nil
137 return (Time.now.gmtime + time_limit) - Time.now.gmtime
137 return (Time.now.gmtime + time_limit) - Time.now.gmtime
138 else
138 else
139 finish_time = contest_stat.started_at + time_limit
139 finish_time = contest_stat.started_at + time_limit
140 current_time = Time.now.gmtime
140 current_time = Time.now.gmtime
141 if current_time > finish_time
141 if current_time > finish_time
142 return 0
142 return 0
@@ -169,15 +169,15
169 return true
169 return true
170 end
170 end
171 end
171 end
172
172
173 def update_start_time
173 def update_start_time
174 stat = self.contest_stat
174 stat = self.contest_stat
175 - if stat == nil
175 + if stat == nil or stat.started_at == nil
176 - stat = UserContestStat.new(:user => self,
176 + stat ||= UserContestStat.new(:user => self)
177 - :started_at => Time.now.gmtime)
177 + stat.started_at = Time.now.gmtime
178 stat.save
178 stat.save
179 end
179 end
180 end
180 end
181
181
182 def problem_in_user_contests?(problem)
182 def problem_in_user_contests?(problem)
183 problem_contests = problem.contests.all
183 problem_contests = problem.contests.all
@@ -1,5 +1,10
1 class UserContestStat < ActiveRecord::Base
1 class UserContestStat < ActiveRecord::Base
2
2
3 belongs_to :user
3 belongs_to :user
4
4
5 + def reset_timer_and_save
6 + self.started_at = nil
7 + save
5 end
8 end
9 +
10 + end
@@ -6,13 +6,13
6 # to create the application database on another system, you should be using db:schema:load, not running
6 # to create the application database on another system, you should be using db:schema:load, not running
7 # all the migrations from scratch. The latter is a flawed and unsustainable approach (the more migrations
7 # all the migrations from scratch. The latter is a flawed and unsustainable approach (the more migrations
8 # you'll amass, the slower it'll run and the greater likelihood for issues).
8 # you'll amass, the slower it'll run and the greater likelihood for issues).
9 #
9 #
10 # It's strongly recommended to check this file into your version control system.
10 # It's strongly recommended to check this file into your version control system.
11
11
12 - ActiveRecord::Schema.define(:version => 20100303095700) do
12 + ActiveRecord::Schema.define(:version => 20100328123325) do
13
13
14 create_table "announcements", :force => true do |t|
14 create_table "announcements", :force => true do |t|
15 t.string "author"
15 t.string "author"
16 t.text "body"
16 t.text "body"
17 t.boolean "published"
17 t.boolean "published"
18 t.datetime "created_at"
18 t.datetime "created_at"
@@ -207,12 +207,13
207
207
208 create_table "user_contest_stats", :force => true do |t|
208 create_table "user_contest_stats", :force => true do |t|
209 t.integer "user_id"
209 t.integer "user_id"
210 t.datetime "started_at"
210 t.datetime "started_at"
211 t.datetime "created_at"
211 t.datetime "created_at"
212 t.datetime "updated_at"
212 t.datetime "updated_at"
213 + t.boolean "forced_logout"
213 end
214 end
214
215
215 create_table "users", :force => true do |t|
216 create_table "users", :force => true do |t|
216 t.string "login", :limit => 50
217 t.string "login", :limit => 50
217 t.string "full_name"
218 t.string "full_name"
218 t.string "hashed_password"
219 t.string "hashed_password"
@@ -17,22 +17,23
17 refreshAnnouncement: function() {
17 refreshAnnouncement: function() {
18 var url = Announcement.refreshUrl;
18 var url = Announcement.refreshUrl;
19 new Ajax.Request(url, {
19 new Ajax.Request(url, {
20 method: 'get',
20 method: 'get',
21 parameters: { recent: Announcement.mostRecentId },
21 parameters: { recent: Announcement.mostRecentId },
22 onSuccess: function(transport) {
22 onSuccess: function(transport) {
23 - if(transport.responseText.match(/\S/)!=null) {
23 + if((transport.status == 200) &&
24 + (transport.responseText.match(/\S/)!=null)) {
24 var announcementBody = $("announcementbox-body");
25 var announcementBody = $("announcementbox-body");
25 announcementBody.insert({ top: transport.responseText });
26 announcementBody.insert({ top: transport.responseText });
26 var announcementBoxes = $$(".announcementbox");
27 var announcementBoxes = $$(".announcementbox");
27 if(announcementBoxes.length!=0)
28 if(announcementBoxes.length!=0)
28 announcementBoxes[0].show();
29 announcementBoxes[0].show();
30 + Announcement.registerRefreshEventTimer();
29 }
31 }
30 }
32 }
31 });
33 });
32 - Announcement.registerRefreshEventTimer();
33 },
34 },
34
35
35 registerRefreshEventTimer: function() {
36 registerRefreshEventTimer: function() {
36 setTimeout(function () {
37 setTimeout(function () {
37 Announcement.refreshAnnouncement();
38 Announcement.refreshAnnouncement();
38 }, 30000);
39 }, 30000);
@@ -15,12 +15,13
15 @contest_b = contests(:contest_b)
15 @contest_b = contests(:contest_b)
16 @james = users(:james)
16 @james = users(:james)
17 @jack = users(:jack)
17 @jack = users(:jack)
18
18
19 set_contest_time_limit('3:00')
19 set_contest_time_limit('3:00')
20 set_indv_contest_mode
20 set_indv_contest_mode
21 + enable_multicontest
21 end
22 end
22
23
23 it "should reset users' timer when their contests change" do
24 it "should reset users' timer when their contests change" do
24 james_session = open_session
25 james_session = open_session
25 james_session.extend(MainSessionMethods)
26 james_session.extend(MainSessionMethods)
26
27
@@ -39,24 +40,53
39
40
40 james_login_and_get_main_list(james_session)
41 james_login_and_get_main_list(james_session)
41 james_session.response.should_not have_text(/OVER/)
42 james_session.response.should_not have_text(/OVER/)
42 end
43 end
43 end
44 end
44
45
46 + it "should force users to log out when their contests change" do
47 + james_session = open_session
48 + james_session.extend(MainSessionMethods)
49 +
50 + james_login_and_get_main_list(james_session)
51 + james_session.response.should_not have_text(/OVER/)
52 +
53 + Delorean.time_travel_to(190.minutes.since) do
54 + james_session.get_main_list
55 + james_session.response.should have_text(/OVER/)
56 +
57 + admin_change_users_contest_to("james", @contest_b, true)
58 +
59 + james_session.get '/main/list'
60 + james_session.response.should_not render_template 'main/list'
61 + james_session.should be_redirect
62 +
63 + Delorean.time_travel_to(200.minutes.since) do
64 + james_login_and_get_main_list(james_session)
65 + james_session.response.should_not have_text(/OVER/)
66 + end
67 + end
68 + end
69 +
45 private
70 private
46
71
47 module MainSessionMethods
72 module MainSessionMethods
48 def login(login_name, password)
73 def login(login_name, password)
49 post '/login/login', :login => login_name, :password => password
74 post '/login/login', :login => login_name, :password => password
50 assert_redirected_to '/main/list'
75 assert_redirected_to '/main/list'
51 end
76 end
52
77
53 def get_main_list
78 def get_main_list
54 get '/main/list'
79 get '/main/list'
55 assert_template 'main/list'
80 assert_template 'main/list'
56 end
81 end
82 +
83 + def get_main_list_and_assert_logout
84 + get '/main/list'
85 + assert_redirected_to '/main'
86 + end
57 end
87 end
58
88
59 module ContestManagementSessionMethods
89 module ContestManagementSessionMethods
60 def change_users_contest_to(user_login_list, contest, reset_timer=false)
90 def change_users_contest_to(user_login_list, contest, reset_timer=false)
61 post_data = {
91 post_data = {
62 :contest => {:id => contest.id},
92 :contest => {:id => contest.id},
You need to be logged in to leave comments. Login now