diff --git a/app/controllers/prompts_controller.rb b/app/controllers/prompts_controller.rb index 5ba4ca6..daa4e75 100644 --- a/app/controllers/prompts_controller.rb +++ b/app/controllers/prompts_controller.rb @@ -49,52 +49,39 @@ class PromptsController < InheritedResources::Base def skip logger.info "#{current_user.inspect} is skipping." - @question = Question.find(params[:question_id]) - - @prompt = @question.prompts.find(params[:id]) #, :include => [{ :left_choice => :item }, { :right_choice => :item }]) - - time_viewed = params['params']['time_viewed'] - raise "time_viewed cannot be nil" if time_viewed.nil? - - visitor_identifier = params['params']['auto'] - raise "visitor identifier cannot be nil" if visitor_identifier.nil? - appearance_lookup = params['params']['appearance_lookup'] - raise "appearance_lookup cannot be nil" if appearance_lookup.nil? + @question = current_user.questions.find(params[:question_id]) + @prompt = @question.prompts.find(params[:id]) - skip_reason = params['params']['skip_reason'] # optional parameter - + skip_options = params[:skip] || {} + skip_options.merge!(:prompt => @prompt, :question => @question) - respond_to do |format| - if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason) - - #This is not hte right way to do this. See def vote for a better example + successful = response = current_user.record_skip(skip_options) + optional_information = [] + if params[:next_prompt] begin - @next_prompt = @question.choose_prompt + params[:next_prompt].merge!(:with_prompt => true) # We always want to get the next possible prompt + @question_optional_information = @question.get_optional_information(params[:next_prompt]) rescue RuntimeError respond_to do |format| format.xml { render :xml => @prompt.to_xml, :status => :conflict and return} end end - - visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) - @a = current_user.record_appearance(visitor, @next_prompt) - - appearance_id = Proc.new { |options| options[:builder].tag!('appearance_id', @a.lookup) } - - visitor_votes = Proc.new { |options| options[:builder].tag!('visitor_votes', visitor.votes.count(:conditions => {:question_id => @question.id})) } - visitor_ideas = Proc.new { |options| options[:builder].tag!('visitor_ideas', visitor.items.count) } - - - format.xml { render :xml => @next_prompt.to_xml(:procs => [appearance_id, visitor_votes, visitor_ideas],:methods => [:left_choice_text, :right_choice_text]), :status => :ok } - format.json { render :json => @next_prompt.to_json, :status => :ok } + response = @question.prompts.find(@question_optional_information.delete(:picked_prompt_id)) + @question_optional_information.each do |key, value| + optional_information << Proc.new { |options| options[:builder].tag!(key, value)} + end + end + respond_to do |format| + if !successful.nil? + 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 } + 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 } else - format.xml { render :xml => @prompt.to_xml, :status => :conflict} - format.json { render :json => @prompt.to_xml, :status => :conflict} + format.xml { render :xml => @prompt.to_xml, :status => :unprocessable_entity } + format.json { render :json => @prompt.to_xml, :status => :unprocessable_entity } end end end - def show @question = current_user.questions.find(params[:question_id]) diff --git a/app/models/user.rb b/app/models/user.rb index 1d95b41..bfc7290 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -47,9 +47,14 @@ class User < ActiveRecord::Base end - def record_skip(visitor_identifier, appearance_lookup, prompt, time_viewed, options = {}) - visitor = visitors.find_or_create_by_identifier(visitor_identifier) - visitor.skip!(appearance_lookup, prompt, time_viewed, options) + def record_skip(options) + visitor_identifier = options.delete(:visitor_identifier) + if visitor_identifier.nil? + visitor = default_visitor + else + visitor = visitors.find_or_create_by_identifier(visitor_identifier) + end + visitor.skip!(options) end def activate_question(question_id, options) diff --git a/app/models/visitor.rb b/app/models/visitor.rb index c6e1983..d42f978 100644 --- a/app/models/visitor.rb +++ b/app/models/visitor.rb @@ -36,14 +36,19 @@ class Visitor < ActiveRecord::Base v = votes.create!(options) end - - def skip!(appearance_lookup, prompt, time_viewed, options = {}) - @a = Appearance.find_by_lookup(appearance_lookup) - - skip_create_options = { :question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id=> self.id, :time_viewed => time_viewed, :appearance_id => @a.id} - #the most common optional reason is 'skip_reason', probably want to refactor to make time viewed an optional parameter - prompt_skip = skips.create!(skip_create_options.merge(options)) + def skip!(options) + return nil if !options || !options[:prompt] + + prompt = options.delete(:prompt) + + if options[:appearance_lookup] + @appearance = prompt.appearances.find_by_lookup(options.delete(:apperance_lookup)) + return nil unless @appearance + options.merge!(:appearance_id => @appearance.id) + end + options.merge!(:question_id => prompt.question_id, :prompt_id => prompt.id, :skipper_id => self.id) + prompt_skip = skips.create!(options) end end diff --git a/spec/controllers/prompts_controller_spec.rb b/spec/controllers/prompts_controller_spec.rb index 61d2f8e..2116966 100644 --- a/spec/controllers/prompts_controller_spec.rb +++ b/spec/controllers/prompts_controller_spec.rb @@ -21,16 +21,41 @@ describe PromptsController do assigns[:prompt].should == @prompt end end - - describe "POST skip" do - it "records a skip, responds with next prompt" do - post :skip, :id => @prompt.id, :question_id => @question.id, :params => {:auto => @visitor, :time_viewed => 30, :appearance_lookup => @appearance.lookup} - assigns[:next_prompt].should_not be_nil - assigns[:a].should_not be_nil - assigns[:a].should_not == @appearance - assigns[:skip].should_not be_nil + + describe "POST skip" do + it "records a skip without any optional params" do + controller.current_user.should_receive(:record_skip).and_return(true) + post(:skip, :question_id => @question.id, :id => @prompt.id, + :format => :xml) end - end + + it "records a skip with optional params" do + controller.current_user.should_receive(:record_skip).and_return(true) + post(:skip, :question_id => @question.id, :id => @prompt.id, + :skip => { + :visitor_identifier => "jim", + :time_viewed => rand(1000), + :skip_reason => "some reason"}) + end + + it "records a skip, responds with next prompt" do + controller.current_user.should_receive(:record_skip).and_return(true) + post(:skip, :question_id => @question.id, :id => @prompt.id, + :skip => { + :visitor_identifier => "jim", + :time_viewed => rand(1000), + :skip_reason => "some reason"}, + :next_prompt => { + :with_appearance => true, + :with_visitor_stats => true, + :visitor_identifier => "jim"} + ) + assigns[:question_optional_information].should_not be_nil + assigns[:question_optional_information][:appearance_id].should_not be_nil + assigns[:question_optional_information][:visitor_votes].should_not be_nil + assigns[:question_optional_information][:visitor_ideas].should_not be_nil + end + end describe "POST vote" do it "votes on a prompt" do diff --git a/spec/models/question_spec.rb b/spec/models/question_spec.rb index a6d61f5..e22ad35 100644 --- a/spec/models/question_spec.rb +++ b/spec/models/question_spec.rb @@ -219,12 +219,18 @@ describe Question do :time_viewed => rand(1000), :direction => (rand(2) == 0) ? "left" : "right"} + skip_options = {:visitor_identifier => visitor.identifier, + :appearance_lookup => @a.lookup, + :prompt => @p, + :time_viewed => rand(1000), + :skip_reason => "some reason"} + choice = rand(3) case choice when 0 user.record_vote(vote_options) when 1 - user.record_skip(visitor.identifier, @a.lookup, @p, rand(1000)) + user.record_skip(skip_options) when 2 #this is an orphaned appearance, so do nothing end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 0adf7e0..535cb17 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -83,7 +83,14 @@ describe User do it "should be able to record a visitor's skip" do @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier") @appearance = @aoi_clone.record_appearance(@visitor, @prompt) - s = @aoi_clone.record_skip("johnnydoe", @appearance.lookup, @prompt, 340) + optional_skip_params = { + :visitor_identifier => @visitor.identifier, + :appearance_lookup => @appearance.lookup, + :time_viewed => 340 + } + required_skip_params = {:prompt => @prompt } + params = optional_skip_params.merge(required_skip_params) + s = @aoi_clone.record_skip(params) end end diff --git a/spec/models/visitor_spec.rb b/spec/models/visitor_spec.rb index 39a1a7e..4f86d66 100644 --- a/spec/models/visitor_spec.rb +++ b/spec/models/visitor_spec.rb @@ -18,6 +18,7 @@ describe Visitor do @required_vote_params = {:prompt => @prompt, :direction => "left"} + @required_skip_params = {:prompt => @prompt} end it "should create a new instance given valid attributes" do @@ -93,7 +94,13 @@ describe Visitor do it "should be able to skip a prompt" do @appearance = @aoi_clone.record_appearance(@visitor, @prompt) - s = @visitor.skip! @appearance.lookup, @prompt, 304 + @optional_skip_params = { + :appearance_lookup => @appearance.lookup, + :time_viewed => 304, + :skip_reason => "some reason" + } + allparams = @required_skip_params.merge(@optional_skip_params) + s = @visitor.skip!(allparams) end it "should accurately update score counts after vote" do -- libgit2 0.21.2