Skip to content

Commit

Permalink
FIX: move data to separate tables (#52)
Browse files Browse the repository at this point in the history
We are trying to avoid custom tables. Changes:
CategoryCustomField -> DiscourseVoting::CategorySetting # contains infromation if voting is enabled for category
UserCustomField -> DiscourseVoting::Vote # user's votes
TopicCustomField -> DiscourseVoting::VoteCounter # cache count for topics
  • Loading branch information
lis2 committed Aug 17, 2020
1 parent 06adb63 commit 0f0e76f
Show file tree
Hide file tree
Showing 22 changed files with 527 additions and 347 deletions.
12 changes: 5 additions & 7 deletions app/controllers/discourse_voting/votes_controller.rb
Expand Up @@ -16,15 +16,14 @@ def vote
topic_id = params["topic_id"].to_i
topic = Topic.find_by(id: topic_id)

raise Discourse::InvalidAccess if !topic.can_vote? || topic.user_voted(current_user)
raise Discourse::InvalidAccess if !topic.can_vote? || topic.user_voted?(current_user)
guardian.ensure_can_see!(topic)

voted = false

unless current_user.reached_voting_limit?

current_user.custom_fields[DiscourseVoting::VOTES] = current_user.votes.dup.push(topic_id).uniq
current_user.save!
DiscourseVoting::Vote.find_or_create_by(user: current_user, topic_id: topic_id)

topic.update_vote_count
voted = true
Expand All @@ -33,7 +32,7 @@ def vote
obj = {
can_vote: !current_user.reached_voting_limit?,
vote_limit: current_user.vote_limit,
vote_count: topic.custom_fields[DiscourseVoting::VOTE_COUNT].to_i,
vote_count: topic.topic_vote_count&.votes_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
Expand All @@ -48,15 +47,14 @@ def unvote

guardian.ensure_can_see!(topic)

current_user.custom_fields[DiscourseVoting::VOTES] = current_user.votes.dup - [topic_id]
current_user.save!
DiscourseVoting::Vote.destroy_by(user: current_user, topic_id: topic_id)

topic.update_vote_count

obj = {
can_vote: !current_user.reached_voting_limit?,
vote_limit: current_user.vote_limit,
vote_count: topic.custom_fields[DiscourseVoting::VOTE_COUNT].to_i,
vote_count: topic.topic_vote_count&.votes_count&.to_i,
who_voted: who_voted(topic),
votes_left: [(current_user.vote_limit - current_user.vote_count), 0].max
}
Expand Down
116 changes: 50 additions & 66 deletions app/jobs/onceoff/voting_ensure_consistency.rb
Expand Up @@ -3,101 +3,85 @@
module Jobs
class VotingEnsureConsistency < ::Jobs::Onceoff
def execute_onceoff(args)
aliases = {
vote_count: DiscourseVoting::VOTE_COUNT,
votes: DiscourseVoting::VOTES,
votes_archive: DiscourseVoting::VOTES_ARCHIVE,
}

# archive votes to closed or archived or deleted topics
DB.exec(<<~SQL, aliases)
UPDATE user_custom_fields ucf
SET name = :votes_archive
FROM topics t
WHERE ucf.name = :votes
AND (t.closed OR t.archived OR t.deleted_at IS NOT NULL)
AND t.id::text = ucf.value
DB.exec(<<~SQL)
UPDATE discourse_voting_votes
SET archive=true
FROM topics
WHERE topics.id = discourse_voting_votes.topic_id
AND discourse_voting_votes.archive IS NOT TRUE
AND (topics.closed OR topics.archived OR topics.deleted_at IS NOT NULL)
SQL

# un-archive votes to open topics
DB.exec(<<~SQL, aliases)
UPDATE user_custom_fields ucf
SET name = :votes
FROM topics t
WHERE ucf.name = :votes_archive
AND NOT t.closed
AND NOT t.archived
AND t.deleted_at IS NULL
AND t.id::text = ucf.value
DB.exec(<<~SQL)
UPDATE discourse_voting_votes
SET archive=false
FROM topics
WHERE topics.id = discourse_voting_votes.topic_id
AND discourse_voting_votes.archive IS TRUE
AND NOT topics.closed
AND NOT topics.archived
AND topics.deleted_at IS NULL
SQL

# delete duplicate votes
DB.exec(<<~SQL, aliases)
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 (:votes, :votes_archive))
DB.exec(<<~SQL)
DELETE FROM discourse_voting_votes dvv1
USING discourse_voting_votes dvv2
WHERE dvv1.id < dvv2.id AND
dvv1.user_id = dvv2.user_id AND
dvv1.topic_id = dvv2.topic_id AND
dvv1.archive = dvv2.archive
SQL

# delete votes associated with no topics
DB.exec(<<~SQL, aliases)
DELETE FROM user_custom_fields ucf
WHERE ucf.value IS NULL
AND ucf.name IN (:votes, :votes_archive)
DB.exec(<<~SQL)
DELETE FROM discourse_voting_votes
WHERE discourse_voting_votes.topic_id IS NULL
SQL

# delete duplicate vote counts for topics
DB.exec(<<~SQL, aliases)
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 = :vote_count
DB.exec(<<~SQL)
DELETE FROM discourse_voting_topic_vote_count dvtvc
USING discourse_voting_topic_vote_count dvtvc2
WHERE dvtvc.id < dvtvc2.id AND
dvtvc.topic_id = dvtvc2.topic_id AND
dvtvc.votes_count = dvtvc2.votes_count
SQL

# insert missing vote counts for topics
# ensures we have "something" for every topic with votes
DB.exec(<<~SQL, aliases)
DB.exec(<<~SQL)
WITH missing_ids AS (
SELECT DISTINCT t.id FROM topics t
JOIN user_custom_fields ucf ON t.id::text = ucf.value AND
ucf.name IN (:votes, :votes_archive)
LEFT JOIN topic_custom_fields tcf ON t.id = tcf.topic_id
AND tcf.name = :vote_count
WHERE tcf.topic_id IS NULL
JOIN discourse_voting_votes dvv ON t.id = dvv.topic_id
LEFT JOIN discourse_voting_topic_vote_count dvtvc ON t.id = dvtvc.topic_id
WHERE dvtvc.topic_id IS NULL
)
INSERT INTO topic_custom_fields (value, topic_id, name, created_at, updated_at)
SELECT '0', id, :vote_count, now(), now() FROM missing_ids
INSERT INTO discourse_voting_topic_vote_count (votes_count, topic_id, created_at, updated_at)
SELECT '0', id, now(), now() FROM missing_ids
SQL

# remove all superflous vote count custom fields
DB.exec(<<~SQL, aliases)
DELETE FROM topic_custom_fields
WHERE name = :vote_count
AND topic_id IN (
DB.exec(<<~SQL)
DELETE FROM discourse_voting_topic_vote_count
WHERE topic_id IN (
SELECT t1.id FROM topics t1
LEFT JOIN user_custom_fields ucf
ON ucf.value = t1.id::text AND
ucf.name IN (:votes, :votes_archive)
WHERE ucf.id IS NULL
LEFT JOIN discourse_voting_votes dvv
ON dvv.topic_id = t1.id
WHERE dvv.id IS NULL
)
SQL

# correct topics vote counts
DB.exec(<<~SQL, aliases)
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 (:votes, :votes_archive)
GROUP BY ucf.value
DB.exec(<<~SQL)
UPDATE discourse_voting_topic_vote_count dvtvc
SET votes_count = (
SELECT COUNT(*) FROM discourse_voting_votes dvv
WHERE dvtvc.topic_id = dvv.topic_id
GROUP BY dvv.topic_id
)
WHERE tcf.name = :vote_count
SQL
end
end
Expand Down
40 changes: 40 additions & 0 deletions app/models/discourse_voting/category_setting.rb
@@ -0,0 +1,40 @@
# frozen_string_literal: true

module DiscourseVoting
class CategorySetting < ActiveRecord::Base
self.table_name = 'discourse_voting_category_settings'

belongs_to :category

before_create :unarchive_votes
before_destroy :archive_votes
after_save :reset_voting_cache

def unarchive_votes
DB.exec(<<~SQL, { category_id: self.category_id })
UPDATE discourse_voting_votes
SET archive=false
FROM topics
WHERE topics.category_id = :category_id
AND topics.deleted_at is NULL
AND NOT topics.closed
AND NOT topics.archived
AND discourse_voting_votes.topic_id = topics.id
SQL
end

def archive_votes
DB.exec(<<~SQL, { category_id: self.category_id })
UPDATE discourse_voting_votes
SET archive=true
FROM topics
WHERE topics.category_id = :category_id
AND discourse_voting_votes.topic_id = topics.id
SQL
end

def reset_voting_cache
::Category.reset_voting_cache
end
end
end
9 changes: 9 additions & 0 deletions app/models/discourse_voting/topic_vote_count.rb
@@ -0,0 +1,9 @@
# frozen_string_literal: true

module DiscourseVoting
class TopicVoteCount < ActiveRecord::Base
self.table_name = 'discourse_voting_topic_vote_count'

belongs_to :topic
end
end
10 changes: 10 additions & 0 deletions app/models/discourse_voting/vote.rb
@@ -0,0 +1,10 @@
# frozen_string_literal: true

module DiscourseVoting
class Vote < ActiveRecord::Base
self.table_name = 'discourse_voting_votes'

belongs_to :user
belongs_to :topic
end
end
32 changes: 29 additions & 3 deletions assets/javascripts/discourse/initializers/discourse-voting.js.es6
Expand Up @@ -8,16 +8,23 @@ export default {
withPluginApi("0.8.32", api => {
const siteSettings = api.container.lookup("site-settings:main");
if (siteSettings.voting_enabled) {
const pageSearchController = api.container.lookup(
"controller:full-page-search"
);
pageSearchController.sortOrders.pushObject({
name: I18n.t("search.most_votes"),
id: 5,
term: "order:votes"
});

api.addNavigationBarItem({
name: "votes",
before: "top",
customFilter: category => {
return category && category.can_vote;
},
customHref: (category, args) => {
const currentFilterType = (args.filterType || "").split("/").pop();
const path = NavItem.pathFor(currentFilterType, args);

const path = NavItem.pathFor("latest", args);
return `${path}?order=votes`;
},
forceActive: (category, args, router) => {
Expand All @@ -29,6 +36,25 @@ export default {
);
}
});
api.addNavigationBarItem({
name: "my_votes",
before: "top",
customFilter: category => {
return category && category.can_vote && api.getCurrentUser();
},
customHref: (category, args) => {
const path = NavItem.pathFor("latest", args);
return `${path}?state=my_votes`;
},
forceActive: (category, args, router) => {
const queryParams = router.currentRoute.queryParams;
return (
queryParams &&
Object.keys(queryParams).length === 1 &&
queryParams["state"] === "my_votes"
);
}
});
}
});
}
Expand Down
10 changes: 8 additions & 2 deletions assets/javascripts/discourse/widgets/vote-box.js.es6
Expand Up @@ -64,7 +64,10 @@ export default createWidget("vote-box", {
.then(result => {
topic.set("vote_count", result.vote_count);
topic.set("user_voted", true);
this.currentUser.set("votes_exceeded", !result.can_vote);
this.currentUser.setProperties({
votes_exceeded: !result.can_vote,
votes_left: result.votes_left
});
if (result.alert) {
state.votesAlert = result.votes_left;
}
Expand All @@ -87,7 +90,10 @@ export default createWidget("vote-box", {
.then(result => {
topic.set("vote_count", result.vote_count);
topic.set("user_voted", false);
this.currentUser.set("votes_exceeded", !result.can_vote);
this.currentUser.setProperties({
votes_exceeded: !result.can_vote,
votes_left: result.votes_left
});
topic.set("who_voted", result.who_voted);
state.allowClick = true;
this.scheduleRerender();
Expand Down
19 changes: 17 additions & 2 deletions assets/javascripts/discourse/widgets/vote-button.js.es6
@@ -1,7 +1,8 @@
import { createWidget } from "discourse/widgets/widget";
import { h } from "virtual-dom";

export default createWidget("vote-button", {
tagName: "button.btn.btn-primary.vote-button",
tagName: "div",

buildClasses(attrs) {
var buttonClass = "";
Expand Down Expand Up @@ -49,7 +50,21 @@ export default createWidget("vote-button", {
}
}
}
return buttonTitle;

return h(
"button",
{
attributes: {
title: this.currentUser
? I18n.t("voting.votes_left_button_title", {
count: this.currentUser.votes_left
})
: ""
},
className: "btn btn-primary vote-button"
},
buttonTitle
);
},

click() {
Expand Down
12 changes: 12 additions & 0 deletions config/locales/client.en.yml
Expand Up @@ -18,6 +18,9 @@ en:
votes_left:
one: "You have %{count} vote left, see <a href='%{path}'>your votes</a>."
other: "You have %{count} votes left, see <a href='%{path}'>your votes</a>."
votes_left_button_title:
one: "You have %{count} vote left."
other: "You have %{count} votes left."
votes:
one: "%{count} vote"
other: "%{count} votes"
Expand All @@ -29,3 +32,12 @@ en:
votes:
title: "Votes"
help: "topics with the most votes"
my_votes:
title: "My Votes"
help: "topics you voted on"
search:
most_votes: "Most Votes"
advanced:
vote:
count:
label: "Minimum Vote Count"

0 comments on commit 0f0e76f

Please sign in to comment.