Tweak arguments to allow for partial edits

- Add specs for mutation

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

@@ -37,15 +37,15 @@ module Mutations
           ::IncidentManagement::OncallRotationsFinder.new(current_user, project, schedule, args).execute.first
         end
 
-        def service_params(schedule, participants, args)
-          rotation_length = args[:rotation_length][:length]
-          rotation_length_unit = args[:rotation_length][:unit]
+        def parsed_params(schedule, participants, args)
+          rotation_length = args.dig(:rotation_length, :length)
+          rotation_length_unit = args.dig(: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, active_period_end = active_period_times(args)
 
-          args.slice(:name).merge(
+          {
             length: rotation_length,
             length_unit: rotation_length_unit,
             starts_at: starts_at,
@@ -53,14 +53,17 @@ module Mutations
             participants: find_participants(participants),
             active_period_start: active_period_start,
             active_period_end: active_period_end
-          )
+          }
         end
 
         def parse_datetime(schedule, timestamp)
-          timestamp.asctime.in_time_zone(schedule.timezone)
+          timestamp&.asctime&.in_time_zone(schedule.timezone)
         end
 
         def find_participants(user_array)
+          return if user_array.nil?
+          return [] if user_array == []
+
           raise_too_many_users_error if user_array.size > MAXIMUM_PARTICIPANTS
 
           usernames = user_array.map {|h| h[:username] }

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

@@ -57,7 +57,7 @@ module Mutations
             schedule,
             project,
             current_user,
-            service_params(schedule, participants, args)
+            create_service_params(schedule, participants, args)
           ).execute
 
           response(result)
@@ -65,6 +65,12 @@ module Mutations
         rescue ActiveRecord::RecordInvalid => e
           raise Gitlab::Graphql::Errors::ArgumentError, e.message
         end
+
+        private
+
+        def create_service_params(schedule, participants, args)
+          args.slice(:name).merge(parsed_params(schedule, participants, args))
+        end
       end
     end
   end

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

@@ -13,11 +13,11 @@ module Mutations
                  description: 'The ID of the on-call schedule to create the on-call rotation in.'
 
         argument :name, GraphQL::STRING_TYPE,
-                 required: true,
+                 required: false,
                  description: 'The name of the on-call rotation.'
 
         argument :starts_at, Types::IncidentManagement::OncallRotationDateInputType,
-                 required: true,
+                 required: false,
                  description: 'The start date and time of the on-call rotation, in the timezone of the on-call schedule.'
 
         argument :ends_at, Types::IncidentManagement::OncallRotationDateInputType,
@@ -25,7 +25,7 @@ module Mutations
                  description: 'The end date and time of the on-call rotation, in the timezone of the on-call schedule.'
 
         argument :rotation_length, Types::IncidentManagement::OncallRotationLengthInputType,
-                 required: true,
+                 required: false,
                  description: 'The rotation length of the on-call rotation.'
 
         argument :active_period, Types::IncidentManagement::OncallRotationActivePeriodInputType,
@@ -34,16 +34,16 @@ module Mutations
 
         argument :participants,
                  [Types::IncidentManagement::OncallUserInputType],
-                 required: true,
+                 required: false,
                  description: 'The usernames of users participating in the on-call rotation.'
 
-        def resolve(id:, participants:, **args)
+        def resolve(id:, **args)
           rotation = authorized_find!(id: id)
 
           result = ::IncidentManagement::OncallRotations::EditService.new(
             rotation,
             current_user,
-            service_params(rotation.schedule, participants, args)
+            edit_service_params(rotation.schedule, args[:participants], args)
           ).execute
 
           response(result)
@@ -51,6 +51,12 @@ module Mutations
 
         private
 
+        def edit_service_params(schedule, participants, args)
+          parsed_service_params = parsed_params(schedule, participants, args)
+
+          args.slice(:name).merge(parsed_service_params.slice(*args.keys))
+        end
+
         def find_object(id:)
           GitlabSchema.object_from_id(id, expected_type: ::IncidentManagement::OncallRotation)
         end

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

@@ -56,6 +56,10 @@ module Types
             description: 'Blocks of time for which a participant is on-call within a given time frame. Time frame cannot exceed one month.',
             max_page_size: MAX_SHIFTS_FOR_TIMEFRAME,
             resolver: ::Resolvers::IncidentManagement::OncallShiftsResolver
+
+      def participants
+        object.participants.not_removed
+      end
     end
   end
 end

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

@@ -179,7 +179,7 @@ RSpec.describe Mutations::IncidentManagement::OncallRotation::Create do
 
         context 'too many users' do
           before do
