From 8f8c6d50c6fa23dfa154a726cf8e0bb07af75603 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Tue, 7 Feb 2023 12:46:17 +0100 Subject: [PATCH] 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;