Merge pull request #11854 from opf/implementation/45473-update-the-workpackagesapplyworkingdayschangejob-to-take-nonworkingdays-into-account

Implementation/45473 update the WorkpackagesApplyWorkingdaysChangeJob to take NonWorkingDays into account

app/models/work_package.rb

@@ -120,7 +120,7 @@ class WorkPackage < ApplicationRecord
     where(author_id: author.id)
   }
 
-  scopes :covering_days_of_week,
+  scopes :covering_dates_and_days_of_week,
          :for_scheduling,
          :include_derived_dates,
          :include_spent_time,

app/models/work_packages/scopes/covering_dates_and_days_of_week.rb

@@ -26,27 +26,29 @@
 # See COPYRIGHT and LICENSE files for more details.
 #++
 
-module WorkPackages::Scopes::CoveringDaysOfWeek
+module WorkPackages::Scopes::CoveringDatesAndDaysOfWeek
   extend ActiveSupport::Concern
   using CoreExtensions::SquishSql
 
   class_methods do
-    # Fetches all work packages that cover specific days of the week.
+    # Fetches all work packages that cover specific days of the week, and/or specific dates.
     #
     # The period considered is from the work package start date to the due date.
     #
+    # @param dates Date[] An array of the Date objects.
     # @param days_of_week number[] An array of the ISO days of the week to
     #   consider. 1 is Monday, 7 is Sunday.
-    def covering_days_of_week(days_of_week)
+    def covering_dates_and_days_of_week(days_of_week: [], dates: [])
       days_of_week = Array(days_of_week)
-      return none if days_of_week.empty?
+      dates = Array(dates)
+      return none if days_of_week.empty? && dates.empty?
 
-      where("id IN (#{query(days_of_week)})")
+      where("id IN (#{query(days_of_week, dates)})")
     end
 
     private
 
-    def query(days_of_week)
+    def query(days_of_week, dates)
       sql = <<~SQL.squish
         -- select work packages dates with their followers dates
         WITH work_packages_with_dates AS (
@@ -59,7 +61,6 @@ module WorkPackages::Scopes::CoveringDaysOfWeek
               work_packages.start_date IS NOT NULL
               OR work_packages.due_date IS NOT NULL
             )
-          ORDER BY work_packages.id
         ),
         -- coalesce non-existing dates of work package to get period start/end
         work_packages_periods AS (
@@ -68,29 +69,29 @@ module WorkPackages::Scopes::CoveringDaysOfWeek
             GREATEST(work_package_start_date, work_package_due_date) AS end_date
           FROM work_packages_with_dates
         ),
