Merge branch 'track-iteration-change-events' into 'master'

Track iteration change events

See merge request gitlab-org/gitlab!37620

app/services/resource_events/base_change_timebox_service.rb

+# frozen_string_literal: true
+
+module ResourceEvents
+  class BaseChangeTimeboxService
+    attr_reader :resource, :user, :event_created_at
+
+    def initialize(resource, user, created_at: Time.current)
+      @resource = resource
+      @user = user
+      @event_created_at = created_at
+    end
+
+    def execute
+      create_event
+
+      resource.expire_note_etag_cache
+    end
+
+    private
+
+    def create_event
+      raise NotImplementedError
+    end
+
+    def build_resource_args
+      key = resource.class.name.foreign_key
+
+      {
+        user_id: user.id,
+        created_at: event_created_at,
+        key => resource.id
+      }
+    end
+  end
+end

app/services/resource_events/change_milestone_service.rb

 # frozen_string_literal: true
 
 module ResourceEvents
-  class ChangeMilestoneService
-    attr_reader :resource, :user, :event_created_at, :milestone, :old_milestone
+  class ChangeMilestoneService < BaseChangeTimeboxService
+    attr_reader :milestone, :old_milestone
 
     def initialize(resource, user, created_at: Time.current, old_milestone:)
-      @resource = resource
-      @user = user
-      @event_created_at = created_at
+      super(resource, user, created_at: created_at)
+
       @milestone = resource&.milestone
       @old_milestone = old_milestone
     end
 
-    def execute
-      ResourceMilestoneEvent.create(build_resource_args)
+    private
 
-      resource.expire_note_etag_cache
+    def create_event
+      ResourceMilestoneEvent.create(build_resource_args)
     end
 
-    private
-
     def build_resource_args
       action = milestone.blank? ? :remove : :add
-      key = resource.class.name.foreign_key
 
-      {
-        user_id: user.id,
-        created_at: event_created_at,
-        milestone_id: action == :add ? milestone&.id : old_milestone&.id,
+      super.merge({
         state: ResourceMilestoneEvent.states[resource.state],
-        action: ResourceMilestoneEvent.actions[action],
-        key => resource.id
-      }
+        action: ResourceTimeboxEvent.actions[action],
+        milestone_id: milestone.blank? ? old_milestone&.id : milestone&.id
+      })
     end
   end
 end

ee/app/models/concerns/ee/iteration_eventable.rb

+# frozen_string_literal: true
+
+module EE
+  module IterationEventable
+    extend ActiveSupport::Concern
+
+    included do
+      has_many :resource_iteration_events
+    end
+  end
+end

ee/app/models/ee/issue.rb

@@ -15,6 +15,7 @@ module EE
       include Elastic::ApplicationVersionedSearch
       include UsageStatistics
       include WeightEventable
+      include IterationEventable
       include HealthStatus
 
       scope :order_blocking_issues_desc, -> { reorder(blocking_issues_count: :desc) }

ee/app/models/iteration_note.rb

+# frozen_string_literal: true
+
+class IterationNote < ::SyntheticNote
+  attr_accessor :iteration
+
+  def self.from_event(event, resource: nil, resource_parent: nil)
+    attrs = note_attributes('iteration', event, resource, resource_parent).merge(iteration: event.iteration)
+
+    IterationNote.new(attrs)
+  end
+
+  def note_html
+    @note_html ||= Banzai::Renderer.cacheless_render_field(self, :note, { group: group, project: project })
+  end
+
+  private
+
+  def note_text(html: false)
+    event.remove? ? 'removed iteration' : "changed iteration to #{iteration.to_reference(resource_parent, format: :id)}"
+  end
+end

ee/app/services/ee/issuable/common_system_notes_service.rb

@@ -36,8 +36,12 @@ module EE
       def handle_iteration_change
         return unless issuable.previous_changes.include?('sprint_id')
 
+        if iteration_changes_tracking_enabled?
+          ::EE::ResourceEvents::ChangeIterationService.new(issuable, current_user, old_iteration_id: issuable.sprint_id_before_last_save).execute
+        else
           ::SystemNoteService.change_iteration(issuable, current_user, issuable.iteration)
         end
