From 6a143030f8bde9342b988dcb332912bd872aac76 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 5 Oct 2021 11:38:49 +0300 Subject: [PATCH] FEATURE: Allow users to remove their vote (#14459) They can use the remove vote button or select the same option again for single choice polls. This commit refactor the plugin to properly organize code and make it easier to follow. --- .../poll/app/controllers/polls_controller.rb | 72 +++ .../initializers/extend-for-poll.js.es6 | 1 + .../javascripts/widgets/discourse-poll.js.es6 | 65 ++- plugins/poll/config/locales/client.en.yml | 4 + plugins/poll/jobs/regular/close_poll.rb | 2 +- plugins/poll/lib/poll.rb | 338 +++++++++++++ plugins/poll/plugin.rb | 464 +----------------- .../spec/controllers/polls_controller_spec.rb | 72 +-- .../spec/controllers/posts_controller_spec.rb | 4 +- .../spec/integration/poll_endpoints_spec.rb | 26 +- plugins/poll/spec/lib/polls_updater_spec.rb | 6 +- .../acceptance/poll-results-test.js.es6 | 16 + .../widgets/discourse-poll-test.js.es6 | 1 - 13 files changed, 550 insertions(+), 521 deletions(-) create mode 100644 plugins/poll/app/controllers/polls_controller.rb create mode 100644 plugins/poll/lib/poll.rb diff --git a/plugins/poll/app/controllers/polls_controller.rb b/plugins/poll/app/controllers/polls_controller.rb new file mode 100644 index 00000000000..86ae5c1ba4d --- /dev/null +++ b/plugins/poll/app/controllers/polls_controller.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +class DiscoursePoll::PollsController < ::ApplicationController + requires_plugin DiscoursePoll::PLUGIN_NAME + + before_action :ensure_logged_in, except: [:voters, :grouped_poll_results] + + def vote + post_id = params.require(:post_id) + poll_name = params.require(:poll_name) + options = params.require(:options) + + begin + poll, options = DiscoursePoll::Poll.vote(current_user, post_id, poll_name, options) + render json: { poll: poll, vote: options } + rescue DiscoursePoll::Error => e + render_json_error e.message + end + end + + def remove_vote + post_id = params.require(:post_id) + poll_name = params.require(:poll_name) + + begin + poll = DiscoursePoll::Poll.remove_vote(current_user, post_id, poll_name) + render json: { poll: poll } + rescue DiscoursePoll::Error => e + render_json_error e.message + end + end + + def toggle_status + post_id = params.require(:post_id) + poll_name = params.require(:poll_name) + status = params.require(:status) + + begin + poll = DiscoursePoll::Poll.toggle_status(current_user, post_id, poll_name, status) + render json: { poll: poll } + rescue DiscoursePoll::Error => e + render_json_error e.message + end + end + + def voters + post_id = params.require(:post_id) + poll_name = params.require(:poll_name) + opts = params.permit(:limit, :page, :option_id) + + raise Discourse::InvalidParameters.new(:post_id) if !Post.where(id: post_id).exists? + + poll = Poll.find_by(post_id: post_id, name: poll_name) + raise Discourse::InvalidParameters.new(:poll_name) if !poll&.can_see_voters?(current_user) + + render json: { voters: DiscoursePoll::Poll.serialized_voters(poll, opts) } + end + + def grouped_poll_results + post_id = params.require(:post_id) + poll_name = params.require(:poll_name) + user_field_name = params.require(:user_field_name) + + begin + render json: { + grouped_results: DiscoursePoll::Poll.grouped_poll_results(current_user, post_id, poll_name, user_field_name) + } + rescue DiscoursePoll::Error => e + render_json_error e.message + end + end +end diff --git a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 index ab0fb09570d..67212d5ef23 100644 --- a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 +++ b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 @@ -109,6 +109,7 @@ function initializePolls(api) { post: pollPost, poll, vote, + hasSavedVote: vote.length > 0, titleHTML: titleElement && titleElement.outerHTML, groupableUserFields: ( api.container.lookup("site-settings:main") diff --git a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 index a6937e51efc..a68560d0c82 100644 --- a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 +++ b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 @@ -636,24 +636,44 @@ createWidget("discourse-poll-buttons", { }) ); } else { + let showResultsButton; + let infoText; + if (poll.results === "on_vote" && !attrs.hasVoted && !isMe) { - contents.push(infoTextHtml(I18n.t("poll.results.vote.title"))); + infoText = infoTextHtml(I18n.t("poll.results.vote.title")); } else if (poll.results === "on_close" && !closed) { - contents.push(infoTextHtml(I18n.t("poll.results.closed.title"))); + infoText = infoTextHtml(I18n.t("poll.results.closed.title")); } else if (poll.results === "staff_only" && !isStaff) { - contents.push(infoTextHtml(I18n.t("poll.results.staff.title"))); + infoText = infoTextHtml(I18n.t("poll.results.staff.title")); } else { + showResultsButton = this.attach("button", { + className: "btn-default toggle-results", + label: "poll.show-results.label", + title: "poll.show-results.title", + icon: "far-eye", + action: "toggleResults", + }); + } + + if (showResultsButton) { + contents.push(showResultsButton); + } + + if (attrs.hasSavedVote) { contents.push( this.attach("button", { - className: "btn-default toggle-results", - label: "poll.show-results.label", - title: "poll.show-results.title", - icon: "far-eye", - disabled: poll.voters === 0, - action: "toggleResults", + className: "btn-default remove-vote", + label: "poll.remove-vote.label", + title: "poll.remove-vote.title", + icon: "trash-alt", + action: "removeVote", }) ); } + + if (infoText) { + contents.push(infoText); + } } if (attrs.groupableUserFields.length && poll.voters > 0) { @@ -894,6 +914,28 @@ export default createWidget("discourse-poll", { this.state.showResults = !this.state.showResults; }, + removeVote() { + const { attrs, state } = this; + state.loading = true; + return ajax("/polls/vote", { + type: "DELETE", + data: { + post_id: attrs.post.id, + poll_name: attrs.poll.name, + }, + }) + .then(({ poll }) => { + attrs.poll.setProperties(poll); + attrs.vote.length = 0; + attrs.hasSavedVote = false; + this.appEvents.trigger("poll:voted", poll, attrs.post, attrs.vote); + }) + .catch((error) => popupAjaxError(error)) + .finally(() => { + state.loading = false; + }); + }, + exportResults() { const { attrs } = this; const queryID = this.siteSettings.poll_export_data_explorer_query_id; @@ -963,6 +1005,10 @@ export default createWidget("discourse-poll", { } const { vote } = attrs; + if (!this.isMultiple() && vote.length === 1 && vote[0] === option.id) { + return this.removeVote(); + } + if (!this.isMultiple()) { vote.length = 0; } @@ -994,6 +1040,7 @@ export default createWidget("discourse-poll", { }, }) .then(({ poll }) => { + attrs.hasSavedVote = true; attrs.poll.setProperties(poll); this.appEvents.trigger("poll:voted", poll, attrs.post, attrs.vote); diff --git a/plugins/poll/config/locales/client.en.yml b/plugins/poll/config/locales/client.en.yml index b1a0246fc2a..c7fa3890414 100644 --- a/plugins/poll/config/locales/client.en.yml +++ b/plugins/poll/config/locales/client.en.yml @@ -44,6 +44,10 @@ en: title: "Display the poll results" label: "Show results" + remove-vote: + title: "Remove your vote" + label: "Remove vote" + hide-results: title: "Back to your votes" label: "Show vote" diff --git a/plugins/poll/jobs/regular/close_poll.rb b/plugins/poll/jobs/regular/close_poll.rb index 864bef086da..c803e44376b 100644 --- a/plugins/poll/jobs/regular/close_poll.rb +++ b/plugins/poll/jobs/regular/close_poll.rb @@ -13,10 +13,10 @@ module Jobs end DiscoursePoll::Poll.toggle_status( + Discourse.system_user, args[:post_id], args[:poll_name], "closed", - Discourse.system_user, false ) end diff --git a/plugins/poll/lib/poll.rb b/plugins/poll/lib/poll.rb new file mode 100644 index 00000000000..fba328e5f66 --- /dev/null +++ b/plugins/poll/lib/poll.rb @@ -0,0 +1,338 @@ +# frozen_string_literal: true + +class DiscoursePoll::Poll + def self.vote(user, post_id, poll_name, options) + serialized_poll = DiscoursePoll::Poll.change_vote(user, post_id, poll_name) do |poll| + # remove options that aren't available in the poll + available_options = poll.poll_options.map { |o| o.digest }.to_set + options.select! { |o| available_options.include?(o) } + + raise DiscoursePoll::Error.new I18n.t("poll.requires_at_least_1_valid_option") if options.empty? + + new_option_ids = poll.poll_options.each_with_object([]) do |option, obj| + obj << option.id if options.include?(option.digest) + end + + old_option_ids = poll.poll_options.each_with_object([]) do |option, obj| + if option.poll_votes.where(user_id: user.id).exists? + obj << option.id + end + end + + # remove non-selected votes + PollVote + .where(poll: poll, user: user) + .where.not(poll_option_id: new_option_ids) + .delete_all + + # create missing votes + (new_option_ids - old_option_ids).each do |option_id| + PollVote.create!(poll: poll, user: user, poll_option_id: option_id) + end + end + + [serialized_poll, options] + end + + def self.remove_vote(user, post_id, poll_name) + DiscoursePoll::Poll.change_vote(user, post_id, poll_name) do |poll| + PollVote.where(poll: poll, user: user).delete_all + end + end + + def self.toggle_status(user, post_id, poll_name, status, raise_errors = true) + Poll.transaction do + post = Post.find_by(id: post_id) + guardian = Guardian.new(user) + + # post must not be deleted + if post.nil? || post.trashed? + raise DiscoursePoll::Error.new I18n.t("poll.post_is_deleted") if raise_errors + return + end + + # topic must not be archived + if post.topic&.archived + raise DiscoursePoll::Error.new I18n.t("poll.topic_must_be_open_to_toggle_status") if raise_errors + return + end + + # either staff member or OP + unless post.user_id == user&.id || user&.staff? + raise DiscoursePoll::Error.new I18n.t("poll.only_staff_or_op_can_toggle_status") if raise_errors + return + end + + poll = Poll.find_by(post_id: post_id, name: poll_name) + + if !poll + raise DiscoursePoll::Error.new I18n.t("poll.no_poll_with_this_name", name: poll_name) if raise_errors + return + end + + poll.status = status + poll.save! + + serialized_poll = PollSerializer.new(poll, root: false, scope: guardian).as_json + payload = { post_id: post_id, polls: [serialized_poll] } + + post.publish_message!("/polls/#{post.topic_id}", payload) + + serialized_poll + end + end + + def self.serialized_voters(poll, opts = {}) + limit = (opts["limit"] || 25).to_i + limit = 0 if limit < 0 + limit = 50 if limit > 50 + + page = (opts["page"] || 1).to_i + page = 1 if page < 1 + + offset = (page - 1) * limit + + option_digest = opts["option_id"].to_s + + if poll.number? + user_ids = PollVote + .where(poll: poll) + .group(:user_id) + .order("MIN(created_at)") + .offset(offset) + .limit(limit) + .pluck(:user_id) + + result = User.where(id: user_ids).map { |u| UserNameSerializer.new(u).serializable_hash } + elsif option_digest.present? + poll_option = PollOption.find_by(poll: poll, digest: option_digest) + + raise Discourse::InvalidParameters.new(:option_id) unless poll_option + + user_ids = PollVote + .where(poll: poll, poll_option: poll_option) + .group(:user_id) + .order("MIN(created_at)") + .offset(offset) + .limit(limit) + .pluck(:user_id) + + user_hashes = User.where(id: user_ids).map { |u| UserNameSerializer.new(u).serializable_hash } + + result = { option_digest => user_hashes } + else + votes = DB.query <<~SQL + SELECT digest, user_id + FROM ( + SELECT digest + , user_id + , ROW_NUMBER() OVER (PARTITION BY poll_option_id ORDER BY pv.created_at) AS row + FROM poll_votes pv + JOIN poll_options po ON pv.poll_option_id = po.id + WHERE pv.poll_id = #{poll.id} + AND po.poll_id = #{poll.id} + ) v + WHERE row BETWEEN #{offset} AND #{offset + limit} + SQL + + user_ids = votes.map(&:user_id).uniq + + user_hashes = User + .where(id: user_ids) + .map { |u| [u.id, UserNameSerializer.new(u).serializable_hash] } + .to_h + + result = {} + votes.each do |v| + result[v.digest] ||= [] + result[v.digest] << user_hashes[v.user_id] + end + end + + result + end + + def self.transform_for_user_field_override(custom_user_field) + existing_field = UserField.find_by(name: custom_user_field) + existing_field ? "user_field_#{existing_field.id}" : custom_user_field + end + + def self.grouped_poll_results(user, post_id, poll_name, user_field_name) + raise Discourse::InvalidParameters.new(:post_id) if !Post.where(id: post_id).exists? + + poll = Poll.includes(:poll_options).includes(:poll_votes).find_by(post_id: post_id, name: poll_name) + raise Discourse::InvalidParameters.new(:poll_name) unless poll + + raise Discourse::InvalidParameters.new(:user_field_name) unless SiteSetting.poll_groupable_user_fields.split('|').include?(user_field_name) + + poll_votes = poll.poll_votes + + poll_options = {} + poll.poll_options.each do |option| + poll_options[option.id.to_s] = { html: option.html, digest: option.digest } + end + + user_ids = poll_votes.map(&:user_id).uniq + user_fields = UserCustomField.where(user_id: user_ids, name: transform_for_user_field_override(user_field_name)) + + user_field_map = {} + user_fields.each do |f| + # Build hash, so we can quickly look up field values for each user. + user_field_map[f.user_id] = f.value + end + + votes_with_field = poll_votes.map do |vote| + v = vote.attributes + v[:field_value] = user_field_map[vote.user_id] + v + end + + chart_data = [] + votes_with_field.group_by { |vote| vote[:field_value] }.each do |field_answer, votes| + grouped_selected_options = {} + + # Create all the options with 0 votes. This ensures all the charts will have the same order of options, and same colors per option. + poll_options.each do |id, option| + grouped_selected_options[id] = { + digest: option[:digest], + html: option[:html], + votes: 0 + } + end + + # Now go back and update the vote counts. Using hashes so we dont have n^2 + votes.group_by { |v| v["poll_option_id"] }.each do |option_id, votes_for_option| + grouped_selected_options[option_id.to_s][:votes] = votes_for_option.length + end + + group_label = field_answer ? field_answer.titleize : I18n.t("poll.user_field.no_data") + chart_data << { group: group_label, options: grouped_selected_options.values } + end + chart_data + end + + def self.schedule_jobs(post) + Poll.where(post: post).find_each do |poll| + job_args = { + post_id: post.id, + poll_name: poll.name + } + + Jobs.cancel_scheduled_job(:close_poll, job_args) + + if poll.open? && poll.close_at && poll.close_at > Time.zone.now + Jobs.enqueue_at(poll.close_at, :close_poll, job_args) + end + end + end + + def self.create!(post_id, poll) + close_at = begin + Time.zone.parse(poll["close"] || '') + rescue ArgumentError + end + + created_poll = Poll.create!( + post_id: post_id, + name: poll["name"].presence || "poll", + close_at: close_at, + type: poll["type"].presence || "regular", + status: poll["status"].presence || "open", + visibility: poll["public"] == "true" ? "everyone" : "secret", + title: poll["title"], + results: poll["results"].presence || "always", + min: poll["min"], + max: poll["max"], + step: poll["step"], + chart_type: poll["charttype"] || "bar", + groups: poll["groups"] + ) + + poll["options"].each do |option| + PollOption.create!( + poll: created_poll, + digest: option["id"].presence, + html: option["html"].presence&.strip + ) + end + end + + def self.extract(raw, topic_id, user_id = nil) + # TODO: we should fix the callback mess so that the cooked version is available + # in the validators instead of cooking twice + cooked = PrettyText.cook(raw, topic_id: topic_id, user_id: user_id) + + Nokogiri::HTML5(cooked).css("div.poll").map do |p| + poll = { "options" => [], "name" => DiscoursePoll::DEFAULT_POLL_NAME } + + # attributes + p.attributes.values.each do |attribute| + if attribute.name.start_with?(DiscoursePoll::DATA_PREFIX) + poll[attribute.name[DiscoursePoll::DATA_PREFIX.length..-1]] = CGI.escapeHTML(attribute.value || "") + end + end + + # options + p.css("li[#{DiscoursePoll::DATA_PREFIX}option-id]").each do |o| + option_id = o.attributes[DiscoursePoll::DATA_PREFIX + "option-id"].value.to_s + poll["options"] << { "id" => option_id, "html" => o.inner_html.strip } + end + + # title + title_element = p.css(".poll-title").first + if title_element + poll["title"] = title_element.inner_html.strip + end + + poll + end + end + + private + + def self.change_vote(user, post_id, poll_name) + Poll.transaction do + post = Post.find_by(id: post_id) + + # post must not be deleted + if post.nil? || post.trashed? + raise DiscoursePoll::Error.new I18n.t("poll.post_is_deleted") + end + + # topic must not be archived + if post.topic&.archived + raise DiscoursePoll::Error.new I18n.t("poll.topic_must_be_open_to_vote") + end + + # user must be allowed to post in topic + guardian = Guardian.new(user) + if !guardian.can_create_post?(post.topic) + raise DiscoursePoll::Error.new I18n.t("poll.user_cant_post_in_topic") + end + + poll = Poll.includes(:poll_options).find_by(post_id: post_id, name: poll_name) + + raise DiscoursePoll::Error.new I18n.t("poll.no_poll_with_this_name", name: poll_name) unless poll + raise DiscoursePoll::Error.new I18n.t("poll.poll_must_be_open_to_vote") if poll.is_closed? + + if poll.groups + poll_groups = poll.groups.split(",").map(&:downcase) + user_groups = user.groups.map { |g| g.name.downcase } + if (poll_groups & user_groups).empty? + raise DiscoursePoll::Error.new I18n.t("js.poll.results.groups.title", groups: poll.groups) + end + end + + yield(poll) + + poll.reload + + serialized_poll = PollSerializer.new(poll, root: false, scope: guardian).as_json + payload = { post_id: post_id, polls: [serialized_poll] } + + post.publish_message!("/polls/#{post.topic_id}", payload) + + serialized_poll + end + end +end diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb index 8965d9ca00e..de3c8a18ecc 100644 --- a/plugins/poll/plugin.rb +++ b/plugins/poll/plugin.rb @@ -7,35 +7,21 @@ # url: https://github.com/discourse/discourse/tree/main/plugins/poll register_asset "stylesheets/common/poll.scss" -register_asset "stylesheets/common/poll-ui-builder.scss" -register_asset "stylesheets/common/poll-breakdown.scss" register_asset "stylesheets/desktop/poll.scss", :desktop -register_asset "stylesheets/desktop/poll-ui-builder.scss", :desktop register_asset "stylesheets/mobile/poll.scss", :mobile +register_asset "stylesheets/common/poll-ui-builder.scss" +register_asset "stylesheets/desktop/poll-ui-builder.scss", :desktop +register_asset "stylesheets/common/poll-breakdown.scss" register_svg_icon "far fa-check-square" enabled_site_setting :poll_enabled -hide_plugin if self.respond_to?(:hide_plugin) - -PLUGIN_NAME ||= "discourse_poll" -DATA_PREFIX ||= "data-poll-" +hide_plugin after_initialize do - - [ - "../app/models/poll_vote", - "../app/models/poll_option", - "../app/models/poll", - "../app/serializers/poll_option_serializer", - "../app/serializers/poll_serializer", - "../lib/polls_validator", - "../lib/polls_updater", - "../lib/post_validator", - "../jobs/regular/close_poll", - ].each { |path| require File.expand_path(path, __FILE__) } - module ::DiscoursePoll + PLUGIN_NAME ||= "discourse_poll" + DATA_PREFIX ||= "data-poll-" HAS_POLLS ||= "has_polls" DEFAULT_POLL_NAME ||= "poll" @@ -43,434 +29,38 @@ after_initialize do engine_name PLUGIN_NAME isolate_namespace DiscoursePoll end + + class Error < StandardError; end end - class DiscoursePoll::Poll - class << self - - def vote(post_id, poll_name, options, user) - Poll.transaction do - post = Post.find_by(id: post_id) - - # post must not be deleted - if post.nil? || post.trashed? - raise StandardError.new I18n.t("poll.post_is_deleted") - end - - # topic must not be archived - if post.topic&.archived - raise StandardError.new I18n.t("poll.topic_must_be_open_to_vote") - end - - # user must be allowed to post in topic - guardian = Guardian.new(user) - if !guardian.can_create_post?(post.topic) - raise StandardError.new I18n.t("poll.user_cant_post_in_topic") - end - - poll = Poll.includes(:poll_options).find_by(post_id: post_id, name: poll_name) - - raise StandardError.new I18n.t("poll.no_poll_with_this_name", name: poll_name) unless poll - raise StandardError.new I18n.t("poll.poll_must_be_open_to_vote") if poll.is_closed? - - if poll.groups - poll_groups = poll.groups.split(",").map(&:downcase) - user_groups = user.groups.map { |g| g.name.downcase } - if (poll_groups & user_groups).empty? - raise StandardError.new I18n.t("js.poll.results.groups.title", groups: poll.groups) - end - end - - # remove options that aren't available in the poll - available_options = poll.poll_options.map { |o| o.digest }.to_set - options.select! { |o| available_options.include?(o) } - - raise StandardError.new I18n.t("poll.requires_at_least_1_valid_option") if options.empty? - - new_option_ids = poll.poll_options.each_with_object([]) do |option, obj| - obj << option.id if options.include?(option.digest) - end - - old_option_ids = poll.poll_options.each_with_object([]) do |option, obj| - if option.poll_votes.where(user_id: user.id).exists? - obj << option.id - end - end - - # remove non-selected votes - PollVote - .where(poll: poll, user: user) - .where.not(poll_option_id: new_option_ids) - .delete_all - - # create missing votes - (new_option_ids - old_option_ids).each do |option_id| - PollVote.create!(poll: poll, user: user, poll_option_id: option_id) - end - - poll.reload - - serialized_poll = PollSerializer.new(poll, root: false, scope: guardian).as_json - payload = { post_id: post_id, polls: [serialized_poll] } - - post.publish_message!("/polls/#{post.topic_id}", payload) - - [serialized_poll, options] - end - end - - def toggle_status(post_id, poll_name, status, user, raise_errors = true) - Poll.transaction do - post = Post.find_by(id: post_id) - guardian = Guardian.new(user) - - # post must not be deleted - if post.nil? || post.trashed? - raise StandardError.new I18n.t("poll.post_is_deleted") if raise_errors - return - end - - # topic must not be archived - if post.topic&.archived - raise StandardError.new I18n.t("poll.topic_must_be_open_to_toggle_status") if raise_errors - return - end - - # either staff member or OP - unless post.user_id == user&.id || user&.staff? - raise StandardError.new I18n.t("poll.only_staff_or_op_can_toggle_status") if raise_errors - return - end - - poll = Poll.find_by(post_id: post_id, name: poll_name) - - if !poll - raise StandardError.new I18n.t("poll.no_poll_with_this_name", name: poll_name) if raise_errors - return - end - - poll.status = status - poll.save! - - serialized_poll = PollSerializer.new(poll, root: false, scope: guardian).as_json - payload = { post_id: post_id, polls: [serialized_poll] } - - post.publish_message!("/polls/#{post.topic_id}", payload) - - serialized_poll - end - end - - def serialized_voters(poll, opts = {}) - limit = (opts["limit"] || 25).to_i - limit = 0 if limit < 0 - limit = 50 if limit > 50 - - page = (opts["page"] || 1).to_i - page = 1 if page < 1 - - offset = (page - 1) * limit - - option_digest = opts["option_id"].to_s - - if poll.number? - user_ids = PollVote - .where(poll: poll) - .group(:user_id) - .order("MIN(created_at)") - .offset(offset) - .limit(limit) - .pluck(:user_id) - - result = User.where(id: user_ids).map { |u| UserNameSerializer.new(u).serializable_hash } - elsif option_digest.present? - poll_option = PollOption.find_by(poll: poll, digest: option_digest) - - raise Discourse::InvalidParameters.new(:option_id) unless poll_option - - user_ids = PollVote - .where(poll: poll, poll_option: poll_option) - .group(:user_id) - .order("MIN(created_at)") - .offset(offset) - .limit(limit) - .pluck(:user_id) - - user_hashes = User.where(id: user_ids).map { |u| UserNameSerializer.new(u).serializable_hash } - - result = { option_digest => user_hashes } - else - votes = DB.query <<~SQL - SELECT digest, user_id - FROM ( - SELECT digest - , user_id - , ROW_NUMBER() OVER (PARTITION BY poll_option_id ORDER BY pv.created_at) AS row - FROM poll_votes pv - JOIN poll_options po ON pv.poll_option_id = po.id - WHERE pv.poll_id = #{poll.id} - AND po.poll_id = #{poll.id} - ) v - WHERE row BETWEEN #{offset} AND #{offset + limit} - SQL - - user_ids = votes.map(&:user_id).uniq - - user_hashes = User - .where(id: user_ids) - .map { |u| [u.id, UserNameSerializer.new(u).serializable_hash] } - .to_h - - result = {} - votes.each do |v| - result[v.digest] ||= [] - result[v.digest] << user_hashes[v.user_id] - end - end - - result - end - - def voters(post_id, poll_name, user, opts = {}) - post = Post.find_by(id: post_id) - raise Discourse::InvalidParameters.new(:post_id) unless post - - poll = Poll.find_by(post_id: post_id, name: poll_name) - raise Discourse::InvalidParameters.new(:poll_name) unless poll&.can_see_voters?(user) - - serialized_voters(poll, opts) - end - - def transform_for_user_field_override(custom_user_field) - existing_field = UserField.find_by(name: custom_user_field) - existing_field ? "user_field_#{existing_field.id}" : custom_user_field - end - - def grouped_poll_results(post_id, poll_name, user_field_name, user) - post = Post.find_by(id: post_id) - raise Discourse::InvalidParameters.new(:post_id) unless post - - poll = Poll.includes(:poll_options).includes(:poll_votes).find_by(post_id: post_id, name: poll_name) - raise Discourse::InvalidParameters.new(:poll_name) unless poll - - raise Discourse::InvalidParameters.new(:user_field_name) unless SiteSetting.poll_groupable_user_fields.split('|').include?(user_field_name) - - poll_votes = poll.poll_votes - - poll_options = {} - poll.poll_options.each do |option| - poll_options[option.id.to_s] = { html: option.html, digest: option.digest } - end - - user_ids = poll_votes.map(&:user_id).uniq - user_fields = UserCustomField.where(user_id: user_ids, name: transform_for_user_field_override(user_field_name)) - - user_field_map = {} - user_fields.each do |f| - # Build hash, so we can quickly look up field values for each user. - user_field_map[f.user_id] = f.value - end - - votes_with_field = poll_votes.map do |vote| - v = vote.attributes - v[:field_value] = user_field_map[vote.user_id] - v - end - - chart_data = [] - votes_with_field.group_by { |vote| vote[:field_value] }.each do |field_answer, votes| - grouped_selected_options = {} - - # Create all the options with 0 votes. This ensures all the charts will have the same order of options, and same colors per option. - poll_options.each do |id, option| - grouped_selected_options[id] = { - digest: option[:digest], - html: option[:html], - votes: 0 - } - end - - # Now go back and update the vote counts. Using hashes so we dont have n^2 - votes.group_by { |v| v["poll_option_id"] }.each do |option_id, votes_for_option| - grouped_selected_options[option_id.to_s][:votes] = votes_for_option.length - end - - group_label = field_answer ? field_answer.titleize : I18n.t("poll.user_field.no_data") - chart_data << { group: group_label, options: grouped_selected_options.values } - end - chart_data - end - - def schedule_jobs(post) - Poll.where(post: post).find_each do |poll| - job_args = { - post_id: post.id, - poll_name: poll.name - } - - Jobs.cancel_scheduled_job(:close_poll, job_args) - - if poll.open? && poll.close_at && poll.close_at > Time.zone.now - Jobs.enqueue_at(poll.close_at, :close_poll, job_args) - end - end - end - - def create!(post_id, poll) - close_at = begin - Time.zone.parse(poll["close"] || '') - rescue ArgumentError - end - - created_poll = Poll.create!( - post_id: post_id, - name: poll["name"].presence || "poll", - close_at: close_at, - type: poll["type"].presence || "regular", - status: poll["status"].presence || "open", - visibility: poll["public"] == "true" ? "everyone" : "secret", - title: poll["title"], - results: poll["results"].presence || "always", - min: poll["min"], - max: poll["max"], - step: poll["step"], - chart_type: poll["charttype"] || "bar", - groups: poll["groups"] - ) - - poll["options"].each do |option| - PollOption.create!( - poll: created_poll, - digest: option["id"].presence, - html: option["html"].presence&.strip - ) - end - end - - def extract(raw, topic_id, user_id = nil) - # TODO: we should fix the callback mess so that the cooked version is available - # in the validators instead of cooking twice - cooked = PrettyText.cook(raw, topic_id: topic_id, user_id: user_id) - - Nokogiri::HTML5(cooked).css("div.poll").map do |p| - poll = { "options" => [], "name" => DiscoursePoll::DEFAULT_POLL_NAME } - - # attributes - p.attributes.values.each do |attribute| - if attribute.name.start_with?(DATA_PREFIX) - poll[attribute.name[DATA_PREFIX.length..-1]] = CGI.escapeHTML(attribute.value || "") - end - end - - # options - p.css("li[#{DATA_PREFIX}option-id]").each do |o| - option_id = o.attributes[DATA_PREFIX + "option-id"].value.to_s - poll["options"] << { "id" => option_id, "html" => o.inner_html.strip } - end - - # title - title_element = p.css(".poll-title").first - if title_element - poll["title"] = title_element.inner_html.strip - end - - poll - end - end - end - end - - class DiscoursePoll::PollsController < ::ApplicationController - requires_plugin PLUGIN_NAME - - before_action :ensure_logged_in, except: [:voters, :grouped_poll_results] - - def vote - post_id = params.require(:post_id) - poll_name = params.require(:poll_name) - options = params.require(:options) - - begin - poll, options = DiscoursePoll::Poll.vote(post_id, poll_name, options, current_user) - render json: { poll: poll, vote: options } - rescue StandardError => e - render_json_error e.message - end - end - - def current_user_voted - poll = Poll.includes(:post).find_by(id: params[:id]) - raise Discourse::NotFound.new(:id) if poll.nil? - - can_see_poll = Guardian.new(current_user).can_see_post?(poll.post) - raise Discourse::NotFound.new(:id) if !can_see_poll - - presence = PollVote.where(poll: poll, user: current_user).exists? - render json: { voted: presence } - end - - def toggle_status - post_id = params.require(:post_id) - poll_name = params.require(:poll_name) - status = params.require(:status) - - begin - poll = DiscoursePoll::Poll.toggle_status(post_id, poll_name, status, current_user) - render json: { poll: poll } - rescue StandardError => e - render_json_error e.message - end - end - - def voters - post_id = params.require(:post_id) - poll_name = params.require(:poll_name) - - opts = params.permit(:limit, :page, :option_id) - - begin - render json: { voters: DiscoursePoll::Poll.voters(post_id, poll_name, current_user, opts) } - rescue StandardError => e - render_json_error e.message - end - end - - def grouped_poll_results - post_id = params.require(:post_id) - poll_name = params.require(:poll_name) - user_field_name = params.require(:user_field_name) - - begin - render json: { - grouped_results: DiscoursePoll::Poll.grouped_poll_results(post_id, poll_name, user_field_name, current_user) - } - rescue StandardError => e - render_json_error e.message - end - end - - def groupable_user_fields - render json: { - fields: SiteSetting.poll_groupable_user_fields.split('|').map do |field| - { name: field.humanize.capitalize, value: field } - end - } - end - end + require_relative "app/controllers/polls_controller.rb" + require_relative "app/models/poll_option.rb" + require_relative "app/models/poll_vote.rb" + require_relative "app/models/poll.rb" + require_relative "app/serializers/poll_option_serializer.rb" + require_relative "app/serializers/poll_serializer.rb" + require_relative "jobs/regular/close_poll.rb" + require_relative "lib/poll.rb" + require_relative "lib/polls_updater.rb" + require_relative "lib/polls_validator.rb" + require_relative "lib/post_validator.rb" DiscoursePoll::Engine.routes.draw do put "/vote" => "polls#vote" + delete "/vote" => "polls#remove_vote" put "/toggle_status" => "polls#toggle_status" get "/voters" => 'polls#voters' get "/grouped_poll_results" => 'polls#grouped_poll_results' - get "/groupable_user_fields" => 'polls#groupable_user_fields' - get "/:id/votes/current_user_voted" => "polls#current_user_voted" end Discourse::Application.routes.append do mount ::DiscoursePoll::Engine, at: "/polls" end + allow_new_queued_post_payload_attribute("is_poll") + register_post_custom_field_type(DiscoursePoll::HAS_POLLS, :boolean) + topic_view_post_custom_fields_allowlister { [DiscoursePoll::HAS_POLLS] } + reloadable_patch do Post.class_eval do attr_accessor :extracted_polls @@ -519,8 +109,6 @@ after_initialize do true end - allow_new_queued_post_payload_attribute("is_poll") - NewPostManager.add_handler(1) do |manager| post = Post.new(raw: manager.args[:raw]) @@ -581,10 +169,6 @@ after_initialize do PollVote.where(user_id: source_user.id).update_all(user_id: target_user.id) end - register_post_custom_field_type(DiscoursePoll::HAS_POLLS, :boolean) - - topic_view_post_custom_fields_allowlister { [DiscoursePoll::HAS_POLLS] } - add_to_class(:topic_view, :polls) do @polls ||= begin polls = {} diff --git a/plugins/poll/spec/controllers/polls_controller_spec.rb b/plugins/poll/spec/controllers/polls_controller_spec.rb index 81628c7937b..6eb55205155 100644 --- a/plugins/poll/spec/controllers/polls_controller_spec.rb +++ b/plugins/poll/spec/controllers/polls_controller_spec.rb @@ -13,7 +13,6 @@ describe ::DiscoursePoll::PollsController do let(:public_poll_on_close) { Fabricate(:post, topic: topic, user: user, raw: "[poll public=true results=on_close]\n- A\n- B\n[/poll]") } describe "#vote" do - it "works" do channel = "/polls/#{poll.topic_id}" @@ -118,6 +117,24 @@ describe ::DiscoursePoll::PollsController do expect(json["poll"]["options"][1]["votes"]).to eq(1) end + it "supports removing votes" do + put :vote, params: { + post_id: poll.id, poll_name: "poll", options: ["5c24fc1df56d764b550ceae1b9319125"] + }, format: :json + + expect(response.status).to eq(200) + + delete :remove_vote, params: { + post_id: poll.id, poll_name: "poll" + }, format: :json + + expect(response.status).to eq(200) + json = response.parsed_body + expect(json["poll"]["voters"]).to eq(0) + expect(json["poll"]["options"][0]["votes"]).to eq(0) + expect(json["poll"]["options"][1]["votes"]).to eq(0) + end + it "works on closed topics" do topic.update_attribute(:closed, true) @@ -218,7 +235,6 @@ describe ::DiscoursePoll::PollsController do end describe "#toggle_status" do - it "works for OP" do channel = "/polls/#{poll.topic_id}" @@ -264,11 +280,9 @@ describe ::DiscoursePoll::PollsController do json = response.parsed_body expect(json["errors"][0]).to eq(I18n.t("poll.post_is_deleted")) end - end describe "#voters" do - let(:first) { "5c24fc1df56d764b550ceae1b9319125" } let(:second) { "e89dec30bbd9bf50fabf6a05b4324edf" } @@ -333,7 +347,7 @@ describe ::DiscoursePoll::PollsController do poll_name: "poll", post_id: public_poll_on_vote.id }, format: :json - expect(response.status).to eq(422) + expect(response.status).to eq(400) put :vote, params: { post_id: public_poll_on_vote.id, poll_name: "poll", options: [second] @@ -364,7 +378,7 @@ describe ::DiscoursePoll::PollsController do poll_name: "poll", post_id: public_poll_on_close.id }, format: :json - expect(response.status).to eq(422) + expect(response.status).to eq(400) put :toggle_status, params: { post_id: public_poll_on_close.id, poll_name: "poll", status: "closed" @@ -382,51 +396,5 @@ describe ::DiscoursePoll::PollsController do expect(json["voters"][first].size).to eq(1) end - - end - - describe '#current_user_voted' do - let(:logged_user) { Fabricate(:user) } - let(:post_with_poll) { Fabricate(:post, raw: "[poll]\n- A\n- B\n[/poll]") } - - before { log_in_user(logged_user) } - - it 'returns true if the logged user already voted' do - poll = post_with_poll.polls.last - PollVote.create!(poll: poll, user: logged_user) - - get :current_user_voted, params: { id: poll.id }, format: :json - parsed_body = JSON.parse(response.body) - - expect(response.status).to eq(200) - expect(parsed_body['voted']).to eq(true) - end - - it 'returns a 404 if there is no poll' do - unknown_poll_id = 999999 - - get :current_user_voted, params: { id: unknown_poll_id }, format: :json - - expect(response.status).to eq(404) - end - - it "returns a 404 if the user doesn't have access to the poll" do - pm_with_poll = Fabricate(:private_message_post, raw: "[poll]\n- A\n- B\n[/poll]") - poll = pm_with_poll.polls.last - - get :current_user_voted, params: { id: poll.id }, format: :json - - expect(response.status).to eq(404) - end - - it "returns false if the user didn't vote yet" do - poll = post_with_poll.polls.last - - get :current_user_voted, params: { id: poll.id }, format: :json - parsed_body = JSON.parse(response.body) - - expect(response.status).to eq(200) - expect(parsed_body['voted']).to eq(false) - end end end diff --git a/plugins/poll/spec/controllers/posts_controller_spec.rb b/plugins/poll/spec/controllers/posts_controller_spec.rb index 37bcb45e75c..ab0df1e6b22 100644 --- a/plugins/poll/spec/controllers/posts_controller_spec.rb +++ b/plugins/poll/spec/controllers/posts_controller_spec.rb @@ -185,7 +185,7 @@ describe PostsController do end it "resets the votes" do - DiscoursePoll::Poll.vote(post_id, "poll", ["5c24fc1df56d764b550ceae1b9319125"], user) + DiscoursePoll::Poll.vote(user, post_id, "poll", ["5c24fc1df56d764b550ceae1b9319125"]) put :update, params: { id: post_id, post: { raw: "[poll]\n- A\n- B\n- C\n[/poll]" } @@ -244,7 +244,7 @@ describe PostsController do describe "with at least one vote" do before do - DiscoursePoll::Poll.vote(post_id, "poll", ["5c24fc1df56d764b550ceae1b9319125"], user) + DiscoursePoll::Poll.vote(user, post_id, "poll", ["5c24fc1df56d764b550ceae1b9319125"]) end it "cannot change the options" do diff --git a/plugins/poll/spec/integration/poll_endpoints_spec.rb b/plugins/poll/spec/integration/poll_endpoints_spec.rb index 21ed171f977..61e1e8b5f5b 100644 --- a/plugins/poll/spec/integration/poll_endpoints_spec.rb +++ b/plugins/poll/spec/integration/poll_endpoints_spec.rb @@ -11,10 +11,10 @@ describe "DiscoursePoll endpoints" do it "should return the right response" do DiscoursePoll::Poll.vote( + user, post.id, DiscoursePoll::DEFAULT_POLL_NAME, - [option_a], - user + [option_a] ) get "/polls/voters.json", params: { @@ -34,10 +34,10 @@ describe "DiscoursePoll endpoints" do it 'should return the right response for a single option' do DiscoursePoll::Poll.vote( + user, post.id, DiscoursePoll::DEFAULT_POLL_NAME, - [option_a, option_b], - user + [option_a, option_b] ) get "/polls/voters.json", params: { @@ -72,7 +72,7 @@ describe "DiscoursePoll endpoints" do post_id: -1, poll_name: DiscoursePoll::DEFAULT_POLL_NAME } - expect(response.status).to eq(422) + expect(response.status).to eq(400) expect(response.body).to include('post_id') end end @@ -87,7 +87,7 @@ describe "DiscoursePoll endpoints" do describe 'when poll_name is not valid' do it 'should raise the right error' do get "/polls/voters.json", params: { post_id: post.id, poll_name: 'wrongpoll' } - expect(response.status).to eq(422) + expect(response.status).to eq(400) expect(response.body).to include('poll_name') end end @@ -99,10 +99,10 @@ describe "DiscoursePoll endpoints" do post DiscoursePoll::Poll.vote( + user, post.id, DiscoursePoll::DEFAULT_POLL_NAME, - ["4d8a15e3cc35750f016ce15a43937620"], - user + ["4d8a15e3cc35750f016ce15a43937620"] ) get "/polls/voters.json", params: { @@ -137,20 +137,20 @@ describe "DiscoursePoll endpoints" do } [user1, user2, user3].each_with_index do |user, index| DiscoursePoll::Poll.vote( + user, post.id, DiscoursePoll::DEFAULT_POLL_NAME, - [user_votes["user_#{index}".to_sym]], - user + [user_votes["user_#{index}".to_sym]] ) UserCustomField.create(user_id: user.id, name: "something", value: "value#{index}") end # Add another user to one of the fields to prove it groups users properly DiscoursePoll::Poll.vote( + user4, post.id, DiscoursePoll::DEFAULT_POLL_NAME, - [option_a, option_b], - user4 + [option_a, option_b] ) UserCustomField.create(user_id: user4.id, name: "something", value: "value1") end @@ -182,7 +182,7 @@ describe "DiscoursePoll endpoints" do user_field_name: "something" } - expect(response.status).to eq(422) + expect(response.status).to eq(400) expect(response.body).to include('user_field_name') end end diff --git a/plugins/poll/spec/lib/polls_updater_spec.rb b/plugins/poll/spec/lib/polls_updater_spec.rb index 2a103d13424..4f22cad054b 100644 --- a/plugins/poll/spec/lib/polls_updater_spec.rb +++ b/plugins/poll/spec/lib/polls_updater_spec.rb @@ -78,8 +78,8 @@ describe DiscoursePoll::PollsUpdater do let(:post) { Fabricate(:post, raw: raw) } it "works if poll is closed and unmodified" do - DiscoursePoll::Poll.vote(post.id, "poll", ["e55de753c08b93d04d677ce05e942d3c"], post.user) - DiscoursePoll::Poll.toggle_status(post.id, "poll", "closed", post.user) + DiscoursePoll::Poll.vote(post.user, post.id, "poll", ["e55de753c08b93d04d677ce05e942d3c"]) + DiscoursePoll::Poll.toggle_status(post.user, post.id, "poll", "closed") freeze_time (SiteSetting.poll_edit_window_mins + 1).minutes.from_now update(post, DiscoursePoll::PollsValidator.new(post).validate_polls) @@ -159,7 +159,7 @@ describe DiscoursePoll::PollsUpdater do before do expect { - DiscoursePoll::Poll.vote(post.id, "poll", [polls["poll"]["options"][0]["id"]], user) + DiscoursePoll::Poll.vote(user, post.id, "poll", [polls["poll"]["options"][0]["id"]]) }.to change { PollVote.count }.by(1) end diff --git a/plugins/poll/test/javascripts/acceptance/poll-results-test.js.es6 b/plugins/poll/test/javascripts/acceptance/poll-results-test.js.es6 index cf2e9f70685..185bfc8c81e 100644 --- a/plugins/poll/test/javascripts/acceptance/poll-results-test.js.es6 +++ b/plugins/poll/test/javascripts/acceptance/poll-results-test.js.es6 @@ -1,5 +1,6 @@ import { acceptance, + count, publishToMessageBus, } from "discourse/tests/helpers/qunit-helpers"; import { test } from "qunit"; @@ -558,6 +559,8 @@ acceptance("Poll results", function (needs) { }); } }); + + server.delete("/polls/vote", () => helper.response({ success: "OK" })); }); test("can load more voters", async function (assert) { @@ -643,4 +646,17 @@ acceptance("Poll results", function (needs) { 0 ); }); + + test("can unvote", async function (assert) { + await visit("/t/-/load-more-poll-voters"); + await click(".toggle-results"); + + assert.equal(count(".poll-container .d-icon-circle"), 1); + assert.equal(count(".poll-container .d-icon-far-circle"), 1); + + await click(".remove-vote"); + + assert.equal(count(".poll-container .d-icon-circle"), 0); + assert.equal(count(".poll-container .d-icon-far-circle"), 2); + }); }); diff --git a/plugins/poll/test/javascripts/widgets/discourse-poll-test.js.es6 b/plugins/poll/test/javascripts/widgets/discourse-poll-test.js.es6 index 5666f832b24..90da62b6b2f 100644 --- a/plugins/poll/test/javascripts/widgets/discourse-poll-test.js.es6 +++ b/plugins/poll/test/javascripts/widgets/discourse-poll-test.js.es6 @@ -75,7 +75,6 @@ discourseModule( ], voters: 1, chart_type: "bar", - groups: "foo", }, vote: ["1f972d1df351de3ce35a787c89faad29"], },