cafe-grader-web

Commit r295:4f81b9ab5d77 public

parent | child
Description:
a cleaner, testable way to log out user after contest changed
Commit status:
[Not Reviewed]
References:
default
Diff options:
Raw Diff | Patch Diff | Download Diff | Ignore whitespace | Increase context
Comments:
0 Commit comments 0 Inline Comments
Unresolved TODOs:
There are no unresolved TODOs
Side by Side Unified
Expand All Files Collapse All Files Wide Mode Diff
Add another comment
Browse Files

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

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