Skip to content

Commit 05fa43a

Browse files
committed
Merge branch 'survey-session-revamp'
2 parents e089da1 + d698058 commit 05fa43a

36 files changed

+797
-63
lines changed

.travis.yml

-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
# please build
22
language: ruby
33
rvm:
4-
- 1.8.7
54
- 1.9.3
65
services:
76
- redis-server

Gemfile.lock

+9-11
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ GEM
1212
activeresource (2.3.18)
1313
activesupport (= 2.3.18)
1414
activesupport (2.3.18)
15-
addressable (2.3.2)
1615
airbrake (3.1.12)
1716
activesupport
1817
builder
@@ -36,8 +35,8 @@ GEM
3635
rack-test (>= 0.5.4)
3736
selenium-webdriver (~> 2.0)
3837
xpath (~> 0.1.4)
39-
childprocess (0.3.5)
40-
ffi (~> 1.0, >= 1.0.6)
38+
childprocess (0.5.1)
39+
ffi (~> 1.0, >= 1.0.11)
4140
chunky_png (1.2.6)
4241
cocaine (0.3.2)
4342
compass (0.12.2)
@@ -80,7 +79,7 @@ GEM
8079
fastercsv (1.5.1)
8180
faye-websocket (0.4.6)
8281
eventmachine (>= 0.12.0)
83-
ffi (1.1.5)
82+
ffi (1.9.3)
8483
formtastic (0.2.5)
8584
friendly_id (2.2.5)
8685
activerecord (>= 2.2.3)
@@ -107,8 +106,6 @@ GEM
107106
rake
108107
json (1.7.7)
109108
json_pure (1.7.5)
110-
libwebsocket (0.1.5)
111-
addressable
112109
libxml-ruby (2.2.2)
113110
mime-types (1.16)
114111
multi_json (1.0.4)
@@ -152,15 +149,15 @@ GEM
152149
rspec-rails (1.3.4)
153150
rack (>= 1.0.0)
154151
rspec (~> 1.3.1)
155-
rubyzip (0.9.9)
152+
rubyzip (1.1.0)
156153
sass (3.2.1)
157154
sax-machine (0.0.14)
158155
nokogiri (> 0.0.0)
159-
selenium-webdriver (2.25.0)
160-
childprocess (>= 0.2.5)
161-
libwebsocket (~> 0.1.3)
156+
selenium-webdriver (2.40.0)
157+
childprocess (>= 0.5.0)
162158
multi_json (~> 1.0)
163-
rubyzip
159+
rubyzip (~> 1.0)
160+
websocket (~> 1.0.4)
164161
sendgrid (0.1.4)
165162
shoulda (2.10.3)
166163
sqlite3 (1.3.5)
@@ -172,6 +169,7 @@ GEM
172169
thoughtbot-clearance (0.8.2)
173170
timecop (0.3.5)
174171
uuidtools (2.1.4)
172+
websocket (1.0.7)
175173
will_paginate (2.3.14)
176174
xml-simple (1.0.12)
177175
xpath (0.1.4)

app/controllers/application_controller.rb

+79-26
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,8 @@ class ApplicationController < ActionController::Base
44
helper :all
55
protect_from_forgery
66

7-
before_filter :initialize_session, :set_session_timestamp, :record_action, :view_filter, :set_pairwise_credentials, :set_locale, :set_p3p_header
7+
before_filter :initialize_session, :get_survey_session, :record_action, :view_filter, :set_pairwise_credentials, :set_locale, :set_p3p_header
8+
after_filter :write_survey_session_cookie
89

910
# preprocess photocracy_view_path on boot because
1011
# doing pathset generation during a request is very costly.
@@ -30,13 +31,6 @@ def set_pairwise_credentials
3031

3132
def initialize_session
3233
session[:session_id] # this forces load of the session in Rails 2.3.x
33-
if signed_in?
34-
logger.info "current user is #{current_user.inspect}"
35-
end
36-
37-
if white_label_request?
38-
logger.info "white_label request - no header and footer displayed"
39-
end
4034
end
4135

4236
helper_method :white_label_request?
@@ -60,19 +54,85 @@ def show_aoi_nav?
6054
# called when the request is not verified via the authenticity_token
6155
def handle_unverified_request
6256
super
63-
raise(ActionController::InvalidAuthenticityToken)
57+
# Appearance_lookup can act like an authenticity token because
58+
# get_survey_session will raise an error if no cookie found with proper appearance_lookup
59+
raise(ActionController::InvalidAuthenticityToken) unless params[:appearance_lookup]
6460
end
6561

