diff --git a/CHANGELOG b/CHANGELOG index dd5162a53db1b8469ffdc42f8979b4c4548cd323..fe0992b5a0c0600dfb0166facf83024690ba855b 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 81535450ac15f6cc0974f3d6452b7b74c3034197..0a73244774a20c22ea070c97215a6f4854d4e4a6 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 62cef9db5344f1754c156cf4ca551287d44b0598..c483060fd73994235eb2c069bb936b79ce78ad60 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