Merge branch '262859-restrict-oncall-rotation-to-interval' into 'master'

Restrict on-call to certain times during rotations

See merge request gitlab-org/gitlab!52998

changelogs/unreleased/262859-restrict-oncall-rotation-to-interval.yml

+---
+title: Add active period columns to on-call rotations.
+merge_request: 52998
+author:
+type: added

db/migrate/20210201034649_add_active_periods_to_on_call_rotations.rb

+# frozen_string_literal: true
+
+class AddActivePeriodsToOnCallRotations < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :incident_management_oncall_rotations, :active_period_start, :time, null: true
+    add_column :incident_management_oncall_rotations, :active_period_end, :time, null: true
+  end
+end

db/schema_migrations/20210201034649

+e5492820a8618d5599429ece04ea941e869c84c22d213d536644bcefc5775363
\ No newline at end of file

db/structure.sql

@@ -13217,6 +13217,8 @@ CREATE TABLE incident_management_oncall_rotations (
     starts_at timestamp with time zone NOT NULL,
     name text NOT NULL,
     ends_at timestamp with time zone,
+    active_period_start time without time zone,
+    active_period_end time without time zone,
     CONSTRAINT check_5209fb5d02 CHECK ((char_length(name) <= 200))
 );
 

doc/api/graphql/reference/index.md

@@ -2240,6 +2240,7 @@ Describes an incident management on-call rotation.
 
 | Field | Type | Description |
 | ----- | ---- | ----------- |
+| `activePeriod` | OncallRotationActivePeriodType | Active period for the on-call rotation. |
 | `endsAt` | Time | End date and time of the on-call rotation. |
 | `id` | IncidentManagementOncallRotationID! | ID of the on-call rotation. |
 | `length` | Int | Length of the on-call schedule, in the units specified by lengthUnit. |
@@ -2948,6 +2949,15 @@ The rotation participant and color palette.
 | `id` | IncidentManagementOncallParticipantID! | ID of the on-call participant. |
 | `user` | User! | The user who is participating. |
 
+### OncallRotationActivePeriodType
+
+Active period time range for on-call rotation.
+
+| Field | Type | Description |
+| ----- | ---- | ----------- |
+| `endTime` | String | The end of the rotation active period. |
+| `startTime` | String | The start of the rotation active period. |
+
 ### OncallRotationCreatePayload
 
 Autogenerated return type of OncallRotationCreate.

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

@@ -33,12 +33,17 @@ module Mutations
                  required: true,
                  description: 'The rotation length of the on-call rotation.'
 
+        argument :active_period, Types::IncidentManagement::OncallRotationActivePeriodInputType,
+                 required: false,
+                 description: 'The active period of time that the on-call rotation should take place.'
+
         argument :participants,
                  [Types::IncidentManagement::OncallUserInputType],
                  required: true,
                  description: 'The usernames of users participating in the on-call rotation.'
 
         MAXIMUM_PARTICIPANTS = 100
+        TIME_FORMAT = /^(0\d|1\d|2[0-3]):[0-5]\d$/.freeze
 
         def resolve(iid:, project_path:, participants:, **args)
           project = Project.find_by_full_path(project_path)
@@ -72,12 +77,16 @@ module Mutations
           starts_at = parse_datetime(schedule, args[:starts_at])
           ends_at = parse_datetime(schedule, args[:ends_at]) if args[:ends_at]
 
+          active_period_start, active_period_end = active_period_times(args)
+
           args.slice(:name).merge(
             length: rotation_length,
             length_unit: rotation_length_unit,
             starts_at: starts_at,
             ends_at: ends_at,
-            participants: find_participants(participants)
+            participants: find_participants(participants),
+            active_period_start: active_period_start,
+            active_period_end: active_period_end
           )
         end
 
@@ -99,6 +108,32 @@ module Mutations
           user_array.map.with_index { |param, i| param.to_h.merge(user: matched_users[i]) }
         end
 
+        def active_period_times(args)
+          active_period_args = args.dig(:active_period)
+
+          return [nil, nil] if active_period_args.blank?
+
+          start_time = active_period_args[:start_time]
+          end_time = active_period_args[:end_time]
+
+          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
+
         def raise_project_not_found
           raise Gitlab::Graphql::Errors::ArgumentError, 'The project could not be found'
         end
@@ -118,6 +153,10 @@ module Mutations
         def raise_user_not_found
           raise Gitlab::Graphql::Errors::ArgumentError, "A provided username couldn't be matched to a user"
         end
+
+        def invalid_time_error
+          ::Gitlab::Graphql::Errors::ArgumentError.new 'Time given is invalid'
+        end
       end
     end
   end

ee/app/graphql/types/incident_management/oncall_rotation_active_period_input_type.rb

+# frozen_string_literal: true
+
+module Types
+  module IncidentManagement
+    # rubocop: disable Graphql/AuthorizeTypes
+    class OncallRotationActivePeriodInputType < BaseInputObject
+      graphql_name 'OncallRotationActivePeriodInputType'
+      description 'Active period time range for on-call rotation'
+
+      argument :start_time, GraphQL::STRING_TYPE,
+                required: true,
+                description: 'The start of the rotation active period.'
+
+      argument :end_time, GraphQL::STRING_TYPE,
+                required: true,
+                description: 'The end of the rotation active period.'
+    end
+    # rubocop: enable Graphql/AuthorizeTypes
+  end
+end

ee/app/graphql/types/incident_management/oncall_rotation_active_period_type.rb

+# frozen_string_literal: true
+
+module Types
+  module IncidentManagement
+    # rubocop: disable Graphql/AuthorizeTypes
+    class OncallRotationActivePeriodType < BaseObject
+      graphql_name 'OncallRotationActivePeriodType'
+      description 'Active period time range for on-call rotation'
+
+      field :start_time, GraphQL::STRING_TYPE,
+            null: true,
+            description: 'The start of the rotation active period.'
+
+      field :end_time, GraphQL::STRING_TYPE,
+            null: true,
+            description: 'The end of the rotation active period.'
+
+      alias_method :active_period, :object
+
+      def start_time
+        active_period.start_time&.strftime('%H:%M')
+      end
+
+      def end_time
+        active_period.end_time&.strftime('%H:%M')
+      end
+    end
+    # rubocop: enable Graphql/AuthorizeTypes
+  end
+end

ee/app/graphql/types/incident_management/oncall_rotation_type.rb

@@ -38,6 +38,11 @@ module Types
             null: true,
             description: 'Unit of the on-call rotation length.'
 
+      field :active_period,
+            Types::IncidentManagement::OncallRotationActivePeriodType,
+            null: true,
+            description: 'Active period for the on-call rotation.'
+
       field :participants,
             ::Types::IncidentManagement::OncallParticipantType.connection_type,
             null: true,

ee/app/models/incident_management/oncall_rotation.rb

@@ -2,6 +2,23 @@
 
 module IncidentManagement
   class OncallRotation < ApplicationRecord
+    ActivePeriod = Struct.new(:start_time, :end_time) do
+      def present?
+        start_time && end_time
+      end
+
+      def end_after_start?
+        end_time > start_time if present?
+      end
+
+      def for_date(date)
+        [
+          date.change(hour: start_time.hour, min: start_time.min),
+          date.change(hour: end_time.hour, min: end_time.min)
+        ]
+      end
+    end
+
     self.table_name = 'incident_management_oncall_rotations'
 
     enum length_unit: {
@@ -24,6 +41,11 @@ module IncidentManagement
     validates :length_unit, presence: true
     validate :valid_ends_at, if: -> { ends_at && starts_at }
 
+    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) }
     scope :except_ids, -> (ids) { where.not(id: ids) }
     scope :with_shift_generation_associations, -> do
@@ -39,15 +61,46 @@ module IncidentManagement
       joins(shifts: { participant: :user }).pluck(:id, 'users.id')
     end
 
-    def shift_duration
+    # The duration of a shift cycle, which is the time until the next participant is on-call.
+    # If a shift active period is setup then many shifts will be within a shift_cycle_duration.
+    def shift_cycle_duration
       # As length_unit is an enum, input is guaranteed to be appropriate
       length.public_send(length_unit) # rubocop:disable GitlabSecurity/PublicSend
     end
 
+    # If there is an active period, and the rotation unit is weeks, we multiple the
+    # count of shifts per cycle by 7, as there are 7 days in a week.
+    def shifts_per_cycle
+      return 1 unless has_shift_active_period?
+
+      weeks? ? (7 * length) : length
+    end
+
+    def active_period
+      ActivePeriod.new(active_period_start, active_period_end)
+    end
+
+    def has_shift_active_period?
+      !hours? && active_period.present?
+    end
+
     private
 
     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'))
+      end
+    end
   end
 end

ee/app/services/incident_management/oncall_rotations/create_service.rb

