diff --git a/ee/app/graphql/mutations/incident_management/oncall_rotation/base.rb b/ee/app/graphql/mutations/incident_management/oncall_rotation/base.rb index 74452c606a9477eeefc5d6fb1e56bb8aab53cc63..cbdb2daebaacae6e95c62cc8111f9528d46308cd 100644 --- a/ee/app/graphql/mutations/incident_management/oncall_rotation/base.rb +++ b/ee/app/graphql/mutations/incident_management/oncall_rotation/base.rb @@ -92,18 +92,6 @@ def active_period_times(args) 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 diff --git a/ee/app/models/incident_management/oncall_rotation.rb b/ee/app/models/incident_management/oncall_rotation.rb index ddf0efb646f76a3f94662eebe79b4f2da55414de..e7334dda0b2ba7767038748b110ca7860819e910 100644 --- a/ee/app/models/incident_management/oncall_rotation.rb +++ b/ee/app/models/incident_management/oncall_rotation.rb @@ -11,10 +11,10 @@ def end_after_start? end_time > start_time if present? end - def for_date(date) + def for_dates(start_date, end_date) [ - 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 +44,6 @@ def for_date(date) 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 +97,6 @@ def valid_ends_at 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')) diff --git a/ee/lib/incident_management/oncall_shift_generator.rb b/ee/lib/incident_management/oncall_shift_generator.rb index c63b1a46080b199b77002ffca8c78ed1d9d21b8c..f1644c7e0280da1e7e6ab31358af3f0b6a506815 100644 --- a/ee/lib/incident_management/oncall_shift_generator.rb +++ b/ee/lib/incident_management/oncall_shift_generator.rb @@ -129,8 +129,15 @@ def shift_cycle_for(elapsed_shift_cycle_count, shift_cycle_starts_at) # 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 + start_date = shift_cycle_starts_at + shift_count.days - starts_at, ends_at = rotation.active_period.for_date(shift_cycle_starts_at + shift_count.days) + shift_start_date, shift_end_date = if rotation.active_period.end_after_start? + [start_date, start_date] + else + [start_date, start_date + 1.day] + end + + starts_at, ends_at = rotation.active_period.for_dates(shift_start_date, shift_end_date) shift_for(participant, [rotation.starts_at, starts_at].max, limit_end_time(ends_at)) end diff --git a/ee/spec/graphql/mutations/incident_management/oncall_rotation/create_spec.rb b/ee/spec/graphql/mutations/incident_management/oncall_rotation/create_spec.rb index 51c9bf3f0e9f71dd332b886f71273633d98ac824..2d719dcae5f11a94d7cac7d3ded9e2e73c88f4a6 100644 --- a/ee/spec/graphql/mutations/incident_management/oncall_rotation/create_spec.rb +++ b/ee/spec/graphql/mutations/incident_management/oncall_rotation/create_spec.rb @@ -124,8 +124,11 @@ 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 diff --git a/ee/spec/graphql/mutations/incident_management/oncall_rotation/update_spec.rb b/ee/spec/graphql/mutations/incident_management/oncall_rotation/update_spec.rb index 0d734acbb5275925eed9782fd165e8b45b926803..c16c3715a8f139d7d27d58942cc417f436523646 100644 --- a/ee/spec/graphql/mutations/incident_management/oncall_rotation/update_spec.rb +++ b/ee/spec/graphql/mutations/incident_management/oncall_rotation/update_spec.rb @@ -155,8 +155,11 @@ 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 diff --git a/ee/spec/lib/incident_management/oncall_shift_generator_spec.rb b/ee/spec/lib/incident_management/oncall_shift_generator_spec.rb index 946e287bb643c4e3870bcd5f96ef8254ff1b0953..67c5326f5b489b833467eb88ae9fc97e2e7ad66f 100644 --- a/ee/spec/lib/incident_management/oncall_shift_generator_spec.rb +++ b/ee/spec/lib/incident_management/oncall_shift_generator_spec.rb @@ -93,10 +93,13 @@ [: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 @@ [: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 diff --git a/ee/spec/models/incident_management/oncall_rotation_spec.rb b/ee/spec/models/incident_management/oncall_rotation_spec.rb index cc880d8e214012693943cc0a83d26b5d00e8d031..9851d3a476c87e37e8acf28990e8210b805b7f96 100644 --- a/ee/spec/models/incident_management/oncall_rotation_spec.rb +++ b/ee/spec/models/incident_management/oncall_rotation_spec.rb @@ -93,16 +93,6 @@ 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 diff --git a/ee/spec/requests/api/issues_spec.rb b/ee/spec/requests/api/issues_spec.rb index a9a5ce148e798586ba854e008b0c90b6bba4ed25..1ca3975e8989e78b9a9fa31429b9fab9a7886ede 100644 --- a/ee/spec/requests/api/issues_spec.rb +++ b/ee/spec/requests/api/issues_spec.rb @@ -10,7 +10,7 @@ let_it_be(:group) { create(:group) } let_it_be(:epic) { create(:epic, group: group) } - let_it_be(:group_project) { create(:project, :public, creator_id: user.id, namespace: group) } + let_it_be(:group_project) { create( :project, :public, creator_id: user.id, namespace: group) } let(:user2) { create(:user) } diff --git a/ee/spec/services/incident_management/oncall_rotations/create_service_spec.rb b/ee/spec/services/incident_management/oncall_rotations/create_service_spec.rb index 2048f2d7cfe3fdbd286a0713e6e85c97e5b0368d..41ee62452810f0554df71f560e9189eb10da95a7 100644 --- a/ee/spec/services/incident_management/oncall_rotations/create_service_spec.rb +++ b/ee/spec/services/incident_management/oncall_rotations/create_service_spec.rb @@ -163,6 +163,14 @@ 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 @@ 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 diff --git a/ee/spec/support/helpers/oncall_helpers.rb b/ee/spec/support/helpers/oncall_helpers.rb index 0a6792362a9d0640eade574549543df8a15f8aaa..b501a6c901a0f8501a63f241de905b2a1d628376 100644 --- a/ee/spec/support/helpers/oncall_helpers.rb +++ b/ee/spec/support/helpers/oncall_helpers.rb @@ -4,6 +4,6 @@ module OncallHelpers def active_period_for_date_with_tz(date, rotation) date = date.in_time_zone(rotation.schedule.timezone) - rotation.active_period.for_date(date) + rotation.active_period.for_dates(date, date) end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 2b62445f1823f76b6ff1e7189d31dc3f634b1355..608731e94b3e00c81a957aeee640edeff8a61efd 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -36566,9 +36566,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 ""