Remove user from rotation if removed from project/group

This adds a service to remove a user from an On-call Rotation.

We ensure that the rotation is up to date by running the job
top persist current shifts. Then, we update the `is_removed`
boolean on the Participant model associated to the
user/rotation.

Deletion from rotations are scoped to the member being removed:

- If a member is a project member, only that project's rotations
  are affected
- If a member is a group member, all of the projects within that
  group are affected

https://gitlab.com/gitlab-org/gitlab/-/issues/323631

ee/323631-remove-participant-on-removal.yml

+---
+title: Remove user from rotation if they are removed from a project or group
+merge_request: 59427
+author:
+type: added

ee/app/finders/incident_management/member_oncall_rotations_finder.rb

+# frozen_string_literal: true
+
+module IncidentManagement
+  # A finder used to find all rotations related to a member.
+  # For most cases you will want to use `OncallRotationsFinder` instead.
+  # For a group member, finds all rotations that user is part of in the group
+  # For a project member, find all the rotations that user is part of in the project.
+  class MemberOncallRotationsFinder
+    def initialize(member)
+      @member = member
+      @user = member.user
+    end
+
+    def execute
+      projects = member.source.is_a?(Group) ? member.source.projects : member.source
+
+      for_projects(projects)
+    end
+
+    private
+
+    attr_reader :member, :user
+
+    def for_projects(projects)
+      user.oncall_rotations.for_project(projects)
+    end
+  end
+end

ee/app/models/incident_management/oncall_participant.rb

@@ -23,5 +23,6 @@ module IncidentManagement
 
     scope :not_removed, -> { where(is_removed: false) }
     scope :removed, -> { where(is_removed: true) }
+    scope :for_user, -> (user) { where(user: user) }
   end
 end

ee/app/models/incident_management/oncall_rotation.rb

@@ -48,6 +48,7 @@ module IncidentManagement
     validates :active_period_end, presence: true, if: :active_period_start
     validate :no_active_period_for_hourly_shifts, if: :hours?
 
+    scope :for_project, -> (project) { joins(:schedule).merge(OncallSchedule.for_project(project)) }
     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_active_period, -> { where.not(active_period_start: nil) }

ee/app/models/incident_management/oncall_schedule.rb

@@ -21,6 +21,7 @@ module IncidentManagement
     validates :timezone, presence: true, inclusion: { in: :timezones }
 
     scope :for_iid, -> (iid) { where(iid: iid) }
+    scope :for_project, -> (project) { where(project: project) }
 
     delegate :name, to: :project, prefix: true
 

ee/app/services/concerns/incident_management/oncall_rotations/shared_rotation_logic.rb

@@ -5,6 +5,11 @@ module IncidentManagement
     module SharedRotationLogic
       MAXIMUM_PARTICIPANTS = 100
 
+      def ensure_rotation_is_up_to_date
+        # Ensure shift history is up to date before saving new params
+        IncidentManagement::OncallRotations::PersistShiftsJob.new.perform(oncall_rotation.id)
+      end
+
       def save_participants!
         participants = participants_for(oncall_rotation).each(&:validate!)
 

ee/app/services/ee/members/destroy_service.rb

@@ -14,6 +14,7 @@ module EE
 
         cleanup_group_identity(member)
         cleanup_group_deletion_schedule(member) if member.source.is_a?(Group)
+        cleanup_oncall_rotations(member)
       end
 
       private
@@ -49,6 +50,21 @@ module EE
 
         deletion_schedule.destroy if deletion_schedule.deleting_user == member.user
       end
+
+      def cleanup_oncall_rotations(member)
+        user = member.user
+
+        return unless user
+
+        user_rotations = ::IncidentManagement::MemberOncallRotationsFinder.new(member).execute
+
+        return unless user_rotations.present?
+
+        ::IncidentManagement::OncallRotations::RemoveParticipantsService.new(
+          user_rotations,
+          user
+        ).execute
+      end
     end
   end
 end

ee/app/services/ee/users/destroy_service.rb

@@ -9,6 +9,7 @@ module EE
       def execute(user, options = {})
         result = super(user, options) do |delete_user|
           mirror_cleanup(delete_user)
+          oncall_rotations_cleanup(delete_user)
         end
 
         log_audit_event(user) if result.try(:destroyed?)
@@ -31,6 +32,13 @@ module EE
         end
       end
 
+      def oncall_rotations_cleanup(user)
+        ::IncidentManagement::OncallRotations::RemoveParticipantsService.new(
+          user.oncall_rotations,
+          user
+        ).execute
+      end
+
       private
 
       def first_mirror_owner(user, mirror)

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

@@ -34,8 +34,7 @@ module IncidentManagement
           return error_participants_without_permission if users_without_permissions?
         end
 