@@ -14,6 +14,8 @@ module IncidentManagement
       # @param params - length_unit [String] The unit of the rotation length. (One of 'hours', days', 'weeks')
       # @param params - starts_at [DateTime] The datetime the rotation starts on.
       # @param params - ends_at [DateTime] The datetime the rotation ends on.
+      # @param params - active_period_start [String] The time the on-call shifts should start, for example: "08:00"
+      # @param params - active_period_end [String] The time the on-call shifts should end, for example: "17:00"
       # @param params - participants [Array<hash>] An array of hashes defining participants of the on-call rotations.
       # @option opts  - participant [User] The user who is part of the rotation
       # @option opts  - color_palette [String] The color palette to assign to the on-call user, for example: "blue".

ee/changelogs/unreleased/262859-restrict-oncall-rotation-to-interval.yml

+---
+title: Restrict on-call to certain times during rotations via GraphQL
+merge_request: 52998
+author:
+type: added

ee/lib/incident_management/oncall_shift_generator.rb

@@ -23,16 +23,19 @@ module IncidentManagement
 
       # The first shift within the timeframe may begin before
       # the timeframe. We want to begin generating shifts
-      # based on the actual start time of the shift.
-      elapsed_shift_count = elapsed_whole_shifts(starts_at)
-      shift_starts_at = shift_start_time(elapsed_shift_count)
+      # based on the actual start time of the shift cycle.
+      elapsed_shift_cycle_count = elapsed_whole_shift_cycles(starts_at)
+      shift_cycle_starts_at = shift_cycle_start_time(elapsed_shift_cycle_count)
       shifts = []
 
-      while shift_starts_at < ends_at
-        shifts << shift_for(elapsed_shift_count, shift_starts_at)
+      while shift_cycle_starts_at < ends_at
+        new_shifts = Array(shift_cycle_for(elapsed_shift_cycle_count, shift_cycle_starts_at))
+        new_shifts = remove_out_of_bounds_shifts(new_shifts, shift_cycle_starts_at, starts_at, ends_at)
 
-        shift_starts_at += shift_duration
-        elapsed_shift_count += 1
+        shifts.concat(new_shifts)
+
+        shift_cycle_starts_at += shift_cycle_duration
+        elapsed_shift_cycle_count += 1
       end
 
       shifts
@@ -49,27 +52,29 @@ module IncidentManagement
       return if rotation_ends_at && rotation_ends_at <= timestamp
       return unless rotation.participants.any?
 
-      elapsed_shift_count = elapsed_whole_shifts(timestamp)
-      shift_starts_at = shift_start_time(elapsed_shift_count)
+      elapsed_shift_cycle_count = elapsed_whole_shift_cycles(timestamp)
+      shift_cycle_starts_at = shift_cycle_start_time(elapsed_shift_cycle_count)
+
+      new_shifts = Array(shift_cycle_for(elapsed_shift_cycle_count, shift_cycle_starts_at))
 
-      shift_for(elapsed_shift_count, shift_starts_at)
+      new_shifts.detect { |shift| timestamp.between?(shift.starts_at, shift.ends_at) && timestamp < shift.ends_at }
     end
 
     private
 
     attr_reader :rotation
-    delegate :shift_duration, to: :rotation
+    delegate :shift_cycle_duration, to: :rotation
 
     # Starting time of a shift which covers the timestamp.
     # @return [ActiveSupport::TimeWithZone]
-    def shift_start_time(elapsed_shift_count)
-      rotation_starts_at + (elapsed_shift_count * shift_duration)
+    def shift_cycle_start_time(elapsed_shift_count)
+      rotation_starts_at + (elapsed_shift_count * shift_cycle_duration)
     end
 
     # Total completed shifts passed between rotation start
     # time and the provided timestamp.
     # @return [Integer]
-    def elapsed_whole_shifts(timestamp)
+    def elapsed_whole_shift_cycles(timestamp)
       elapsed_duration = timestamp - rotation_starts_at
 
       unless rotation.hours?
@@ -104,18 +109,56 @@ module IncidentManagement
       end
 
       # Uses #round to account for floating point inconsistencies.
