Improved after_callbacks and skip_callback behaviour on bulk changes (e. g. CSV import).

app/models/concerns/can_csv_import.rb

@@ -214,8 +214,7 @@ returns
         end
 
         # create object
-        BulkImportInfo.enable
-        Transaction.execute(disable_notification: true, reset_user_id: true) do
+        Transaction.execute(disable_notification: true, reset_user_id: true, bulk: true) do
           UserInfo.current_user_id = clean_params[:updated_by_id] || clean_params[:created_by_id]
           if !record || delete == true
             stats[:created] += 1
@@ -263,7 +262,6 @@ returns
             end
           end
         end
-        BulkImportInfo.disable
 
         records.push record
       end

app/models/cti/caller_id.rb

@@ -6,11 +6,8 @@ module Cti
 
     # adopt/orphan matching Cti::Log records
     # (see https://github.com/zammad/zammad/issues/2057)
-    after_commit :update_cti_logs, on: :destroy
-    after_commit :update_cti_logs_with_fg_optimization, on: :create
-
-    skip_callback :commit, :after, :update_cti_logs, if: -> { BulkImportInfo.enabled? }
-    skip_callback :commit, :after, :update_cti_logs_with_fg_optimization, if: -> { BulkImportInfo.enabled? }
+    after_commit :update_cti_logs, on: :destroy, unless: -> { BulkImportInfo.enabled? }
+    after_commit :update_cti_logs_with_fg_optimization, on: :create, unless: -> { BulkImportInfo.enabled? }
 
 =begin
 

app/models/transaction.rb

@@ -3,6 +3,9 @@ class Transaction
     if options[:reset_user_id] == true
       UserInfo.current_user_id = 1
     end
+    if options[:bulk] == true
+      BulkImportInfo.enable
+    end
     original_interface_handle = ApplicationHandleInfo.current
     if options[:interface_handle]
       ApplicationHandleInfo.current = options[:interface_handle]
@@ -16,5 +19,8 @@ class Transaction
       Observer::Transaction.commit(options)
       PushMessages.finish
     end
+    return if options[:bulk] != true
+
+    BulkImportInfo.disable
   end
 end

app/models/user.rb

@@ -25,14 +25,11 @@ class User < ApplicationModel
   before_validation :check_mail_delivery_failed, on: :update
   before_create     :check_preferences_default, :validate_preferences, :validate_ooo, :domain_based_assignment, :set_locale
   before_update     :check_preferences_default, :validate_preferences, :validate_ooo, :reset_login_failed, :validate_agent_limit_by_attributes, :last_admin_check_by_attribute
-  after_create      :avatar_for_email_check
-  after_update      :avatar_for_email_check
+  after_create      :avatar_for_email_check, unless: -> { BulkImportInfo.enabled? }
+  after_update      :avatar_for_email_check, unless: -> { BulkImportInfo.enabled? }
   after_commit      :update_caller_id
   before_destroy    :destroy_longer_required_objects
 
-  skip_callback :create, :after, :avatar_for_email_check, if: -> { BulkImportInfo.enabled? }
-  skip_callback :update, :after, :avatar_for_email_check, if: -> { BulkImportInfo.enabled? }
-
   store :preferences
 
   activity_stream_permission 'admin.user'

lib/sequencer/sequence/import/ldap/user.rb

@@ -29,7 +29,7 @@ class Sequencer
               'Import::Common::Model::Update',
               'Import::Common::Model::Create',
               'Import::Common::Model::Associations::Assign',
-              'Import::Ldap::User::Model::Save',
+              'Import::Common::Model::Save',
               'Import::Common::Model::ExternalSync::Integrity',
               'Import::Ldap::User::HttpLog',
               'Import::Ldap::User::Statistics::Diff',

lib/sequencer/unit/import/common/model/save.rb

@@ -18,12 +18,19 @@ class Sequencer
               return if dry_run
               return if instance.blank?
 
+              save!
+            end
+
+            def save!
+              BulkImportInfo.enable
               instance.save!
             rescue => e
               handle_failure(e)
 
               # unset instance if something went wrong
               state.provide(:instance, nil)
+            ensure
+              BulkImportInfo.disable
             end
           end
         end

lib/sequencer/unit/import/ldap/user/model/save.rb

@@ -1,20 +0,0 @@
-require_dependency 'sequencer/unit/import/common/model/mixin/without_callback'
-
-class Sequencer
-  class Unit
-    module Import
-      module Ldap
-        module User
-          module Model
-            class Save < Import::Common::Model::Save
-              prepend ::Sequencer::Unit::Import::Common::Model::Mixin::WithoutCallback
-
-              without_callback :create, :after, :avatar_for_email_check
-              without_callback :update, :after, :avatar_for_email_check
-            end
-          end
-        end
-      end
-    end
-  end
-end

spec/lib/sequencer/unit/import/common/model/save_spec.rb

@@ -44,4 +44,17 @@ RSpec.describe Sequencer::Unit::Import::Common::Model::Save, sequencer: :unit do
       expect(user).not_to have_received(:save!)
     end
   end
+
+  context 'for BulkImportInfo flag' do
+
+    it 'enables BulkImportInfo' do
+      expect(BulkImportInfo).to receive(:enable)
+      process(action: :created, instance: user, dry_run: false)
+    end
+
+    it 'ensures BulkImportInfo is disabled' do
+      expect(BulkImportInfo).to receive(:disable)
+      process(action: :created, instance: user, dry_run: false)
+    end
+  end
 end

spec/support/cache.rb

@@ -8,5 +8,8 @@ RSpec.configure do |config|
     # clear Setting cache to prevent leaking
     # of Setting changes from previous test examples
     Setting.reload
+
+    # reset bulk import to prevent wrong base setting
+    BulkImportInfo.disable
   end
 end

spec/support/capybara/set_up.rb

@@ -4,6 +4,7 @@ RSpec.configure do |config|
     # make sure system is in a fresh state
     Cache.clear
     Setting.reload
+    BulkImportInfo.disable
 
     # check if system is already set up
     next if Setting.get('system_init_done')