From 911a47db2846fc6f85f3a6f1ca78dde98150c870 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 20 Jul 2021 15:32:18 +0800 Subject: [PATCH 1/5] IMPROVE: Remove sortable and add manual step and field sorting (#135) * IMPROVE: Remove sortable and add manual step and field sorting * Apply eslint * Apply prettier --- .../discourse/components/wizard-links.js.es6 | 42 +++++++++---------- .../templates/components/wizard-links.hbs | 14 +++++-- plugin.rb | 3 +- 3 files changed, 31 insertions(+), 28 deletions(-) diff --git a/assets/javascripts/discourse/components/wizard-links.js.es6 b/assets/javascripts/discourse/components/wizard-links.js.es6 index c32809aa..50dadfe6 100644 --- a/assets/javascripts/discourse/components/wizard-links.js.es6 +++ b/assets/javascripts/discourse/components/wizard-links.js.es6 @@ -1,15 +1,10 @@ -import { - default as discourseComputed, - observes, - on, -} from "discourse-common/utils/decorators"; +import discourseComputed from "discourse-common/utils/decorators"; import { generateName } from "../lib/wizard"; import { setWizardDefaults, default as wizardSchema, } from "../lib/wizard-schema"; import { notEmpty } from "@ember/object/computed"; -import { scheduleOnce } from "@ember/runloop"; import EmberObject from "@ember/object"; import Component from "@ember/component"; import { A } from "@ember/array"; @@ -19,28 +14,12 @@ export default Component.extend({ items: A(), anyLinks: notEmpty("links"), - @on("didInsertElement") - @observes("links.[]") - setupSortable() { - scheduleOnce("afterRender", () => this.applySortable()); - }, - - applySortable() { - $(this.element) - .find(".link-list") - .sortable({ tolerance: "pointer" }) - .on("sortupdate", (e, ui) => { - this.updateItemOrder(ui.item.data("id"), ui.item.index()); - }); - }, - updateItemOrder(itemId, newIndex) { const items = this.items; const item = items.findBy("id", itemId); items.removeObject(item); item.set("index", newIndex); items.insertAt(newIndex, item); - scheduleOnce("afterRender", this, () => this.applySortable()); }, @discourseComputed("itemType") @@ -58,7 +37,7 @@ export default Component.extend({ return; } - return items.map((item) => { + return items.map((item, index) => { if (item) { let link = { id: item.id, @@ -77,6 +56,15 @@ export default Component.extend({ } link.classes = classes; + link.index = index; + + if (index === 0) { + link.first = true; + } + + if (index === items.length - 1) { + link.last = true; + } return link; } @@ -118,6 +106,14 @@ export default Component.extend({ this.set("current", newItem); }, + back(item) { + this.updateItemOrder(item.id, item.index - 1); + }, + + forward(item) { + this.updateItemOrder(item.id, item.index + 1); + }, + change(itemId) { this.set("current", this.items.findBy("id", itemId)); }, diff --git a/assets/javascripts/discourse/templates/components/wizard-links.hbs b/assets/javascripts/discourse/templates/components/wizard-links.hbs index 5271b043..a7a7662e 100644 --- a/assets/javascripts/discourse/templates/components/wizard-links.hbs +++ b/assets/javascripts/discourse/templates/components/wizard-links.hbs @@ -2,10 +2,16 @@
{{#if anyLinks}} - {{#each links as |l|}} -
- {{d-button action="change" actionParam=l.id translatedLabel=l.label class=l.classes}} - {{d-button action="remove" actionParam=l.id icon="times" class="remove"}} + {{#each links as |link|}} +
+ {{d-button action="change" actionParam=link.id translatedLabel=link.label class=link.classes}} + {{#unless link.first}} + {{d-button action="back" actionParam=link icon="arrow-left" class="back"}} + {{/unless}} + {{#unless link.last}} + {{d-button action="forward" actionParam=link icon="arrow-right" class="forward"}} + {{/unless}} + {{d-button action="remove" actionParam=link.id icon="times" class="remove"}}
{{/each}} {{/if}} diff --git a/plugin.rb b/plugin.rb index 449d0237..dd9d814a 100644 --- a/plugin.rb +++ b/plugin.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # name: discourse-custom-wizard # about: Create custom wizards -# version: 0.8.0 +# version: 0.8.1 # authors: Angus McLeod # url: https://github.com/paviliondev/discourse-custom-wizard # contact emails: angus@thepavilion.io @@ -33,6 +33,7 @@ if respond_to?(:register_svg_icon) register_svg_icon "chevron-right" register_svg_icon "chevron-left" register_svg_icon "save" + register_svg_icon "arrow-right" end class ::Sprockets::DirectiveProcessor From ee9e9a1d9492728d46454edf0c53fabff37e0ed7 Mon Sep 17 00:00:00 2001 From: Faizaan Gagan Date: Wed, 28 Jul 2021 19:01:09 +0530 Subject: [PATCH 2/5] FIX: skip_guardian option is removed from discourse core --- lib/custom_wizard/action.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index 1b5770d7..6a36af41 100644 --- a/lib/custom_wizard/action.rb +++ b/lib/custom_wizard/action.rb @@ -392,7 +392,7 @@ class CustomWizard::Action user_ids.each { |user_id| group.group_users.build(user_id: user_id) } end - GroupActionLogger.new(user, group, skip_guardian: true).log_change_group_settings + GroupActionLogger.new(user, group).log_change_group_settings log_success("Group created", group.name) result.output = group.name From 998757f85717069faf45a43a70b582d4741e0f28 Mon Sep 17 00:00:00 2001 From: Faizaan Gagan Date: Fri, 30 Jul 2021 16:03:48 +0530 Subject: [PATCH 3/5] FIX: apply text length validations for non-empty fields only (#137) * apply text length validations for non-empty fields only * consolidated logic * fixed formatting * fix formatting * calculate length only for string type values * fix assignment * added specs --- lib/custom_wizard/validators/update.rb | 12 +++++---- .../custom_wizard/update_validator_spec.rb | 25 +++++++++++++++++++ 2 files changed, 32 insertions(+), 5 deletions(-) diff --git a/lib/custom_wizard/validators/update.rb b/lib/custom_wizard/validators/update.rb index c722a763..2301760f 100644 --- a/lib/custom_wizard/validators/update.rb +++ b/lib/custom_wizard/validators/update.rb @@ -32,12 +32,14 @@ class ::CustomWizard::UpdateValidator @updater.errors.add(field_id, I18n.t('wizard.field.required', label: label)) end - if min_length.present? && value.is_a?(String) && value.strip.length < min_length.to_i - @updater.errors.add(field_id, I18n.t('wizard.field.too_short', label: label, min: min_length.to_i)) - end + if value.is_a?(String) && (stripped_length = value.strip.length) > 0 + if min_length.present? && stripped_length < min_length.to_i + @updater.errors.add(field_id, I18n.t('wizard.field.too_short', label: label, min: min_length.to_i)) + end - if max_length.present? && value.is_a?(String) && value.strip.length > max_length.to_i - @updater.errors.add(field_id, I18n.t('wizard.field.too_long', label: label, max: max_length.to_i)) + if max_length.present? && stripped_length > max_length.to_i + @updater.errors.add(field_id, I18n.t('wizard.field.too_long', label: label, max: max_length.to_i)) + end end if is_url_type(field) && value.present? && !check_if_url(value) diff --git a/spec/components/custom_wizard/update_validator_spec.rb b/spec/components/custom_wizard/update_validator_spec.rb index e7658d8c..e976e1ff 100644 --- a/spec/components/custom_wizard/update_validator_spec.rb +++ b/spec/components/custom_wizard/update_validator_spec.rb @@ -97,6 +97,31 @@ describe CustomWizard::UpdateValidator do ).to eq(nil) end + it "applies min length only if the input is non-empty" do + min_length = 3 + + @template[:steps][0][:fields][0][:min_length] = min_length + + CustomWizard::Template.save(@template) + + updater = perform_validation('step_1', step_1_field_1: '') + expect( + updater.errors.messages[:step_1_field_1].first + ).to eq(nil) + end + + it "applies max length only if the input is non-empty" do + max_length = 100 + + @template[:steps][0][:fields][0][:max_length] = max_length + + CustomWizard::Template.save(@template) + updater = perform_validation('step_1', step_1_field_1: "") + expect( + updater.errors.messages[:step_1_field_1].first + ).to eq(nil) + end + it 'standardises boolean entries' do updater = perform_validation('step_2', step_2_field_5: 'false') expect(updater.submission['step_2_field_5']).to eq(false) From f00d01c50f4f76ea0a9cc68e9b39b0636df9beb6 Mon Sep 17 00:00:00 2001 From: Faizaan Gagan Date: Mon, 2 Aug 2021 13:01:49 +0530 Subject: [PATCH 4/5] FIX: ensure single incomplete submission existence (#136) * created method to fetch incomplete submissions * added new field 'updated_at' to submission schema * Revert "created method to fetch incomplete submissions" This reverts commit 15c65236e7cb6310be827e187e2a041d37090ea8. * added logic to cleanup redundant incomplete submissions * logic improvements and spec * simplified the cleanup logic * simplified the logic further * early return to improve performance * consolidated cleanup logic * perform cleanup in wizard.update * fixed issue * minor fix * use consistent naming --- lib/custom_wizard/builder.rb | 1 + lib/custom_wizard/submission.rb | 26 ++++++++++- .../custom_wizard/submission_spec.rb | 46 +++++++++++++++++++ 3 files changed, 72 insertions(+), 1 deletion(-) diff --git a/lib/custom_wizard/builder.rb b/lib/custom_wizard/builder.rb index 51ca4183..aede16a2 100644 --- a/lib/custom_wizard/builder.rb +++ b/lib/custom_wizard/builder.rb @@ -75,6 +75,7 @@ class CustomWizard::Builder end @wizard.update! + CustomWizard::Submission.cleanup_incomplete_submissions(@wizard) @wizard end diff --git a/lib/custom_wizard/submission.rb b/lib/custom_wizard/submission.rb index e50cb259..e12299d1 100644 --- a/lib/custom_wizard/submission.rb +++ b/lib/custom_wizard/submission.rb @@ -3,7 +3,7 @@ class CustomWizard::Submission include ActiveModel::SerializerSupport KEY ||= "submissions" - META ||= %w(submitted_at route_to redirect_on_complete redirect_to) + META ||= %w(updated_at submitted_at route_to redirect_on_complete redirect_to) attr_reader :id, :user, @@ -45,6 +45,7 @@ class CustomWizard::Submission submission_list = self.class.list(wizard, user_id: user.id) submissions = submission_list.select { |submission| submission.id != self.id } + self.updated_at = Time.now.iso8601 submissions.push(self) submission_data = submissions.map { |submission| data_to_save(submission) } @@ -96,6 +97,29 @@ class CustomWizard::Submission new(wizard, data, user_id) end + def self.cleanup_incomplete_submissions(wizard) + user_id = wizard.user.id + all_submissions = list(wizard, user_id: user_id) + sorted_submissions = all_submissions.sort_by do |submission| + zero_epoch_time = DateTime.strptime("0", '%s') + [ + submission.submitted_at ? Time.iso8601(submission.submitted_at) : zero_epoch_time, + submission.updated_at ? Time.iso8601(submission.updated_at) : zero_epoch_time + ] + end.reverse + + has_incomplete = false + valid_submissions = sorted_submissions.select do |submission| + to_be_included = submission.submitted_at || !has_incomplete + has_incomplete = true if !submission.submitted_at + + to_be_included + end + + valid_data = valid_submissions.map { |submission| submission.data_to_save(submission) } + PluginStore.set("#{wizard.id}_#{KEY}", user_id, valid_data) + end + def self.list(wizard, user_id: nil, order_by: nil) params = { plugin_name: "#{wizard.id}_#{KEY}" } params[:key] = user_id if user_id.present? diff --git a/spec/components/custom_wizard/submission_spec.rb b/spec/components/custom_wizard/submission_spec.rb index a8c33861..b85af243 100644 --- a/spec/components/custom_wizard/submission_spec.rb +++ b/spec/components/custom_wizard/submission_spec.rb @@ -40,4 +40,50 @@ describe CustomWizard::Submission do it "list submissions by wizard and user" do expect(described_class.list(@wizard, user_id: user.id).size).to eq(1) end + + context "#cleanup_incomplete_submissions" do + it "cleans up redundant incomplete submissions on each build" do + freeze_time Time.now + 1 + described_class.new(@wizard, step_1_field_1: "I am the second submission").save + builder = CustomWizard::Builder.new(@wizard.id, @wizard.user) + builder.build + sub_list = described_class.list(@wizard, user_id: @wizard.user.id) + + expect(sub_list.length).to eq(1) + expect(sub_list.first.fields["step_1_field_1"]).to eq("I am the second submission") + end + + it "handles submissions without 'updated_at' field correctly" do + described_class.new(@wizard, step_1_field_1: "I am the second submission").save + described_class.new(@wizard, step_1_field_1: "I am the third submission").save + sub_data = PluginStore.get("#{@wizard.id}_submissions", @wizard.user.id) + sub_data.each do |sub| + sub['updated_at'] = nil + end + PluginStore.set("#{@wizard.id}_submissions", @wizard.user.id, sub_data) + builder = CustomWizard::Builder.new(@wizard.id, @wizard.user) + builder.build + sub_list = described_class.list(@wizard, user_id: @wizard.user.id) + + expect(sub_list.length).to eq(1) + expect(sub_list.first.fields["step_1_field_1"]).to eq("I am the third submission") + end + + it "handles submissions with and without 'updated_at' field correctly" do + freeze_time Time.now + 1 + described_class.new(@wizard, step_1_field_1: "I am the second submission").save + freeze_time Time.now + 2 + described_class.new(@wizard, step_1_field_1: "I am the third submission").save + sub_data = PluginStore.get("#{@wizard.id}_submissions", @wizard.user.id) + sub_data[0]['updated_at'] = nil + PluginStore.set("#{@wizard.id}_submissions", @wizard.user.id, sub_data) + + builder = CustomWizard::Builder.new(@wizard.id, @wizard.user) + builder.build + sub_list = described_class.list(@wizard, user_id: @wizard.user.id) + + expect(sub_list.length).to eq(1) + expect(sub_list.first.fields["step_1_field_1"]).to eq("I am the third submission") + end + end end From 973cffee26bfa28b243db0b7502d68826b12f8e2 Mon Sep 17 00:00:00 2001 From: Faizaan Gagan Date: Thu, 12 Aug 2021 19:56:17 +0530 Subject: [PATCH 5/5] FIX: disable string caching in `Oj` gem while running specs (#139) --- spec/plugin_helper.rb | 3 +++ 1 file changed, 3 insertions(+) diff --git a/spec/plugin_helper.rb b/spec/plugin_helper.rb index 93f33a81..9e4bbbbe 100644 --- a/spec/plugin_helper.rb +++ b/spec/plugin_helper.rb @@ -11,4 +11,7 @@ if ENV['SIMPLECOV'] end end +require 'oj' +Oj.default_options = Oj.default_options.merge(cache_str: -1) + require 'rails_helper'