diff --git a/db/migrate/20180828065005_change_bounce_score_to_float.rb b/db/migrate/20180828065005_change_bounce_score_to_float.rb index b2cd5a4a71f..3164f7c2b62 100644 --- a/db/migrate/20180828065005_change_bounce_score_to_float.rb +++ b/db/migrate/20180828065005_change_bounce_score_to_float.rb @@ -1,5 +1,9 @@ class ChangeBounceScoreToFloat < ActiveRecord::Migration[5.2] - def change + def up change_column :user_stats, :bounce_score, :float end + + def down + change_column :user_stats, :bounce_score, :integer + end end diff --git a/plugins/poll/app/models/poll.rb b/plugins/poll/app/models/poll.rb new file mode 100644 index 00000000000..192e599ef8a --- /dev/null +++ b/plugins/poll/app/models/poll.rb @@ -0,0 +1,76 @@ +class Poll < ActiveRecord::Base + # because we want to use the 'type' column and don't want to use STI + self.inheritance_column = nil + + belongs_to :post + + has_many :poll_options, dependent: :destroy + has_many :poll_votes + + enum type: { + regular: 0, + multiple: 1, + number: 2, + } + + enum status: { + open: 0, + closed: 1, + } + + enum results: { + always: 0, + on_vote: 1, + on_close: 2, + } + + enum visibility: { + secret: 0, + everyone: 1, + } + + validates :min, numericality: { allow_nil: true, only_integer: true, greater_than: 0 } + validates :max, numericality: { allow_nil: true, only_integer: true, greater_than: 0 } + validates :step, numericality: { allow_nil: true, only_integer: true, greater_than: 0 } + + def is_closed? + closed? || (close_at && close_at <= Time.zone.now) + end + + def can_see_results?(user) + always? || is_closed? || (on_vote? && has_voted?(user)) + end + + def has_voted?(user) + user&.id && poll_votes.any? { |v| v.user_id == user.id } + end + + def can_see_voters?(user) + everyone? && can_see_results?(user) + end +end + +# == Schema Information +# +# Table name: polls +# +# id :bigint(8) not null, primary key +# post_id :bigint(8) +# name :string default("poll"), not null +# close_at :datetime +# type :integer default("regular"), not null +# status :integer default("open"), not null +# results :integer default("always"), not null +# visibility :integer default("secret"), not null +# min :integer +# max :integer +# step :integer +# anonymous_voters :integer +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_polls_on_post_id (post_id) +# index_polls_on_post_id_and_name (post_id,name) UNIQUE +# diff --git a/plugins/poll/app/models/poll_option.rb b/plugins/poll/app/models/poll_option.rb new file mode 100644 index 00000000000..f245d612447 --- /dev/null +++ b/plugins/poll/app/models/poll_option.rb @@ -0,0 +1,24 @@ +class PollOption < ActiveRecord::Base + belongs_to :poll + has_many :poll_votes, dependent: :delete_all + + default_scope { order(created_at: :asc) } +end + +# == Schema Information +# +# Table name: poll_options +# +# id :bigint(8) not null, primary key +# poll_id :bigint(8) +# digest :string not null +# html :text not null +# anonymous_votes :integer +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_poll_options_on_poll_id (poll_id) +# index_poll_options_on_poll_id_and_digest (poll_id,digest) UNIQUE +# diff --git a/plugins/poll/app/models/poll_vote.rb b/plugins/poll/app/models/poll_vote.rb new file mode 100644 index 00000000000..6f5e7c22611 --- /dev/null +++ b/plugins/poll/app/models/poll_vote.rb @@ -0,0 +1,23 @@ +class PollVote < ActiveRecord::Base + belongs_to :poll + belongs_to :poll_option + belongs_to :user +end + +# == Schema Information +# +# Table name: poll_votes +# +# poll_id :bigint(8) +# poll_option_id :bigint(8) +# user_id :bigint(8) +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_poll_votes_on_poll_id (poll_id) +# index_poll_votes_on_poll_id_and_poll_option_id_and_user_id (poll_id,poll_option_id,user_id) UNIQUE +# index_poll_votes_on_poll_option_id (poll_option_id) +# index_poll_votes_on_user_id (user_id) +# diff --git a/plugins/poll/app/serializers/poll_option_serializer.rb b/plugins/poll/app/serializers/poll_option_serializer.rb new file mode 100644 index 00000000000..b23ac7a86d8 --- /dev/null +++ b/plugins/poll/app/serializers/poll_option_serializer.rb @@ -0,0 +1,14 @@ +class PollOptionSerializer < ApplicationSerializer + + attributes :id, :html, :votes + + def id + object.digest + end + + def votes + # `size` instead of `count` to prevent N+1 + object.poll_votes.size + object.anonymous_votes.to_i + end + +end diff --git a/plugins/poll/app/serializers/poll_serializer.rb b/plugins/poll/app/serializers/poll_serializer.rb new file mode 100644 index 00000000000..fb8cd269a86 --- /dev/null +++ b/plugins/poll/app/serializers/poll_serializer.rb @@ -0,0 +1,50 @@ +class PollSerializer < ApplicationSerializer + attributes :name, + :type, + :status, + :public, + :results, + :min, + :max, + :step, + :options, + :voters, + :close + + def public + true + end + + def include_public? + object.everyone? + end + + def include_min? + object.min.present? && (object.number? || object.multiple?) + end + + def include_max? + object.max.present? && (object.number? || object.multiple?) + end + + def include_step? + object.step.present? && object.number? + end + + def options + object.poll_options.map { |o| PollOptionSerializer.new(o, root: false).as_json } + end + + def voters + object.poll_votes.map { |v| v.user_id }.uniq.count + object.anonymous_voters.to_i + end + + def close + object.close_at + end + + def include_close? + object.close_at.present? + end + +end diff --git a/plugins/poll/assets/javascripts/controllers/poll-ui-builder.js.es6 b/plugins/poll/assets/javascripts/controllers/poll-ui-builder.js.es6 index eab85cafe5e..fc599f349f5 100644 --- a/plugins/poll/assets/javascripts/controllers/poll-ui-builder.js.es6 +++ b/plugins/poll/assets/javascripts/controllers/poll-ui-builder.js.es6 @@ -9,6 +9,10 @@ export default Ember.Controller.extend({ numberPollType: "number", multiplePollType: "multiple", + alwaysPollResult: "always", + votePollResult: "on_vote", + closedPollResult: "on_close", + init() { this._super(); this._setupPoll(); @@ -32,6 +36,24 @@ export default Ember.Controller.extend({ ]; }, + @computed("alwaysPollResult", "votePollResult", "closedPollResult") + pollResults(alwaysPollResult, votePollResult, closedPollResult) { + return [ + { + name: I18n.t("poll.ui_builder.poll_result.always"), + value: alwaysPollResult + }, + { + name: I18n.t("poll.ui_builder.poll_result.vote"), + value: votePollResult + }, + { + name: I18n.t("poll.ui_builder.poll_result.closed"), + value: closedPollResult + } + ]; + }, + @computed("pollType", "regularPollType") isRegular(pollType, regularPollType) { return pollType === regularPollType; @@ -128,6 +150,7 @@ export default Ember.Controller.extend({ "isNumber", "showMinMax", "pollType", + "pollResult", "publicPoll", "pollOptions", "pollMin", @@ -141,6 +164,7 @@ export default Ember.Controller.extend({ isNumber, showMinMax, pollType, + pollResult, publicPoll, pollOptions, pollMin, @@ -167,6 +191,7 @@ export default Ember.Controller.extend({ } if (pollType) pollHeader += ` type=${pollType}`; + if (pollResult) pollHeader += ` results=${pollResult}`; if (pollMin && showMinMax) pollHeader += ` min=${pollMin}`; if (pollMax) pollHeader += ` max=${pollMax}`; if (isNumber) pollHeader += ` step=${step}`; diff --git a/plugins/poll/assets/javascripts/discourse/templates/modal/poll-ui-builder.hbs b/plugins/poll/assets/javascripts/discourse/templates/modal/poll-ui-builder.hbs index 04eb9a61ea3..891dd5ba9af 100644 --- a/plugins/poll/assets/javascripts/discourse/templates/modal/poll-ui-builder.hbs +++ b/plugins/poll/assets/javascripts/discourse/templates/modal/poll-ui-builder.hbs @@ -8,6 +8,15 @@ valueAttribute="value"}} +
+ + {{combo-box content=pollResults + value=pollResult + allowInitialValueMutation=true + valueAttribute="value"}} +
+ + {{#if showMinMax}}
{{input-tip validation=minMaxValueValidation}} 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 2ea99f1374b..760c3390b5d 100644 --- a/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 +++ b/plugins/poll/assets/javascripts/initializers/extend-for-poll.js.es6 @@ -39,12 +39,12 @@ function initializePolls(api) { const polls = this.get("polls"); if (polls) { this._polls = this._polls || {}; - _.map(polls, (v, k) => { - const existing = this._polls[k]; + polls.forEach(p => { + const existing = this._polls[p.name]; if (existing) { - this._polls[k].setProperties(v); + this._polls[p.name].setProperties(p); } else { - this._polls[k] = Em.Object.create(v); + this._polls[p.name] = Em.Object.create(p); } }); this.set("pollsObject", this._polls); @@ -81,14 +81,11 @@ function initializePolls(api) { const pollName = $poll.data("poll-name"); const poll = polls[pollName]; if (poll) { - const isMultiple = poll.get("type") === "multiple"; - const glue = new WidgetGlue("discourse-poll", register, { id: `${pollName}-${post.id}`, post, poll, - vote: votes[pollName] || [], - isMultiple + vote: votes[pollName] || [] }); glue.appendTo(pollElem); _glued.push(glue); diff --git a/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 b/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 index 4b5de264317..aef569fb765 100644 --- a/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 +++ b/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 @@ -3,15 +3,16 @@ const DATA_PREFIX = "data-poll-"; const DEFAULT_POLL_NAME = "poll"; const WHITELISTED_ATTRIBUTES = [ - "type", - "name", - "min", + "close", "max", - "step", + "min", + "name", "order", - "status", "public", - "close" + "results", + "status", + "step", + "type", ]; function replaceToken(tokens, target, list) { diff --git a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 index 2925a22d985..7d569edc432 100644 --- a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 +++ b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 @@ -13,11 +13,14 @@ function optionHtml(option) { return new RawHtml({ html: `${option.html}` }); } -function fetchVoters(payload) { - return ajax("/polls/voters.json", { - type: "get", - data: payload - }).catch(error => { +function infoTextHtml(text) { + return new RawHtml({ + html: `${text}` + }); +} + +function _fetchVoters(data) { + return ajax("/polls/voters.json", { data }).catch(error => { if (error) { popupAjaxError(error); } else { @@ -34,19 +37,20 @@ createWidget("discourse-poll-option", { }, html(attrs) { - const result = []; + const contents = []; const { option, vote } = attrs; - const chosen = vote.indexOf(option.id) !== -1; + const chosen = vote.includes(option.id); if (attrs.isMultiple) { - result.push(iconNode(chosen ? "check-square-o" : "square-o")); + contents.push(iconNode(chosen ? "check-square-o" : "square-o")); } else { - result.push(iconNode(chosen ? "dot-circle-o" : "circle-o")); + contents.push(iconNode(chosen ? "dot-circle-o" : "circle-o")); } - result.push(" "); - result.push(optionHtml(option)); - return result; + contents.push(" "); + contents.push(optionHtml(option)); + + return contents; }, click(e) { @@ -58,7 +62,7 @@ createWidget("discourse-poll-option", { createWidget("discourse-poll-load-more", { tagName: "div.poll-voters-toggle-expand", - buildKey: attrs => `${attrs.id}-load-more`, + buildKey: attrs => `load-more-${attrs.optionId}`, defaultState() { return { loading: false }; @@ -72,50 +76,45 @@ createWidget("discourse-poll-load-more", { click() { const { state } = this; - if (state.loading) { - return; - } + + if (state.loading) return; state.loading = true; return this.sendWidgetAction("loadMore").finally( - () => (state.loading = false) + () => state.loading = false ); } }); createWidget("discourse-poll-voters", { tagName: "ul.poll-voters-list", - buildKey: attrs => attrs.id(), + buildKey: attrs => `poll-voters-${attrs.optionId}`, defaultState() { return { loaded: "new", - pollVoters: [], - offset: 1 + voters: [], + page: 1 }; }, fetchVoters() { const { attrs, state } = this; - if (state.loaded === "loading") { - return; - } + if (state.loaded === "loading") return; state.loaded = "loading"; - return fetchVoters({ + return _fetchVoters({ post_id: attrs.postId, poll_name: attrs.pollName, option_id: attrs.optionId, - offset: state.offset + page: state.page }).then(result => { state.loaded = "loaded"; - state.offset += 1; + state.page += 1; - const pollResult = result[attrs.pollName]; - const newVoters = - attrs.pollType === "number" ? pollResult : pollResult[attrs.optionId]; - state.pollVoters = state.pollVoters.concat(newVoters); + const newVoters = attrs.pollType === "number" ? result.voters : result.voters[attrs.optionId]; + state.voters = [...new Set([...state.voters, ...newVoters])]; this.scheduleRerender(); }); @@ -126,11 +125,11 @@ createWidget("discourse-poll-voters", { }, html(attrs, state) { - if (attrs.pollVoters && state.loaded === "new") { - state.pollVoters = attrs.pollVoters; + if (attrs.voters && state.loaded === "new") { + state.voters = attrs.voters; } - const contents = state.pollVoters.map(user => { + const contents = state.voters.map(user => { return h("li", [ avatarFor("tiny", { username: user.username, @@ -140,10 +139,8 @@ createWidget("discourse-poll-voters", { ]); }); - if (state.pollVoters.length < attrs.totalVotes) { - contents.push( - this.attach("discourse-poll-load-more", { id: attrs.id() }) - ); + if (state.voters.length < attrs.totalVotes) { + contents.push(this.attach("discourse-poll-load-more", attrs)); } return h("div.poll-voters", contents); @@ -152,27 +149,22 @@ createWidget("discourse-poll-voters", { createWidget("discourse-poll-standard-results", { tagName: "ul.results", - buildKey: attrs => `${attrs.id}-standard-results`, + buildKey: attrs => `poll-standard-results-${attrs.id}`, defaultState() { - return { - loaded: "new" - }; + return { loaded: false }; }, fetchVoters() { const { attrs, state } = this; - if (state.loaded === "new") { - fetchVoters({ - post_id: attrs.post.id, - poll_name: attrs.poll.get("name") - }).then(result => { - state.voters = result[attrs.poll.get("name")]; - state.loaded = "loaded"; - this.scheduleRerender(); - }); - } + return _fetchVoters({ + post_id: attrs.post.id, + poll_name: attrs.poll.get("name") + }).then(result => { + state.voters = result.voters; + this.scheduleRerender(); + }); }, html(attrs, state) { @@ -197,6 +189,11 @@ createWidget("discourse-poll-standard-results", { } }); + if (isPublic && !state.loaded) { + state.loaded = true; + this.fetchVoters(); + } + const percentages = voters === 0 ? Array(ordered.length).fill(0) @@ -206,8 +203,6 @@ createWidget("discourse-poll-standard-results", { ? percentages.map(Math.floor) : evenRound(percentages); - if (isPublic) this.fetchVoters(); - return ordered.map((option, idx) => { const contents = []; const per = rounded[idx].toString(); @@ -230,12 +225,11 @@ createWidget("discourse-poll-standard-results", { if (isPublic) { contents.push( this.attach("discourse-poll-voters", { - id: () => `poll-voters-${option.id}`, postId: attrs.post.id, optionId: option.id, pollName: poll.get("name"), totalVotes: option.votes, - pollVoters: (state.voters && state.voters[option.id]) || [] + voters: (state.voters && state.voters[option.id]) || [] }) ); } @@ -247,55 +241,51 @@ createWidget("discourse-poll-standard-results", { }); createWidget("discourse-poll-number-results", { - buildKey: attrs => `${attrs.id}-number-results`, + buildKey: attrs => `poll-number-results-${attrs.id}`, defaultState() { - return { - loaded: "new" - }; + return { loaded: false }; }, fetchVoters() { const { attrs, state } = this; - if (state.loaded === "new") { - fetchVoters({ - post_id: attrs.post.id, - poll_name: attrs.poll.get("name") - }).then(result => { - state.voters = result[attrs.poll.get("name")]; - state.loaded = "loaded"; - this.scheduleRerender(); - }); - } + return _fetchVoters({ + post_id: attrs.post.id, + poll_name: attrs.poll.get("name") + }).then(result => { + state.voters = result.voters; + this.scheduleRerender(); + }); }, html(attrs, state) { const { poll } = attrs; - const isPublic = poll.get("public"); const totalScore = poll.get("options").reduce((total, o) => { return total + parseInt(o.html, 10) * parseInt(o.votes, 10); }, 0); - const voters = poll.voters; + const voters = poll.get("voters"); const average = voters === 0 ? 0 : round(totalScore / voters, -2); const averageRating = I18n.t("poll.average_rating", { average }); - const results = [ + const contents = [ h( "div.poll-results-number-rating", new RawHtml({ html: `${averageRating}` }) ) ]; - if (isPublic) { - this.fetchVoters(); + if (poll.get("public")) { + if (!state.loaded) { + state.loaded = true; + this.fetchVoters(); + } - results.push( + contents.push( this.attach("discourse-poll-voters", { - id: () => `poll-voters-${poll.get("name")}`, totalVotes: poll.get("voters"), - pollVoters: state.voters || [], + voters: state.voters || [], postId: attrs.post.id, pollName: poll.get("name"), pollType: poll.get("type") @@ -303,22 +293,21 @@ createWidget("discourse-poll-number-results", { ); } - return results; + return contents; } }); createWidget("discourse-poll-container", { tagName: "div.poll-container", + html(attrs) { const { poll } = attrs; + const options = poll.get("options"); - if (attrs.showResults || attrs.isClosed) { + if (attrs.showResults) { const type = poll.get("type") === "number" ? "number" : "standard"; return this.attach(`discourse-poll-${type}-results`, attrs); - } - - const options = poll.get("options"); - if (options) { + } else if (options) { return h( "ul", options.map(option => { @@ -362,7 +351,7 @@ createWidget("discourse-poll-info", { html(attrs) { const { poll } = attrs; const count = poll.get("voters"); - const result = [ + const contents = [ h("p", [ h("span.info-number", count.toString()), h("span.info-label", I18n.t("poll.voters", { count })) @@ -375,7 +364,7 @@ createWidget("discourse-poll-info", { return total + parseInt(o.votes, 10); }, 0); - result.push( + contents.push( h("p", [ h("span.info-number", totalVotes.toString()), h( @@ -391,37 +380,16 @@ createWidget("discourse-poll-info", { poll.get("options.length") ); if (help) { - result.push( - new RawHtml({ html: `${help}` }) - ); + contents.push(infoTextHtml(help)); } } } - if (!attrs.isClosed) { - if (!attrs.showResults && poll.get("public")) { - result.push(h("span.info-text", I18n.t("poll.public.title"))); - } - - if (poll.close) { - const closeDate = moment.utc(poll.close); - if (closeDate.isValid()) { - const title = closeDate.format("LLL"); - const timeLeft = moment().to(closeDate.local(), true); - - result.push( - new RawHtml({ - html: `${I18n.t( - "poll.automatic_close.closes_in", - { timeLeft } - )}` - }) - ); - } - } + if (!attrs.isClosed && !attrs.showResults && poll.get("public")) { + contents.push(infoTextHtml(I18n.t("poll.public.title"))); } - return result; + return contents; } }); @@ -429,7 +397,7 @@ createWidget("discourse-poll-buttons", { tagName: "div.poll-buttons", html(attrs) { - const results = []; + const contents = []; const { poll, post } = attrs; const topicArchived = post.get("topic.archived"); const closed = attrs.isClosed; @@ -437,7 +405,7 @@ createWidget("discourse-poll-buttons", { if (attrs.isMultiple && !hideResultsDisabled) { const castVotesDisabled = !attrs.canCastVotes; - results.push( + contents.push( this.attach("button", { className: `btn cast-votes ${castVotesDisabled ? "" : "btn-primary"}`, label: "poll.cast-votes.label", @@ -446,11 +414,11 @@ createWidget("discourse-poll-buttons", { action: "castVotes" }) ); - results.push(" "); + contents.push(" "); } if (attrs.showResults || hideResultsDisabled) { - results.push( + contents.push( this.attach("button", { className: "btn toggle-results", label: "poll.hide-results.label", @@ -461,31 +429,44 @@ createWidget("discourse-poll-buttons", { }) ); } else { - results.push( - this.attach("button", { - className: "btn toggle-results", - label: "poll.show-results.label", - title: "poll.show-results.title", - icon: "eye", - disabled: poll.get("voters") === 0, - action: "toggleResults" - }) - ); + if (poll.get("results") === "on_vote" && !attrs.hasVoted) { + contents.push(infoTextHtml(I18n.t("poll.results.vote.title"))); + } else if (poll.get("results") === "on_close" && !closed) { + contents.push(infoTextHtml(I18n.t("poll.results.closed.title"))); + } else { + contents.push( + this.attach("button", { + className: "btn toggle-results", + label: "poll.show-results.label", + title: "poll.show-results.title", + icon: "eye", + disabled: poll.get("voters") === 0, + action: "toggleResults" + }) + ); + } } - if (attrs.isAutomaticallyClosed) { + if (poll.get("close")) { const closeDate = moment.utc(poll.get("close")); - const title = closeDate.format("LLL"); - const age = relativeAge(closeDate.toDate(), { addAgo: true }); + if (closeDate.isValid()) { + const title = closeDate.format("LLL"); + let label; - results.push( - new RawHtml({ - html: `${I18n.t( - "poll.automatic_close.age", - { age } - )}` - }) - ); + if (attrs.isAutomaticallyClosed) { + const age = relativeAge(closeDate.toDate(), { addAgo: true }); + label = I18n.t("poll.automatic_close.age", { age }); + } else { + const timeLeft = moment().to(closeDate.local(), true); + label = I18n.t("poll.automatic_close.closes_in", { timeLeft }); + } + + contents.push( + new RawHtml({ + html: `${label}` + }) + ); + } } if ( @@ -496,7 +477,7 @@ createWidget("discourse-poll-buttons", { ) { if (closed) { if (!attrs.isAutomaticallyClosed) { - results.push( + contents.push( this.attach("button", { className: "btn toggle-status", label: "poll.open.label", @@ -507,7 +488,7 @@ createWidget("discourse-poll-buttons", { ); } } else { - results.push( + contents.push( this.attach("button", { className: "btn toggle-status btn-danger", label: "poll.close.label", @@ -519,39 +500,49 @@ createWidget("discourse-poll-buttons", { } } - return results; + return contents; } }); export default createWidget("discourse-poll", { tagName: "div.poll", - buildKey: attrs => attrs.id, + buildKey: attrs => `poll-${attrs.id}`, buildAttributes(attrs) { - const { poll } = attrs; return { - "data-poll-type": poll.get("type"), - "data-poll-name": poll.get("name"), - "data-poll-status": poll.get("status"), - "data-poll-public": poll.get("public"), - "data-poll-close": poll.get("close") + "data-poll-name": attrs.poll.get("name"), + "data-poll-type": attrs.poll.get("type") }; }, defaultState(attrs) { - const showResults = this.isClosed() || attrs.post.get("topic.archived"); + const { post, poll } = attrs; + + const showResults = ( + post.get("topic.archived") || + this.isClosed() || + (poll.get("results") !== "on_close" && this.hasVoted()) + ); + return { loading: false, showResults }; }, html(attrs, state) { - const { showResults } = state; + const showResults = ( + state.showResults || + attrs.post.get("topic.archived") || + this.isClosed() + ); + const newAttrs = jQuery.extend({}, attrs, { - showResults, canCastVotes: this.canCastVotes(), - isClosed: this.isClosed(), + hasVoted: this.hasVoted(), isAutomaticallyClosed: this.isAutomaticallyClosed(), + isClosed: this.isClosed(), + isMultiple: this.isMultiple(), + max: this.max(), min: this.min(), - max: this.max() + showResults, }); return h("div", [ @@ -562,7 +553,7 @@ export default createWidget("discourse-poll", { }, min() { - let min = parseInt(this.attrs.poll.min, 10); + let min = parseInt(this.attrs.poll.get("min"), 10); if (isNaN(min) || min < 1) { min = 1; } @@ -570,8 +561,8 @@ export default createWidget("discourse-poll", { }, max() { - let max = parseInt(this.attrs.poll.max, 10); - const numOptions = this.attrs.poll.options.length; + let max = parseInt(this.attrs.poll.get("max"), 10); + const numOptions = this.attrs.poll.get("options.length"); if (isNaN(max) || max > numOptions) { max = numOptions; } @@ -588,6 +579,16 @@ export default createWidget("discourse-poll", { return poll.get("status") === "closed" || this.isAutomaticallyClosed(); }, + isMultiple() { + const { poll } = this.attrs; + return poll.get("type") === "multiple"; + }, + + hasVoted() { + const { vote } = this.attrs; + return vote && vote.length > 0; + }, + canCastVotes() { const { state, attrs } = this; @@ -597,7 +598,7 @@ export default createWidget("discourse-poll", { const selectedOptionCount = attrs.vote.length; - if (attrs.isMultiple) { + if (this.isMultiple()) { return ( selectedOptionCount >= this.min() && selectedOptionCount <= this.max() ); @@ -630,21 +631,21 @@ export default createWidget("discourse-poll", { poll_name: poll.get("name"), status } - }) - .then(() => { - poll.set("status", status); - this.scheduleRerender(); - }) - .catch(error => { - if (error) { - popupAjaxError(error); - } else { - bootbox.alert(I18n.t("poll.error_while_toggling_status")); - } - }) - .finally(() => { - state.loading = false; - }); + }).then(() => { + poll.set("status", status); + if (poll.get("results") === "on_close") { + state.showResults = status === "closed"; + } + this.scheduleRerender(); + }).catch(error => { + if (error) { + popupAjaxError(error); + } else { + bootbox.alert(I18n.t("poll.error_while_toggling_status")); + } + }).finally(() => { + state.loading = false; + }); } } ); @@ -661,17 +662,13 @@ export default createWidget("discourse-poll", { toggleOption(option) { const { attrs } = this; - if (this.isClosed()) { - return; - } - if (!this.currentUser) { - this.showLogin(); - } + if (this.isClosed()) return; + if (!this.currentUser) return this.showLogin(); const { vote } = attrs; - const chosenIdx = vote.indexOf(option.id); - if (!attrs.isMultiple) { + + if (!this.isMultiple()) { vote.length = 0; } @@ -681,18 +678,14 @@ export default createWidget("discourse-poll", { vote.push(option.id); } - if (!attrs.isMultiple) { + if (!this.isMultiple()) { return this.castVotes(); } }, castVotes() { - if (!this.canCastVotes()) { - return; - } - if (!this.currentUser) { - return this.showLogin(); - } + if (!this.canCastVotes()) return; + if (!this.currentUser) return this.showLogin(); const { attrs, state } = this; @@ -702,22 +695,22 @@ export default createWidget("discourse-poll", { type: "PUT", data: { post_id: attrs.post.id, - poll_name: attrs.poll.name, + poll_name: attrs.poll.get("name"), options: attrs.vote } - }) - .then(() => { + }).then(({ poll }) => { + attrs.poll.setProperties(poll); + if (attrs.poll.get("results") !== "on_close") { state.showResults = true; - }) - .catch(error => { - if (error) { - popupAjaxError(error); - } else { - bootbox.alert(I18n.t("poll.error_while_casting_votes")); - } - }) - .finally(() => { - state.loading = false; - }); + } + }).catch(error => { + if (error) { + popupAjaxError(error); + } else { + bootbox.alert(I18n.t("poll.error_while_casting_votes")); + } + }).finally(() => { + state.loading = false; + }); } }); diff --git a/plugins/poll/assets/stylesheets/common/poll-ui-builder.scss b/plugins/poll/assets/stylesheets/common/poll-ui-builder.scss index 404074d6100..723f332ef44 100644 --- a/plugins/poll/assets/stylesheets/common/poll-ui-builder.scss +++ b/plugins/poll/assets/stylesheets/common/poll-ui-builder.scss @@ -8,12 +8,15 @@ .tip { display: block; - min-height: 20px; } .input-group-label { display: inline-block; - width: 45px; + min-width: 55px; + } + + .poll-select { + line-height: 3em; } .poll-number { diff --git a/plugins/poll/assets/stylesheets/common/poll.scss b/plugins/poll/assets/stylesheets/common/poll.scss index df8d719f46c..8510fe6c56b 100644 --- a/plugins/poll/assets/stylesheets/common/poll.scss +++ b/plugins/poll/assets/stylesheets/common/poll.scss @@ -58,9 +58,6 @@ div.poll { } .poll-buttons { - button { - float: none; - } .info-text { margin: 0 5px; color: $primary-medium; @@ -81,6 +78,7 @@ div.poll { .results { > li { + cursor: default; padding: 0.5em 0.7em 0.7em 0.5em; } .option { diff --git a/plugins/poll/assets/stylesheets/desktop/poll.scss b/plugins/poll/assets/stylesheets/desktop/poll.scss index e6ac930025a..7283964d9e5 100644 --- a/plugins/poll/assets/stylesheets/desktop/poll.scss +++ b/plugins/poll/assets/stylesheets/desktop/poll.scss @@ -32,6 +32,10 @@ div.poll { border-top: 1px solid $primary-low; padding: 1em 1.25em; + .info-text { + line-height: 2em; + } + .toggle-status { float: right; } diff --git a/plugins/poll/config/locales/client.en.yml b/plugins/poll/config/locales/client.en.yml index 7e03127b051..a44a852cb6a 100644 --- a/plugins/poll/config/locales/client.en.yml +++ b/plugins/poll/config/locales/client.en.yml @@ -28,7 +28,13 @@ en: average_rating: "Average rating: %{average}." public: - title: "Votes are public." + title: "Votes are public." + + results: + vote: + title: "Results will be shown on vote." + closed: + title: "Results will be shown once closed." multiple: help: @@ -85,6 +91,11 @@ en: regular: Single Choice multiple: Multiple Choice number: Number Rating + poll_result: + label: Results + always: Always visible + vote: On vote + closed: When closed poll_config: max: Max min: Min diff --git a/plugins/poll/config/locales/server.en.yml b/plugins/poll/config/locales/server.en.yml index 45859bb1894..6d5274d6413 100644 --- a/plugins/poll/config/locales/server.en.yml +++ b/plugins/poll/config/locales/server.en.yml @@ -43,14 +43,10 @@ en: requires_at_least_1_valid_option: "You must select at least 1 valid option." - default_cannot_be_made_public: "Poll with votes cannot be made public." - named_cannot_be_made_public: "Poll named %{name} has votes and cannot be made public." - edit_window_expired: - op_cannot_edit_options: "You cannot add or remove poll options after the first %{minutes} minutes. Please contact a moderator if you need to edit a poll option." - staff_cannot_add_or_remove_options: "You cannot add or remove poll options after the first %{minutes} minutes. You should close this topic and create a new one instead." + cannot_edit_default_poll_with_votes: "You cannot change a poll after the first %{minutes} minutes." + cannot_edit_named_poll_with_votes: "You cannot change the poll name ${name} after the first %{minutes} minutes." - no_polls_associated_with_this_post: "No polls are associated with this post." no_poll_with_this_name: "No poll named %{name} associated with this post." post_is_deleted: "Cannot act on a deleted post." diff --git a/plugins/poll/config/settings.yml b/plugins/poll/config/settings.yml index 5f4cfa825b7..24bd92e0a1c 100644 --- a/plugins/poll/config/settings.yml +++ b/plugins/poll/config/settings.yml @@ -4,9 +4,12 @@ plugins: client: true poll_maximum_options: default: 20 + min: 2 + max: 100 client: true poll_edit_window_mins: default: 5 + min: 0 poll_minimum_trust_level_to_create: default: 1 client: true diff --git a/plugins/poll/db/migrate/20180820073549_create_polls_tables.rb b/plugins/poll/db/migrate/20180820073549_create_polls_tables.rb new file mode 100644 index 00000000000..99a1229c083 --- /dev/null +++ b/plugins/poll/db/migrate/20180820073549_create_polls_tables.rb @@ -0,0 +1,39 @@ +class CreatePollsTables < ActiveRecord::Migration[5.2] + def change + create_table :polls do |t| + t.references :post, index: true, foreign_key: true + t.string :name, null: false, default: "poll" + t.datetime :close_at + t.integer :type, null: false, default: 0 + t.integer :status, null: false, default: 0 + t.integer :results, null: false, default: 0 + t.integer :visibility, null: false, default: 0 + t.integer :min + t.integer :max + t.integer :step + t.integer :anonymous_voters + t.timestamps + end + + add_index :polls, [:post_id, :name], unique: true + + create_table :poll_options do |t| + t.references :poll, index: true, foreign_key: true + t.string :digest, null: false + t.text :html, null: false + t.integer :anonymous_votes + t.timestamps + end + + add_index :poll_options, [:poll_id, :digest], unique: true + + create_table :poll_votes, id: false do |t| + t.references :poll, foreign_key: true + t.references :poll_option, foreign_key: true + t.references :user, foreign_key: true + t.timestamps + end + + add_index :poll_votes, [:poll_id, :poll_option_id, :user_id], unique: true + end +end diff --git a/plugins/poll/db/post_migrate/20180820080623_migrate_polls_data.rb b/plugins/poll/db/post_migrate/20180820080623_migrate_polls_data.rb new file mode 100644 index 00000000000..2cab457a89e --- /dev/null +++ b/plugins/poll/db/post_migrate/20180820080623_migrate_polls_data.rb @@ -0,0 +1,159 @@ +class MigratePollsData < ActiveRecord::Migration[5.2] + def escape(text) + PG::Connection.escape_string(text) + end + + POLL_TYPES ||= { + "regular" => 0, + "multiple" => 1, + "number" => 2, + } + + def up + # Ensure we don't have duplicate polls + DB.exec <<~SQL + WITH duplicates AS ( + SELECT id, row_number() OVER (PARTITION BY post_id) r + FROM post_custom_fields + WHERE name = 'polls' + ORDER BY created_at + ) + DELETE FROM post_custom_fields + WHERE id IN (SELECT id FROM duplicates WHERE r > 1) + SQL + + # Ensure we don't have duplicate votes + DB.exec <<~SQL + WITH duplicates AS ( + SELECT id, row_number() OVER (PARTITION BY post_id) r + FROM post_custom_fields + WHERE name = 'polls-votes' + ORDER BY created_at + ) + DELETE FROM post_custom_fields + WHERE id IN (SELECT id FROM duplicates WHERE r > 1) + SQL + + # Ensure we have votes records + DB.exec <<~SQL + INSERT INTO post_custom_fields (post_id, name, value, created_at, updated_at) + SELECT post_id, 'polls-votes', '{}', created_at, updated_at + FROM post_custom_fields + WHERE name = 'polls' + AND post_id NOT IN (SELECT post_id FROM post_custom_fields WHERE name = 'polls-votes') + SQL + + sql = <<~SQL + SELECT polls.post_id + , polls.created_at + , polls.updated_at + , polls.value::json "polls" + , votes.value::json "votes" + FROM post_custom_fields polls + JOIN post_custom_fields votes + ON polls.post_id = votes.post_id + WHERE polls.name = 'polls' + AND votes.name = 'polls-votes' + ORDER BY polls.post_id + SQL + + DB.query(sql).each do |r| + existing_user_ids = User.where(id: r.votes.keys).pluck(:id).to_set + + # Poll votes are stored in a JSON object with the following hierarchy + # user_id -> poll_name -> options + # Since we're iterating over polls, we need to change the hierarchy to + # poll_name -> user_id -> options + + votes = {} + r.votes.each do |user_id, user_votes| + # don't migrate votes from deleted/non-existing users + next unless existing_user_ids.include?(user_id.to_i) + + user_votes.each do |poll_name, options| + votes[poll_name] ||= {} + votes[poll_name][user_id] = options + end + end + + r.polls.values.each do |poll| + name = poll["name"].presence || "poll" + type = POLL_TYPES[(poll["type"].presence || "")[/(regular|multiple|number)/, 1] || "regular"] + status = poll["status"] == "open" ? 0 : 1 + visibility = poll["public"] == "true" ? 1 : 0 + close_at = (Time.zone.parse(poll["close"]) rescue nil) + min = poll["min"].to_i + max = poll["max"].to_i + step = poll["step"].to_i + anonymous_voters = poll["anonymous_voters"].to_i + + poll_id = execute(<<~SQL + INSERT INTO polls ( + post_id, + name, + type, + status, + visibility, + close_at, + min, + max, + step, + anonymous_voters, + created_at, + updated_at + ) VALUES ( + #{r.post_id}, + '#{escape(name)}', + #{type}, + #{status}, + #{visibility}, + #{close_at ? "'#{close_at}'" : "NULL"}, + #{min > 0 ? min : "NULL"}, + #{max > min ? max : "NULL"}, + #{step > 0 ? step : "NULL"}, + #{anonymous_voters > 0 ? anonymous_voters : "NULL"}, + '#{r.created_at}', + '#{r.updated_at}' + ) RETURNING id + SQL + )[0]["id"] + + option_ids = Hash[*DB.query_single(<<~SQL + INSERT INTO poll_options + (poll_id, digest, html, anonymous_votes, created_at, updated_at) + VALUES + #{poll["options"].map { |option| + "(#{poll_id}, '#{escape(option["id"])}', '#{escape(option["html"].strip)}', #{option["anonymous_votes"].to_i}, '#{r.created_at}', '#{r.updated_at}')" }.join(",") + } + RETURNING digest, id + SQL + )] + + if votes[name].present? + poll_votes = votes[name].map do |user_id, options| + options + .select { |o| option_ids.has_key?(o) } + .map { |o| "(#{poll_id}, #{option_ids[o]}, #{user_id.to_i}, '#{r.created_at}', '#{r.updated_at}')" } + end.flatten + + if poll_votes.present? + execute <<~SQL + INSERT INTO poll_votes (poll_id, poll_option_id, user_id, created_at, updated_at) + VALUES #{poll_votes.join(",")} + SQL + end + end + end + end + + execute <<~SQL + INSERT INTO post_custom_fields (name, value, post_id, created_at, updated_at) + SELECT 'has_polls', 't', post_id, MIN(created_at), MIN(updated_at) + FROM polls + GROUP BY post_id + SQL + end + + def down + end +end diff --git a/plugins/poll/lib/polls_updater.rb b/plugins/poll/lib/polls_updater.rb index 50a4bb81b44..e697728072b 100644 --- a/plugins/poll/lib/polls_updater.rb +++ b/plugins/poll/lib/polls_updater.rb @@ -1,132 +1,115 @@ +# frozen_string_literal: true + module DiscoursePoll class PollsUpdater - VALID_POLLS_CONFIGS = %w{type min max public close}.map(&:freeze) + + POLL_ATTRIBUTES ||= %w{close_at max min results status step type visibility} def self.update(post, polls) - # load previous polls - previous_polls = post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] || {} + ::Poll.transaction do + has_changed = false + edit_window = SiteSetting.poll_edit_window_mins - # extract options - current_option_ids = extract_option_ids(polls) - previous_option_ids = extract_option_ids(previous_polls) + old_poll_names = ::Poll.where(post: post).pluck(:name) + new_poll_names = polls.keys - # are the polls different? - if polls_updated?(polls, previous_polls) || (current_option_ids != previous_option_ids) - has_votes = total_votes(previous_polls) > 0 + deleted_poll_names = old_poll_names - new_poll_names + created_poll_names = new_poll_names - old_poll_names - # outside of the edit window? - poll_edit_window_mins = SiteSetting.poll_edit_window_mins + # delete polls + if deleted_poll_names.present? + ::Poll.where(post: post, name: deleted_poll_names).destroy_all + end - if post.created_at < poll_edit_window_mins.minutes.ago && has_votes - # deal with option changes - if User.staff.where(id: post.last_editor_id).exists? - # staff can edit options - polls.each_key do |poll_name| - if polls.dig(poll_name, "options")&.size != previous_polls.dig(poll_name, "options")&.size && previous_polls.dig(poll_name, "voters").to_i > 0 - post.errors.add(:base, I18n.t("poll.edit_window_expired.staff_cannot_add_or_remove_options", minutes: poll_edit_window_mins)) + # create polls + if created_poll_names.present? + has_changed = true + polls.slice(*created_poll_names).values.each do |poll| + Poll.create!(post.id, poll) + end + end + + # update polls + ::Poll.includes(:poll_votes, :poll_options).where(post: post).find_each do |old_poll| + new_poll = polls[old_poll.name] + new_poll_options = new_poll["options"] + + attributes = new_poll.slice(*POLL_ATTRIBUTES) + attributes["visibility"] = new_poll["public"] == "true" ? "everyone" : "secret" + attributes["close_at"] = Time.zone.parse(new_poll["close"]) rescue nil + poll = ::Poll.new(attributes) + + if is_different?(old_poll, poll, new_poll_options) + + # only prevent changes when there's at least 1 vote + if old_poll.poll_votes.size > 0 + # can't change after edit window (when enabled) + if edit_window > 0 && old_poll.created_at < edit_window.minutes.ago + error = poll.name == DiscoursePoll::DEFAULT_POLL_NAME ? + I18n.t("poll.edit_window_expired.cannot_edit_default_poll_with_votes", minutes: edit_window) : + I18n.t("poll.edit_window_expired.cannot_edit_named_poll_with_votes", minutes: edit_window, name: poll.name) + + post.errors.add(:base, error) return end end - else - # OP cannot edit poll options - post.errors.add(:base, I18n.t("poll.edit_window_expired.op_cannot_edit_options", minutes: poll_edit_window_mins)) - return + + # update poll + POLL_ATTRIBUTES.each do |attr| + old_poll.send("#{attr}=", poll.send(attr)) + end + old_poll.save! + + # keep track of anonymous votes + anonymous_votes = old_poll.poll_options.map { |pv| [pv.digest, pv.anonymous_votes] }.to_h + + # destroy existing options & votes + ::PollOption.where(poll: old_poll).destroy_all + + # create new options + new_poll_options.each do |option| + ::PollOption.create!( + poll: old_poll, + digest: option["id"], + html: option["html"].strip, + anonymous_votes: anonymous_votes[option["id"]], + ) + end + + has_changed = true end end - # try to merge votes - polls.each_key do |poll_name| - next unless previous_polls.has_key?(poll_name) - return if has_votes && private_to_public_poll?(post, previous_polls, polls, poll_name) - - # when the # of options has changed, reset all the votes - if polls[poll_name]["options"].size != previous_polls[poll_name]["options"].size - PostCustomField.where(post_id: post.id, name: DiscoursePoll::VOTES_CUSTOM_FIELD).destroy_all - post.clear_custom_fields - next - end - - polls[poll_name]["voters"] = previous_polls[poll_name]["voters"] - - if previous_polls[poll_name].has_key?("anonymous_voters") - polls[poll_name]["anonymous_voters"] = previous_polls[poll_name]["anonymous_voters"] - end - - previous_options = previous_polls[poll_name]["options"] - public_poll = polls[poll_name]["public"] == "true" - - polls[poll_name]["options"].each_with_index do |option, index| - previous_option = previous_options[index] - option["votes"] = previous_option["votes"] - - if previous_option["id"] != option["id"] - if votes_fields = post.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD] - votes_fields.each do |key, value| - next unless value[poll_name] - index = value[poll_name].index(previous_option["id"]) - votes_fields[key][poll_name][index] = option["id"] if index - end - end - end - - if previous_option.has_key?("anonymous_votes") - option["anonymous_votes"] = previous_option["anonymous_votes"] - end - - if public_poll && previous_option.has_key?("voter_ids") - option["voter_ids"] = previous_option["voter_ids"] - end - end + if ::Poll.exists?(post: post) + post.custom_fields[HAS_POLLS] = true + else + post.custom_fields.delete(HAS_POLLS) end - # immediately store the polls - post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] = polls post.save_custom_fields(true) - # re-schedule jobs - DiscoursePoll::Poll.schedule_jobs(post) - - # publish the changes - MessageBus.publish("/polls/#{post.topic_id}", post_id: post.id, polls: polls) - end - end - - def self.polls_updated?(current_polls, previous_polls) - return true if (current_polls.keys.sort != previous_polls.keys.sort) - - current_polls.each_key do |poll_name| - if !previous_polls[poll_name] || (current_polls[poll_name].values_at(*VALID_POLLS_CONFIGS) != previous_polls[poll_name].values_at(*VALID_POLLS_CONFIGS)) - return true + if has_changed + polls = ::Poll.includes(poll_options: :poll_votes).where(post: post) + polls = ActiveModel::ArraySerializer.new(polls, each_serializer: PollSerializer, root: false).as_json + MessageBus.publish("/polls/#{post.topic_id}", post_id: post.id, polls: polls) end end - - false - end - - def self.extract_option_ids(polls) - polls.values.map { |p| p["options"].map { |o| o["id"] } }.flatten.sort - end - - def self.total_votes(polls) - polls.map { |key, value| value["voters"].to_i }.sum end private - def self.private_to_public_poll?(post, previous_polls, current_polls, poll_name) - previous_poll = previous_polls[poll_name] - current_poll = current_polls[poll_name] - - if previous_poll["public"].nil? && current_poll["public"] == "true" - error = poll_name == DiscoursePoll::DEFAULT_POLL_NAME ? - I18n.t("poll.default_cannot_be_made_public") : - I18n.t("poll.named_cannot_be_made_public", name: poll_name) - - post.errors.add(:base, error) - return true + def self.is_different?(old_poll, new_poll, new_options) + # an attribute was changed? + POLL_ATTRIBUTES.each do |attr| + return true if old_poll.send(attr) != new_poll.send(attr) end + # an option was changed? + return true if old_poll.poll_options.map { |o| o.digest }.sort != new_options.map { |o| o["id"] }.sort + + # it's the same! false end + end end diff --git a/plugins/poll/lib/polls_validator.rb b/plugins/poll/lib/polls_validator.rb index 26c8a9c5b01..790d8e2c010 100644 --- a/plugins/poll/lib/polls_validator.rb +++ b/plugins/poll/lib/polls_validator.rb @@ -8,22 +8,11 @@ module DiscoursePoll polls = {} DiscoursePoll::Poll::extract(@post.raw, @post.topic_id, @post.user_id).each do |poll| - # polls should have a unique name return false unless unique_poll_name?(polls, poll) - - # options must be unique return false unless unique_options?(poll) - - # at least 2 options return false unless at_least_two_options?(poll) - - # maximum # of options return false unless valid_number_of_options?(poll) - - # poll with multiple choices return false unless valid_multiple_choice_settings?(poll) - - # store the valid poll polls[poll["name"]] = poll end @@ -90,11 +79,11 @@ module DiscoursePoll def valid_multiple_choice_settings?(poll) if poll["type"] == "multiple" - num_of_options = poll["options"].size + options = poll["options"].size min = (poll["min"].presence || 1).to_i - max = (poll["max"].presence || num_of_options).to_i + max = (poll["max"].presence || options).to_i - if min > max || min <= 0 || max <= 0 || max > num_of_options || min >= num_of_options + if min > max || min <= 0 || max <= 0 || max > options || min >= options if poll["name"] == ::DiscoursePoll::DEFAULT_POLL_NAME @post.errors.add(:base, I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")) else diff --git a/plugins/poll/lib/votes_updater.rb b/plugins/poll/lib/votes_updater.rb deleted file mode 100644 index ae705d51135..00000000000 --- a/plugins/poll/lib/votes_updater.rb +++ /dev/null @@ -1,59 +0,0 @@ -module DiscoursePoll - class VotesUpdater - def self.merge_users!(source_user, target_user) - post_ids = PostCustomField.where(name: DiscoursePoll::VOTES_CUSTOM_FIELD) - .where("value :: JSON -> ? IS NOT NULL", source_user.id.to_s) - .pluck(:post_id) - - post_ids.each do |post_id| - DistributedMutex.synchronize("discourse_poll-#{post_id}") do - post = Post.find_by(id: post_id) - update_votes(post, source_user, target_user) if post - end - end - end - - def self.update_votes(post, source_user, target_user) - polls = post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] - votes = post.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD] - return if polls.nil? || votes.nil? || !votes.has_key?(source_user.id.to_s) - - if votes.has_key?(target_user.id.to_s) - remove_votes(polls, votes, source_user) - else - replace_voter_id(polls, votes, source_user, target_user) - end - - post.save_custom_fields(true) - end - - def self.remove_votes(polls, votes, source_user) - votes.delete(source_user.id.to_s).each do |poll_name, option_ids| - poll = polls[poll_name] - next unless poll && option_ids - - poll["options"].each do |option| - if option_ids.include?(option["id"]) - option["votes"] -= 1 - - voter_ids = option["voter_ids"] - voter_ids.delete(source_user.id) if voter_ids - end - end - end - end - - def self.replace_voter_id(polls, votes, source_user, target_user) - votes[target_user.id.to_s] = votes.delete(source_user.id.to_s) - - polls.each_value do |poll| - next unless poll["public"] == "true" - - poll["options"].each do |option| - voter_ids = option["voter_ids"] - voter_ids << target_user.id if voter_ids&.delete(source_user.id) - end - end - end - end -end diff --git a/plugins/poll/plugin.rb b/plugins/poll/plugin.rb index 21b586f98b2..fd3070b4fb3 100644 --- a/plugins/poll/plugin.rb +++ b/plugins/poll/plugin.rb @@ -1,6 +1,8 @@ +# frozen_string_literal: true + # name: poll # about: Official poll plugin for Discourse -# version: 0.9 +# version: 1.0 # authors: Vikhyat Korrapati (vikhyat), Régis Hanol (zogstrip) # url: https://github.com/discourse/discourse/tree/master/plugins/poll @@ -12,22 +14,26 @@ register_asset "stylesheets/mobile/poll.scss", :mobile enabled_site_setting :poll_enabled hide_plugin if self.respond_to?(:hide_plugin) -PLUGIN_NAME ||= "discourse_poll".freeze -DATA_PREFIX ||= "data-poll-".freeze +PLUGIN_NAME ||= "discourse_poll" +DATA_PREFIX ||= "data-poll-" after_initialize do - require File.expand_path("../jobs/regular/close_poll", __FILE__) + [ + "../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 - DEFAULT_POLL_NAME ||= "poll".freeze - POLLS_CUSTOM_FIELD ||= "polls".freeze - VOTES_CUSTOM_FIELD ||= "polls-votes".freeze - - autoload :PostValidator, "#{Rails.root}/plugins/poll/lib/post_validator" - autoload :PollsValidator, "#{Rails.root}/plugins/poll/lib/polls_validator" - autoload :PollsUpdater, "#{Rails.root}/plugins/poll/lib/polls_updater" - autoload :VotesUpdater, "#{Rails.root}/plugins/poll/lib/votes_updater" + HAS_POLLS ||= "has_polls" + DEFAULT_POLL_NAME ||= "poll" class Engine < ::Rails::Engine engine_name PLUGIN_NAME @@ -39,8 +45,7 @@ after_initialize do class << self def vote(post_id, poll_name, options, user) - DistributedMutex.synchronize("#{PLUGIN_NAME}-#{post_id}") do - user_id = user.id + Poll.transaction do post = Post.find_by(id: post_id) # post must not be deleted @@ -49,85 +54,60 @@ after_initialize do end # topic must not be archived - if post.topic.try(: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 - unless Guardian.new(user).can_create_post?(post.topic) + if !Guardian.new(user).can_create_post?(post.topic) raise StandardError.new I18n.t("poll.user_cant_post_in_topic") end - polls = post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] + poll = Poll.includes(poll_options: :poll_votes).find_by(post_id: post_id, name: poll_name) - raise StandardError.new I18n.t("poll.no_polls_associated_with_this_post") if polls.blank? - - poll = polls[poll_name] - - raise StandardError.new I18n.t("poll.no_poll_with_this_name", name: poll_name) if poll.blank? - raise StandardError.new I18n.t("poll.poll_must_be_open_to_vote") if poll["status"] != "open" - - # ensure no race condition when poll is automatically closed - if poll["close"].present? - close_date = - begin - close_date = Time.zone.parse(poll["close"]) - rescue ArgumentError - end - - raise StandardError.new I18n.t("poll.poll_must_be_open_to_vote") if close_date && close_date <= Time.zone.now - end + 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? # remove options that aren't available in the poll - available_options = poll["options"].map { |o| o["id"] }.to_set + 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? - poll["voters"] = poll["anonymous_voters"] || 0 - all_options = Hash.new(0) - - post.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD] ||= {} - post.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD]["#{user_id}"] ||= {} - post.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD]["#{user_id}"][poll_name] = options - - post.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD].each do |_, user_votes| - next unless votes = user_votes[poll_name] - votes.each { |option| all_options[option] += 1 } - poll["voters"] += 1 if (available_options & votes.to_set).size > 0 + new_option_ids = poll.poll_options.each_with_object([]) do |option, obj| + obj << option.id if options.include?(option.digest) end - public_poll = (poll["public"] == "true") + # remove non-selected votes + PollVote + .where(poll: poll, user: user) + .where.not(poll_option_id: new_option_ids) + .delete_all - poll["options"].each do |option| - anonymous_votes = option["anonymous_votes"] || 0 - option["votes"] = all_options[option["id"]] + anonymous_votes - - if public_poll - option["voter_ids"] ||= [] - - if options.include?(option["id"]) - option["voter_ids"] << user_id if !option["voter_ids"].include?(user_id) - else - option["voter_ids"].delete(user_id) - end + old_option_ids = poll.poll_options.each_with_object([]) do |option, obj| + if option.poll_votes.any? { |v| v.user_id == user.id } + obj << option.id end end - post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] = polls - post.save_custom_fields(true) + # 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 - payload = { post_id: post_id, polls: polls } - payload.merge!(user: UserNameSerializer.new(user).serializable_hash) if public_poll + poll.reload + + serialized_poll = PollSerializer.new(poll, root: false).as_json + payload = { post_id: post_id, polls: [serialized_poll] } MessageBus.publish("/polls/#{post.topic_id}", payload) - [poll, options] + [serialized_poll, options] end end - def toggle_status(post_id, poll_name, status, user_id) - DistributedMutex.synchronize("#{PLUGIN_NAME}-#{post_id}") do + def toggle_status(post_id, poll_name, status, user) + Poll.transaction do post = Post.find_by(id: post_id) # post must not be deleted @@ -136,46 +116,143 @@ after_initialize do end # topic must not be archived - if post.topic.try(:archived) + if post.topic&.archived raise StandardError.new I18n.t("poll.topic_must_be_open_to_toggle_status") end - user = User.find_by(id: user_id) - # either staff member or OP - unless user_id == post.user_id || user.try(:staff?) + unless post.user_id == user&.id || user&.staff? raise StandardError.new I18n.t("poll.only_staff_or_op_can_toggle_status") end - polls = post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] + poll = Poll.find_by(post_id: post_id, name: poll_name) - raise StandardError.new I18n.t("poll.no_polls_associated_with_this_post") if polls.blank? - raise StandardError.new I18n.t("poll.no_poll_with_this_name", name: poll_name) if polls[poll_name].blank? + raise StandardError.new I18n.t("poll.no_poll_with_this_name", name: poll_name) unless poll - polls[poll_name]["status"] = status + poll.status = status + poll.save! - post.save_custom_fields(true) + serialized_poll = PollSerializer.new(poll, root: false).as_json + payload = { post_id: post_id, polls: [serialized_poll] } - MessageBus.publish("/polls/#{post.topic_id}", post_id: post.id, polls: polls) + MessageBus.publish("/polls/#{post.topic_id}", payload) - polls[poll_name] + serialized_poll end end - def schedule_jobs(post) - post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD].each do |name, poll| - Jobs.cancel_scheduled_job(:close_poll, post_id: post.id, poll_name: name) + def voters(post_id, poll_name, user, opts = {}) + post = Post.find_by(id: post_id) + raise Discourse::InvalidParameters.new("post_id is invalid") unless post - if poll["status"] == "open" && poll["close"].present? - close_date = - begin - Time.zone.parse(poll["close"]) - rescue ArgumentError - end + poll = Poll.find_by(post_id: post_id, name: poll_name) + raise Discourse::InvalidParameters.new("poll_name is invalid") unless poll&.can_see_voters?(user) - Jobs.enqueue_at(close_date, :close_poll, post_id: post.id, poll_name: name) if close_date && close_date > Time.zone.now + 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 is invalid") 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 { |v| v.user_id }.to_set + + 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 schedule_jobs(post) + Poll.where(post: post).find_each do |poll| + Jobs.cancel_scheduled_job(:close_poll, poll_id: poll.id) + + if poll.open? && poll.close_at && poll.close_at > Time.zone.now + Jobs.enqueue_at(poll.close_at, :close_poll, poll_id: poll.id) + 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", + results: poll["results"].presence || "always", + min: poll["min"], + max: poll["max"], + step: poll["step"] + ) + + 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) @@ -184,7 +261,7 @@ after_initialize do cooked = PrettyText.cook(raw, topic_id: topic_id, user_id: user_id) Nokogiri::HTML(cooked).css("div.poll").map do |p| - poll = { "options" => [], "voters" => 0, "name" => DiscoursePoll::DEFAULT_POLL_NAME } + poll = { "options" => [], "name" => DiscoursePoll::DEFAULT_POLL_NAME } # attributes p.attributes.values.each do |attribute| @@ -195,8 +272,8 @@ after_initialize do # options p.css("li[#{DATA_PREFIX}option-id]").each do |o| - option_id = o.attributes[DATA_PREFIX + "option-id"].value || "" - poll["options"] << { "id" => option_id, "html" => o.inner_html, "votes" => 0 } + option_id = o.attributes[DATA_PREFIX + "option-id"].value.to_s + poll["options"] << { "id" => option_id, "html" => o.inner_html.strip } end poll @@ -229,10 +306,9 @@ after_initialize do post_id = params.require(:post_id) poll_name = params.require(:poll_name) status = params.require(:status) - user_id = current_user.id begin - poll = DiscoursePoll::Poll.toggle_status(post_id, poll_name, status, user_id) + poll = DiscoursePoll::Poll.toggle_status(post_id, poll_name, status, current_user) render json: { poll: poll } rescue StandardError => e render_json_error e.message @@ -240,70 +316,16 @@ after_initialize do end def voters - post_id = params.require(:post_id) + post_id = params.require(:post_id) poll_name = params.require(:poll_name) - post = Post.find_by(id: post_id) - raise Discourse::InvalidParameters.new("post_id is invalid") if !post - raise Discourse::InvalidParameters.new("no poll exists for this post_id") unless post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] + opts = params.permit(:limit, :page, :option_id) - poll = post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD][poll_name] - raise Discourse::InvalidParameters.new("poll_name is invalid") if !poll - - voter_limit = (params[:voter_limit] || 25).to_i - voter_limit = 0 if voter_limit < 0 - voter_limit = 50 if voter_limit > 50 - - user_ids = [] - options = poll["options"] - - if poll["type"] != "number" - per_option_voters = {} - - options.each do |option| - if (params[:option_id]) - next unless option["id"] == params[:option_id].to_s - end - - next unless option["voter_ids"] - voters = option["voter_ids"].slice((params[:offset].to_i || 0) * voter_limit, voter_limit) - per_option_voters[option["id"]] = Set.new(voters) - user_ids << voters - end - - user_ids.flatten! - user_ids.uniq! - - poll_votes = post.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD] - - result = {} - - User.where(id: user_ids).each do |user| - user_hash = UserNameSerializer.new(user).serializable_hash - - # protect against poorly denormalized data - poll_votes&.dig(user.id.to_s, poll_name)&.each do |option_id| - if (params[:option_id]) - next unless option_id == params[:option_id].to_s - end - - voters = per_option_voters[option_id] - # we may have a user from a different vote - next unless voters.include?(user.id) - - result[option_id] ||= [] - result[option_id] << user_hash - end - end - else - user_ids = options.map { |option| option["voter_ids"] }.sort! - user_ids.flatten! - user_ids.uniq! - user_ids = user_ids.slice((params[:offset].to_i || 0) * voter_limit, voter_limit) - result = User.where(id: user_ids).map { |user| UserNameSerializer.new(user).serializable_hash } + begin + render json: { voters: DiscoursePoll::Poll.voters(post_id, poll_name, current_user, opts) } + rescue StandardError => e + render_json_error e.message end - - render json: { poll_name => result } end end @@ -318,40 +340,41 @@ after_initialize do end Post.class_eval do - attr_accessor :polls + attr_accessor :extracted_polls + + has_many :polls, dependent: :destroy after_save do - next if self.polls.blank? || !self.polls.is_a?(Hash) - + polls = self.extracted_polls + next if polls.blank? || !polls.is_a?(Hash) post = self - polls = self.polls - DistributedMutex.synchronize("#{PLUGIN_NAME}-#{post.id}") do - post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] = polls + Poll.transaction do + polls.values.each do |poll| + DiscoursePoll::Poll.create!(post.id, poll) + end + post.custom_fields[DiscoursePoll::HAS_POLLS] = true post.save_custom_fields(true) end end end validate(:post, :validate_polls) do |force = nil| - # only care when raw has changed! return unless self.raw_changed? || force validator = DiscoursePoll::PollsValidator.new(self) return unless (polls = validator.validate_polls) - if !polls.empty? + if polls.present? validator = DiscoursePoll::PostValidator.new(self) return unless validator.validate_post end # are we updating a post? if self.id.present? - DistributedMutex.synchronize("#{PLUGIN_NAME}-#{self.id}") do - DiscoursePoll::PollsUpdater.update(self, polls) - end + DiscoursePoll::PollsUpdater.update(self, polls) else - self.polls = polls + self.extracted_polls = polls end true @@ -380,13 +403,6 @@ after_initialize do end end - register_post_custom_field_type(DiscoursePoll::POLLS_CUSTOM_FIELD, :json) - register_post_custom_field_type(DiscoursePoll::VOTES_CUSTOM_FIELD, :json) - - topic_view_post_custom_fields_whitelister do |user| - user ? [DiscoursePoll::POLLS_CUSTOM_FIELD, DiscoursePoll::VOTES_CUSTOM_FIELD] : [DiscoursePoll::POLLS_CUSTOM_FIELD] - end - on(:reduce_cooked) do |fragment, post| if post.nil? || post.trashed? fragment.css(".poll, [data-poll-name]").each(&:remove) @@ -399,38 +415,83 @@ after_initialize do end on(:post_created) do |post| - next if post.is_first_post? || post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD].blank? - # signals the front-end we have polls for that post - MessageBus.publish("/polls/#{post.topic_id}", post_id: post.id, polls: post.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]) - # schedule automatic close jobs DiscoursePoll::Poll.schedule_jobs(post) - end - on(:merging_users) do |source_user, target_user| - DiscoursePoll::VotesUpdater.merge_users!(source_user, target_user) - end - - add_to_serializer(:post, :polls, false) do - polls = post_custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD].dup - - polls.each do |_, poll| - next if !poll - poll["options"].each do |option| - option.delete("voter_ids") - end + unless post.is_first_post? + polls = ActiveModel::ArraySerializer.new(post.polls, each_serializer: PollSerializer, root: false).as_json + MessageBus.publish("/polls/#{post.topic_id}", post_id: post.id, polls: polls) end end - add_to_serializer(:post, :include_polls?) { post_custom_fields.present? && post_custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD].present? } + on(:merging_users) do |source_user, target_user| + PollVote.where(user_id: source_user.id).update_all(user_id: target_user.id) + end + + on(:user_destroyed) do |user| + PollVote.where(user_id: user.id).delete_all + end + + register_post_custom_field_type(DiscoursePoll::HAS_POLLS, :boolean) + + topic_view_post_custom_fields_whitelister { [DiscoursePoll::HAS_POLLS] } + + add_to_class(:topic_view, :polls) do + @polls ||= begin + polls = {} + + post_with_polls = @post_custom_fields.each_with_object([]) do |fields, obj| + obj << fields[0] if fields[1][DiscoursePoll::HAS_POLLS] + end + + if post_with_polls.present? + Poll + .includes(poll_options: :poll_votes, poll_votes: :poll_option) + .where(post_id: post_with_polls) + .each do |p| + polls[p.post_id] ||= [] + polls[p.post_id] << p + end + end + + polls + end + end + + add_to_serializer(:post, :preloaded_polls, false) do + @preloaded_polls ||= if @topic_view.present? + @topic_view.polls[object.id] + else + Poll.includes(poll_options: :poll_votes).where(post: object) + end + end + + add_to_serializer(:post, :include_preloaded_polls?) do + false + end + + add_to_serializer(:post, :polls, false) do + preloaded_polls.map { |p| PollSerializer.new(p, root: false) } + end + + add_to_serializer(:post, :include_polls?) do + preloaded_polls.present? + end add_to_serializer(:post, :polls_votes, false) do - post_custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD]["#{scope.user.id}"] + preloaded_polls.map do |poll| + user_poll_votes = poll.poll_votes.each_with_object([]) do |vote, obj| + if vote.user_id == scope.user.id + obj << vote.poll_option.digest + end + end + + [poll.name, user_poll_votes] + end.to_h end add_to_serializer(:post, :include_polls_votes?) do - return unless scope.user - return unless post_custom_fields.present? - return unless post_custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD].present? - post_custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD].has_key?("#{scope.user.id}") + scope.user&.id.present? && + preloaded_polls.present? && + preloaded_polls.any? { |p| p.has_voted?(scope.user) } end end diff --git a/plugins/poll/spec/controllers/polls_controller_spec.rb b/plugins/poll/spec/controllers/polls_controller_spec.rb index 6b84f72ad43..d03d5616da2 100644 --- a/plugins/poll/spec/controllers/polls_controller_spec.rb +++ b/plugins/poll/spec/controllers/polls_controller_spec.rb @@ -1,5 +1,4 @@ require "rails_helper" -require_relative "../helpers" describe ::DiscoursePoll::PollsController do routes { ::DiscoursePoll::Engine.routes } @@ -8,6 +7,8 @@ describe ::DiscoursePoll::PollsController do let(:topic) { Fabricate(:topic) } let(:poll) { Fabricate(:post, topic: topic, user: user, raw: "[poll]\n- A\n- B\n[/poll]") } let(:multi_poll) { Fabricate(:post, topic: topic, user: user, raw: "[poll min=1 max=2 type=multiple public=true]\n- A\n- B\n[/poll]") } + let(:public_poll_on_vote) { Fabricate(:post, topic: topic, user: user, raw: "[poll public=true results=on_vote]\n- A\n- B\n[/poll]") } + 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 @@ -56,7 +57,7 @@ describe ::DiscoursePoll::PollsController do expect(json["poll"]["options"][1]["votes"]).to eq(1) end - it "works even if topic is closed" do + it "works on closed topics" do topic.update_attribute(:closed, true) put :vote, params: { @@ -102,16 +103,6 @@ describe ::DiscoursePoll::PollsController do expect(json["errors"][0]).to eq(I18n.t("poll.user_cant_post_in_topic")) end - it "ensures polls are associated with the post" do - put :vote, params: { - post_id: Fabricate(:post).id, poll_name: "foobar", options: ["A"] - }, format: :json - - expect(response.status).not_to eq(200) - json = ::JSON.parse(response.body) - expect(json["errors"][0]).to eq(I18n.t("poll.no_polls_associated_with_this_post")) - end - it "checks the name of the poll" do put :vote, params: { post_id: poll.id, poll_name: "foobar", options: ["A"] @@ -135,8 +126,10 @@ describe ::DiscoursePoll::PollsController do end it "doesn't discard anonymous votes when someone votes" do - default_poll = poll.custom_fields["polls"]["poll"] - add_anonymous_votes(poll, default_poll, 17, "5c24fc1df56d764b550ceae1b9319125" => 11, "e89dec30bbd9bf50fabf6a05b4324edf" => 6) + the_poll = poll.polls.first + the_poll.update_attribute(:anonymous_voters, 17) + the_poll.poll_options[0].update_attribute(:anonymous_votes, 11) + the_poll.poll_options[1].update_attribute(:anonymous_votes, 6) put :vote, params: { post_id: poll.id, poll_name: "poll", options: ["5c24fc1df56d764b550ceae1b9319125"] @@ -149,57 +142,6 @@ describe ::DiscoursePoll::PollsController do expect(json["poll"]["options"][0]["votes"]).to eq(12) expect(json["poll"]["options"][1]["votes"]).to eq(6) end - - it "tracks the users ids for public polls" do - public_poll = Fabricate(:post, topic_id: topic.id, user_id: user.id, raw: "[poll public=true]\n- A\n- B\n[/poll]") - body = { post_id: public_poll.id, poll_name: "poll" } - - message = MessageBus.track_publish do - put :vote, - params: body.merge(options: ["5c24fc1df56d764b550ceae1b9319125"]), - format: :json - end.first - - expect(response.status).to eq(200) - - json = ::JSON.parse(response.body) - expect(json["poll"]["voters"]).to eq(1) - expect(json["poll"]["options"][0]["votes"]).to eq(1) - expect(json["poll"]["options"][1]["votes"]).to eq(0) - expect(json["poll"]["options"][0]["voter_ids"]).to eq([user.id]) - expect(json["poll"]["options"][1]["voter_ids"]).to eq([]) - expect(message.data[:post_id].to_i).to eq(public_poll.id) - expect(message.data[:user][:id].to_i).to eq(user.id) - - put :vote, - params: body.merge(options: ["e89dec30bbd9bf50fabf6a05b4324edf"]), - format: :json - - expect(response.status).to eq(200) - - json = ::JSON.parse(response.body) - expect(json["poll"]["voters"]).to eq(1) - expect(json["poll"]["options"][0]["votes"]).to eq(0) - expect(json["poll"]["options"][1]["votes"]).to eq(1) - expect(json["poll"]["options"][0]["voter_ids"]).to eq([]) - expect(json["poll"]["options"][1]["voter_ids"]).to eq([user.id]) - - another_user = Fabricate(:user) - log_in_user(another_user) - - put :vote, - params: body.merge(options: ["e89dec30bbd9bf50fabf6a05b4324edf", "5c24fc1df56d764b550ceae1b9319125"]), - format: :json - - expect(response.status).to eq(200) - - json = ::JSON.parse(response.body) - expect(json["poll"]["voters"]).to eq(2) - expect(json["poll"]["options"][0]["votes"]).to eq(1) - expect(json["poll"]["options"][1]["votes"]).to eq(2) - expect(json["poll"]["options"][0]["voter_ids"]).to eq([another_user.id]) - expect(json["poll"]["options"][1]["voter_ids"]).to eq([user.id, another_user.id]) - end end describe "#toggle_status" do @@ -248,13 +190,12 @@ describe ::DiscoursePoll::PollsController do end - describe "votes" do + describe "#voters" do + + let(:first) { "5c24fc1df56d764b550ceae1b9319125" } + let(:second) { "e89dec30bbd9bf50fabf6a05b4324edf" } it "correctly handles offset" do - - first = "5c24fc1df56d764b550ceae1b9319125" - second = "e89dec30bbd9bf50fabf6a05b4324edf" - user1 = log_in put :vote, params: { @@ -274,15 +215,13 @@ describe ::DiscoursePoll::PollsController do user3 = log_in put :vote, params: { - post_id: multi_poll.id, - poll_name: "poll", - options: [first, second] + post_id: multi_poll.id, poll_name: "poll", options: [first, second] }, format: :json expect(response.status).to eq(200) get :voters, params: { - poll_name: 'poll', post_id: multi_poll.id, voter_limit: 2 + poll_name: 'poll', post_id: multi_poll.id, limit: 2 }, format: :json expect(response.status).to eq(200) @@ -290,25 +229,81 @@ describe ::DiscoursePoll::PollsController do json = JSON.parse(response.body) # no user3 cause voter_limit is 2 - expect(json["poll"][first].map { |h| h["id"] }.sort).to eq([user1.id, user2.id]) - expect(json["poll"][second].map { |h| h["id"] }).to eq([user3.id]) + expect(json["voters"][first].map { |h| h["id"] }).to contain_exactly(user1.id, user2.id) + expect(json["voters"][second].map { |h| h["id"] }).to contain_exactly(user3.id) + end - reloaded = Post.find(multi_poll.id) - - # break the custom poll and make sure we still return something sane here - # TODO: normalize this data so we don't store the information twice and there is a chance - # that somehow a bg job can cause both fields to be out-of-sync - poll_votes = reloaded.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD] - poll_votes.delete user2.id.to_s - - reloaded.save_custom_fields(true) - - get :voters, params: { - poll_name: 'poll', post_id: multi_poll.id, voter_limit: 2 + it "ensures voters can only be seen after casting a vote" do + put :vote, params: { + post_id: public_poll_on_vote.id, poll_name: "poll", options: [first] }, format: :json expect(response.status).to eq(200) + get :voters, params: { + poll_name: "poll", post_id: public_poll_on_vote.id + }, format: :json + + expect(response.status).to eq(200) + + json = JSON.parse(response.body) + + expect(json["voters"][first].size).to eq(1) + + user2 = log_in + + get :voters, params: { + poll_name: "poll", post_id: public_poll_on_vote.id + }, format: :json + + expect(response.status).to eq(422) + + put :vote, params: { + post_id: public_poll_on_vote.id, poll_name: "poll", options: [second] + }, format: :json + + expect(response.status).to eq(200) + + get :voters, params: { + poll_name: "poll", post_id: public_poll_on_vote.id + }, format: :json + + expect(response.status).to eq(200) + + json = JSON.parse(response.body) + + expect(json["voters"][first].size).to eq(1) + expect(json["voters"][second].size).to eq(1) + end + + it "ensures voters can only be seen when poll is closed" do + put :vote, params: { + post_id: public_poll_on_close.id, poll_name: "poll", options: [first] + }, format: :json + + expect(response.status).to eq(200) + + get :voters, params: { + poll_name: "poll", post_id: public_poll_on_close.id + }, format: :json + + expect(response.status).to eq(422) + + put :toggle_status, params: { + post_id: public_poll_on_close.id, poll_name: "poll", status: "closed" + }, format: :json + + expect(response.status).to eq(200) + + get :voters, params: { + poll_name: "poll", post_id: public_poll_on_close.id + }, format: :json + + expect(response.status).to eq(200) + + json = JSON.parse(response.body) + + expect(json["voters"][first].size).to eq(1) end end diff --git a/plugins/poll/spec/controllers/posts_controller_spec.rb b/plugins/poll/spec/controllers/posts_controller_spec.rb index 96ccc97e7c2..912e58c017c 100644 --- a/plugins/poll/spec/controllers/posts_controller_spec.rb +++ b/plugins/poll/spec/controllers/posts_controller_spec.rb @@ -1,5 +1,4 @@ require "rails_helper" -require_relative "../helpers" describe PostsController do let!(:user) { log_in } @@ -19,7 +18,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") - expect(json["polls"]["poll"]).to be + expect(Poll.exists?(post_id: json["id"])).to eq(true) end it "works on any post" do @@ -32,7 +31,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") - expect(json["polls"]["poll"]).to be + expect(Poll.exists?(post_id: json["id"])).to eq(true) end it "schedules auto-close job" do @@ -45,9 +44,8 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) - expect(json["polls"][name]["close"]).to be - - expect(Jobs.scheduled_for(:close_poll, post_id: Post.last.id, poll_name: name)).to be + expect(Poll.find_by(post_id: json["id"]).close_at).to be + expect(Jobs.scheduled_for(:close_poll, post_id: json["id"], poll_name: name)).to be end it "should have different options" do @@ -55,7 +53,7 @@ describe PostsController do title: title, raw: "[poll]\n- A\n- A\n[/poll]" }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.default_poll_must_have_different_options")) end @@ -65,7 +63,7 @@ describe PostsController do title: title, raw: "[poll]\n- A\n[/poll]" }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.default_poll_must_have_at_least_2_options")) end @@ -79,7 +77,7 @@ describe PostsController do title: title, raw: raw }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.default_poll_must_have_less_options", count: SiteSetting.poll_maximum_options)) end @@ -89,7 +87,7 @@ describe PostsController do title: title, raw: "[poll type=multiple min=5]\n- A\n- B\n[/poll]" }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters")) end @@ -103,7 +101,7 @@ describe PostsController do json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") expect(json["cooked"]).to include("<script>") - expect(json["polls"]["<script>alert('xss')</script>"]).to be + expect(Poll.find_by(post_id: json["id"]).name).to eq("<script>alert('xss')</script>") end it "also works whe there is a link starting with '[poll'" do @@ -114,7 +112,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") - expect(json["polls"]).to be + expect(Poll.exists?(post_id: json["id"])).to eq(true) end it "prevents pollception" do @@ -125,8 +123,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") - expect(json["polls"]["1"]).to_not be - expect(json["polls"]["2"]).to be + expect(Poll.where(post_id: json["id"]).count).to eq(1) end describe "edit window" do @@ -150,7 +147,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) - expect(json["post"]["polls"]["poll"]["options"][2]["html"]).to eq("C") + expect(json["post"]["polls"][0]["options"][2]["html"]).to eq("C") end it "resets the votes" do @@ -191,26 +188,14 @@ describe PostsController do describe "with no vote" do - it "OP can change the options" do + it "can change the options" do put :update, params: { id: post_id, post: { raw: new_option } }, format: :json expect(response.status).to eq(200) json = ::JSON.parse(response.body) - expect(json["post"]["polls"]["poll"]["options"][1]["html"]).to eq("C") - end - - it "staff can change the options" do - log_in_user(Fabricate(:moderator)) - - put :update, params: { - id: post_id, post: { raw: new_option } - }, format: :json - - expect(response.status).to eq(200) - json = ::JSON.parse(response.body) - expect(json["post"]["polls"]["poll"]["options"][1]["html"]).to eq("C") + expect(json["post"]["polls"][0]["options"][1]["html"]).to eq("C") end it "support changes on the post" do @@ -228,54 +213,19 @@ describe PostsController do DiscoursePoll::Poll.vote(post_id, "poll", ["5c24fc1df56d764b550ceae1b9319125"], user) end - it "OP cannot change the options" do + it "cannot change the options" do put :update, params: { id: post_id, post: { raw: new_option } }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t( - "poll.edit_window_expired.op_cannot_edit_options", + "poll.edit_window_expired.cannot_edit_default_poll_with_votes", minutes: poll_edit_window_mins )) end - it "staff can change the options and votes are merged" do - log_in_user(Fabricate(:moderator)) - - put :update, params: { - id: post_id, post: { raw: new_option } - }, format: :json - - expect(response.status).to eq(200) - json = ::JSON.parse(response.body) - expect(json["post"]["polls"]["poll"]["options"][1]["html"]).to eq("C") - expect(json["post"]["polls"]["poll"]["voters"]).to eq(1) - expect(json["post"]["polls"]["poll"]["options"][0]["votes"]).to eq(1) - expect(json["post"]["polls"]["poll"]["options"][1]["votes"]).to eq(0) - end - - it "staff can change the options and anonymous votes are merged" do - post = Post.find_by(id: post_id) - default_poll = post.custom_fields["polls"]["poll"] - add_anonymous_votes(post, default_poll, 7, "5c24fc1df56d764b550ceae1b9319125" => 7) - - log_in_user(Fabricate(:moderator)) - - put :update, params: { - id: post_id, post: { raw: new_option } - }, format: :json - - expect(response.status).to eq(200) - - json = ::JSON.parse(response.body) - expect(json["post"]["polls"]["poll"]["options"][1]["html"]).to eq("C") - expect(json["post"]["polls"]["poll"]["voters"]).to eq(8) - expect(json["post"]["polls"]["poll"]["options"][0]["votes"]).to eq(8) - expect(json["post"]["polls"]["poll"]["options"][1]["votes"]).to eq(0) - end - it "support changes on the post" do put :update, params: { id: post_id, post: { raw: updated } }, format: :json expect(response.status).to eq(200) @@ -298,7 +248,7 @@ describe PostsController do title: title, raw: "[poll name=""foo""]\n- A\n- A\n[/poll]" }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.named_poll_must_have_different_options", name: "foo")) end @@ -308,7 +258,7 @@ describe PostsController do title: title, raw: "[poll name='foo']\n- A\n[/poll]" }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.named_poll_must_have_at_least_2_options", name: "foo")) end @@ -325,8 +275,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") - expect(json["polls"]["poll"]).to be - expect(json["polls"]["foo"]).to be + expect(Poll.where(post_id: json["id"]).count).to eq(2) end it "should have a name" do @@ -334,7 +283,7 @@ describe PostsController do title: title, raw: "[poll]\n- A\n- B\n[/poll]\n[poll]\n- A\n- B\n[/poll]" }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.multiple_polls_without_name")) end @@ -344,7 +293,7 @@ describe PostsController do title: title, raw: "[poll name=foo]\n- A\n- B\n[/poll]\n[poll name=foo]\n- A\n- B\n[/poll]" }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.multiple_polls_with_same_name", name: "foo")) end @@ -381,7 +330,7 @@ describe PostsController do title: title, raw: "[poll]\n- A\n- B\n[/poll]" }, format: :json - expect(response).not_to be_success + expect(response).not_to be_successful json = ::JSON.parse(response.body) expect(json["errors"][0]).to eq(I18n.t("poll.insufficient_rights_to_create")) end @@ -402,7 +351,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") - expect(json["polls"]["poll"]).to be + expect(Poll.exists?(post_id: json["id"])).to eq(true) end end @@ -421,7 +370,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") - expect(json["polls"]["poll"]).to be + expect(Poll.exists?(post_id: json["id"])).to eq(true) end end @@ -440,7 +389,7 @@ describe PostsController do expect(response.status).to eq(200) json = ::JSON.parse(response.body) expect(json["cooked"]).to match("data-poll-") - expect(json["polls"]["poll"]).to be + expect(Poll.exists?(post_id: json["id"])).to eq(true) end end end diff --git a/plugins/poll/spec/db/post_migrate/migrate_polls_data_spec.rb b/plugins/poll/spec/db/post_migrate/migrate_polls_data_spec.rb new file mode 100644 index 00000000000..f7e3096a1f3 --- /dev/null +++ b/plugins/poll/spec/db/post_migrate/migrate_polls_data_spec.rb @@ -0,0 +1,330 @@ +require 'rails_helper' +require_relative '../../../db/post_migrate/20180820080623_migrate_polls_data' + +RSpec.describe MigratePollsData do + let!(:user) { Fabricate(:user, id: 1) } + let!(:user2) { Fabricate(:user, id: 2) } + let!(:user3) { Fabricate(:user, id: 3) } + let!(:user4) { Fabricate(:user, id: 4) } + let!(:user5) { Fabricate(:user, id: 5) } + let(:post) { Fabricate(:post, user: user) } + + describe 'for a number poll' do + before do + post.custom_fields = { + "polls" => { + "poll" => { + "options" => [ + { "id" => "4d8a15e3cc35750f016ce15a43937620", "html" => "1", "votes" => 0 }, + { "id" => "aa2393b424f2f395abb63bf785760a3b", "html" => "4", "votes" => 0 }, + { "id" => "9ab1070dec27185440cdabb4948a5e9a", "html" => "7", "votes" => 1 }, + { "id" => "46c01f638a50d86e020f47469733b8be", "html" => "10", "votes" => 0 }, + { "id" => "b4f15431e07443c372d521e4ed131abe", "html" => "13", "votes" => 0 }, + { "id" => "4e885ead68ff4456f102843df9fbbd7f", "html" => "16", "votes" => 0 }, + { "id" => "eb8661f072794ea57baa7827cd8ffc88", "html" => "19", "votes" => 0 } + ], + "voters" => 1, + "name" => "poll", + "status" => "open", + "type" => "number", + "min" => "1", + "max" => "20", + "step" => "3" + }, + }, + "polls-votes" => { + "1" => { + "poll" => [ + "9ab1070dec27185440cdabb4948a5e9a" + ] + } + } + } + + post.save_custom_fields + end + + it "should migrate the data correctly" do + expect do + silence_stdout { MigratePollsData.new.up } + end.to \ + change { Poll.count }.by(1) & + change { PollOption.count }.by(7) & + change { PollVote.count }.by(1) + + poll = Poll.find_by(name: "poll", post: post) + + expect(poll.close_at).to eq(nil) + + expect(poll.number?).to eq(true) + expect(poll.open?).to eq(true) + expect(poll.always?).to eq(true) + expect(poll.secret?).to eq(true) + + expect(poll.min).to eq(1) + expect(poll.max).to eq(20) + expect(poll.step).to eq(3) + + expect(PollOption.all.pluck(:digest, :html)).to eq([ + ["4d8a15e3cc35750f016ce15a43937620", "1"], + ["aa2393b424f2f395abb63bf785760a3b", "4"], + ["9ab1070dec27185440cdabb4948a5e9a", "7"], + ["46c01f638a50d86e020f47469733b8be", "10"], + ["b4f15431e07443c372d521e4ed131abe", "13"], + ["4e885ead68ff4456f102843df9fbbd7f", "16"], + ["eb8661f072794ea57baa7827cd8ffc88", "19"] + ]) + + poll_vote = PollVote.first + + expect(poll_vote.poll).to eq(poll) + expect(poll_vote.poll_option.html).to eq("7") + expect(poll_vote.user).to eq(user) + end + end + + describe 'for a multiple poll' do + before do + post.custom_fields = { + "polls-votes" => { + "1" => { + "testing" => [ + "b2c3e3668a886d09e97e38b8adde7d45", + "28df49fa9e9c09d3a1eb8cfbcdcda7790", + ] + }, + "2" => { + "testing" => [ + "b2c3e3668a886d09e97e38b8adde7d45", + "d01af008ec373e948c0ab3ad61009f35", + ] + }, + }, + "polls" => { + "poll" => { + "options" => [ + { + "id" => "b2c3e3668a886d09e97e38b8adde7d45", + "html" => "Choice 1", + "votes" => 2, + "voter_ids" => [user.id, user2.id] + }, + { + "id" => "28df49fa9e9c09d3a1eb8cfbcdcda7790", + "html" => "Choice 2", + "votes" => 1, + "voter_ids" => [user.id] + }, + { + "id" => "d01af008ec373e948c0ab3ad61009f35", + "html" => "Choice 3", + "votes" => 1, + "voter_ids" => [user2.id] + }, + ], + "voters" => 4, + "name" => "testing", + "status" => "closed", + "type" => "multiple", + "public" => "true", + "min" => 1, + "max" => 2 + } + } + } + + post.save_custom_fields + end + + it 'should migrate the data correctly' do + expect do + silence_stdout { MigratePollsData.new.up } + end.to \ + change { Poll.count }.by(1) & + change { PollOption.count }.by(3) & + change { PollVote.count }.by(4) + + poll = Poll.last + + expect(poll.post_id).to eq(post.id) + expect(poll.name).to eq("testing") + expect(poll.close_at).to eq(nil) + + expect(poll.multiple?).to eq(true) + expect(poll.closed?).to eq(true) + expect(poll.always?).to eq(true) + expect(poll.everyone?).to eq(true) + + expect(poll.min).to eq(1) + expect(poll.max).to eq(2) + expect(poll.step).to eq(nil) + + poll_options = PollOption.all + + poll_option_1 = poll_options[0] + expect(poll_option_1.poll_id).to eq(poll.id) + expect(poll_option_1.digest).to eq("b2c3e3668a886d09e97e38b8adde7d45") + expect(poll_option_1.html).to eq("Choice 1") + + poll_option_2 = poll_options[1] + expect(poll_option_2.poll_id).to eq(poll.id) + expect(poll_option_2.digest).to eq("28df49fa9e9c09d3a1eb8cfbcdcda7790") + expect(poll_option_2.html).to eq("Choice 2") + + poll_option_3 = poll_options[2] + expect(poll_option_3.poll_id).to eq(poll.id) + expect(poll_option_3.digest).to eq("d01af008ec373e948c0ab3ad61009f35") + expect(poll_option_3.html).to eq("Choice 3") + + expect(PollVote.all.pluck(:poll_id).uniq).to eq([poll.id]) + + { + user => [poll_option_1, poll_option_2], + user2 => [poll_option_1, poll_option_3] + }.each do |user, options| + options.each do |option| + expect(PollVote.exists?(poll_option_id: option.id, user_id: user.id)) + .to eq(true) + end + end + end + end + + describe 'for a regular poll' do + before do + post.custom_fields = { + "polls" => { + "testing" => { + "options" => [ + { + "id" => "e94c09aae2aa071610212a5c5042111b", + "html" => "Yes", + "votes" => 0, + "anonymous_votes" => 1, + "voter_ids" => [] + }, + { + "id" => "802c50392a68e426d4b26d81ddc5ab33", + "html" => "No", + "votes" => 0, + "anonymous_votes" => 2, + "voter_ids" => [] + } + ], + "voters" => 0, + "anonymous_voters" => 3, + "name" => "testing", + "status" => "open", + "type" => "regular" + }, + "poll" => { + "options" => [ + { + "id" => "edeee5dae4802ab24185d41039efb545", + "html" => "Yes", + "votes" => 2, + "voter_ids" => [1, 2] + }, + { + "id" => "38d8e35c8fc80590f836f22189064835", + "html" => + "No", + "votes" => 3, + "voter_ids" => [3, 4, 5] + } + ], + "voters" => 5, + "name" => "poll", + "status" => "open", + "type" => "regular", + "public" => "true", + "close" => "2018-10-08T00:00:00.000Z" + }, + }, + "polls-votes" => { + "1" => { "poll" => ["edeee5dae4802ab24185d41039efb545"] }, + "2" => { "poll" => ["edeee5dae4802ab24185d41039efb545"] }, + "3" => { "poll" => ["38d8e35c8fc80590f836f22189064835"] }, + "4" => { "poll" => ["38d8e35c8fc80590f836f22189064835"] }, + "5" => { "poll" => ["38d8e35c8fc80590f836f22189064835"] } + } + } + + post.save_custom_fields + end + + it 'should migrate the data correctly' do + expect do + silence_stdout { MigratePollsData.new.up } + end.to \ + change { Poll.count }.by(2) & + change { PollOption.count }.by(4) & + change { PollVote.count }.by(5) + + poll = Poll.find_by(name: "poll") + + expect(poll.post_id).to eq(post.id) + expect(poll.close_at).to eq("2018-10-08T00:00:00.000Z") + + expect(poll.regular?).to eq(true) + expect(poll.open?).to eq(true) + expect(poll.always?).to eq(true) + expect(poll.everyone?).to eq(true) + + expect(poll.min).to eq(nil) + expect(poll.max).to eq(nil) + expect(poll.step).to eq(nil) + + poll_options = PollOption.where(poll_id: poll.id).to_a + expect(poll_options.size).to eq(2) + + option_1 = poll_options.first + expect(option_1.digest).to eq("edeee5dae4802ab24185d41039efb545") + expect(option_1.html).to eq("Yes") + + option_2 = poll_options.last + expect(option_2.digest).to eq("38d8e35c8fc80590f836f22189064835") + expect(option_2.html).to eq("No") + + expect(PollVote.pluck(:poll_id).uniq).to eq([poll.id]) + + [user, user2].each do |user| + expect(PollVote.exists?(poll_option_id: option_1.id, user_id: user.id)) + .to eq(true) + end + + [user3, user4, user5].each do |user| + expect(PollVote.exists?(poll_option_id: option_2.id, user_id: user.id)) + .to eq(true) + end + + poll = Poll.find_by(name: "testing") + + expect(poll.post_id).to eq(post.id) + expect(poll.close_at).to eq(nil) + expect(poll.anonymous_voters).to eq(3) + + expect(poll.regular?).to eq(true) + expect(poll.open?).to eq(true) + expect(poll.always?).to eq(true) + expect(poll.secret?).to eq(true) + + expect(poll.min).to eq(nil) + expect(poll.max).to eq(nil) + expect(poll.step).to eq(nil) + + poll_options = PollOption.where(poll: poll).to_a + expect(poll_options.size).to eq(2) + + option_1 = poll_options.first + expect(option_1.digest).to eq("e94c09aae2aa071610212a5c5042111b") + expect(option_1.html).to eq("Yes") + expect(option_1.anonymous_votes).to eq(1) + + option_2 = poll_options.last + expect(option_2.digest).to eq("802c50392a68e426d4b26d81ddc5ab33") + expect(option_2.html).to eq("No") + expect(option_2.anonymous_votes).to eq(2) + end + end +end diff --git a/plugins/poll/spec/helpers.rb b/plugins/poll/spec/helpers.rb deleted file mode 100644 index a0584810ab8..00000000000 --- a/plugins/poll/spec/helpers.rb +++ /dev/null @@ -1,17 +0,0 @@ -module Helpers - def add_anonymous_votes(post, poll, voters, options_with_votes) - poll["voters"] += voters - poll["anonymous_voters"] = voters - - poll["options"].each do |option| - anonymous_votes = options_with_votes[option["id"]] || 0 - - if anonymous_votes > 0 - option["votes"] += anonymous_votes - option["anonymous_votes"] = anonymous_votes - end - end - - post.save_custom_fields(true) - end -end diff --git a/plugins/poll/spec/integration/poll_endpoints_spec.rb b/plugins/poll/spec/integration/poll_endpoints_spec.rb index 8e5e136534d..5caf17dabaf 100644 --- a/plugins/poll/spec/integration/poll_endpoints_spec.rb +++ b/plugins/poll/spec/integration/poll_endpoints_spec.rb @@ -4,12 +4,14 @@ describe "DiscoursePoll endpoints" do describe "fetch voters for a poll" do let(:user) { Fabricate(:user) } let(:post) { Fabricate(:post, raw: "[poll public=true]\n- A\n- B\n[/poll]") } + let(:option_a) { "5c24fc1df56d764b550ceae1b9319125" } + let(:option_b) { "e89dec30bbd9bf50fabf6a05b4324edf" } it "should return the right response" do DiscoursePoll::Poll.vote( post.id, DiscoursePoll::DEFAULT_POLL_NAME, - ["5c24fc1df56d764b550ceae1b9319125"], + [option_a], user ) @@ -20,8 +22,8 @@ describe "DiscoursePoll endpoints" do expect(response.status).to eq(200) - poll = JSON.parse(response.body)[DiscoursePoll::DEFAULT_POLL_NAME] - option = poll["5c24fc1df56d764b550ceae1b9319125"] + poll = JSON.parse(response.body)["voters"] + option = poll[option_a] expect(option.length).to eq(1) expect(option.first["id"]).to eq(user.id) @@ -32,23 +34,23 @@ describe "DiscoursePoll endpoints" do DiscoursePoll::Poll.vote( post.id, DiscoursePoll::DEFAULT_POLL_NAME, - ["5c24fc1df56d764b550ceae1b9319125", "e89dec30bbd9bf50fabf6a05b4324edf"], + [option_a, option_b], user ) get "/polls/voters.json", params: { post_id: post.id, poll_name: DiscoursePoll::DEFAULT_POLL_NAME, - option_id: 'e89dec30bbd9bf50fabf6a05b4324edf' + option_id: option_b } expect(response.status).to eq(200) - poll = JSON.parse(response.body)[DiscoursePoll::DEFAULT_POLL_NAME] + poll = JSON.parse(response.body)["voters"] - expect(poll['5c24fc1df56d764b550ceae1b9319125']).to eq(nil) + expect(poll[option_a]).to eq(nil) - option = poll['e89dec30bbd9bf50fabf6a05b4324edf'] + option = poll[option_b] expect(option.length).to eq(1) expect(option.first["id"]).to eq(user.id) @@ -68,7 +70,7 @@ describe "DiscoursePoll endpoints" do post_id: -1, poll_name: DiscoursePoll::DEFAULT_POLL_NAME } - expect(response.status).to eq(400) + expect(response.status).to eq(422) expect(response.body).to include('post_id is invalid') end end @@ -83,7 +85,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(400) + expect(response.status).to eq(422) expect(response.body).to include('poll_name is invalid') end end @@ -108,7 +110,7 @@ describe "DiscoursePoll endpoints" do expect(response.status).to eq(200) - poll = JSON.parse(response.body)[DiscoursePoll::DEFAULT_POLL_NAME] + poll = JSON.parse(response.body)["voters"] expect(poll.first["id"]).to eq(user.id) expect(poll.first["username"]).to eq(user.username) diff --git a/plugins/poll/spec/lib/new_post_manager_spec.rb b/plugins/poll/spec/lib/new_post_manager_spec.rb index cf076e6a065..19edb832e8d 100644 --- a/plugins/poll/spec/lib/new_post_manager_spec.rb +++ b/plugins/poll/spec/lib/new_post_manager_spec.rb @@ -1,15 +1,15 @@ -require 'rails_helper' +require "rails_helper" describe NewPostManager do let(:user) { Fabricate(:newuser) } let(:admin) { Fabricate(:admin) } - describe 'when new post containing a poll is queued for approval' do + describe "when new post containing a poll is queued for approval" do before do SiteSetting.poll_minimum_trust_level_to_create = 0 end - it 'should render the poll upon approval' do + it "should render the poll upon approval" do params = { raw: "[poll]\n* 1\n* 2\n* 3\n[/poll]", archetype: "regular", @@ -29,11 +29,9 @@ describe NewPostManager do expect { NewPostManager.new(user, params).perform } .to change { QueuedPost.count }.by(1) - queued_post = QueuedPost.last - queued_post.approve!(admin) + QueuedPost.last.approve!(admin) - expect(Post.last.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]) - .to_not eq(nil) + expect(Poll.where(post: Post.last).exists?).to eq(true) end end end diff --git a/plugins/poll/spec/lib/polls_updater_spec.rb b/plugins/poll/spec/lib/polls_updater_spec.rb index 76d22573903..461f68839ec 100644 --- a/plugins/poll/spec/lib/polls_updater_spec.rb +++ b/plugins/poll/spec/lib/polls_updater_spec.rb @@ -1,428 +1,192 @@ require 'rails_helper' describe DiscoursePoll::PollsUpdater do + + def update(post, polls) + DiscoursePoll::PollsUpdater.update(post, polls) + end + let(:user) { Fabricate(:user) } - let(:post_with_two_polls) do - raw = <<-RAW.strip_heredoc - [poll] - * 1 - * 2 - [/poll] - - [poll name=test] - * 1 - * 2 - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - let(:post) do - raw = <<-RAW.strip_heredoc - [poll] - * 1 - * 2 - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - let(:other_post) do - raw = <<-RAW.strip_heredoc - [poll] - * 3 - * 4 - * 5 - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - let(:polls) do - DiscoursePoll::PollsValidator.new(post).validate_polls - end - - let(:polls_with_3_options) do - DiscoursePoll::PollsValidator.new(other_post).validate_polls - end - - let(:two_polls) do - DiscoursePoll::PollsValidator.new(post_with_two_polls).validate_polls - end - - describe '.update' do - describe 'when post does not contain any polls' do - it 'should update polls correctly' do - post = Fabricate(:post) - - message = MessageBus.track_publish do - described_class.update(post, polls) - end.first - - expect(post.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]).to eq(polls) - expect(message.data[:post_id]).to eq(post.id) - expect(message.data[:polls]).to eq(polls) - end - end - - describe 'when post contains existing polls' do - it "should be able to update polls correctly" do - message = MessageBus.track_publish do - described_class.update(post, polls_with_3_options) - end.first - - expect(post.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]).to eq(polls_with_3_options) - expect(message.data[:post_id]).to eq(post.id) - expect(message.data[:polls]).to eq(polls_with_3_options) - end - end - - describe 'when there are no changes' do - it "should not do anything" do - messages = MessageBus.track_publish do - described_class.update(post, polls) - end - - expect(messages).to eq([]) - end - end - - context "public polls" do - let(:post) do - raw = <<-RAW.strip_heredoc - [poll public=true] - - A - - B - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - let(:private_poll_post) do - raw = <<-RAW.strip_heredoc - [poll] - - A - - B - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - let(:private_poll) do - DiscoursePoll::PollsValidator.new(private_poll_post).validate_polls - end - - let(:public_poll) do - raw = <<-RAW.strip_heredoc - [poll public=true] - - A - - C - [/poll] - RAW - - DiscoursePoll::PollsValidator.new(Fabricate(:post, raw: raw)).validate_polls - end - - before do - DiscoursePoll::Poll.vote(post.id, "poll", ["5c24fc1df56d764b550ceae1b9319125"], user) - post.reload - end - - it "should not allow a private poll with votes to be made public" do - DiscoursePoll::Poll.vote(private_poll_post.id, "poll", ["5c24fc1df56d764b550ceae1b9319125"], user) - private_poll_post.reload - - messages = MessageBus.track_publish do - described_class.update(private_poll_post, public_poll) - end - - expect(messages).to eq([]) - - expect(private_poll_post.errors[:base]).to include( - I18n.t("poll.default_cannot_be_made_public") - ) - end - - it "should retain voter_ids when options have been edited" do - described_class.update(post, public_poll) - - polls = post.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] - - expect(polls["poll"]["options"][0]["voter_ids"]).to eq([user.id]) - expect(polls["poll"]["options"][1]["voter_ids"]).to eq([]) - end - - it "should delete voter_ids when poll is set to private" do - described_class.update(post, private_poll) - - polls = post.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] - - expect(post.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]) - .to eq(private_poll) - - expect(polls["poll"]["options"][0]["voter_ids"]).to eq(nil) - expect(polls["poll"]["options"][1]["voter_ids"]).to eq(nil) - end - end - - context "polls of type 'multiple'" do - let(:min_2_post) do - raw = <<-RAW.strip_heredoc - [poll type=multiple min=2 max=3] - - Option 1 - - Option 2 - - Option 3 - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - let(:min_2_poll) do - DiscoursePoll::PollsValidator.new(min_2_post).validate_polls - end - - let(:min_1_post) do - raw = <<-RAW.strip_heredoc - [poll type=multiple min=1 max=2] - - Option 1 - - Option 2 - - Option 3 - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - let(:min_1_poll) do - DiscoursePoll::PollsValidator.new(min_1_post).validate_polls - end - - it "should be able to update options" do - min_2_poll - - message = MessageBus.track_publish do - described_class.update(min_2_post, min_1_poll) - end.first - - expect(min_2_post.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]).to eq(min_1_poll) - expect(message.data[:post_id]).to eq(min_2_post.id) - expect(message.data[:polls]).to eq(min_1_poll) - end - end - - it 'should be able to edit multiple polls with votes' do - DiscoursePoll::Poll.vote( - post_with_two_polls.id, - "poll", - [two_polls["poll"]["options"].first["id"]], - user - ) - - raw = <<-RAW.strip_heredoc + let(:post) { + Fabricate(:post, raw: <<~RAW) [poll] - * 12 - * 34 + * 1 + * 2 [/poll] + RAW + } - [poll name=test] - * 12 - * 34 + let(:post_with_3_options) { + Fabricate(:post, raw: <<~RAW) + [poll] + - a + - b + - c [/poll] - RAW + RAW + } - different_post = Fabricate(:post, raw: raw) - different_polls = DiscoursePoll::PollsValidator.new(different_post).validate_polls + let(:post_with_some_attributes) { + Fabricate(:post, raw: <<~RAW) + [poll close=#{1.week.from_now.to_formatted_s(:iso8601)} results=on_close] + - A + - B + - C + [/poll] + RAW + } + let(:polls) { + DiscoursePoll::PollsValidator.new(post).validate_polls + } + + let(:polls_with_3_options) { + DiscoursePoll::PollsValidator.new(post_with_3_options).validate_polls + } + + let(:polls_with_some_attributes) { + DiscoursePoll::PollsValidator.new(post_with_some_attributes).validate_polls + } + + describe "update" do + + it "does nothing when there are no changes" do message = MessageBus.track_publish do - described_class.update(post_with_two_polls.reload, different_polls) + update(post, polls) end.first - expect(post_with_two_polls.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]) - .to eq(different_polls) - - expect(message.data[:post_id]).to eq(post_with_two_polls.id) - expect(message.data[:polls]).to eq(different_polls) + expect(message).to be(nil) end - describe "when poll edit window has expired" do - let(:poll_edit_window_mins) { 6 } - let(:another_post) { Fabricate(:post, created_at: Time.zone.now - poll_edit_window_mins.minutes) } + describe "deletes polls" do - before do - described_class.update(another_post, polls) - another_post.reload - SiteSetting.poll_edit_window_mins = poll_edit_window_mins + it "that were removed" do + update(post, {}) - DiscoursePoll::Poll.vote( - another_post.id, - "poll", - [polls["poll"]["options"].first["id"]], - user - ) + post.reload + + expect(Poll.where(post: post).exists?).to eq(false) + expect(post.custom_fields[DiscoursePoll::HAS_POLLS]).to eq(nil) end - it "should not allow users to edit options of current poll" do - messages = MessageBus.track_publish do - described_class.update(another_post, polls_with_3_options) + end + + describe "creates polls" do + + it "that were added" do + post = Fabricate(:post) + + expect(Poll.find_by(post: post)).to_not be + + message = MessageBus.track_publish do + update(post, polls) + end.first + + poll = Poll.find_by(post: post) + + expect(poll).to be + expect(poll.poll_options.size).to eq(2) + + expect(poll.post.custom_fields[DiscoursePoll::HAS_POLLS]).to eq(true) + + expect(message.data[:post_id]).to eq(post.id) + expect(message.data[:polls][0][:name]).to eq(poll.name) + end + + end + + describe "updates polls" do + + describe "when there are no votes" do + + it "at any time" do + post # create the post + + freeze_time 1.month.from_now + + message = MessageBus.track_publish do + update(post, polls_with_some_attributes) + end.first + + poll = Poll.find_by(post: post) + + expect(poll).to be + expect(poll.poll_options.size).to eq(3) + expect(poll.poll_votes.size).to eq(0) + expect(poll.on_close?).to eq(true) + expect(poll.close_at).to be + + expect(poll.post.custom_fields[DiscoursePoll::HAS_POLLS]).to eq(true) + + expect(message.data[:post_id]).to eq(post.id) + expect(message.data[:polls][0][:name]).to eq(poll.name) end - expect(another_post.errors[:base]).to include(I18n.t( - "poll.edit_window_expired.op_cannot_edit_options", - minutes: poll_edit_window_mins - )) - - expect(messages).to eq([]) end - context "staff" do - let(:another_user) { Fabricate(:user) } + describe "when there are votes" do before do - another_post.update_attributes!(last_editor_id: User.staff.first.id) + expect { + DiscoursePoll::Poll.vote(post.id, "poll", [polls["poll"]["options"][0]["id"]], user) + }.to change { PollVote.count }.by(1) end - it "should allow staff to add polls" do - message = MessageBus.track_publish do - described_class.update(another_post, two_polls) - end.first + describe "inside the edit window" do - expect(another_post.errors.full_messages).to eq([]) + it "and deletes the votes" do + message = MessageBus.track_publish do + update(post, polls_with_some_attributes) + end.first - expect(message.data[:post_id]).to eq(another_post.id) - expect(message.data[:polls]).to eq(two_polls) - end + poll = Poll.find_by(post: post) - it "should not allow staff to add options if votes have been casted" do - another_post.update_attributes!(last_editor_id: User.staff.first.id) + expect(poll).to be + expect(poll.poll_options.size).to eq(3) + expect(poll.poll_votes.size).to eq(0) + expect(poll.on_close?).to eq(true) + expect(poll.close_at).to be - messages = MessageBus.track_publish do - described_class.update(another_post, polls_with_3_options) + expect(poll.post.custom_fields[DiscoursePoll::HAS_POLLS]).to eq(true) + + expect(message.data[:post_id]).to eq(post.id) + expect(message.data[:polls][0][:name]).to eq(poll.name) end - expect(another_post.errors[:base]).to include(I18n.t( - "poll.edit_window_expired.staff_cannot_add_or_remove_options", - minutes: poll_edit_window_mins - )) - - expect(messages).to eq([]) end - it "should allow staff to add options if no votes have been casted" do - post.update_attributes!( - created_at: Time.zone.now - 5.minutes, - last_editor_id: User.staff.first.id - ) + describe "outside the edit window" do - message = MessageBus.track_publish do - described_class.update(post, polls_with_3_options) - end.first + it "throws an error" do + edit_window = SiteSetting.poll_edit_window_mins - expect(post.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]).to eq(polls_with_3_options) - expect(message.data[:post_id]).to eq(post.id) - expect(message.data[:polls]).to eq(polls_with_3_options) - end + freeze_time (edit_window + 1).minutes.from_now - it "should allow staff to edit options even if votes have been casted" do - another_post.update!(last_editor_id: User.staff.first.id) + update(post, polls_with_some_attributes) - DiscoursePoll::Poll.vote( - another_post.id, - "poll", - [polls["poll"]["options"].first["id"]], - another_user - ) + poll = Poll.find_by(post: post) - raw = <<-RAW.strip_heredoc - [poll] - * 3 - * 4 - [/poll] - RAW + expect(poll).to be + expect(poll.poll_options.size).to eq(2) + expect(poll.poll_votes.size).to eq(1) + expect(poll.on_close?).to eq(false) + expect(poll.close_at).to_not be - different_post = Fabricate(:post, raw: raw) - different_polls = DiscoursePoll::PollsValidator.new(different_post).validate_polls - - message = MessageBus.track_publish do - described_class.update(another_post, different_polls) - end.first - - custom_fields = another_post.reload.custom_fields - - expect(custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]) - .to eq(different_polls) - - [user, another_user].each do |u| - expect(custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD][u.id.to_s]["poll"]) - .to eq(["68b434ff88aeae7054e42cd05a4d9056"]) + expect(post.errors[:base]).to include( + I18n.t( + "poll.edit_window_expired.cannot_edit_default_poll_with_votes", + minutes: edit_window + ) + ) end - expect(message.data[:post_id]).to eq(another_post.id) - expect(message.data[:polls]).to eq(different_polls) end - it "should allow staff to edit options if votes have not been casted" do - post.update_attributes!(last_editor_id: User.staff.first.id) - - raw = <<-RAW.strip_heredoc - [poll] - * 3 - * 4 - [/poll] - RAW - - different_post = Fabricate(:post, raw: raw) - different_polls = DiscoursePoll::PollsValidator.new(different_post).validate_polls - - message = MessageBus.track_publish do - described_class.update(post, different_polls) - end.first - - expect(post.reload.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD]).to eq(different_polls) - expect(message.data[:post_id]).to eq(post.id) - expect(message.data[:polls]).to eq(different_polls) - end end + end + end - describe '.extract_option_ids' do - it 'should return an array of the options id' do - expect(described_class.extract_option_ids(polls)).to eq( - ["4d8a15e3cc35750f016ce15a43937620", "cd314db7dfbac2b10687b6f39abfdf41"] - ) - end - end - - describe '.total_votes' do - let!(:post) do - raw = <<-RAW.strip_heredoc - [poll] - * 1 - * 2 - [/poll] - - [poll name=test] - * 1 - * 2 - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - it "should return the right number of votes" do - expect(described_class.total_votes(polls)).to eq(0) - - polls.each { |key, value| value["voters"] = 2 } - - expect(described_class.total_votes(polls)).to eq(4) - end - end end diff --git a/plugins/poll/spec/lib/polls_validator_spec.rb b/plugins/poll/spec/lib/polls_validator_spec.rb index a4d6fc27b63..56cd7d17b8d 100644 --- a/plugins/poll/spec/lib/polls_validator_spec.rb +++ b/plugins/poll/spec/lib/polls_validator_spec.rb @@ -1,11 +1,11 @@ -require 'rails_helper' +require "rails_helper" describe ::DiscoursePoll::PollsValidator do let(:post) { Fabricate(:post) } subject { described_class.new(post) } describe "#validate_polls" do - it "should ensure that polls have unique names" do + it "ensure that polls have unique names" do raw = <<~RAW [poll] * 1 @@ -39,11 +39,11 @@ describe ::DiscoursePoll::PollsValidator do expect(post.update_attributes(raw: raw)).to eq(false) expect(post.errors[:base]).to include( - I18n.t("poll.multiple_polls_with_same_name", name: 'test') + I18n.t("poll.multiple_polls_with_same_name", name: "test") ) end - it 'should ensure that polls have unique options' do + it "ensure that polls have unique options" do raw = <<~RAW [poll] * 1 @@ -67,11 +67,11 @@ describe ::DiscoursePoll::PollsValidator do expect(post.update_attributes(raw: raw)).to eq(false) expect(post.errors[:base]).to include( - I18n.t("poll.named_poll_must_have_different_options", name: 'test') + I18n.t("poll.named_poll_must_have_different_options", name: "test") ) end - it 'should ensure that polls have at least 2 options' do + it "ensure that polls have at least 2 options" do raw = <<~RAW [poll] * 1 @@ -93,11 +93,11 @@ describe ::DiscoursePoll::PollsValidator do expect(post.update_attributes(raw: raw)).to eq(false) expect(post.errors[:base]).to include( - I18n.t("poll.named_poll_must_have_at_least_2_options", name: 'test') + I18n.t("poll.named_poll_must_have_at_least_2_options", name: "test") ) end - it "should ensure that polls' options do not exceed site settings" do + it "ensure that polls options do not exceed site settings" do SiteSetting.poll_maximum_options = 2 raw = <<~RAW @@ -127,12 +127,12 @@ describe ::DiscoursePoll::PollsValidator do expect(post.errors[:base]).to include(I18n.t( "poll.named_poll_must_have_less_options", - name: 'test', count: SiteSetting.poll_maximum_options + name: "test", count: SiteSetting.poll_maximum_options )) end - describe 'multiple type polls' do - it "should ensure that min should not be greater than max" do + describe "multiple type polls" do + it "ensure that min < max" do raw = <<~RAW [poll type=multiple min=2 max=1] * 1 @@ -158,11 +158,11 @@ describe ::DiscoursePoll::PollsValidator do expect(post.update_attributes(raw: raw)).to eq(false) expect(post.errors[:base]).to include( - I18n.t("poll.named_poll_with_multiple_choices_has_invalid_parameters", name: 'test') + I18n.t("poll.named_poll_with_multiple_choices_has_invalid_parameters", name: "test") ) end - it "should ensure max setting is greater than 0" do + it "ensure max > 0" do raw = <<~RAW [poll type=multiple max=-2] * 1 @@ -177,7 +177,7 @@ describe ::DiscoursePoll::PollsValidator do ) end - it "should ensure that max settings is smaller or equal to the number of options" do + it "ensure that max <= number of options" do raw = <<~RAW [poll type=multiple max=3] * 1 @@ -192,7 +192,7 @@ describe ::DiscoursePoll::PollsValidator do ) end - it "should ensure that min settings is not negative" do + it "ensure that min > 0" do raw = <<~RAW [poll type=multiple min=-1] * 1 @@ -207,7 +207,7 @@ describe ::DiscoursePoll::PollsValidator do ) end - it "should ensure that min settings it not equal to zero" do + it "ensure that min != 0" do raw = <<~RAW [poll type=multiple min=0] * 1 @@ -222,7 +222,7 @@ describe ::DiscoursePoll::PollsValidator do ) end - it "should ensure that min settings is not equal to the number of options" do + it "ensure that min != number of options" do raw = <<~RAW [poll type=multiple min=2] * 1 @@ -237,7 +237,7 @@ describe ::DiscoursePoll::PollsValidator do ) end - it "should ensure that min settings is not greater than the number of options" do + it "ensure that min < number of options" do raw = <<~RAW [poll type=multiple min=3] * 1 diff --git a/plugins/poll/spec/lib/votes_updater_spec.rb b/plugins/poll/spec/lib/votes_updater_spec.rb deleted file mode 100644 index 243c5f3b853..00000000000 --- a/plugins/poll/spec/lib/votes_updater_spec.rb +++ /dev/null @@ -1,94 +0,0 @@ -require 'rails_helper' - -describe DiscoursePoll::VotesUpdater do - let(:target_user) { Fabricate(:user_single_email, username: 'alice', email: 'alice@example.com') } - let(:source_user) { Fabricate(:user_single_email, username: 'alice1', email: 'alice@work.com') } - let(:walter) { Fabricate(:walter_white) } - - let(:target_user_id) { target_user.id.to_s } - let(:source_user_id) { source_user.id.to_s } - let(:walter_id) { walter.id.to_s } - - let(:post_with_two_polls) do - raw = <<~RAW - [poll type=multiple min=2 max=3 public=true] - - Option 1 - - Option 2 - - Option 3 - [/poll] - - [poll name=private_poll] - - Option 1 - - Option 2 - - Option 3 - [/poll] - RAW - - Fabricate(:post, raw: raw) - end - - let(:option1_id) { "63eb791ab5d08fc4cc855a0703ac0dd1" } - let(:option2_id) { "773a193533027393806fff6edd6c04f7" } - let(:option3_id) { "f42f567ca3136ee1322d71d7745084c7" } - - def vote(post, user, option_ids, poll_name = nil) - poll_name ||= DiscoursePoll::DEFAULT_POLL_NAME - DiscoursePoll::Poll.vote(post.id, poll_name, option_ids, user) - end - - it "should move votes to the target_user when only the source_user voted" do - vote(post_with_two_polls, source_user, [option1_id, option3_id]) - vote(post_with_two_polls, walter, [option1_id, option2_id]) - - DiscoursePoll::VotesUpdater.merge_users!(source_user, target_user) - post_with_two_polls.reload - - polls = post_with_two_polls.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] - expect(polls["poll"]["options"][0]["votes"]).to eq(2) - expect(polls["poll"]["options"][1]["votes"]).to eq(1) - expect(polls["poll"]["options"][2]["votes"]).to eq(1) - - expect(polls["poll"]["options"][0]["voter_ids"]).to contain_exactly(target_user.id, walter.id) - expect(polls["poll"]["options"][1]["voter_ids"]).to contain_exactly(walter.id) - expect(polls["poll"]["options"][2]["voter_ids"]).to contain_exactly(target_user.id) - - votes = post_with_two_polls.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD] - expect(votes.keys).to contain_exactly(target_user_id, walter_id) - expect(votes[target_user_id]["poll"]).to contain_exactly(option1_id, option3_id) - expect(votes[walter_id]["poll"]).to contain_exactly(option1_id, option2_id) - end - - it "should delete votes of the source_user if the target_user voted" do - vote(post_with_two_polls, source_user, [option1_id, option3_id]) - vote(post_with_two_polls, target_user, [option2_id, option3_id]) - vote(post_with_two_polls, walter, [option1_id, option2_id]) - - DiscoursePoll::VotesUpdater.merge_users!(source_user, target_user) - post_with_two_polls.reload - - polls = post_with_two_polls.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] - expect(polls["poll"]["options"][0]["votes"]).to eq(1) - expect(polls["poll"]["options"][1]["votes"]).to eq(2) - expect(polls["poll"]["options"][2]["votes"]).to eq(1) - - expect(polls["poll"]["options"][0]["voter_ids"]).to contain_exactly(walter.id) - expect(polls["poll"]["options"][1]["voter_ids"]).to contain_exactly(target_user.id, walter.id) - expect(polls["poll"]["options"][2]["voter_ids"]).to contain_exactly(target_user.id) - - votes = post_with_two_polls.custom_fields[DiscoursePoll::VOTES_CUSTOM_FIELD] - expect(votes.keys).to contain_exactly(target_user_id, walter_id) - expect(votes[target_user_id]["poll"]).to contain_exactly(option2_id, option3_id) - expect(votes[walter_id]["poll"]).to contain_exactly(option1_id, option2_id) - end - - it "does not add voter_ids unless the poll is public" do - vote(post_with_two_polls, source_user, [option1_id, option3_id], "private_poll") - vote(post_with_two_polls, walter, [option1_id, option2_id], "private_poll") - - DiscoursePoll::VotesUpdater.merge_users!(source_user, target_user) - post_with_two_polls.reload - - polls = post_with_two_polls.custom_fields[DiscoursePoll::POLLS_CUSTOM_FIELD] - polls["private_poll"]["options"].each { |o| expect(o).to_not have_key("voter_ids") } - end -end diff --git a/plugins/poll/test/javascripts/acceptance/polls-test.js.es6 b/plugins/poll/test/javascripts/acceptance/polls-test.js.es6 index ebfe91455e3..4ae448bc0aa 100644 --- a/plugins/poll/test/javascripts/acceptance/polls-test.js.es6 +++ b/plugins/poll/test/javascripts/acceptance/polls-test.js.es6 @@ -70,8 +70,8 @@ test("Single Poll", async assert => { edit_reason: null, can_view_edit_history: true, wiki: false, - polls: { - poll: { + polls: [ + { options: [ { id: "57ddd734344eb7436d64a7d68a0df444", @@ -88,7 +88,7 @@ test("Single Poll", async assert => { status: "open", name: "poll" }, - test: { + { options: [ { id: "c26ad90783b0d80936e5fdb292b7963c", @@ -105,7 +105,7 @@ test("Single Poll", async assert => { status: "open", name: "test" } - } + ] } ], stream: [19] @@ -391,8 +391,8 @@ test("Public poll", async assert => { edit_reason: null, can_view_edit_history: true, wiki: false, - polls: { - poll: { + polls: [ + { options: [ { id: "4d8a15e3cc35750f016ce15a43937620", @@ -418,7 +418,7 @@ test("Public poll", async assert => { max: "3", public: "true" } - } + ] } ], stream: [15] @@ -596,9 +596,199 @@ test("Public poll", async assert => { server.get("/polls/voters.json", request => { // eslint-disable-line no-undef let body = {}; - if (_.isEqual(request.queryParams, { post_id: "15", poll_name: "poll" })) { + if ( + request.queryParams["post_id"] === "15" && + request.queryParams["poll_name"] === "poll" && + request.queryParams["page"] === "1" && + request.queryParams["option_id"] === "68b434ff88aeae7054e42cd05a4d9056" + ) { body = { - poll: { + voters: { + "68b434ff88aeae7054e42cd05a4d9056": [ + { + id: 402, + username: "bruce400", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 409, + username: "bruce407", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 410, + username: "bruce408", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 411, + username: "bruce409", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 421, + username: "bruce419", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 422, + username: "bruce420", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 423, + username: "bruce421", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 426, + username: "bruce424", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 429, + username: "bruce427", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 437, + username: "bruce435", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 440, + username: "bruce438", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 442, + username: "bruce440", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 443, + username: "bruce441", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 445, + username: "bruce443", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 450, + username: "bruce448", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 451, + username: "bruce449", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 453, + username: "bruce451", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 455, + username: "bruce453", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 456, + username: "bruce454", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 461, + username: "bruce459", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 466, + username: "bruce464", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 468, + username: "bruce466", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 477, + username: "bruce475", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 478, + username: "bruce476", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 498, + username: "bruce496", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + } + ] + } + }; + } else if ( + request.queryParams["post_id"] === "15" && + request.queryParams["poll_name"] === "poll" + ) { + body = { + voters: { "68b434ff88aeae7054e42cd05a4d9056": [ { id: 402, @@ -1132,195 +1322,6 @@ test("Public poll", async assert => { ] } }; - } else if ( - _.isEqual(request.queryParams, { - post_id: "15", - poll_name: "poll", - offset: "1", - option_id: "68b434ff88aeae7054e42cd05a4d9056" - }) - ) { - body = { - poll: { - "68b434ff88aeae7054e42cd05a4d9056": [ - { - id: 402, - username: "bruce400", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 409, - username: "bruce407", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 410, - username: "bruce408", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 411, - username: "bruce409", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 421, - username: "bruce419", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 422, - username: "bruce420", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 423, - username: "bruce421", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 426, - username: "bruce424", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 429, - username: "bruce427", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 437, - username: "bruce435", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 440, - username: "bruce438", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 442, - username: "bruce440", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 443, - username: "bruce441", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 445, - username: "bruce443", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 450, - username: "bruce448", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 451, - username: "bruce449", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 453, - username: "bruce451", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 455, - username: "bruce453", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 456, - username: "bruce454", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 461, - username: "bruce459", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 466, - username: "bruce464", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 468, - username: "bruce466", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 477, - username: "bruce475", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 478, - username: "bruce476", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 498, - username: "bruce496", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - } - ] - } - }; } return [200, { "Content-Type": "application/json" }, body]; @@ -1409,8 +1410,8 @@ test("Public number poll", async assert => { edit_reason: null, can_view_edit_history: true, wiki: false, - polls: { - poll: { + polls: [ + { options: [ { id: "4d8a15e3cc35750f016ce15a43937620", @@ -1522,7 +1523,7 @@ test("Public number poll", async assert => { step: "1", public: "true" } - } + ] } ], stream: [16] @@ -1742,9 +1743,91 @@ test("Public number poll", async assert => { server.get("/polls/voters.json", request => { // eslint-disable-line no-undef let body = {}; - if (_.isEqual(request.queryParams, { post_id: "16", poll_name: "poll" })) { + if ( + request.queryParams["post_id"] === "16" && + request.queryParams["poll_name"] === "poll" && + request.queryParams["page"] === "1" + ) { body = { - poll: [ + voters: [ + { + id: 418, + username: "bruce416", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 420, + username: "bruce418", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 423, + username: "bruce421", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 426, + username: "bruce424", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 428, + username: "bruce426", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 429, + username: "bruce427", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 432, + username: "bruce430", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 433, + username: "bruce431", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 434, + username: "bruce432", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + }, + { + id: 436, + username: "bruce434", + avatar_template: "/images/avatar.png", + name: "Bruce Wayne", + title: null + } + ] + }; + } else if ( + request.queryParams["post_id"] === "16" && + request.queryParams["poll_name"] === "poll" + ) { + body = { + voters: [ { id: 402, username: "bruce400", @@ -1922,87 +2005,6 @@ test("Public number poll", async assert => { } ] }; - } else if ( - _.isEqual(request.queryParams, { - post_id: "16", - poll_name: "poll", - offset: "1" - }) - ) { - body = { - poll: [ - { - id: 418, - username: "bruce416", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 420, - username: "bruce418", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 423, - username: "bruce421", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 426, - username: "bruce424", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 428, - username: "bruce426", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 429, - username: "bruce427", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 432, - username: "bruce430", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 433, - username: "bruce431", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 434, - username: "bruce432", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - }, - { - id: 436, - username: "bruce434", - avatar_template: "/images/avatar.png", - name: "Bruce Wayne", - title: null - } - ] - }; } return [200, { "Content-Type": "application/json" }, body]; diff --git a/spec/components/migration/safe_migrate_spec.rb b/spec/components/migration/safe_migrate_spec.rb index 315cedd646d..62b94805cbd 100644 --- a/spec/components/migration/safe_migrate_spec.rb +++ b/spec/components/migration/safe_migrate_spec.rb @@ -104,7 +104,7 @@ describe Migration::SafeMigrate do migrate_up(path) end - expect(output).to include("drop_table(:users)") + expect(output).to include("drop_table(:email_logs)") end describe 'for a post deployment migration' do @@ -112,13 +112,13 @@ describe Migration::SafeMigrate do user = Fabricate(:user) Migration::SafeMigrate::SafeMigration.enable_safe! - path = File.expand_path "#{Rails.root}/spec/fixtures/db/post_migrate/drop_table" + path = File.expand_path "#{Rails.root}/spec/fixtures/db/post_migrate" output = capture_stdout do migrate_up(path) end - expect(output).to include("drop_table(:users)") + expect(output).to include("drop_table(:email_logs)") expect(user.reload).to eq(user) end end diff --git a/spec/fixtures/db/migrate/drop_table/20990309014014_drop_table.rb b/spec/fixtures/db/migrate/drop_table/20990309014014_drop_table.rb index 211c70d7643..923af0a207a 100644 --- a/spec/fixtures/db/migrate/drop_table/20990309014014_drop_table.rb +++ b/spec/fixtures/db/migrate/drop_table/20990309014014_drop_table.rb @@ -1,6 +1,6 @@ class DropTable < ActiveRecord::Migration[5.1] def up - drop_table :users + drop_table :email_logs end def down diff --git a/spec/fixtures/db/post_migrate/drop_table/20990309014013_drop_users_table.rb b/spec/fixtures/db/post_migrate/drop_table/20990309014013_drop_email_logs_table.rb similarity index 52% rename from spec/fixtures/db/post_migrate/drop_table/20990309014013_drop_users_table.rb rename to spec/fixtures/db/post_migrate/drop_table/20990309014013_drop_email_logs_table.rb index 7dbcf227afb..aac6ac29dfc 100644 --- a/spec/fixtures/db/post_migrate/drop_table/20990309014013_drop_users_table.rb +++ b/spec/fixtures/db/post_migrate/drop_table/20990309014013_drop_email_logs_table.rb @@ -1,6 +1,6 @@ -class DropUsersTable < ActiveRecord::Migration[5.2] +class DropEmailLogsTable < ActiveRecord::Migration[5.2] def up - drop_table :users + drop_table :email_logs raise ActiveRecord::Rollback end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 320b380144c..6898279475c 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -291,3 +291,10 @@ def has_trigger?(trigger_name) WHERE trigger_name = '#{trigger_name}' SQL end + +def silence_stdout + STDOUT.stubs(:write) + yield +ensure + STDOUT.unstub(:write) +end