-        # Ensure shift history is up to date before saving new params
-        IncidentManagement::OncallRotations::PersistShiftsJob.new.perform(oncall_rotation.id)
+        ensure_rotation_is_up_to_date
 
         OncallRotation.transaction do
           oncall_rotation.update!(params)

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

+# frozen_string_literal: true
+
+module IncidentManagement
+  module OncallRotations
+    class RemoveParticipantService < OncallRotations::BaseService
+      include IncidentManagement::OncallRotations::SharedRotationLogic
+      # @param oncall_rotations [IncidentManagement::OncallRotation]
+      # @param user_to_remove [User]
+      def initialize(oncall_rotation, user_to_remove)
+        @oncall_rotation = oncall_rotation
+        @user_to_remove = user_to_remove
+      end
+
+      def execute
+        ensure_rotation_is_up_to_date
+
+        deleted = remove_user_from_rotation
+
+        if deleted
+          save_current_shift!
+        end
+      end
+
+      private
+
+      attr_reader :oncall_rotation, :user_to_remove
+
+      def remove_user_from_rotation
+        participant = oncall_rotation.participants.for_user(user_to_remove).first
+
+        return unless participant
+
+        participant.update!(is_removed: true)
+
+        oncall_rotation.touch
+      end
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+module IncidentManagement
+  module OncallRotations
+    class RemoveParticipantsService
+      # @param oncall_rotations [Array<IncidentManagement::OncallRotation>]
+      # @param user_to_remove [User]
+      def initialize(oncall_rotations, user_to_remove)
+        @oncall_rotations = oncall_rotations
+        @user_to_remove = user_to_remove
+      end
+
+      attr_reader :oncall_rotations, :user_to_remove
+
+      def execute
+        oncall_rotations.each do |oncall_rotation|
+          RemoveParticipantService.new(oncall_rotation, user_to_remove).execute
+        end
+      end
+    end
+  end
+end

ee/spec/finders/incident_management/member_oncall_rotations_finder_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::MemberOncallRotationsFinder do
+  let_it_be(:user) { create(:user) }
+
+  describe '#execute' do
+    subject(:execute) { described_class.new(member).execute }
+
+    # 2 Group Projects
+    let_it_be(:group) { create(:group) }
+
+    let_it_be(:group_project) { create(:project, group: group) }
+    let_it_be(:group_schedule) { create(:incident_management_oncall_schedule, project: group_project) }
+    let_it_be(:group_rotation) { add_rotation_for_user(user, group_schedule) }
+
+    let_it_be(:second_group_project) { create(:project, group: group) }
+    let_it_be(:second_group_schedule) { create(:incident_management_oncall_schedule, project: second_group_project) }
+    let_it_be(:second_group_project_user_rotation) { add_rotation_for_user(user, second_group_schedule) }
+    let_it_be(:second_group_project_other_rotation) { create(:incident_management_oncall_rotation, schedule: second_group_schedule) }
+
+    # 1 standalone Project
+    let_it_be(:project) { create(:project) }
+    let_it_be(:schedule) { create(:incident_management_oncall_schedule, project: project) }
+    let_it_be(:rotation) { add_rotation_for_user(user, schedule) }
+
+    context 'group member' do
+      let!(:member) { create(:group_member, source: group, user: user) }
+
+      it 'returns the group rotations the user is in across many projects' do
+        expect(execute).to contain_exactly(group_rotation, second_group_project_user_rotation)
+      end
+    end
+
+    context 'project member' do
+      # Member is project member
+      let!(:member) { create(:project_member, source: project, user: user) }
+
+      it "returns the rotations the user is in for the member's project" do
+        expect(execute).to contain_exactly(rotation)
+      end
+    end
+  end
+
+  def add_rotation_for_user(user, schedule)
+    rotation = create(:incident_management_oncall_rotation, schedule: schedule)
+    create(:incident_management_oncall_participant, rotation: rotation, user: user)
+
+    rotation
+  end
+end

ee/spec/models/incident_management/oncall_participant_spec.rb

@@ -53,6 +53,12 @@ RSpec.describe IncidentManagement::OncallParticipant do
 
       it { is_expected.to contain_exactly(removed_participant) }
     end
+
+    describe '.for_user' do
+      subject { described_class.for_user(participant.user) }
+
+      it { is_expected.to contain_exactly(participant) }
+    end
   end
 
   private

ee/spec/models/incident_management/oncall_rotation_spec.rb

@@ -121,6 +121,15 @@ RSpec.describe IncidentManagement::OncallRotation do
     end
   end
 