-        -- expand period into days of the week. Limit to 7 days (more would be useless).
-        work_packages_days_of_week AS (
-          SELECT id,
-            extract(
-              isodow
-              from generate_series(
-                  work_packages_periods.start_date,
-                  LEAST(
-                    work_packages_periods.start_date + 6,
-                    work_packages_periods.end_date
-                  ),
-                  '1 day'
-                )
-            ) AS dow
+        -- All days between the start date of a work package and its due date
+        covered_dates AS (
+          SELECT
+           id,
+           generate_series(work_packages_periods.start_date,
+                           work_packages_periods.end_date,
+                           '1 day')          AS date
           FROM work_packages_periods
+        ),
+        -- All days between the start date of a work package and its due date including the day of the week for each date
+        covered_dates_and_wday AS (
+          SELECT
+            id,
+            date,
+            EXTRACT(isodow FROM date) dow
+          FROM covered_dates
         )
         -- select id of work packages covering the given days
-        SELECT DISTINCT id
-        FROM work_packages_days_of_week
-        WHERE dow IN (:days_of_week)
+        SELECT id FROM covered_dates_and_wday
+        WHERE dow IN (:days_of_week) OR date IN (:dates)
       SQL
 
-      sanitize_sql([sql, { days_of_week: }])
+      sanitize_sql([sql, { days_of_week:, dates: }])
     end
   end
 end

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

app/services/settings/working_days_update_service.rb

@@ -30,6 +30,8 @@ class Settings::WorkingDaysUpdateService < Settings::UpdateService
   def call(params)
     params = params.to_h.deep_symbolize_keys
     self.non_working_days_params = params.delete(:non_working_days) || []
+    self.previous_working_days = Setting[:working_days]
+    self.previous_non_working_days = NonWorkingDay.pluck(:date)
     super
   end
 
@@ -54,9 +56,19 @@ class Settings::WorkingDaysUpdateService < Settings::UpdateService
     results
   end
 
+  def after_perform(call)
+    super.tap do
+      WorkPackages::ApplyWorkingDaysChangeJob.perform_later(
+        user_id: User.current.id,
+        previous_working_days:,
+        previous_non_working_days:
+      )
+    end
+  end
+
   private
 
-  attr_accessor :non_working_days_params
+  attr_accessor :non_working_days_params, :previous_working_days, :previous_non_working_days
 
   def persist_non_working_days
     # We don't support update for now

app/workers/work_packages/apply_working_days_change_job.rb

@@ -30,18 +30,19 @@ class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob
   queue_with_priority :above_normal
   include ::ScheduledJob
 
-  def perform(user_id:, previous_working_days:)
+  def perform(user_id:, previous_working_days:, previous_non_working_days:)
     user = User.find(user_id)
 
     User.execute_as user do
-      updated_work_package_ids = collect_id_for_each(applicable_work_package(previous_working_days)) do |work_package|
+      updated_work_package_ids = collect_id_for_each(applicable_work_package(previous_working_days,
+                                                                             previous_non_working_days)) do |work_package|
         apply_change_to_work_package(user, work_package)
       end
       updated_work_package_ids += collect_id_for_each(applicable_predecessor(updated_work_package_ids)) do |predecessor|
         apply_change_to_predecessor(user, predecessor)
       end
 
-      set_journal_notice(updated_work_package_ids, previous_working_days)
+      set_journal_notice(updated_work_package_ids, previous_working_days, previous_non_working_days)
     end
   end
 
@@ -68,11 +69,11 @@ class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob
     end
   end
 
-  def applicable_work_package(previous_working_days)
-    changed_days = changed_days(previous_working_days)
-
+  def applicable_work_package(previous_working_days, previous_non_working_days)
+    days_of_week = changed_days(previous_working_days).keys
+    dates = changed_non_working_dates(previous_non_working_days).keys
     WorkPackage
-      .covering_days_of_week(changed_days)
+      .covering_dates_and_days_of_week(days_of_week:, dates:)
       .order(WorkPackage.arel_table[:start_date].asc.nulls_first,
              WorkPackage.arel_table[:due_date].asc)
   end
@@ -83,7 +84,16 @@ class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob
 
     # `^` is a Set method returning a new set containing elements exclusive to
     # each other
-    (previous ^ current).to_a
+    (previous ^ current).index_with { |day| current.include?(day) }
+  end
+
+  def changed_non_working_dates(previous_non_working_days)
+    previous = Set.new(previous_non_working_days)
+    current = Set.new(NonWorkingDay.pluck(:date))
+
+    # `^` is a Set method returning a new set containing elements exclusive to
+    # each other
+    (previous ^ current).index_with { |day| current.exclude?(day) }
   end
 
   def applicable_predecessor(excluded)
@@ -92,9 +102,10 @@ class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob
       .where.not(id: excluded)
   end
 
-  def set_journal_notice(updated_work_package_ids, previous_working_days)
-    day_changes = changed_days(previous_working_days).index_with { |day| Setting.working_days.include?(day) }
-    journal_note = journal_notice_text(day_changes)
+  def set_journal_notice(updated_work_package_ids, previous_working_days, previous_non_working_days)
+    day_changes = changed_days(previous_working_days)
+    date_changes = changed_non_working_dates(previous_non_working_days)
+    journal_note = journal_notice_text(day_changes, date_changes)
 
     WorkPackage
       .where(id: updated_work_package_ids.uniq)
@@ -105,10 +116,12 @@ class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob
     end
   end
 
-  def journal_notice_text(day_changes)
+  def journal_notice_text(day_changes, date_changes)
     I18n.with_locale(Setting.default_language) do
+      day_changes_messages = day_changes.collect { |day, working| working_day_change_message(day, working) }
+      date_changes_messages = date_changes.collect { |date, working| working_date_change_message(date, working) }
       I18n.t(:'working_days.journal_note.changed',
-             changes: day_changes.collect { |day, working| working_day_change_message(day, working) }.join(', '))
+             changes: (day_changes_messages + date_changes_messages).join(', '))
     end
   end
 
