From 1661a8745b4ef3bdf0a1fe6a6ac2f33158c95ceb Mon Sep 17 00:00:00 2001 From: Sam Date: Mon, 17 Jul 2017 17:42:13 -0400 Subject: [PATCH] correct issues with poll specs fixes regression where an error message is missing from a poll with one option --- .../lib/discourse-markdown/poll.js.es6 | 168 +----------------- plugins/poll/spec/lib/polls_validator_spec.rb | 32 ++-- plugins/poll/spec/lib/pretty_text_spec.rb | 20 --- 3 files changed, 17 insertions(+), 203 deletions(-) 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 8e9555570b0..76ebf8cdc18 100644 --- a/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 +++ b/plugins/poll/assets/javascripts/lib/discourse-markdown/poll.js.es6 @@ -1,18 +1,8 @@ /*eslint no-bitwise:0 */ -import { registerOption } from 'pretty-text/pretty-text'; const DATA_PREFIX = "data-poll-"; const DEFAULT_POLL_NAME = "poll"; const WHITELISTED_ATTRIBUTES = ["type", "name", "min", "max", "step", "order", "status", "public"]; -const ATTRIBUTES_REGEX = new RegExp("(" + WHITELISTED_ATTRIBUTES.join("|") + ")=['\"]?[^\\s\\]]+['\"]?", "g"); - -registerOption((siteSettings, opts) => { - const currentUser = (opts.getCurrentUser && opts.getCurrentUser(opts.userId)) || opts.currentUser; - const staff = currentUser && currentUser.staff; - - opts.features.poll = !!siteSettings.poll_enabled || staff; - opts.pollMaximumOptions = siteSettings.poll_maximum_options; -}); function getHelpText(count, min, max) { @@ -187,10 +177,6 @@ const rule = { header.push(token); } - if (items.length < 2) { - return invalidPoll(state, raw); - } - // flag items so we add hashes for (let o = 0; o < items.length; o++) { token = items[o][0]; @@ -313,160 +299,8 @@ export function setup(helper) { 'li[data-*]' ]); - if (helper.markdownIt) { - newApiInit(helper); - return; - } + newApiInit(helper); - helper.replaceBlock({ - start: /\[poll((?:\s+\w+=[^\s\]]+)*)\]([\s\S]*)/igm, - stop: /\[\/poll\]/igm, - - emitter(blockContents, matches) { - const contents = []; - - // post-process inside block contents - if (blockContents.length) { - const postProcess = bc => { - if (typeof bc === "string" || bc instanceof String) { - const processed = this.processInline(String(bc)); - if (processed.length) { - contents.push(["p"].concat(processed)); - } - } else { - contents.push(bc); - } - }; - - let b; - while ((b = blockContents.shift()) !== undefined) { - this.processBlock(b, blockContents).forEach(postProcess); - } - } - - // default poll attributes - const attributes = { "class": "poll" }; - attributes[DATA_PREFIX + "status"] = "open"; - attributes[DATA_PREFIX + "name"] = DEFAULT_POLL_NAME; - - // extract poll attributes - (matches[1].match(ATTRIBUTES_REGEX) || []).forEach(function(m) { - const [ name, value ] = m.split("="); - const escaped = helper.escape(value.replace(/["']/g, "")); - attributes[DATA_PREFIX + name] = escaped; - }); - - // we might need these values later... - let min = parseInt(attributes[DATA_PREFIX + "min"], 10); - let max = parseInt(attributes[DATA_PREFIX + "max"], 10); - let step = parseInt(attributes[DATA_PREFIX + "step"], 10); - - // generate the options when the type is "number" - if (attributes[DATA_PREFIX + "type"] === "number") { - // default values - if (isNaN(min)) { min = 1; } - if (isNaN(max)) { max = helper.getOptions().pollMaximumOptions; } - if (isNaN(step)) { step = 1; } - // dynamically generate options - contents.push(["bulletlist"]); - for (let o = min; o <= max; o += step) { - contents[0].push(["listitem", String(o)]); - } - } - - // make sure there's only 1 child and it's a list with at least 1 option - if (contents.length !== 1 || contents[0].length <= 1 || (contents[0][0] !== "numberlist" && contents[0][0] !== "bulletlist")) { - return ["div"].concat(contents); - } - - // make sure there's only options in the list - for (let o=1; o < contents[0].length; o++) { - if (contents[0][o][0] !== "listitem") { - return ["div"].concat(contents); - } - } - - // TODO: remove non whitelisted content - - // add option id (hash) - for (let o = 1; o < contents[0].length; o++) { - const attr = {}; - // compute md5 hash of the content of the option - attr[DATA_PREFIX + "option-id"] = md5(JSON.stringify(contents[0][o].slice(1))); - // store options attributes - contents[0][o].splice(1, 0, attr); - } - - const result = ["div", attributes], - poll = ["div"]; - - // 1 - POLL CONTAINER - const container = ["div", { "class": "poll-container" }].concat(contents); - poll.push(container); - - // 2 - POLL INFO - const info = ["div", { "class": "poll-info" }]; - - // # of voters - info.push(["p", - ["span", { "class": "info-number" }, "0"], - ["span", { "class": "info-text"}, I18n.t("poll.voters", { count: 0 })] - ]); - - // multiple help text - if (attributes[DATA_PREFIX + "type"] === "multiple") { - const optionCount = contents[0].length - 1; - - // default values - if (isNaN(min) || min < 1) { min = 1; } - if (isNaN(max) || max > optionCount) { max = optionCount; } - - // add some help text - let help; - - if (max > 0) { - if (min === max) { - if (min > 1) { - help = I18n.t("poll.multiple.help.x_options", { count: min }); - } - } else if (min > 1) { - if (max < optionCount) { - help = I18n.t("poll.multiple.help.between_min_and_max_options", { min: min, max: max }); - } else { - help = I18n.t("poll.multiple.help.at_least_min_options", { count: min }); - } - } else if (max <= optionCount) { - help = I18n.t("poll.multiple.help.up_to_max_options", { count: max }); - } - } - - if (help) { info.push(["p", help]); } - } - - if (attributes[DATA_PREFIX + "public"] === "true") { - info.push(["p", I18n.t("poll.public.title")]); - } - - poll.push(info); - - // 3 - BUTTONS - const buttons = ["div", { "class": "poll-buttons" }]; - - // add "cast-votes" button - if (attributes[DATA_PREFIX + "type"] === "multiple") { - buttons.push(["a", { "class": "button cast-votes", "title": I18n.t("poll.cast-votes.title") }, I18n.t("poll.cast-votes.label")]); - } - - // add "toggle-results" button - buttons.push(["a", { "class": "button toggle-results", "title": I18n.t("poll.show-results.title") }, I18n.t("poll.show-results.label")]); - - // 4 - MIX IT ALL UP - result.push(poll); - result.push(buttons); - - return result; - } - }); } /*! diff --git a/plugins/poll/spec/lib/polls_validator_spec.rb b/plugins/poll/spec/lib/polls_validator_spec.rb index d5178fe854b..629fc0527af 100644 --- a/plugins/poll/spec/lib/polls_validator_spec.rb +++ b/plugins/poll/spec/lib/polls_validator_spec.rb @@ -6,7 +6,7 @@ describe ::DiscoursePoll::PollsValidator do describe "#validate_polls" do it "should ensure that polls have unique names" do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll] * 1 * 2 @@ -24,7 +24,7 @@ describe ::DiscoursePoll::PollsValidator do I18n.t("poll.multiple_polls_without_name") ) - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll name=test] * 1 * 2 @@ -44,7 +44,7 @@ describe ::DiscoursePoll::PollsValidator do end it 'should ensure that polls have unique options' do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll] * 1 * 1 @@ -57,7 +57,7 @@ describe ::DiscoursePoll::PollsValidator do I18n.t("poll.default_poll_must_have_different_options") ) - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll name=test] * 1 * 1 @@ -73,7 +73,7 @@ describe ::DiscoursePoll::PollsValidator do it 'should ensure that polls have at least 2 options' do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll] * 1 [/poll] @@ -85,7 +85,7 @@ describe ::DiscoursePoll::PollsValidator do I18n.t("poll.default_poll_must_have_at_least_2_options") ) - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll name=test] * 1 [/poll] @@ -101,7 +101,7 @@ describe ::DiscoursePoll::PollsValidator do it "should ensure that polls' options do not exceed site settings" do SiteSetting.poll_maximum_options = 2 - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll] * 1 * 2 @@ -116,7 +116,7 @@ describe ::DiscoursePoll::PollsValidator do count: SiteSetting.poll_maximum_options )) - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll name=test] * 1 * 2 @@ -134,7 +134,7 @@ describe ::DiscoursePoll::PollsValidator do describe 'multiple type polls' do it "should ensure that min should not be greater than max" do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll type=multiple min=2 max=1] * 1 * 2 @@ -148,7 +148,7 @@ describe ::DiscoursePoll::PollsValidator do I18n.t("poll.default_poll_with_multiple_choices_has_invalid_parameters") ) - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll type=multiple min=2 max=1 name=test] * 1 * 2 @@ -164,7 +164,7 @@ describe ::DiscoursePoll::PollsValidator do end it "should ensure max setting is greater than 0" do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll type=multiple max=-2] * 1 * 2 @@ -179,7 +179,7 @@ describe ::DiscoursePoll::PollsValidator do end it "should ensure that max settings is smaller or equal to the number of options" do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll type=multiple max=3] * 1 * 2 @@ -194,7 +194,7 @@ describe ::DiscoursePoll::PollsValidator do end it "should ensure that min settings is not negative" do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll type=multiple min=-1] * 1 * 2 @@ -209,7 +209,7 @@ describe ::DiscoursePoll::PollsValidator do end it "should ensure that min settings it not equal to zero" do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll type=multiple min=0] * 1 * 2 @@ -224,7 +224,7 @@ describe ::DiscoursePoll::PollsValidator do end it "should ensure that min settings is not equal to the number of options" do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll type=multiple min=2] * 1 * 2 @@ -239,7 +239,7 @@ describe ::DiscoursePoll::PollsValidator do end it "should ensure that min settings is not greater than the number of options" do - raw = <<-RAW.strip_heredoc + raw = <<~RAW [poll type=multiple min=3] * 1 * 2 diff --git a/plugins/poll/spec/lib/pretty_text_spec.rb b/plugins/poll/spec/lib/pretty_text_spec.rb index 083a9b2d9f4..35bbd18530d 100644 --- a/plugins/poll/spec/lib/pretty_text_spec.rb +++ b/plugins/poll/spec/lib/pretty_text_spec.rb @@ -8,10 +8,6 @@ describe PrettyText do end context 'markdown it' do - before do - SiteSetting.enable_experimental_markdown_it = true - end - it 'supports multi choice polls' do cooked = PrettyText.cook <<~MD [poll type=multiple min=1 max=3 public=true] @@ -61,22 +57,6 @@ describe PrettyText do expect(cooked.scan('class="poll"').length).to eq(2) end - it 'works correctly for new vs old engine with trivial cases' do - md = <<~MD - [poll] - 1. test 1 - 2. test 2 - [/poll] - MD - - new_engine = n(PrettyText.cook(md)) - - SiteSetting.enable_experimental_markdown_it = false - old_engine = n(PrettyText.cook(md)) - - expect(new_engine).to eq(old_engine) - end - it 'does not break poll options when going from loose to tight' do md = <<~MD [poll type=multiple]