From c1007e78f5315ca1ed4dd10507a57843e842d87b Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Sat, 24 Dec 2022 09:42:09 +0100 Subject: [PATCH 01/16] WIP --- app/controllers/custom_wizard/admin/wizard.rb | 1 + app/controllers/custom_wizard/steps.rb | 1 - app/controllers/custom_wizard/wizard.rb | 1 - .../custom_wizard/wizard_serializer.rb | 3 +- .../wizard-subscription-selector.js.es6 | 23 +- .../discourse/lib/wizard-schema.js.es6 | 11 + .../routes/custom-wizard-index.js.es6 | 3 +- .../routes/custom-wizard-step.js.es6 | 2 +- .../templates/admin-wizards-wizard-show.hbs | 10 + .../components/wizard-custom-action.hbs | 1 + config/locales/client.en.yml | 2 + lib/custom_wizard/action.rb | 15 + lib/custom_wizard/builder.rb | 8 +- lib/custom_wizard/mapper.rb | 2 +- lib/custom_wizard/subscription.rb | 1 + lib/custom_wizard/validators/template.rb | 7 + lib/custom_wizard/wizard.rb | 4 + .../custom_wizard/steps_controller_spec.rb | 409 +++++++++--------- .../custom_wizard/wizard_controller_spec.rb | 104 ++--- 19 files changed, 342 insertions(+), 266 deletions(-) diff --git a/app/controllers/custom_wizard/admin/wizard.rb b/app/controllers/custom_wizard/admin/wizard.rb index 08e7b6d0..4cad3f42 100644 --- a/app/controllers/custom_wizard/admin/wizard.rb +++ b/app/controllers/custom_wizard/admin/wizard.rb @@ -80,6 +80,7 @@ class CustomWizard::AdminWizardController < CustomWizard::AdminController :prompt_completion, :restart_on_revisit, :resume_on_revisit, + :allow_guests, :theme_id, permitted: mapped_params, steps: [ diff --git a/app/controllers/custom_wizard/steps.rb b/app/controllers/custom_wizard/steps.rb index df3c2cb3..ea2a75b8 100644 --- a/app/controllers/custom_wizard/steps.rb +++ b/app/controllers/custom_wizard/steps.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true class CustomWizard::StepsController < ::ApplicationController - before_action :ensure_logged_in before_action :ensure_can_update def update diff --git a/app/controllers/custom_wizard/wizard.rb b/app/controllers/custom_wizard/wizard.rb index 7aafdd3b..86265af4 100644 --- a/app/controllers/custom_wizard/wizard.rb +++ b/app/controllers/custom_wizard/wizard.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true class CustomWizard::WizardController < ::ApplicationController before_action :ensure_plugin_enabled - before_action :ensure_logged_in, only: [:skip] def show if wizard.present? diff --git a/app/serializers/custom_wizard/wizard_serializer.rb b/app/serializers/custom_wizard/wizard_serializer.rb index 9741d7af..8b3caba1 100644 --- a/app/serializers/custom_wizard/wizard_serializer.rb +++ b/app/serializers/custom_wizard/wizard_serializer.rb @@ -9,7 +9,8 @@ class CustomWizard::WizardSerializer < CustomWizard::BasicWizardSerializer :completed, :required, :permitted, - :resume_on_revisit + :resume_on_revisit, + :allow_guests has_many :steps, serializer: ::CustomWizard::StepSerializer, embed: :objects has_one :user, serializer: ::BasicUserSerializer, embed: :objects diff --git a/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 b/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 index 53f7d19c..58c6715d 100644 --- a/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 +++ b/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 @@ -40,9 +40,26 @@ export default SingleSelectComponent.extend(Subscription, { return allowedTypes; }, - @discourseComputed("feature", "attribute") - content(feature, attribute) { - return wizardSchema[feature][attribute] + contentList(feature, attribute, allowGuests) { + let attributes = wizardSchema[feature][attribute]; + + if (allowGuests) { + const filteredFeature = wizardSchema.filters.allow_guests[feature]; + if (filteredFeature) { + + const filteredAttribute = filteredFeature[attribute]; + if (filteredAttribute) { + attributes = attributes.filter(a => filteredAttribute.includes(a)) + } + } + } + + return attributes; + }, + + @discourseComputed("feature", "attribute", "wizard.allow_guests") + content(feature, attribute, allowGuests) { + return this.contentList(feature, attribute, allowGuests) .map((value) => { let allowedSubscriptionTypes = this.allowedSubscriptionTypes( feature, diff --git a/assets/javascripts/discourse/lib/wizard-schema.js.es6 b/assets/javascripts/discourse/lib/wizard-schema.js.es6 index 69254695..a1756b5f 100644 --- a/assets/javascripts/discourse/lib/wizard-schema.js.es6 +++ b/assets/javascripts/discourse/lib/wizard-schema.js.es6 @@ -15,8 +15,10 @@ const wizard = { prompt_completion: null, restart_on_revisit: null, resume_on_revisit: null, + allow_guests: null, theme_id: null, permitted: null, + allow_guests: null }, mapped: ["permitted"], required: ["id"], @@ -204,6 +206,14 @@ const action = { objectArrays: {}, }; +const filters = { + allow_guests: { + action: { + type: ['route_to'] + } + } +} + const custom_field = { klass: ["topic", "post", "group", "category"], type: ["string", "boolean", "integer", "json"], @@ -218,6 +228,7 @@ const wizardSchema = { field, custom_field, action, + filters }; export function buildFieldTypes(types) { diff --git a/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 b/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 index 1d5a71c7..d6917650 100644 --- a/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 +++ b/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 @@ -6,7 +6,6 @@ export default Route.extend({ const wizard = getCachedWizard(); if ( wizard && - wizard.user && wizard.permitted && !wizard.completed && wizard.start @@ -26,7 +25,7 @@ export default Route.extend({ const wizardId = model.get("id"); const user = model.get("user"); const name = model.get("name"); - const requiresLogin = !user; + const requiresLogin = !user && !model.get("allow_guests"); const notPermitted = !permitted; const props = { diff --git a/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 b/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 index 969df1eb..dd7b8be8 100644 --- a/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 +++ b/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 @@ -7,7 +7,7 @@ export default Route.extend({ const wizard = getCachedWizard(); this.set("wizard", wizard); - if (!wizard || !wizard.user || !wizard.permitted || wizard.completed) { + if (!wizard || !wizard.permitted || wizard.completed) { this.replaceWith("customWizard"); } }, diff --git a/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs b/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs index 11a2b415..c14dbfea 100644 --- a/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs +++ b/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs @@ -146,6 +146,16 @@ )}} + +
+
+ +
+
+ {{input type="checkbox" checked=wizard.allow_guests}} + {{i18n "admin.wizard.allow_guests_label"}} +
+
{{/wizard-subscription-container}} diff --git a/assets/javascripts/discourse/templates/components/wizard-custom-action.hbs b/assets/javascripts/discourse/templates/components/wizard-custom-action.hbs index 51ff000e..d23ca0b5 100644 --- a/assets/javascripts/discourse/templates/components/wizard-custom-action.hbs +++ b/assets/javascripts/discourse/templates/components/wizard-custom-action.hbs @@ -17,6 +17,7 @@ feature="action" attribute="type" onChange=(action "changeType") + wizard=wizard options=(hash none="admin.wizard.select_type" ) diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index b3adf8d4..d70cc9a1 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -105,6 +105,8 @@ en: restart_on_revisit_label: "Clear submissions on each visit." resume_on_revisit: "Resume" resume_on_revisit_label: "Ask the user if they want to resume on each visit." + allow_guests: "Guests" + allow_guests_label: "Allow guests to use the wizard (disables user-specific features)." theme_id: "Theme" no_theme: "Select a Theme (optional)" save: "Save Changes" diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index 5917a1bc..00a200ce 100644 --- a/lib/custom_wizard/action.rb +++ b/lib/custom_wizard/action.rb @@ -6,6 +6,15 @@ class CustomWizard::Action :guardian, :result + REQUIRES_USER = %w[ + create_topic + update_profile + open_composer + send_message + watch_categories + add_to_group + ] + def initialize(opts) @wizard = opts[:wizard] @action = opts[:action] @@ -17,6 +26,12 @@ class CustomWizard::Action end def perform + if REQUIRES_USER.include?(action['id']) && !@user + log_error("action requires user", "id: #{action['id']};") + @result.success = false + return @result + end + ActiveRecord::Base.transaction do self.send(action['type'].to_sym) end diff --git a/lib/custom_wizard/builder.rb b/lib/custom_wizard/builder.rb index 3b12ad27..c510861e 100644 --- a/lib/custom_wizard/builder.rb +++ b/lib/custom_wizard/builder.rb @@ -182,7 +182,7 @@ class CustomWizard::Builder if field_template['description'].present? params[:description] = mapper.interpolate( field_template['description'], - user: true, + user: @wizard.user.present?, value: true, wizard: true, template: true @@ -192,7 +192,7 @@ class CustomWizard::Builder if field_template['preview_template'].present? preview_template = mapper.interpolate( field_template['preview_template'], - user: true, + user: @wizard.user.present?, value: true, wizard: true, template: true @@ -204,7 +204,7 @@ class CustomWizard::Builder if field_template['placeholder'].present? params[:placeholder] = mapper.interpolate( field_template['placeholder'], - user: true, + user: @wizard.user.present?, value: true, wizard: true, template: true @@ -248,7 +248,7 @@ class CustomWizard::Builder if step_template['description'] step.description = mapper.interpolate( step_template['description'], - user: true, + user: @wizard.user.present?, value: true, wizard: true, template: true diff --git a/lib/custom_wizard/mapper.rb b/lib/custom_wizard/mapper.rb index b677a710..e23decee 100644 --- a/lib/custom_wizard/mapper.rb +++ b/lib/custom_wizard/mapper.rb @@ -229,7 +229,7 @@ class CustomWizard::Mapper def interpolate(string, opts = { user: true, wizard: true, value: true, template: false }) return string if string.blank? || string.frozen? - if opts[:user] + if opts[:user] && @user.present? string.gsub!(/u\{(.*?)\}/) { |match| map_user_field($1) || '' } end diff --git a/lib/custom_wizard/subscription.rb b/lib/custom_wizard/subscription.rb index 700e6087..548ae67d 100644 --- a/lib/custom_wizard/subscription.rb +++ b/lib/custom_wizard/subscription.rb @@ -137,6 +137,7 @@ class CustomWizard::Subscription end def business? + return true @subscription.product_id === BUSINESS_PRODUCT_ID end diff --git a/lib/custom_wizard/validators/template.rb b/lib/custom_wizard/validators/template.rb index 079f9884..59626a69 100644 --- a/lib/custom_wizard/validators/template.rb +++ b/lib/custom_wizard/validators/template.rb @@ -39,6 +39,7 @@ class CustomWizard::TemplateValidator validate_subscription(action, :action) check_required(action, :action) validate_liquid_template(action, :action) + validate_action(action) end end @@ -80,6 +81,12 @@ class CustomWizard::TemplateValidator end end + def validate_action(action) + if @data[:allow_guests] && CustomWizard::Action::REQUIRES_USER.include?(action[:type]) + errors.add :base, I18n.t("wizard.validation.conflict", wizard_id: action[:id]) + end + end + def validate_after_signup return unless ActiveRecord::Type::Boolean.new.cast(@data[:after_signup]) diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index 223aeaa5..1293cfe3 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -22,6 +22,7 @@ class CustomWizard::Wizard :prompt_completion, :restart_on_revisit, :resume_on_revisit, + :allow_guests, :permitted, :steps, :step_ids, @@ -48,6 +49,7 @@ class CustomWizard::Wizard @prompt_completion = cast_bool(attrs['prompt_completion']) @restart_on_revisit = cast_bool(attrs['restart_on_revisit']) @resume_on_revisit = cast_bool(attrs['resume_on_revisit']) + @allow_guests = cast_bool(attrs['allow_guests']) @after_signup = cast_bool(attrs['after_signup']) @after_time = cast_bool(attrs['after_time']) @after_time_scheduled = attrs['after_time_scheduled'] @@ -200,6 +202,7 @@ class CustomWizard::Wizard end def permitted? + return true if allow_guests return false unless user return true if user.admin? || permitted.blank? @@ -227,6 +230,7 @@ class CustomWizard::Wizard end def can_access? + return true if allow_guests return false unless user return true if user.admin permitted? && (multiple_submissions || !completed?) diff --git a/spec/requests/custom_wizard/steps_controller_spec.rb b/spec/requests/custom_wizard/steps_controller_spec.rb index e05ba917..cec02bc4 100644 --- a/spec/requests/custom_wizard/steps_controller_spec.rb +++ b/spec/requests/custom_wizard/steps_controller_spec.rb @@ -10,189 +10,108 @@ describe CustomWizard::StepsController do before do CustomWizard::Template.save(wizard_template, skip_jobs: true) - sign_in(user) end - it 'performs a step update' do - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Text input" - } - } - expect(response.status).to eq(200) - expect(response.parsed_body['wizard']['start']).to eq("step_2") - - wizard_id = response.parsed_body['wizard']['id'] - wizard = CustomWizard::Wizard.create(wizard_id, user) - expect(wizard.current_submission.fields['step_1_field_1']).to eq("Text input") - end - - context "raises an error" do - it "when the wizard doesnt exist" do - put '/w/not-super-mega-fun-wizard/steps/step_1.json' - expect(response.status).to eq(400) - end - - it "when the user cant access the wizard" do - enable_subscription("standard") - new_template = wizard_template.dup - new_template["permitted"] = permitted_json["permitted"] - CustomWizard::Template.save(new_template, skip_jobs: true) - - put '/w/super-mega-fun-wizard/steps/step_1.json' - expect(response.status).to eq(403) - end - - it "when the step doesnt exist" do - put '/w/super-mega-fun-wizard/steps/step_10.json' - expect(response.status).to eq(400) - end - end - - it "works if the step has no fields" do - put '/w/super-mega-fun-wizard/steps/step_1.json' - expect(response.status).to eq(200) - expect(response.parsed_body['wizard']['start']).to eq("step_2") - end - - it "returns an updated wizard when condition passes" do - new_template = wizard_template.dup - new_template['steps'][1]['condition'] = wizard_field_condition_template['condition'] - CustomWizard::Template.save(new_template, skip_jobs: true) - - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Condition will pass" - } - } - expect(response.status).to eq(200) - expect(response.parsed_body['wizard']['start']).to eq("step_2") - end - - it "runs completion actions if user has completed wizard" do - new_template = wizard_template.dup - - ## route_to action - new_template['actions'].last['run_after'] = 'wizard_completion' - CustomWizard::Template.save(new_template, skip_jobs: true) - - put '/w/super-mega-fun-wizard/steps/step_1.json' - put '/w/super-mega-fun-wizard/steps/step_2.json' - put '/w/super-mega-fun-wizard/steps/step_3.json' - expect(response.status).to eq(200) - expect(response.parsed_body['redirect_on_complete']).to eq("https://google.com") - end - - it "saves results of completion actions if user has completed wizard" do - new_template = wizard_template.dup - new_template['actions'].first['run_after'] = 'wizard_completion' - CustomWizard::Template.save(new_template, skip_jobs: true) - - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Topic title", - step_1_field_2: "Topic post" - } - } - put '/w/super-mega-fun-wizard/steps/step_2.json' - put '/w/super-mega-fun-wizard/steps/step_3.json' - - wizard_id = response.parsed_body['wizard']['id'] - wizard = CustomWizard::Wizard.create(wizard_id, user) - - topic_id = wizard.submissions.first.fields[new_template['actions'].first['id']] - topic = Topic.find(topic_id) - expect(topic.present?).to eq(true) - end - - it "returns a final step without conditions" do - put '/w/super-mega-fun-wizard/steps/step_1.json' - expect(response.status).to eq(200) - expect(response.parsed_body['final']).to eq(false) - - put '/w/super-mega-fun-wizard/steps/step_2.json' - expect(response.status).to eq(200) - expect(response.parsed_body['final']).to eq(false) - - put '/w/super-mega-fun-wizard/steps/step_3.json' - expect(response.status).to eq(200) - expect(response.parsed_body['final']).to eq(true) - end - - context "subscription" do + context "with user" do before do - enable_subscription("standard") + sign_in(user) end - it "raises an error when user cant see the step due to conditions" do - sign_in(user2) - - new_wizard_template = wizard_template.dup - new_wizard_template['steps'][0]['condition'] = user_condition_template['condition'] - CustomWizard::Template.save(new_wizard_template, skip_jobs: true) - - put '/w/super-mega-fun-wizard/steps/step_1.json' - expect(response.status).to eq(403) - end - - it "returns an updated wizard when condition doesnt pass" do - new_template = wizard_template.dup - new_template['steps'][1]['condition'] = wizard_field_condition_template['condition'] - CustomWizard::Template.save(new_template, skip_jobs: true) - + it 'performs a step update' do put '/w/super-mega-fun-wizard/steps/step_1.json', params: { fields: { - step_1_field_1: "Condition wont pass" + step_1_field_1: "Text input" } } expect(response.status).to eq(200) - expect(response.parsed_body['wizard']['start']).to eq("step_3") + expect(response.parsed_body['wizard']['start']).to eq("step_2") + + wizard_id = response.parsed_body['wizard']['id'] + wizard = CustomWizard::Wizard.create(wizard_id, user) + expect(wizard.current_submission.fields['step_1_field_1']).to eq("Text input") end - it "returns the correct final step when the conditional final step and last step are the same" do - new_template = wizard_template.dup - new_template['steps'][0]['condition'] = user_condition_template['condition'] - new_template['steps'][2]['condition'] = wizard_field_condition_template['condition'] - CustomWizard::Template.save(new_template, skip_jobs: true) + context "raises an error" do + it "when the wizard doesnt exist" do + put '/w/not-super-mega-fun-wizard/steps/step_1.json' + expect(response.status).to eq(400) + end + + it "when the user cant access the wizard" do + enable_subscription("standard") + new_template = wizard_template.dup + new_template["permitted"] = permitted_json["permitted"] + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json' + expect(response.status).to eq(403) + end + + it "when the step doesnt exist" do + put '/w/super-mega-fun-wizard/steps/step_10.json' + expect(response.status).to eq(400) + end end - it "raises an error when user cant see the step due to conditions" do - sign_in(user2) - - new_wizard_template = wizard_template.dup - new_wizard_template['steps'][0]['condition'] = user_condition_template['condition'] - CustomWizard::Template.save(new_wizard_template, skip_jobs: true) - + it "works if the step has no fields" do put '/w/super-mega-fun-wizard/steps/step_1.json' - expect(response.status).to eq(403) + expect(response.status).to eq(200) + expect(response.parsed_body['wizard']['start']).to eq("step_2") end - it "returns an updated wizard when condition doesnt pass" do + it "returns an updated wizard when condition passes" do new_template = wizard_template.dup new_template['steps'][1]['condition'] = wizard_field_condition_template['condition'] CustomWizard::Template.save(new_template, skip_jobs: true) - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Condition wont pass" - } - } - expect(response.status).to eq(200) - expect(response.parsed_body['wizard']['start']).to eq("step_3") - end - - it "returns the correct final step when the conditional final step and last step are the same" do - new_template = wizard_template.dup - new_template['steps'][0]['condition'] = user_condition_template['condition'] - new_template['steps'][2]['condition'] = wizard_field_condition_template['condition'] - CustomWizard::Template.save(new_template, skip_jobs: true) - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { fields: { step_1_field_1: "Condition will pass" } } expect(response.status).to eq(200) + expect(response.parsed_body['wizard']['start']).to eq("step_2") + end + + it "runs completion actions if user has completed wizard" do + new_template = wizard_template.dup + + ## route_to action + new_template['actions'].last['run_after'] = 'wizard_completion' + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json' + put '/w/super-mega-fun-wizard/steps/step_2.json' + put '/w/super-mega-fun-wizard/steps/step_3.json' + expect(response.status).to eq(200) + expect(response.parsed_body['redirect_on_complete']).to eq("https://google.com") + end + + it "saves results of completion actions if user has completed wizard" do + new_template = wizard_template.dup + new_template['actions'].first['run_after'] = 'wizard_completion' + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Topic title", + step_1_field_2: "Topic post" + } + } + put '/w/super-mega-fun-wizard/steps/step_2.json' + put '/w/super-mega-fun-wizard/steps/step_3.json' + + wizard_id = response.parsed_body['wizard']['id'] + wizard = CustomWizard::Wizard.create(wizard_id, user) + + topic_id = wizard.submissions.first.fields[new_template['actions'].first['id']] + topic = Topic.find(topic_id) + expect(topic.present?).to eq(true) + end + + it "returns a final step without conditions" do + put '/w/super-mega-fun-wizard/steps/step_1.json' + expect(response.status).to eq(200) expect(response.parsed_body['final']).to eq(false) put '/w/super-mega-fun-wizard/steps/step_2.json' @@ -204,66 +123,152 @@ describe CustomWizard::StepsController do expect(response.parsed_body['final']).to eq(true) end - it "returns the correct final step when the conditional final step and last step are different" do - new_template = wizard_template.dup - new_template['steps'][2]['condition'] = wizard_field_condition_template['condition'] - CustomWizard::Template.save(new_template, skip_jobs: true) + context "subscription" do + before do + enable_subscription("standard") + end - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Condition will not pass" + it "raises an error when user cant see the step due to conditions" do + sign_in(user2) + + new_wizard_template = wizard_template.dup + new_wizard_template['steps'][0]['condition'] = user_condition_template['condition'] + CustomWizard::Template.save(new_wizard_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json' + expect(response.status).to eq(403) + end + + it "returns an updated wizard when condition doesnt pass" do + new_template = wizard_template.dup + new_template['steps'][1]['condition'] = wizard_field_condition_template['condition'] + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Condition wont pass" + } } - } - expect(response.status).to eq(200) - expect(response.parsed_body['final']).to eq(false) + expect(response.status).to eq(200) + expect(response.parsed_body['wizard']['start']).to eq("step_3") + end - put '/w/super-mega-fun-wizard/steps/step_2.json' - expect(response.status).to eq(200) - expect(response.parsed_body['final']).to eq(true) - end + it "returns the correct final step when the conditional final step and last step are the same" do + new_template = wizard_template.dup + new_template['steps'][0]['condition'] = user_condition_template['condition'] + new_template['steps'][2]['condition'] = wizard_field_condition_template['condition'] + CustomWizard::Template.save(new_template, skip_jobs: true) + end - it "returns the correct final step when the conditional final step is determined in the same action" do - new_template = wizard_template.dup - new_template['steps'][1]['condition'] = wizard_field_condition_template['condition'] - new_template['steps'][2]['condition'] = wizard_field_condition_template['condition'] - CustomWizard::Template.save(new_template, skip_jobs: true) + it "raises an error when user cant see the step due to conditions" do + sign_in(user2) - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Condition will not pass" + new_wizard_template = wizard_template.dup + new_wizard_template['steps'][0]['condition'] = user_condition_template['condition'] + CustomWizard::Template.save(new_wizard_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json' + expect(response.status).to eq(403) + end + + it "returns an updated wizard when condition doesnt pass" do + new_template = wizard_template.dup + new_template['steps'][1]['condition'] = wizard_field_condition_template['condition'] + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Condition wont pass" + } } - } - expect(response.status).to eq(200) - expect(response.parsed_body['final']).to eq(true) - end + expect(response.status).to eq(200) + expect(response.parsed_body['wizard']['start']).to eq("step_3") + end - it "excludes the non-included conditional fields from the submissions" do - new_template = wizard_template.dup - new_template['steps'][1]['fields'][0]['condition'] = wizard_field_condition_template['condition'] - CustomWizard::Template.save(new_template, skip_jobs: true) + it "returns the correct final step when the conditional final step and last step are the same" do + new_template = wizard_template.dup + new_template['steps'][0]['condition'] = user_condition_template['condition'] + new_template['steps'][2]['condition'] = wizard_field_condition_template['condition'] + CustomWizard::Template.save(new_template, skip_jobs: true) - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Condition will pass" + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Condition will pass" + } } - } + expect(response.status).to eq(200) + expect(response.parsed_body['final']).to eq(false) - put '/w/super-mega-fun-wizard/steps/step_2.json', params: { - fields: { - step_2_field_1: "1995-04-23" + put '/w/super-mega-fun-wizard/steps/step_2.json' + expect(response.status).to eq(200) + expect(response.parsed_body['final']).to eq(false) + + put '/w/super-mega-fun-wizard/steps/step_3.json' + expect(response.status).to eq(200) + expect(response.parsed_body['final']).to eq(true) + end + + it "returns the correct final step when the conditional final step and last step are different" do + new_template = wizard_template.dup + new_template['steps'][2]['condition'] = wizard_field_condition_template['condition'] + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Condition will not pass" + } } - } + expect(response.status).to eq(200) + expect(response.parsed_body['final']).to eq(false) - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Condition will not pass" + put '/w/super-mega-fun-wizard/steps/step_2.json' + expect(response.status).to eq(200) + expect(response.parsed_body['final']).to eq(true) + end + + it "returns the correct final step when the conditional final step is determined in the same action" do + new_template = wizard_template.dup + new_template['steps'][1]['condition'] = wizard_field_condition_template['condition'] + new_template['steps'][2]['condition'] = wizard_field_condition_template['condition'] + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Condition will not pass" + } } - } + expect(response.status).to eq(200) + expect(response.parsed_body['final']).to eq(true) + end - wizard_id = response.parsed_body['wizard']['id'] - wizard = CustomWizard::Wizard.create(wizard_id, user) - submission = wizard.current_submission - expect(submission.fields.keys).not_to include("step_2_field_1") + it "excludes the non-included conditional fields from the submissions" do + new_template = wizard_template.dup + new_template['steps'][1]['fields'][0]['condition'] = wizard_field_condition_template['condition'] + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Condition will pass" + } + } + + put '/w/super-mega-fun-wizard/steps/step_2.json', params: { + fields: { + step_2_field_1: "1995-04-23" + } + } + + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Condition will not pass" + } + } + + wizard_id = response.parsed_body['wizard']['id'] + wizard = CustomWizard::Wizard.create(wizard_id, user) + submission = wizard.current_submission + expect(submission.fields.keys).not_to include("step_2_field_1") + end end end end diff --git a/spec/requests/custom_wizard/wizard_controller_spec.rb b/spec/requests/custom_wizard/wizard_controller_spec.rb index aa1f479b..93ec196b 100644 --- a/spec/requests/custom_wizard/wizard_controller_spec.rb +++ b/spec/requests/custom_wizard/wizard_controller_spec.rb @@ -8,7 +8,6 @@ describe CustomWizard::WizardController do before do CustomWizard::Template.save(wizard_template, skip_jobs: true) @template = CustomWizard::Template.find("super_mega_fun_wizard") - sign_in(user) end context 'plugin disabled' do @@ -32,65 +31,70 @@ describe CustomWizard::WizardController do expect(response.parsed_body["error"]).to eq("We couldn't find a wizard at that address.") end - context 'when user skips the wizard' do - - it 'skips a wizard if user is allowed to skip' do - put '/w/super-mega-fun-wizard/skip.json' - expect(response.status).to eq(200) + context "with user" do + before do + sign_in(user) end - it 'lets user skip if user cant access wizard' do - enable_subscription("standard") - @template["permitted"] = permitted_json["permitted"] - CustomWizard::Template.save(@template, skip_jobs: true) - put '/w/super-mega-fun-wizard/skip.json' - expect(response.status).to eq(200) - end + context 'when user skips' do + it 'skips a wizard if user is allowed to skip' do + put '/w/super-mega-fun-wizard/skip.json' + expect(response.status).to eq(200) + end - it 'returns a no skip message if user is not allowed to skip' do - enable_subscription("standard") - @template['required'] = 'true' - CustomWizard::Template.save(@template) - put '/w/super-mega-fun-wizard/skip.json' - expect(response.parsed_body['error']).to eq("Wizard can't be skipped") - end + it 'lets user skip if user cant access wizard' do + enable_subscription("standard") + @template["permitted"] = permitted_json["permitted"] + CustomWizard::Template.save(@template, skip_jobs: true) + put '/w/super-mega-fun-wizard/skip.json' + expect(response.status).to eq(200) + end - it 'skip response contains a redirect_to if in users submissions' do - @wizard = CustomWizard::Wizard.create(@template["id"], user) - CustomWizard::Submission.new(@wizard, redirect_to: "/t/2").save - put '/w/super-mega-fun-wizard/skip.json' - expect(response.parsed_body['redirect_to']).to eq('/t/2') - end + it 'returns a no skip message if user is not allowed to skip' do + enable_subscription("standard") + @template['required'] = 'true' + CustomWizard::Template.save(@template) + put '/w/super-mega-fun-wizard/skip.json' + expect(response.parsed_body['error']).to eq("Wizard can't be skipped") + end - it 'deletes the users redirect_to_wizard if present' do - user.custom_fields['redirect_to_wizard'] = @template["id"] - user.save_custom_fields(true) - @wizard = CustomWizard::Wizard.create(@template["id"], user) - put '/w/super-mega-fun-wizard/skip.json' - expect(response.status).to eq(200) - expect(user.reload.redirect_to_wizard).to eq(nil) - end + it 'skip response contains a redirect_to if in users submissions' do + @wizard = CustomWizard::Wizard.create(@template["id"], user) + CustomWizard::Submission.new(@wizard, redirect_to: "/t/2").save + put '/w/super-mega-fun-wizard/skip.json' + expect(response.parsed_body['redirect_to']).to eq('/t/2') + end - it "deletes the submission if user has filled up some data" do - @wizard = CustomWizard::Wizard.create(@template["id"], user) - CustomWizard::Submission.new(@wizard, step_1_field_1: "Hello World").save - current_submission = @wizard.current_submission - put '/w/super-mega-fun-wizard/skip.json' - submissions = CustomWizard::Submission.list(@wizard).submissions + it 'deletes the users redirect_to_wizard if present' do + user.custom_fields['redirect_to_wizard'] = @template["id"] + user.save_custom_fields(true) + @wizard = CustomWizard::Wizard.create(@template["id"], user) + put '/w/super-mega-fun-wizard/skip.json' + expect(response.status).to eq(200) + expect(user.reload.redirect_to_wizard).to eq(nil) + end - expect(submissions.any? { |submission| submission.id == current_submission.id }).to eq(false) - end + it "deletes the submission if user has filled up some data" do + @wizard = CustomWizard::Wizard.create(@template["id"], user) + CustomWizard::Submission.new(@wizard, step_1_field_1: "Hello World").save + current_submission = @wizard.current_submission + put '/w/super-mega-fun-wizard/skip.json' + submissions = CustomWizard::Submission.list(@wizard).submissions - it "starts from the first step if user visits after skipping the wizard" do - put '/w/super-mega-fun-wizard/steps/step_1.json', params: { - fields: { - step_1_field_1: "Text input" + expect(submissions.any? { |submission| submission.id == current_submission.id }).to eq(false) + end + + it "starts from the first step if user visits after skipping the wizard" do + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Text input" + } } - } - put '/w/super-mega-fun-wizard/skip.json' - get '/w/super-mega-fun-wizard.json' + put '/w/super-mega-fun-wizard/skip.json' + get '/w/super-mega-fun-wizard.json' - expect(response.parsed_body["start"]).to eq('step_1') + expect(response.parsed_body["start"]).to eq('step_1') + end end end end From 7d2e8765842a59fe86a86d19d0039d75dbdb12f9 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Wed, 18 Jan 2023 19:53:36 +0100 Subject: [PATCH 02/16] First working version --- app/controllers/custom_wizard/steps.rb | 5 +- app/controllers/custom_wizard/wizard.rb | 19 +---- .../custom_wizard/wizard_client.rb | 23 ++++++ .../custom_wizard/submission_serializer.rb | 11 ++- .../discourse/lib/wizard-schema.js.es6 | 2 +- config/locales/server.en.yml | 3 +- lib/custom_wizard/action.rb | 11 +-- lib/custom_wizard/builder.rb | 12 +-- lib/custom_wizard/mapper.rb | 2 + lib/custom_wizard/step_updater.rb | 9 +-- lib/custom_wizard/submission.rb | 41 ++++------ lib/custom_wizard/subscription.rb | 1 - lib/custom_wizard/template.rb | 1 - lib/custom_wizard/user_history.rb | 54 +++++++++++++ lib/custom_wizard/validators/template.rb | 2 +- lib/custom_wizard/wizard.rb | 78 ++++++++++++------- plugin.rb | 2 + spec/components/custom_wizard/action_spec.rb | 26 ++++++- spec/components/custom_wizard/builder_spec.rb | 13 ++-- .../custom_wizard/submission_spec.rb | 24 ++++-- spec/components/custom_wizard/wizard_spec.rb | 20 ++--- .../custom_wizard/steps_controller_spec.rb | 34 ++++++++ 22 files changed, 267 insertions(+), 126 deletions(-) create mode 100644 app/controllers/custom_wizard/wizard_client.rb create mode 100644 lib/custom_wizard/user_history.rb diff --git a/app/controllers/custom_wizard/steps.rb b/app/controllers/custom_wizard/steps.rb index ea2a75b8..2a4305c7 100644 --- a/app/controllers/custom_wizard/steps.rb +++ b/app/controllers/custom_wizard/steps.rb @@ -1,5 +1,5 @@ # frozen_string_literal: true -class CustomWizard::StepsController < ::ApplicationController +class CustomWizard::StepsController < ::CustomWizard::WizardClientController before_action :ensure_can_update def update @@ -21,7 +21,7 @@ class CustomWizard::StepsController < ::ApplicationController if updater.success? wizard_id = update_params[:wizard_id] - builder = CustomWizard::Builder.new(wizard_id, current_user) + builder = CustomWizard::Builder.new(wizard_id, current_user, guest_id) @wizard = builder.build(force: true) current_step = @wizard.find_step(update[:step_id]) @@ -84,7 +84,6 @@ class CustomWizard::StepsController < ::ApplicationController private def ensure_can_update - @builder = CustomWizard::Builder.new(update_params[:wizard_id], current_user) raise Discourse::InvalidParameters.new(:wizard_id) if @builder.template.nil? raise Discourse::InvalidAccess.new if !@builder.wizard || !@builder.wizard.can_access? diff --git a/app/controllers/custom_wizard/wizard.rb b/app/controllers/custom_wizard/wizard.rb index 86265af4..dd4ea4ca 100644 --- a/app/controllers/custom_wizard/wizard.rb +++ b/app/controllers/custom_wizard/wizard.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -class CustomWizard::WizardController < ::ApplicationController - before_action :ensure_plugin_enabled - +class CustomWizard::WizardController < ::CustomWizard::WizardClientController def show if wizard.present? render json: CustomWizard::WizardSerializer.new(wizard, scope: guardian, root: false).as_json, status: 200 @@ -34,19 +32,8 @@ class CustomWizard::WizardController < ::ApplicationController def wizard @wizard ||= begin - builder = CustomWizard::Builder.new(params[:wizard_id].underscore, current_user) - return nil unless builder.present? - opts = {} - opts[:reset] = params[:reset] - builder.build(opts, params) - end - end - - private - - def ensure_plugin_enabled - unless SiteSetting.custom_wizard_enabled - redirect_to path("/") + return nil unless @builder.present? + @builder.build({ reset: params[:reset] }, params) end end end diff --git a/app/controllers/custom_wizard/wizard_client.rb b/app/controllers/custom_wizard/wizard_client.rb new file mode 100644 index 00000000..e898852a --- /dev/null +++ b/app/controllers/custom_wizard/wizard_client.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true +class CustomWizard::WizardClientController < ::ApplicationController + before_action :ensure_plugin_enabled + before_action :set_builder + + private + + def ensure_plugin_enabled + unless SiteSetting.custom_wizard_enabled + redirect_to path("/") + end + end + + def guest_id + return nil if current_user.present? + cookies[:custom_wizard_guest_id] ||= CustomWizard::Wizard.generate_guest_id + cookies[:custom_wizard_guest_id] + end + + def set_builder + @builder = CustomWizard::Builder.new(params[:wizard_id].underscore, current_user, guest_id) + end +end diff --git a/app/serializers/custom_wizard/submission_serializer.rb b/app/serializers/custom_wizard/submission_serializer.rb index 732d6743..48892c21 100644 --- a/app/serializers/custom_wizard/submission_serializer.rb +++ b/app/serializers/custom_wizard/submission_serializer.rb @@ -2,12 +2,15 @@ class CustomWizard::SubmissionSerializer < ApplicationSerializer attributes :id, :fields, - :submitted_at - - has_one :user, serializer: ::BasicUserSerializer, embed: :objects + :submitted_at, + :user def include_user? - object.user.present? + object.wizard.user.present? + end + + def user + ::BasicUserSerializer.new(object.wizard.user).as_json end def fields diff --git a/assets/javascripts/discourse/lib/wizard-schema.js.es6 b/assets/javascripts/discourse/lib/wizard-schema.js.es6 index a1756b5f..350d91a1 100644 --- a/assets/javascripts/discourse/lib/wizard-schema.js.es6 +++ b/assets/javascripts/discourse/lib/wizard-schema.js.es6 @@ -209,7 +209,7 @@ const action = { const filters = { allow_guests: { action: { - type: ['route_to'] + type: ['route_to', 'send_message'] } } } diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 08cf5336..af25fec2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -53,7 +53,8 @@ en: after_signup_after_time: "You can't use 'after time' and 'after signup' on the same wizard." after_time: "After time setting is invalid." liquid_syntax_error: "Liquid syntax error in %{attribute}: %{message}" - subscription: "%{type} %{property} is subscription only" + subscription: "%{type} %{property} is subscription only" + allow_guests: "%{object_id} is not permitted when allow_guests is enabled" site_settings: custom_wizard_enabled: "Enable custom wizards." diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index 00a200ce..24fe2576 100644 --- a/lib/custom_wizard/action.rb +++ b/lib/custom_wizard/action.rb @@ -10,7 +10,6 @@ class CustomWizard::Action create_topic update_profile open_composer - send_message watch_categories add_to_group ] @@ -91,7 +90,6 @@ class CustomWizard::Action end def send_message - if action['required'].present? required = CustomWizard::Mapper.new( inputs: action['required'], @@ -138,13 +136,14 @@ class CustomWizard::Action params[:archetype] = Archetype.private_message - creator = PostCreator.new(user, params) + poster = @wizard.allow_guests ? Discourse.system_user : user + creator = PostCreator.new(poster, params) post = creator.create if creator.errors.present? messages = creator.errors.full_messages.join(" ") log_error("failed to create message", messages) - elsif action['skip_redirect'].blank? + elsif user && action['skip_redirect'].blank? @submission.redirect_on_complete = post.topic.url end @@ -778,10 +777,12 @@ class CustomWizard::Action end def save_log + username = user ? user.username : @wizard.actor_id + CustomWizard::Log.create( @wizard.id, action['type'], - user.username, + username, @log.join('; ') ) end diff --git a/lib/custom_wizard/builder.rb b/lib/custom_wizard/builder.rb index c510861e..0d0b689d 100644 --- a/lib/custom_wizard/builder.rb +++ b/lib/custom_wizard/builder.rb @@ -2,10 +2,10 @@ class CustomWizard::Builder attr_accessor :wizard, :updater, :template - def initialize(wizard_id, user = nil) + def initialize(wizard_id, user = nil, guest_id = nil) @template = CustomWizard::Template.create(wizard_id) return nil if @template.nil? - @wizard = CustomWizard::Wizard.new(template.data, user) + @wizard = CustomWizard::Wizard.new(template.data, user, guest_id) end def self.sorted_handlers @@ -182,7 +182,7 @@ class CustomWizard::Builder if field_template['description'].present? params[:description] = mapper.interpolate( field_template['description'], - user: @wizard.user.present?, + user: @wizard.user, value: true, wizard: true, template: true @@ -192,7 +192,7 @@ class CustomWizard::Builder if field_template['preview_template'].present? preview_template = mapper.interpolate( field_template['preview_template'], - user: @wizard.user.present?, + user: @wizard.user, value: true, wizard: true, template: true @@ -204,7 +204,7 @@ class CustomWizard::Builder if field_template['placeholder'].present? params[:placeholder] = mapper.interpolate( field_template['placeholder'], - user: @wizard.user.present?, + user: @wizard.user, value: true, wizard: true, template: true @@ -248,7 +248,7 @@ class CustomWizard::Builder if step_template['description'] step.description = mapper.interpolate( step_template['description'], - user: @wizard.user.present?, + user: @wizard.user, value: true, wizard: true, template: true diff --git a/lib/custom_wizard/mapper.rb b/lib/custom_wizard/mapper.rb index e23decee..e4d0db50 100644 --- a/lib/custom_wizard/mapper.rb +++ b/lib/custom_wizard/mapper.rb @@ -203,6 +203,8 @@ class CustomWizard::Mapper end def map_user_field(value) + return nil unless user + if value.include?(User::USER_FIELD_PREFIX) user.custom_fields[value] elsif PROFILE_FIELDS.include?(value) diff --git a/lib/custom_wizard/step_updater.rb b/lib/custom_wizard/step_updater.rb index ab86f3fa..511001c2 100644 --- a/lib/custom_wizard/step_updater.rb +++ b/lib/custom_wizard/step_updater.rb @@ -5,8 +5,7 @@ class CustomWizard::StepUpdater attr_accessor :refresh_required, :result attr_reader :step, :submission - def initialize(current_user, wizard, step, submission) - @current_user = current_user + def initialize(wizard, step, submission) @wizard = wizard @step = step @refresh_required = false @@ -22,9 +21,9 @@ class CustomWizard::StepUpdater @step.updater.call(self) - UserHistory.create( - action: UserHistory.actions[:custom_wizard_step], - acting_user_id: @current_user.id, + CustomWizard::UserHistory.create( + action: CustomWizard::UserHistory.actions[:step], + actor_id: @wizard.actor_id, context: @wizard.id, subject: @step.id ) diff --git a/lib/custom_wizard/submission.rb b/lib/custom_wizard/submission.rb index a52172e3..fc10cf31 100644 --- a/lib/custom_wizard/submission.rb +++ b/lib/custom_wizard/submission.rb @@ -7,8 +7,6 @@ class CustomWizard::Submission META ||= %w(updated_at submitted_at route_to redirect_on_complete redirect_to) attr_reader :id, - :user, - :user_id, :wizard attr_accessor :fields, @@ -18,15 +16,8 @@ class CustomWizard::Submission class_eval { attr_accessor attr } end - def initialize(wizard, data = {}, user_id = nil) + def initialize(wizard, data = {}) @wizard = wizard - @user_id = user_id - - if user_id - @user = User.find_by(id: user_id) - else - @user = wizard.user - end data = (data || {}).with_indifferent_access @id = data['id'] || SecureRandom.hex(12) @@ -44,13 +35,13 @@ class CustomWizard::Submission return nil unless wizard.save_submissions validate - submission_list = self.class.list(wizard, user_id: user.id) + submission_list = self.class.list(wizard) submissions = submission_list.submissions.select { |submission| submission.id != self.id } self.updated_at = Time.now.iso8601 submissions.push(self) submission_data = submissions.map { |submission| data_to_save(submission) } - PluginStore.set("#{wizard.id}_#{KEY}", user.id, submission_data) + PluginStore.set("#{wizard.id}_#{KEY}", wizard.actor_id, submission_data) end def validate @@ -93,25 +84,21 @@ class CustomWizard::Submission data end - def self.get(wizard, user_id) - data = PluginStore.get("#{wizard.id}_#{KEY}", user_id).last - new(wizard, data, user_id) + def self.get(wizard) + data = PluginStore.get("#{wizard.id}_#{KEY}", wizard.actor_id).last + new(wizard, data) end def remove if present? - user_id = @user.id - wizard_id = @wizard.id - submission_id = @id - data = PluginStore.get("#{wizard_id}_#{KEY}", user_id) - data.delete_if { |sub| sub["id"] == submission_id } - PluginStore.set("#{wizard_id}_#{KEY}", user_id, data) + data = PluginStore.get("#{@wizard.id}_#{KEY}", wizard.actor_id) + data.delete_if { |sub| sub["id"] == @id } + PluginStore.set("#{@wizard.id}_#{KEY}", wizard.actor_id, data) end end def self.cleanup_incomplete_submissions(wizard) - user_id = wizard.user.id - all_submissions = list(wizard, user_id: user_id) + all_submissions = list(wizard) sorted_submissions = all_submissions.submissions.sort_by do |submission| zero_epoch_time = DateTime.strptime("0", '%s') [ @@ -129,12 +116,12 @@ class CustomWizard::Submission end valid_data = valid_submissions.map { |submission| submission.data_to_save(submission) } - PluginStore.set("#{wizard.id}_#{KEY}", user_id, valid_data) + PluginStore.set("#{wizard.id}_#{KEY}", wizard.actor_id, valid_data) end - def self.list(wizard, user_id: nil, order_by: nil, page: nil) + def self.list(wizard, order_by: nil, page: nil) params = { plugin_name: "#{wizard.id}_#{KEY}" } - params[:key] = user_id if user_id.present? + params[:key] = wizard.actor_id if wizard.actor_id query = PluginStoreRow.where(params) result = OpenStruct.new(submissions: [], total: nil) @@ -142,7 +129,7 @@ class CustomWizard::Submission query.each do |record| if (submission_data = ::JSON.parse(record.value)).any? submission_data.each do |data| - result.submissions.push(new(wizard, data, record.key)) + result.submissions.push(new(wizard, data)) end end end diff --git a/lib/custom_wizard/subscription.rb b/lib/custom_wizard/subscription.rb index 548ae67d..700e6087 100644 --- a/lib/custom_wizard/subscription.rb +++ b/lib/custom_wizard/subscription.rb @@ -137,7 +137,6 @@ class CustomWizard::Subscription end def business? - return true @subscription.product_id === BUSINESS_PRODUCT_ID end diff --git a/lib/custom_wizard/template.rb b/lib/custom_wizard/template.rb index 4163a1f7..12a86bf6 100644 --- a/lib/custom_wizard/template.rb +++ b/lib/custom_wizard/template.rb @@ -23,7 +23,6 @@ class CustomWizard::Template normalize_data validate_data prepare_data - return false if errors.any? ActiveRecord::Base.transaction do diff --git a/lib/custom_wizard/user_history.rb b/lib/custom_wizard/user_history.rb new file mode 100644 index 00000000..1d5ee3e1 --- /dev/null +++ b/lib/custom_wizard/user_history.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true +UserHistory.actions[:custom_wizard_step] = 1000 + +class CustomWizard::UserHistory + def self.where(actor_id: nil, action: nil, context: nil, subject: nil) + ::UserHistory.where(where_opts(actor_id, action, context, subject)) + end + + def self.create(actor_id: nil, action: nil, context: nil, subject: nil) + ::UserHistory.create(create_opts(actor_id, action, context, subject)) + end + + def self.create!(actor_id: nil, action: nil, context: nil, subject: nil) + ::UserHistory.create!(create_opts(actor_id, action, context, subject)) + end + + def self.actions + @actions ||= + Enum.new( + step: UserHistory.actions[:custom_wizard_step] + ) + end + + def self.where_opts(actor_id, action, context, subject) + opts = { + context: context + } + opts[:action] = action if action + opts[:subject] = subject if subject + add_actor(opts, actor_id) + end + + def self.create_opts(actor_id, action, context, subject) + opts = { + action: action, + context: context + } + opts[:subject] = subject if subject + add_actor(opts, actor_id) + end + + def self.add_actor(opts, actor_id) + acting_user_id = actor_id + + if actor_id.is_a?(String) && actor_id.include?(CustomWizard::Wizard::GUEST_ID_PREFIX) + opts[:acting_user_id] = Discourse.system_user.id + opts[:details] = actor_id + else + opts[:acting_user_id] = actor_id + end + + opts + end +end diff --git a/lib/custom_wizard/validators/template.rb b/lib/custom_wizard/validators/template.rb index 59626a69..c2f6f8b4 100644 --- a/lib/custom_wizard/validators/template.rb +++ b/lib/custom_wizard/validators/template.rb @@ -83,7 +83,7 @@ class CustomWizard::TemplateValidator def validate_action(action) if @data[:allow_guests] && CustomWizard::Action::REQUIRES_USER.include?(action[:type]) - errors.add :base, I18n.t("wizard.validation.conflict", wizard_id: action[:id]) + errors.add :base, I18n.t("wizard.validation.allow_guests", object_id: action[:id]) end end diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index 1293cfe3..35aed456 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -4,8 +4,6 @@ require_dependency 'wizard/field' require_dependency 'wizard/step_updater' require_dependency 'wizard/builder' -UserHistory.actions[:custom_wizard_step] = 1000 - class CustomWizard::Wizard include ActiveModel::SerializerSupport @@ -32,13 +30,21 @@ class CustomWizard::Wizard :actions, :action_ids, :user, + :guest_id, :submissions, :template attr_reader :all_step_ids - def initialize(attrs = {}, user = nil) - @user = user + GUEST_ID_PREFIX ||= "guest" + + def initialize(attrs = {}, user = nil, guest_id = nil) + if user + @user = user + elsif guest_id + @guest_id = guest_id + end + attrs = attrs.with_indifferent_access @id = attrs['id'] @@ -83,6 +89,10 @@ class CustomWizard::Wizard @template = attrs end + def actor_id + user ? user.id : guest_id + end + def cast_bool(val) val.nil? ? false : ActiveRecord::Type::Boolean.new.cast(val) end @@ -143,17 +153,16 @@ class CustomWizard::Wizard end def last_completed_step_id - if user && unfinished? && last_completed_step = ::UserHistory.where( - acting_user_id: user.id, - action: ::UserHistory.actions[:custom_wizard_step], - context: id, - subject: all_step_ids - ).order("created_at").last + return nil unless actor_id && unfinished? - last_completed_step.subject - else - nil - end + last_completed_step = CustomWizard::UserHistory.where( + actor_id: actor_id, + action: CustomWizard::UserHistory.actions[:step], + context: id, + subject: all_step_ids + ).order("created_at").last + + last_completed_step&.subject end def find_step(step_id) @@ -163,15 +172,15 @@ class CustomWizard::Wizard def create_updater(step_id, submission) step = @steps.find { |s| s.id == step_id } wizard = self - CustomWizard::StepUpdater.new(user, wizard, step, submission) + CustomWizard::StepUpdater.new(wizard, step, submission) end def unfinished? - return nil if !user + return nil unless actor_id - most_recent = ::UserHistory.where( - acting_user_id: user.id, - action: ::UserHistory.actions[:custom_wizard_step], + most_recent = CustomWizard::UserHistory.where( + actor_id: actor_id, + action: CustomWizard::UserHistory.actions[:step], context: id, ).distinct.order('updated_at DESC').first @@ -185,11 +194,11 @@ class CustomWizard::Wizard end def completed? - return nil if !user + return nil unless actor_id - history = ::UserHistory.where( - acting_user_id: user.id, - action: ::UserHistory.actions[:custom_wizard_step], + history = CustomWizard::UserHistory.where( + actor_id: actor_id, + action: CustomWizard::UserHistory.actions[:step], context: id ) @@ -202,6 +211,7 @@ class CustomWizard::Wizard end def permitted? + return nil unless actor_id return true if allow_guests return false unless user return true if user.admin? || permitted.blank? @@ -230,6 +240,7 @@ class CustomWizard::Wizard end def can_access? + return nil unless actor_id return true if allow_guests return false unless user return true if user.admin @@ -237,9 +248,11 @@ class CustomWizard::Wizard end def reset - ::UserHistory.create( - action: ::UserHistory.actions[:custom_wizard_step], - acting_user_id: user.id, + return nil unless actor_id + + CustomWizard::UserHistory.create( + action: CustomWizard::UserHistory.actions[:step], + actor_id: actor_id, context: id, subject: "reset" ) @@ -267,8 +280,7 @@ class CustomWizard::Wizard end def submissions - return nil unless user.present? - @submissions ||= CustomWizard::Submission.list(self, user_id: user.id).submissions + @submissions ||= CustomWizard::Submission.list(self).submissions end def current_submission @@ -304,15 +316,17 @@ class CustomWizard::Wizard end def remove_user_redirect + return unless user.present? + if id == user.redirect_to_wizard user.custom_fields.delete('redirect_to_wizard') user.save_custom_fields(true) end end - def self.create(wizard_id, user = nil) + def self.create(wizard_id, user = nil, guest_id = nil) if template = CustomWizard::Template.find(wizard_id) - new(template.to_h, user) + new(template.to_h, user, guest_id) else false end @@ -384,4 +398,8 @@ class CustomWizard::Wizard false end end + + def self.generate_guest_id + "#{self::GUEST_ID_PREFIX}_#{SecureRandom.hex(12)}" + end end diff --git a/plugin.rb b/plugin.rb index 73701d84..54022edf 100644 --- a/plugin.rb +++ b/plugin.rb @@ -41,6 +41,7 @@ after_initialize do ../app/controllers/custom_wizard/admin/logs.rb ../app/controllers/custom_wizard/admin/manager.rb ../app/controllers/custom_wizard/admin/custom_fields.rb + ../app/controllers/custom_wizard/wizard_client.rb ../app/controllers/custom_wizard/wizard.rb ../app/controllers/custom_wizard/steps.rb ../app/controllers/custom_wizard/realtime_validations.rb @@ -65,6 +66,7 @@ after_initialize do ../lib/custom_wizard/subscription.rb ../lib/custom_wizard/template.rb ../lib/custom_wizard/wizard.rb + ../lib/custom_wizard/user_history.rb ../lib/custom_wizard/api/api.rb ../lib/custom_wizard/api/authorization.rb ../lib/custom_wizard/api/endpoint.rb diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index e5dedfa9..624844c3 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/spec/components/custom_wizard/action_spec.rb @@ -76,8 +76,8 @@ describe CustomWizard::Action do updater.update expect(updater.success?).to eq(true) - expect(UserHistory.where( - acting_user_id: user.id, + expect(CustomWizard::UserHistory.where( + actor_id: user.id, context: "super_mega_fun_wizard", subject: "step_3" ).exists?).to eq(true) @@ -285,6 +285,28 @@ describe CustomWizard::Action do expect(topic.first.allowed_groups.map(&:name)).to include('cool_group', 'cool_group_1') expect(post.exists?).to eq(true) end + + it "send_message works with allow_guests enabled" do + wizard_template["allow_guests"] = true + wizard_template.delete("actions") + wizard_template['actions'] = [send_message] + update_template(wizard_template) + + User.create(username: 'angus1', email: "angus1@email.com") + + wizard = CustomWizard::Builder.new(wizard_template["id"], nil, CustomWizard::Wizard.generate_guest_id).build + wizard.create_updater(wizard.steps[0].id, {}).update + updater = wizard.create_updater(wizard.steps[1].id, {}) + updater.update + + topic = Topic.where(archetype: Archetype.private_message, title: "Message title") + post = Post.where(topic_id: topic.pluck(:id)) + + expect(topic.exists?).to eq(true) + expect(topic.first.topic_allowed_users.first.user.username).to eq('angus1') + expect(topic.first.topic_allowed_users.second.user.username).to eq(Discourse.system_user.username) + expect(post.exists?).to eq(true) + end end context "business subscription actions" do diff --git a/spec/components/custom_wizard/builder_spec.rb b/spec/components/custom_wizard/builder_spec.rb index ebcc355b..1e55b203 100644 --- a/spec/components/custom_wizard/builder_spec.rb +++ b/spec/components/custom_wizard/builder_spec.rb @@ -80,14 +80,11 @@ describe CustomWizard::Builder do it 'returns no steps if user has completed it' do @template[:steps].each do |step| - UserHistory.create!( - { - action: UserHistory.actions[:custom_wizard_step], - acting_user_id: user.id, - context: @template[:id] - }.merge( - subject: step[:id] - ) + CustomWizard::UserHistory.create!( + action: CustomWizard::UserHistory.actions[:step], + actor_id: user.id, + context: @template[:id], + subject: step[:id] ) end diff --git a/spec/components/custom_wizard/submission_spec.rb b/spec/components/custom_wizard/submission_spec.rb index ff9df88a..d0e0c986 100644 --- a/spec/components/custom_wizard/submission_spec.rb +++ b/spec/components/custom_wizard/submission_spec.rb @@ -4,6 +4,7 @@ describe CustomWizard::Submission do fab!(:user) { Fabricate(:user) } fab!(:user2) { Fabricate(:user) } let(:template_json) { get_wizard_fixture("wizard") } + let(:guest_id) { CustomWizard::Wizard.generate_guest_id } before do CustomWizard::Template.save(template_json, skip_jobs: true) @@ -13,10 +14,20 @@ describe CustomWizard::Submission do it "saves a user's submission" do expect( - described_class.get(@wizard, user.id).fields["step_1_field_1"] + described_class.get(@wizard).fields["step_1_field_1"] ).to eq("I am user submission") end + it "saves a guest's submission" do + CustomWizard::Template.save(template_json, skip_jobs: true) + @wizard = CustomWizard::Wizard.create(template_json["id"], nil, guest_id) + described_class.new(@wizard, step_1_field_1: "I am guest submission").save + + expect( + described_class.get(@wizard).fields["step_1_field_1"] + ).to eq("I am guest submission") + end + describe "#list" do before do freeze_time Time.now @@ -37,14 +48,17 @@ describe CustomWizard::Submission do end it "list submissions by wizard" do + @wizard.user = nil expect(described_class.list(@wizard).total).to eq(@count + 2) end it "list submissions by wizard and user" do - expect(described_class.list(@wizard, user_id: user.id).total).to eq(@count + 1) + @wizard.user = user + expect(described_class.list(@wizard).total).to eq(@count + 1) end it "paginates submission lists" do + @wizard.user = nil expect(described_class.list(@wizard, page: 1).submissions.size).to eq((@count + 2) - CustomWizard::Submission::PAGE_LIMIT) end @@ -59,7 +73,7 @@ describe CustomWizard::Submission do described_class.new(@wizard, step_1_field_1: "I am the second submission").save builder = CustomWizard::Builder.new(@wizard.id, @wizard.user) builder.build - submissions = described_class.list(@wizard, user_id: @wizard.user.id).submissions + submissions = described_class.list(@wizard).submissions expect(submissions.length).to eq(1) expect(submissions.first.fields["step_1_field_1"]).to eq("I am the second submission") @@ -75,7 +89,7 @@ describe CustomWizard::Submission do PluginStore.set("#{@wizard.id}_submissions", @wizard.user.id, sub_data) builder = CustomWizard::Builder.new(@wizard.id, @wizard.user) builder.build - submissions = described_class.list(@wizard, user_id: @wizard.user.id).submissions + submissions = described_class.list(@wizard).submissions expect(submissions.length).to eq(1) expect(submissions.first.fields["step_1_field_1"]).to eq("I am the second submission") @@ -92,7 +106,7 @@ describe CustomWizard::Submission do builder = CustomWizard::Builder.new(@wizard.id, @wizard.user) builder.build - submissions = described_class.list(@wizard, user_id: @wizard.user.id).submissions + submissions = described_class.list(@wizard).submissions expect(submissions.length).to eq(1) expect(submissions.first.fields["step_1_field_1"]).to eq("I am the third submission") diff --git a/spec/components/custom_wizard/wizard_spec.rb b/spec/components/custom_wizard/wizard_spec.rb index 8268849c..ed6ebbea 100644 --- a/spec/components/custom_wizard/wizard_spec.rb +++ b/spec/components/custom_wizard/wizard_spec.rb @@ -21,10 +21,10 @@ describe CustomWizard::Wizard do @wizard.update! end - def progress_step(step_id, acting_user: user, wizard: @wizard) - UserHistory.create( - action: UserHistory.actions[:custom_wizard_step], - acting_user_id: acting_user.id, + def progress_step(step_id, actor_id: user.id, wizard: @wizard) + CustomWizard::UserHistory.create( + action: CustomWizard::UserHistory.actions[:step], + actor_id: actor_id, context: wizard.id, subject: step_id ) @@ -158,9 +158,9 @@ describe CustomWizard::Wizard do it "lets a permitted user access a complete wizard with multiple submissions" do append_steps - progress_step("step_1", acting_user: trusted_user) - progress_step("step_2", acting_user: trusted_user) - progress_step("step_3", acting_user: trusted_user) + progress_step("step_1", actor_id: trusted_user.id) + progress_step("step_2", actor_id: trusted_user.id) + progress_step("step_3", actor_id: trusted_user.id) @permitted_template["multiple_submissions"] = true @@ -172,9 +172,9 @@ describe CustomWizard::Wizard do it "does not let an unpermitted user access a complete wizard without multiple submissions" do append_steps - progress_step("step_1", acting_user: trusted_user) - progress_step("step_2", acting_user: trusted_user) - progress_step("step_3", acting_user: trusted_user) + progress_step("step_1", actor_id: trusted_user.id) + progress_step("step_2", actor_id: trusted_user.id) + progress_step("step_3", actor_id: trusted_user.id) @permitted_template['multiple_submissions'] = false diff --git a/spec/requests/custom_wizard/steps_controller_spec.rb b/spec/requests/custom_wizard/steps_controller_spec.rb index cec02bc4..68d9f3f9 100644 --- a/spec/requests/custom_wizard/steps_controller_spec.rb +++ b/spec/requests/custom_wizard/steps_controller_spec.rb @@ -12,6 +12,40 @@ describe CustomWizard::StepsController do CustomWizard::Template.save(wizard_template, skip_jobs: true) end + context "with guest" do + it "does not perform a step update" do + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Text input" + } + } + expect(response.status).to eq(403) + end + + context "with allow_guests enabled" do + before do + new_template = wizard_template.dup + new_template["allow_guests"] = true + new_template.delete("actions") + result = CustomWizard::Template.save(new_template, skip_jobs: true) + end + + it "performs a step update" do + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Text input" + } + } + expect(response.status).to eq(200) + expect(response.parsed_body['wizard']['start']).to eq("step_2") + + wizard_id = response.parsed_body['wizard']['id'] + wizard = CustomWizard::Wizard.create(wizard_id, nil, cookies[:custom_wizard_guest_id]) + expect(wizard.current_submission.fields['step_1_field_1']).to eq("Text input") + end + end + end + context "with user" do before do sign_in(user) From dfc1540d52225fd23d6911486d556c5bde14fe90 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 26 Jan 2023 11:26:24 +0100 Subject: [PATCH 03/16] Fix tests and linting --- .../wizard-subscription-selector.js.es6 | 3 +-- .../discourse/lib/wizard-schema.js.es6 | 11 +++++------ .../discourse/routes/custom-wizard-index.js.es6 | 1 + .../discourse/routes/custom-wizard-step.js.es6 | 7 ++++++- lib/custom_wizard/subscription.rb | 6 ++++++ .../custom_wizard/steps_controller_spec.rb | 1 + test/javascripts/acceptance/wizard-test.js | 17 +++++++++++++++++ test/javascripts/helpers/wizard.js | 3 +++ 8 files changed, 40 insertions(+), 9 deletions(-) diff --git a/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 b/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 index 58c6715d..bb29653b 100644 --- a/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 +++ b/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 @@ -46,10 +46,9 @@ export default SingleSelectComponent.extend(Subscription, { if (allowGuests) { const filteredFeature = wizardSchema.filters.allow_guests[feature]; if (filteredFeature) { - const filteredAttribute = filteredFeature[attribute]; if (filteredAttribute) { - attributes = attributes.filter(a => filteredAttribute.includes(a)) + attributes = attributes.filter((a) => filteredAttribute.includes(a)); } } } diff --git a/assets/javascripts/discourse/lib/wizard-schema.js.es6 b/assets/javascripts/discourse/lib/wizard-schema.js.es6 index 350d91a1..d6d9d49d 100644 --- a/assets/javascripts/discourse/lib/wizard-schema.js.es6 +++ b/assets/javascripts/discourse/lib/wizard-schema.js.es6 @@ -18,7 +18,6 @@ const wizard = { allow_guests: null, theme_id: null, permitted: null, - allow_guests: null }, mapped: ["permitted"], required: ["id"], @@ -209,10 +208,10 @@ const action = { const filters = { allow_guests: { action: { - type: ['route_to', 'send_message'] - } - } -} + type: ["route_to", "send_message"], + }, + }, +}; const custom_field = { klass: ["topic", "post", "group", "category"], @@ -228,7 +227,7 @@ const wizardSchema = { field, custom_field, action, - filters + filters, }; export function buildFieldTypes(types) { diff --git a/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 b/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 index d6917650..f7860ef8 100644 --- a/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 +++ b/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 @@ -6,6 +6,7 @@ export default Route.extend({ const wizard = getCachedWizard(); if ( wizard && + (wizard.user || wizard.allow_guests) && wizard.permitted && !wizard.completed && wizard.start diff --git a/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 b/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 index dd7b8be8..3193d783 100644 --- a/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 +++ b/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 @@ -7,7 +7,12 @@ export default Route.extend({ const wizard = getCachedWizard(); this.set("wizard", wizard); - if (!wizard || !wizard.permitted || wizard.completed) { + if ( + !wizard || + (!wizard.user && !wizard.allow_guests) || + !wizard.permitted || + wizard.completed + ) { this.replaceWith("customWizard"); } }, diff --git a/lib/custom_wizard/subscription.rb b/lib/custom_wizard/subscription.rb index 700e6087..20a444eb 100644 --- a/lib/custom_wizard/subscription.rb +++ b/lib/custom_wizard/subscription.rb @@ -24,6 +24,12 @@ class CustomWizard::Subscription standard: ['*'], business: ['*'], community: ['*'] + }, + allow_guests: { + none: [], + standard: ['*'], + business: ['*'], + community: ['*'] } }, step: { diff --git a/spec/requests/custom_wizard/steps_controller_spec.rb b/spec/requests/custom_wizard/steps_controller_spec.rb index 68d9f3f9..9f77c100 100644 --- a/spec/requests/custom_wizard/steps_controller_spec.rb +++ b/spec/requests/custom_wizard/steps_controller_spec.rb @@ -24,6 +24,7 @@ describe CustomWizard::StepsController do context "with allow_guests enabled" do before do + enable_subscription("standard") new_template = wizard_template.dup new_template["allow_guests"] = true new_template.delete("actions") diff --git a/test/javascripts/acceptance/wizard-test.js b/test/javascripts/acceptance/wizard-test.js index 063c80fb..d7078166 100644 --- a/test/javascripts/acceptance/wizard-test.js +++ b/test/javascripts/acceptance/wizard-test.js @@ -9,6 +9,7 @@ import { import { wizard, wizardCompleted, + wizardGuest, wizardNoUser, wizardNotPermitted, } from "../helpers/wizard"; @@ -106,3 +107,19 @@ acceptance("Wizard | Wizard", function (needs) { assert.strictEqual($("body.custom-wizard").length, 0); }); }); + +acceptance("Wizard | Guest access", function (needs) { + needs.pretender((server, helper) => { + server.get("/w/wizard.json", () => helper.response(wizardGuest)); + }); + + test("Does not require login", async function (assert) { + await visit("/w/wizard"); + assert.ok(!exists(".wizard-no-access.requires-login")); + }); + + test("Starts", async function (assert) { + await visit("/w/wizard"); + assert.ok(query(".wizard-column"), true); + }); +}); diff --git a/test/javascripts/helpers/wizard.js b/test/javascripts/helpers/wizard.js index 700cedc7..25ccccbb 100644 --- a/test/javascripts/helpers/wizard.js +++ b/test/javascripts/helpers/wizard.js @@ -6,6 +6,8 @@ import updateJson from "../fixtures/update"; import { cloneJSON } from "discourse-common/lib/object"; const wizardNoUser = cloneJSON(wizardJson); +const wizardGuest = cloneJSON(wizardJson); +wizardGuest.allow_guests = true; const wizard = cloneJSON(wizardJson); wizard.user = cloneJSON(userJson); @@ -40,6 +42,7 @@ export { wizardNoUser, wizardNotPermitted, wizardCompleted, + wizardGuest, stepNotPermitted, allFieldsWizard, wizard, From 1b3551b13d1e94fca8c0fbec798242ee977a785d Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 26 Jan 2023 11:27:16 +0100 Subject: [PATCH 04/16] Bump version --- plugin.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugin.rb b/plugin.rb index 54022edf..ccb08d6f 100644 --- a/plugin.rb +++ b/plugin.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # name: discourse-custom-wizard # about: Forms for Discourse. Better onboarding, structured posting, data enrichment, automated actions and much more. -# version: 2.1.3 +# version: 2.2.0 # authors: Angus McLeod, Faizaan Gagan, Robert Barrow, Keegan George, Kaitlin Maddever # url: https://github.com/paviliondev/discourse-custom-wizard # contact_emails: development@pavilion.tech From b712b268966aec2c703b75285a0e833645c8ef3c Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 26 Jan 2023 11:27:40 +0100 Subject: [PATCH 05/16] Update COPYRIGHT.txt --- COPYRIGHT.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/COPYRIGHT.txt b/COPYRIGHT.txt index 48cea364..66b401ac 100644 --- a/COPYRIGHT.txt +++ b/COPYRIGHT.txt @@ -1,4 +1,4 @@ -All code in this repository is Copyright 2018 by Angus McLeod. +All code in this repository is Copyright 2023 by Angus McLeod. This program is free software; you can redistribute it and/or modify it under the terms of the GNU General Public License as published by From 82d2eee414c268a359285674f8f068aaab2ade9e Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 26 Jan 2023 11:31:04 +0100 Subject: [PATCH 06/16] Fix version --- plugin.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/plugin.rb b/plugin.rb index 0613e88e..ccb08d6f 100644 --- a/plugin.rb +++ b/plugin.rb @@ -2,7 +2,6 @@ # name: discourse-custom-wizard # about: Forms for Discourse. Better onboarding, structured posting, data enrichment, automated actions and much more. # version: 2.2.0 -# version: 2.1.4 # authors: Angus McLeod, Faizaan Gagan, Robert Barrow, Keegan George, Kaitlin Maddever # url: https://github.com/paviliondev/discourse-custom-wizard # contact_emails: development@pavilion.tech From 2c84f019bb9a85caef02a028622e31c555ba75c3 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 2 Feb 2023 14:30:54 +0100 Subject: [PATCH 07/16] Update subscription.rb --- lib/custom_wizard/subscription.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/custom_wizard/subscription.rb b/lib/custom_wizard/subscription.rb index 593dc391..6049d1a8 100644 --- a/lib/custom_wizard/subscription.rb +++ b/lib/custom_wizard/subscription.rb @@ -29,7 +29,7 @@ class CustomWizard::Subscription none: [], standard: ['*'], business: ['*'], - community: ['*'] + community: [] } }, step: { From 8f8c6d50c6fa23dfa154a726cf8e0bb07af75603 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 7 Feb 2023 12:46:17 +0100 Subject: [PATCH 08/16] move guest toggle to permitted attribute --- app/controllers/custom_wizard/admin/wizard.rb | 1 - .../custom_wizard/wizard_serializer.rb | 3 +- .../components/wizard-mapper-selector.js.es6 | 24 +++++- .../discourse/components/wizard-mapper.js.es6 | 1 + .../wizard-subscription-selector.js.es6 | 2 +- .../discourse/lib/wizard-schema.js.es6 | 1 - .../models/custom-wizard-admin.js.es6 | 8 ++ .../routes/custom-wizard-index.js.es6 | 10 +-- .../routes/custom-wizard-step.js.es6 | 7 +- .../templates/admin-wizards-wizard-show.hbs | 11 +-- config/locales/client.en.yml | 4 +- config/locales/server.en.yml | 2 +- lib/custom_wizard/action.rb | 2 +- lib/custom_wizard/subscription.rb | 6 -- lib/custom_wizard/validators/template.rb | 8 +- lib/custom_wizard/wizard.rb | 28 ++++--- spec/components/custom_wizard/action_spec.rb | 5 +- spec/fixtures/actions/route_to.json | 12 +++ spec/fixtures/wizard/guests_permitted.json | 12 +++ .../custom_wizard/steps_controller_spec.rb | 73 +++++++++++++++++-- test/javascripts/acceptance/wizard-test.js | 40 ++++++++++ test/javascripts/fixtures/wizard.js.es6 | 2 +- test/javascripts/helpers/wizard.js | 3 +- 23 files changed, 199 insertions(+), 66 deletions(-) create mode 100644 spec/fixtures/actions/route_to.json create mode 100644 spec/fixtures/wizard/guests_permitted.json diff --git a/app/controllers/custom_wizard/admin/wizard.rb b/app/controllers/custom_wizard/admin/wizard.rb index 4cad3f42..08e7b6d0 100644 --- a/app/controllers/custom_wizard/admin/wizard.rb +++ b/app/controllers/custom_wizard/admin/wizard.rb @@ -80,7 +80,6 @@ class CustomWizard::AdminWizardController < CustomWizard::AdminController :prompt_completion, :restart_on_revisit, :resume_on_revisit, - :allow_guests, :theme_id, permitted: mapped_params, steps: [ diff --git a/app/serializers/custom_wizard/wizard_serializer.rb b/app/serializers/custom_wizard/wizard_serializer.rb index 8b3caba1..9741d7af 100644 --- a/app/serializers/custom_wizard/wizard_serializer.rb +++ b/app/serializers/custom_wizard/wizard_serializer.rb @@ -9,8 +9,7 @@ class CustomWizard::WizardSerializer < CustomWizard::BasicWizardSerializer :completed, :required, :permitted, - :resume_on_revisit, - :allow_guests + :resume_on_revisit has_many :steps, serializer: ::CustomWizard::StepSerializer, embed: :objects has_one :user, serializer: ::BasicUserSerializer, embed: :objects diff --git a/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 b/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 index a257ed12..811564a5 100644 --- a/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 +++ b/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 @@ -116,6 +116,9 @@ export default Component.extend({ groupEnabled: computed("options.groupSelection", "inputType", function () { return this.optionEnabled("groupSelection"); }), + guestGroup: computed("options.guestGroup", "inputType", function () { + return this.optionEnabled("guestGroup"); + }), userEnabled: computed("options.userSelection", "inputType", function () { return this.optionEnabled("userSelection"); }), @@ -126,7 +129,26 @@ export default Component.extend({ return this.connector === "is"; }), - groups: alias("site.groups"), + @discourseComputed("site.groups", "guestGroup") + groups(groups, guestGroup) { + let result = groups; + if (!guestGroup) { + return result; + } + + let guestIndex; + result.forEach((r, index) => { + if (r.id === 0) { + r.name = I18n.t("admin.wizard.selector.label.users"); + guestIndex = index; + } + }); + result.splice(guestIndex, 0, { + id: -1, + name: I18n.t("admin.wizard.selector.label.guests"), + }); + return result; + }, categories: alias("site.categories"), showComboBox: or( "showWizardField", diff --git a/assets/javascripts/discourse/components/wizard-mapper.js.es6 b/assets/javascripts/discourse/components/wizard-mapper.js.es6 index 95aabb1c..ec58e3f2 100644 --- a/assets/javascripts/discourse/components/wizard-mapper.js.es6 +++ b/assets/javascripts/discourse/components/wizard-mapper.js.es6 @@ -32,6 +32,7 @@ export default Component.extend({ pairConnector: options.pairConnector || null, outputConnector: options.outputConnector || null, context: options.context || null, + guestGroup: options.guestGroup || false, }; let inputTypes = ["key", "value", "output"]; diff --git a/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 b/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 index bb29653b..dce6f781 100644 --- a/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 +++ b/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 @@ -56,7 +56,7 @@ export default SingleSelectComponent.extend(Subscription, { return attributes; }, - @discourseComputed("feature", "attribute", "wizard.allow_guests") + @discourseComputed("feature", "attribute", "wizard.allowGuests") content(feature, attribute, allowGuests) { return this.contentList(feature, attribute, allowGuests) .map((value) => { diff --git a/assets/javascripts/discourse/lib/wizard-schema.js.es6 b/assets/javascripts/discourse/lib/wizard-schema.js.es6 index d2b78b7c..d3237de1 100644 --- a/assets/javascripts/discourse/lib/wizard-schema.js.es6 +++ b/assets/javascripts/discourse/lib/wizard-schema.js.es6 @@ -15,7 +15,6 @@ const wizard = { prompt_completion: null, restart_on_revisit: null, resume_on_revisit: null, - allow_guests: null, theme_id: null, permitted: null, }, diff --git a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 index 65c7aa7f..03afd32e 100644 --- a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 +++ b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 @@ -5,8 +5,16 @@ import wizardSchema from "../lib/wizard-schema"; import { Promise } from "rsvp"; import { ajax } from "discourse/lib/ajax"; import { popupAjaxError } from "discourse/lib/ajax-error"; +import discourseComputed from "discourse-common/utils/decorators"; + +const GUEST_GROUP_ID = -1; const CustomWizardAdmin = EmberObject.extend({ + @discourseComputed("permitted.@each") + allowGuests(permitted) { + return permitted.filter((p) => p.output === GUEST_GROUP_ID).length; + }, + save(opts) { return new Promise((resolve, reject) => { let wizard = this.buildJson(this, "wizard"); diff --git a/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 b/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 index f7860ef8..5ffe83c6 100644 --- a/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 +++ b/assets/javascripts/discourse/routes/custom-wizard-index.js.es6 @@ -4,13 +4,7 @@ import Route from "@ember/routing/route"; export default Route.extend({ beforeModel() { const wizard = getCachedWizard(); - if ( - wizard && - (wizard.user || wizard.allow_guests) && - wizard.permitted && - !wizard.completed && - wizard.start - ) { + if (wizard && wizard.permitted && !wizard.completed && wizard.start) { this.replaceWith("customWizardStep", wizard.start); } }, @@ -26,7 +20,7 @@ export default Route.extend({ const wizardId = model.get("id"); const user = model.get("user"); const name = model.get("name"); - const requiresLogin = !user && !model.get("allow_guests"); + const requiresLogin = !user && !permitted; const notPermitted = !permitted; const props = { diff --git a/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 b/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 index 3193d783..dd7b8be8 100644 --- a/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 +++ b/assets/javascripts/discourse/routes/custom-wizard-step.js.es6 @@ -7,12 +7,7 @@ export default Route.extend({ const wizard = getCachedWizard(); this.set("wizard", wizard); - if ( - !wizard || - (!wizard.user && !wizard.allow_guests) || - !wizard.permitted || - wizard.completed - ) { + if (!wizard || !wizard.permitted || wizard.completed) { this.replaceWith("customWizard"); } }, diff --git a/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs b/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs index c14dbfea..a3582780 100644 --- a/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs +++ b/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs @@ -140,22 +140,13 @@ context="wizard" inputTypes="assignment,validation" groupSelection="output" + guestGroup=true userFieldSelection="key" textSelection="value" inputConnector="and" )}} - -
-
- -
-
- {{input type="checkbox" checked=wizard.allow_guests}} - {{i18n "admin.wizard.allow_guests_label"}} -
-
{{/wizard-subscription-container}} diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 99b51324..8a856636 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -105,8 +105,6 @@ en: restart_on_revisit_label: "Clear submissions on each visit." resume_on_revisit: "Resume" resume_on_revisit_label: "Ask the user if they want to resume on each visit." - allow_guests: "Guests" - allow_guests_label: "Allow guests to use the wizard (disables user-specific features)." theme_id: "Theme" no_theme: "Select a Theme (optional)" save: "Save Changes" @@ -219,6 +217,8 @@ en: list: "list" custom_field: "custom field" value: "value" + users: "users" + guests: "users and guests" placeholder: text: "Enter text" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index af25fec2..b283364e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -54,7 +54,7 @@ en: after_time: "After time setting is invalid." liquid_syntax_error: "Liquid syntax error in %{attribute}: %{message}" subscription: "%{type} %{property} is subscription only" - allow_guests: "%{object_id} is not permitted when allow_guests is enabled" + not_permitted_for_guests: "%{object_id} is not permitted when guests can access the wizard" site_settings: custom_wizard_enabled: "Enable custom wizards." diff --git a/lib/custom_wizard/action.rb b/lib/custom_wizard/action.rb index c1f04c0c..34f81455 100644 --- a/lib/custom_wizard/action.rb +++ b/lib/custom_wizard/action.rb @@ -136,7 +136,7 @@ class CustomWizard::Action params[:archetype] = Archetype.private_message - poster = @wizard.allow_guests ? Discourse.system_user : user + poster = user || Discourse.system_user creator = PostCreator.new(poster, params) post = creator.create diff --git a/lib/custom_wizard/subscription.rb b/lib/custom_wizard/subscription.rb index 6049d1a8..c3c9803d 100644 --- a/lib/custom_wizard/subscription.rb +++ b/lib/custom_wizard/subscription.rb @@ -24,12 +24,6 @@ class CustomWizard::Subscription standard: ['*'], business: ['*'], community: ['*'] - }, - allow_guests: { - none: [], - standard: ['*'], - business: ['*'], - community: [] } }, step: { diff --git a/lib/custom_wizard/validators/template.rb b/lib/custom_wizard/validators/template.rb index c2f6f8b4..38b2595c 100644 --- a/lib/custom_wizard/validators/template.rb +++ b/lib/custom_wizard/validators/template.rb @@ -82,8 +82,12 @@ class CustomWizard::TemplateValidator end def validate_action(action) - if @data[:allow_guests] && CustomWizard::Action::REQUIRES_USER.include?(action[:type]) - errors.add :base, I18n.t("wizard.validation.allow_guests", object_id: action[:id]) + guests_permitted = @data[:permitted] && @data[:permitted].any? do |m| + m[:output] === CustomWizard::Wizard::GUEST_GROUP_ID + end + + if guests_permitted && CustomWizard::Action::REQUIRES_USER.include?(action[:type]) + errors.add :base, I18n.t("wizard.validation.not_permitted_for_guests", object_id: action[:id]) end end diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index 35aed456..d6d453ca 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -20,7 +20,6 @@ class CustomWizard::Wizard :prompt_completion, :restart_on_revisit, :resume_on_revisit, - :allow_guests, :permitted, :steps, :step_ids, @@ -37,6 +36,7 @@ class CustomWizard::Wizard attr_reader :all_step_ids GUEST_ID_PREFIX ||= "guest" + GUEST_GROUP_ID = -1 def initialize(attrs = {}, user = nil, guest_id = nil) if user @@ -55,7 +55,6 @@ class CustomWizard::Wizard @prompt_completion = cast_bool(attrs['prompt_completion']) @restart_on_revisit = cast_bool(attrs['restart_on_revisit']) @resume_on_revisit = cast_bool(attrs['resume_on_revisit']) - @allow_guests = cast_bool(attrs['allow_guests']) @after_signup = cast_bool(attrs['after_signup']) @after_time = cast_bool(attrs['after_time']) @after_time_scheduled = attrs['after_time_scheduled'] @@ -212,9 +211,8 @@ class CustomWizard::Wizard def permitted? return nil unless actor_id - return true if allow_guests - return false unless user - return true if user.admin? || permitted.blank? + return true if user && (user.admin? || permitted.blank?) + return false if !user && permitted.blank? mapper = CustomWizard::Mapper.new( inputs: permitted, @@ -228,22 +226,22 @@ class CustomWizard::Wizard return true if mapper.blank? mapper.all? do |m| - if m[:type] === 'assignment' - [*m[:result]].include?(Group::AUTO_GROUPS[:everyone]) || - GroupUser.exists?(group_id: m[:result], user_id: user.id) - elsif m[:type] === 'validation' - m[:result] + if !user + m[:type] === 'assignment' && [*m[:result]].include?(GUEST_GROUP_ID) else - true + if m[:type] === 'assignment' + [*m[:result]].include?(Group::AUTO_GROUPS[:everyone]) || + GroupUser.exists?(group_id: m[:result], user_id: user.id) + elsif m[:type] === 'validation' + m[:result] + else + true + end end end end def can_access? - return nil unless actor_id - return true if allow_guests - return false unless user - return true if user.admin permitted? && (multiple_submissions || !completed?) end diff --git a/spec/components/custom_wizard/action_spec.rb b/spec/components/custom_wizard/action_spec.rb index 6db010b5..79c64520 100644 --- a/spec/components/custom_wizard/action_spec.rb +++ b/spec/components/custom_wizard/action_spec.rb @@ -18,6 +18,7 @@ describe CustomWizard::Action do let(:api_test_endpoint) { get_wizard_fixture("endpoints/test_endpoint") } let(:api_test_endpoint_body) { get_wizard_fixture("endpoints/test_endpoint_body") } let(:api_test_no_authorization) { get_wizard_fixture("api/no_authorization") } + let(:guests_permitted) { get_wizard_fixture("wizard/guests_permitted") } def update_template(template) CustomWizard::Template.save(template, skip_jobs: true) @@ -302,8 +303,8 @@ describe CustomWizard::Action do expect(post.exists?).to eq(true) end - it "send_message works with allow_guests enabled" do - wizard_template["allow_guests"] = true + it "send_message works with guests are permitted" do + wizard_template["permitted"] = guests_permitted["permitted"] wizard_template.delete("actions") wizard_template['actions'] = [send_message] update_template(wizard_template) diff --git a/spec/fixtures/actions/route_to.json b/spec/fixtures/actions/route_to.json new file mode 100644 index 00000000..442b1556 --- /dev/null +++ b/spec/fixtures/actions/route_to.json @@ -0,0 +1,12 @@ +{ + "id": "route_to", + "type": "route_to", + "url": [ + { + "type": "assignment", + "output": "https://google.com", + "output_type": "text", + "output_connector": "set" + } + ] +} \ No newline at end of file diff --git a/spec/fixtures/wizard/guests_permitted.json b/spec/fixtures/wizard/guests_permitted.json new file mode 100644 index 00000000..3a332f31 --- /dev/null +++ b/spec/fixtures/wizard/guests_permitted.json @@ -0,0 +1,12 @@ +{ + "permitted": [ + { + "type": "assignment", + "output_type": "group", + "output_connector": "set", + "output": [ + -1 + ] + } + ] +} \ No newline at end of file diff --git a/spec/requests/custom_wizard/steps_controller_spec.rb b/spec/requests/custom_wizard/steps_controller_spec.rb index 9f77c100..4d8b96eb 100644 --- a/spec/requests/custom_wizard/steps_controller_spec.rb +++ b/spec/requests/custom_wizard/steps_controller_spec.rb @@ -7,11 +7,21 @@ describe CustomWizard::StepsController do let(:wizard_field_condition_template) { get_wizard_fixture("condition/wizard_field_condition") } let(:user_condition_template) { get_wizard_fixture("condition/user_condition") } let(:permitted_json) { get_wizard_fixture("wizard/permitted") } + let(:route_to_template) { get_wizard_fixture("actions/route_to") } + let(:guests_permitted) { get_wizard_fixture("wizard/guests_permitted") } before do CustomWizard::Template.save(wizard_template, skip_jobs: true) end + def guest_template + temp = wizard_template.dup + temp["permitted"] = guests_permitted["permitted"] + temp.delete("actions") + temp["actions"] = [route_to_template] + temp + end + context "with guest" do it "does not perform a step update" do put '/w/super-mega-fun-wizard/steps/step_1.json', params: { @@ -22,13 +32,10 @@ describe CustomWizard::StepsController do expect(response.status).to eq(403) end - context "with allow_guests enabled" do + context "with guests permitted" do before do enable_subscription("standard") - new_template = wizard_template.dup - new_template["allow_guests"] = true - new_template.delete("actions") - result = CustomWizard::Template.save(new_template, skip_jobs: true) + result = CustomWizard::Template.save(guest_template, skip_jobs: true) end it "performs a step update" do @@ -44,6 +51,62 @@ describe CustomWizard::StepsController do wizard = CustomWizard::Wizard.create(wizard_id, nil, cookies[:custom_wizard_guest_id]) expect(wizard.current_submission.fields['step_1_field_1']).to eq("Text input") end + + context "raises an error" do + it "when the wizard doesnt exist" do + put '/w/not-super-mega-fun-wizard/steps/step_1.json' + expect(response.status).to eq(400) + end + + it "when the user cant access the wizard" do + enable_subscription("standard") + new_template = guest_template.dup + new_template["permitted"] = permitted_json["permitted"] + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json' + expect(response.status).to eq(403) + end + + it "when the step doesnt exist" do + put '/w/super-mega-fun-wizard/steps/step_10.json' + expect(response.status).to eq(400) + end + end + + it "works if the step has no fields" do + put '/w/super-mega-fun-wizard/steps/step_1.json' + expect(response.status).to eq(200) + expect(response.parsed_body['wizard']['start']).to eq("step_2") + end + + it "returns an updated wizard when condition passes" do + new_template = guest_template.dup + new_template['steps'][1]['condition'] = wizard_field_condition_template['condition'] + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json', params: { + fields: { + step_1_field_1: "Condition will pass" + } + } + expect(response.status).to eq(200) + expect(response.parsed_body['wizard']['start']).to eq("step_2") + end + + it "runs completion actions if guest has completed wizard" do + new_template = guest_template.dup + + ## route_to action + new_template['actions'].last['run_after'] = 'wizard_completion' + CustomWizard::Template.save(new_template, skip_jobs: true) + + put '/w/super-mega-fun-wizard/steps/step_1.json' + put '/w/super-mega-fun-wizard/steps/step_2.json' + put '/w/super-mega-fun-wizard/steps/step_3.json' + expect(response.status).to eq(200) + expect(response.parsed_body['redirect_on_complete']).to eq("https://google.com") + end end end diff --git a/test/javascripts/acceptance/wizard-test.js b/test/javascripts/acceptance/wizard-test.js index d7078166..e2e6ce04 100644 --- a/test/javascripts/acceptance/wizard-test.js +++ b/test/javascripts/acceptance/wizard-test.js @@ -122,4 +122,44 @@ acceptance("Wizard | Guest access", function (needs) { await visit("/w/wizard"); assert.ok(query(".wizard-column"), true); }); + + test("Applies the wizard body class", async function (assert) { + await visit("/w/wizard"); + assert.ok($("body.custom-wizard").length); + }); + + test("Applies the body background color", async function (assert) { + await visit("/w/wizard"); + assert.ok($("body")[0].style.background); + }); + + test("Renders the wizard form", async function (assert) { + await visit("/w/wizard"); + assert.ok(exists(".wizard-column-contents .wizard-step"), true); + assert.ok(exists(".wizard-footer img"), true); + }); + + test("Renders the first step", async function (assert) { + await visit("/w/wizard"); + assert.strictEqual( + query(".wizard-step-title p").textContent.trim(), + "Text" + ); + assert.strictEqual( + query(".wizard-step-description p").textContent.trim(), + "Text inputs!" + ); + assert.strictEqual( + query(".wizard-step-description p").textContent.trim(), + "Text inputs!" + ); + assert.strictEqual(count(".wizard-step-form .wizard-field"), 6); + assert.ok(exists(".wizard-step-footer .wizard-progress"), true); + assert.ok(exists(".wizard-step-footer .wizard-buttons"), true); + }); + + test("Removes the wizard body class when navigating away", async function (assert) { + await visit("/"); + assert.strictEqual($("body.custom-wizard").length, 0); + }); }); diff --git a/test/javascripts/fixtures/wizard.js.es6 b/test/javascripts/fixtures/wizard.js.es6 index 73fe45c1..a3b83063 100644 --- a/test/javascripts/fixtures/wizard.js.es6 +++ b/test/javascripts/fixtures/wizard.js.es6 @@ -6,7 +6,7 @@ export default { submission_last_updated_at: "2022-03-15T21:11:01+01:00", theme_id: 2, required: false, - permitted: true, + permitted: false, uncategorized_category_id: 1, categories: [], subscribed: false, diff --git a/test/javascripts/helpers/wizard.js b/test/javascripts/helpers/wizard.js index 25ccccbb..e02e2e99 100644 --- a/test/javascripts/helpers/wizard.js +++ b/test/javascripts/helpers/wizard.js @@ -7,9 +7,10 @@ import { cloneJSON } from "discourse-common/lib/object"; const wizardNoUser = cloneJSON(wizardJson); const wizardGuest = cloneJSON(wizardJson); -wizardGuest.allow_guests = true; +wizardGuest.permitted = true; const wizard = cloneJSON(wizardJson); wizard.user = cloneJSON(userJson); +wizard.permitted = true; const wizardNotPermitted = cloneJSON(wizard); wizardNotPermitted.permitted = false; From 1eefd99c6aed6893059911dd0797546e6e377e39 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 7 Feb 2023 13:17:40 +0100 Subject: [PATCH 09/16] Minor fixes --- .../discourse/models/custom-wizard-admin.js.es6 | 4 ++-- lib/custom_wizard/validators/template.rb | 2 +- .../custom_wizard/template_validator_spec.rb | 10 ++++++++++ 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 index 03afd32e..746ca00c 100644 --- a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 +++ b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 @@ -10,9 +10,9 @@ import discourseComputed from "discourse-common/utils/decorators"; const GUEST_GROUP_ID = -1; const CustomWizardAdmin = EmberObject.extend({ - @discourseComputed("permitted.@each") + @discourseComputed("permitted.@each.output") allowGuests(permitted) { - return permitted.filter((p) => p.output === GUEST_GROUP_ID).length; + return permitted.filter((p) => p.output.includes(GUEST_GROUP_ID)).length; }, save(opts) { diff --git a/lib/custom_wizard/validators/template.rb b/lib/custom_wizard/validators/template.rb index 38b2595c..154f525e 100644 --- a/lib/custom_wizard/validators/template.rb +++ b/lib/custom_wizard/validators/template.rb @@ -83,7 +83,7 @@ class CustomWizard::TemplateValidator def validate_action(action) guests_permitted = @data[:permitted] && @data[:permitted].any? do |m| - m[:output] === CustomWizard::Wizard::GUEST_GROUP_ID + m["output"].include?(CustomWizard::Wizard::GUEST_GROUP_ID) end if guests_permitted && CustomWizard::Action::REQUIRES_USER.include?(action[:type]) diff --git a/spec/components/custom_wizard/template_validator_spec.rb b/spec/components/custom_wizard/template_validator_spec.rb index b149706f..54ec9cfa 100644 --- a/spec/components/custom_wizard/template_validator_spec.rb +++ b/spec/components/custom_wizard/template_validator_spec.rb @@ -7,6 +7,7 @@ describe CustomWizard::TemplateValidator do let(:user_condition) { get_wizard_fixture("condition/user_condition") } let(:permitted_json) { get_wizard_fixture("wizard/permitted") } let(:composer_preview) { get_wizard_fixture("field/composer_preview") } + let(:guests_permitted) { get_wizard_fixture("wizard/guests_permitted") } let(:valid_liquid_template) { <<-LIQUID.strip @@ -146,6 +147,15 @@ describe CustomWizard::TemplateValidator do ).to eq(true) end + it "validates restrictions on wizards that permit guests" do + template[:permitted] = guests_permitted['permitted'] + validator = CustomWizard::TemplateValidator.new(template) + expect(validator.perform).to eq(false) + expect(validator.errors.first.type).to eq( + I18n.t("wizard.validation.not_permitted_for_guests", object_id: "action_1") + ) + end + it "validates step attributes" do template[:steps][0][:condition] = user_condition['condition'] expect( From 7657149e6f5200514500cf5834ed2594b9a9c1ea Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 7 Feb 2023 13:53:20 +0100 Subject: [PATCH 10/16] Update custom-wizard-admin.js.es6 --- assets/javascripts/discourse/models/custom-wizard-admin.js.es6 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 index 746ca00c..0cd677f0 100644 --- a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 +++ b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 @@ -12,7 +12,7 @@ const GUEST_GROUP_ID = -1; const CustomWizardAdmin = EmberObject.extend({ @discourseComputed("permitted.@each.output") allowGuests(permitted) { - return permitted.filter((p) => p.output.includes(GUEST_GROUP_ID)).length; + return permitted && permitted.filter((p) => p.output.includes(GUEST_GROUP_ID)).length; }, save(opts) { From 0cb76659e90ad33b99fcae4a910b078c946b5250 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 7 Feb 2023 13:55:08 +0100 Subject: [PATCH 11/16] Update custom-wizard-admin.js.es6 --- .../javascripts/discourse/models/custom-wizard-admin.js.es6 | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 index 0cd677f0..280150af 100644 --- a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 +++ b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 @@ -12,7 +12,10 @@ const GUEST_GROUP_ID = -1; const CustomWizardAdmin = EmberObject.extend({ @discourseComputed("permitted.@each.output") allowGuests(permitted) { - return permitted && permitted.filter((p) => p.output.includes(GUEST_GROUP_ID)).length; + return ( + permitted && + permitted.filter((p) => p.output.includes(GUEST_GROUP_ID)).length + ); }, save(opts) { From edc94b6ea7d86b6f570d167242bf4346d736a754 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Wed, 8 Feb 2023 13:32:24 +0100 Subject: [PATCH 12/16] Restrict guest support to standard and business subscriptions - Support mapped value subscription restrictions - Restrict permitted guest value to standard and business --- .../components/wizard-mapper-selector.js.es6 | 32 +++++++++++-------- config/locales/server.en.yml | 2 +- lib/custom_wizard/mapper.rb | 4 +++ lib/custom_wizard/subscription.rb | 28 ++++++++++++++-- .../custom_wizard/subscription_spec.rb | 14 +++++++- 5 files changed, 62 insertions(+), 18 deletions(-) diff --git a/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 b/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 index 811564a5..e19e4917 100644 --- a/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 +++ b/assets/javascripts/discourse/components/wizard-mapper-selector.js.es6 @@ -15,6 +15,7 @@ import { import Component from "@ember/component"; import { bind, later } from "@ember/runloop"; import I18n from "I18n"; +import Subscription from "../mixins/subscription"; const customFieldActionMap = { topic: ["create_topic", "send_message"], @@ -26,7 +27,7 @@ const customFieldActionMap = { const values = ["present", "true", "false"]; -export default Component.extend({ +export default Component.extend(Subscription, { classNameBindings: [":mapper-selector", "activeType"], showText: computed("activeType", function () { @@ -129,24 +130,27 @@ export default Component.extend({ return this.connector === "is"; }), - @discourseComputed("site.groups", "guestGroup") - groups(groups, guestGroup) { + @discourseComputed("site.groups", "guestGroup", "subscriptionType") + groups(groups, guestGroup, subscriptionType) { let result = groups; if (!guestGroup) { return result; } - let guestIndex; - result.forEach((r, index) => { - if (r.id === 0) { - r.name = I18n.t("admin.wizard.selector.label.users"); - guestIndex = index; - } - }); - result.splice(guestIndex, 0, { - id: -1, - name: I18n.t("admin.wizard.selector.label.guests"), - }); + if (["standard", "business"].includes(subscriptionType)) { + let guestIndex; + result.forEach((r, index) => { + if (r.id === 0) { + r.name = I18n.t("admin.wizard.selector.label.users"); + guestIndex = index; + } + }); + result.splice(guestIndex, 0, { + id: -1, + name: I18n.t("admin.wizard.selector.label.guests"), + }); + } + return result; }, categories: alias("site.categories"), diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b283364e..e8ceb44b 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -53,7 +53,7 @@ en: after_signup_after_time: "You can't use 'after time' and 'after signup' on the same wizard." after_time: "After time setting is invalid." liquid_syntax_error: "Liquid syntax error in %{attribute}: %{message}" - subscription: "%{type} %{property} is subscription only" + subscription: "%{type} %{property} usage is not supported on your subscription" not_permitted_for_guests: "%{object_id} is not permitted when guests can access the wizard" site_settings: diff --git a/lib/custom_wizard/mapper.rb b/lib/custom_wizard/mapper.rb index e4d0db50..410a50e4 100644 --- a/lib/custom_wizard/mapper.rb +++ b/lib/custom_wizard/mapper.rb @@ -284,4 +284,8 @@ class CustomWizard::Mapper user.avatar_template_url.gsub("{size}", parts.last) end end + + def self.mapped_value?(value) + value.is_a?(Array) && value.all? { |v| v.is_a?(Hash) && v.key?("type") } + end end diff --git a/lib/custom_wizard/subscription.rb b/lib/custom_wizard/subscription.rb index c3c9803d..dfb75324 100644 --- a/lib/custom_wizard/subscription.rb +++ b/lib/custom_wizard/subscription.rb @@ -17,7 +17,7 @@ class CustomWizard::Subscription none: [], standard: ['*'], business: ['*'], - community: ['*'] + community: ['*', "!#{CustomWizard::Wizard::GUEST_GROUP_ID}"] }, restart_on_revisit: { none: [], @@ -114,8 +114,15 @@ class CustomWizard::Subscription ## Subscription type does not support the attribute. return false if values.blank? + ## Value is an exception for the subscription type + if (exceptions = get_exceptions(values)).any? + value = mapped_output(value) if CustomWizard::Mapper.mapped_value?(value) + value = [*value].map(&:to_s) + return false if (exceptions & value).length > 0 + end + ## Subscription type supports all values of the attribute. - return true if values.first === "*" + return true if values.include?("*") ## Subscription type supports some values of the attributes. values.include?(value) @@ -192,4 +199,21 @@ class CustomWizard::Subscription def self.includes?(feature, attribute, value) new.includes?(feature, attribute, value) end + + protected + + def get_exceptions(values) + values.reduce([]) do |result, value| + result << value.split("!").last if value.start_with?("!") + result + end + end + + def mapped_output(value) + value.reduce([]) do |result, v| + ## We can only validate mapped assignment values at the moment + result << v["output"] if v.is_a?(Hash) && v["type"] === "assignment" + result + end.flatten + end end diff --git a/spec/components/custom_wizard/subscription_spec.rb b/spec/components/custom_wizard/subscription_spec.rb index 5f06397b..2ac191e1 100644 --- a/spec/components/custom_wizard/subscription_spec.rb +++ b/spec/components/custom_wizard/subscription_spec.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true describe CustomWizard::Subscription do + let(:guests_permitted) { get_wizard_fixture("wizard/guests_permitted") } + def undefine_client_classes Object.send(:remove_const, :SubscriptionClient) if Object.constants.include?(:SubscriptionClient) Object.send(:remove_const, :SubscriptionClientSubscription) if Object.constants.include?(:SubscriptionClientSubscription) @@ -40,7 +42,7 @@ describe CustomWizard::Subscription do expect(described_class.includes?(:wizard, :after_signup, true)).to eq(true) end - it "ubscriber features are not included" do + it "subscriber features are not included" do expect(described_class.includes?(:wizard, :permitted, {})).to eq(false) end end @@ -69,6 +71,16 @@ describe CustomWizard::Subscription do end end + context "with a subscription" do + it "handles mapped values" do + SubscriptionClientSubscription.stubs(:product_id).returns(CustomWizard::Subscription::STANDARD_PRODUCT_ID) + expect(described_class.includes?(:wizard, :permitted, guests_permitted["permitted"])).to eq(true) + + SubscriptionClientSubscription.stubs(:product_id).returns(CustomWizard::Subscription::COMMUNITY_PRODUCT_ID) + expect(described_class.includes?(:wizard, :permitted, guests_permitted["permitted"])).to eq(false) + end + end + context "with standard subscription" do before do SubscriptionClientSubscription.stubs(:product_id).returns(CustomWizard::Subscription::STANDARD_PRODUCT_ID) From 7c8f530c8687e560b8ba5af7449a4e2c6503da41 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 9 Feb 2023 12:33:55 +0100 Subject: [PATCH 13/16] Update wizard.rb --- lib/custom_wizard/wizard.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index d6d453ca..76157148 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -335,7 +335,7 @@ class CustomWizard::Wizard CustomWizard::Template.list(**template_opts).reduce([]) do |result, template| wizard = new(template, user) - result.push(wizard) if wizard.can_access? && ( + result.push(wizard) if wizard.permitted? && ( !not_completed || !wizard.completed? ) result From b2b93aad59a1a2f920e6e31b6dec8a731a7f60a5 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 9 Feb 2023 13:10:55 +0100 Subject: [PATCH 14/16] Ensure admin can access wizard multiple times --- lib/custom_wizard/wizard.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index 76157148..c815c764 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -242,7 +242,7 @@ class CustomWizard::Wizard end def can_access? - permitted? && (multiple_submissions || !completed?) + permitted? && (user&.admin? || (multiple_submissions || !completed?)) end def reset From e7ee89048ada9de1342019669d9765e5287ac7bd Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 9 Feb 2023 14:18:25 +0100 Subject: [PATCH 15/16] Guest wizards cannot use composer or upload --- .../wizard-subscription-selector.js.es6 | 22 ++--------- .../admin-wizards-wizard-show.js.es6 | 14 +++++++ .../discourse/lib/wizard-schema.js.es6 | 38 +++++++++++++++++-- .../models/custom-wizard-admin.js.es6 | 3 +- .../routes/admin-wizards-wizard.js.es6 | 3 +- .../templates/admin-wizards-wizard-show.hbs | 4 +- lib/custom_wizard/field.rb | 5 +++ lib/custom_wizard/validators/template.rb | 14 +++++-- .../custom_wizard/template_validator_spec.rb | 8 +++- 9 files changed, 77 insertions(+), 34 deletions(-) diff --git a/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 b/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 index dce6f781..351b5782 100644 --- a/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 +++ b/assets/javascripts/discourse/components/wizard-subscription-selector.js.es6 @@ -1,6 +1,6 @@ import SingleSelectComponent from "select-kit/components/single-select"; import Subscription from "../mixins/subscription"; -import wizardSchema from "discourse/plugins/discourse-custom-wizard/discourse/lib/wizard-schema"; +import { filterValues } from "discourse/plugins/discourse-custom-wizard/discourse/lib/wizard-schema"; import discourseComputed from "discourse-common/utils/decorators"; import I18n from "I18n"; @@ -40,25 +40,9 @@ export default SingleSelectComponent.extend(Subscription, { return allowedTypes; }, - contentList(feature, attribute, allowGuests) { - let attributes = wizardSchema[feature][attribute]; - - if (allowGuests) { - const filteredFeature = wizardSchema.filters.allow_guests[feature]; - if (filteredFeature) { - const filteredAttribute = filteredFeature[attribute]; - if (filteredAttribute) { - attributes = attributes.filter((a) => filteredAttribute.includes(a)); - } - } - } - - return attributes; - }, - @discourseComputed("feature", "attribute", "wizard.allowGuests") - content(feature, attribute, allowGuests) { - return this.contentList(feature, attribute, allowGuests) + content(feature, attribute) { + return filterValues(this.wizard, feature, attribute) .map((value) => { let allowedSubscriptionTypes = this.allowedSubscriptionTypes( feature, diff --git a/assets/javascripts/discourse/controllers/admin-wizards-wizard-show.js.es6 b/assets/javascripts/discourse/controllers/admin-wizards-wizard-show.js.es6 index c9a80e0e..75ea0ff7 100644 --- a/assets/javascripts/discourse/controllers/admin-wizards-wizard-show.js.es6 +++ b/assets/javascripts/discourse/controllers/admin-wizards-wizard-show.js.es6 @@ -10,6 +10,7 @@ import { later, scheduleOnce } from "@ember/runloop"; import Controller from "@ember/controller"; import copyText from "discourse/lib/copy-text"; import I18n from "I18n"; +import { filterValues } from "discourse/plugins/discourse-custom-wizard/discourse/lib/wizard-schema"; export default Controller.extend({ hasName: notEmpty("wizard.name"), @@ -59,6 +60,19 @@ export default Controller.extend({ } return wizardFieldList(steps); }, + + @discourseComputed("fieldTypes", "wizard.allowGuests") + filteredFieldTypes(fieldTypes) { + const fieldTypeIds = fieldTypes.map((f) => f.id); + const allowedTypeIds = filterValues( + this.wizard, + "field", + "type", + fieldTypeIds + ); + return fieldTypes.filter((f) => allowedTypeIds.includes(f.id)); + }, + getErrorMessage(result) { if (result.backend_validation_error) { return result.backend_validation_error; diff --git a/assets/javascripts/discourse/lib/wizard-schema.js.es6 b/assets/javascripts/discourse/lib/wizard-schema.js.es6 index d3237de1..dcb60a0e 100644 --- a/assets/javascripts/discourse/lib/wizard-schema.js.es6 +++ b/assets/javascripts/discourse/lib/wizard-schema.js.es6 @@ -212,6 +212,24 @@ const action = { const filters = { allow_guests: { + field: { + type: [ + "text", + "textarea", + "text_only", + "date", + "time", + "date_time", + "number", + "checkbox", + "url", + "dropdown", + "tag", + "category", + "group", + "user_selector", + ], + }, action: { type: ["route_to", "send_message"], }, @@ -235,14 +253,26 @@ const wizardSchema = { filters, }; -export function buildFieldTypes(types) { - wizardSchema.field.types = types; -} - export function buildFieldValidations(validations) { wizardSchema.field.validations = validations; } +export function filterValues(currentWizard, feature, attribute, values = null) { + values = values || wizardSchema[feature][attribute]; + + if (currentWizard.allowGuests) { + const filteredFeature = wizardSchema.filters.allow_guests[feature]; + if (filteredFeature) { + const filtered = filteredFeature[attribute]; + if (filtered) { + values = values.filter((v) => filtered.includes(v)); + } + } + } + + return values; +} + const siteSettings = getOwner(this).lookup("service:site-settings"); if (siteSettings.wizard_apis_enabled) { wizardSchema.action.types.send_to_api = { diff --git a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 index 280150af..afca4833 100644 --- a/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 +++ b/assets/javascripts/discourse/models/custom-wizard-admin.js.es6 @@ -14,7 +14,8 @@ const CustomWizardAdmin = EmberObject.extend({ allowGuests(permitted) { return ( permitted && - permitted.filter((p) => p.output.includes(GUEST_GROUP_ID)).length + permitted.filter((p) => p.output && p.output.includes(GUEST_GROUP_ID)) + .length ); }, diff --git a/assets/javascripts/discourse/routes/admin-wizards-wizard.js.es6 b/assets/javascripts/discourse/routes/admin-wizards-wizard.js.es6 index b23b63f6..6e42bcbd 100644 --- a/assets/javascripts/discourse/routes/admin-wizards-wizard.js.es6 +++ b/assets/javascripts/discourse/routes/admin-wizards-wizard.js.es6 @@ -1,5 +1,5 @@ import DiscourseRoute from "discourse/routes/discourse"; -import { buildFieldTypes, buildFieldValidations } from "../lib/wizard-schema"; +import { buildFieldValidations } from "../lib/wizard-schema"; import EmberObject, { set } from "@ember/object"; import { A } from "@ember/array"; import { all } from "rsvp"; @@ -11,7 +11,6 @@ export default DiscourseRoute.extend({ }, afterModel(model) { - buildFieldTypes(model.field_types); buildFieldValidations(model.realtime_validations); return all([ diff --git a/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs b/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs index a3582780..a81ec6db 100644 --- a/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs +++ b/assets/javascripts/discourse/templates/admin-wizards-wizard-show.hbs @@ -161,7 +161,7 @@ wizard=wizard currentField=currentField wizardFields=wizardFields - fieldTypes=fieldTypes + fieldTypes=filteredFieldTypes subscribed=subscribed}} {{/if}} @@ -179,7 +179,7 @@ apis=apis removeAction="removeAction" wizardFields=wizardFields - fieldTypes=fieldTypes}} + fieldTypes=filteredFieldTypes}} {{/each}}
diff --git a/lib/custom_wizard/field.rb b/lib/custom_wizard/field.rb index 6215fc8c..ec85ff3a 100644 --- a/lib/custom_wizard/field.rb +++ b/lib/custom_wizard/field.rb @@ -29,6 +29,11 @@ class CustomWizard::Field attr_accessor :index, :step + REQUIRES_USER = %w[ + composer + upload + ] + def initialize(attrs) @raw = attrs || {} @id = attrs[:id] diff --git a/lib/custom_wizard/validators/template.rb b/lib/custom_wizard/validators/template.rb index 154f525e..60652322 100644 --- a/lib/custom_wizard/validators/template.rb +++ b/lib/custom_wizard/validators/template.rb @@ -30,6 +30,7 @@ class CustomWizard::TemplateValidator validate_subscription(field, :field) check_required(field, :field) validate_liquid_template(field, :field) + validate_guests(field, :field) end end end @@ -39,7 +40,7 @@ class CustomWizard::TemplateValidator validate_subscription(action, :action) check_required(action, :action) validate_liquid_template(action, :action) - validate_action(action) + validate_guests(action, :action) end end @@ -81,13 +82,18 @@ class CustomWizard::TemplateValidator end end - def validate_action(action) + def validate_guests(object, type) guests_permitted = @data[:permitted] && @data[:permitted].any? do |m| m["output"].include?(CustomWizard::Wizard::GUEST_GROUP_ID) end + return unless guests_permitted - if guests_permitted && CustomWizard::Action::REQUIRES_USER.include?(action[:type]) - errors.add :base, I18n.t("wizard.validation.not_permitted_for_guests", object_id: action[:id]) + if type === :action && CustomWizard::Action::REQUIRES_USER.include?(object[:type]) + errors.add :base, I18n.t("wizard.validation.not_permitted_for_guests", object_id: object[:id]) + end + + if type === :field && CustomWizard::Field::REQUIRES_USER.include?(object[:type]) + errors.add :base, I18n.t("wizard.validation.not_permitted_for_guests", object_id: object[:id]) end end diff --git a/spec/components/custom_wizard/template_validator_spec.rb b/spec/components/custom_wizard/template_validator_spec.rb index 54ec9cfa..71f12e12 100644 --- a/spec/components/custom_wizard/template_validator_spec.rb +++ b/spec/components/custom_wizard/template_validator_spec.rb @@ -147,13 +147,17 @@ describe CustomWizard::TemplateValidator do ).to eq(true) end - it "validates restrictions on wizards that permit guests" do + it "validates user-only features" do template[:permitted] = guests_permitted['permitted'] validator = CustomWizard::TemplateValidator.new(template) expect(validator.perform).to eq(false) - expect(validator.errors.first.type).to eq( + errors = validator.errors.to_a + expect(errors).to include( I18n.t("wizard.validation.not_permitted_for_guests", object_id: "action_1") ) + expect(errors).to include( + I18n.t("wizard.validation.not_permitted_for_guests", object_id: "step_2_field_7") + ) end it "validates step attributes" do From e5d6a205323f077253e7079e68c889be99585425 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 9 Feb 2023 14:32:01 +0100 Subject: [PATCH 16/16] Fix specs --- spec/components/custom_wizard/template_validator_spec.rb | 2 ++ spec/fixtures/field/upload.json | 6 ++++++ spec/fixtures/wizard.json | 6 ------ .../custom_wizard/wizard_field_serializer_spec.rb | 1 - 4 files changed, 8 insertions(+), 7 deletions(-) create mode 100644 spec/fixtures/field/upload.json diff --git a/spec/components/custom_wizard/template_validator_spec.rb b/spec/components/custom_wizard/template_validator_spec.rb index 71f12e12..b9b257e3 100644 --- a/spec/components/custom_wizard/template_validator_spec.rb +++ b/spec/components/custom_wizard/template_validator_spec.rb @@ -8,6 +8,7 @@ describe CustomWizard::TemplateValidator do let(:permitted_json) { get_wizard_fixture("wizard/permitted") } let(:composer_preview) { get_wizard_fixture("field/composer_preview") } let(:guests_permitted) { get_wizard_fixture("wizard/guests_permitted") } + let(:upload_field) { get_wizard_fixture("field/upload") } let(:valid_liquid_template) { <<-LIQUID.strip @@ -149,6 +150,7 @@ describe CustomWizard::TemplateValidator do it "validates user-only features" do template[:permitted] = guests_permitted['permitted'] + template[:steps][0][:fields] << upload_field validator = CustomWizard::TemplateValidator.new(template) expect(validator.perform).to eq(false) errors = validator.errors.to_a diff --git a/spec/fixtures/field/upload.json b/spec/fixtures/field/upload.json new file mode 100644 index 00000000..ebf6c21d --- /dev/null +++ b/spec/fixtures/field/upload.json @@ -0,0 +1,6 @@ +{ + "id": "step_2_field_7", + "label": "Upload", + "type": "upload", + "file_types": ".jpg,.jpeg,.png" +} \ No newline at end of file diff --git a/spec/fixtures/wizard.json b/spec/fixtures/wizard.json index de5e636e..5868001e 100644 --- a/spec/fixtures/wizard.json +++ b/spec/fixtures/wizard.json @@ -74,12 +74,6 @@ "id": "step_2_field_5", "label": "Checkbox", "type": "checkbox" - }, - { - "id": "step_2_field_7", - "label": "Upload", - "type": "upload", - "file_types": ".jpg,.jpeg,.png" } ], "description": "Because I couldn't think of another name for this step :)" diff --git a/spec/serializers/custom_wizard/wizard_field_serializer_spec.rb b/spec/serializers/custom_wizard/wizard_field_serializer_spec.rb index 1ac2579e..0568f898 100644 --- a/spec/serializers/custom_wizard/wizard_field_serializer_spec.rb +++ b/spec/serializers/custom_wizard/wizard_field_serializer_spec.rb @@ -29,6 +29,5 @@ describe CustomWizard::FieldSerializer do scope: Guardian.new(user) ).as_json expect(json_array[0][:format]).to eq("YYYY-MM-DD") - expect(json_array[5][:file_types]).to eq(".jpg,.jpeg,.png") end end