[Votes] Deduplicate Comment/Post vote code

This commit is contained in:
Earlopain 2022-10-08 12:04:17 +02:00
parent e3575a6efe
commit 696ae62ef6
No known key found for this signature in database
GPG Key ID: 6CFB948E15246897
10 changed files with 126 additions and 188 deletions

View File

@ -13,14 +13,14 @@ class CommentVotesController < ApplicationController
end
@comment.reload
render json: {score: @comment.score, our_score: @comment_vote != :need_unvote ? @comment_vote.score : 0}
rescue CommentVote::Error, ActiveRecord::RecordInvalid => x
rescue UserVote::Error, ActiveRecord::RecordInvalid => x
render_expected_error(422, x)
end
def destroy
@comment = Comment.find(params[:comment_id])
VoteManager.comment_unvote!(comment: @comment, user: CurrentUser.user)
rescue CommentVote::Error => x
rescue UserVote::Error => x
render_expected_error(422, x)
end

View File

@ -10,14 +10,14 @@ class PostVotesController < ApplicationController
VoteManager.unvote!(post: @post, user: CurrentUser.user)
end
render json: {score: @post.score, up: @post.up_score, down: @post.down_score, our_score: @post_vote != :need_unvote ? @post_vote.score : 0}
rescue PostVote::Error, ActiveRecord::RecordInvalid => x
rescue UserVote::Error, ActiveRecord::RecordInvalid => x
render_expected_error(422, x)
end
def destroy
@post = Post.find(params[:post_id])
VoteManager.unvote!(post: @post, user: CurrentUser.user)
rescue PostVote::Error => x
rescue UserVote::Error => x
render_expected_error(422, x)
end

View File

@ -4,8 +4,8 @@ class VoteManager
retries = 5
score = score.to_i
begin
raise PostVote::Error.new("Invalid vote") unless [1, -1].include?(score)
raise PostVote::Error.new("You do not have permission to vote") unless user.is_voter?
raise UserVote::Error.new("Invalid vote") unless [1, -1].include?(score)
raise UserVote::Error.new("You do not have permission to vote") unless user.is_voter?
target_isolation = !Rails.env.test? ? { isolation: :serializable } : {}
PostVote.transaction(**target_isolation) do
PostVote.uncached do
@ -13,7 +13,7 @@ class VoteManager
score_modifier = score
old_vote = PostVote.where(user_id: user.id, post_id: post.id).first
if old_vote
raise PostVote::Error.new("Vote is locked") if old_vote.score == 0
raise UserVote::Error.new("Vote is locked") if old_vote.score == 0
if old_vote.score == score
return :need_unvote
else
@ -38,9 +38,9 @@ class VoteManager
rescue ActiveRecord::SerializationFailure
retries -= 1
retry if retries > 0
raise PostVote::Error.new("Failed to vote, please try again later")
raise UserVote::Error.new("Failed to vote, please try again later")
rescue ActiveRecord::RecordNotUnique
raise PostVote::Error.new("You have already voted for this post")
raise UserVote::Error.new("You have already voted for this post")
end
post.update_index
@vote
@ -55,7 +55,7 @@ class VoteManager
post.with_lock do
vote = PostVote.where(user_id: user.id, post_id: post.id).first
return unless vote
raise PostVote::Error.new "You can't remove locked votes" if vote.score == 0 && !force
raise UserVote::Error.new "You can't remove locked votes" if vote.score == 0 && !force
post.votes.where(user: user).delete_all
subtract_vote(post, vote)
post.reload
@ -65,7 +65,7 @@ class VoteManager
rescue ActiveRecord::SerializationFailure
retries -= 1
retry if retries > 0
raise PostVote::Error.new("Failed to unvote, please try again later")
raise UserVote::Error.new("Failed to unvote, please try again later")
end
post.update_index
end
@ -93,8 +93,8 @@ class VoteManager
@vote = nil
score = score.to_i
begin
raise CommentVote::Error.new("Invalid vote") unless [1, -1].include?(score)
raise CommentVote::Error.new("You do not have permission to vote") unless user.is_voter?
raise UserVote::Error.new("Invalid vote") unless [1, -1].include?(score)
raise UserVote::Error.new("You do not have permission to vote") unless user.is_voter?
target_isolation = !Rails.env.test? ? { isolation: :serializable } : {}
CommentVote.transaction(**target_isolation) do
CommentVote.uncached do
@ -102,7 +102,7 @@ class VoteManager
score_modifier = score
old_vote = CommentVote.where(user_id: user.id, comment_id: comment.id).first
if old_vote
raise CommentVote::Error.new("Vote is locked") if old_vote.score == 0
raise UserVote::Error.new("Vote is locked") if old_vote.score == 0
if old_vote.score == score
return :need_unvote
else
@ -118,9 +118,9 @@ class VoteManager
rescue ActiveRecord::SerializationFailure
retries -= 1
retry if retries > 0
raise PostVote::Error.new("Failed to vote, please try again later.")
raise UserVote::Error.new("Failed to vote, please try again later.")
rescue ActiveRecord::RecordNotUnique
raise CommentVote::Error.new("You have already voted for this comment")
raise UserVote::Error.new("You have already voted for this comment")
end
@vote
end
@ -132,7 +132,7 @@ class VoteManager
comment.with_lock do
vote = CommentVote.where(user_id: user.id, comment_id: comment.id).first
return unless vote
raise CommentVote::Error.new("You can't remove locked votes") if vote.score == 0 && !force
raise UserVote::Error.new("You can't remove locked votes") if vote.score == 0 && !force
CommentVote.where(user_id: user.id, comment_id: comment.id).delete_all
Comment.where(id: comment.id).update_all("score = score - #{vote.score}")
end

