Merge branch '262862-add-deleted-at-to-oncall-rotation-participant' into 'master'

Add is_removed to oncall participants table

See merge request gitlab-org/gitlab!54779

changelogs/unreleased/262862-add-deleted-at-to-oncall-rotation-participant.yml

+---
+title: Add is_removed column on Oncall Participant model
+merge_request: 54779
+author:
+type: added

db/migrate/20210222030537_add_is_removed_to_oncall_participant.rb

+# frozen_string_literal: true
+
+class AddIsRemovedToOncallParticipant < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def up
+    with_lock_retries do
+      add_column :incident_management_oncall_participants, :is_removed, :boolean, default: false, null: false
+    end
+  end
+
+  def down
+    with_lock_retries do
+      remove_column :incident_management_oncall_participants, :is_removed
+    end
+  end
+end

db/migrate/20210222042745_add_is_removed_index_to_oncall_participant.rb

+# frozen_string_literal: true
+
+class AddIsRemovedIndexToOncallParticipant < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+  EXISTING_INDEX_NAME = 'index_inc_mgmnt_oncall_participants_on_oncall_rotation_id'
+  NEW_INDEX_NAME = 'index_inc_mgmnt_oncall_pcpnt_on_oncall_rotation_id_is_removed'
+
+  def up
+    add_concurrent_index :incident_management_oncall_participants, [:oncall_rotation_id, :is_removed], name: NEW_INDEX_NAME
+    remove_concurrent_index_by_name(:incident_management_oncall_participants, EXISTING_INDEX_NAME)
+  end
+
+  def down
+    add_concurrent_index :incident_management_oncall_participants, :oncall_rotation_id, name: EXISTING_INDEX_NAME
+    remove_concurrent_index_by_name(:incident_management_oncall_participants, NEW_INDEX_NAME)
+  end
+end

db/schema_migrations/20210222030537

+44b0e2b3e32e45fb557a5c9c41f69de94a5d41bad43267f00090b9c527b2b1e1
\ No newline at end of file

db/schema_migrations/20210222042745

+88bbd8cbc4398c3c05a834c4d8d36ee55aca9d698da3cf3c1df0afe916bc051f
\ No newline at end of file

db/structure.sql

@@ -13235,7 +13235,8 @@ CREATE TABLE incident_management_oncall_participants (
     oncall_rotation_id bigint NOT NULL,
     user_id bigint NOT NULL,
     color_palette smallint NOT NULL,
-    color_weight smallint NOT NULL
+    color_weight smallint NOT NULL,
+    is_removed boolean DEFAULT false NOT NULL
 );
 
 CREATE SEQUENCE incident_management_oncall_participants_id_seq
@@ -22450,12 +22451,12 @@ CREATE INDEX index_import_failures_on_project_id_not_null ON import_failures USI
 
 CREATE INDEX index_imported_projects_on_import_type_creator_id_created_at ON projects USING btree (import_type, creator_id, created_at) WHERE (import_type IS NOT NULL);
 
-CREATE INDEX index_inc_mgmnt_oncall_participants_on_oncall_rotation_id ON incident_management_oncall_participants USING btree (oncall_rotation_id);
-
 CREATE INDEX index_inc_mgmnt_oncall_participants_on_oncall_user_id ON incident_management_oncall_participants USING btree (user_id);
 
 CREATE UNIQUE INDEX index_inc_mgmnt_oncall_participants_on_user_id_and_rotation_id ON incident_management_oncall_participants USING btree (user_id, oncall_rotation_id);
 
+CREATE INDEX index_inc_mgmnt_oncall_pcpnt_on_oncall_rotation_id_is_removed ON incident_management_oncall_participants USING btree (oncall_rotation_id, is_removed);
+
 CREATE UNIQUE INDEX index_inc_mgmnt_oncall_rotations_on_oncall_schedule_id_and_id ON incident_management_oncall_rotations USING btree (oncall_schedule_id, id);
 
 CREATE UNIQUE INDEX index_inc_mgmnt_oncall_rotations_on_oncall_schedule_id_and_name ON incident_management_oncall_rotations USING btree (oncall_schedule_id, name);

ee/app/models/incident_management/oncall_participant.rb

@@ -20,5 +20,12 @@ module IncidentManagement
     validates :user, presence: true, uniqueness: { scope: :oncall_rotation_id }
 
     delegate :project, to: :rotation, allow_nil: true
+
+    scope :not_removed, -> { where(is_removed: false) }
+    scope :removed, -> { where(is_removed: true) }
+
+    def mark_as_removed
+      update_column(:is_removed, true)
+    end
   end
 end

ee/app/models/incident_management/oncall_rotation.rb

@@ -30,8 +30,9 @@ module IncidentManagement
     NAME_LENGTH = 200
 
     belongs_to :schedule, class_name: 'OncallSchedule', inverse_of: 'rotations', foreign_key: 'oncall_schedule_id'
