From 9f0ec712f3369a626f7484d3cd24ef9acfb0e295 Mon Sep 17 00:00:00 2001 From: Dhruv Kapadia Date: Wed, 28 Apr 2010 19:58:51 -0400 Subject: [PATCH] Rewriting skips model, providing support --- app/controllers/prompts_controller.rb | 42 +++++++++++++++++++++++++++++++++++++----- app/models/user.rb | 5 ++--- app/models/visitor.rb | 10 ++++++++-- db/migrate/20100428190803_rename_skips_table.rb | 9 +++++++++ db/migrate/20100428190907_create_skips_model.rb | 19 +++++++++++++++++++ db/migrate/20100428211749_add_skip_reason_to_skips.rb | 9 +++++++++ spec/controllers/prompts_controller_spec.rb | 57 ++++++++++++++++++++++++++++++++++++++------------------- spec/models/user_spec.rb | 2 +- spec/models/visitor_spec.rb | 2 +- 9 files changed, 124 insertions(+), 31 deletions(-) create mode 100644 db/migrate/20100428190803_rename_skips_table.rb create mode 100644 db/migrate/20100428190907_create_skips_model.rb create mode 100644 db/migrate/20100428211749_add_skip_reason_to_skips.rb diff --git a/app/controllers/prompts_controller.rb b/app/controllers/prompts_controller.rb index ab5efd6..16277b4 100644 --- a/app/controllers/prompts_controller.rb +++ b/app/controllers/prompts_controller.rb @@ -53,9 +53,7 @@ class PromptsController < InheritedResources::Base @prompt = @question.prompts.find(params[:id]) time_viewed = params['params']['time_viewed'] - visitor_identifier = params['params']['auto'] - appearance_lookup = params['params']['appearance_lookup'] case direction @@ -71,6 +69,7 @@ class PromptsController < InheritedResources::Base respond_to do |format| if successful #catchup_marketplace_ids = [120, 117, 1, 116] + #TODO refactor into question model if @question.uses_catchup? logger.info("Question #{@question.id} is using catchup algorithm!") next_prompt = @question.pop_prompt_queue @@ -125,12 +124,45 @@ class PromptsController < InheritedResources::Base authenticate 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 }]) + @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? + + skip_reason = params['params']['skip_reason'] # optional parameter respond_to do |format| - if @skip = current_user.record_skip(params['params']['auto'], @prompt) - format.xml { render :xml => @question.picked_prompt.to_xml(:methods => [:left_choice_text, :right_choice_text]), :status => :ok } + if @skip = current_user.record_skip(visitor_identifier, appearance_lookup, @prompt, time_viewed, :skip_reason => skip_reason) + + if @question.uses_catchup? + logger.info("Question #{@question.id} is using catchup algorithm!") + @next_prompt = @question.pop_prompt_queue + if @next_prompt.nil? + logger.info("Catchup prompt cache miss! Nothing in prompt_queue") + @next_prompt = @question.catchup_choose_prompt + end + @question.send_later :add_prompt_to_queue + else + @next_prompt = @question.picked_prompt + 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 => @question.picked_prompt.to_xml(:procs => [appearance_id, visitor_votes, visitor_ideas],:methods => [:left_choice_text, :right_choice_text]), :status => :ok } format.json { render :json => @question.picked_prompt.to_json, :status => :ok } else format.xml { render :xml => @skip, :status => :unprocessable_entity } diff --git a/app/models/user.rb b/app/models/user.rb index 71b9c37..b1d7233 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -45,10 +45,9 @@ class User < ActiveRecord::Base end - def record_skip(visitor_identifier, prompt) + def record_skip(visitor_identifier, appearance_lookup, prompt, time_viewed, options = {}) visitor = visitors.find_or_create_by_identifier(visitor_identifier) - question = prompt.question - visitor.skip!(prompt) + visitor.skip!(appearance_lookup, prompt, time_viewed, options) end def activate_question(question_id, options) diff --git a/app/models/visitor.rb b/app/models/visitor.rb index 4c21b88..742b8a3 100644 --- a/app/models/visitor.rb +++ b/app/models/visitor.rb @@ -18,6 +18,7 @@ class Visitor < ActiveRecord::Base def vote_for!(appearance_lookup, prompt, ordinality, time_viewed) @a = Appearance.find_by_lookup(appearance_lookup) + #make votefor fail if we cant find the appearance choices = prompt.choices choice = choices[ordinality] #we need to guarantee that the choices are in the right order (by position) other_choices = choices - [choice] @@ -37,7 +38,12 @@ class Visitor < ActiveRecord::Base end - def skip!(prompt) - prompt_skip = skips.create!(:prompt => prompt) + 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} + + prompt_skip = skips.create!(skip_create_options.merge(options)) + end end diff --git a/db/migrate/20100428190803_rename_skips_table.rb b/db/migrate/20100428190803_rename_skips_table.rb new file mode 100644 index 0000000..eb54c1f --- /dev/null +++ b/db/migrate/20100428190803_rename_skips_table.rb @@ -0,0 +1,9 @@ +class RenameSkipsTable < ActiveRecord::Migration + def self.up + rename_table :skips, :oldskips + end + + def self.down + rename_table :oldskips, :skips + end +end diff --git a/db/migrate/20100428190907_create_skips_model.rb b/db/migrate/20100428190907_create_skips_model.rb new file mode 100644 index 0000000..7f5042e --- /dev/null +++ b/db/migrate/20100428190907_create_skips_model.rb @@ -0,0 +1,19 @@ +class CreateSkipsModel < ActiveRecord::Migration + def self.up + create_table :skips do |table| + table.text :tracking + table.integer :site_id + table.integer :skipper_id + table.integer :question_id + table.integer :prompt_id + table.integer :appearance_id + table.integer :time_viewed #msecs + table.timestamps + end + + end + + def self.down + drop_table :skips + end +end diff --git a/db/migrate/20100428211749_add_skip_reason_to_skips.rb b/db/migrate/20100428211749_add_skip_reason_to_skips.rb new file mode 100644 index 0000000..0532141 --- /dev/null +++ b/db/migrate/20100428211749_add_skip_reason_to_skips.rb @@ -0,0 +1,9 @@ +class AddSkipReasonToSkips < ActiveRecord::Migration + def self.up + add_column :skips, :skip_reason, :string + end + + def self.down + drop_column :skips, :skip_reason + end +end diff --git a/spec/controllers/prompts_controller_spec.rb b/spec/controllers/prompts_controller_spec.rb index 308a1c3..2074ddc 100644 --- a/spec/controllers/prompts_controller_spec.rb +++ b/spec/controllers/prompts_controller_spec.rb @@ -1,31 +1,50 @@ require File.expand_path(File.dirname(__FILE__) + '/../spec_helper') describe PromptsController do + def sign_in_as(user) + @controller.current_user = user + return user + end + # + before(:each) do + @aoi_clone = Factory.create(:user) + sign_in_as(@user = Factory(:email_confirmed_user)) + @johndoe = Factory.create(:visitor, :identifier => 'johndoe', :site => @aoi_clone) + @question = Factory.create(:question, :name => 'which do you like better?', :site => @aoi_clone, :creator => @aoi_clone.default_visitor) + @lc = Factory.create(:choice, :question => @question, :creator => @johndoe, :data => 'hello gorgeous') + @rc = Factory.create(:choice, :question => @question, :creator => @johndoe, :data => 'goodbye gorgeous') + @prompt = Factory.create(:prompt, :question => @question, :tracking => 'sample', :left_choice => @lc, :right_choice => @rc) + + @visitor = @aoi_clone.visitors.find_or_create_by_identifier("test_visitor_identifier") + @appearance = @aoi_clone.record_appearance(@visitor, @prompt) + end + # - def mock_prompt(stubs={}) - @mock_prompt ||= mock_model(Prompt, stubs) - end - - before(:each) do - @aoi_clone = Factory.create(:user) - @question = Factory.create(:question, :site => @aoi_clone, :creator => @aoi_clone.default_visitor) - end - - describe "GET index" do - it "assigns all prompts as @prompts" do +# describe "GET index" do +# it "assigns all prompts as @prompts" do # Question.stub!(:find).with(:all).and_return(@question) # Question.stub!(:prompts).with(:all).and_return([mock_prompt]) - get :index, :question_id => @question.id - assigns[:prompts].should == [mock_prompt] - end - end +# get :index, :question_id => @question.id +# assigns[:prompts].should == [@prompt] +# end +# end describe "GET show" do it "assigns the requested prompt as @prompt" do -# Question.stub!(:find).with(:all).and_return(@question) -# Prompt.stub!(:find).with("37").and_return(mock_prompt) - get :show, :id => "37", :question_id => @question.id - assigns[:prompt].should equal(mock_prompt) + get :show, :id => @prompt.id, :question_id => @question.id + 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 == @prompt + assigns[:next_prompt].should_not be_nil + assigns[:a].should_not be_nil + assigns[:a].should_not == @appearance + assigns[:skip].should_not be_nil + end + end + end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fd69fa5..a37d203 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -47,7 +47,7 @@ describe User do end it "should be able to record a visitor's skip" do - s = @aoi_clone.record_skip("johnnydoe", @prompt) + s = @aoi_clone.record_skip("johnnydoe", @appearance.lookup, @prompt, 340) end end diff --git a/spec/models/visitor_spec.rb b/spec/models/visitor_spec.rb index 232e666..297a806 100644 --- a/spec/models/visitor_spec.rb +++ b/spec/models/visitor_spec.rb @@ -47,7 +47,7 @@ describe Visitor do it "should be able to skip a prompt" do #@prompt = @question.prompts.first @prompt.should_not be_nil - v = @v.skip! @prompt + v = @v.skip! @appearance.lookup, @prompt, 304 end it "should accurately update score counts after vote" do -- libgit2 0.21.2