Fix input type params

Move logic into mutation
Update docs

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

@@ -43,6 +43,7 @@ module Mutations
                  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)
@@ -75,8 +76,8 @@ module Mutations
           rotation_length_unit = args[:rotation_length][:unit]
           starts_at = parse_datetime(schedule, args[:starts_at])
           ends_at = parse_datetime(schedule, args[:ends_at]) if args[:ends_at]
-          active_period_start = args.dig(:active_period, :start_time)
-          active_period_end = args.dig(:active_period, :end_time)
+
+          active_period_start, active_period_end = active_period_times(args)
 
           args.slice(:name).merge(
             length: rotation_length,
@@ -107,6 +108,27 @@ 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)
+
+          parsed_from = Time.parse(start_time)
+          parsed_to = Time.parse(end_time)
+
+          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
@@ -126,6 +148,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

@@ -13,29 +13,7 @@ module Types
 
       argument :end_time, GraphQL::STRING_TYPE,
                 required: true,
-                description: 'The end of the rotation active period..'
-
-      TIME_FORMAT = %r[^(0[0-9]|1[0-9]|2[0-3]):[0-5][0-9]$].freeze
-
-      def prepare
-        raise invalid_time_error unless TIME_FORMAT.match?(start_time)
-        raise invalid_time_error unless TIME_FORMAT.match?(end_time)
-
-        parsed_from = Time.parse(start_time)
-        parsed_to = Time.parse(end_time)
-
-        if parsed_to < parsed_from
-          raise ::Gitlab::Graphql::Errors::ArgumentError, "'start_time' time must be before 'end_time' time"
-        end
-
-        to_h
-      end
-
-      private
-
-      def invalid_time_error
-        ::Gitlab::Graphql::Errors::ArgumentError.new 'Time given is invalid'
-      end
+                description: 'The end of the rotation active period.'
     end
     # rubocop: enable Graphql/AuthorizeTypes
   end

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

@@ -17,11 +17,11 @@ module Types
 
       alias_method :active_period, :object
 
-      def from
+      def start_time
         active_period.start_time&.strftime('%H:%M')
       end
 
-      def to
+      def end_time
         active_period.end_time&.strftime('%H:%M')
       end
     end

ee/app/models/incident_management/oncall_rotation.rb

@@ -68,6 +68,8 @@ module IncidentManagement
       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?
 

ee/lib/incident_management/oncall_shift_generator.rb

@@ -119,7 +119,7 @@ module IncidentManagement
         # 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 - 1).map do |shift_count|
+        (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.
@@ -144,7 +144,7 @@ module IncidentManagement
 
     # 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.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

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

@@ -83,10 +83,13 @@ RSpec.describe Mutations::IncidentManagement::OncallRotation::Create do
       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: '08:00',
-            end_time: '17:00'
+            start_time: start_time,
+            end_time: end_time
           }
         end
 
@@ -116,6 +119,23 @@ RSpec.describe Mutations::IncidentManagement::OncallRotation::Create do
             )
           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

ee/spec/lib/incident_management/oncall_shift_generator_spec.rb

@@ -72,6 +72,15 @@ 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