Eliminate duplicated VSA event

This change eliminates a duplicated value stream analytics event. The
event definition exists in both EE and FOSS with different identifier
(numeric). We can safely remove the EE definition and migrate all
existing records to use the FOSS identifier.

app/models/concerns/analytics/cycle_analytics/stage.rb

@@ -49,6 +49,14 @@ module Analytics
         end
       end
 
+      def start_event_identifier
+        backward_compatible_identifier(:start_event_identifier) || super
+      end
+
+      def end_event_identifier
+        backward_compatible_identifier(:end_event_identifier) || super
+      end
+
       def start_event_label_based?
         start_event_identifier && start_event.label_based?
       end
@@ -128,6 +136,17 @@ module Analytics
           .id_in(label_id)
           .exists?
       end
+
+      # Temporary, will be removed in 13.10
+      def backward_compatible_identifier(attribute_name)
+        removed_identifier = 6 # References IssueFirstMentionedInCommit removed on https://gitlab.com/gitlab-org/gitlab/-/merge_requests/51975
+        replacement_identifier = :issue_first_mentioned_in_commit
+
+        # ActiveRecord returns nil if the column value is not part of the Enum definition
+        if self[attribute_name].nil? && read_attribute_before_type_cast(attribute_name) == removed_identifier
+          replacement_identifier
+        end
+      end
     end
   end
 end

changelogs/unreleased/214631-remove-duplicated-vsa-stage-event.yml

+---
+title: Migrate incorrect value stream group stage records
+merge_request: 51975
+author:
+type: fixed

db/post_migrate/20210119122354_alter_vsa_issue_first_mentioned_in_commit_value.rb

+# frozen_string_literal: true
+
+class AlterVsaIssueFirstMentionedInCommitValue < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  ISSUE_FIRST_MENTIONED_IN_COMMIT_FOSS = 2
+  ISSUE_FIRST_MENTIONED_IN_COMMIT_EE = 6
+
+  class GroupStage < ActiveRecord::Base
+    self.table_name = 'analytics_cycle_analytics_group_stages'
+
+    include EachBatch
+  end
+
+  def up
+    GroupStage.each_batch(of: 100) do |relation|
+      relation
+        .where(start_event_identifier: ISSUE_FIRST_MENTIONED_IN_COMMIT_EE)
+        .update_all(start_event_identifier: ISSUE_FIRST_MENTIONED_IN_COMMIT_FOSS)
+
+      relation
+        .where(end_event_identifier: ISSUE_FIRST_MENTIONED_IN_COMMIT_EE)
+        .update_all(end_event_identifier: ISSUE_FIRST_MENTIONED_IN_COMMIT_FOSS)
+    end
+  end
+
+  def down
+    # rollback is not needed, the identifier "6" is the same as identifier "2" on the application level
+  end
+end

db/schema_migrations/20210119122354

+e8264993f6503268cd99e8ca26ccdc0e986f31a2818b9bbef2a9cef36191e686
\ No newline at end of file

ee/lib/ee/gitlab/analytics/cycle_analytics/stage_events.rb

@@ -15,7 +15,6 @@ module EE
             ::Gitlab::Analytics::CycleAnalytics::StageEvents::IssueClosed => 3,
             ::Gitlab::Analytics::CycleAnalytics::StageEvents::IssueFirstAddedToBoard => 4,
             ::Gitlab::Analytics::CycleAnalytics::StageEvents::IssueFirstAssociatedWithMilestone => 5,
-            ::Gitlab::Analytics::CycleAnalytics::StageEvents::IssueFirstMentionedInCommit => 6,
             ::Gitlab::Analytics::CycleAnalytics::StageEvents::IssueLastEdited => 7,
             ::Gitlab::Analytics::CycleAnalytics::StageEvents::IssueLabelAdded => 8,
             ::Gitlab::Analytics::CycleAnalytics::StageEvents::IssueLabelRemoved => 9,
@@ -148,7 +147,7 @@ module EE
             override :events
             def events
               strong_memoize(:events) do
-                (super + EE_EVENTS).uniq
+                (super + EE_EVENTS)
               end
             end
 

ee/lib/gitlab/analytics/cycle_analytics/stage_events/issue_first_mentioned_in_commit.rb

