From a9e409179bb6be53e1fb45e930739fc73dbb77ea Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ma=C3=ABl=20Valais?= <mael.valais@univ-tlse3.fr>
Date: Fri, 21 Aug 2015 15:05:43 +0200
Subject: [PATCH] Create cross-reference for closing references on commits
pushed to non-default branches.
I also revamped the tests on "closing reference commits" (= "Fixes #xxxx" for example).
Now, there are two different contexts:
- when the commits with "closing reference" are pushed to the default branch,
- when the commits with "closing reference" are pushed to a non-default branch.
Closes gitlab-org/gitlab-ce#2190.
---
CHANGELOG | 1 +
app/services/git_push_service.rb | 15 ++++---
spec/services/git_push_service_spec.rb | 57 +++++++++++++++-----------
3 files changed, 45 insertions(+), 28 deletions(-)
diff --git a/CHANGELOG b/CHANGELOG
index dd5162a53db..fe0992b5a0c 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -9,6 +9,7 @@ v 8.0.0 (unreleased)
- Allow displaying of archived projects in the admin interface (Artem Sidorenko)
- Allow configuration of import sources for new projects (Artem Sidorenko)
- Search for comments should be case insensetive
+ - Create cross-reference for closing references on commits pushed to non-default branches (Maël Valais)
v 7.14.0 (unreleased)
- Fix bug where non-project members of the target project could set labels on new merge requests.
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index 81535450ac1..0a73244774a 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -78,24 +78,29 @@ class GitPushService
# For push with 1k commits it prevents 900+ requests in database
author = nil
+ # Keep track of the issues that will be actually closed because they are on a default branch.
+ # Hence, when creating cross-reference notes, the not-closed issues (on non-default branches)
+ # will also have cross-reference.
+ actually_closed_issues = []
+
if issues_to_close.present? && is_default_branch
author ||= commit_user(commit)
-
+ actually_closed_issues = issues_to_close
issues_to_close.each do |issue|
Issues::CloseService.new(project, author, {}).execute(issue, commit)
end
end
if project.default_issues_tracker?
- create_cross_reference_notes(commit, issues_to_close)
+ create_cross_reference_notes(commit, actually_closed_issues)
end
end
end
def create_cross_reference_notes(commit, issues_to_close)
- # Create cross-reference notes for any other references. Omit any issues that were referenced in an
- # issue-closing phrase, or have already been mentioned from this commit (probably from this commit
- # being pushed to a different branch).
+ # Create cross-reference notes for any other references than those given in issues_to_close.
+ # Omit any issues that were referenced in an issue-closing phrase, or have already been
+ # mentioned from this commit (probably from this commit being pushed to a different branch).
refs = commit.references(project, user) - issues_to_close
refs.reject! { |r| commit.has_mentioned?(r) }
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 62cef9db534..c483060fd73 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -197,7 +197,7 @@ describe GitPushService do
end
end
- describe "closing issues from pushed commits" do
+ describe "closing issues from pushed commits containing a closing reference" do
let(:issue) { create :issue, project: project }
let(:other_issue) { create :issue, project: project }
let(:commit_author) { create :user }
@@ -215,36 +215,47 @@ describe GitPushService do
and_return([closing_commit])
end
- it "closes issues with commit messages" do
- service.execute(project, user, @oldrev, @newrev, @ref)
-
- expect(Issue.find(issue.id)).to be_closed
- end
+ context "to default branches" do
+ it "closes issues" do
+ service.execute(project, user, @oldrev, @newrev, @ref)
+ expect(Issue.find(issue.id)).to be_closed
+ end
- it "doesn't create cross-reference notes for a closing reference" do
- expect do
+ it "adds a note indicating that the issue is now closed" do
+ expect(SystemNoteService).to receive(:change_status).with(issue, project, commit_author, "closed", closing_commit)
service.execute(project, user, @oldrev, @newrev, @ref)
- end.not_to change { Note.where(project_id: project.id, system: true, commit_id: closing_commit.id).count }
- end
+ end
- it "doesn't close issues when pushed to non-default branches" do
- allow(project).to receive(:default_branch).and_return('durf')
+ it "doesn't create additional cross-reference notes" do
+ expect(SystemNoteService).not_to receive(:cross_reference)
+ service.execute(project, user, @oldrev, @newrev, @ref)
+ end
- # The push still shouldn't create cross-reference notes.
- expect do
- service.execute(project, user, @oldrev, @newrev, 'refs/heads/hurf')
- end.not_to change { Note.where(project_id: project.id, system: true).count }
+ it "doesn't close issues when external issue tracker is in use" do
+ allow(project).to receive(:default_issues_tracker?).and_return(false)
- expect(Issue.find(issue.id)).to be_opened
+ # The push still shouldn't create cross-reference notes.
+ expect do
+ service.execute(project, user, @oldrev, @newrev, 'refs/heads/hurf')
+ end.not_to change { Note.where(project_id: project.id, system: true).count }
+ end
end
- it "doesn't close issues when external issue tracker is in use" do
- allow(project).to receive(:default_issues_tracker?).and_return(false)
+ context "to non-default branches" do
+ before do
+ # Make sure the "default" branch is different
+ allow(project).to receive(:default_branch).and_return('not-master')
+ end
+
+ it "creates cross-reference notes" do
+ expect(SystemNoteService).to receive(:cross_reference).with(issue, closing_commit, commit_author)
+ service.execute(project, user, @oldrev, @newrev, @ref)
+ end
- # The push still shouldn't create cross-reference notes.
- expect do
- service.execute(project, user, @oldrev, @newrev, 'refs/heads/hurf')
- end.not_to change { Note.where(project_id: project.id, system: true).count }
+ it "doesn't close issues" do
+ service.execute(project, user, @oldrev, @newrev, @ref)
+ expect(Issue.find(issue.id)).to be_opened
+ end
end
end
--
GitLab