Commit d5746ee99c5d4cdbad85bee0c9ca41e7f2e5175e
1 parent
64a0f42e
Exists in
master
and in
1 other branch
Preventing users with visitor identifiers from skipping prompts by reloading
Showing
6 changed files
with
44 additions
and
4 deletions
Show diff stats
app/models/appearance.rb
@@ -7,4 +7,8 @@ class Appearance < ActiveRecord::Base | @@ -7,4 +7,8 @@ class Appearance < ActiveRecord::Base | ||
7 | # we could refactor this to use rails polymorphism, but currently the foreign key is stored in the vote and skip object | 7 | # we could refactor this to use rails polymorphism, but currently the foreign key is stored in the vote and skip object |
8 | has_one :vote | 8 | has_one :vote |
9 | has_one :skip | 9 | has_one :skip |
10 | + | ||
11 | + def answered? | ||
12 | + vote || skip | ||
13 | + end | ||
10 | end | 14 | end |
app/models/question.rb
@@ -146,7 +146,15 @@ class Question < ActiveRecord::Base | @@ -146,7 +146,15 @@ class Question < ActiveRecord::Base | ||
146 | 146 | ||
147 | if params[:with_appearance] && visitor_identifier.present? | 147 | if params[:with_appearance] && visitor_identifier.present? |
148 | visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) | 148 | visitor = current_user.visitors.find_or_create_by_identifier(visitor_identifier) |
149 | - @appearance = current_user.record_appearance(visitor, @prompt) | 149 | + |
150 | + if visitor.appearances.last.nil? || visitor.appearances.last.answered? | ||
151 | + @appearance = current_user.record_appearance(visitor, @prompt) | ||
152 | + else | ||
153 | + #only display a new prompt and new appearance if the old prompt has not been voted on | ||
154 | + @appearance = visitor.appearances.last | ||
155 | + @prompt = @appearance.prompt | ||
156 | + result.merge!({:picked_prompt_id => @prompt.id}) | ||
157 | + end | ||
150 | result.merge!({:appearance_id => @appearance.lookup}) | 158 | result.merge!({:appearance_id => @appearance.lookup}) |
151 | else | 159 | else |
152 | # throw some error | 160 | # throw some error |
app/models/visitor.rb
@@ -5,7 +5,7 @@ class Visitor < ActiveRecord::Base | @@ -5,7 +5,7 @@ class Visitor < ActiveRecord::Base | ||
5 | has_many :skips, :class_name => "Skip", :foreign_key => "skipper_id" | 5 | has_many :skips, :class_name => "Skip", :foreign_key => "skipper_id" |
6 | has_many :items, :class_name => "Item", :foreign_key => "creator_id" | 6 | has_many :items, :class_name => "Item", :foreign_key => "creator_id" |
7 | has_many :clicks | 7 | has_many :clicks |
8 | - has_many :appearances | 8 | + has_many :appearances, :foreign_key => "voter_id" |
9 | 9 | ||
10 | validates_presence_of :site, :on => :create, :message => "can't be blank" | 10 | validates_presence_of :site, :on => :create, :message => "can't be blank" |
11 | # validates_uniqueness_of :identifier, :on => :create, :message => "must be unique", :scope => :site_id | 11 | # validates_uniqueness_of :identifier, :on => :create, :message => "must be unique", :scope => :site_id |
spec/models/appearance_spec.rb
@@ -3,8 +3,24 @@ require 'spec_helper' | @@ -3,8 +3,24 @@ require 'spec_helper' | ||
3 | describe Appearance do | 3 | describe Appearance do |
4 | before(:each) do | 4 | before(:each) do |
5 | end | 5 | end |
6 | + | ||
7 | + it {should belong_to :voter} | ||
8 | + it {should belong_to :prompt} | ||
9 | + it {should belong_to :question} | ||
10 | + it {should have_one :vote} | ||
11 | + it {should have_one :skip} | ||
6 | 12 | ||
7 | it "should create a new instance given valid attributes" do | 13 | it "should create a new instance given valid attributes" do |
8 | Appearance.create!(@valid_attributes) | 14 | Appearance.create!(@valid_attributes) |
9 | end | 15 | end |
16 | + it "should mark voted upon appearances as answered == true" do | ||
17 | + @appearance = Appearance.create!(@valid_attributes) | ||
18 | + @vote = Factory.create(:vote, :appearance => @appearance) | ||
19 | + @appearance.should be_answered | ||
20 | + end | ||
21 | + it "should mark voted upon appearances as answered == true" do | ||
22 | + @appearance = Appearance.create!(@valid_attributes) | ||
23 | + @skip = Skip.create!(:appearance => @appearance) | ||
24 | + @appearance.should be_answered | ||
25 | + end | ||
10 | end | 26 | end |
spec/models/question_spec.rb
@@ -13,10 +13,8 @@ describe Question do | @@ -13,10 +13,8 @@ describe Question do | ||
13 | it {should validate_presence_of :creator} | 13 | it {should validate_presence_of :creator} |
14 | 14 | ||
15 | before(:each) do | 15 | before(:each) do |
16 | - | ||
17 | @question = Factory.create(:aoi_question) | 16 | @question = Factory.create(:aoi_question) |
18 | @aoi_clone = @question.site | 17 | @aoi_clone = @question.site |
19 | - | ||
20 | end | 18 | end |
21 | 19 | ||
22 | it "should have 2 active choices" do | 20 | it "should have 2 active choices" do |
@@ -101,6 +99,19 @@ describe Question do | @@ -101,6 +99,19 @@ describe Question do | ||
101 | @question_optional_information[:appearance_id].should be_an_instance_of(String) | 99 | @question_optional_information[:appearance_id].should be_an_instance_of(String) |
102 | end | 100 | end |
103 | 101 | ||
102 | + it "should return the same appearance when a visitor requests two prompts without voting" do | ||
103 | + params = {:id => 124, :with_visitor_stats=> true, :visitor_identifier => "jim", :with_prompt => true, :with_appearance => true} | ||
104 | + @question_optional_information = @question.get_optional_information(params) | ||
105 | + @question_optional_information[:appearance_id].should be_an_instance_of(String) | ||
106 | + @question_optional_information[:picked_prompt_id].should be_an_instance_of(Fixnum) | ||
107 | + saved_appearance_id = @question_optional_information[:appearance_id] | ||
108 | + saved_prompt_id = @question_optional_information[:picked_prompt_id] | ||
109 | + | ||
110 | + @question_optional_information = @question.get_optional_information(params) | ||
111 | + @question_optional_information[:appearance_id].should == saved_appearance_id | ||
112 | + @question_optional_information[:picked_prompt_id].should == saved_prompt_id | ||
113 | + end | ||
114 | + | ||
104 | context "catchup algorithm" do | 115 | context "catchup algorithm" do |
105 | before(:all) do | 116 | before(:all) do |
106 | @catchup_q = Factory.create(:aoi_question) | 117 | @catchup_q = Factory.create(:aoi_question) |
spec/models/visitor_spec.rb
@@ -7,6 +7,7 @@ describe Visitor do | @@ -7,6 +7,7 @@ describe Visitor do | ||
7 | it {should have_many :votes} | 7 | it {should have_many :votes} |
8 | it {should have_many :skips} | 8 | it {should have_many :skips} |
9 | it {should have_many :clicks} | 9 | it {should have_many :clicks} |
10 | + it {should have_many :appearances} | ||
10 | 11 | ||
11 | before(:each) do | 12 | before(:each) do |
12 | @question = Factory.create(:aoi_question) | 13 | @question = Factory.create(:aoi_question) |