Merge pull request #2937 from evazion/fix-move-fav-upvotes
Upvote correctly when moving favorites (partial fix for #2936)
This commit is contained in:
@@ -943,7 +943,7 @@ class Post < ActiveRecord::Base
|
||||
|
||||
def add_favorite!(user)
|
||||
Favorite.add(self, user)
|
||||
vote!("up") if CurrentUser.is_gold?
|
||||
vote!("up", user) if user.is_voter?
|
||||
rescue PostVote::Error
|
||||
end
|
||||
|
||||
@@ -953,7 +953,7 @@ class Post < ActiveRecord::Base
|
||||
|
||||
def remove_favorite!(user)
|
||||
Favorite.remove(self, user)
|
||||
unvote! if CurrentUser.is_gold?
|
||||
unvote!(user) if user.is_voter?
|
||||
rescue PostVote::Error
|
||||
end
|
||||
|
||||
@@ -1063,27 +1063,25 @@ class Post < ActiveRecord::Base
|
||||
!PostVote.exists?(:user_id => user.id, :post_id => id)
|
||||
end
|
||||
|
||||
def vote!(score)
|
||||
unless CurrentUser.is_voter?
|
||||
def vote!(vote, voter = CurrentUser.user)
|
||||
unless voter.is_voter?
|
||||
raise PostVote::Error.new("You do not have permission to vote")
|
||||
end
|
||||
|
||||
unless can_be_voted_by?(CurrentUser.user)
|
||||
unless can_be_voted_by?(voter)
|
||||
raise PostVote::Error.new("You have already voted for this post")
|
||||
end
|
||||
|
||||
PostVote.create!(:post_id => id, :score => score)
|
||||
votes.create!(user: voter, vote: vote)
|
||||
reload # PostVote.create modifies our score. Reload to get the new score.
|
||||
end
|
||||
|
||||
def unvote!
|
||||
if can_be_voted_by?(CurrentUser.user)
|
||||
def unvote!(voter = CurrentUser.user)
|
||||
if can_be_voted_by?(voter)
|
||||
raise PostVote::Error.new("You have not voted for this post")
|
||||
else
|
||||
vote = PostVote.where("post_id = ? and user_id = ?", id, CurrentUser.user.id).first
|
||||
vote.destroy
|
||||
|
||||
self.reload
|
||||
votes.where(user: voter).destroy_all
|
||||
reload
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
@@ -2,10 +2,14 @@ class PostVote < ActiveRecord::Base
|
||||
class Error < Exception ; end
|
||||
|
||||
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_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
|
||||
|
||||
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)
|
||||
end
|
||||
|
||||
def score=(x)
|
||||
if x == "up"
|
||||
Post.where(:id => post_id).update_all("score = score + #{magnitude}, up_score = up_score + #{magnitude}")
|
||||
write_attribute(:score, magnitude)
|
||||
elsif x == "down"
|
||||
Post.where(:id => post_id).update_all("score = score - #{magnitude}, down_score = down_score - #{magnitude}")
|
||||
write_attribute(:score, -magnitude)
|
||||
def initialize_attributes
|
||||
self.user_id ||= CurrentUser.user.id
|
||||
|
||||
if vote == "up"
|
||||
self.score = magnitude
|
||||
elsif vote == "down"
|
||||
self.score = -magnitude
|
||||
end
|
||||
end
|
||||
|
||||
def initialize_user
|
||||
self.user_id = CurrentUser.user.id
|
||||
def update_post_on_create
|
||||
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
|
||||
|
||||
def update_post_on_destroy
|
||||
@@ -47,7 +55,7 @@ class PostVote < ActiveRecord::Base
|
||||
end
|
||||
|
||||
def magnitude
|
||||
if CurrentUser.is_super_voter?
|
||||
if user.is_super_voter?
|
||||
SuperVoter::MAGNITUDE
|
||||
else
|
||||
1
|
||||
|
||||
@@ -77,6 +77,7 @@ class User < ActiveRecord::Base
|
||||
#after_create :notify_sock_puppets
|
||||
has_many :feedback, :class_name => "UserFeedback", :dependent => :destroy
|
||||
has_many :posts, :foreign_key => "uploader_id"
|
||||
has_many :post_votes
|
||||
has_many :bans, lambda {order("bans.id desc")}
|
||||
has_one :recent_ban, lambda {order("bans.id desc")}, :class_name => "Ban"
|
||||
has_one :api_key
|
||||
|
||||
@@ -1420,45 +1420,34 @@ class PostTest < ActiveSupport::TestCase
|
||||
context "Removing a post from a user's favorites" do
|
||||
setup do
|
||||
@user = FactoryGirl.create(:contributor_user)
|
||||
CurrentUser.user = @user
|
||||
CurrentUser.ip_addr = "127.0.0.1"
|
||||
@post = FactoryGirl.create(:post)
|
||||
@post.add_favorite!(@user)
|
||||
@user.reload
|
||||
end
|
||||
|
||||
teardown do
|
||||
CurrentUser.user = nil
|
||||
CurrentUser.ip_addr = nil
|
||||
end
|
||||
|
||||
should "decrement the user's favorite_count" do
|
||||
assert_difference("@user.favorite_count", -1) do
|
||||
@post.remove_favorite!(@user)
|
||||
@user.reload
|
||||
end
|
||||
end
|
||||
|
||||
should "decrement the post's score for gold users" do
|
||||
@post.remove_favorite!(@user)
|
||||
@post.reload
|
||||
assert_equal(0, @post.score)
|
||||
assert_difference("@post.score", -1) do
|
||||
@post.remove_favorite!(@user)
|
||||
end
|
||||
end
|
||||
|
||||
should "not decrement the post's score for basic users" do
|
||||
@member = FactoryGirl.create(:user)
|
||||
CurrentUser.scoped(@member, "127.0.0.1") do
|
||||
@post.remove_favorite!(@member)
|
||||
end
|
||||
@post.reload
|
||||
assert_equal(1, @post.score)
|
||||
|
||||
assert_no_difference("@post.score") { @post.add_favorite!(@member) }
|
||||
assert_no_difference("@post.score") { @post.remove_favorite!(@member) }
|
||||
end
|
||||
|
||||
should "not decrement the user's favorite_count if the user did not favorite the post" do
|
||||
@post2 = FactoryGirl.create(:post)
|
||||
assert_difference("@user.favorite_count", 0) do
|
||||
assert_no_difference("@user.favorite_count") do
|
||||
@post2.remove_favorite!(@user)
|
||||
@user.reload
|
||||
end
|
||||
end
|
||||
end
|
||||
@@ -1466,16 +1455,9 @@ class PostTest < ActiveSupport::TestCase
|
||||
context "Adding a post to a user's favorites" do
|
||||
setup do
|
||||
@user = FactoryGirl.create(:contributor_user)
|
||||
CurrentUser.user = @user
|
||||
CurrentUser.ip_addr = "127.0.0.1"
|
||||
@post = FactoryGirl.create(:post)
|
||||
end
|
||||
|
||||
teardown do
|
||||
CurrentUser.user = nil
|
||||
CurrentUser.ip_addr = nil
|
||||
end
|
||||
|
||||
should "periodically clean the fav_string" do
|
||||
@post.update_column(:fav_string, "fav:1 fav:1 fav:1")
|
||||
@post.update_column(:fav_count, 3)
|
||||
@@ -1486,24 +1468,19 @@ class PostTest < ActiveSupport::TestCase
|
||||
end
|
||||
|
||||
should "increment the user's favorite_count" do
|
||||
assert_difference("CurrentUser.favorite_count", 1) do
|
||||
assert_difference("@user.favorite_count", 1) do
|
||||
@post.add_favorite!(@user)
|
||||
CurrentUser.reload
|
||||
end
|
||||
end
|
||||
|
||||
should "increment the post's score for gold users" do
|
||||
@post.add_favorite!(@user)
|
||||
@post.reload
|
||||
assert_equal(1, @post.score)
|
||||
end
|
||||
|
||||
should "not increment the post's score for basic users" do
|
||||
@member = FactoryGirl.create(:user)
|
||||
CurrentUser.scoped(@member, "127.0.0.1") do
|
||||
@post.add_favorite!(@member)
|
||||
end
|
||||
@post.reload
|
||||
@post.add_favorite!(@member)
|
||||
assert_equal(0, @post.score)
|
||||
end
|
||||
|
||||
@@ -1529,6 +1506,42 @@ class PostTest < ActiveSupport::TestCase
|
||||
assert(!Favorite.exists?(:user_id => @user.id, :post_id => @post.id))
|
||||
end
|
||||
end
|
||||
|
||||
context "Moving favorites to a parent post" do
|
||||
setup do
|
||||
@parent = FactoryGirl.create(:post)
|
||||
@child = FactoryGirl.create(:post, parent: @parent)
|
||||
|
||||
@user1 = FactoryGirl.create(:user)
|
||||
@gold1 = FactoryGirl.create(:gold_user)
|
||||
@supervoter1 = FactoryGirl.create(:user, is_super_voter: true)
|
||||
|
||||
@child.add_favorite!(@user1)
|
||||
@child.add_favorite!(@gold1)
|
||||
@child.add_favorite!(@supervoter1)
|
||||
@parent.add_favorite!(@supervoter1)
|
||||
|
||||
@child.give_favorites_to_parent
|
||||
@child.reload
|
||||
@parent.reload
|
||||
end
|
||||
|
||||
should "move the favorites" do
|
||||
assert_equal(0, @child.fav_count)
|
||||
assert_equal(0, @child.favorites.count)
|
||||
assert_equal("", @child.fav_string)
|
||||
assert_equal([], @child.favorites.pluck(:user_id))
|
||||
|
||||
assert_equal(3, @parent.fav_count)
|
||||
assert_equal(3, @parent.favorites.count)
|
||||
end
|
||||
|
||||
should "create a vote for each user who can vote" do
|
||||
assert(@parent.votes.where(user: @gold1).exists?)
|
||||
assert(@parent.votes.where(user: @supervoter1).exists?)
|
||||
assert_equal(4, @parent.score)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
context "Pools:" do
|
||||
|
||||
@@ -4,8 +4,9 @@ class PostVoteTest < ActiveSupport::TestCase
|
||||
def setup
|
||||
super
|
||||
|
||||
user = FactoryGirl.create(:user)
|
||||
CurrentUser.user = user
|
||||
@supervoter = FactoryGirl.create(:user, is_super_voter: true)
|
||||
@user = FactoryGirl.create(:user)
|
||||
CurrentUser.user = @user
|
||||
CurrentUser.ip_addr = "127.0.0.1"
|
||||
MEMCACHE.flush_all
|
||||
|
||||
@@ -14,18 +15,47 @@ class PostVoteTest < ActiveSupport::TestCase
|
||||
|
||||
context "Voting for a post" 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)
|
||||
end
|
||||
|
||||
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)
|
||||
end
|
||||
|
||||
should "interpret up as +3 score for a supervoter" do
|
||||
vote = PostVote.create(:user_id => @supervoter.id, :post_id => @post.id, :vote => "up")
|
||||
assert_equal(3, vote.score)
|
||||
end
|
||||
|
||||
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?)
|
||||
end
|
||||
|
||||
should "increase the score of the post" do
|
||||
@post.votes.create(vote: "up")
|
||||
@post.reload
|
||||
|
||||
assert_equal(1, @post.score)
|
||||
assert_equal(1, @post.up_score)
|
||||
end
|
||||
|
||||
should "decrease the score of the post when removed" do
|
||||
@post.votes.create(vote: "up").destroy
|
||||
@post.reload
|
||||
|
||||
assert_equal(0, @post.score)
|
||||
assert_equal(0, @post.up_score)
|
||||
end
|
||||
|
||||
should "upvote by 3 points if the voter is a supervoter" do
|
||||
@post.votes.create(user: @supervoter, vote: "up")
|
||||
@post.reload
|
||||
|
||||
assert_equal(3, @post.score)
|
||||
assert_equal(3, @post.up_score)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
Reference in New Issue
Block a user