From d4148ebd3fe796ddc74f81605d3ed2c26b0614f8 Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Thu, 10 Oct 2024 10:19:43 +0200 Subject: [PATCH] FIX: ensure user is not redirected to single-submission wizard they have completed --- .../custom-wizard-redirect.js.es6 | 1 + lib/custom_wizard/wizard.rb | 14 ++++--- plugin.rb | 4 +- spec/components/custom_wizard/wizard_spec.rb | 39 ++++++++++++++----- spec/jobs/set_after_time_wizard_spec.rb | 27 +++++++++++++ .../application_controller_spec.rb | 19 +++++++++ 6 files changed, 87 insertions(+), 17 deletions(-) diff --git a/assets/javascripts/discourse/initializers/custom-wizard-redirect.js.es6 b/assets/javascripts/discourse/initializers/custom-wizard-redirect.js.es6 index c02f0f3d..da4187e1 100644 --- a/assets/javascripts/discourse/initializers/custom-wizard-redirect.js.es6 +++ b/assets/javascripts/discourse/initializers/custom-wizard-redirect.js.es6 @@ -32,6 +32,7 @@ export default { redirectToWizard && !data.url.includes("ignore_redirect") && data.currentRouteName !== "customWizardStep" && + data.currentRouteName !== "customWizard.index" && !excludedPaths.find((p) => { return data.currentRouteName.indexOf(p) > -1; }) diff --git a/lib/custom_wizard/wizard.rb b/lib/custom_wizard/wizard.rb index e2933754..35124dfa 100644 --- a/lib/custom_wizard/wizard.rb +++ b/lib/custom_wizard/wizard.rb @@ -203,7 +203,7 @@ class CustomWizard::Wizard context: id ) - if after_time + if after_time && multiple_submissions history = history.where("updated_at > ?", after_time_scheduled) end @@ -244,8 +244,12 @@ class CustomWizard::Wizard end end - def can_access? - permitted? && (user&.admin? || (multiple_submissions || !completed?)) + def can_submit? + multiple_submissions || !completed? + end + + def can_access?(always_allow_admin: true) + permitted?(always_allow_admin: always_allow_admin) && can_submit? end def reset @@ -321,7 +325,7 @@ class CustomWizard::Wizard end def remove_user_redirect - return unless user.present? + return if user.blank? if id == user.redirect_to_wizard user.custom_fields.delete('redirect_to_wizard') @@ -384,7 +388,7 @@ class CustomWizard::Wizard def self.set_user_redirect(wizard_id, user) wizard = self.create(wizard_id, user) - if wizard.permitted?(always_allow_admin: false) + if wizard.can_access?(always_allow_admin: false) user.custom_fields['redirect_to_wizard'] = wizard_id user.save_custom_fields(true) else diff --git a/plugin.rb b/plugin.rb index 2e43f5fa..3f0c1e24 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.8.7 +# version: 2.8.8 # authors: Angus McLeod, Faizaan Gagan, Robert Barrow, Keegan George, Kaitlin Maddever, Juan Marcos Gutierrez Ramos # url: https://github.com/paviliondev/discourse-custom-wizard # contact_emails: development@pavilion.tech @@ -181,7 +181,7 @@ after_initialize do end wizard = CustomWizard::Wizard.create(wizard_id, current_user) - redirect_to "/w/#{wizard_id.dasherize}" if wizard.permitted?(always_allow_admin: false) + redirect_to "/w/#{wizard_id.dasherize}" if wizard.can_access?(always_allow_admin: false) end end end diff --git a/spec/components/custom_wizard/wizard_spec.rb b/spec/components/custom_wizard/wizard_spec.rb index 850633d8..069011c6 100644 --- a/spec/components/custom_wizard/wizard_spec.rb +++ b/spec/components/custom_wizard/wizard_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true describe CustomWizard::Wizard do - fab!(:user) { Fabricate(:user) } + fab!(:user) fab!(:trusted_user) { Fabricate(:user, trust_level: TrustLevel[3]) } fab!(:admin_user) { Fabricate(:user, admin: true) } let(:template_json) { get_wizard_fixture("wizard") } @@ -125,18 +125,37 @@ describe CustomWizard::Wizard do expect(@wizard.completed?).to eq(true) end - it "is not completed if steps submitted before after time" do - append_steps + context "without mutliple submissions" do + it "is completed if steps submitted before after time" do + append_steps - progress_step("step_1") - progress_step("step_2") - progress_step("step_3") + progress_step("step_1") + progress_step("step_2") + progress_step("step_3") - template_json['after_time'] = true - template_json['after_time_scheduled'] = Time.now + 3.hours + template_json['after_time'] = true + template_json['after_time_scheduled'] = Time.now + 3.hours - wizard = CustomWizard::Wizard.new(template_json, user) - expect(wizard.completed?).to eq(false) + wizard = CustomWizard::Wizard.new(template_json, user) + expect(wizard.completed?).to eq(true) + end + end + + context "with multiple submissions" do + it "is completed if steps submitted before after time" do + append_steps + + progress_step("step_1") + progress_step("step_2") + progress_step("step_3") + + template_json['after_time'] = true + template_json['multiple_submissions'] = true + template_json['after_time_scheduled'] = Time.now + 3.hours + + wizard = CustomWizard::Wizard.new(template_json, user) + expect(wizard.completed?).to eq(false) + end end context "with subscription" do diff --git a/spec/jobs/set_after_time_wizard_spec.rb b/spec/jobs/set_after_time_wizard_spec.rb index 33c2fa35..20f76cdd 100644 --- a/spec/jobs/set_after_time_wizard_spec.rb +++ b/spec/jobs/set_after_time_wizard_spec.rb @@ -50,4 +50,31 @@ describe Jobs::SetAfterTimeWizard do ).to eq(1) end end + + context "when user has completed the wizard" do + before do + @after_time_template[:steps].each do |step| + CustomWizard::UserHistory.create!( + action: CustomWizard::UserHistory.actions[:step], + actor_id: user1.id, + context: @after_time_template[:id], + subject: step[:id] + ) + end + end + + it "does not redirect to user" do + messages = MessageBus.track_publish("/redirect_to_wizard") do + described_class.new.execute(wizard_id: 'super_mega_fun_wizard') + end + expect(messages.first.data).to eq("super_mega_fun_wizard") + expect(messages.first.user_ids).to match_array([user2.id, user3.id]) + expect( + UserCustomField.where( + name: 'redirect_to_wizard', + value: 'super_mega_fun_wizard' + ).length + ).to eq(2) + end + end end diff --git a/spec/requests/custom_wizard/application_controller_spec.rb b/spec/requests/custom_wizard/application_controller_spec.rb index 428a2311..3a1f618f 100644 --- a/spec/requests/custom_wizard/application_controller_spec.rb +++ b/spec/requests/custom_wizard/application_controller_spec.rb @@ -123,6 +123,25 @@ describe ApplicationController do end end end + + context "when user has completed the wizard" do + before do + @template[:steps].each do |step| + CustomWizard::UserHistory.create!( + action: CustomWizard::UserHistory.actions[:step], + actor_id: user.id, + context: @template[:id], + subject: step[:id] + ) + end + end + + it "does not redirect" do + travel_to Time.now + 4.hours + get "/" + expect(response).not_to redirect_to("/w/super-mega-fun-wizard") + end + end end end end