Commit b85754e3 authored by Stan Hu's avatar Stan Hu

Merge branch 'master' into 'master'

"Fixes #xxxx" now shows up in the issue log for non-default branches. #2190

I don't understand why the commits containing "closing references" (like `closes #xxxx`) were not mentioned in the corresponding issues **when pushed to a non-default branch**.

So I tried to discover how it works -- hence learning Ruby! I don't expect that MR to pass, this is my very first attempt of contribution.

**Update:** my modifications are now done. To sum up:
- when a commit with a reference `fixes #xxxx` is pushed to a non-default branch, a cross-reference to that issue will be created;
- when that same commit is pushed to a default branch, no cross-reference will be created because a message `This commit closes issue` will be emitted.
- I also refined some of the existing tests and added 2 tests on the new behavior on non-default branches.

See merge request !1150
parents f0bdf7f8 a9e40917
...@@ -9,6 +9,7 @@ v 8.0.0 (unreleased) ...@@ -9,6 +9,7 @@ v 8.0.0 (unreleased)
- Allow displaying of archived projects in the admin interface (Artem Sidorenko) - Allow displaying of archived projects in the admin interface (Artem Sidorenko)
- Allow configuration of import sources for new projects (Artem Sidorenko) - Allow configuration of import sources for new projects (Artem Sidorenko)
- Search for comments should be case insensetive - 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) v 7.14.0 (unreleased)
- Fix bug where non-project members of the target project could set labels on new merge requests. - Fix bug where non-project members of the target project could set labels on new merge requests.
......
...@@ -78,24 +78,29 @@ class GitPushService ...@@ -78,24 +78,29 @@ class GitPushService
# For push with 1k commits it prevents 900+ requests in database # For push with 1k commits it prevents 900+ requests in database
author = nil 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 if issues_to_close.present? && is_default_branch
author ||= commit_user(commit) author ||= commit_user(commit)
actually_closed_issues = issues_to_close
issues_to_close.each do |issue| issues_to_close.each do |issue|
Issues::CloseService.new(project, author, {}).execute(issue, commit) Issues::CloseService.new(project, author, {}).execute(issue, commit)
end end
end end
if project.default_issues_tracker? if project.default_issues_tracker?
create_cross_reference_notes(commit, issues_to_close) create_cross_reference_notes(commit, actually_closed_issues)
end end
end end
end end
def create_cross_reference_notes(commit, issues_to_close) 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 # Create cross-reference notes for any other references than those given in issues_to_close.
# issue-closing phrase, or have already been mentioned from this commit (probably from this commit # Omit any issues that were referenced in an issue-closing phrase, or have already been
# being pushed to a different branch). # mentioned from this commit (probably from this commit being pushed to a different branch).
refs = commit.references(project, user) - issues_to_close refs = commit.references(project, user) - issues_to_close
refs.reject! { |r| commit.has_mentioned?(r) } refs.reject! { |r| commit.has_mentioned?(r) }
......
...@@ -197,7 +197,7 @@ describe GitPushService do ...@@ -197,7 +197,7 @@ describe GitPushService do
end end
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(:issue) { create :issue, project: project }
let(:other_issue) { create :issue, project: project } let(:other_issue) { create :issue, project: project }
let(:commit_author) { create :user } let(:commit_author) { create :user }
...@@ -215,27 +215,20 @@ describe GitPushService do ...@@ -215,27 +215,20 @@ describe GitPushService do
and_return([closing_commit]) and_return([closing_commit])
end end
it "closes issues with commit messages" do context "to default branches" do
it "closes issues" do
service.execute(project, user, @oldrev, @newrev, @ref) service.execute(project, user, @oldrev, @newrev, @ref)
expect(Issue.find(issue.id)).to be_closed expect(Issue.find(issue.id)).to be_closed
end end
it "doesn't create cross-reference notes for a closing reference" do it "adds a note indicating that the issue is now closed" do
expect do expect(SystemNoteService).to receive(:change_status).with(issue, project, commit_author, "closed", closing_commit)
service.execute(project, user, @oldrev, @newrev, @ref) 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 it "doesn't create additional cross-reference notes" do
allow(project).to receive(:default_branch).and_return('durf') expect(SystemNoteService).not_to receive(:cross_reference)
service.execute(project, user, @oldrev, @newrev, @ref)
# 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 }
expect(Issue.find(issue.id)).to be_opened
end end
it "doesn't close issues when external issue tracker is in use" do it "doesn't close issues when external issue tracker is in use" do
...@@ -248,6 +241,24 @@ describe GitPushService do ...@@ -248,6 +241,24 @@ describe GitPushService do
end end
end end
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
it "doesn't close issues" do
service.execute(project, user, @oldrev, @newrev, @ref)
expect(Issue.find(issue.id)).to be_opened
end
end
end
describe "empty project" do describe "empty project" do
let(:project) { create(:project_empty_repo) } let(:project) { create(:project_empty_repo) }
let(:new_ref) { 'refs/heads/feature'} let(:new_ref) { 'refs/heads/feature'}
......
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