From 522d4e94893de1c9a233242573f64f16aed8e7bb Mon Sep 17 00:00:00 2001 From: Angus McLeod Date: Mon, 7 Jun 2021 23:15:41 +1000 Subject: [PATCH] IMPROVE: ensure redirect_to_wizard is cleaned up even if sidekiq is not working (#116) * Add test of redirect_to_wizard when wizard is removed * Make clear_user_wizard_redirect a synchronous operation --- jobs/clear_after_time_wizard.rb | 15 ------- lib/custom_wizard/template.rb | 15 +++---- plugin.rb | 1 - .../components/custom_wizard/template_spec.rb | 8 ++++ spec/jobs/clear_after_time_wizard_spec.rb | 41 ------------------- .../application_controller_spec.rb | 6 +++ 6 files changed, 22 insertions(+), 64 deletions(-) delete mode 100644 jobs/clear_after_time_wizard.rb delete mode 100644 spec/jobs/clear_after_time_wizard_spec.rb diff --git a/jobs/clear_after_time_wizard.rb b/jobs/clear_after_time_wizard.rb deleted file mode 100644 index 37d997db..00000000 --- a/jobs/clear_after_time_wizard.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true -module Jobs - class ClearAfterTimeWizard < ::Jobs::Base - sidekiq_options queue: 'critical' - - def execute(args) - User.human_users.each do |u| - if u.custom_fields['redirect_to_wizard'] == args[:wizard_id] - u.custom_fields.delete('redirect_to_wizard') - u.save_custom_fields(true) - end - end - end - end -end diff --git a/lib/custom_wizard/template.rb b/lib/custom_wizard/template.rb index a1c0aad0..8e944dca 100644 --- a/lib/custom_wizard/template.rb +++ b/lib/custom_wizard/template.rb @@ -49,18 +49,15 @@ class CustomWizard::Template def self.remove(wizard_id) wizard = CustomWizard::Wizard.create(wizard_id) - return false if !wizard ActiveRecord::Base.transaction do PluginStore.remove(CustomWizard::PLUGIN_NAME, wizard.id) - - if wizard.after_time - Jobs.cancel_scheduled_job(:set_after_time_wizard) - Jobs.enqueue(:clear_after_time_wizard, wizard_id: wizard_id) - end + clear_user_wizard_redirect(wizard_id) end + Jobs.cancel_scheduled_job(:set_after_time_wizard) if wizard.after_time + true end @@ -88,6 +85,10 @@ class CustomWizard::Template end end + def self.clear_user_wizard_redirect(wizard_id) + UserCustomField.where(name: 'redirect_to_wizard', value: wizard_id).destroy_all + end + private def normalize_data @@ -132,7 +133,7 @@ class CustomWizard::Template Jobs.enqueue_at(enqueue_wizard_at, :set_after_time_wizard, wizard_id: wizard_id) elsif old_data && old_data[:after_time] Jobs.cancel_scheduled_job(:set_after_time_wizard, wizard_id: wizard_id) - Jobs.enqueue(:clear_after_time_wizard, wizard_id: wizard_id) + self.class.clear_user_wizard_redirect(wizard_id) end end end diff --git a/plugin.rb b/plugin.rb index e3d32129..0beab1cc 100644 --- a/plugin.rb +++ b/plugin.rb @@ -49,7 +49,6 @@ after_initialize do ../controllers/custom_wizard/wizard.rb ../controllers/custom_wizard/steps.rb ../controllers/custom_wizard/realtime_validations.rb - ../jobs/clear_after_time_wizard.rb ../jobs/refresh_api_access_token.rb ../jobs/set_after_time_wizard.rb ../lib/custom_wizard/validators/template.rb diff --git a/spec/components/custom_wizard/template_spec.rb b/spec/components/custom_wizard/template_spec.rb index fb76e0c4..0e3dbdbe 100644 --- a/spec/components/custom_wizard/template_spec.rb +++ b/spec/components/custom_wizard/template_spec.rb @@ -41,6 +41,14 @@ describe CustomWizard::Template do ).to eq(nil) end + it "removes user wizard redirects if template is removed" do + user.custom_fields['redirect_to_wizard'] = 'super_mega_fun_wizard' + user.save_custom_fields(true) + + CustomWizard::Template.remove('super_mega_fun_wizard') + expect(user.reload.custom_fields['redirect_to_wizard']).to eq(nil) + end + it "checks for wizard template existence" do expect( CustomWizard::Template.exists?('super_mega_fun_wizard') diff --git a/spec/jobs/clear_after_time_wizard_spec.rb b/spec/jobs/clear_after_time_wizard_spec.rb deleted file mode 100644 index 935036a3..00000000 --- a/spec/jobs/clear_after_time_wizard_spec.rb +++ /dev/null @@ -1,41 +0,0 @@ -# frozen_string_literal: true - -require_relative '../plugin_helper' - -describe Jobs::ClearAfterTimeWizard do - fab!(:user1) { Fabricate(:user) } - fab!(:user2) { Fabricate(:user) } - fab!(:user3) { Fabricate(:user) } - - let(:template) { - JSON.parse(File.open( - "#{Rails.root}/plugins/discourse-custom-wizard/spec/fixtures/wizard.json" - ).read).with_indifferent_access - } - - it "clears wizard redirect for all users " do - after_time_template = template.dup - after_time_template["after_time"] = true - after_time_template["after_time_scheduled"] = (Time.now + 3.hours).iso8601 - - CustomWizard::Template.save(after_time_template) - - Jobs::SetAfterTimeWizard.new.execute(wizard_id: 'super_mega_fun_wizard') - - expect( - UserCustomField.where( - name: 'redirect_to_wizard', - value: 'super_mega_fun_wizard' - ).length - ).to eq(3) - - described_class.new.execute(wizard_id: 'super_mega_fun_wizard') - - expect( - UserCustomField.where(" - name = 'redirect_to_wizard' AND - value = 'super_mega_fun_wizard' - ").exists? - ).to eq(false) - end -end diff --git a/spec/requests/custom_wizard/application_controller_spec.rb b/spec/requests/custom_wizard/application_controller_spec.rb index f79db877..e8b45c48 100644 --- a/spec/requests/custom_wizard/application_controller_spec.rb +++ b/spec/requests/custom_wizard/application_controller_spec.rb @@ -43,6 +43,12 @@ describe ApplicationController do .first['redirect_to'] ).to eq("/t/2") end + + it "does not redirect if wizard does not exist" do + CustomWizard::Template.remove('super_mega_fun_wizard') + get "/" + expect(response.status).to eq(200) + end end context "who is not required to complete wizard" do