Commit 0f657ee8 authored by Pavel Shutsin's avatar Pavel Shutsin

Adjust issue metrics first_mentioned_in_commit_at calculation

We should consider commits committed_date which comes earlier
than metrics value in database while updating
first_mentioned_in_commit_at.
This way we'll calculate first mentioned date properly.
parent 4c7be075
...@@ -3,6 +3,12 @@ ...@@ -3,6 +3,12 @@
class Issue::Metrics < ApplicationRecord class Issue::Metrics < ApplicationRecord
belongs_to :issue 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! def record!
if issue.milestone_id.present? && self.first_associated_with_milestone_at.blank? if issue.milestone_id.present? && self.first_associated_with_milestone_at.blank?
self.first_associated_with_milestone_at = Time.now self.first_associated_with_milestone_at = Time.now
......
...@@ -55,16 +55,15 @@ class ProcessCommitWorker ...@@ -55,16 +55,15 @@ class ProcessCommitWorker
end end
end end
# rubocop: disable CodeReuse/ActiveRecord
def update_issue_metrics(commit, author) def update_issue_metrics(commit, author)
mentioned_issues = commit.all_references(author).issues mentioned_issues = commit.all_references(author).issues
return if mentioned_issues.empty? 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) .update_all(first_mentioned_in_commit_at: commit.committed_date)
end end
# rubocop: enable CodeReuse/ActiveRecord
def build_commit(project, hash) def build_commit(project, hash)
date_suffix = '_date' date_suffix = '_date'
......
---
title: Adjust issue metrics first_mentioned_in_commit_at calculation
merge_request: 20923
author:
type: fixed
...@@ -7,6 +7,33 @@ describe Issue::Metrics do ...@@ -7,6 +7,33 @@ describe Issue::Metrics do
subject { create(:issue, project: project) } 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 describe "when recording the default set of issue metrics on issue save" do
context "milestones" do context "milestones" do
it "records the first time an issue is associated with a milestone" do it "records the first time an issue is associated with a milestone" do
......
...@@ -129,21 +129,54 @@ describe ProcessCommitWorker do ...@@ -129,21 +129,54 @@ describe ProcessCommitWorker do
end end
describe '#update_issue_metrics' do describe '#update_issue_metrics' do
it 'updates any existing issue metrics' do context 'when commit has issue reference' do
allow(commit).to receive(:safe_message).and_return("Closes #{issue.to_reference}") 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 end
it "doesn't execute any queries with false conditions" do context 'when commit has no issue references' do
allow(commit).to receive(:safe_message).and_return("Lorem Ipsum") 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) } expect { worker.update_issue_metrics(commit, user) }
.not_to make_queries_matching(/WHERE (?:1=0|0=1)/) .not_to make_queries_matching(/WHERE (?:1=0|0=1)/)
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment