Merge branch '16050-improve-first-mentioned-issue-metric-calc' into 'master'

Adjust issue metrics first_mentioned_in_commit_at calculation

Closes #16050

See merge request gitlab-org/gitlab!20923

app/models/issue/metrics.rb

@@ -3,6 +3,12 @@
 class Issue::Metrics < ApplicationRecord
   belongs_to :issue
 
+  scope :for_issues, ->(issues) { where(issue: issues) }
+  scope :with_first_mention_not_earlier_than, -> (timestamp) {
+    where(first_mentioned_in_commit_at: nil)
+      .or(where(arel_table['first_mentioned_in_commit_at'].gteq(timestamp)))
+  }
+
   def record!
     if issue.milestone_id.present? && self.first_associated_with_milestone_at.blank?
       self.first_associated_with_milestone_at = Time.now

app/workers/process_commit_worker.rb

@@ -55,16 +55,15 @@ class ProcessCommitWorker
     end
   end
 
-  # rubocop: disable CodeReuse/ActiveRecord
   def update_issue_metrics(commit, author)
     mentioned_issues = commit.all_references(author).issues
 
     return if mentioned_issues.empty?
 
-    Issue::Metrics.where(issue_id: mentioned_issues.map(&:id), first_mentioned_in_commit_at: nil)
+    Issue::Metrics.for_issues(mentioned_issues)
+      .with_first_mention_not_earlier_than(commit.committed_date)
       .update_all(first_mentioned_in_commit_at: commit.committed_date)
   end
-  # rubocop: enable CodeReuse/ActiveRecord
 
   def build_commit(project, hash)
     date_suffix = '_date'

changelogs/unreleased/16050-improve-first-mentioned-issue-metric-calc.yml

+---
+title: Adjust issue metrics first_mentioned_in_commit_at calculation
+merge_request: 20923
+author:
+type: fixed

spec/models/issue/metrics_spec.rb

@@ -7,6 +7,33 @@ describe Issue::Metrics do
 
   subject { create(:issue, project: project) }
 
+  describe '.for_issues' do
+    subject(:scope) { described_class.for_issues([issue1, issue2]) }
+
+    let(:issue1) { create(:issue) }
+    let(:issue2) { create(:issue) }
+
+    it 'returns metrics associated with given issues' do
+      create(:issue)
+
+      expect(scope).to match_array([issue1.metrics, issue2.metrics])
+    end
+  end
+
+  describe '.with_first_mention_not_earlier_than' do
+    subject(:scope) { described_class.with_first_mention_not_earlier_than(timestamp) }
+
+    let(:timestamp) { DateTime.now }
+
+    it 'returns metrics without mentioning in commit or with mentioning after given timestamp' do
+      issue1 = create(:issue)
+      issue2 = create(:issue).tap { |i| i.metrics.update!(first_mentioned_in_commit_at: timestamp + 1.day) }
+      create(:issue).tap { |i| i.metrics.update!(first_mentioned_in_commit_at: timestamp - 1.day) }
+
+      expect(scope).to match_array([issue1.metrics, issue2.metrics])
+    end
+  end
+
   describe "when recording the default set of issue metrics on issue save" do
     context "milestones" do
       it "records the first time an issue is associated with a milestone" do

spec/workers/process_commit_worker_spec.rb

@@ -129,21 +129,54 @@ describe ProcessCommitWorker do
   end
 
   describe '#update_issue_metrics' do
-    it 'updates any existing issue metrics' do
-      allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}")
+    context 'when commit has issue reference' do
+      subject(:update_metrics_and_reload) do
+        -> {
+          worker.update_issue_metrics(commit, user)
+          issue.metrics.reload
+        }
+      end
+
+      before do
+        allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}")
+      end
 
-      worker.update_issue_metrics(commit, user)
+      context 'when issue has no first_mentioned_in_commit_at set' do
+        it 'updates issue metrics' do
+          expect(update_metrics_and_reload)
+            .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date)
+        end
+      end
 
-      metric = Issue::Metrics.first
+      context 'when issue has first_mentioned_in_commit_at earlier than given committed_date' do
+        before do
+          issue.metrics.update(first_mentioned_in_commit_at: commit.committed_date - 1.day)
+        end
 
-      expect(metric.first_mentioned_in_commit_at).to eq(commit.committed_date)
+        it "doesn't update issue metrics" do
+          expect(update_metrics_and_reload).not_to change { issue.metrics.first_mentioned_in_commit_at }
+        end
+      end
+
+      context 'when issue has first_mentioned_in_commit_at later than given committed_date' do
+        before do
+          issue.metrics.update(first_mentioned_in_commit_at: commit.committed_date + 1.day)
+        end
+
+        it "doesn't update issue metrics" do
+          expect(update_metrics_and_reload)
+            .to change { issue.metrics.first_mentioned_in_commit_at }.to(commit.committed_date)
+        end
+      end
     end
 
-    it "doesn't execute any queries with false conditions" do
-      allow(commit).to receive(:safe_message).and_return("Lorem Ipsum")
+    context 'when commit has no issue references' do
+      it "doesn't execute any queries with false conditions" do
+        allow(commit).to receive(:safe_message).and_return("Lorem Ipsum")
 
-      expect { worker.update_issue_metrics(commit, user) }
-        .not_to make_queries_matching(/WHERE (?:1=0|0=1)/)
+        expect { worker.update_issue_metrics(commit, user) }
+          .not_to make_queries_matching(/WHERE (?:1=0|0=1)/)
+      end
     end
   end