66-
def set_session_timestamp
67-
# ActiveResource::HttpMock only matches static strings for query parameters
68-
# when in test set this to a static value, so we can match the resulting API queries for mocking
69-
request.session_options[:id] = "test123" if Rails.env == "test"
70-
expiration_time = session[:expiration_time]
71-
if expiration_time && expiration_time < Time.now || session[:session_id].nil?
72-
session[:session_id] = ActiveSupport::SecureRandom.hex(16)
73-
request.session_options[:id] = session[:session_id]
62+
# This method sets question_id based on the URL and parameters of the request
63+
# We want to know the question_id early in the request process because we
64+
# use it to determine the proper session for this request.
65+
#
66+
# The different controller / actions have differing ways of determining the
67+
# question_id. Some are only passed the Earl.name, while others get the
68+
# question_id directly as a parameter.
69+
#
70+
# Some requests like the homepage will have @question_id = nil. This is
71+
# okay as they don't pass any session information to pairwise. A separate
72+
# session is kept for requests that have no question_id.
73+
def set_question_id_earl
74+
@question_id = nil
75+
if [controller_name, action_name] == ['earls', 'show']
76+
@earl = Earl.find_by_name(params[:id])
77+
@question_id = @earl.try(:question_id)
78+
elsif controller_name == 'prompts'
79+
@question_id = params[:question_id]
80+
elsif controller_name == 'questions'
81+
if ['add_idea', 'visitor_voting_history'].include?(action_name)
82+
@question_id = params[:id]
83+
elsif ['results', 'about', 'admin', 'update_name'].include?(action_name)
84+
@earl = Earl.find_by_name(params[:id])
85+
@question_id = @earl.try(:question_id)
86+
end
87+
elsif controller_name == 'choices'
88+
if action_name == 'toggle'
89+
@earl = Earl.find(params[:earl_id])
90+
else
91+
@earl = Earl.find_by_name(params[:question_id])
92+
end
93+
@question_id = @earl.try(:question_id)
7494
end
75-
session[:expiration_time] = 10.minutes.from_now
95+
end
96+
97+
# Called as a before_filter.
98+
def get_survey_session
99+
# First order of business is to set the question_id.
100+
set_question_id_earl
101+
102+
begin
103+
# Based on the cookies, question_id, and appearance_lookup, find the
104+
# proper session for this request.
105+
session_data = SurveySession.find(cookies, @question_id, params[:appearance_lookup])
106+
rescue CantFindSessionFromCookies => e
107+
# if no appearance_lookup, then we can safely create a new sesssion
108+
# otherwise this request ought to fail as they are attempting some action
109+
# without the proper session being found
110+
if params[:appearance_lookup].nil?
111+
session_data = [{:question_id => @question_id }]
112+
else
113+
raise e
114+
end
115+
end
116+
# Create new SurveySession object for this request.
117+
@survey_session = SurveySession.send(:new, *session_data)
118+
if @survey_session.expired?
119+
# This will regenerate the session_id, saving the old one.
120+
# We can send along both the new and old session_id to pairwise
121+
# for requests that have sessions that have expired.
122+
@survey_session.regenerate
123+
end
124+
125+
# We want the session to expire after X minutes of inactivity, so update
126+
# the expiry with every request.
127+
@survey_session.update_expiry
128+
end
129+
130+
# Called as a after_filter to ensure we pass along the updated survey session
131+
# cookie in the response to this request.
132+
def write_survey_session_cookie
133+
cookies[@survey_session.cookie_name] = {
134+
:value => @survey_session.cookie_value
135+
}
76136
end
77137

78138
def record_action
@@ -88,7 +148,7 @@ def record_action
88148
end
89149

90150
visitor = Visitor.find_or_create_by_remember_token(:remember_token => visitor_remember_token)
91-
user_session = SessionInfo.find_or_create_by_session_id(:session_id => request.session_options[:id],
151+
user_session = SessionInfo.find_or_create_by_session_id(:session_id => @survey_session.session_id,
92152
:ip_addr => request.remote_ip,
93153
:user_agent => request.env["HTTP_USER_AGENT"],
94154
:white_label_request => white_label_request?,
@@ -103,13 +163,6 @@ def record_action
103163
user_session.save!
104164
end
105165

106-
if current_user
107-
logger.info "CLICKSTREAM: #{controller_name}##{action_name} by Session #{request.session_options[:id]} (User: #{current_user.email})"
108-
else
109-
logger.info "CLICKSTREAM: #{controller_name}##{action_name} by Session #{request.session_options[:id]} (not logged in)"
110-
end
111-
# Click.create( :sid => request.session_options[:id], :ip_addr => request.remote_ip, :url => request.url,
112-
# :controller => controller_name, :action => action_name, :user => nil, :referrer => request.referrer)
113166
if (session[:abingo_identity])
114167
Abingo.identity = session[:abingo_identity]
115168
else

app/controllers/earls_controller.rb

+2-2
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ def show
2525
show_params = {:with_prompt => true,
2626
:with_appearance => true,
2727
:with_visitor_stats => true,
28-
:visitor_identifier => request.session_options[:id]}
28+
:visitor_identifier => @survey_session.session_id}
2929

3030
show_params.merge!({:future_prompts => {:number => 1}, :with_average_votes => true}) if @photocracy
3131

@@ -38,6 +38,7 @@ def show
3838
end
3939

4040

41+
@survey_session.appearance_lookup = @question.attributes["appearance_id"]
4142
logger.info "inside questions#show " + @question.inspect
4243

4344
# we can probably make this into one api call
@@ -218,7 +219,6 @@ def validate_hex_color(color)
218219
end
219220

220221
def dumb_cleartext_authentication
221-
@earl = Earl.find_by_name(params[:id])
222222
redirect_to('/') and return unless @earl
223223
unless @earl.pass.blank?
224224
authenticate_or_request_with_http_basic(t('questions.owner_password_exp')) do |user_name, password|

app/controllers/prompts_controller.rb

+9-3
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ def vote
3030
:appearance_lookup => next_prompt['appearance_id'],
3131
:prompt_id => next_prompt['id'],
3232
}
33+
@survey_session.appearance_lookup = result[:appearance_lookup]
3334

