FIX: Validate poll arguments. (#6740)

* FIX: Validate number poll.
* FEATURE: Poll's min can be 0.
* FIX: Fix URL to user profile.
This commit is contained in:
Bianca Nenciu 2018-12-31 11:48:30 +02:00 committed by Régis Hanol
parent 0fca3205b5
commit e49bcebb35
6 changed files with 180 additions and 25 deletions

View File

@ -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 }

View File

@ -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: `<span>${option.html}</span>` });
@ -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;
},

View File

@ -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 '<code>name</code>' attribute to uniquely identify your polls."
multiple_polls_with_same_name: "There are multiple polls with the same name: <strong>%{name}</strong>. Use the '<code>name</code>' attribute to uniquely identify your polls."

View File

@ -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

View File

@ -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

View File

@ -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(