Remove the Settings::Definition on_change callback as it is not neccessary anymore

app/services/settings/update_service.rb

@@ -32,13 +32,6 @@ class Settings::UpdateService < BaseServices::BaseContracted
           contract_class: Settings::UpdateContract
   end
 
-  def after_validate(params, call)
-    params.keys.each(&method(:remember_previous_value))
-    call
-  end
-
-  # We will have a problem with error handling on the form.
-  # How can we still display the user changed values in case the form is not successfully saved?
   def persist(call)
     params.each do |name, value|
       set_setting_value(name, value)
@@ -46,34 +39,12 @@ class Settings::UpdateService < BaseServices::BaseContracted
     call
   end
 
-  def after_perform(call)
-    super.tap do
-      params.each_key do |name|
-        run_on_change_callback(name)
-      end
-    end
-  end
-
   private
 
-  def remember_previous_value(name)
-    previous_values[name] = Setting[name]
-  end
-
   def set_setting_value(name, value)
     Setting[name] = derive_value(value)
   end
 
-  def previous_values
-    @previous_values ||= {}
-  end
-
-  def run_on_change_callback(name)
-    if (definition = Settings::Definition[name]) && definition.on_change
-      definition.on_change.call(previous_values[name])
-    end
-  end
-
   def derive_value(value)
     case value
     when Array, Hash

config/constants/settings/definition.rb

@@ -33,8 +33,7 @@ module Settings
 
     attr_accessor :name,
                   :format,
-                  :env_alias,
-                  :on_change
+                  :env_alias
 
     attr_writer :value,
                 :allowed
@@ -44,8 +43,7 @@ module Settings
                    format: nil,
                    writable: true,
                    allowed: nil,
-                   env_alias: nil,
-                   on_change: nil)
+                   env_alias: nil)
       self.name = name.to_s
       @default = default.is_a?(Hash) ? default.deep_stringify_keys : default
       @default.freeze
@@ -54,7 +52,6 @@ module Settings
       self.writable = writable
       self.allowed = allowed
       self.env_alias = env_alias
-      self.on_change = on_change
     end
 
     def default
@@ -136,14 +133,12 @@ module Settings
       # @param [nil] env_alias Alternative for the default env name to also look up. E.g. with the alias set to
       #  `OPENPROJECT_2FA` for a definition with the name `two_factor_authentication`, the value is fetched
       #  from the ENV OPENPROJECT_2FA as well.
-      # @param [nil] on_change A callback lambda to be triggered whenever the setting is stored to the database.
       def add(name,
               default:,
               format: nil,
               writable: true,
               allowed: nil,
-              env_alias: nil,
-              on_change: nil)
+              env_alias: nil)
         return if @by_name.present? && @by_name[name.to_s].present?
 
         @by_name = nil
@@ -153,8 +148,7 @@ module Settings
                          default:,
                          writable:,
                          allowed:,
-                         env_alias:,
-                         on_change:)
+                         env_alias:)
 
         override_value(definition)
 

config/constants/settings/definitions.rb

@@ -938,10 +938,7 @@ Settings::Definition.define do
   add :working_days,
       format: :array,
       allowed: Array(1..7),
-      default: Array(1..5), # Sat, Sun being non-working days
-      on_change: ->(previous_working_days) do
-        WorkPackages::ApplyWorkingDaysChangeJob.perform_later(user_id: User.current.id, previous_working_days:)
-      end
+      default: Array(1..5) # Sat, Sun being non-working days
 
   add :youtube_channel,
       default: 'https://www.youtube.com/c/OpenProjectCommunity',

spec/constants/settings/definition_spec.rb

@@ -968,32 +968,4 @@ describe Settings::Definition do
       end
     end
   end
-
-  describe '#on_change', :settings_reset do
-    context 'for a definition with a callback' do
-      let(:callback) { -> { 'foobar ' } }
-
-      it 'includes the callback' do
-        described_class.add 'bogus',
-                            default: 1,
-                            format: :integer,
-                            on_change: callback
-
-        expect(described_class['bogus'].on_change)
-          .to eq callback
-      end
-    end
-
-    context 'for a definition without a callback' do
-      it 'includes the callback' do
-        described_class.add 'bogus',
-                            default: 1,
-                            format: :integer,
-                            on_change: nil
-
-        expect(described_class['bogus'].on_change)
-          .to be_nil
-      end
-    end
-  end
 end

spec/services/settings/update_service_spec.rb

@@ -39,8 +39,7 @@ describe Settings::UpdateService do
   end
   let(:contract_success) { true }
   let(:setting_definition) do
-    instance_double(Settings::Definition,
-                    on_change: definition_on_change)
+    instance_double(Settings::Definition)
   end
   let(:definition_on_change) do
     instance_double(Proc,
@@ -80,24 +79,10 @@ describe Settings::UpdateService do
 
     include_examples 'successful call'
 
-    it 'calls the on_change handler' do
-      subject
-
-      expect(definition_on_change)
-        .to have_received(:call).with(previous_setting_value)
-    end
-
     context 'when the contract is not successfully validated' do
       let(:contract_success) { false }
 
       include_examples 'unsuccessful call'
-
-      it 'does not call the on_change handler' do
-        subject
-
-        expect(definition_on_change)
-          .not_to have_received(:call)
-      end
     end
   end
 end