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

@@ -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

lib/custom_wizard/template.rb

@@ -49,17 +49,14 @@ 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)
+      clear_user_wizard_redirect(wizard_id)
+    end
 
-      if wizard.after_time
+    Jobs.cancel_scheduled_job(:set_after_time_wizard) if wizard.after_time
-        Jobs.cancel_scheduled_job(:set_after_time_wizard)
-        Jobs.enqueue(:clear_after_time_wizard, wizard_id: wizard_id)
-      end
-    end
 
     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

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

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')

spec/jobs/clear_after_time_wizard_spec.rb

@@ -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

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