diff --git a/plugins/poll/app/models/poll.rb b/plugins/poll/app/models/poll.rb index 192e599ef8a..fd5422a620b 100644 --- a/plugins/poll/app/models/poll.rb +++ b/plugins/poll/app/models/poll.rb @@ -29,7 +29,7 @@ class Poll < ActiveRecord::Base everyone: 1, } - validates :min, numericality: { allow_nil: true, only_integer: true, greater_than: 0 } + validates :min, numericality: { allow_nil: true, only_integer: true, greater_than_or_equal_to: 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 } diff --git a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 index 88195905657..0ac240dee62 100644 --- a/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 +++ b/plugins/poll/assets/javascripts/widgets/discourse-poll.js.es6 @@ -8,6 +8,7 @@ import evenRound from "discourse/plugins/poll/lib/even-round"; import { avatarFor } from "discourse/widgets/post"; import round from "discourse/lib/round"; import { relativeAge } from "discourse/lib/formatter"; +import { userPath } from "discourse/lib/url"; function optionHtml(option) { return new RawHtml({ html: `${option.html}` }); @@ -143,6 +144,7 @@ createWidget("discourse-poll-voters", { return h("li", [ avatarFor("tiny", { username: user.username, + url: userPath(user.username), template: user.avatar_template }), " " @@ -560,8 +562,8 @@ export default createWidget("discourse-poll", { min() { let min = parseInt(this.attrs.poll.get("min"), 10); - if (isNaN(min) || min < 1) { - min = 1; + if (isNaN(min) || min < 0) { + min = 0; } return min; }, diff --git a/plugins/poll/config/locales/server.en.yml b/plugins/poll/config/locales/server.en.yml index 6d5274d6413..8e4abe64a84 100644 --- a/plugins/poll/config/locales/server.en.yml +++ b/plugins/poll/config/locales/server.en.yml @@ -22,6 +22,8 @@ en: poll_minimum_trust_level_to_create: "Define the minimum trust level needed to create polls." poll: + invalid_argument: "Invalid value '%{value}' for argument '%{argument}'." + multiple_polls_without_name: "There are multiple polls without a name. Use the 'name' attribute to uniquely identify your polls." multiple_polls_with_same_name: "There are multiple polls with the same name: %{name}. Use the 'name' attribute to uniquely identify your polls." diff --git a/plugins/poll/lib/polls_validator.rb b/plugins/poll/lib/polls_validator.rb index 790d8e2c010..fca71f89fc4 100644 --- a/plugins/poll/lib/polls_validator.rb +++ b/plugins/poll/lib/polls_validator.rb @@ -1,5 +1,8 @@ module DiscoursePoll class PollsValidator + + MAX_VALUE = 2_147_483_647 + def initialize(post) @post = post end @@ -8,6 +11,8 @@ module DiscoursePoll polls = {} DiscoursePoll::Poll::extract(@post.raw, @post.topic_id, @post.user_id).each do |poll| + return false unless valid_arguments?(poll) + return false unless valid_numbers?(poll) return false unless unique_poll_name?(polls, poll) return false unless unique_options?(poll) return false unless at_least_two_options?(poll) @@ -21,6 +26,27 @@ module DiscoursePoll private + def valid_arguments?(poll) + valid = true + + if poll["type"].present? && !::Poll.types.has_key?(poll["type"]) + @post.errors.add(:base, I18n.t("poll.invalid_argument", argument: "type", value: poll["type"])) + valid = false + end + + if poll["status"].present? && !::Poll.statuses.has_key?(poll["status"]) + @post.errors.add(:base, I18n.t("poll.invalid_argument", argument: "status", value: poll["status"])) + valid = false + end + + if poll["results"].present? && !::Poll.results.has_key?(poll["results"]) + @post.errors.add(:base, I18n.t("poll.invalid_argument", argument: "results", value: poll["results"])) + valid = false + end + + valid + end + def unique_poll_name?(polls, poll) if polls.has_key?(poll["name"]) if poll["name"] == ::DiscoursePoll::DEFAULT_POLL_NAME @@ -96,5 +122,45 @@ module DiscoursePoll true end + + def valid_numbers?(poll) + return true if poll["type"] != "number" + + valid = true + + min = poll["min"].to_i + max = (poll["max"].presence || MAX_VALUE).to_i + step = (poll["step"].presence || 1).to_i + + if min < 0 + @post.errors.add(:base, "Min #{I18n.t("errors.messages.greater_than", count: 0)}") + valid = false + elsif min > MAX_VALUE + @post.errors.add(:base, "Min #{I18n.t("errors.messages.less_than", count: MAX_VALUE)}") + valid = false + end + + if max < min + @post.errors.add(:base, "Max #{I18n.t("errors.messages.greater_than", count: "min")}") + valid = false + elsif max > MAX_VALUE + @post.errors.add(:base, "Max #{I18n.t("errors.messages.less_than", count: MAX_VALUE)}") + valid = false + end + + if step <= 0 + @post.errors.add(:base, "Step #{I18n.t("errors.messages.greater_than", count: 0)}") + valid = false + elsif ((max - min + 1) / step) < 2 + if poll["name"] == ::DiscoursePoll::DEFAULT_POLL_NAME + @post.errors.add(:base, I18n.t("poll.default_poll_must_have_at_least_2_options")) + else + @post.errors.add(:base, I18n.t("poll.named_poll_must_have_at_least_2_options", name: poll["name"])) + end + valid = false + end + + valid + end end end diff --git a/plugins/poll/spec/lib/polls_validator_spec.rb b/plugins/poll/spec/lib/polls_validator_spec.rb index 56cd7d17b8d..ee2d4b37e70 100644 --- a/plugins/poll/spec/lib/polls_validator_spec.rb +++ b/plugins/poll/spec/lib/polls_validator_spec.rb @@ -5,6 +5,53 @@ describe ::DiscoursePoll::PollsValidator do subject { described_class.new(post) } describe "#validate_polls" do + it "ensures that polls have valid arguments" do + raw = <<~RAW + [poll type=not_good1 status=not_good2 results=not_good3] + * 1 + * 2 + [/poll] + RAW + + post.raw = raw + expect(post.valid?).to eq(false) + + expect(post.errors[:base]).to include(I18n.t("poll.invalid_argument", argument: "type", value: "not_good1")) + expect(post.errors[:base]).to include(I18n.t("poll.invalid_argument", argument: "status", value: "not_good2")) + expect(post.errors[:base]).to include(I18n.t("poll.invalid_argument", argument: "results", value: "not_good3")) + end + + it "ensures that all possible values are valid" do + raw = <<~RAW + [poll type=regular result=always] + * 1 + * 2 + [/poll] + RAW + + post.raw = raw + expect(post.valid?).to eq(true) + + raw = <<~RAW + [poll type=multiple result=on_vote min=1 max=2] + * 1 + * 2 + * 3 + [/poll] + RAW + + post.raw = raw + expect(post.valid?).to eq(true) + + raw = <<~RAW + [poll type=number result=on_close min=3 max=7] + [/poll] + RAW + + post.raw = raw + expect(post.valid?).to eq(true) + end + it "ensure that polls have unique names" do raw = <<~RAW [poll] @@ -18,7 +65,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.multiple_polls_without_name") @@ -36,7 +84,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.multiple_polls_with_same_name", name: "test") @@ -51,7 +100,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.default_poll_must_have_different_options") @@ -64,7 +114,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.named_poll_must_have_different_options", name: "test") @@ -78,7 +129,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.default_poll_must_have_at_least_2_options") @@ -90,7 +142,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.named_poll_must_have_at_least_2_options", name: "test") @@ -108,7 +161,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include(I18n.t( "poll.default_poll_must_have_less_options", @@ -123,7 +177,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include(I18n.t( "poll.named_poll_must_have_less_options", @@ -141,7 +196,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters") @@ -155,7 +211,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.named_poll_with_multiple_choices_has_invalid_parameters", name: "test") @@ -170,7 +227,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters") @@ -185,14 +243,15 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters") ) end - it "ensure that min > 0" do + it "ensure that min >= 0" do raw = <<~RAW [poll type=multiple min=-1] * 1 @@ -200,14 +259,15 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters") ) end - it "ensure that min != 0" do + it "ensure that min cannot be 0" do raw = <<~RAW [poll type=multiple min=0] * 1 @@ -215,11 +275,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) - - expect(post.errors[:base]).to include( - I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters") - ) + post.raw = raw + expect(post.valid?).to eq(false) end it "ensure that min != number of options" do @@ -230,7 +287,8 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters") @@ -245,12 +303,37 @@ describe ::DiscoursePoll::PollsValidator do [/poll] RAW - expect(post.update_attributes(raw: raw)).to eq(false) + post.raw = raw + expect(post.valid?).to eq(false) expect(post.errors[:base]).to include( I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters") ) end end + + it "number type polls are validated" do + raw = <<~RAW + [poll type=number min=-5 max=-10 step=-1] + [/poll] + RAW + + post.raw = raw + expect(post.valid?).to eq(false) + expect(post.errors[:base]).to include("Min #{I18n.t("errors.messages.greater_than", count: 0)}") + expect(post.errors[:base]).to include("Max #{I18n.t("errors.messages.greater_than", count: "min")}") + expect(post.errors[:base]).to include("Step #{I18n.t("errors.messages.greater_than", count: 0)}") + + raw = <<~RAW + [poll type=number min=9999999999 max=9999999999 step=1] + [/poll] + RAW + + post.raw = raw + expect(post.valid?).to eq(false) + expect(post.errors[:base]).to include("Min #{I18n.t("errors.messages.less_than", count: 2_147_483_647)}") + expect(post.errors[:base]).to include("Max #{I18n.t("errors.messages.less_than", count: 2_147_483_647)}") + expect(post.errors[:base]).to include(I18n.t("poll.default_poll_must_have_at_least_2_options")) + end 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 1f4dbf4f5b1..357af88094b 100644 --- a/plugins/poll/test/javascripts/acceptance/polls-test.js.es6 +++ b/plugins/poll/test/javascripts/acceptance/polls-test.js.es6 @@ -2023,6 +2023,8 @@ test("Public number poll", async assert => { "it should display the right number of voters" ); + assert.ok(find(".poll-voters:first li:first a").attr("href"), "user URL exists"); + await click(".poll-voters-toggle-expand:first a"); assert.equal(