3435
if wikipedia?
3536
# wikipedia ideas are prepended by a 4 character integer
@@ -71,6 +72,7 @@ def skip
7172
:right_choice_url => question_choice_path(@earl.name, next_prompt['right_choice_id']),
7273
:message => t('vote.cant_decide_message')
7374
}
75+
@survey_session.appearance_lookup = result[:appearance_lookup]
7476

7577
if wikipedia?
7678
# wikipedia ideas are prepended by a 4 character integer
@@ -103,7 +105,7 @@ def flag
103105
@choice.prefix_options[:question_id] = question_id
104106

105107
c = @choice.put(:flag,
106-
:visitor_identifier => request.session_options[:id],
108+
:visitor_identifier => @survey_session.session_id,
107109
:explanation => reason)
108110

109111
new_choice = Crack::XML.parse(c.body)['choice']
@@ -134,6 +136,7 @@ def flag
134136
:prompt_id => next_prompt['id'],
135137
:message => t('vote.flag_complete_message')
136138
}
139+
@survey_session.appearance_lookup = result[:appearance_lookup]
137140

138141
result = add_photocracy_info(result, next_prompt, params[:question_id]) if @photocracy
139142
render :json => result.to_json
@@ -175,11 +178,14 @@ def add_photocracy_info(result, next_prompt, question_id)
175178
end
176179

177180
def get_object_request_options(params, request_type)
178-
options = { :visitor_identifier => request.session_options[:id],
181+
options = { :visitor_identifier => @survey_session.session_id,
179182
# use static value of 5 if in test, so we can mock resulting API queries
180183
:time_viewed => (Rails.env == 'test') ? 5 : params[:time_viewed],
181184
:appearance_lookup => params[:appearance_lookup]
182185
}
186+
if @survey_session.old_session_id
187+
options.merge!({:old_visitor_identifier => @survey_session.old_session_id})
188+
end
183189
case request_type
184190
when :vote
185191
options.merge!({:direction => params[:direction],
@@ -203,7 +209,7 @@ def get_object_request_options(params, request_type)
203209
def get_next_prompt_options
204210
next_prompt_params = { :with_appearance => true,
205211
:with_visitor_stats => true,
206-
:visitor_identifier => request.session_options[:id]
212+
:visitor_identifier => @survey_session.session_id
207213
}
208214
next_prompt_params.merge!(:future_prompts => {:number => 1}) if @photocracy
209215
next_prompt_params

app/controllers/questions_controller.rb

+4-4
Original file line numberDiff line numberDiff line change
@@ -963,7 +963,7 @@ def add_idea
963963
end
964964
end
965965

966-
choice_params = {:visitor_identifier => request.session_options[:id],
966+
choice_params = {:visitor_identifier => @survey_session.session_id,
967967
:data => new_idea_data,
968968
:question_id => params[:id]}
969969

@@ -973,7 +973,7 @@ def add_idea
973973
if @choice = Choice.create(choice_params)
974974
@question = Question.find(params[:id], :params => {
975975
:with_visitor_stats => true,
976-
:visitor_identifier => request.session_options[:id]
976+
:visitor_identifier => @survey_session.session_id
977977
})
978978
@earl = Earl.find_by_question_id(params[:id])
979979

@@ -1093,7 +1093,7 @@ def create
10931093
def question_params_valid
10941094
if @question.valid?(@photocracy) && (signed_in? || (@user.valid? && @user.save && sign_in(@user)))
10951095
@question.attributes.merge!({'local_identifier' => current_user.id,
1096-
'visitor_identifier' => request.session_options[:id]})
1096+
'visitor_identifier' => @survey_session.session_id})
10971097
return true if @question.save
10981098
else
10991099
return false
@@ -1252,7 +1252,7 @@ def upload_photos
12521252
end
12531253

12541254
def visitor_voting_history
1255-
@votes = Session.new(:id => request.session_options[:id]).get(:votes, :question_id => params[:id])
1255+
@votes = Session.new(:id => @survey_session.session_id).get(:votes, :question_id => params[:id])
12561256

12571257
if @photocracy
12581258
@votes['votes'].each do |vote|

0 commit comments

Comments
 (0)