discourse
/
discourse-topic-voting
mirror of https://github.com/discourse/discourse-topic-voting.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: release/relcaim votes when topic is closed, clear duplicate and null votes

This commit is contained in:
OsamaSayegh 2018-09-06 16:27:08 +03:00
parent 45c4c3b583
commit 5bcdfaa47b
5 changed files with 155 additions and 75 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
app/controllers/discourse_voting/votes_controller.rb
View File

@ -20,8 +20,8 @@ module DiscourseVoting
unless current_user.reached_voting_limit?
current_user.custom_fields["votes"] = current_user.votes.dup.push(params["topic_id"])
current_user.save
current_user.custom_fields[DiscourseVoting::VOTES] = current_user.votes.dup.push(params["topic_id"]).uniq
current_user.save!
topic.update_vote_count
@ -31,7 +31,7 @@ module DiscourseVoting
obj = {
can_vote: !current_user.reached_voting_limit?,
vote_limit: current_user.vote_limit,
vote_count: topic.custom_fields["vote_count"].to_i,
vote_count: topic.custom_fields[DiscourseVoting::VOTE_COUNT].to_i,
who_voted: who_voted(topic),
alert: current_user.alert_low_votes?,
votes_left: [(current_user.vote_limit - current_user.vote_count), 0].max
@ -45,15 +45,15 @@ module DiscourseVoting
guardian.ensure_can_see!(topic)
current_user.custom_fields["votes"] = current_user.votes.dup - [params["topic_id"].to_s]
current_user.save
current_user.custom_fields[DiscourseVoting::VOTES] = current_user.votes.dup - [params["topic_id"].to_s]
current_user.save!
topic.update_vote_count
obj = {
can_vote: !current_user.reached_voting_limit?,
vote_limit: current_user.vote_limit,
vote_count: topic.custom_fields["vote_count"].to_i,
vote_count: topic.custom_fields[DiscourseVoting::VOTE_COUNT].to_i,
who_voted: who_voted(topic),
votes_left: [(current_user.vote_limit - current_user.vote_count), 0].max
}

60
app/jobs/onceoff/clean_dup_null_votes.rb Normal file
View File

@ -0,0 +1,60 @@
module Jobs
class CleanDupNullVotes < Jobs::Onceoff
def execute_onceoff(args)
# delete duplicate votes
DB.exec("
DELETE FROM user_custom_fields ucf1
USING user_custom_fields ucf2
WHERE ucf1.id < ucf2.id AND
ucf1.user_id = ucf2.user_id AND
ucf1.value = ucf2.value AND
ucf1.name = ucf2.name AND
(ucf1.name IN ('#{DiscourseVoting::VOTES}', '#{DiscourseVoting::VOTES_ARCHIVE}'))
")
# delete votes associated with no topics
DB.exec("
DELETE FROM user_custom_fields ucf
WHERE ucf.value IS NULL
AND ucf.name IN ('#{DiscourseVoting::VOTES}', '#{DiscourseVoting::VOTES_ARCHIVE}')
")
# delete duplicate vote counts for topics
DB.exec("
DELETE FROM topic_custom_fields tcf
USING topic_custom_fields tcf2
WHERE tcf.id < tcf2.id AND
tcf.name = tcf2.name AND
tcf.topic_id = tcf2.topic_id AND
tcf.value = tcf.value AND
tcf.name = '#{DiscourseVoting::VOTE_COUNT}'
")
# insert missing vote counts for topics
DB.exec("
WITH missing_ids AS (
SELECT t.id FROM topics t
JOIN user_custom_fields ucf
ON t.id::text = ucf.value
LEFT JOIN topic_custom_fields tcf
ON t.id = tcf.topic_id
WHERE ucf.name IN ('#{DiscourseVoting::VOTES}', '#{DiscourseVoting::VOTES_ARCHIVE}') AND
tcf.topic_id IS NULL OR tcf.name <> '#{DiscourseVoting::VOTE_COUNT}'
)
INSERT INTO topic_custom_fields (value, topic_id, name, created_at, updated_at)
SELECT '0', id, '#{DiscourseVoting::VOTE_COUNT}', now(), now() FROM missing_ids
")
# correct topics vote counts
DB.exec("
UPDATE topic_custom_fields tcf
SET value = (
SELECT COUNT(*) FROM user_custom_fields ucf
WHERE tcf.topic_id::text = ucf.value AND ucf.name IN ('#{DiscourseVoting::VOTES}', '#{DiscourseVoting::VOTES_ARCHIVE}')
GROUP BY ucf.value
)
WHERE tcf.name = '#{DiscourseVoting::VOTE_COUNT}'
")
end
end
end

109
plugin.rb
View File

@ -16,6 +16,17 @@ Discourse.filters.push(:votes)
Discourse.anonymous_filters.push(:votes)
after_initialize do
module ::DiscourseVoting
VOTES = "votes".freeze
VOTES_ARCHIVE = "votes_archive".freeze
VOTE_COUNT = "vote_count".freeze
class Engine < ::Rails::Engine
isolate_namespace DiscourseVoting
end
end
load File.expand_path('../app/jobs/onceoff/clean_dup_null_votes.rb', __FILE__)
require_dependency 'basic_category_serializer'
class ::BasicCategorySerializer
@ -108,11 +119,7 @@ after_initialize do
require_dependency 'user'
class ::User
def vote_count
if self.custom_fields["votes"] && self.custom_fields["votes"].kind_of?(Array)
user_votes = self.custom_fields["votes"].reject { |v| v.blank? }.length
else
0
end
votes.length
end
def alert_low_votes?
@ -120,19 +127,17 @@ after_initialize do
end
def votes
if self.custom_fields["votes"]
self.custom_fields["votes"]
else
[nil]
end
votes = self.custom_fields[DiscourseVoting::VOTES]
votes = [votes] unless votes.kind_of?(Array)
votes = votes.reject { |v| v.blank? }.uniq
votes
end
def votes_archive
if self.custom_fields["votes_archive"]
return self.custom_fields["votes_archive"]
else
return [nil]
end
archived_votes = self.custom_fields[DiscourseVoting::VOTES_ARCHIVE]
archived_votes = [archived_votes] unless archived_votes.kind_of?(Array)
archived_votes = archived_votes.reject { |v| v.blank? }.uniq
archived_votes
end
def reached_voting_limit?
@ -167,7 +172,7 @@ after_initialize do
end
def vote_count
if count = self.custom_fields["vote_count"]
if count = self.custom_fields[DiscourseVoting::VOTE_COUNT]
# we may have a weird array here, don't explode
# need to fix core to enforce types on fields
count.try(:to_i) || 0
@ -177,25 +182,25 @@ after_initialize do
end
def user_voted(user)
if user && user.custom_fields["votes"]
user.custom_fields["votes"].include? self.id.to_s
if user && user.custom_fields[DiscourseVoting::VOTES]
user.custom_fields[DiscourseVoting::VOTES].include? self.id.to_s
else
false
end
end
def update_vote_count
custom_fields["vote_count"] = UserCustomField.where(value: id.to_s, name: 'votes').count
custom_fields[DiscourseVoting::VOTE_COUNT] = UserCustomField.where("value = :value AND name IN (:keys)", value: id.to_s, keys: [DiscourseVoting::VOTES, DiscourseVoting::VOTES_ARCHIVE]).count
save
save!
end
def who_voted
return nil unless SiteSetting.voting_show_who_voted
User.where("id in (
SELECT user_id FROM user_custom_fields WHERE name IN ('votes', 'votes_archive') AND value = ?
)", id.to_s)
SELECT user_id FROM user_custom_fields WHERE name IN (:keys) AND value = :value
)", value: id.to_s, keys: [DiscourseVoting::VOTES, DiscourseVoting::VOTES_ARCHIVE])
end
end
@ -217,17 +222,17 @@ after_initialize do
require_dependency 'topic_query'
class ::TopicQuery
SORTABLE_MAPPING["votes"] = "custom_fields.vote_count"
SORTABLE_MAPPING["votes"] = "custom_fields.#{::DiscourseVoting::VOTE_COUNT}"
def list_voted_by(user)
create_list(:user_topics) do |topics|
topics.where(id: user.custom_fields["votes"])
topics.where(id: user.custom_fields[DiscourseVoting::VOTES])
end
end
def list_votes
create_list(:votes, unordered: true) do |topics|
topics.joins("left join topic_custom_fields tfv ON tfv.topic_id = topics.id AND tfv.name = 'vote_count'")
topics.joins("left join topic_custom_fields tfv ON tfv.topic_id = topics.id AND tfv.name = '#{DiscourseVoting::VOTE_COUNT}'")
.order("coalesce(tfv.value,'0')::integer desc, topics.bumped_at desc")
end
end
@ -238,46 +243,54 @@ after_initialize do
class VoteRelease < Jobs::Base
def execute(args)
if Topic.find_by(id: args[:topic_id])
UserCustomField.where(name: "votes", value: args[:topic_id]).find_each do |user_field|
if topic = Topic.find_by(id: args[:topic_id])
UserCustomField.where(name: DiscourseVoting::VOTES, value: args[:topic_id]).find_each do |user_field|
user = User.find(user_field.user_id)
user.custom_fields["votes"] = user.votes.dup - [args[:topic_id].to_s]
user.custom_fields["votes_archive"] = user.votes_archive.dup.push(args[:topic_id])
user.save
user.custom_fields[DiscourseVoting::VOTES] = user.votes.dup - [args[:topic_id].to_s]
user.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = user.votes_archive.dup.push(args[:topic_id]).uniq
user.save!
end
topic.update_vote_count
end
end
end
class VoteReclaim < Jobs::Base
def execute(args)
if Topic.find_by(id: args[:topic_id])
UserCustomField.where(name: "votes_archive", value: args[:topic_id]).find_each do |user_field|
if topic = Topic.find_by(id: args[:topic_id])
UserCustomField.where(name: DiscourseVoting::VOTES_ARCHIVE, value: args[:topic_id]).find_each do |user_field|
user = User.find(user_field.user_id)
user.custom_fields["votes"] = user.votes.dup.push(args[:topic_id])
user.custom_fields["votes_archive"] = user.votes_archive.dup - [args[:topic_id].to_s]
user.save
user.custom_fields[DiscourseVoting::VOTES] = user.votes.dup.push(args[:topic_id]).uniq
user.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = user.votes_archive.dup - [args[:topic_id].to_s]
user.save!
end
topic.update_vote_count
end
end
end
end
DiscourseEvent.on(:topic_status_updated) do |topic_id, status, enabled|
DiscourseEvent.on(:topic_status_updated) do |topic, status, enabled|
if (status == 'closed' || status == 'autoclosed' || status == 'archived') && enabled == true
Jobs.enqueue(:vote_release, {topic_id: topic_id})
Jobs.enqueue(:vote_release, { topic_id: topic.id })
end
if (status == 'closed' || status == 'autoclosed' || status == 'archived') && enabled == false
Jobs.enqueue(:vote_reclaim, {topic_id: topic_id})
Jobs.enqueue(:vote_reclaim, { topic_id: topic.id })
end
end
DiscourseEvent.on(:post_edited) do |post, topic_changed|
if topic_changed && SiteSetting.voting_enabled && UserCustomField.where(value: post.topic_id).where("name = 'votes_archive' OR name = 'votes'").exists?
category_id = post.topic.category_id
if Category.can_vote?(category_id)
if topic_changed &&
SiteSetting.voting_enabled &&
UserCustomField.where(
"value = :value AND name in (:keys)",
value: post.topic_id.to_s,
keys: [DiscourseVoting::VOTES, DiscourseVoting::VOTES_ARCHIVE]
).exists?
new_category_id = post.reload.topic.category_id
if Category.can_vote?(new_category_id)
Jobs.enqueue(:vote_reclaim, { topic_id: post.topic_id })
else
Jobs.enqueue(:vote_release, { topic_id: post.topic_id })
@ -292,13 +305,13 @@ after_initialize do
if user.votes.include?(dest.id.to_s)
# User has voted for both +orig+ and +dest+.
# Remove vote for topic +orig+.
user.custom_fields["votes"] = user.votes.dup - [orig.id.to_s]
user.custom_fields[DiscourseVoting::VOTES] = user.votes.dup - [orig.id.to_s]
else
# Change the vote for +orig+ in a vote for +dest+.
user.custom_fields["votes"] = user.votes.map { |vote| vote == orig.id.to_s ? dest.id.to_s : vote }
user.custom_fields[DiscourseVoting::VOTES] = user.votes.map { |vote| vote == orig.id.to_s ? dest.id.to_s : vote }
end
user.save
user.save!
end
end
@ -306,12 +319,6 @@ after_initialize do
dest.update_vote_count
end
module ::DiscourseVoting
class Engine < ::Rails::Engine
isolate_namespace DiscourseVoting
end
end
require File.expand_path(File.dirname(__FILE__) + '/app/controllers/discourse_voting/votes_controller')
DiscourseVoting::Engine.routes.draw do
@ -327,5 +334,5 @@ after_initialize do
get "topics/voted-by/:username" => "list#voted_by", as: "voted_by", constraints: {username: username_route_format}
end
TopicList.preloaded_custom_fields << "vote_count" if TopicList.respond_to? :preloaded_custom_fields
TopicList.preloaded_custom_fields << DiscourseVoting::VOTE_COUNT if TopicList.respond_to? :preloaded_custom_fields
end

2
spec/requests/votes_controller_spec.rb
View File

@ -26,7 +26,7 @@ describe DiscourseVoting::VotesController do
context "when a user has tallyed votes with no topic id" do
before do
user.custom_fields["votes"] = [nil, nil, nil]
user.custom_fields[DiscourseVoting::VOTES] = [nil, nil, nil]
user.save
end

47
spec/voting_spec.rb
View File

@ -19,15 +19,14 @@ describe DiscourseVoting do
end
it 'moves votes when topics are merged' do
users = [user0, user1, user2, user3]
# +user0+ votes +topic0+, +user1+ votes +topic1+ and +user2+ votes both
# topics.
users[0].custom_fields['votes'] = users[0].votes.dup.push(topic0.id.to_s)
users[1].custom_fields['votes'] = users[1].votes.dup.push(topic1.id.to_s)
users[2].custom_fields['votes'] = users[2].votes.dup.push(topic0.id.to_s)
users[2].custom_fields['votes'] = users[2].votes.dup.push(topic1.id.to_s)
users[0].custom_fields[DiscourseVoting::VOTES] = users[0].votes.dup.push(topic0.id.to_s)
users[1].custom_fields[DiscourseVoting::VOTES] = users[1].votes.dup.push(topic1.id.to_s)
users[2].custom_fields[DiscourseVoting::VOTES] = users[2].votes.dup.push(topic0.id.to_s)
users[2].custom_fields[DiscourseVoting::VOTES] = users[2].votes.dup.push(topic1.id.to_s)
users.each { |u| u.save }
[topic0, topic1].each { |t| t.update_vote_count }
@ -35,12 +34,12 @@ describe DiscourseVoting do
DiscourseEvent.trigger(:topic_merged, topic0, topic1)
# Force user refresh.
users.map! { |u| User.find_by(id: u.id) }
users.each(&:reload)
expect(users[0].votes).to eq([nil, topic1.id.to_s])
expect(users[1].votes).to eq([nil, topic1.id.to_s])
expect(users[2].votes).to eq([nil, topic1.id.to_s])
expect(users[3].votes).to eq([nil])
expect(users[0].votes).to eq([topic1.id.to_s])
expect(users[1].votes).to eq([topic1.id.to_s])
expect(users[2].votes).to eq([topic1.id.to_s])
expect(users[3].votes).to eq([])
expect(topic0.vote_count).to eq(0)
expect(topic1.vote_count).to eq(3)
@ -48,7 +47,7 @@ describe DiscourseVoting do
context "when a user has an empty string as the votes custom field" do
before do
user0.custom_fields["votes"] = ""
user0.custom_fields[DiscourseVoting::VOTES] = ""
user0.save
end
@ -57,6 +56,20 @@ describe DiscourseVoting do
end
end
context "when topic status is changed" do
it "enqueues a job for releasing/reclaiming votes" do
DiscourseEvent.on(:topic_status_updated) do |topic|
expect(topic).to be_instance_of(Topic)
end
topic1.update_status('closed', true, Discourse.system_user)
expect(Jobs::VoteRelease.jobs.first["args"].first["topic_id"]).to eq(topic1.id)
topic1.update_status('closed', false, Discourse.system_user)
expect(Jobs::VoteReclaim.jobs.first["args"].first["topic_id"]).to eq(topic1.id)
end
end
context "when a topic is moved to a category" do
let(:admin) { Fabricate(:admin) }
let(:post0) { Fabricate(:post, topic: topic0, post_number: 1) }
@ -70,7 +83,7 @@ describe DiscourseVoting do
it "enqueus a job to reclaim votes if voting is enabled for the new category" do
user = post1.user
user.custom_fields["votes_archive"] = [post1.topic_id, 456456]
user.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = [post1.topic_id, 456456]
user.save!
PostRevisor.new(post1).revise!(admin, category_id: category1.id)
@ -79,13 +92,13 @@ describe DiscourseVoting do
Jobs::VoteReclaim.new.execute(topic_id: post1.topic_id)
user.reload
expect(user.votes).to contain_exactly(post1.topic_id.to_s, nil)
expect([user.votes_archive]).to contain_exactly("456456")
expect(user.votes).to contain_exactly(post1.topic_id.to_s)
expect(user.votes_archive).to contain_exactly("456456")
end
it "enqueus a job to release votes if voting is disabled for the new category" do
user = post0.user
user.custom_fields["votes"] = [post0.topic_id, 456456]
user.custom_fields[DiscourseVoting::VOTES] = [post0.topic_id, 456456]
user.save!
PostRevisor.new(post0).revise!(admin, category_id: category2.id)
@ -94,8 +107,8 @@ describe DiscourseVoting do
Jobs::VoteRelease.new.execute(topic_id: post0.topic_id)
user.reload
expect(user.votes_archive).to contain_exactly(post0.topic_id.to_s, nil)
expect([user.votes]).to contain_exactly("456456")
expect(user.votes_archive).to contain_exactly(post0.topic_id.to_s)
expect(user.votes).to contain_exactly("456456")
end
it "doesn't enqueue a job if the topic has no votes" do