Merge branch '322079-allow-overnight-oncall-shifts' into 'master'

Allow overnight restricted shift times

See merge request gitlab-org/gitlab!57202

ee/app/graphql/mutations/incident_management/oncall_rotation/base.rb

@@ -92,18 +92,6 @@ module Mutations
           raise invalid_time_error unless TIME_FORMAT.match?(start_time)
           raise invalid_time_error unless TIME_FORMAT.match?(end_time)
 
-          # We parse the times into dates to compare.
-          # Time.parse parses a timestamp into a Time with todays date
-          # Time.parse("22:11") => 2021-02-23 22:11:00 +0000
-          parsed_from = Time.parse(start_time)
-          parsed_to = Time.parse(end_time)
-
-          # Overnight shift times will be supported via
-          # https://gitlab.com/gitlab-org/gitlab/-/issues/322079
-          if parsed_to < parsed_from
-            raise ::Gitlab::Graphql::Errors::ArgumentError, "'start_time' time must be before 'end_time' time"
-          end
-
           [start_time, end_time]
         end
 

ee/app/models/incident_management/oncall_rotation.rb

@@ -11,10 +11,12 @@ module IncidentManagement
         end_time > start_time if present?
       end
 
-      def for_date(date)
+      def for_date(start_date)
+        end_date = end_after_start? ? start_date : start_date.next_day
+
         [
-          date.change(hour: start_time.hour, min: start_time.min),
-          date.change(hour: end_time.hour, min: end_time.min)
+          start_date.change(hour: start_time.hour, min: start_time.min),
+          end_date.change(hour: end_time.hour, min: end_time.min)
         ]
       end
     end
@@ -44,7 +46,6 @@ module IncidentManagement
 
     validates :active_period_start, presence: true, if: :active_period_end
     validates :active_period_end, presence: true, if: :active_period_start
-    validate :active_period_end_after_start, if: :active_period_start
     validate :no_active_period_for_hourly_shifts, if: :hours?
 
     scope :in_progress, -> { where('starts_at < :time AND (ends_at > :time OR ends_at IS NULL)', time: Time.current) }
@@ -98,13 +99,6 @@ module IncidentManagement
       errors.add(:ends_at, s_('must be after start')) if ends_at <= starts_at
     end
 
-    def active_period_end_after_start
-      return unless active_period.present?
-      return if active_period.end_after_start?
-
-      errors.add(:active_period_end, _('must be later than active period start'))
-    end
-
     def no_active_period_for_hourly_shifts
       if active_period_start || active_period_end
         errors.add(:length_unit, _('Restricted shift times are not available for hourly shifts'))

ee/changelogs/322079-allow-overnight-oncall-shifts.yml

+---
+title: Allow overnight shifts for oncall rotations
+merge_request: 57202
+author:
+type: added

ee/lib/incident_management/oncall_shift_generator.rb

@@ -129,8 +129,8 @@ module IncidentManagement
           #     expected_shift_count = 14          -> pretend it's a 2-week rotation
           #     shift_count = 2                    -> we're calculating the shift for the 3rd day
           # starts_at = Monday 00:00:00 + 8.hours + 2.days => Thursday 08:00:00
-
-          starts_at, ends_at = rotation.active_period.for_date(shift_cycle_starts_at + shift_count.days)
+          start_date = shift_cycle_starts_at + shift_count.days
+          starts_at, ends_at = rotation.active_period.for_date(start_date)
 
           shift_for(participant, [rotation.starts_at, starts_at].max, limit_end_time(ends_at))
         end

ee/spec/graphql/mutations/incident_management/oncall_rotation/create_spec.rb

@@ -124,8 +124,11 @@ RSpec.describe Mutations::IncidentManagement::OncallRotation::Create do
           let(:start_time) { '17:00' }
           let(:end_time) { '08:00' }
 
-          it 'raises an error' do
-            expect { resolve }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, "'start_time' time must be before 'end_time' time")
+          it 'saves the on-call rotation with active period times' do
+            rotation = resolve[:oncall_rotation]
+
+            expect(rotation.active_period_start.strftime('%H:%M')).to eql('17:00')
+            expect(rotation.active_period_end.strftime('%H:%M')).to eql('08:00')
           end
         end
 

ee/spec/graphql/mutations/incident_management/oncall_rotation/update_spec.rb

@@ -155,8 +155,11 @@ RSpec.describe Mutations::IncidentManagement::OncallRotation::Update do
           let(:start_time) { '17:00' }
           let(:end_time) { '08:00' }
 
-          it 'raises an error' do
-            expect { resolve }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, "'start_time' time must be before 'end_time' time")
+          it 'saves the on-call rotation with active period times' do
+            rotation = resolve[:oncall_rotation]
+
+            expect(rotation.active_period_start.strftime('%H:%M')).to eql('17:00')
+            expect(rotation.active_period_end.strftime('%H:%M')).to eql('08:00')
           end
         end
 

