Commit eef63813 authored by Micaël Bergeron's avatar Micaël Bergeron

stop ProcessCommitWorker from processing MR merge commit

When a merge request is merged, it creates a commit with the
description of the MR, which may contain references and issue
closing references. As this will be handled in the PostMergeService
anyways, let's ignore merge commit generated from a MR.
parent dfb14e4d
...@@ -417,6 +417,10 @@ class Commit ...@@ -417,6 +417,10 @@ class Commit
!!(title =~ WIP_REGEX) !!(title =~ WIP_REGEX)
end end
def merged_merge_request?(user)
!!merged_merge_request(user)
end
private private
def commit_reference(from, referable_commit_id, full: false) def commit_reference(from, referable_commit_id, full: false)
...@@ -445,10 +449,6 @@ class Commit ...@@ -445,10 +449,6 @@ class Commit
changes changes
end end
def merged_merge_request?(user)
!!merged_merge_request(user)
end
def merged_merge_request_no_cache(user) def merged_merge_request_no_cache(user)
MergeRequestsFinder.new(user, project_id: project.id).find_by(merge_commit_sha: id) if merge_commit? MergeRequestsFinder.new(user, project_id: project.id).find_by(merge_commit_sha: id) if merge_commit?
end end
......
...@@ -23,27 +23,25 @@ class ProcessCommitWorker ...@@ -23,27 +23,25 @@ class ProcessCommitWorker
return unless user return unless user
commit = build_commit(project, commit_hash) commit = build_commit(project, commit_hash)
author = commit.author || user author = commit.author || user
process_commit_message(project, commit, user, author, default) # this is a GitLab generated commit message, ignore it.
return if commit.merged_merge_request?(user)
process_commit_message(project, commit, user, author, default)
update_issue_metrics(commit, author) update_issue_metrics(commit, author)
end end
def process_commit_message(project, commit, user, author, default = false) def process_commit_message(project, commit, user, author, default = false)
closed_issues = default ? commit.closes_issues(user) : [] closed_issues = default ? commit.closes_issues(user) : []
unless closed_issues.empty? close_issues(project, user, author, commit, closed_issues) if closed_issues.any?
close_issues(project, user, author, commit, closed_issues)
end
commit.create_cross_references!(author, closed_issues) commit.create_cross_references!(author, closed_issues)
end end
def close_issues(project, user, author, commit, issues) def close_issues(project, user, author, commit, issues)
# We don't want to run permission related queries for every single issue, # We don't want to run permission related queries for every single issue,
# therefor we use IssueCollection here and skip the authorization check in # therefore we use IssueCollection here and skip the authorization check in
# Issues::CloseService#execute. # Issues::CloseService#execute.
IssueCollection.new(issues).updatable_by_user(user).each do |issue| IssueCollection.new(issues).updatable_by_user(user).each do |issue|
Issues::CloseService.new(project, author) Issues::CloseService.new(project, author)
......
...@@ -20,6 +20,32 @@ describe ProcessCommitWorker do ...@@ -20,6 +20,32 @@ describe ProcessCommitWorker do
worker.perform(project.id, -1, commit.to_hash) worker.perform(project.id, -1, commit.to_hash)
end end
context 'when commit is a merge request merge commit' do
let(:merge_request) do
create(:merge_request,
description: "Closes #{issue.to_reference}",
source_branch: 'feature-merged',
target_branch: 'master',
source_project: project)
end
let(:commit) do
project.repository.create_branch('feature-merged', 'feature')
sha = project.repository.merge(user,
merge_request.diff_head_sha,
merge_request,
"Closes #{issue.to_reference}")
project.repository.commit(sha)
end
it 'does not process the commit' do
expect(worker).not_to receive(:close_issues)
worker.perform(project.id, user.id, commit.to_hash)
end
end
it 'processes the commit message' do it 'processes the commit message' do
expect(worker).to receive(:process_commit_message).and_call_original expect(worker).to receive(:process_commit_message).and_call_original
...@@ -49,10 +75,10 @@ describe ProcessCommitWorker do ...@@ -49,10 +75,10 @@ describe ProcessCommitWorker do
context 'when pushing to the default branch' do context 'when pushing to the default branch' do
it 'closes issues that should be closed per the commit message' do it 'closes issues that should be closed per the commit message' do
allow(commit).to receive(:safe_message) allow(commit).to receive(:safe_message)
.and_return("Closes #{issue.to_reference}") .and_return("Closes #{issue.to_reference}")
expect(worker).to receive(:close_issues) expect(worker).to receive(:close_issues)
.with(project, user, user, commit, [issue]) .with(project, user, user, commit, [issue])
worker.process_commit_message(project, commit, user, user, true) worker.process_commit_message(project, commit, user, user, true)
end end
...@@ -61,7 +87,7 @@ describe ProcessCommitWorker do ...@@ -61,7 +87,7 @@ describe ProcessCommitWorker do
context 'when pushing to a non-default branch' do context 'when pushing to a non-default branch' do
it 'does not close any issues' do it 'does not close any issues' do
allow(commit).to receive(:safe_message) allow(commit).to receive(:safe_message)
.and_return("Closes #{issue.to_reference}") .and_return("Closes #{issue.to_reference}")
expect(worker).not_to receive(:close_issues) expect(worker).not_to receive(:close_issues)
...@@ -103,7 +129,7 @@ describe ProcessCommitWorker do ...@@ -103,7 +129,7 @@ describe ProcessCommitWorker do
describe '#update_issue_metrics' do describe '#update_issue_metrics' do
it 'updates any existing issue metrics' do it 'updates any existing issue metrics' do
allow(commit).to receive(:safe_message) allow(commit).to receive(:safe_message)
.and_return("Closes #{issue.to_reference}") .and_return("Closes #{issue.to_reference}")
worker.update_issue_metrics(commit, user) worker.update_issue_metrics(commit, user)
...@@ -114,7 +140,7 @@ describe ProcessCommitWorker do ...@@ -114,7 +140,7 @@ describe ProcessCommitWorker do
it "doesn't execute any queries with false conditions" do it "doesn't execute any queries with false conditions" do
allow(commit).to receive(:safe_message) allow(commit).to receive(:safe_message)
.and_return("Lorem Ipsum") .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
...@@ -129,7 +155,7 @@ describe ProcessCommitWorker do ...@@ -129,7 +155,7 @@ describe ProcessCommitWorker do
it 'parses date strings into Time instances' do it 'parses date strings into Time instances' do
commit = worker commit = worker
.build_commit(project, id: '123', authored_date: Time.now.to_s) .build_commit(project, id: '123', authored_date: Time.now.to_s)
expect(commit.authored_date).to be_an_instance_of(Time) expect(commit.authored_date).to be_an_instance_of(Time)
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