@@ -117,6 +130,10 @@ class WorkPackages::ApplyWorkingDaysChangeJob < ApplicationJob
            day: WeekDay.find_by!(day:).name)
   end
 
+  def working_date_change_message(date, working)
+    I18n.t(:"working_days.journal_note.dates.#{working ? :working : :non_working}", date:)
+  end
+
   def collect_id_for_each(scope)
     scope.pluck(:id).map do |id|
       yield(WorkPackage.find(id)).pluck(:id)

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

config/locales/en.yml

@@ -3160,6 +3160,9 @@ en:
       days:
         working: "%{day} is now working"
         non_working: "%{day} is now non-working"
+      dates:
+        working: "%{date} is now working"
+        non_working: "%{date} is now non-working"
   nothing_to_preview: "Nothing to preview"
 
   api_v3:

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/features/admin/working_days_spec.rb

@@ -161,7 +161,7 @@ describe 'Working Days', js: true do
     # delayed jobs are handled.
     ActiveJob::QueueAdapters::DelayedJobAdapter
       .new
-      .enqueue(WorkPackages::ApplyWorkingDaysChangeJob.new(user_id: 5, previous_working_days: []))
+      .enqueue(WorkPackages::ApplyWorkingDaysChangeJob.new(user_id: 5))
 
     uncheck 'Tuesday'
     click_on 'Save'

spec/models/work_packages/scopes/covering_dates_and_days_of_week_spec.rb

