post_vote.rb: determine vote magnitude from voter, not CurrentUser.
Bug: when moving favorites, the parent is given an upvote for each gold+
favoriter, but the magnitude of these upvotes was based on the
CurrentUser, rather than the upvoter. This meant that upvotes for
supervoter favorites weren't given correctly.
To fix this, `magnitude` is changed to use the voting `user` instead of
CurrentUser.
New problem: when setting the score, `score=` calls `magnitude`, but
`user` isn't initialized yet. So we also refactor so that
1. `initialize_attributes` initializes `user` before setting `score`
2. the vote direction is given by `vote`, so it's separate from `score`
3. updating the score on the post happens in a callback instead of
directly in `score=`.
This commit is contained in:
@@ -1063,7 +1063,7 @@ class Post < ActiveRecord::Base
|
|||||||
!PostVote.exists?(:user_id => user.id, :post_id => id)
|
!PostVote.exists?(:user_id => user.id, :post_id => id)
|
||||||
end
|
end
|
||||||
|
|
||||||
def vote!(score, voter = CurrentUser.user)
|
def vote!(vote, voter = CurrentUser.user)
|
||||||
unless voter.is_voter?
|
unless voter.is_voter?
|
||||||
raise PostVote::Error.new("You do not have permission to vote")
|
raise PostVote::Error.new("You do not have permission to vote")
|
||||||
end
|
end
|
||||||
@@ -1072,7 +1072,7 @@ class Post < ActiveRecord::Base
|
|||||||
raise PostVote::Error.new("You have already voted for this post")
|
raise PostVote::Error.new("You have already voted for this post")
|
||||||
end
|
end
|
||||||
|
|
||||||
votes.create!(user: voter, score: vote)
|
votes.create!(user: voter, vote: vote)
|
||||||
reload # PostVote.create modifies our score. Reload to get the new score.
|
reload # PostVote.create modifies our score. Reload to get the new score.
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|||||||
@@ -2,10 +2,14 @@ class PostVote < ActiveRecord::Base
|
|||||||
class Error < Exception ; end
|
class Error < Exception ; end
|
||||||
|
|
||||||
belongs_to :post
|
belongs_to :post
|
||||||
before_validation :initialize_user, :on => :create
|
belongs_to :user
|
||||||
|
attr_accessor :vote
|
||||||
|
attr_accessible :post, :post_id, :user, :user_id, :score, :vote
|
||||||
|
|
||||||
|
after_initialize :initialize_attributes, if: :new_record?
|
||||||
validates_presence_of :post_id, :user_id, :score
|
validates_presence_of :post_id, :user_id, :score
|
||||||
validates_inclusion_of :score, :in => [SuperVoter::MAGNITUDE, 1, -1, -SuperVoter::MAGNITUDE]
|
validates_inclusion_of :score, :in => [SuperVoter::MAGNITUDE, 1, -1, -SuperVoter::MAGNITUDE]
|
||||||
attr_accessible :post_id, :user_id, :score
|
after_create :update_post_on_create
|
||||||
after_destroy :update_post_on_destroy
|
after_destroy :update_post_on_destroy
|
||||||
|
|
||||||
def self.prune!
|
def self.prune!
|
||||||
@@ -24,18 +28,22 @@ class PostVote < ActiveRecord::Base
|
|||||||
select_values_sql("select post_id from post_votes where score > 0 and user_id = ?", user_id)
|
select_values_sql("select post_id from post_votes where score > 0 and user_id = ?", user_id)
|
||||||
end
|
end
|
||||||
|
|
||||||
def score=(x)
|
def initialize_attributes
|
||||||
if x == "up"
|
self.user_id ||= CurrentUser.user.id
|
||||||
Post.where(:id => post_id).update_all("score = score + #{magnitude}, up_score = up_score + #{magnitude}")
|
|
||||||
write_attribute(:score, magnitude)
|
if vote == "up"
|
||||||
elsif x == "down"
|
self.score = magnitude
|
||||||
Post.where(:id => post_id).update_all("score = score - #{magnitude}, down_score = down_score - #{magnitude}")
|
elsif vote == "down"
|
||||||
write_attribute(:score, -magnitude)
|
self.score = -magnitude
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def initialize_user
|
def update_post_on_create
|
||||||
self.user_id ||= CurrentUser.user.id
|
if score > 0
|
||||||
|
Post.where(:id => post_id).update_all("score = score + #{score}, up_score = up_score + #{score}")
|
||||||
|
else
|
||||||
|
Post.where(:id => post_id).update_all("score = score + #{score}, down_score = down_score + #{score}")
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def update_post_on_destroy
|
def update_post_on_destroy
|
||||||
@@ -47,7 +55,7 @@ class PostVote < ActiveRecord::Base
|
|||||||
end
|
end
|
||||||
|
|
||||||
def magnitude
|
def magnitude
|
||||||
if CurrentUser.is_super_voter?
|
if user.is_super_voter?
|
||||||
SuperVoter::MAGNITUDE
|
SuperVoter::MAGNITUDE
|
||||||
else
|
else
|
||||||
1
|
1
|
||||||
|
|||||||
@@ -14,17 +14,17 @@ class PostVoteTest < ActiveSupport::TestCase
|
|||||||
|
|
||||||
context "Voting for a post" do
|
context "Voting for a post" do
|
||||||
should "interpret up as +1 score" do
|
should "interpret up as +1 score" do
|
||||||
vote = PostVote.create(:post_id => @post.id, :score => "up")
|
vote = PostVote.create(:post_id => @post.id, :vote => "up")
|
||||||
assert_equal(1, vote.score)
|
assert_equal(1, vote.score)
|
||||||
end
|
end
|
||||||
|
|
||||||
should "interpret down as -1 score" do
|
should "interpret down as -1 score" do
|
||||||
vote = PostVote.create(:post_id => @post.id, :score => "down")
|
vote = PostVote.create(:post_id => @post.id, :vote => "down")
|
||||||
assert_equal(-1, vote.score)
|
assert_equal(-1, vote.score)
|
||||||
end
|
end
|
||||||
|
|
||||||
should "not accept any other scores" do
|
should "not accept any other scores" do
|
||||||
vote = PostVote.create(:post_id => @post.id, :score => "xxx")
|
vote = PostVote.create(:post_id => @post.id, :vote => "xxx")
|
||||||
assert(vote.errors.any?)
|
assert(vote.errors.any?)
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|||||||
Reference in New Issue
Block a user