-      (elapsed_duration / shift_duration).round(5).floor
+      (elapsed_duration / shift_cycle_duration).round(5).floor
+    end
+
+    def shift_cycle_for(elapsed_shift_cycle_count, shift_cycle_starts_at)
+      participant = participants[participant_rank(elapsed_shift_cycle_count)]
+
+      if rotation.has_shift_active_period?
+        # the number of shifts we expect to be included in the
+        # shift_cycle. 1.week is the same as 7.days.
+        expected_shift_count = rotation.shifts_per_cycle
+        (0...expected_shift_count).map do |shift_count|
+          # we know the start/end time of the active period,
+          # so the date is dependent on the cycle start time
+          # and how many days have elapsed in the cycle.
+          # EX) shift_cycle_starts_at = Monday @ 8am
+          #     active_period_start = 8am
+          #     active_period_end = 5pm
+          #     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)
+
+          shift_for(participant, [rotation.starts_at, starts_at].max, limit_end_time(ends_at))
+        end
+      else
+        # This is the normal shift start/end times
+        shift_cycle_ends_at = limit_end_time(shift_cycle_starts_at + shift_cycle_duration)
+
+        shift_for(participant, shift_cycle_starts_at, shift_cycle_ends_at)
+      end
+    end
+
+    # Removes shifts which are out of bounds from the given starts_at and ends_at timestamps.
+    def remove_out_of_bounds_shifts(shifts, shift_cycle_starts_at, starts_at, ends_at)
+      shifts.reject! { |shift| shift.ends_at <= starts_at } if shift_cycle_starts_at <= starts_at
+      shifts.reject! { |shift| shift.starts_at > ends_at } if (shift_cycle_starts_at + shift_cycle_duration) > ends_at
+
+      shifts
     end
 
     # Returns an UNSAVED shift, as this shift won't necessarily
     # be persisted.
     # @return [IncidentManagement::OncallShift]
-    def shift_for(elapsed_shift_count, shift_starts_at)
+    def shift_for(participant, starts_at, ends_at)
       IncidentManagement::OncallShift.new(
         rotation: rotation,
-        participant: participants[participant_rank(elapsed_shift_count)],
-        starts_at: shift_starts_at,
-        ends_at: limit_end_time(shift_starts_at + shift_duration)
+        participant: participant,
+        starts_at: starts_at,
+        ends_at: ends_at
       )
     end
 

ee/spec/factories/incident_management/oncall_rotations.rb

@@ -9,6 +9,11 @@ FactoryBot.define do
     length { 5 }
     length_unit { :days }
 
+    trait :with_active_period do
+      active_period_start { '08:00' }
+      active_period_end { '17:00' }
+    end
+
     trait :with_participant do
       after(:create) do |rotation|
         user = create(:user)

ee/spec/factories/incident_management/oncall_shifts.rb

@@ -5,6 +5,6 @@ FactoryBot.define do
     association :participant, :with_developer_access, factory: :incident_management_oncall_participant
     rotation { participant.rotation }
     starts_at { rotation.starts_at }
-    ends_at { starts_at + rotation.shift_duration }
+    ends_at { starts_at + rotation.shift_cycle_duration }
   end
 end

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

@@ -82,6 +82,62 @@ RSpec.describe Mutations::IncidentManagement::OncallRotation::Create do
         end
       end
 
+      context 'with active period times given' do
+        let(:start_time) { '08:00' }
+        let(:end_time) { '17:00' }
+
+        before do
+          args[:active_period] = {
+            start_time: start_time,
+            end_time: end_time
+          }
+        end
+
+        it 'returns the on-call rotation with no errors' do
+          expect(resolve).to match(
+            oncall_rotation: ::IncidentManagement::OncallRotation.last!,
+            errors: be_empty
+          )
+        end
+
+        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('08:00')
+          expect(rotation.active_period_end.strftime('%H:%M')).to eql('17:00')
+        end
+
+        context 'hours rotation length unit' do
+          before do
+            args[:rotation_length][:unit] = ::IncidentManagement::OncallRotation.length_units[:hours]
+          end
+
+          it 'returns errors' do
+            expect(resolve).to match(
+              oncall_rotation: nil,
+              errors: [/Restricted shift times are not available for hourly shifts/]
+            )
+          end
+        end
+
+        context 'end time is before start time' 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")
+          end
+        end
+
+        context 'invalid time given' do
+          let(:start_time) {  'an invalid time' }
+
+          it 'raises an error' do
+            expect { resolve }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, 'Time given is invalid')
+          end
+        end
+      end
+
       describe 'error cases' do
         context 'user cannot be found' do
           before do

ee/spec/graphql/resolvers/incident_management/oncall_shifts_resolver_spec.rb

@@ -8,7 +8,7 @@ RSpec.describe Resolvers::IncidentManagement::OncallShiftsResolver do
   let_it_be(:current_user) { create(:user) }
   let_it_be(:rotation) { create(:incident_management_oncall_rotation, :with_participant) }
   let_it_be(:project) { rotation.project }