View File

@ -1,18 +1,6 @@
class CommentVote < ApplicationRecord
class Error < Exception;
end
belongs_to :comment
belongs_to :user
before_validation :initialize_user, :on => :create
validates :user_id, :comment_id, :score, presence: true
# validates :user_id, uniqueness: { :scope => :comment_id, :message => "have already voted for this comment" }
class CommentVote < UserVote
validate :validate_user_can_vote
validate :validate_comment_can_be_voted
validates :score, inclusion: { :in => [-1, 0, 1], :message => "must be 1 or -1" }
scope :for_user, ->(uid) {where("user_id = ?", uid)}
def self.for_comments_and_user(comment_ids, user_id)
return {} unless user_id
@ -37,82 +25,16 @@ class CommentVote < ApplicationRecord
end
end
def is_positive?
score == 1
end
def is_negative?
score == -1
end
def is_locked?
score == 0
end
def initialize_user
self.user_id ||= CurrentUser.user.id
self.user_ip_addr ||= CurrentUser.ip_addr
end
module SearchMethods
def search(params)
q = super
if params[:comment_id].present?
q = q.where("comment_id = ?", params[:comment_id].to_i)
end
if params[:user_name].present?
user_id = User.name_to_id(params[:user_name])
if user_id
q = q.where('user_id = ?', user_id)
else
q = q.none
end
end
if params[:user_id].present?
q = q.where('user_id = ?', params[:user_id].to_i)
end
allow_complex_parameters = (params.keys & %w[comment_id user_name user_id]).any?
if allow_complex_parameters
if params[:timeframe].present?
q = q.where("comment_votes.updated_at >= ?", params[:timeframe].to_i.days.ago)
end
if params[:user_ip_addr].present?
q = q.where("user_ip_addr <<= ?", params[:user_ip_addr])
end
if params[:score].present?
q = q.where("comment_votes.score = ?", params[:score])
end
if params[:comment_creator_name].present?
comment_creator_id = User.name_to_id(params[:comment_creator_name])
if comment_creator_id
q = q.joins(:comment).where("comments.creator_id = ?", comment_creator_id)
else
q = q.none
end
end
if params[:duplicates_only] == "1"
subselect = CommentVote.search(params.except("duplicates_only")).select(:user_ip_addr).group(:user_ip_addr).having("count(user_ip_addr) > 1").reorder("")
q = q.where(user_ip_addr: subselect)
end
end
if params[:order] == "ip_addr" && allow_complex_parameters
q = q.order(:user_ip_addr)
def self.search(params)
q = super
if params[:comment_creator_name].present? && allow_complex_parameters?(params)
comment_creator_id = User.name_to_id(params[:comment_creator_name])
if comment_creator_id
q = q.joins(:comment).where("comments.creator_id = ?", comment_creator_id)
else
q = q.apply_default_order(params)
q = q.none
end
q
end
q
end
extend SearchMethods
end

View File

@ -1,20 +1,5 @@
class PostVote < ApplicationRecord
class Error < Exception ; end
belongs_to :post
belongs_to :user
after_initialize :initialize_attributes, if: :new_record?
class PostVote < UserVote
validate :validate_user_can_vote
validates :post_id, :user_id, :score, presence: true
validates :score, inclusion: { :in => [1, 0, -1] }
scope :for_user, ->(uid) {where("user_id = ?", uid)}
def initialize_attributes
self.user_id ||= CurrentUser.user.id
self.user_ip_addr ||= CurrentUser.ip_addr
end
def validate_user_can_vote
if user.younger_than(3.days) && score == -1
@ -28,57 +13,4 @@ class PostVote < ApplicationRecord
end
true
end
module SearchMethods
def search(params)
q = super
if params[:post_id].present?
q = q.where('post_id = ?', params[:post_id])
end
if params[:user_name].present?
user_id = User.name_to_id(params[:user_name])
if user_id
q = q.where('user_id = ?', user_id)
else
q = q.none
end
end
if params[:user_id].present?
q = q.where('user_id = ?', params[:user_id].to_i)
end
allow_complex_parameters = (params.keys & %w[post_id user_name user_id]).any?
if allow_complex_parameters
if params[:timeframe].present?
q = q.where("updated_at >= ?", params[:timeframe].to_i.days.ago)
end
if params[:user_ip_addr].present?
q = q.where("user_ip_addr <<= ?", params[:user_ip_addr])
end
if params[:score].present?
q = q.where("score = ?", params[:score])
end
if params[:duplicates_only] == "1"
subselect = PostVote.search(params.except("duplicates_only")).select(:user_ip_addr).group(:user_ip_addr).having("count(user_ip_addr) > 1").reorder("")
q = q.where(user_ip_addr: subselect)
end
end
if params[:order] == "ip_addr" && allow_complex_parameters
q = q.order(:user_ip_addr)
else
q = q.apply_default_order(params)
end
q
end
end
extend SearchMethods
end