@@ -0,0 +1,192 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2022 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+require 'rails_helper'
+
+RSpec.describe WorkPackages::Scopes::CoveringDatesAndDaysOfWeek do
+  create_shared_association_defaults_for_work_package_factory
+
+  shared_let(:next_monday) { Date.current.next_occurring(:monday) }
+
+  shared_examples_for 'covering days' do
+    # Construct the keyword arguments for the `#covering_dates_and_days_of_week` method.
+    # It builds the expected day values based on the argument type provided (`:dates`, `:days_of_week`)
+    # and based on the day values provided.
+    # By providing the `expected_days`, the specific tetscase can decide which days are expected.
+    let(:day_args) do
+      ->(*expected_days) do
+        days.keys.reduce(Hash.new { |hsh, key| hsh[key] = [] }) do |day_args, dt|
+          Array(expected_days).each do |day|
+            day_args[dt] << days[dt][day] if days[dt][day]
+          end
+          day_args
+        end
+      end
+    end
+
+    it 'returns work packages having start date or due date being in the given days of week' do
+      schedule =
+        create_schedule(<<~CHART)
+          days         | MTWTFSS |
+          covered1     | XX      |
+          covered2     |  XX     |
+          covered3     |  X      |
+          covered4     |  [      |
+          covered5     |  ]      |
+          not_covered1 | X       |
+          not_covered2 |   X     |
+          not_covered3 |    XX   |
+          not_covered4 |         |
+        CHART
+
+      expect(WorkPackage.covering_dates_and_days_of_week(**day_args[:tuesday]))
+        .to contain_exactly(
+          schedule.work_package("covered1"),
+          schedule.work_package("covered2"),
+          schedule.work_package("covered3"),
+          schedule.work_package("covered4"),
+          schedule.work_package("covered5")
+        )
+    end
+
+    it 'returns work packages having days between start date and due date being in the given days of week' do
+      schedule =
+        create_schedule(<<~CHART)
+          days         | MTWTFSS |
+          covered1     | XXXX    |
+          covered2     |  XXX    |
+          not_covered1 |    XX   |
+          not_covered2 | X       |
+        CHART
+
+      expect(WorkPackage.covering_dates_and_days_of_week(**day_args[:tuesday, :wednesday]))
+        .to contain_exactly(
+          schedule.work_package("covered1"),
+          schedule.work_package("covered2")
+        )
+    end
+
+    context 'if work package ignores non working days' do
+      it 'does not returns it' do
+        create_schedule(<<~CHART)
+          days         | MTWTFSS |
+          not_covered  | XXXXXXX | working days include weekends
+        CHART
+
+        expect(WorkPackage.covering_dates_and_days_of_week(**day_args[:wednesday]))
+          .to eq([])
+      end
+    end
+
+    it 'does not return work packages having follows relation covering the given days of week' do
+      create_schedule(<<~CHART)
+        days         | MTWTFSS |
+        not_covered1 | X       |
+        follower1    |     X   | follows not_covered1
+        not_covered2 | X       |
+        follower2    |   X     | follows not_covered2
+      CHART
+
+      expect(WorkPackage.covering_dates_and_days_of_week(**day_args[:tuesday, :thursday]))
+        .to eq([])
+    end
+
+    it 'does not return work packages having follows relation with delay covering the given days of week' do
+      create_schedule(<<~CHART)
+        days         | MTWTFSS |
+        not_covered1 | X       |
+        follower1    |     X   | follows not_covered1 with delay 3
+        not_covered2 | X       |
+        follower2    |   X     | follows not_covered2 with delay 1
+      CHART
+
+      expect(WorkPackage.covering_dates_and_days_of_week(**day_args[:tuesday, :thursday]))
+        .to eq([])
+    end
+
+    it 'accepts a single day of week or an array of days' do
+      schedule =
+        create_schedule(<<~CHART)
+          days          | MTWTFSS |
+          covered       |  X      |
+          not_covered   | X       |
+        CHART
+
+      single_value = day_args[:tuesday].transform_values { |v| Array(v).first }
+
+      expect(WorkPackage.covering_dates_and_days_of_week(**single_value))
+        .to eq([schedule.work_package("covered")])
+      expect(WorkPackage.covering_dates_and_days_of_week(**day_args[:tuesday]))
+        .to eq([schedule.work_package("covered")])
+      expect(WorkPackage.covering_dates_and_days_of_week(**day_args[:tuesday, :wednesday]))
+        .to eq([schedule.work_package("covered")])
+    end
+  end
+
+  context 'with the days of week' do
+    let(:days)  do
+      {
+        days_of_week: {
+          tuesday: 2,
+          wednesday: 3,
+          thursday: 4
+        }
+      }
+    end
+
+    it_behaves_like 'covering days'
+  end
+
+  context 'with specific dates' do
+    let(:days) do
+      {
+        dates: {
+          tuesday: next_monday.next_occurring(:tuesday),
+          wednesday: next_monday.next_occurring(:wednesday),
+          thursday: next_monday.next_occurring(:thursday)
+        }
+      }
+    end
+
+    it_behaves_like 'covering days'
+  end
+
+  context 'with days of week and specific dates mixed' do
+    let(:days) do
+      {
+        days_of_week: { wednesday: 3 },
+        dates: {
+          tuesday: next_monday.next_occurring(:tuesday),
+          thursday: next_monday.next_occurring(:thursday)
+        }
+      }
+    end
+
+    it_behaves_like 'covering days'
+  end
+end

spec/models/work_packages/scopes/covering_days_of_week_spec.rb