-# frozen_string_literal: true
-
-module Gitlab
-  module Analytics
-    module CycleAnalytics
-      module StageEvents
-        class IssueFirstMentionedInCommit < MetricsBasedStageEvent
-          def self.name
-            s_("CycleAnalyticsEvent|Issue first mentioned in a commit")
-          end
-
-          def self.identifier
-            :issue_first_mentioned_in_commit
-          end
-
-          def object_type
-            Issue
-          end
-
-          def timestamp_projection
-            issue_metrics_table[:first_mentioned_in_commit_at]
-          end
-        end
-      end
-    end
-  end
-end

ee/spec/lib/gitlab/analytics/cycle_analytics/stage_events/issue_first_mentioned_in_commit_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-RSpec.describe Gitlab::Analytics::CycleAnalytics::StageEvents::IssueFirstMentionedInCommit do
-  it_behaves_like 'value stream analytics event'
-end

ee/spec/models/analytics/cycle_analytics/group_stage_spec.rb

@@ -33,4 +33,38 @@ RSpec.describe Analytics::CycleAnalytics::GroupStage do
       let(:default_params) { { group: parent } }
     end
   end
+
+  context 'when the event identifier is using the old, recently deduplicated identifier' do
+    let(:group) { create(:group) }
+    let(:value_stream) { create(:cycle_analytics_group_value_stream, group: group) }
+    let(:invalid_identifier) { 6 }
+
+    let(:stage_params) do
+      {
+        name: 'My Stage',
+        parent: group,
+        start_event_identifier: :merge_request_created,
+        end_event_identifier: :merge_request_merged,
+        value_stream: value_stream
+      }
+    end
+
+    let(:stage) { described_class.create!(stage_params) }
+
+    before do
+      # update the columns directly so validations are skipped
+      stage.update_column(:start_event_identifier, invalid_identifier)
+      stage.update_column(:end_event_identifier, invalid_identifier)
+    end
+
+    subject { described_class.find(stage.id) }
+
+    it 'loads the correct start event' do
+      expect(subject.start_event).to be_a_kind_of(Gitlab::Analytics::CycleAnalytics::StageEvents::IssueFirstMentionedInCommit)
+    end
+
+    it 'loads the correct end event' do
+      expect(subject.end_event).to be_a_kind_of(Gitlab::Analytics::CycleAnalytics::StageEvents::IssueFirstMentionedInCommit)
+    end
+  end
 end

spec/migrations/20210119122354_alter_vsa_issue_first_mentioned_in_commit_value_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require Rails.root.join('db', 'post_migrate', '20210119122354_alter_vsa_issue_first_mentioned_in_commit_value.rb')
+
+RSpec.describe AlterVsaIssueFirstMentionedInCommitValue, schema: 20210114033715 do
+  let(:group_stages) { table(:analytics_cycle_analytics_group_stages) }
+  let(:value_streams) { table(:analytics_cycle_analytics_group_value_streams) }
+  let(:namespaces) { table(:namespaces) }
+
+  let(:namespace) { namespaces.create!(id: 1, name: 'group', path: 'group') }
+  let(:value_stream) { value_streams.create!(name: 'test', group_id: namespace.id) }
+
+  let!(:stage_1) { group_stages.create!(group_value_stream_id: value_stream.id, group_id: namespace.id, name: 'stage 1', start_event_identifier: described_class::ISSUE_FIRST_MENTIONED_IN_COMMIT_EE, end_event_identifier: 1) }
+  let!(:stage_2) { group_stages.create!(group_value_stream_id: value_stream.id, group_id: namespace.id, name: 'stage 2', start_event_identifier: 2, end_event_identifier: described_class::ISSUE_FIRST_MENTIONED_IN_COMMIT_EE) }
+  let!(:stage_3) { group_stages.create!(group_value_stream_id: value_stream.id, group_id: namespace.id, name: 'stage 3', start_event_identifier: described_class::ISSUE_FIRST_MENTIONED_IN_COMMIT_FOSS, end_event_identifier: 3) }
+
+  describe '#up' do
+    it 'changes the EE specific identifier values to the FOSS version' do
+      migrate!
+
+      expect(stage_1.reload.start_event_identifier).to eq(described_class::ISSUE_FIRST_MENTIONED_IN_COMMIT_FOSS)
+      expect(stage_2.reload.end_event_identifier).to eq(described_class::ISSUE_FIRST_MENTIONED_IN_COMMIT_FOSS)
+    end
+
+    it 'does not change irrelevant records' do
+      expect { migrate! }.not_to change { stage_3.reload }
+    end
+  end
+end