Merge branch '324712-update-rotation-active-period-after-schedule-tz-change' into 'master'

Update rotation active period after schedule changes

See merge request gitlab-org/gitlab!57069

doc/operations/incident_management/index.md

@@ -13,6 +13,7 @@ generated by their application. By surfacing alerts and incidents where the code
 being developed, efficiency and awareness can be increased. Check out the following sections for more information:
 
 - [Integrate your monitoring tools](integrations.md).
-- Receive [notifications](paging.md) for triggered alerts.
+- Manage [on-call schedules](oncall_schedules.md) and receive [notifications](paging.md) for
+  triggered alerts.
 - Triage [Alerts](alerts.md) and [Incidents](incidents.md).
 - Inform stakeholders with [Status Page](status_page.md).

doc/operations/incident_management/oncall_schedules.md

@@ -37,7 +37,7 @@ create [rotations](#rotations) for your schedule.
 
 ![Schedule Empty Grid](img/oncall_schedule_empty_grid_v13_10.png)
 
-### Update a schedule
+### Edit a schedule
 
 Follow these steps to update a schedule:
 
@@ -46,6 +46,9 @@ Follow these steps to update a schedule:
 1. In the **Edit schedule** form, edit the information you wish to update.
 1. Click the **Edit schedule** button to save your changes.
 
+If you change the schedule's time zone, GitLab automatically updates the rotation's restricted time
+interval (if one is set) to the corresponding times in the new time zone.
+
 ### Delete a schedule
 
 Follow these steps to delete a schedule:
@@ -70,8 +73,8 @@ Follow these steps to create a rotation:
    - **Starts on:** The date and time the rotation begins.
    - **Enable end date:** With the toggle set to on, you can select the date and time your rotation
      ends.
-   - **Restrict to time intervals:** With the toggle set to on, you can restrict your rotation to
-     the time period you select.
+   - **Restrict to time intervals:** With the toggle set to on, you can restrict your rotation to the
+     time period you select.
 
 ### Edit a rotation
 

ee/app/models/incident_management/oncall_rotation.rb

@@ -50,6 +50,7 @@ module IncidentManagement
 
     scope :in_progress, -> { where('starts_at < :time AND (ends_at > :time OR ends_at IS NULL)', time: Time.current) }
     scope :except_ids, -> (ids) { where.not(id: ids) }
+    scope :with_active_period, -> { where.not(active_period_start: nil) }
     scope :with_shift_generation_associations, -> do
       joins(:active_participants, :schedule)
         .distinct

ee/app/services/incident_management/oncall_schedules/update_service.rb

@@ -8,6 +8,7 @@ module IncidentManagement
       # @param params [Hash]
       def initialize(oncall_schedule, user, params)
         @oncall_schedule = oncall_schedule
+        @original_schedule_timezone = oncall_schedule.timezone
         @user = user
         @params = params
         @project = oncall_schedule.project
@@ -17,16 +18,55 @@ module IncidentManagement
         return error_no_license unless available?
         return error_no_permissions unless allowed?
 
-        if oncall_schedule.update(params)
-          success(oncall_schedule)
-        else
-          error(oncall_schedule.errors.full_messages.to_sentence)
+        IncidentManagement::OncallSchedule.transaction do
+          oncall_schedule.update!(params)
+          update_rotations!
         end
+
+        success(oncall_schedule)
+      rescue ActiveRecord::RecordInvalid => e
+        error(e.record.errors.full_messages.to_sentence)
+      rescue StandardError => e
+        error(e.message)
       end
 
       private
 
-      attr_reader :oncall_schedule, :user, :params, :project
+      attr_reader :oncall_schedule, :original_schedule_timezone, :user, :params, :project
+
+      def update_rotations!
+        return if same_schedule_timezone?
+
+        update_rotation_active_periods!
+      end
+
+      def same_schedule_timezone?
+        original_schedule_timezone == oncall_schedule.timezone
+      end
+
+      # Converts & updates the active period to the new timezone
+      # Ex: 8:00 - 17:00 Europe/Berlin becomes 6:00 - 15:00 UTC
+      def update_rotation_active_periods!
+        original_schedule_current_time = Time.current.in_time_zone(original_schedule_timezone)
+
+        oncall_schedule.rotations.with_active_period.each do |rotation|
+          active_period = rotation.active_period.for_date(original_schedule_current_time)
+          new_start_time, new_end_time = active_period.map { |time| time.in_time_zone(oncall_schedule.timezone).strftime('%H:%M') }
+
+          service = IncidentManagement::OncallRotations::EditService.new(
+            rotation,
+            user,
+            {
+              active_period_start: new_start_time,
+              active_period_end: new_end_time
+            }
+          )
+
+          response = service.execute
+
+          raise response.message if response.error?
+        end
+      end
 
       def error_no_permissions
         error(_('You have insufficient permissions to update an on-call schedule for this project'))

ee/changelogs/unreleased/324712-update-rotation-active-period-after-schedule-tz-change.yml

+---
+title: Update oncall rotation active period after schedule timezone update
+merge_request: 57069
+author:
+type: added

ee/spec/models/incident_management/oncall_rotation_spec.rb

@@ -107,6 +107,18 @@ RSpec.describe IncidentManagement::OncallRotation do
 
       it { is_expected.to contain_exactly(rotation_1, rotation_2) }
     end
+
+    describe '.with_active_period' do
+      subject { described_class.with_active_period }
+
+      it { is_expected.to be_empty }
+
+      context 'rotation has active period' do
+        let(:rotation) { create(:incident_management_oncall_rotation, :with_active_period, schedule: schedule) }
+
+        it { is_expected.to contain_exactly(rotation) }
+      end
+    end
   end
 
   describe '#shift_cycle_duration' do

ee/spec/services/incident_management/oncall_schedules/update_service_spec.rb

@@ -6,10 +6,11 @@ RSpec.describe IncidentManagement::OncallSchedules::UpdateService do
   let_it_be(:user_with_permissions) { create(:user) }
   let_it_be(:user_without_permissions) { create(:user) }
   let_it_be_with_refind(:project) { create(:project) }
-  let_it_be_with_reload(:oncall_schedule) { create(:incident_management_oncall_schedule, project: project) }
+  let_it_be_with_reload(:oncall_schedule) { create(:incident_management_oncall_schedule, :utc, project: project) }
 
   let(:current_user) { user_with_permissions }
-  let(:params) { { name: 'Updated name', description: 'Updated description', timezone: 'America/New_York' } }
+  let(:new_timezone) { 'America/New_York' }
+  let(:params) { { name: 'Updated name', description: 'Updated description', timezone: new_timezone } }
   let(:service) { described_class.new(oncall_schedule, current_user, params) }
 
   before do
@@ -68,6 +69,90 @@ RSpec.describe IncidentManagement::OncallSchedules::UpdateService do
         expect(oncall_schedule.description).to eq(params[:description])
         expect(oncall_schedule.timezone).to eq(params[:timezone])
       end
+
+      context 'schedule has a rotation' do
+        # Setting fixed timezone for rotation active period updates
+        around do |example|
+          travel_to Time.utc(2021, 03, 22, 0, 0)
+
+          example.run
+        end
+
+        let_it_be_with_reload(:oncall_rotation) { create(:incident_management_oncall_rotation, :with_active_period, schedule: oncall_schedule) }
+
+        shared_examples 'updates the rotation active periods' do |new_start_time, new_end_time|
+          it 'updates the rotation active periods with new timezone' do
+            initial_start_time = oncall_rotation.reload.attributes_before_type_cast['active_period_start']
+            initial_end_time = oncall_rotation.attributes_before_type_cast['active_period_end']
+
+            expect { execute }.to change { oncall_rotation.reload.attributes_before_type_cast['active_period_start'] }.from(initial_start_time).to("#{new_start_time}:00")
+             .and change { oncall_rotation.reload.attributes_before_type_cast['active_period_end'] }.from(initial_end_time).to("#{new_end_time}:00")
+             .and change { oncall_schedule.reload.timezone }.to(new_timezone)
+          end
+        end
+
+        # This expects the active periods are updated according to the date above (22nd March, 2021 in the new timezone).
+        it_behaves_like 'updates the rotation active periods', '04:00', '13:00'
+
+        context 'from non-overnight shifts to overnight' do
+          let(:new_timezone) { 'Pacific/Auckland' }
+
+          it_behaves_like 'updates the rotation active periods', '21:00', '06:00'
+        end
+
+        context 'from overnight shifts to non-overnight' do
+          before do
+            oncall_rotation.update!(active_period_start: '21:00', active_period_end: '06:00')
+          end
+
+          let(:new_timezone) { 'Pacific/Auckland' }
+
+          it_behaves_like 'updates the rotation active periods', '10:00', '19:00'
+        end
+
+        context 'new timezone has non-whole-hour change' do
+          let(:new_timezone) { 'Asia/Kolkata' }
+
+          it_behaves_like 'updates the rotation active periods', '13:30', '22:30'
+        end
+
+        context 'new timezone but same offset' do
+          let(:new_timezone) { 'Europe/London' }
+
+          it 'updates the timezone' do
+            expect { execute }.to change { oncall_schedule.reload.timezone }.to(new_timezone)
+          end
+
+          it 'does not update the active period times' do
+            expect { execute }.to not_change { time_from_time_column(oncall_rotation.reload.active_period_start) }
+             .and not_change { time_from_time_column(oncall_rotation.active_period_end) }
+          end
+        end
+
+        context 'timezone is not changed' do
+          before do
+            params.delete(:timezone)
+          end
+
+          it 'does not update rotations' do
+            expect { execute }.to not_change { oncall_rotation.updated_at }
+          end
+        end
+
+        context 'error updating' do
+          before do
+            allow_next_instance_of(IncidentManagement::OncallRotations::EditService) do |edit_service|
+              allow(edit_service).to receive(:execute).and_return(double(error?: true, message: 'Test something went wrong'))
+            end
+          end
+
+          it_behaves_like 'error response', 'Test something went wrong'
+        end
+      end
+
+      def time_from_time_column(attribute)
+        attribute.strftime('%H:%M')
+      end
     end
   end
 end