@@ -1,129 +0,0 @@
-#-- copyright
-# OpenProject is an open source project management software.
-# Copyright (C) 2012-2023 the OpenProject GmbH
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License version 3.
-#
-# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
-# Copyright (C) 2006-2013 Jean-Philippe Lang
-# Copyright (C) 2010-2013 the ChiliProject Team
-#
-# This program is free software; you can redistribute it and/or
-# modify it under the terms of the GNU General Public License
-# as published by the Free Software Foundation; either version 2
-# of the License, or (at your option) any later version.
-#
-# This program is distributed in the hope that it will be useful,
-# but WITHOUT ANY WARRANTY; without even the implied warranty of
-# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
-# GNU General Public License for more details.
-#
-# You should have received a copy of the GNU General Public License
-# along with this program; if not, write to the Free Software
-# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
-#
-# See COPYRIGHT and LICENSE files for more details.
-#++
-
-require 'rails_helper'
-
-RSpec.describe WorkPackages::Scopes::CoveringDaysOfWeek do
-  create_shared_association_defaults_for_work_package_factory
-
-  it 'returns work packages having start date or due date being in the given days of week' do
-    schedule =
-      create_schedule(<<~CHART)
-        days         | MTWTFSS |
-        covered1     | XX      |
-        covered2     |  XX     |
-        covered3     |  X      |
-        covered4     |  [      |
-        covered5     |  ]      |
-        not_covered1 | X       |
-        not_covered2 |   X     |
-        not_covered3 |    XX   |
-        not_covered4 |         |
-      CHART
-
-    expect(WorkPackage.covering_days_of_week(2))
-      .to contain_exactly(
-        schedule.work_package("covered1"),
-        schedule.work_package("covered2"),
-        schedule.work_package("covered3"),
-        schedule.work_package("covered4"),
-        schedule.work_package("covered5")
-      )
-  end
-
-  it 'returns work packages having days between start date and due date being in the given days of week' do
-    schedule =
-      create_schedule(<<~CHART)
-        days         | MTWTFSS |
-        covered1     | XXXX    |
-        covered2     |  XXX    |
-        not_covered1 |    XX   |
-        not_covered2 | X       |
-      CHART
-
-    expect(WorkPackage.covering_days_of_week([2, 3]))
-      .to contain_exactly(
-        schedule.work_package("covered1"),
-        schedule.work_package("covered2")
-      )
-  end
-
-  context 'if work package ignores non working days' do
-    it 'does not returns it' do
-      create_schedule(<<~CHART)
-        days         | MTWTFSS |
-        not_covered  | XXXXXXX | working days include weekends
-      CHART
-
-      expect(WorkPackage.covering_days_of_week(3))
-        .to eq([])
-    end
-  end
-
-  it 'does not return work packages having follows relation covering the given days of week' do
-    create_schedule(<<~CHART)
-      days         | MTWTFSS |
-      not_covered1 | X       |
-      follower1    |     X   | follows not_covered1
-      not_covered2 | X       |
-      follower2    |   X     | follows not_covered2
-    CHART
-
-    expect(WorkPackage.covering_days_of_week([2, 4]))
-      .to eq([])
-  end
-
-  it 'does not return work packages having follows relation with delay covering the given days of week' do
-    create_schedule(<<~CHART)
-      days         | MTWTFSS |
-      not_covered1 | X       |
-      follower1    |     X   | follows not_covered1 with delay 3
-      not_covered2 | X       |
-      follower2    |   X     | follows not_covered2 with delay 1
-    CHART
-
-    expect(WorkPackage.covering_days_of_week([2, 4]))
-      .to eq([])
-  end
-
-  it 'accepts a single day of week or an array of days' do
-    schedule =
-      create_schedule(<<~CHART)
-        days          | MTWTFSS |
-        covered       |  X      |
-        not_covered   | X       |
-      CHART
-
-    expect(WorkPackage.covering_days_of_week(2))
-      .to eq([schedule.work_package("covered")])
-    expect(WorkPackage.covering_days_of_week([2]))
-      .to eq([schedule.work_package("covered")])
-    expect(WorkPackage.covering_days_of_week([2, 3]))
-      .to eq([schedule.work_package("covered")])
-  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

spec/services/settings/working_days_update_service_spec.rb

@@ -74,15 +74,39 @@ describe Settings::WorkingDaysUpdateService do
   describe '#call' do
     subject { instance.call(params) }
 
+    shared_examples 'successful working days settings call' do
+      include_examples 'successful call'
+
+      it 'calls the WorkPackages::ApplyWorkingDaysChangeJob' do
+        previous_working_days = Setting[:working_days]
+        previous_non_working_days = NonWorkingDay.pluck(:date)
+
+        allow(WorkPackages::ApplyWorkingDaysChangeJob).to receive(:perform_later)
+
+        subject
+
+        expect(WorkPackages::ApplyWorkingDaysChangeJob)
+          .to have_received(:perform_later)
+          .with(user_id: user.id, previous_working_days:, previous_non_working_days:)
+      end
+    end
+
     shared_examples 'unsuccessful working days settings call' do
       include_examples 'unsuccessful call'
 
       it 'does not persists the non working days' do
         expect { subject }.not_to change(NonWorkingDay, :count)
       end
