From 0f0e76f170c2c6d18e2fbdb31bea2c9cd02a8de2 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Mon, 17 Aug 2020 15:01:33 +1000 Subject: [PATCH] FIX: move data to separate tables (#52) 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 --- .../discourse_voting/votes_controller.rb | 12 +- app/jobs/onceoff/voting_ensure_consistency.rb | 116 ++++----- .../discourse_voting/category_setting.rb | 40 +++ .../discourse_voting/topic_vote_count.rb | 9 + app/models/discourse_voting/vote.rb | 10 + .../initializers/discourse-voting.js.es6 | 32 ++- .../discourse/widgets/vote-box.js.es6 | 10 +- .../discourse/widgets/vote-button.js.es6 | 19 +- config/locales/client.en.yml | 12 + ...18152804_reclaim_from_disabled_category.rb | 6 +- ...eate_discourse_voting_category_settings.rb | 22 ++ ...728222920_create_discourse_voting_votes.rb | 31 +++ ...reate_discourse_voting_topic_vote_count.rb | 23 ++ .../categories_controller_extension.rb | 16 ++ lib/discourse_voting/category_extension.rb | 10 + lib/discourse_voting/topic_extension.rb | 39 +++ lib/discourse_voting/user_extension.rb | 9 + plugin.rb | 244 +++++++----------- spec/ensure_consistency_spec.rb | 31 +-- spec/lib/topic_query_spec.rb | 26 ++ spec/requests/votes_controller_spec.rb | 15 +- spec/voting_spec.rb | 142 +++++----- 22 files changed, 527 insertions(+), 347 deletions(-) create mode 100644 app/models/discourse_voting/category_setting.rb create mode 100644 app/models/discourse_voting/topic_vote_count.rb create mode 100644 app/models/discourse_voting/vote.rb create mode 100644 db/migrate/20200727220143_create_discourse_voting_category_settings.rb create mode 100644 db/migrate/20200728222920_create_discourse_voting_votes.rb create mode 100644 db/migrate/20200729042607_create_discourse_voting_topic_vote_count.rb create mode 100644 lib/discourse_voting/categories_controller_extension.rb create mode 100644 lib/discourse_voting/category_extension.rb create mode 100644 lib/discourse_voting/topic_extension.rb create mode 100644 lib/discourse_voting/user_extension.rb create mode 100644 spec/lib/topic_query_spec.rb diff --git a/app/controllers/discourse_voting/votes_controller.rb b/app/controllers/discourse_voting/votes_controller.rb index cacc2df..c47b18b 100644 --- a/app/controllers/discourse_voting/votes_controller.rb +++ b/app/controllers/discourse_voting/votes_controller.rb @@ -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 @@ -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 @@ -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 } diff --git a/app/jobs/onceoff/voting_ensure_consistency.rb b/app/jobs/onceoff/voting_ensure_consistency.rb index ece3652..5f4c319 100644 --- a/app/jobs/onceoff/voting_ensure_consistency.rb +++ b/app/jobs/onceoff/voting_ensure_consistency.rb @@ -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 diff --git a/app/models/discourse_voting/category_setting.rb b/app/models/discourse_voting/category_setting.rb new file mode 100644 index 0000000..75dba4a --- /dev/null +++ b/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 diff --git a/app/models/discourse_voting/topic_vote_count.rb b/app/models/discourse_voting/topic_vote_count.rb new file mode 100644 index 0000000..14a0830 --- /dev/null +++ b/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 diff --git a/app/models/discourse_voting/vote.rb b/app/models/discourse_voting/vote.rb new file mode 100644 index 0000000..e148825 --- /dev/null +++ b/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 diff --git a/assets/javascripts/discourse/initializers/discourse-voting.js.es6 b/assets/javascripts/discourse/initializers/discourse-voting.js.es6 index 7c791ca..864cc42 100644 --- a/assets/javascripts/discourse/initializers/discourse-voting.js.es6 +++ b/assets/javascripts/discourse/initializers/discourse-voting.js.es6 @@ -8,6 +8,15 @@ 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", @@ -15,9 +24,7 @@ export default { 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) => { @@ -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" + ); + } + }); } }); } diff --git a/assets/javascripts/discourse/widgets/vote-box.js.es6 b/assets/javascripts/discourse/widgets/vote-box.js.es6 index ce63d01..61e376f 100644 --- a/assets/javascripts/discourse/widgets/vote-box.js.es6 +++ b/assets/javascripts/discourse/widgets/vote-box.js.es6 @@ -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; } @@ -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(); diff --git a/assets/javascripts/discourse/widgets/vote-button.js.es6 b/assets/javascripts/discourse/widgets/vote-button.js.es6 index 7be1f1a..6a1c90f 100644 --- a/assets/javascripts/discourse/widgets/vote-button.js.es6 +++ b/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 = ""; @@ -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() { diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index c2b46e2..2d9f328 100755 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -18,6 +18,9 @@ en: votes_left: one: "You have %{count} vote left, see your votes." other: "You have %{count} votes left, see your votes." + votes_left_button_title: + one: "You have %{count} vote left." + other: "You have %{count} votes left." votes: one: "%{count} vote" other: "%{count} votes" @@ -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" diff --git a/db/migrate/20190718152804_reclaim_from_disabled_category.rb b/db/migrate/20190718152804_reclaim_from_disabled_category.rb index 407edf5..8353a9e 100644 --- a/db/migrate/20190718152804_reclaim_from_disabled_category.rb +++ b/db/migrate/20190718152804_reclaim_from_disabled_category.rb @@ -3,9 +3,9 @@ class ReclaimFromDisabledCategory < ActiveRecord::Migration[5.2] def up aliases = { - votes: DiscourseVoting::VOTES, - votes_archive: DiscourseVoting::VOTES_ARCHIVE, - voting_enabled: DiscourseVoting::VOTING_ENABLED + votes: "votes", + votes_archive: "votes_archive", + voting_enabled: "enable_topic_voting" } # archive votes in non-voting categories diff --git a/db/migrate/20200727220143_create_discourse_voting_category_settings.rb b/db/migrate/20200727220143_create_discourse_voting_category_settings.rb new file mode 100644 index 0000000..1137e5e --- /dev/null +++ b/db/migrate/20200727220143_create_discourse_voting_category_settings.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class CreateDiscourseVotingCategorySettings < ActiveRecord::Migration[6.0] + def up + create_table :discourse_voting_category_settings do |t| + t.integer :category_id + t.timestamps + end + add_index :discourse_voting_category_settings, :category_id, unique: true + + DB.exec <<~SQL + INSERT INTO discourse_voting_category_settings(category_id, created_at, updated_at) + SELECT category_id, created_at, updated_at + FROM category_custom_fields + WHERE name = 'enable_topic_voting' and value = 'true' + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20200728222920_create_discourse_voting_votes.rb b/db/migrate/20200728222920_create_discourse_voting_votes.rb new file mode 100644 index 0000000..b5b8a71 --- /dev/null +++ b/db/migrate/20200728222920_create_discourse_voting_votes.rb @@ -0,0 +1,31 @@ +# frozen_string_literal: true + +class CreateDiscourseVotingVotes < ActiveRecord::Migration[6.0] + def up + create_table :discourse_voting_votes do |t| + t.integer :topic_id + t.integer :user_id + t.boolean :archive, default: false + t.timestamps + end + add_index :discourse_voting_votes, [:user_id, :topic_id], unique: true + + DB.exec <<~SQL + INSERT INTO discourse_voting_votes(topic_id, user_id, archive, created_at, updated_at) + SELECT value::integer, user_id, 'false', created_at, updated_at + FROM user_custom_fields + WHERE name = 'votes' + SQL + + DB.exec <<~SQL + INSERT INTO discourse_voting_votes(topic_id, user_id, archive, created_at, updated_at) + SELECT value::integer, user_id, 'true', created_at, updated_at + FROM user_custom_fields + WHERE name = 'votes_archive' + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20200729042607_create_discourse_voting_topic_vote_count.rb b/db/migrate/20200729042607_create_discourse_voting_topic_vote_count.rb new file mode 100644 index 0000000..4a24625 --- /dev/null +++ b/db/migrate/20200729042607_create_discourse_voting_topic_vote_count.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class CreateDiscourseVotingTopicVoteCount < ActiveRecord::Migration[6.0] + def up + create_table :discourse_voting_topic_vote_count do |t| + t.integer :topic_id + t.integer :votes_count + t.timestamps + end + add_index :discourse_voting_topic_vote_count, :topic_id, unique: true + + DB.exec <<~SQL + INSERT INTO discourse_voting_topic_vote_count(topic_id, votes_count, created_at, updated_at) + SELECT topic_id::integer, value::integer, created_at, updated_at + FROM topic_custom_fields + WHERE name = 'vote_count' + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/discourse_voting/categories_controller_extension.rb b/lib/discourse_voting/categories_controller_extension.rb new file mode 100644 index 0000000..f74fc1e --- /dev/null +++ b/lib/discourse_voting/categories_controller_extension.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module DiscourseVoting + module CategoriesControllerExtension + def category_params + @vote_enabled ||= params[:custom_fields] && params[:custom_fields].delete(:enable_topic_voting) == "true" + category_params = super + if @vote_enabled + category_params[:category_setting_attributes] = {} + elsif @category&.category_setting + category_params[:category_setting_attributes] = { id: @category.category_setting.id, _destroy: '1' } + end + category_params + end + end +end diff --git a/lib/discourse_voting/category_extension.rb b/lib/discourse_voting/category_extension.rb new file mode 100644 index 0000000..7de7dbe --- /dev/null +++ b/lib/discourse_voting/category_extension.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module DiscourseVoting + module CategoryExtension + def self.prepended(base) + base.has_one :category_setting, class_name: 'DiscourseVoting::CategorySetting', dependent: :destroy + base.accepts_nested_attributes_for :category_setting, allow_destroy: true + end + end +end diff --git a/lib/discourse_voting/topic_extension.rb b/lib/discourse_voting/topic_extension.rb new file mode 100644 index 0000000..cb386e4 --- /dev/null +++ b/lib/discourse_voting/topic_extension.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module DiscourseVoting + module TopicExtension + def self.prepended(base) + base.has_one :topic_vote_count, class_name: 'DiscourseVoting::TopicVoteCount', dependent: :destroy + base.has_many :votes, class_name: 'DiscourseVoting::Vote', dependent: :destroy + base.attribute :current_user_voted + end + + def can_vote? + SiteSetting.voting_enabled && Category.can_vote?(category_id) && category.topic_id != id + end + + def vote_count + self.topic_vote_count&.votes_count.to_i + end + + def user_voted?(user) + if self.current_user_voted + self.current_user_voted == 1 + else + votes.map(&:user_id).include?(user.id) + end + end + + def update_vote_count + count = self.votes.count + + topic_vote_count = self.topic_vote_count || DiscourseVoting::TopicVoteCount.new(topic: self) + topic_vote_count.update!(votes_count: count) + end + + def who_voted + return nil unless SiteSetting.voting_show_who_voted + self.votes.map(&:user) + end + end +end diff --git a/lib/discourse_voting/user_extension.rb b/lib/discourse_voting/user_extension.rb new file mode 100644 index 0000000..d8814a1 --- /dev/null +++ b/lib/discourse_voting/user_extension.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module DiscourseVoting + module UserExtension + def self.prepended(base) + base.has_many :votes, class_name: 'DiscourseVoting::Vote' + end + end +end diff --git a/plugin.rb b/plugin.rb index decef7b..041e9bb 100755 --- a/plugin.rb +++ b/plugin.rb @@ -19,24 +19,26 @@ after_initialize do module ::DiscourseVoting - VOTES = "votes".freeze - VOTES_ARCHIVE = "votes_archive".freeze - VOTE_COUNT = "vote_count".freeze - VOTING_ENABLED = "enable_topic_voting" - class Engine < ::Rails::Engine isolate_namespace DiscourseVoting end end - User.register_custom_field_type(::DiscourseVoting::VOTES, [:integer]) - User.register_custom_field_type(::DiscourseVoting::VOTES_ARCHIVE, [:integer]) - Topic.register_custom_field_type(::DiscourseVoting::VOTE_COUNT, :integer) - Category.register_custom_field_type(::DiscourseVoting::VOTING_ENABLED, :boolean) - load File.expand_path('../app/jobs/onceoff/voting_ensure_consistency.rb', __FILE__) + load File.expand_path('../app/models/discourse_voting/category_setting.rb', __FILE__) + load File.expand_path('../app/models/discourse_voting/topic_vote_count.rb', __FILE__) + load File.expand_path('../app/models/discourse_voting/vote.rb', __FILE__) + load File.expand_path('../lib/discourse_voting/categories_controller_extension.rb', __FILE__) + load File.expand_path('../lib/discourse_voting/category_extension.rb', __FILE__) + load File.expand_path('../lib/discourse_voting/topic_extension.rb', __FILE__) + load File.expand_path('../lib/discourse_voting/user_extension.rb', __FILE__) reloadable_patch do |plugin| + CategoriesController.class_eval { prepend DiscourseVoting::CategoriesControllerExtension } + Category.class_eval { prepend DiscourseVoting::CategoryExtension } + Topic.class_eval { prepend DiscourseVoting::TopicExtension } + User.class_eval { prepend DiscourseVoting::UserExtension } + require_dependency 'post_serializer' class ::PostSerializer attributes :can_vote @@ -63,18 +65,36 @@ def vote_count end def user_voted - if scope.user - object.topic.user_voted(scope.user) - else - false - end + scope.user ? object.topic.user_voted?(scope.user) : false end end + TopicQuery.results_filter_callbacks << ->(_type, result, user, options) { + result = result.includes(:topic_vote_count) + result = result.select("*, COALESCE((SELECT 1 FROM discourse_voting_votes WHERE user_id = #{user.id} AND topic_id = topics.id), 0) AS current_user_voted") if user + result + } + + TopicQuery.results_filter_callbacks << ->(_type, result, _user, options) { + return result if options[:order] != "votes" + sort_dir = (options[:ascending] == "true") ? "ASC" : "DESC" + result + .joins("LEFT JOIN discourse_voting_topic_vote_count ON discourse_voting_topic_vote_count.topic_id = topics.id") + .reorder("COALESCE(discourse_voting_topic_vote_count.votes_count,'0')::integer #{sort_dir}") + } + + TopicQuery.results_filter_callbacks << ->(_type, result, user, options) { + return result if options[:state] != "my_votes" || !user + result.joins("INNER JOIN discourse_voting_votes ON discourse_voting_votes.topic_id = topics.id AND discourse_voting_votes.user_id = #{user.id}") + } + + add_to_serializer(:category, :custom_fields) do + object.custom_fields.merge(enable_topic_voting: DiscourseVoting::CategorySetting.find_by(category_id: object.id).present?) + end add_to_serializer(:topic_list_item, :vote_count) { object.vote_count } add_to_serializer(:topic_list_item, :can_vote) { object.can_vote? } add_to_serializer(:topic_list_item, :user_voted) { - object.user_voted(scope.user) if scope.user + object.user_voted?(scope.user) if scope.user } add_to_serializer(:basic_category, :can_vote, false) do @@ -85,15 +105,19 @@ def user_voted Category.can_vote?(object.id) end + register_search_advanced_filter(/^min_vote_count:(\d+)$/) do |posts, match| + posts.where("(SELECT votes_count FROM discourse_voting_topic_vote_count WHERE discourse_voting_topic_vote_count.topic_id = posts.topic_id) >= ?", match.to_i) + end + + register_search_advanced_order(:votes) do |posts| + posts.reorder("COALESCE((SELECT dvtvc.votes_count FROM discourse_voting_topic_vote_count dvtvc WHERE dvtvc.topic_id = subquery.topic_id), 0) DESC") + end + class ::Category def self.reset_voting_cache @allowed_voting_cache["allowed"] = begin - Set.new( - CategoryCustomField - .where(name: ::DiscourseVoting::VOTING_ENABLED, value: "true") - .pluck(:category_id) - ) + DiscourseVoting::CategorySetting.pluck(:category_id) end end @@ -109,80 +133,29 @@ def self.can_vote?(category_id) end after_save :reset_voting_cache - before_save :reclaim_release_votes protected def reset_voting_cache ::Category.reset_voting_cache end - - def reclaim_release_votes - return if self.new_record? - return if !SiteSetting.voting_enabled - - aliases = { - votes: DiscourseVoting::VOTES, - votes_archive: DiscourseVoting::VOTES_ARCHIVE, - category_id: id - } - - was_enabled = CategoryCustomField.where( - "name = :name AND value similar to :value AND category_id = :id", - id: id, - name: ::DiscourseVoting::VOTING_ENABLED, - value: '(t|T|1)%' - ).exists? - - is_enabled = custom_fields[::DiscourseVoting::VOTING_ENABLED] - - if !was_enabled && is_enabled - # Unarchive all votes in the category - 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 - AND t.category_id = :category_id - SQL - elsif was_enabled && !is_enabled - # Archive all votes in category - DB.exec(<<~SQL, aliases) - UPDATE user_custom_fields ucf - SET name = :votes_archive - FROM topics t - WHERE ucf.name = :votes - AND t.id::text = ucf.value - AND t.category_id = :category_id - SQL - end - end end require_dependency 'user' class ::User def vote_count - votes.length + topics_with_vote.length end def alert_low_votes? (vote_limit - vote_count) <= SiteSetting.voting_alert_votes_left end - def votes - votes = self.custom_fields[DiscourseVoting::VOTES] || [] - # "" can be in there sometimes, it gets turned into a 0 - votes = votes.reject { |v| v == 0 }.uniq - votes + def topics_with_vote + self.votes.where(archive: false) end - def votes_archive - archived_votes = self.custom_fields[DiscourseVoting::VOTES_ARCHIVE] || [] - archived_votes = archived_votes.reject { |v| v == 0 }.uniq - archived_votes + def topics_with_archived_vote + self.votes.where(archive: true) end def reached_voting_limit? @@ -192,12 +165,11 @@ def reached_voting_limit? def vote_limit SiteSetting.public_send("voting_tl#{self.trust_level}_vote_limit") end - end require_dependency 'current_user_serializer' class ::CurrentUserSerializer - attributes :votes_exceeded, :vote_count + attributes :votes_exceeded, :vote_count, :votes_left def votes_exceeded object.reached_voting_limit? @@ -207,50 +179,9 @@ def vote_count object.vote_count end - end - - require_dependency 'topic' - class ::Topic - - def can_vote? - SiteSetting.voting_enabled && Category.can_vote?(category_id) && category.topic_id != id + def votes_left + [object.vote_limit - object.vote_count, 0].max end - - def 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 - else - 0 if self.can_vote? - end - end - - def user_voted(user) - if user && user.custom_fields[DiscourseVoting::VOTES] - user.custom_fields[DiscourseVoting::VOTES].include? self.id - else - false - end - end - - def update_vote_count - count = - UserCustomField.where("value = :value AND name IN (:keys)", - value: id.to_s, keys: [DiscourseVoting::VOTES, DiscourseVoting::VOTES_ARCHIVE]).count - - custom_fields[DiscourseVoting::VOTE_COUNT] = count - save_custom_fields - 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 (:keys) AND value = :value - )", value: id.to_s, keys: [DiscourseVoting::VOTES, DiscourseVoting::VOTES_ARCHIVE]) - end - end require_dependency 'list_controller' @@ -270,18 +201,18 @@ def voted_by require_dependency 'topic_query' class ::TopicQuery - 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[DiscourseVoting::VOTES]) + topics + .joins("INNER JOIN discourse_voting_votes ON discourse_voting_votes.topic_id = topics.id") + .where("discourse_voting_votes.user_id = ?", user.id) 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 = '#{DiscourseVoting::VOTE_COUNT}'") - .order("coalesce(tfv.value,'0')::integer desc, topics.bumped_at desc") + topics.joins("LEFT JOIN discourse_voting_topic_vote_count dvtvc ON dvtvc.topic_id = topics.id") + .order("COALESCE(dvtvc.votes_count,'0')::integer DESC, topics.bumped_at DESC") end end end @@ -292,13 +223,23 @@ module ::Jobs class VoteRelease < ::Jobs::Base def execute(args) if topic = Topic.with_deleted.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[DiscourseVoting::VOTES] = user.votes.dup - [args[:topic_id]] - user.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = user.votes_archive.dup.push(args[:topic_id]).uniq - user.save! - end + votes = DiscourseVoting::Vote.where(topic_id: args[:topic_id]) + votes.update_all(archive: true) + topic.update_vote_count + + return if args[:trashed] + + votes.find_each do |vote| + Notification.create!(user_id: vote.user_id, + notification_type: Notification.types[:votes_released], + topic_id: vote.topic_id, + data: { message: "votes_released", + title: "votes_released" }.to_json) + rescue + # If one notifcation crash, inform others + end + end end end @@ -306,13 +247,10 @@ def execute(args) class VoteReclaim < ::Jobs::Base def execute(args) if topic = Topic.with_deleted.find_by(id: args[:topic_id]) - UserCustomField.where(name: DiscourseVoting::VOTES_ARCHIVE, value: topic.id).find_each do |user_field| - user = User.find(user_field.user_id) - user.custom_fields[DiscourseVoting::VOTES] = user.votes.dup.push(topic.id).uniq - user.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = user.votes_archive.dup - [topic.id] - user.save! + ActiveRecord::Base.transaction do + DiscourseVoting::Vote.where(topic_id: args[:topic_id]).update_all(archive: false) + topic.update_vote_count end - topic.update_vote_count end end end @@ -331,7 +269,7 @@ def execute(args) end DiscourseEvent.on(:topic_trashed) do |topic| - Jobs.enqueue(:vote_release, topic_id: topic.id) if !topic.closed && !topic.archived + Jobs.enqueue(:vote_release, topic_id: topic.id, trashed: true) if !topic.closed && !topic.archived end DiscourseEvent.on(:topic_recovered) do |topic| @@ -341,11 +279,7 @@ def execute(args) DiscourseEvent.on(:post_edited) do |post, topic_changed| 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? + DiscourseVoting::Vote.exists?(topic_id: post.topic_id) new_category_id = post.reload.topic.category_id if Category.can_vote?(new_category_id) Jobs.enqueue(:vote_reclaim, topic_id: post.topic_id) @@ -361,27 +295,25 @@ def execute(args) if orig.who_voted.present? && orig.closed orig.who_voted.each do |user| - if user.votes.include?(orig.id) - if user.votes.include?(dest.id) + if user.topics_with_vote.pluck(:topic_id).include?(orig.id) + if user.topics_with_vote.pluck(:topic_id).include?(dest.id) duplicated_votes += 1 - user.custom_fields[DiscourseVoting::VOTES] = user.votes.dup - [orig.id] + user.votes.destroy_by(topic_id: orig.id, archive: false) else - user.custom_fields[DiscourseVoting::VOTES] = user.votes.map { |vote| vote == orig.id ? dest.id : vote } + user.votes.find_by(topic_id: orig.id, archive: false).update!(topic_id: dest.id) moved_votes += 1 end - elsif user.votes_archive.include?(orig.id) - if user.votes_archive.include?(dest.id) + elsif user.topics_with_archived_vote.pluck(:topic_id).include?(orig.id) + if user.topics_with_archived_vote.pluck(:topic_id).include?(dest.id) duplicated_votes += 1 - user.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = user.votes_archive.dup - [orig.id] + user.votes.destroy_by(topic_id: orig.id, user_id: user.id, archive: true) else - user.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = user.votes_archive.map { |vote| vote == orig.id ? dest.id : vote } + user.votes.find_by(topic_id: orig.id, user_id: user.id, archive: true).update!(topic_id: dest.id) moved_votes += 1 end else next end - - user.save_custom_fields end end @@ -411,6 +343,4 @@ def execute(args) username_route_format = defined?(RouteFormat) ? RouteFormat.username : USERNAME_ROUTE_FORMAT get "topics/voted-by/:username" => "list#voted_by", as: "voted_by", constraints: { username: username_route_format } end - - TopicList.preloaded_custom_fields << DiscourseVoting::VOTE_COUNT if TopicList.respond_to? :preloaded_custom_fields end diff --git a/spec/ensure_consistency_spec.rb b/spec/ensure_consistency_spec.rb index 317b4f7..a55d762 100644 --- a/spec/ensure_consistency_spec.rb +++ b/spec/ensure_consistency_spec.rb @@ -8,44 +8,31 @@ user2 = Fabricate(:user) no_vote_topic = Fabricate(:topic) - no_vote_topic.custom_fields[DiscourseVoting::VOTE_COUNT] = "10" - no_vote_topic.custom_fields["random1"] = "random" - no_vote_topic.save_custom_fields + DiscourseVoting::TopicVoteCount.create!(topic: no_vote_topic, votes_count: 10) one_vote_topic = Fabricate(:topic) - one_vote_topic.custom_fields[DiscourseVoting::VOTE_COUNT] = "10" - one_vote_topic.custom_fields["random2"] = "random" - one_vote_topic.save_custom_fields + DiscourseVoting::TopicVoteCount.create!(topic: one_vote_topic, votes_count: 10) two_vote_topic = Fabricate(:topic) - two_vote_topic.custom_fields["random3"] = "random" - two_vote_topic.save_custom_fields # one vote - UserCustomField.create!(user_id: user.id, name: DiscourseVoting::VOTES_ARCHIVE, value: one_vote_topic.id) + DiscourseVoting::Vote.create!(user: user, topic: one_vote_topic, archive: true) # two votes - UserCustomField.create!(user_id: user.id, name: DiscourseVoting::VOTES_ARCHIVE, value: two_vote_topic.id) - UserCustomField.create!(user_id: user2.id, name: DiscourseVoting::VOTES, value: two_vote_topic.id) + DiscourseVoting::Vote.create!(user: user, topic: two_vote_topic, archive: true) + DiscourseVoting::Vote.create!(user: user2, topic: two_vote_topic) subject.execute_onceoff(nil) no_vote_topic.reload - expect(no_vote_topic.custom_fields["random1"]).to eq("random") - expect(user.reload.custom_fields).to eq("votes" => [one_vote_topic.id, two_vote_topic.id]) - expect(user2.reload.custom_fields).to eq("votes" => [two_vote_topic.id]) + expect(DiscourseVoting::Vote.where(user: user).pluck(:topic_id)).to eq([one_vote_topic.id, two_vote_topic.id]) + expect(DiscourseVoting::Vote.where(user: user2).pluck(:topic_id)).to eq([two_vote_topic.id]) one_vote_topic.reload - expect(one_vote_topic.custom_fields["vote_count"]).to eq(1) - expect(one_vote_topic.custom_fields["random2"]).to eq("random") + expect(one_vote_topic.topic_vote_count.votes_count).to eq(1) two_vote_topic.reload - expect(two_vote_topic.custom_fields["vote_count"]).to eq(2) - expect(two_vote_topic.custom_fields["random3"]).to eq("random") - - expect(no_vote_topic.reload.custom_fields).to eq("random1" => "random") - expect(one_vote_topic.reload.custom_fields).to eq("vote_count" => 1, "random2" => "random") - expect(two_vote_topic.reload.custom_fields).to eq("vote_count" => 2, "random3" => "random") + expect(two_vote_topic.topic_vote_count.votes_count).to eq(2) end end diff --git a/spec/lib/topic_query_spec.rb b/spec/lib/topic_query_spec.rb new file mode 100644 index 0000000..80c1b26 --- /dev/null +++ b/spec/lib/topic_query_spec.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe TopicQuery do + fab!(:user0) { Fabricate(:user) } + fab!(:category1) { Fabricate(:category) } + fab!(:topic0) { Fabricate(:topic, category: category1) } + fab!(:topic1) { Fabricate(:topic, category: category1) } + fab!(:category_setting) { DiscourseVoting::CategorySetting.create!(category_id: category1) } + fab!(:vote) { DiscourseVoting::Vote.create!(topic_id: topic1.id, user_id: user0.id) } + fab!(:topic_vote_count) { DiscourseVoting::TopicVoteCount.create!(topic_id: topic1.id, votes_count: 1) } + + before do + SiteSetting.voting_enabled = true + SiteSetting.voting_show_who_voted = true + end + + it "order topic by votes" do + expect(TopicQuery.new(user0, { order: 'votes' }).list_latest.topics.map(&:id)).to eq([topic1.id, topic0.id]) + end + + it "returns topics voted by user" do + expect(TopicQuery.new(user0, { state: 'my_votes' }).list_latest.topics.map(&:id)).to eq([topic1.id]) + end +end diff --git a/spec/requests/votes_controller_spec.rb b/spec/requests/votes_controller_spec.rb index 33eabfa..8c0374c 100644 --- a/spec/requests/votes_controller_spec.rb +++ b/spec/requests/votes_controller_spec.rb @@ -9,7 +9,7 @@ let(:topic) { Fabricate(:topic, category_id: category.id) } before do - CategoryCustomField.create!(category_id: category.id, name: "enable_topic_voting", value: "true") + DiscourseVoting::CategorySetting.create!(category: category) Category.reset_voting_cache SiteSetting.voting_show_who_voted = true SiteSetting.voting_enabled = true @@ -47,17 +47,4 @@ expect(topic.reload.vote_count).to eq(0) expect(user.reload.vote_count).to eq(0) end - - context "when a user has tallyed votes with no topic id" do - before do - user.custom_fields[DiscourseVoting::VOTES] = [nil, nil, nil] - user.save - end - - it "removes extra votes" do - post "/voting/vote.json", params: { topic_id: topic.id } - expect(response.status).to eq(200) - expect(user.reload.vote_count).to eq (1) - end - end end diff --git a/spec/voting_spec.rb b/spec/voting_spec.rb index 02370f7..f4535f1 100644 --- a/spec/voting_spec.rb +++ b/spec/voting_spec.rb @@ -9,6 +9,7 @@ let(:user2) { Fabricate(:user) } let(:user3) { Fabricate(:user) } let(:user4) { Fabricate(:user) } + let(:user5) { Fabricate(:user) } let(:category1) { Fabricate(:category) } let(:category2) { Fabricate(:category) } @@ -27,14 +28,13 @@ expect(user0.reached_voting_limit?).to eq(false) - user0.custom_fields["votes"] = [topic0.id.to_s] - user0.save! + DiscourseVoting::Vote.create!(user: user0, topic: topic0) expect(user0.reached_voting_limit?).to eq(true) end context "with two topics" do - let(:users) { [user0, user1, user2, user3, user4] } + let(:users) { [user0, user1, user2, user3, user4, user5] } before do Fabricate(:post, topic: topic0, user: user0) @@ -42,12 +42,13 @@ # +user0+ votes +topic0+, +user1+ votes +topic1+ and +user2+ votes both # topics. - users[0].custom_fields[DiscourseVoting::VOTES] = (users[0].custom_fields[DiscourseVoting::VOTES].dup || []).push(topic0.id.to_s) - users[1].custom_fields[DiscourseVoting::VOTES] = (users[1].custom_fields[DiscourseVoting::VOTES].dup || []).push(topic1.id.to_s) - users[2].custom_fields[DiscourseVoting::VOTES] = (users[2].custom_fields[DiscourseVoting::VOTES].dup || []).push(topic0.id.to_s) - users[2].custom_fields[DiscourseVoting::VOTES] = (users[2].custom_fields[DiscourseVoting::VOTES].dup || []).push(topic1.id.to_s) - users[4].custom_fields[DiscourseVoting::VOTES_ARCHIVE] = (users[4].custom_fields[DiscourseVoting::VOTES_ARCHIVE].dup || []).push(topic0.id.to_s) - users.each { |u| u.save! } + DiscourseVoting::Vote.create!(user: users[0], topic: topic0) + DiscourseVoting::Vote.create!(user: users[1], topic: topic1) + DiscourseVoting::Vote.create!(user: users[2], topic: topic0) + DiscourseVoting::Vote.create!(user: users[2], topic: topic1) + DiscourseVoting::Vote.create!(user: users[4], topic: topic0, archive: true) + DiscourseVoting::Vote.create!(user: users[5], topic: topic0, archive: true) + DiscourseVoting::Vote.create!(user: users[5], topic: topic1, archive: true) [topic0, topic1].each { |t| t.update_vote_count } end @@ -56,56 +57,53 @@ topic0.move_posts(Discourse.system_user, topic0.posts.pluck(:id), destination_topic_id: topic1.id) users.each { |user| user.reload } - expect(users[0].votes).to contain_exactly(topic1.id) - expect(users[0].votes_archive).to contain_exactly() - expect(users[1].votes).to contain_exactly(topic1.id) - expect(users[1].votes_archive).to contain_exactly() - expect(users[2].votes).to contain_exactly(topic1.id) - expect(users[2].votes_archive).to contain_exactly() - expect(users[3].votes).to contain_exactly() - expect(users[3].votes_archive).to contain_exactly() - expect(users[4].votes).to contain_exactly() - expect(users[4].votes_archive).to contain_exactly(topic1.id) + expect(users[0].topics_with_vote.pluck(:topic_id)).to contain_exactly(topic1.id) + expect(users[0].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly() + expect(users[1].topics_with_vote.pluck(:topic_id)).to contain_exactly(topic1.id) + expect(users[1].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly() + expect(users[2].topics_with_vote.pluck(:topic_id)).to contain_exactly(topic1.id) + expect(users[2].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly() + expect(users[3].topics_with_vote.pluck(:topic_id)).to contain_exactly() + expect(users[3].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly() + expect(users[4].topics_with_vote.pluck(:topic_id)).to contain_exactly() + expect(users[4].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly(topic1.id) expect(topic0.reload.vote_count).to eq(0) - expect(topic1.reload.vote_count).to eq(4) + expect(topic1.reload.vote_count).to eq(5) merged_post = topic0.posts.find_by(action_code: 'split_topic') expect(merged_post.raw).to include(I18n.t('voting.votes_moved', count: 2)) - expect(merged_post.raw).to include(I18n.t('voting.duplicated_votes', count: 1)) + expect(merged_post.raw).to include(I18n.t('voting.duplicated_votes', count: 2)) end it 'does not move votes when a single post is moved' do topic0.move_posts(Discourse.system_user, topic0.posts[1, 2].map(&:id), destination_topic_id: topic1.id) users.each { |user| user.reload } - expect(users[0].votes).to contain_exactly(topic0.id) - expect(users[0].votes_archive).to contain_exactly() - expect(users[1].votes).to contain_exactly(topic1.id) - expect(users[1].votes_archive).to contain_exactly() - expect(users[2].votes).to contain_exactly(topic0.id, topic1.id) - expect(users[2].votes_archive).to contain_exactly() - expect(users[3].votes).to contain_exactly() - expect(users[3].votes_archive).to contain_exactly() - expect(users[4].votes).to contain_exactly() - expect(users[4].votes_archive).to contain_exactly(topic0.id) - - expect(topic0.reload.vote_count).to eq(3) - expect(topic1.reload.vote_count).to eq(2) + expect(users[0].topics_with_vote.pluck(:topic_id)).to contain_exactly(topic0.id) + expect(users[0].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly() + expect(users[1].topics_with_vote.pluck(:topic_id)).to contain_exactly(topic1.id) + expect(users[1].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly() + expect(users[2].topics_with_vote.pluck(:topic_id)).to contain_exactly(topic0.id, topic1.id) + expect(users[2].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly() + expect(users[3].topics_with_vote.pluck(:topic_id)).to contain_exactly() + expect(users[3].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly() + expect(users[4].topics_with_vote.pluck(:topic_id)).to contain_exactly() + expect(users[4].topics_with_archived_vote.pluck(:topic_id)).to contain_exactly(topic0.id) + + expect(topic0.reload.vote_count).to eq(4) + expect(topic1.reload.vote_count).to eq(3) end end context "when a user has an empty string as the votes custom field" do before do - user0.custom_fields[DiscourseVoting::VOTES] = "" - user0.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = "" - user0.save - user0.reload + user0.votes.delete_all end it "returns a vote count of zero" do expect(user0.vote_count).to eq (0) - expect(user0.votes_archive).to eq ([]) + expect(user0.topics_with_archived_vote.pluck(:topic_id)).to eq ([]) end end @@ -121,19 +119,28 @@ topic1.update_status('closed', false, Discourse.system_user) expect(Jobs::VoteReclaim.jobs.first["args"].first["topic_id"]).to eq(topic1.id) end + + it 'creates notification that topic was completed' do + Jobs.run_immediately! + DiscourseVoting::Vote.create!(user: user0, topic: topic1) + expect { topic1.update_status('closed', true, user0) }.to change { user0.reload.notifications.count }.by(1) + notification = user0.notifications.last + expect(notification.topic_id).to eq(topic1.id) + expect(JSON.parse(notification.data)['message']).to eq('votes_released') + end end context "when a job is trashed and then recovered" do it "released the vote back to the user, then reclaims it on topic recovery" do Jobs.run_immediately! - user0.custom_fields[DiscourseVoting::VOTES] = [topic1.id] - user0.save + DiscourseVoting::Vote.create!(user: user0, topic: topic1) topic1.reload.trash! - expect(user0.reload.votes).to eq([]) + expect(user0.reload.topics_with_vote.pluck(:topic_id)).to eq([]) + expect(user0.notifications.count).to eq(0) topic1.recover! - expect(user0.reload.votes).to eq([topic1.id]) + expect(user0.reload.topics_with_vote.pluck(:topic_id)).to eq([topic1.id]) end end @@ -143,15 +150,15 @@ let(:post1) { Fabricate(:post, topic: topic1, post_number: 1) } before do - category1.custom_fields["enable_topic_voting"] = "true" + DiscourseVoting::CategorySetting.create!(category: category1) category1.save! Category.reset_voting_cache end it "enqueus a job to reclaim votes if voting is enabled for the new category" do user = post1.user - user.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = [post1.topic_id, 456456] - user.save! + DiscourseVoting::Vote.create!(user: user, topic: post1.topic, archive: true) + DiscourseVoting::Vote.create!(user: user, topic_id: 456456, archive: true) PostRevisor.new(post1).revise!(admin, category_id: category1.id) expect(Jobs::VoteReclaim.jobs.first["args"].first["topic_id"]).to eq(post1.reload.topic_id) @@ -159,14 +166,14 @@ Jobs::VoteReclaim.new.execute(topic_id: post1.topic_id) user.reload - expect(user.votes).to eq([post1.topic_id]) - expect(user.votes_archive).to eq([456456]) + expect(user.topics_with_vote.pluck(:topic_id)).to eq([post1.topic_id]) + expect(user.topics_with_archived_vote.pluck(:topic_id)).to eq([456456]) end it "enqueus a job to release votes if voting is disabled for the new category" do user = post0.user - user.custom_fields[DiscourseVoting::VOTES] = [post0.topic_id, 456456] - user.save! + DiscourseVoting::Vote.create!(user: user, topic: post0.topic) + DiscourseVoting::Vote.create!(user: user, topic_id: 456456) PostRevisor.new(post0).revise!(admin, category_id: category2.id) expect(Jobs::VoteRelease.jobs.first["args"].first["topic_id"]).to eq(post0.reload.topic_id) @@ -174,8 +181,8 @@ Jobs::VoteRelease.new.execute(topic_id: post0.topic_id) user.reload - expect(user.votes_archive).to eq([post0.topic_id]) - expect(user.votes).to eq([456456]) + expect(user.topics_with_archived_vote.pluck(:topic_id)).to eq([post0.topic_id]) + expect(user.topics_with_vote.pluck(:topic_id)).to eq([456456]) end it "doesn't enqueue a job if the topic has no votes" do @@ -192,40 +199,33 @@ let(:topic2) { Fabricate(:topic, category: category3) } before do - category1.custom_fields["enable_topic_voting"] = true - category1.save! + DiscourseVoting::CategorySetting.create!(category: category1) - category2.custom_fields["enable_topic_voting"] = true - category2.save! + DiscourseVoting::CategorySetting.create!(category: category2) - category3.custom_fields["enable_topic_voting"] = false - category3.save! + DiscourseVoting::CategorySetting.destroy_by(category: category3) - user0.custom_fields[DiscourseVoting::VOTES] = [topic0.id, topic1.id] - user0.custom_fields[DiscourseVoting::VOTES_ARCHIVE] = [topic2.id] - user0.save! + DiscourseVoting::Vote.create(user: user0, topic: topic0) + DiscourseVoting::Vote.create(user: user0, topic: topic1) + DiscourseVoting::Vote.create(user: user0, topic: topic2, archive: true) end it "reclaims votes when voting is disabled on a category" do - category = Category.find(category1.id) - category.custom_fields["enable_topic_voting"] = false - category.save! + DiscourseVoting::CategorySetting.destroy_by(category: category1) user0.reload - expect(user0.custom_fields[DiscourseVoting::VOTES]).to contain_exactly(topic1.id) - expect(user0.custom_fields[DiscourseVoting::VOTES_ARCHIVE]).to contain_exactly(topic0.id, topic2.id) + expect(DiscourseVoting::Vote.where(user: user0, archive: false).map(&:topic_id)).to contain_exactly(topic1.id) + expect(DiscourseVoting::Vote.where(user: user0, archive: true).map(&:topic_id)).to contain_exactly(topic0.id, topic2.id) end it "restores votes when voting is enabled on a category" do - category = Category.find(category3.id) - category.custom_fields["enable_topic_voting"] = true - category.save! + DiscourseVoting::CategorySetting.create!(category: category3) user0.reload - expect(user0.custom_fields[DiscourseVoting::VOTES]).to contain_exactly(topic0.id, topic1.id, topic2.id) - expect(user0.custom_fields[DiscourseVoting::VOTES_ARCHIVE]).to eq(nil) + expect(DiscourseVoting::Vote.where(user: user0, archive: false).map(&:topic_id)).to contain_exactly(topic0.id, topic1.id, topic2.id) + expect(DiscourseVoting::Vote.where(user: user0, archive: true).map(&:topic_id)).to eq([]) end end end