+  describe '.for_project' do
+    let_it_be(:schedule_rotation) { create(:incident_management_oncall_rotation, schedule: schedule) }
+    let_it_be(:another_rotation) { create(:incident_management_oncall_rotation) }
+
+    subject { described_class.for_project(schedule_rotation.project) }
+
+    it { is_expected.to contain_exactly(schedule_rotation) }
+  end
+
   describe '#shift_cycle_duration' do
     let_it_be(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule, length: 5, length_unit: :days) }
 

ee/spec/models/incident_management/oncall_schedule_spec.rb

@@ -35,6 +35,17 @@ RSpec.describe IncidentManagement::OncallSchedule do
     end
   end
 
+  describe 'scopes' do
+    let_it_be(:schedule) { create(:incident_management_oncall_schedule, project: project) }
+    let_it_be(:other_schedule) { create(:incident_management_oncall_schedule) }
+
+    describe '.for_project' do
+      subject { described_class.for_project(project) }
+
+      it { is_expected.to contain_exactly(schedule) }
+    end
+  end
+
   it_behaves_like 'AtomicInternalId' do
     let(:internal_id_attribute) { :iid }
     let(:instance) { build(:incident_management_oncall_schedule) }

ee/spec/services/ee/members/destroy_service_spec.rb

@@ -81,6 +81,58 @@ RSpec.describe Members::DestroyService do
         end
       end
     end
+
+    context 'on-call rotations' do
+      let!(:project) { create(:project, group: group) }
+
+      context 'when member is in an on-call rotation' do
+        let(:project_1_schedule) {  create(:incident_management_oncall_schedule, project: project) }
+        let(:project_1_rotation) {  create(:incident_management_oncall_rotation, schedule: project_1_schedule) }
+        let!(:project_1_participant) { create(:incident_management_oncall_participant, rotation: project_1_rotation, user: member_user) }
+
+        let(:project_2) { create(:project, group: group) }
+        let(:project_2_schedule) {  create(:incident_management_oncall_schedule, project: project_2) }
+        let(:project_2_rotation) {  create(:incident_management_oncall_rotation, schedule: project_2_schedule) }
+        let!(:project_2_participant) { create(:incident_management_oncall_participant, rotation: project_2_rotation, user: member_user) }
+
+        context 'when group member is removed' do
+          it 'calls the remove service for each project in the group' do
+            expect(IncidentManagement::OncallRotations::RemoveParticipantsService).to receive(:new).with([project_1_rotation, project_2_rotation], member_user).and_call_original
+
+            subject.execute(member)
+
+            expect(project_1_participant.reload.is_removed).to eq(true)
+            expect(project_2_participant.reload.is_removed).to eq(true)
+          end
+        end
+
+        context 'when project member is removed' do
+          let!(:project_member) { create(:project_member, source: project, user: member_user) }
+
+          it 'calls the remove service for that project only' do
+            expect(IncidentManagement::OncallRotations::RemoveParticipantsService).to receive(:new).with([project_1_rotation], member_user).and_call_original
+
+            subject.execute(project_member)
+
+            expect(project_1_participant.reload.is_removed).to eq(true)
+            expect(project_2_participant.reload.is_removed).to eq(false)
+          end
+        end
+      end
+
+      context 'when member is not part of an on-call rotation for the group' do
+        before do
+          # Creates a rotation for another project in another group
+          create(:incident_management_oncall_participant, user: member_user)
+        end
+
+        it 'does not call the remove service' do
+          expect(IncidentManagement::OncallRotations::RemoveParticipantsService).not_to receive(:new)
+
+          subject.execute(member)
+        end
+      end
+    end
   end
 
   context 'when current user is not present' do # ie, when the system initiates the destroy

ee/spec/services/ee/users/destroy_service_spec.rb

@@ -42,6 +42,40 @@ RSpec.describe Users::DestroyService do
         end
       end
 