-            stub_const('Mutations::IncidentManagement::OncallRotation::Create::MAXIMUM_PARTICIPANTS', 0)
+            stub_const('Mutations::IncidentManagement::OncallRotation::Base::MAXIMUM_PARTICIPANTS', 0)
           end
 
           it 'raises an error' do

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Mutations::IncidentManagement::OncallRotation::Update do
+  let_it_be(:current_user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:schedule) { create(:incident_management_oncall_schedule, project: project) }
+  let_it_be_with_reload(:rotation) { create(:incident_management_oncall_rotation, :with_participants, schedule: schedule) }
+
+  let(:args) do
+    {
+      name: 'On-call rotation',
+      starts_at: "2020-01-10 09:00".in_time_zone(schedule.timezone),
+      rotation_length: {
+        length: 1,
+        unit: ::IncidentManagement::OncallRotation.length_units[:days]
+      },
+      participants: [
+        {
+          username: current_user.username,
+          color_weight: ::IncidentManagement::OncallParticipant.color_weights['50'],
+          color_palette: ::IncidentManagement::OncallParticipant.color_palettes[:blue]
+        }
+      ]
+    }
+  end
+
+  describe '#resolve' do
+    subject(:resolve) { mutation_for(current_user).resolve(id: rotation.to_global_id, participants: args[:participants], **args) }
+
+    context 'user has access to project' do
+      before do
+        stub_licensed_features(oncall_schedules: true)
+        project.add_maintainer(current_user)
+      end
+
+      context 'when OncallRotation::UpdateService responds with success' do
+        it 'returns the on-call rotation with no errors' do
+          expect(resolve).to match(
+            oncall_rotation: ::IncidentManagement::OncallRotation.last!,
+            errors: be_empty
+          )
+        end
+
+        it 'adds the participant to the rotation' do
+          rotation = resolve[:oncall_rotation]
+
+          expect(rotation.participants.not_removed.size).to eq(1)
+          expect(rotation.participants.removed.size).to eq(1)
+
+          first_participant = rotation.participants.not_removed.first
+
+          expect(first_participant.user).to eq(current_user)
+          expect(first_participant.color_weight).to eq('50')
+          expect(first_participant.color_palette).to eq('blue')
+        end
+
+        context 'removing participants' do
+          before do
+            args[:participants] = []
+          end
+
+          it 'returns the on-call rotation with no errors' do
+            expect(resolve[:oncall_rotation].participants.not_removed).to be_empty
+          end
+        end
+
+        context 'with endsAt arg' do
+          let(:ends_at) { "2020-02-10 09:00".in_time_zone(schedule.timezone) }
+
+          before do
+            args.merge!(ends_at: ends_at)
+          end
+
+          it 'returns the on-call rotation with no errors' do
+            expect(resolve[:oncall_rotation].ends_at).to eq(ends_at)
+            expect(resolve[:errors]).to be_empty
+          end
+
+          context 'when endsAt is nil' do
+            let(:ends_at) { nil }
+
+            it 'returns the on-call rotation with no errors' do
+              expect(resolve[:oncall_rotation].ends_at).to be_nil
+              expect(resolve[:errors]).to be_empty
+            end
+          end
+        end
+      end
+
+      context 'when OncallRotations::UpdateService responds with an error' do
+        before do
+          allow_next_instance_of(::IncidentManagement::OncallRotations::EditService) do |service|
+            allow(service).to receive(:execute)
+              .and_return(ServiceResponse.error(payload: { oncall_rotation: nil }, message: 'An error has occurred'))
+          end
+        end
+
+        it 'returns errors' do
+          expect(resolve).to eq(
+            oncall_rotation: nil,
+            errors: ['An error has occurred']
+          )
+        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
+            args.merge!(participants: [username: 'unknown'])
+          end
+
+          it 'raises an error' do
+            expect { resolve }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, "A provided username couldn't be matched to a user")
+          end
+        end
+
+        context 'duplicate participants' do
+          before do
+            args[:participants] << args[:participants].first
+          end
+
+          it 'raises an error' do
+            expect { resolve }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, 'A duplicate username is included in the participant list')
+          end
+        end
+
+        context 'too many users' do
+          before do
+            stub_const('Mutations::IncidentManagement::OncallRotation::Base::MAXIMUM_PARTICIPANTS', 0)
+          end
+
+          it 'raises an error' do
+            expect { resolve }.to raise_error(Gitlab::Graphql::Errors::ArgumentError, "A maximum of #{described_class::MAXIMUM_PARTICIPANTS} participants can be added")
+          end
+        end
+      end
+    end
+
+    context 'when resource is not accessible to the user' do
+      it 'raises an error' do
+        expect { resolve }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable, "The resource that you are attempting to access does not exist or you don't have permission to perform this action")
+      end
+    end
+  end
+
+  private
+
+  def mutation_for(user)
+    described_class.new(object: nil, context: { current_user: user }, field: nil)
+  end
+end