94
app/models/user_vote.rb Normal file
View File

@ -0,0 +1,94 @@
class UserVote < ApplicationRecord
class Error < Exception; end
self.abstract_class = true
belongs_to :user
validates :score, inclusion: { in: [-1, 0, 1], message: "must be 1 or -1" }
after_initialize :initialize_attributes, if: :new_record?
scope :for_user, ->(uid) { where("user_id = ?", uid) }
def self.inherited(child_class)
super
child_class.class_eval do
belongs_to model_type
end
end
# PostVote => :post
def self.model_type
model_name.singular.delete_suffix("_vote").to_sym
end
def initialize_attributes
self.user_id ||= CurrentUser.user.id
self.user_ip_addr ||= CurrentUser.ip_addr
end
def is_positive?
score == 1
end
def is_negative?
score == -1
end
def is_locked?
score == 0
end
module SearchMethods
def allow_complex_parameters?(params)
(params.keys & ["#{model_type}_id", "user_name", "user_id"]).any?
end
def search(params)
q = super
if params["#{model_type}_id"].present?
q = q.where("#{model_type}_id = ?", params["#{model_type}_id"])
end
if params[:user_name].present?
user_id = User.name_to_id(params[:user_name])
if user_id
q = q.where("user_id = ?", user_id)
else
q = q.none
end
end
if params[:user_id].present?
q = q.where("user_id = ?", params[:user_id].to_i)
end
if allow_complex_parameters?(params)
if params[:timeframe].present?
q = q.where("#{table_name}.updated_at >= ?", params[:timeframe].to_i.days.ago)
end
if params[:user_ip_addr].present?
q = q.where("user_ip_addr <<= ?", params[:user_ip_addr])
end
if params[:score].present?
q = q.where("#{table_name}.score = ?", params[:score])
end
if params[:duplicates_only] == "1"
subselect = search(params.except("duplicates_only")).select(:user_ip_addr).group(:user_ip_addr).having("count(user_ip_addr) > 1").reorder("")
q = q.where(user_ip_addr: subselect)
end
end
if params[:order] == "ip_addr" && allow_complex_parameters?(params)
q = q.order(:user_ip_addr)
else
q = q.apply_default_order(params)
end
q
end
end
extend SearchMethods
end

View File

@ -39,9 +39,8 @@
<td><%= vote.user.email %>
<td title="Signed up at <%= vote.user.created_at.strftime("%c") %>"><%= time_ago_in_words(vote.user.created_at) %> ago
<td>
<% if vote.score == 1 %><span class='greentext'>Up</span>
<% elsif vote.score == 0 %><span class='yellowtext'>Locked</span>
<% elsif vote.score == nil %>Unrecorded
<% if vote.is_positive? %><span class='greentext'>Up</span>
<% elsif vote.is_locked? %><span class='yellowtext'>Locked</span>
<% else %><span class='redtext'>Down</span>
<% end %></td>
<td title="Created at <%= vote.created_at.strftime("%c") %>"><%= time_ago_in_words(vote.created_at) %> ago

View File

@ -36,8 +36,8 @@
<td><%= vote.user.email %>
<td title="Signed up at <%= vote.user.created_at.strftime("%c") %>"><%= time_ago_in_words(vote.user.created_at) %> ago
<td>
<% if vote.score == 1 %><span class='greentext'>Up</span>
<% elsif vote.score == 0 %><span class='yellowtext'>Locked</span>
<% if vote.is_positive? %><span class='greentext'>Up</span>
<% elsif vote.is_locked? %><span class='yellowtext'>Locked</span>
<% else %><span class='redtext'>Down</span>
<% end %></td>
<td title="Created at <%= vote.created_at.strftime("%c") %>"><%= time_ago_in_words(vote.created_at) %> ago

View File

@ -1988,15 +1988,6 @@ class PostTest < ActiveSupport::TestCase
end
context "Voting:" do
# TODO: What the heck is this about?
# should "not allow members to vote" do
# @user = FactoryBot.create(:user)
# @post = FactoryBot.create(:post)
# as_user do
# assert_raises(PostVote::Error) { VoteManager.vote!(user: @user, post: @post, score: 1) }
# end
# end
should "not allow duplicate votes" do
user = FactoryBot.create(:privileged_user)
post = FactoryBot.create(:post)

View File

@ -23,7 +23,7 @@ class PostVoteTest < ActiveSupport::TestCase
end
should "not accept any other scores" do
error = assert_raises(PostVote::Error) { VoteManager.vote!(user: @user, post: @post, score: 'xxx') }
error = assert_raises(UserVote::Error) { VoteManager.vote!(user: @user, post: @post, score: 'xxx') }
assert_equal("Invalid vote", error.message)
end