+      end
 
       def handle_weight_change(is_update)
         return unless issuable.previous_changes.include?('weight')
@@ -60,6 +64,10 @@ module EE
       def weight_changes_tracking_enabled?
         !issuable.is_a?(Epic) && ::Feature.enabled?(:track_issue_weight_change_events, issuable.project, default_enabled: true)
       end
+
+      def iteration_changes_tracking_enabled?
+        ::Feature.enabled?(:track_iteration_change_events, issuable.project)
+      end
     end
   end
 end

ee/app/services/ee/resource_events/change_iteration_service.rb

+# frozen_string_literal: true
+
+module EE
+  module ResourceEvents
+    class ChangeIterationService < ::ResourceEvents::BaseChangeTimeboxService
+      attr_reader :iteration, :old_iteration_id
+
+      def initialize(resource, user, created_at: Time.current, old_iteration_id:)
+        super(resource, user, created_at: created_at)
+
+        @iteration = resource&.iteration
+        @old_iteration_id = old_iteration_id
+      end
+
+      private
+
+      def create_event
+        ResourceIterationEvent.create(build_resource_args)
+      end
+
+      def build_resource_args
+        action = iteration.blank? ? :remove : :add
+
+        super.merge({
+          action: ResourceTimeboxEvent.actions[action],
+          iteration_id: iteration.blank? ? old_iteration_id : iteration&.id
+        })
+      end
+    end
+  end
+end

ee/app/services/ee/resource_events/merge_into_notes_service.rb

@@ -6,11 +6,22 @@ module EE
       extend ActiveSupport::Concern
       extend ::Gitlab::Utils::Override
 
+      EE_SYNTHETIC_NOTE_BUILDER_SERVICES = [
+        SyntheticWeightNotesBuilderService,
+        SyntheticIterationNotesBuilderService
+      ].freeze
+
       private
 
       override :synthetic_notes
       def synthetic_notes
-        super + SyntheticWeightNotesBuilderService.new(resource, current_user, params).execute
+        super + ee_synthetic_notes
+      end
+
+      def ee_synthetic_notes
+        EE_SYNTHETIC_NOTE_BUILDER_SERVICES.flat_map do |service|
+          service.new(resource, current_user, params).execute
+        end
       end
     end
   end

ee/app/services/ee/resource_events/synthetic_iteration_notes_builder_service.rb

+# frozen_string_literal: true
+
+# We store events about resource iteration changes in a separate table,
+# but we still want to display notes about iteration changes
+# as classic system notes in UI. This service generates "synthetic" notes for
+# iteration event changes.
+
+module EE
+  module ResourceEvents
+    class SyntheticIterationNotesBuilderService < ::ResourceEvents::BaseSyntheticNotesBuilderService
+      private
+
+      def synthetic_notes
+        iteration_change_events.map do |event|
+          IterationNote.from_event(event, resource: resource, resource_parent: resource_parent)
+        end
+      end
+
+      def iteration_change_events
+        return [] unless resource.respond_to?(:resource_iteration_events)
+
+        events = resource.resource_iteration_events.includes(user: :status) # rubocop: disable CodeReuse/ActiveRecord
+        apply_common_filters(events)
+      end
+    end
+  end
+end

ee/spec/models/issue_spec.rb

@@ -11,6 +11,7 @@ RSpec.describe Issue do
     subject { build(:issue) }
 
     it { is_expected.to have_many(:resource_weight_events) }
+    it { is_expected.to have_many(:resource_iteration_events) }
   end
 
   describe 'modules' do

ee/spec/models/iteration_note_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe IterationNote do
+  describe '.from_event' do
+    let(:author) { create(:user) }
+    let(:project) { create(:project, :repository) }
+    let(:noteable) { create(:issue, author: author, project: project) }
+    let(:event) { create(:resource_iteration_event, issue: noteable) }
+
+    subject { described_class.from_event(event, resource: noteable, resource_parent: project) }
+
+    it_behaves_like 'a synthetic note', 'iteration'
+
+    context 'with a remove iteration event' do
+      let(:iteration) { create(:iteration) }
+      let(:event) { create(:resource_iteration_event, action: :remove, issue: noteable, iteration: iteration) }
+
+      it 'creates the expected note' do
+        expect(subject.note_html).to include('removed iteration')
+        expect(subject.note_html).not_to include('changed iteration to')
+      end
+    end
+  end
+end