+      context 'when user has oncall rotations' do
+        let(:schedule) { create(:incident_management_oncall_schedule, project: project) }
+        let(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule) }
+        let!(:participant) { create(:incident_management_oncall_participant, rotation: rotation, user: user) }
+
+        context 'in their own project' do
+          let(:project) { create(:project, namespace: user.namespace) }
+
+          it 'deletes the project and the schedule' do
+            operation
+
+            expect { project.reload }.to raise_error(ActiveRecord::RecordNotFound)
+            expect { schedule.reload }.to raise_error(ActiveRecord::RecordNotFound)
+          end
+        end
+
+        context 'in a group project' do
+          let(:group) { create(:group) }
+          let(:project) { create(:project, namespace: group) }
+
+          before do
+            project.add_developer(user)
+          end
+
+          it 'deletes the participant from the rotation' do
+            expect(rotation.participants.reload).to include(participant)
+
+            operation
+
+            expect(rotation.participants.reload).not_to include(participant)
+          end
+        end
+      end
+
       describe 'audit events' do
         include_examples 'audit event logging' do
           let(:fail_condition!) do

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::OncallRotations::RemoveParticipantService do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:schedule) { create(:incident_management_oncall_schedule, project: project) }
+  let_it_be(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule, length: 5, length_unit: :days) }
+  let_it_be(:other_participant) { create(:incident_management_oncall_participant, rotation: rotation) }
+  let_it_be(:participant) { create(:incident_management_oncall_participant, rotation: rotation, user: user) }
+
+  let(:service) { described_class.new(rotation, user) }
+
+  subject(:execute) { service.execute }
+
+  before do
+    stub_licensed_features(oncall_schedules: true)
+  end
+
+  context 'user is not a participant' do
+    let(:other_user) { create(:user) }
+    let(:service) { described_class.new(rotation, other_user) }
+
+    it 'does not send a notification' do
+      expect(NotificationService).not_to receive(:oncall_user_removed)
+      execute
+    end
+  end
+
+  it 'marks the participant as removed' do
+    expect { execute }.to change { participant.reload.is_removed }.to(true)
+  end
+
+  context 'with existing shift by other participant, and current shift by user to be removed' do
+    let(:current_date) { 1.week.after(rotation.starts_at) }
+
+    around do |example|
+      travel_to(current_date) { example.run }
+    end
+
+    # Create an historial shift (other participant)
+    let!(:historical_shift) { create(:incident_management_oncall_shift, rotation: rotation, participant: other_participant, starts_at: rotation.starts_at, ends_at: ends_at(rotation.starts_at)) }
+
+    context 'with historial and current shift' do
+      # Create a current shift (particpant being removed)
+      let!(:current_shift) { create(:incident_management_oncall_shift, rotation: rotation, participant: participant, starts_at: historical_shift.ends_at, ends_at: ends_at(historical_shift.ends_at)) }
+
+      it 'does not affect existing shifts, ends the current shift, and starts the new shift', :aggregate_failures do
+        historical_shift, current_shift = rotation.shifts.order(starts_at: :asc)
+        expect(historical_shift.participant).to eq(other_participant)
+        expect(current_shift.participant).to eq(participant)
+        expect(current_shift.ends_at).not_to be_like_time(Time.current)
+
+        expect { execute }.not_to change { historical_shift.reload }
+
+        new_shift = rotation.shifts.order(starts_at: :asc).last
+
+        expect(current_shift.reload.ends_at).to be_like_time(Time.current)
+        expect(new_shift.participant).to eq(other_participant)
+        expect(new_shift.starts_at).to be_like_time(Time.current)
+        expect(new_shift.ends_at).to be_like_time(ends_at(historical_shift.ends_at))
+      end
+    end
+
+    context 'when current shift has not been created' do
+      it 'creates the current shift and cuts it short' do
+        expect { execute }.to change { rotation.shifts.count }.from(1).to(3)
+
+        current_shift, new_current_shift = rotation.shifts.order(starts_at: :asc).last(2)
+
+        expect(current_shift.participant).to eq(participant)
+        expect(current_shift.ends_at).to be_like_time(Time.current)
+
+        expect(new_current_shift.participant).to eq(other_participant)
+        expect(new_current_shift.starts_at).to be_like_time(Time.current)
+        expect(new_current_shift.ends_at).to be_like_time(ends_at(current_shift.starts_at))
+      end
+    end
+
+    def ends_at(starts_at)
+      starts_at + rotation.shift_cycle_duration
+    end
+  end
+end

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

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IncidentManagement::OncallRotations::RemoveParticipantsService do
+  let!(:user) { instance_double(User) }
+  let!(:rotation) { instance_double(IncidentManagement::OncallRotation) }
+  let!(:rotation_2) { instance_double(IncidentManagement::OncallRotation) }
+
+  let(:service) { described_class.new([rotation, rotation_2], user) }
+
+  subject(:execute) { service.execute }
+
+  before do
+    stub_licensed_features(oncall_schedules: true)
+  end
+
+  it 'calls the RemoveParticipantService for each rotation' do
+    remove_service = instance_spy(IncidentManagement::OncallRotations::RemoveParticipantService)
+
+    expect(IncidentManagement::OncallRotations::RemoveParticipantService)
+      .to receive(:new)
+      .with(rotation, user)
+      .and_return(remove_service)
+
+    expect(IncidentManagement::OncallRotations::RemoveParticipantService)
+      .to receive(:new)
+      .with(rotation_2, user)
+      .and_return(remove_service)
+
+    expect(remove_service).to receive(:execute).twice
+
+    execute
+  end
+end