-  let(:args) { { start_time: rotation.starts_at, end_time: rotation.starts_at + rotation.shift_duration } }
+  let(:args) { { start_time: rotation.starts_at, end_time: rotation.starts_at + rotation.shift_cycle_duration } }
 
   subject(:shifts) { sync(resolve_oncall_shifts(args).to_a) }
 

ee/spec/graphql/types/incident_management/oncall_rotation_type_spec.rb

@@ -16,6 +16,7 @@ RSpec.describe GitlabSchema.types['IncidentManagementOncallRotation'] do
       length
       length_unit
       participants
+      active_period
       shifts
     ]
 

ee/spec/lib/incident_management/oncall_shift_generator_spec.rb

@@ -3,11 +3,12 @@
 require 'spec_helper'
 
 RSpec.describe IncidentManagement::OncallShiftGenerator do
+  let_it_be(:schedule) { create(:incident_management_oncall_schedule, timezone: 'Etc/UTC') }
   let_it_be(:rotation_start_time) { Time.parse('2020-12-08 00:00:00 UTC').utc }
-  let_it_be_with_reload(:rotation) { create(:incident_management_oncall_rotation, starts_at: rotation_start_time, length: 5, length_unit: :days) }
+  let_it_be_with_reload(:rotation) { create(:incident_management_oncall_rotation, starts_at: rotation_start_time, length: 5, length_unit: :days, schedule: schedule) }
 
   let(:current_time) { Time.parse('2020-12-08 15:00:00 UTC').utc }
-  let(:shift_length) { rotation.shift_duration }
+  let(:shift_length) { rotation.shift_cycle_duration }
 
   around do |example|
     travel_to(current_time) { example.run }
@@ -29,7 +30,7 @@ RSpec.describe IncidentManagement::OncallShiftGenerator do
   # Example) [[:participant2, '2020-12-13 00:00:00 UTC', '2020-12-18 00:00:00 UTC']]
   #          :participant2 would reference `let(:participant2)`
   shared_examples 'unsaved shifts' do |description, shift_params|
-    it "returns #{description}" do
+    it "returns #{description}", :aggregate_failures do
       expect(shifts).to all(be_a(IncidentManagement::OncallShift))
       expect(shifts.length).to eq(shift_params.length)
 
@@ -71,11 +72,101 @@ RSpec.describe IncidentManagement::OncallShiftGenerator do
        [[:participant, '2020-12-08 00:00:00 UTC', '2020-12-13 00:00:00 UTC'],
         [:participant, '2020-12-13 00:00:00 UTC', '2020-12-18 00:00:00 UTC'],
         [:participant, '2020-12-18 00:00:00 UTC', '2020-12-23 00:00:00 UTC']]
+
+      context 'when timestamp is at the end of a shift' do
+        let(:starts_at) { rotation_start_time + shift_length }
+
+        it_behaves_like 'unsaved shifts',
+          'the second and third shift',
+          [[:participant, '2020-12-13 00:00:00 UTC', '2020-12-18 00:00:00 UTC'],
+           [:participant, '2020-12-18 00:00:00 UTC', '2020-12-23 00:00:00 UTC']]
+      end
     end
 
     context 'with many participants' do
       include_context 'with three participants'
 