ee/spec/services/ee/issuable/common_system_notes_service_spec.rb

@@ -7,14 +7,51 @@ RSpec.describe Issuable::CommonSystemNotesService do
   let(:project) { create(:project) }
   let(:issuable) { create(:issue) }
 
+  RSpec.shared_examples 'issuable iteration changed' do
+    context 'when iteration is changed' do
+      let_it_be(:iteration) { create(:iteration) }
+
+      before do
+        issuable.update!(iteration: iteration)
+      end
+
+      it 'creates a resource iteration event' do
+        subject
+        event = issuable.reload.resource_iteration_events.last
+
+        expect(event).not_to be_nil
+        expect(event.iteration.id).to eq iteration.id
+        expect(event.user_id).to eq user.id
+      end
+
+      context 'when resource iteration event tracking is disabled' do
+        before do
+          stub_feature_flags(track_iteration_change_events: false)
+        end
+
+        it 'does not created a resource weight event' do
+          expect { subject }.not_to change { ResourceIterationEvent.count }
+        end
+
+        it 'does create a system note' do
+          expect { subject }.to change { Note.count }.from(0).to(1)
+
+          expect(Note.first.note).to eq("changed iteration to #{iteration.to_reference(issuable.resource_parent, format: :id)}")
+        end
+      end
+    end
+  end
+
   context 'on issuable update' do
+    subject { described_class.new(project, user).execute(issuable, old_labels: []) }
+
     context 'when weight is changed' do
       before do
         issuable.update!(weight: 5)
       end
 
-      it 'creates a resource label event' do
-        described_class.new(project, user).execute(issuable, old_labels: [])
+      it 'creates a resource weight event' do
+        subject
         event = issuable.reload.resource_weight_events.last
 
         expect(event).not_to be_nil
@@ -30,9 +67,7 @@ RSpec.describe Issuable::CommonSystemNotesService do
 
       context 'when setting a health_status' do
         it 'creates system note' do
-          expect do
-            described_class.new(project, user).execute(issuable, old_labels: [])
-          end.to change { Note.count }.from(0).to(1)
+          expect { subject }.to change { Note.count }.from(0).to(1)
 
           expect(Note.last.note).to eq('changed health status to **needs attention**')
         end
@@ -42,9 +77,7 @@ RSpec.describe Issuable::CommonSystemNotesService do
         it 'creates system note' do
           issuable.update!(health_status: nil)
 
-          expect do
-            described_class.new(project, user).execute(issuable, old_labels: [])
-          end.to change { Note.count }.from(0).to(1)
+          expect { subject }.to change { Note.count }.from(0).to(1)
 
           expect(Note.last.note).to eq('removed the health status')
         end
@@ -69,6 +102,8 @@ RSpec.describe Issuable::CommonSystemNotesService do
         expect(Note.second.note).to match('removed the finish date')
       end
     end
+
+    it_behaves_like 'issuable iteration changed'
   end
 
   context 'on issuable create' do
@@ -113,5 +148,7 @@ RSpec.describe Issuable::CommonSystemNotesService do
         expect(Note.first.note).to eq('changed weight to **5**')
       end
     end
+
+    it_behaves_like 'issuable iteration changed'
   end
 end

ee/spec/services/ee/issues/update_service_spec.rb

@@ -119,23 +119,67 @@ RSpec.describe Issues::UpdateService do
         group.add_maintainer(user)
       end
 
-      context 'group iterations' do
-        it 'creates a system note' do
-          group_iteration = create(:iteration, group: group)
+      context 'when track_iteration_change_events is disabled' do
+        before do
+          stub_feature_flags(track_iteration_change_events: false)
+        end
 
