Merge branch '292359-move-rotation-validation-to-service' into 'master'

Move participant permission validation to service

See merge request gitlab-org/gitlab!49537

ee/app/models/incident_management/oncall_participant.rb

@@ -18,16 +18,7 @@ module IncidentManagement
     validates :color_palette, presence: true
     validates :color_weight, presence: true
     validates :user, presence: true, uniqueness: { scope: :oncall_rotation_id }
-    validate  :user_can_read_project, if: :user, on: :create
 
     delegate :project, to: :rotation, allow_nil: true
-
-    private
-
-    def user_can_read_project
-      unless user.can?(:read_project, project)
-        errors.add(:user, 'does not have access to the project')
-      end
-    end
   end
 end

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

@@ -36,6 +36,8 @@ module IncidentManagement
           break error_in_validation(oncall_rotation) unless oncall_rotation.persisted?
 
           participants = participants_for(oncall_rotation)
+          break error_participant_has_no_permission if participants.nil?
+
           first_invalid_participant = participants.find(&:invalid?)
           break error_in_validation(first_invalid_participant) if first_invalid_participant
 
@@ -65,6 +67,8 @@ module IncidentManagement
 
       def participants_for(rotation)
         participants_params.map do |participant|
+          break unless participant[:user].can?(:read_project, project)
+
           OncallParticipant.new(
             rotation: rotation,
             user: participant[:user],
@@ -102,6 +106,10 @@ module IncidentManagement
         ServiceResponse.success(payload: { oncall_rotation: oncall_rotation })
       end
 
+      def error_participant_has_no_permission
+        error('A participant has insufficient permissions to access the project')
+      end
+
       def error_too_many_participants
         error(_('A maximum of %{count} participants can be added') % { count: MAXIMUM_PARTICIPANTS })
       end

ee/spec/models/incident_management/oncall_participant_spec.rb

@@ -36,33 +36,6 @@ RSpec.describe IncidentManagement::OncallParticipant do
         expect(subject.errors.full_messages.to_sentence).to eq('User has already been taken')
       end
     end
-
-    context 'when participant cannot read project' do
-      let_it_be(:other_user) { create(:user) }
-      subject { build(:incident_management_oncall_participant, rotation: rotation, user: other_user) }
-
-      context 'on creation' do
-        it 'has validation errors' do
-          expect(subject).to be_invalid
-          expect(subject.errors.full_messages.to_sentence).to eq('User does not have access to the project')
-        end
-      end
-
-      context 'after creation' do
-        let(:project) { rotation.project }
-
-        before do
-          project.add_developer(other_user)
-        end
-
-        it 'is valid' do
-          subject.save!
-          remove_user_from_project(other_user, project)
-
-          expect(subject).to be_valid
-        end
-      end
-    end
   end
 
   private

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

@@ -85,18 +85,20 @@ RSpec.describe IncidentManagement::OncallRotations::CreateService do
       end
     end
 
-    context 'participants do not have access to the project' do
+    context 'when participant cannot read project' do
+      let_it_be(:other_user) { create(:user) }
+
       let(:participants) do
         [
           {
-            user: create(:user),
+            user: other_user,
             color_palette: 'blue',
             color_weight: '500'
           }
         ]
       end
 
-      it_behaves_like 'error response', 'User does not have access to the project'
+      it_behaves_like 'error response', 'A participant has insufficient permissions to access the project'
     end
 
     context 'participant is included multiple times' do