+      it_behaves_like 'unsaved shifts',
+        'One shift of 5 days long for each participant',
+        [[:participant1, '2020-12-08 00:00:00 UTC', '2020-12-13 00:00:00 UTC'],
+         [:participant2, '2020-12-13 00:00:00 UTC', '2020-12-18 00:00:00 UTC'],
+         [:participant3, '2020-12-18 00:00:00 UTC', '2020-12-23 00:00:00 UTC']]
+
+      context 'with shift active period times set' do
+        before do
+          rotation.update!(
+            active_period_start: "08:00",
+            active_period_end: "17:00"
+          )
+        end
+
+        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 split by each day',
+          [[:participant1, '2020-12-08 08:00:00 UTC', '2020-12-08 17:00:00 UTC'],
+           [:participant1, '2020-12-09 08:00:00 UTC', '2020-12-09 17:00:00 UTC'],
+           [:participant1, '2020-12-10 08:00:00 UTC', '2020-12-10 17:00:00 UTC'],
+           [:participant1, '2020-12-11 08:00:00 UTC', '2020-12-11 17:00:00 UTC'],
+           [:participant1, '2020-12-12 08:00:00 UTC', '2020-12-12 17:00:00 UTC'],
+           [:participant2, '2020-12-13 08:00:00 UTC', '2020-12-13 17:00:00 UTC'],
+           [:participant2, '2020-12-14 08:00:00 UTC', '2020-12-14 17:00:00 UTC'],
+           [:participant2, '2020-12-15 08:00:00 UTC', '2020-12-15 17:00:00 UTC'],
+           [: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']]
+
+        context 'with week length unit' do
+          before do
+            rotation.update!(
+              length_unit: :weeks,
+              length: 1
+            )
+          end
+
+          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',
+            '7 shifts for each participant split by each day',
+            [[:participant1, '2020-12-08 08:00:00 UTC', '2020-12-08 17:00:00 UTC'],
+             [:participant1, '2020-12-09 08:00:00 UTC', '2020-12-09 17:00:00 UTC'],
+             [:participant1, '2020-12-10 08:00:00 UTC', '2020-12-10 17:00:00 UTC'],
+             [:participant1, '2020-12-11 08:00:00 UTC', '2020-12-11 17:00:00 UTC'],
+             [:participant1, '2020-12-12 08:00:00 UTC', '2020-12-12 17:00:00 UTC'],
+             [:participant1, '2020-12-13 08:00:00 UTC', '2020-12-13 17:00:00 UTC'],
+             [:participant1, '2020-12-14 08:00:00 UTC', '2020-12-14 17:00:00 UTC'],
+             [:participant2, '2020-12-15 08:00:00 UTC', '2020-12-15 17:00:00 UTC'],
+             [: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'],
+             [:participant2, '2020-12-18 08:00:00 UTC', '2020-12-18 17:00:00 UTC'],
+             [:participant2, '2020-12-19 08:00:00 UTC', '2020-12-19 17:00:00 UTC'],
+             [:participant2, '2020-12-20 08:00:00 UTC', '2020-12-20 17:00:00 UTC'],
+             [:participant2, '2020-12-21 08:00:00 UTC', '2020-12-21 17:00:00 UTC']]
+        end
+
+        context 'rotation start time is in middle of active period' do
+          before do
+            rotation.update!(starts_at: rotation_start_time.change(hour: 10))
+          end
+
+          it_behaves_like 'unsaved shifts',
+            '5 shifts for each participant split by each day',
+            [[:participant1, '2020-12-08 10:00:00 UTC', '2020-12-08 17:00:00 UTC'],
+             [:participant1, '2020-12-09 08:00:00 UTC', '2020-12-09 17:00:00 UTC'],
+             [:participant1, '2020-12-10 08:00:00 UTC', '2020-12-10 17:00:00 UTC'],
+             [:participant1, '2020-12-11 08:00:00 UTC', '2020-12-11 17:00:00 UTC'],
+             [:participant1, '2020-12-12 08:00:00 UTC', '2020-12-12 17:00:00 UTC'],
+             [:participant2, '2020-12-13 08:00:00 UTC', '2020-12-13 17:00:00 UTC'],
+             [:participant2, '2020-12-14 08:00:00 UTC', '2020-12-14 17:00:00 UTC'],
+             [:participant2, '2020-12-15 08:00:00 UTC', '2020-12-15 17:00:00 UTC'],
+             [: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
+      end
+
       context 'when end time is earlier than start time' do
         let(:ends_at) { starts_at - 1.hour }
 
@@ -741,6 +832,14 @@ RSpec.describe IncidentManagement::OncallShiftGenerator do
           [:participant2, '2020-12-13 00:00:00 UTC', '2020-12-18 00:00:00 UTC']
       end
 
+      context 'when timestamp is at the end of a shift' do
+        let(:timestamp) { rotation_start_time + shift_length }
+
+        it_behaves_like 'unsaved shift',
+          'the second shift',
+          [:participant2, '2020-12-13 00:00:00 UTC', '2020-12-18 00:00:00 UTC']
+      end
+
       context 'with rotation end time' do
         let(:rotation_end_time) { rotation_start_time + (shift_length * 2.5) }
 
@@ -768,6 +867,41 @@ RSpec.describe IncidentManagement::OncallShiftGenerator do
           it { is_expected.to be_nil }
         end
       end
+
+      context 'with shift active period times set' do
+        before do
+          rotation.update!(
+            active_period_start: "08:00",
+            active_period_end: "17:00"
+          )
+        end
+
+        context 'when timestamp is the start of rotation, but before active period' do
+          let(:timestamp) { rotation_start_time }
+
+          it { is_expected.to be_nil }
+        end
+
+        context 'when timestamp is the same time as active period start' do
+          let(:timestamp) { rotation_start_time.change(hour: 8) }
+
+          it_behaves_like 'unsaved shift',
+            'the first shift of the shift cycle (split by the active period)',
+            [:participant1, '2020-12-08 08:00:00 UTC', '2020-12-08 17:00:00 UTC']
+        end
+
+        context 'when timestamp is the same time as active period end' do
+          let(:timestamp) { rotation_start_time.change(hour: 17) }
+
+          it { is_expected.to be_nil }
+        end
+
+        context 'when timestamp is the after the active period ends' do
+          let(:timestamp) { rotation_start_time.change(hour: 17, min: 1) }
+
+          it { is_expected.to be_nil }
+        end
+      end
     end
   end
 end

ee/spec/models/incident_management/oncall_rotation_spec.rb

@@ -51,6 +51,48 @@ RSpec.describe IncidentManagement::OncallRotation do
         end
       end
     end
+
+    describe 'active period start/end time' do
+      context 'missing values' do
+        before do
+          allow(subject).to receive(stubbed_field).and_return('08:00')
+        end
+
+        context 'start time set' do
+          let(:stubbed_field) { :active_period_start }
+
+          it { is_expected.to validate_presence_of(:active_period_end) }
+        end
+
+        context 'end time set' do
+          let(:stubbed_field) { :active_period_end }
+
+          it { is_expected.to validate_presence_of(:active_period_start) }
+        end
+      end
+
+      context 'hourly shifts' do
+        subject { build(:incident_management_oncall_rotation, schedule: schedule, name: 'Test rotation', length_unit: :hours) }
+
+        it 'raises a validation error if an active period is set' do
+          subject.active_period_start = '08:00'
+          subject.active_period_end = '17:00'
+
+          expect(subject.valid?).to eq(false)
+          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
 
   describe 'scopes' do
@@ -66,10 +108,10 @@ RSpec.describe IncidentManagement::OncallRotation do
     end
   end
 
-  describe '#shift_duration' do
+  describe '#shift_cycle_duration' do
     let_it_be(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule, length: 5, length_unit: :days) }
 
-    subject { rotation.shift_duration }
+    subject { rotation.shift_cycle_duration }
 
     it { is_expected.to eq(5.days) }
 
@@ -81,4 +123,40 @@ RSpec.describe IncidentManagement::OncallRotation do
       end
     end
   end
+
+  describe '#shifts_per_cycle' do
+    let(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule, length: 5, length_unit: length_unit, active_period_start: active_period_start, active_period_end: active_period_end) }
+    let(:length_unit) { :weeks }
+    let(:active_period_start) { nil }
+    let(:active_period_end) { nil }
+
+    subject { rotation.shifts_per_cycle }
+
+    context 'when no shift active period set up' do
+      it { is_expected.to eq(1) }
+    end
+
+    context 'when hours' do
+      let(:length_unit) { :hours }
+
+      it { is_expected.to eq(1) }
+    end
+
+    context 'with shift active periods' do
+      let(:active_period_start) { '08:00' }
+      let(:active_period_end) { '17:00' }
+
+      context 'weeks length unit' do
+        let(:length_unit) { :weeks }
+
+        it { is_expected.to eq(35) }
+      end
+
+      context 'days length unit' do
+        let(:length_unit) { :days }
+
+        it { is_expected.to eq(5) }
+      end
+    end
+  end
 end

ee/spec/requests/api/graphql/project/incident_management/oncall_shifts_spec.rb

@@ -11,7 +11,7 @@ RSpec.describe 'getting Incident Management on-call shifts' do
   let_it_be(:current_user) { participant.user }
 
   let(:starts_at) { rotation.starts_at }
-  let(:ends_at) { rotation.starts_at + rotation.shift_duration } # intentionally return one shift
+  let(:ends_at) { rotation.starts_at + rotation.shift_cycle_duration } # intentionally return one shift
   let(:params) { { start_time: starts_at.iso8601, end_time: ends_at.iso8601 } }
 
   let(:shift_fields) do

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

@@ -122,23 +122,68 @@ RSpec.describe IncidentManagement::OncallRotations::CreateService do
     end
 
     context 'with valid params' do
-      it 'successfully creates an on-call rotation with participants' do
-        expect(execute).to be_success
-
-        oncall_rotation = execute.payload[:oncall_rotation]
-        expect(oncall_rotation).to be_a(::IncidentManagement::OncallRotation)
-        expect(oncall_rotation.name).to eq('On-call rotation')
-        expect(oncall_rotation.starts_at).to eq(starts_at)
-        expect(oncall_rotation.ends_at).to eq(1.month.after(starts_at))
-        expect(oncall_rotation.length).to eq(1)
-        expect(oncall_rotation.length_unit).to eq('days')
-
-        expect(oncall_rotation.participants.length).to eq(1)
-        expect(oncall_rotation.participants.first).to have_attributes(
-          **participants.first,
-          rotation: oncall_rotation,
-          persisted?: true
-        )
+      shared_examples 'successfully creates rotation' do
+        it 'successfully creates an on-call rotation with participants' do
+          expect(execute).to be_success
+
+          oncall_rotation = execute.payload[:oncall_rotation]
+          expect(oncall_rotation).to be_a(::IncidentManagement::OncallRotation)
+          expect(oncall_rotation.name).to eq('On-call rotation')
+          expect(oncall_rotation.starts_at).to eq(starts_at)
+          expect(oncall_rotation.ends_at).to eq(1.month.after(starts_at))
+          expect(oncall_rotation.length).to eq(1)
+          expect(oncall_rotation.length_unit).to eq('days')
+
+          expect(oncall_rotation.participants.length).to eq(1)
+          expect(oncall_rotation.participants.first).to have_attributes(
+            **participants.first,
+            rotation: oncall_rotation,
+            persisted?: true
+          )
+        end
+      end
+
+      it_behaves_like 'successfully creates rotation'
+
+      context 'with an active period given' do
+        let(:active_period_start) { '08:00' }
+        let(:active_period_end) { '17:00' }
+
+        before do
+          params[:active_period_start] = active_period_start
+          params[:active_period_end] = active_period_end
+        end
+
+        shared_examples 'saved the active period times' do
+          it 'saves the active period times' do
+            oncall_rotation = execute.payload[:oncall_rotation]
+
+            expect(oncall_rotation.active_period_start.strftime('%H:%M')).to eq(active_period_start)
+            expect(oncall_rotation.active_period_end.strftime('%H:%M')).to eq(active_period_end)
+          end
+        end
+
+        it_behaves_like 'successfully creates rotation'
+        it_behaves_like 'saved the active period times'
+
+        context 'when only active period end time is set' do
+          let(:active_period_start) { nil }
+
+          it_behaves_like 'error response', "Active period start can't be blank"
+        end
+
+        context 'when only active period start time is set' do
+          let(:active_period_end) { nil }
+
+          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
     end
   end

ee/spec/workers/incident_management/oncall_rotations/persist_shifts_job_spec.rb

@@ -73,7 +73,7 @@ RSpec.describe IncidentManagement::OncallRotations::PersistShiftsJob do
       # fill in the correct shift history.
       context 'when current time is several shifts after the last saved shift' do
         around do |example|
-          travel_to(existing_shift.ends_at + (3 * rotation.shift_duration)) { example.run }
+          travel_to(existing_shift.ends_at + (3 * rotation.shift_cycle_duration)) { example.run }
         end
 
         context 'when feature flag is not enabled' do
@@ -97,8 +97,8 @@ RSpec.describe IncidentManagement::OncallRotations::PersistShiftsJob do
           expect(rotation.shifts.length).to eq(4)
           expect(first_shift).to eq(existing_shift)
           expect(second_shift.starts_at).to eq(existing_shift.ends_at)
-          expect(third_shift.starts_at).to eq(existing_shift.ends_at + rotation.shift_duration)
-          expect(fourth_shift.starts_at).to eq(existing_shift.ends_at + (2 * rotation.shift_duration))
+          expect(third_shift.starts_at).to eq(existing_shift.ends_at + rotation.shift_cycle_duration)
+          expect(fourth_shift.starts_at).to eq(existing_shift.ends_at + (2 * rotation.shift_cycle_duration))
         end
       end
     end

locale/gitlab.pot

@@ -25563,6 +25563,9 @@ msgstr ""
 msgid "Restrict projects for this runner"
 msgstr ""
 
+msgid "Restricted shift times are not available for hourly shifts"
+msgstr ""
+
 msgid "Restricts sign-ups for email addresses that match the given regex. See the %{supported_syntax_link_start}supported syntax%{supported_syntax_link_end} for more information."
 msgstr ""
 
@@ -35724,6 +35727,9 @@ 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 ""