+        RSpec.shared_examples 'creates iteration system note' do
+          it 'creates a system note' do
             expect do
-            update_issue(iteration: group_iteration)
+              update_issue(iteration: iteration)
             end.to change { Note.system.count }.by(1)
           end
+
+          it 'does not create a iteration change event' do
+            expect do
+              update_issue(iteration: iteration)
+            end.not_to change { ResourceIterationEvent.count }
+          end
+        end
+
+        context 'group iterations' do
+          let(:iteration) { create(:iteration, group: group) }
+
+          it_behaves_like 'creates iteration system note'
         end
 
         context 'project iterations' do
+          let(:iteration) { create(:iteration, :skip_project_validation, project: project) }
+
+          it_behaves_like 'creates iteration system note'
+        end
+      end
+
+      context 'when track_iteration_change_events is enabled' do
+        before do
+          stub_feature_flags(track_iteration_change_events: true)
+        end
+
+        RSpec.shared_examples 'creates iteration resource event' do
           it 'creates a system note' do
-          project_iteration = create(:iteration, :skip_project_validation, project: project)
+            expect do
+              update_issue(iteration: iteration)
+            end.not_to change { Note.system.count }
+          end
 
+          it 'does not create a iteration change event' do
             expect do
-            update_issue(iteration: project_iteration)
-          end.to change { Note.system.count }.by(1)
+              update_issue(iteration: iteration)
+            end.to change { ResourceIterationEvent.count }.by(1)
+          end
+        end
+
+        context 'group iterations' do
+          let(:iteration) { create(:iteration, group: group) }
+
+          it_behaves_like 'creates iteration resource event'
+        end
+
+        context 'project iterations' do
+          let(:iteration) { create(:iteration, :skip_project_validation, project: project) }
+
+          it_behaves_like 'creates iteration resource event'
         end
       end
     end

ee/spec/services/ee/resource_events/change_iteration_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe EE::ResourceEvents::ChangeIterationService do
+  let_it_be(:timebox) { create(:iteration) }
+
+  let(:created_at_time) { Time.utc(2019, 12, 30) }
+  let(:add_timebox_args) { { created_at: created_at_time, old_iteration_id: nil } }
+  let(:remove_timebox_args) { { created_at: created_at_time, old_iteration_id: timebox.id } }
+
+  [:issue, :merge_request].each do |issuable|
+    it_behaves_like 'timebox(milestone or iteration) resource events creator', ResourceIterationEvent do
+      let_it_be(:resource) { create(issuable) } # rubocop:disable Rails/SaveBang
+    end
+  end
+end

ee/spec/services/ee/resource_events/synthetic_iteration_notes_builder_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe EE::ResourceEvents::SyntheticIterationNotesBuilderService do
+  describe '#execute' do
+    let_it_be(:user) { create(:user) }
+    let_it_be(:issue) { create(:issue, author: user) }
+
+    before do
+      create_list(:resource_iteration_event, 3, issue: issue)
+
+      stub_feature_flags(track_resource_iteration_change_events: false)
+    end
+
+    context 'when resource iteration events are disabled' do
+      # https://gitlab.com/gitlab-org/gitlab/-/issues/212985
+      it 'still builds notes for existing resource iteration events' do
+        notes = described_class.new(issue, user).execute
+
+        expect(notes.size).to eq(3)
+      end
+    end
+  end
+end

spec/lib/gitlab/import_export/all_models.yml

@@ -12,6 +12,7 @@ issues:
 - resource_weight_events
 - resource_milestone_events
 - resource_state_events
+- resource_iteration_events
 - sent_notifications
 - sentry_issue
 - label_links

spec/services/resource_events/change_milestone_service_spec.rb

@@ -3,9 +3,15 @@
 require 'spec_helper'
 
 RSpec.describe ResourceEvents::ChangeMilestoneService do
+  let_it_be(:timebox) { create(:milestone) }
+
+  let(:created_at_time) { Time.utc(2019, 12, 30) }
+  let(:add_timebox_args) { { created_at: created_at_time, old_milestone: nil } }
+  let(:remove_timebox_args) { { created_at: created_at_time, old_milestone: timebox } }
+
   [:issue, :merge_request].each do |issuable|