-    # Note! If changing the order of participants, also change the :with_shift_generation_associations scope.
     has_many :participants, -> { order(id: :asc) }, class_name: 'OncallParticipant', inverse_of: :rotation
+    # Note! If changing the order of participants, also change the :with_shift_generation_associations scope.
+    has_many :active_participants, -> { not_removed.order(id: :asc) }, class_name: 'OncallParticipant', inverse_of: :rotation
     has_many :users, through: :participants
     has_many :shifts, class_name: 'OncallShift', inverse_of: :rotation, foreign_key: :rotation_id
 
@@ -49,9 +50,9 @@ module IncidentManagement
     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
-      joins(:participants, :schedule)
+      joins(:active_participants, :schedule)
         .distinct
-        .includes(:participants, :schedule)
+        .includes(:active_participants, :schedule)
         .order(:id, 'incident_management_oncall_participants.id ASC')
     end
 

ee/lib/incident_management/oncall_shift_generator.rb

@@ -19,7 +19,7 @@ module IncidentManagement
       ends_at = limit_end_time(apply_timezone(ends_at))
 
       return [] unless starts_at < ends_at
-      return [] unless rotation.participants.any?
+      return [] unless participants.any?
 
       # The first shift within the timeframe may begin before
       # the timeframe. We want to begin generating shifts
@@ -50,7 +50,7 @@ module IncidentManagement
 
       return if timestamp < rotation_starts_at
       return if rotation_ends_at && rotation_ends_at <= timestamp
-      return unless rotation.participants.any?
+      return unless participants.any?
 
       elapsed_shift_cycle_count = elapsed_whole_shift_cycles(timestamp)
       shift_cycle_starts_at = shift_cycle_start_time(elapsed_shift_cycle_count)
@@ -175,7 +175,7 @@ module IncidentManagement
 
     def participants
       strong_memoize(:participants) do
-        rotation.participants
+        rotation.active_participants
       end
     end
 

ee/spec/factories/incident_management/oncall_participant.rb

@@ -12,5 +12,9 @@ FactoryBot.define do
         participant.rotation.project.add_developer(participant.user)
       end
     end
+
+    trait :removed do
+      is_removed { true }
+    end
   end
 end

ee/spec/models/incident_management/oncall_participant_spec.rb

@@ -5,6 +5,7 @@ require 'spec_helper'
 RSpec.describe IncidentManagement::OncallParticipant do
   let_it_be(:rotation) { create(:incident_management_oncall_rotation) }
   let_it_be(:user) { create(:user) }
+  let_it_be(:participant) { create(:incident_management_oncall_participant, rotation: rotation) }
 
   subject { build(:incident_management_oncall_participant, rotation: rotation, user: user) }
 
@@ -38,6 +39,30 @@ RSpec.describe IncidentManagement::OncallParticipant do
     end
   end
 
+  describe 'scopes' do
+    let_it_be(:removed_participant) { create(:incident_management_oncall_participant, :removed, rotation: rotation) }
+
+    describe '.not_removed' do
+      subject { described_class.not_removed }
+
+      it { is_expected.to contain_exactly(participant) }
+    end
+
+    describe '.removed' do
+      subject { described_class.removed }
+
+      it { is_expected.to contain_exactly(removed_participant) }
+    end
+  end
+
+  describe '#mark_as_removed' do
+    subject { participant.mark_as_removed }
+
+    it 'updates is_removed to true' do
+      expect { subject }.to change { participant.reload.is_removed }.to(true)
+    end
+  end
+
   private
 
   def remove_user_from_project(user, project)

ee/spec/models/incident_management/oncall_rotation_spec.rb

@@ -8,8 +8,19 @@ RSpec.describe IncidentManagement::OncallRotation do
   describe '.associations' do
     it { is_expected.to belong_to(:schedule).class_name('OncallSchedule').inverse_of(:rotations) }
     it { is_expected.to have_many(:participants).order(id: :asc).class_name('OncallParticipant').inverse_of(:rotation) }
+    it { is_expected.to have_many(:active_participants).order(id: :asc).class_name('OncallParticipant').inverse_of(:rotation) }
     it { is_expected.to have_many(:users).through(:participants) }
     it { is_expected.to have_many(:shifts).class_name('OncallShift').inverse_of(:rotation) }
+
+    describe '.active_participants' do
+      let(:rotation) { create(:incident_management_oncall_rotation, schedule: schedule) }
+      let(:participant) { create(:incident_management_oncall_participant, rotation: rotation) }
+      let(:removed_participant) { create(:incident_management_oncall_participant, :removed, rotation: rotation) }
+
+      subject { rotation.active_participants }
+
+      it { is_expected.to contain_exactly(participant) }
+    end
   end
 
   describe '.validations' do