+
+      it 'does not calls the WorkPackages::ApplyWorkingDaysChangeJob' do
+        allow(WorkPackages::ApplyWorkingDaysChangeJob).to receive(:perform_later)
+        subject
+
+        expect(WorkPackages::ApplyWorkingDaysChangeJob).not_to have_received(:perform_later)
+      end
     end
 
-    include_examples 'successful call'
+    include_examples 'successful working days settings call'
 
     context 'when non working days are present' do
       let!(:existing_nwd) { create(:non_working_day, name: 'Existing NWD') }
@@ -96,7 +120,7 @@ describe Settings::WorkingDaysUpdateService do
         ]
       end
 
-      include_examples 'successful call'
+      include_examples 'successful working days settings call'
 
       it 'persists (create/delete) the non working days' do
         expect { subject }.to change(NonWorkingDay, :count).by(1)
@@ -132,12 +156,23 @@ describe Settings::WorkingDaysUpdateService do
           context 'when deleting and re-creating the duplicate non-working day' do
             let(:non_working_days_params) do
               [
-                existing_nwd.slice(:id, :name, :date).merge('_destroy' => true),
-                existing_nwd.slice(:name, :date)
+                nwd_to_delete.slice(:id, :name, :date).merge('_destroy' => true),
+                nwd_to_delete.slice(:name, :date)
               ]
             end
 
-            include_examples 'successful call'
+            include_examples 'successful working days settings call'
+
+            it 'persists (create/delete) the non working days' do
+              expect { subject }.not_to change(NonWorkingDay, :count)
+              expect { nwd_to_delete.reload }.to raise_error(ActiveRecord::RecordNotFound)
+
+              # The nwd_to_delete is being re-created after the deletion.
+              expect(NonWorkingDay.all).to contain_exactly(
+                have_attributes(existing_nwd.slice(:name, :date)),
+                have_attributes(nwd_to_delete.slice(:name, :date))
+              )
+            end
           end
         end
       end

spec/services/work_packages/set_schedule_service_working_days_spec.rb

@@ -972,13 +972,6 @@ describe WorkPackages::SetScheduleService, 'working days' do
       end
     end
 
-    def set_non_working_week_days(*days)
-      non_working_days = days.map do |day|
-        %w[xxx monday tuesday wednesday thursday friday saturday sunday].index(day.downcase)
-      end
-      Setting.working_days -= non_working_days
-    end
-
     context 'when moving forward due to days and predecessor due date now being non-working days' do
       let_schedule(<<~CHART)
         days         | MTWTFSS |

spec/support/non_working_days.rb

@@ -0,0 +1,48 @@
+#-- copyright
+# OpenProject is an open source project management software.
+# Copyright (C) 2012-2023 the OpenProject GmbH
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License version 3.
+#
+# OpenProject is a fork of ChiliProject, which is a fork of Redmine. The copyright follows:
+# Copyright (C) 2006-2013 Jean-Philippe Lang
+# Copyright (C) 2010-2013 the ChiliProject Team
+#
+# This program is free software; you can redistribute it and/or
+# modify it under the terms of the GNU General Public License
+# as published by the Free Software Foundation; either version 2
+# of the License, or (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+#
+# See COPYRIGHT and LICENSE files for more details.
+#++
+
+def week_with_saturday_and_sunday_as_non_working_day(monday: Date.current.monday, weeks_size: 4)
+  weeks_size.times.map do |week_count|
+    [
+      create(:non_working_day, date: monday.next_occurring(:saturday) + week_count.weeks),
+      create(:non_working_day, date: monday.next_occurring(:sunday) + week_count.weeks)
+    ]
+  end.flatten.pluck(:date)
+end
+
+def week_without_non_working_days(monday: Date.current.monday, weeks_size: 4)
+  NonWorkingDay.where(date: monday...monday + weeks_size.weeks).destroy_all
+end
+
+def set_non_working_days(*dates)
+  dates.map { |date| create(:non_working_day, date:) }
+end
+
+def set_working_days(*dates)
+  NonWorkingDay.where(date: dates).destroy_all
+end