-    it_behaves_like 'a milestone events creator' do
-      let(:resource) { create(issuable) }
+    it_behaves_like 'timebox(milestone or iteration) resource events creator', ResourceMilestoneEvent do
+      let_it_be(:resource) { create(issuable) }
     end
   end
 end

spec/support/shared_examples/services/resource_events/change_milestone_service_shared_examples.rb

 # frozen_string_literal: true
 
-RSpec.shared_examples 'a milestone events creator' do
+RSpec.shared_examples 'timebox(milestone or iteration) resource events creator' do |timebox_event_class|
   let_it_be(:user) { create(:user) }
 
-  let(:created_at_time) { Time.utc(2019, 12, 30) }
-  let(:service) { described_class.new(resource, user, created_at: created_at_time, old_milestone: nil) }
-
-  context 'when milestone is present' do
-    let_it_be(:milestone) { create(:milestone) }
+  context 'when milestone/iteration is added' do
+    let(:service) { described_class.new(resource, user, add_timebox_args) }
 
     before do
-      resource.milestone = milestone
+      set_timebox(timebox_event_class, timebox)
     end
 
     it 'creates the expected event record' do
-      expect { service.execute }.to change { ResourceMilestoneEvent.count }.by(1)
+      expect { service.execute }.to change { timebox_event_class.count }.by(1)
 
-      expect_event_record(ResourceMilestoneEvent.last, action: 'add', milestone: milestone, state: 'opened')
+      expect_event_record(timebox_event_class, timebox_event_class.last, action: 'add', state: 'opened', timebox: timebox)
     end
   end
 
-  context 'when milestones is not present' do
+  context 'when milestone/iteration is removed' do
+    let(:service) { described_class.new(resource, user, remove_timebox_args) }
+
     before do
-      resource.milestone = nil
+      set_timebox(timebox_event_class, nil)
     end
 
-    let(:old_milestone) { create(:milestone, project: resource.project) }
-    let(:service) { described_class.new(resource, user, created_at: created_at_time, old_milestone: old_milestone) }
-
     it 'creates the expected event records' do
-      expect { service.execute }.to change { ResourceMilestoneEvent.count }.by(1)
+      expect { service.execute }.to change { timebox_event_class.count }.by(1)
 
-      expect_event_record(ResourceMilestoneEvent.last, action: 'remove', milestone: old_milestone, state: 'opened')
+      expect_event_record(timebox_event_class, timebox_event_class.last, action: 'remove', timebox: timebox, state: 'opened')
     end
   end
 
-  def expect_event_record(event, expected_attrs)
+  def expect_event_record(timebox_event_class, event, expected_attrs)
     expect(event.action).to eq(expected_attrs[:action])
-    expect(event.state).to eq(expected_attrs[:state])
     expect(event.user).to eq(user)
     expect(event.issue).to eq(resource) if resource.is_a?(Issue)
     expect(event.issue).to be_nil unless resource.is_a?(Issue)
     expect(event.merge_request).to eq(resource) if resource.is_a?(MergeRequest)
     expect(event.merge_request).to be_nil unless resource.is_a?(MergeRequest)
-    expect(event.milestone).to eq(expected_attrs[:milestone])
     expect(event.created_at).to eq(created_at_time)
+    expect_timebox(timebox_event_class, event, expected_attrs)
+  end
+
+  def set_timebox(timebox_event_class, timebox)
+    case timebox_event_class.name
+    when 'ResourceMilestoneEvent'
+      resource.milestone = timebox
+    when 'ResourceIterationEvent'
+      resource.iteration = timebox
+    end
+  end
+
+  def expect_timebox(timebox_event_class, event, expected_attrs)
+    case timebox_event_class.name
+    when 'ResourceMilestoneEvent'
+      expect(event.state).to eq(expected_attrs[:state])
+      expect(event.milestone).to eq(expected_attrs[:timebox])
+    when 'ResourceIterationEvent'
+      expect(event.iteration).to eq(expected_attrs[:timebox])
+    end
   end
 end