ee/spec/lib/incident_management/oncall_shift_generator_spec.rb

@@ -93,10 +93,13 @@ RSpec.describe IncidentManagement::OncallShiftGenerator do
          [:participant3, '2020-12-18 00:00:00 UTC', '2020-12-23 00:00:00 UTC']]
 
       context 'with shift active period times set' do
+        let(:active_period_start) { "08:00" }
+        let(:active_period_end) { "17:00" }
+
         before do
           rotation.update!(
-            active_period_start: "08:00",
-            active_period_end: "17:00"
+            active_period_start: active_period_start,
+            active_period_end: active_period_end
           )
         end
 
@@ -165,6 +168,28 @@ RSpec.describe IncidentManagement::OncallShiftGenerator do
              [:participant2, '2020-12-16 08:00:00 UTC', '2020-12-16 17:00:00 UTC'],
              [:participant2, '2020-12-17 08:00:00 UTC', '2020-12-17 17:00:00 UTC']]
         end
+
+        context 'active period is overnight' do
+          let(:active_period_start) { "17:00" }
+          let(:active_period_end) { "08:00" }
+
+          it 'splits the shifts daily by each active period' do
+            expect(shifts.count).to eq (ends_at.to_date - starts_at.to_date).to_i
+          end
+
+          it_behaves_like 'unsaved shifts',
+          '5 shifts for each participant with overnight shifts',
+          [[:participant1, '2020-12-08 17:00:00 UTC', '2020-12-09 08:00:00 UTC'],
+           [:participant1, '2020-12-09 17:00:00 UTC', '2020-12-10 08:00:00 UTC'],
+           [:participant1, '2020-12-10 17:00:00 UTC', '2020-12-11 08:00:00 UTC'],
+           [:participant1, '2020-12-11 17:00:00 UTC', '2020-12-12 08:00:00 UTC'],
+           [:participant1, '2020-12-12 17:00:00 UTC', '2020-12-13 08:00:00 UTC'],
+           [:participant2, '2020-12-13 17:00:00 UTC', '2020-12-14 08:00:00 UTC'],
+           [:participant2, '2020-12-14 17:00:00 UTC', '2020-12-15 08:00:00 UTC'],
+           [:participant2, '2020-12-15 17:00:00 UTC', '2020-12-16 08:00:00 UTC'],
+           [:participant2, '2020-12-16 17:00:00 UTC', '2020-12-17 08:00:00 UTC'],
+           [:participant2, '2020-12-17 17:00:00 UTC', '2020-12-18 08:00:00 UTC']]
+        end
       end
 
       context 'when end time is earlier than start time' do

ee/spec/models/incident_management/oncall_rotation_spec.rb

@@ -93,16 +93,6 @@ RSpec.describe IncidentManagement::OncallRotation do
           expect(subject.errors.full_messages).to include(/Restricted shift times are not available for hourly shifts/)
         end
       end
-
-      context 'end time before start time' do
-        it 'raises a validation error if an active period is set' do
-          subject.active_period_start = '17:00'
-          subject.active_period_end = '08:00'
-
-          expect(subject.valid?).to eq(false)
-          expect(subject.errors.full_messages).to include('Active period end must be later than active period start')
-        end
-      end
     end
   end
 

ee/spec/services/incident_management/oncall_rotations/create_service_spec.rb

@@ -163,6 +163,14 @@ RSpec.describe IncidentManagement::OncallRotations::CreateService do
         it_behaves_like 'successfully creates rotation'
         it_behaves_like 'saved the active period times'
 
+        context 'when end active time is before start active time' do
+          let(:active_period_start) { '17:00' }
+          let(:active_period_end) { '08:00' }
+
+          it_behaves_like 'successfully creates rotation'
+          it_behaves_like 'saved the active period times'
+        end
+
         context 'when only active period end time is set' do
           let(:active_period_start) { nil }
 
@@ -174,13 +182,6 @@ RSpec.describe IncidentManagement::OncallRotations::CreateService do
 
           it_behaves_like 'error response', "Active period end can't be blank"
         end
-
-        context 'when end active time is before start active time' do
-          let(:active_period_start) { '17:00' }
-          let(:active_period_end) { '08:00' }
-
-          it_behaves_like 'error response', "Active period end must be later than active period start"
-        end
       end
 
       context 'for an in-progress rotation' do

locale/gitlab.pot

@@ -36543,9 +36543,6 @@ msgstr ""
 msgid "must be greater than start date"
 msgstr ""
 
-msgid "must be later than active period start"
-msgstr ""
-
 msgid "must contain only valid frameworks"
 msgstr ""