Commit d1af06d3aa6448fcea563fbcfefb353b835df8af
Exists in
master
and in
1 other branch
Merge branch 'master' of github.com:allourideas/pairwise2
Conflicts: db/schema.rb
Showing
9 changed files
with
106 additions
and
55 deletions
Show diff stats
.gitignore
app/controllers/prompts_controller.rb
... | ... | @@ -49,27 +49,18 @@ class PromptsController < InheritedResources::Base |
49 | 49 | |
50 | 50 | def skip |
51 | 51 | logger.info "#{current_user.inspect} is skipping." |
52 | - @question = Question.find(params[:question_id]) | |
53 | - | |
54 | - @prompt = @question.prompts.find(params[:id]) #, :include => [{ :left_choice => :item }, { :right_choice => :item }]) | |
55 | - | |
56 | - time_viewed = params['params']['time_viewed'] | |
57 | - raise "time_viewed cannot be nil" if time_viewed.nil? | |
58 | - | |
59 | - visitor_identifier = params['params']['auto'] | |
60 | - raise "visitor identifier cannot be nil" if visitor_identifier.nil? | |
61 | - appearance_lookup = params['params']['appearance_lookup'] | |
62 | - raise "appearance_lookup cannot be nil" if appearance_lookup.nil? | |
52 | + @question = current_user.questions.find(params[:question_id]) | |
53 | + @prompt = @question.prompts.find(params[:id]) | |
63 | 54 | |
64 | - skip_reason = params['params']['skip_reason'] # optional parameter | |
65 | - | |
55 | + skip_options = params[:skip] || {} | |
56 | + skip_options.merge!(:prompt => @prompt, :question => @question) | |
66 | 57 | |
67 | - respond_to do |format| | |
68 | - if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason) | |
69 | - | |
70 | - #This is not hte right way to do this. See def vote for a better example | |
58 | + successful = response = current_user.record_skip(skip_options) | |
59 | + optional_information = [] | |
60 | + if params[:next_prompt] | |
71 | 61 | begin |
72 | - @next_prompt = @question.choose_prompt | |
62 | + params[:next_prompt].merge!(:with_prompt => true) # We always want to get the next possible prompt | |
63 | + @question_optional_information = @question.get_optional_information(params[:next_prompt]) | |
73 | 64 | rescue RuntimeError |
74 | 65 | |
75 | 66 | respond_to do |format| |
... | ... | @@ -77,24 +68,21 @@ class PromptsController < InheritedResources::Base |
77 | 68 | end |
78 | 69 | end |
79 | 70 | |
80 | - visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) | |
81 | - @a = current_user.record_appearance(visitor, @next_prompt) | |
82 | - | |
83 | - appearance_id = Proc.new { |options| options[:builder].tag!('appearance_id', @a.lookup) } | |
84 | - | |
85 | - visitor_votes = Proc.new { |options| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) } | |
86 | - visitor_ideas = Proc.new { |options| options[:builder].tag!('visitor_ideas', visitor.choices.count) } | |
87 | - | |
88 | - | |
89 | - format.xml { render :xml => @next_prompt.to_xml(:procs => [appearance_id, visitor_votes, visitor_ideas],:methods => [:left_choice_text, :right_choice_text]), :status => :ok } | |
90 | - format.json { render :json => @next_prompt.to_json, :status => :ok } | |
71 | + response = @question.prompts.find(@question_optional_information.delete(:picked_prompt_id)) | |
72 | + @question_optional_information.each do |key, value| | |
73 | + optional_information << Proc.new { |options| options[:builder].tag!(key, value)} | |
74 | + end | |
75 | + end | |
76 | + respond_to do |format| | |
77 | + if !successful.nil? | |
78 | + format.xml { render :xml => response.to_xml(:procs => optional_information , :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok } | |
79 | + format.json { render :json => response.to_json(:procs => optional_information, :methods => [:left_choice_text, :right_choice_text, :left_choice_id, :right_choice_id]), :status => :ok } | |
91 | 80 | else |
92 | - format.xml { render :xml => @prompt.to_xml, :status => :conflict} | |
93 | - format.json { render :json => @prompt.to_xml, :status => :conflict} | |
81 | + format.xml { render :xml => @prompt.to_xml, :status => :unprocessable_entity } | |
82 | + format.json { render :json => @prompt.to_xml, :status => :unprocessable_entity } | |
94 | 83 | end |
95 | 84 | end |
96 | 85 | end |
97 | - | |
98 | 86 | |
99 | 87 | def show |
100 | 88 | @question = current_user.questions.find(params[:question_id]) | ... | ... |
app/models/user.rb
... | ... | @@ -46,9 +46,14 @@ class User < ActiveRecord::Base |
46 | 46 | end |
47 | 47 | |
48 | 48 | |
49 | - def record_skip(visitor_identifier, appearance_lookup, prompt, time_viewed, options = {}) | |
50 | - visitor = visitors.find_or_create_by_identifier(visitor_identifier) | |
51 | - visitor.skip!(appearance_lookup, prompt, time_viewed, options) | |
49 | + def record_skip(options) | |
50 | + visitor_identifier = options.delete(:visitor_identifier) | |
51 | + if visitor_identifier.nil? | |
52 | + visitor = default_visitor | |
53 | + else | |
54 | + visitor = visitors.find_or_create_by_identifier(visitor_identifier) | |
55 | + end | |
56 | + visitor.skip!(options) | |
52 | 57 | end |
53 | 58 | |
54 | 59 | def activate_question(question_id, options) | ... | ... |
app/models/visitor.rb
... | ... | @@ -36,14 +36,19 @@ class Visitor < ActiveRecord::Base |
36 | 36 | |
37 | 37 | v = votes.create!(options) |
38 | 38 | end |
39 | - | |
40 | - def skip!(appearance_lookup, prompt, time_viewed, options = {}) | |
41 | - @a = Appearance.find_by_lookup(appearance_lookup) | |
42 | - | |
43 | - skip_create_options = { :question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id=> self.id, :time_viewed => time_viewed, :appearance_id => @a.id} | |
44 | 39 | |
45 | - #the most common optional reason is 'skip_reason', probably want to refactor to make time viewed an optional parameter | |
46 | - prompt_skip = skips.create!(skip_create_options.merge(options)) | |
40 | + def skip!(options) | |
41 | + return nil if !options || !options[:prompt] | |
42 | + | |
43 | + prompt = options.delete(:prompt) | |
44 | + | |
45 | + if options[:appearance_lookup] | |
46 | + @appearance = prompt.appearances.find_by_lookup(options.delete(:apperance_lookup)) | |
47 | + return nil unless @appearance | |
48 | + options.merge!(:appearance_id => @appearance.id) | |
49 | + end | |
47 | 50 | |
51 | + options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id => self.id) | |
52 | + prompt_skip = skips.create!(options) | |
48 | 53 | end |
49 | 54 | end | ... | ... |
... | ... | @@ -0,0 +1,9 @@ |
1 | +# rake db:seed | |
2 | +# load the API user for AOI dev environment | |
3 | + | |
4 | +u = User.new(:email => "pairwisetest@dkapadia.com", :password => "wheatthins") | |
5 | +u.save(false) | |
6 | + | |
7 | + | |
8 | +u = User.new(:email => "photocracytest@dkapadia.com", :password => "saltines") | |
9 | +u.save(false) | |
0 | 10 | \ No newline at end of file | ... | ... |
spec/controllers/prompts_controller_spec.rb
... | ... | @@ -21,16 +21,41 @@ describe PromptsController do |
21 | 21 | assigns[:prompt].should == @prompt |
22 | 22 | end |
23 | 23 | end |
24 | - | |
25 | - describe "POST skip" do | |
26 | - it "records a skip, responds with next prompt" do | |
27 | - post :skip, :id => @prompt.id, :question_id => @question.id, :params => {:auto => @visitor, :time_viewed => 30, :appearance_lookup => @appearance.lookup} | |
28 | - assigns[:next_prompt].should_not be_nil | |
29 | - assigns[:a].should_not be_nil | |
30 | - assigns[:a].should_not == @appearance | |
31 | - assigns[:skip].should_not be_nil | |
24 | + | |
25 | + describe "POST skip" do | |
26 | + it "records a skip without any optional params" do | |
27 | + controller.current_user.should_receive(:record_skip).and_return(true) | |
28 | + post(:skip, :question_id => @question.id, :id => @prompt.id, | |
29 | + :format => :xml) | |
32 | 30 | end |
33 | - end | |
31 | + | |
32 | + it "records a skip with optional params" do | |
33 | + controller.current_user.should_receive(:record_skip).and_return(true) | |
34 | + post(:skip, :question_id => @question.id, :id => @prompt.id, | |
35 | + :skip => { | |
36 | + :visitor_identifier => "jim", | |
37 | + :time_viewed => rand(1000), | |
38 | + :skip_reason => "some reason"}) | |
39 | + end | |
40 | + | |
41 | + it "records a skip, responds with next prompt" do | |
42 | + controller.current_user.should_receive(:record_skip).and_return(true) | |
43 | + post(:skip, :question_id => @question.id, :id => @prompt.id, | |
44 | + :skip => { | |
45 | + :visitor_identifier => "jim", | |
46 | + :time_viewed => rand(1000), | |
47 | + :skip_reason => "some reason"}, | |
48 | + :next_prompt => { | |
49 | + :with_appearance => true, | |
50 | + :with_visitor_stats => true, | |
51 | + :visitor_identifier => "jim"} | |
52 | + ) | |
53 | + assigns[:question_optional_information].should_not be_nil | |
54 | + assigns[:question_optional_information][:appearance_id].should_not be_nil | |
55 | + assigns[:question_optional_information][:visitor_votes].should_not be_nil | |
56 | + assigns[:question_optional_information][:visitor_ideas].should_not be_nil | |
57 | + end | |
58 | + end | |
34 | 59 | |
35 | 60 | describe "POST vote" do |
36 | 61 | it "votes on a prompt" do | ... | ... |
spec/models/question_spec.rb
... | ... | @@ -225,12 +225,18 @@ describe Question do |
225 | 225 | :time_viewed => rand(1000), |
226 | 226 | :direction => (rand(2) == 0) ? "left" : "right"} |
227 | 227 | |
228 | + skip_options = {:visitor_identifier => visitor.identifier, | |
229 | + :appearance_lookup => @a.lookup, | |
230 | + :prompt => @p, | |
231 | + :time_viewed => rand(1000), | |
232 | + :skip_reason => "some reason"} | |
233 | + | |
228 | 234 | choice = rand(3) |
229 | 235 | case choice |
230 | 236 | when 0 |
231 | 237 | user.record_vote(vote_options) |
232 | 238 | when 1 |
233 | - user.record_skip(visitor.identifier, @a.lookup, @p, rand(1000)) | |
239 | + user.record_skip(skip_options) | |
234 | 240 | when 2 |
235 | 241 | #this is an orphaned appearance, so do nothing |
236 | 242 | end | ... | ... |
spec/models/user_spec.rb
... | ... | @@ -83,7 +83,14 @@ describe User do |
83 | 83 | it "should be able to record a visitor's skip" do |
84 | 84 | @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier") |
85 | 85 | @appearance = @aoi_clone.record_appearance(@visitor, @prompt) |
86 | - s = @aoi_clone.record_skip("johnnydoe", @appearance.lookup, @prompt, 340) | |
86 | + optional_skip_params = { | |
87 | + :visitor_identifier => @visitor.identifier, | |
88 | + :appearance_lookup => @appearance.lookup, | |
89 | + :time_viewed => 340 | |
90 | + } | |
91 | + required_skip_params = {:prompt => @prompt } | |
92 | + params = optional_skip_params.merge(required_skip_params) | |
93 | + s = @aoi_clone.record_skip(params) | |
87 | 94 | end |
88 | 95 | |
89 | 96 | end | ... | ... |
spec/models/visitor_spec.rb
... | ... | @@ -19,6 +19,7 @@ describe Visitor do |
19 | 19 | |
20 | 20 | @required_vote_params = {:prompt => @prompt, |
21 | 21 | :direction => "left"} |
22 | + @required_skip_params = {:prompt => @prompt} | |
22 | 23 | end |
23 | 24 | |
24 | 25 | it "should create a new instance given valid attributes" do |
... | ... | @@ -94,7 +95,13 @@ describe Visitor do |
94 | 95 | |
95 | 96 | it "should be able to skip a prompt" do |
96 | 97 | @appearance = @aoi_clone.record_appearance(@visitor, @prompt) |
97 | - s = @visitor.skip! @appearance.lookup, @prompt, 304 | |
98 | + @optional_skip_params = { | |
99 | + :appearance_lookup => @appearance.lookup, | |
100 | + :time_viewed => 304, | |
101 | + :skip_reason => "some reason" | |
102 | + } | |
103 | + allparams = @required_skip_params.merge(@optional_skip_params) | |
104 | + s = @visitor.skip!(allparams) | |
98 | 105 | end |
99 | 106 | |
100 | 107 | it "should accurately update score counts after vote" do | ... | ... |