summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2015-10-18 12:07:28 +0000
committerDouwe Maan <douwe@gitlab.com>2015-10-18 12:07:28 +0000
commit41615ddde4d5ae70bef944d9b156b687af2ca532 (patch)
treefaafdceffcafc9e241452eea2beb2920d98b946e
parent38785046f7ec7d834e22add66e4be88f6e985355 (diff)
parentbe10eea95632ba2d6553f0098d9d8536eef1f7c4 (diff)
downloadgitlab-ce-41615ddde4d5ae70bef944d9b156b687af2ca532.tar.gz
Merge branch 'cross-reference-mr-on-issues' into 'master'
Show merge requests which close current issue Closes #2903 ### What does this MR do If an issue is to be closed by a merge request the current user has access to, this will be displayed when the user looks at the issue. ![Screenshot_from_2015-10-12_12-10-37](https://gitlab.com/zj/gitlab-ce/uploads/52e429704e73067b24b69801f13ad7bc/Screenshot_from_2015-10-12_12-10-37.png) /cc @DouweM See merge request !1569
-rw-r--r--CHANGELOG1
-rw-r--r--app/assets/stylesheets/pages/issues.scss5
-rw-r--r--app/controllers/projects/issues_controller.rb7
-rw-r--r--app/controllers/projects/merge_requests_controller.rb2
-rw-r--r--app/helpers/issues_helper.rb4
-rw-r--r--app/models/concerns/issuable.rb4
-rw-r--r--app/models/issue.rb10
-rw-r--r--app/models/merge_request.rb8
-rw-r--r--app/views/projects/issues/_closed_by_box.html.haml3
-rw-r--r--app/views/projects/issues/show.html.haml3
-rw-r--r--spec/helpers/issues_helper_spec.rb10
-rw-r--r--spec/models/concerns/issuable_spec.rb1
-rw-r--r--spec/models/issue_spec.rb37
13 files changed, 88 insertions, 7 deletions
diff --git a/CHANGELOG b/CHANGELOG
index 8f17c5c2ba6..abf9ead6df7 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -8,6 +8,7 @@ v 8.1.0 (unreleased)
- Fix nonatomic database update potentially causing project star counts to go negative (Stan Hu)
- Fix error preventing displaying of commit data for a directory with a leading dot (Stan Hu)
- Speed up load times of issue detail pages by roughly 1.5x
+ - If a merge request is to close an issue, show this on the issue page (Zeger-Jan van de Weg)
- Add a system note and update relevant merge requests when a branch is deleted or re-added (Stan Hu)
- Make diff file view easier to use on mobile screens (Stan Hu)
- Improved performance of finding users by username or Email address
diff --git a/app/assets/stylesheets/pages/issues.scss b/app/assets/stylesheets/pages/issues.scss
index 4bf58cb4a59..41c069f0ad3 100644
--- a/app/assets/stylesheets/pages/issues.scss
+++ b/app/assets/stylesheets/pages/issues.scss
@@ -132,6 +132,11 @@ form.edit-issue {
}
}
+.issue-closed-by-widget {
+ padding: 16px 0;
+ margin: 0px;
+}
+
.issue-form .select2-container {
width: 250px !important;
}
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index 97485c101fb..cc8321d97ad 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -14,6 +14,9 @@ class Projects::IssuesController < Projects::ApplicationController
# Allow issues bulk update
before_action :authorize_admin_issues!, only: [:bulk_update]
+ # Cross-reference merge requests
+ before_action :closed_by_merge_requests, only: [:show]
+
respond_to :html
def index
@@ -112,6 +115,10 @@ class Projects::IssuesController < Projects::ApplicationController
render nothing: true
end
+ def closed_by_merge_requests
+ @closed_by_merge_requests ||= @issue.closed_by_merge_requests(current_user)
+ end
+
protected
def issue
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 98df6984bf7..0d9c5461959 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -259,7 +259,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController
@commits = @merge_request.commits
@merge_request_diff = @merge_request.merge_request_diff
-
+
if @merge_request.locked_long_ago?
@merge_request.unlock_mr
@merge_request.close
diff --git a/app/helpers/issues_helper.rb b/app/helpers/issues_helper.rb
index 6ddb37cd0dc..fda18e7b316 100644
--- a/app/helpers/issues_helper.rb
+++ b/app/helpers/issues_helper.rb
@@ -83,6 +83,10 @@ module IssuesHelper
end
end
+ def merge_requests_sentence(merge_requests)
+ merge_requests.map(&:to_reference).to_sentence(last_word_connector: ', or ')
+ end
+
# Required for Gitlab::Markdown::IssueReferenceFilter
module_function :url_for_issue
end
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 1d85a353a6b..5e964f04ef5 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -85,6 +85,10 @@ module Issuable
assignee_id_changed?
end
+ def open?
+ opened? || reopened?
+ end
+
#
# Votes
#
diff --git a/app/models/issue.rb b/app/models/issue.rb
index fc7e9abe29e..72183108033 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -95,4 +95,14 @@ class Issue < ActiveRecord::Base
def source_project
project
end
+
+ # From all notes on this issue, we'll select the system notes about linked
+ # merge requests. Of those, the MRs closing `self` are returned.
+ def closed_by_merge_requests(current_user = nil)
+ return [] unless open?
+
+ notes.system.flat_map do |note|
+ note.all_references(current_user).merge_requests
+ end.uniq.select { |mr| mr.open? && mr.closes_issue?(self) }
+ end
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index c83b15c7d39..0042b95c4f1 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -222,10 +222,6 @@ class MergeRequest < ActiveRecord::Base
self.target_project.events.where(target_id: self.id, target_type: "MergeRequest", action: Event::CLOSED).last
end
- def open?
- opened? || reopened?
- end
-
def work_in_progress?
!!(title =~ /\A\[?WIP\]?:? /i)
end
@@ -294,6 +290,10 @@ class MergeRequest < ActiveRecord::Base
target_project
end
+ def closes_issue?(issue)
+ closes_issues.include?(issue)
+ end
+
# Return the set of issues that will be closed if this merge request is accepted.
def closes_issues(current_user = self.author)
if target_branch == project.default_branch
diff --git a/app/views/projects/issues/_closed_by_box.html.haml b/app/views/projects/issues/_closed_by_box.html.haml
new file mode 100644
index 00000000000..aef352029d0
--- /dev/null
+++ b/app/views/projects/issues/_closed_by_box.html.haml
@@ -0,0 +1,3 @@
+.issue-closed-by-widget
+ = icon('check')
+ This issue will be closed automatically when merge request #{gfm(merge_requests_sentence(@closed_by_merge_requests.sort))} is accepted.
diff --git a/app/views/projects/issues/show.html.haml b/app/views/projects/issues/show.html.haml
index 5cb814c9ea8..f01bf2505da 100644
--- a/app/views/projects/issues/show.html.haml
+++ b/app/views/projects/issues/show.html.haml
@@ -46,6 +46,7 @@
= markdown(@issue.description)
%textarea.hidden.js-task-list-field
= @issue.description
-
+ - if @closed_by_merge_requests.present?
+ = render 'projects/issues/closed_by_box'
.issue-discussion
= render 'projects/issues/discussion'
diff --git a/spec/helpers/issues_helper_spec.rb b/spec/helpers/issues_helper_spec.rb
index c08ddb4cae1..78a6b631eb2 100644
--- a/spec/helpers/issues_helper_spec.rb
+++ b/spec/helpers/issues_helper_spec.rb
@@ -117,4 +117,14 @@ describe IssuesHelper do
end
end
+ describe "#merge_requests_sentence" do
+ subject { merge_requests_sentence(merge_requests)}
+ let(:merge_requests) do
+ [ build(:merge_request, iid: 1), build(:merge_request, iid: 2),
+ build(:merge_request, iid: 3)]
+ end
+
+ it { is_expected.to eq("!1, !2, or !3") }
+ end
+
end
diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb
index 8f706f8934b..0f13c4410cd 100644
--- a/spec/models/concerns/issuable_spec.rb
+++ b/spec/models/concerns/issuable_spec.rb
@@ -68,7 +68,6 @@ describe Issue, "Issuable" do
end
end
-
describe "#to_hook_data" do
let(:hook_data) { issue.to_hook_data(user) }
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 623332cd2f9..c9aa1b063c6 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -68,6 +68,43 @@ describe Issue do
end
end
+ describe '#closed_by_merge_requests' do
+ let(:project) { create(:project) }
+ let(:issue) { create(:issue, project: project, state: "opened")}
+ let(:closed_issue) { build(:issue, project: project, state: "closed")}
+
+ let(:mr) do
+ opts = {
+ title: 'Awesome merge_request',
+ description: "Fixes #{issue.to_reference}",
+ source_branch: 'feature',
+ target_branch: 'master'
+ }
+ MergeRequests::CreateService.new(project, project.owner, opts).execute
+ end
+
+ let(:closed_mr) do
+ opts = {
+ title: 'Awesome merge_request 2',
+ description: "Fixes #{issue.to_reference}",
+ source_branch: 'feature',
+ target_branch: 'master',
+ state: 'closed'
+ }
+ MergeRequests::CreateService.new(project, project.owner, opts).execute
+ end
+
+ it 'returns the merge request to close this issue' do
+ allow(mr).to receive(:closes_issue?).with(issue).and_return(true)
+
+ expect(issue.closed_by_merge_requests).to eq([mr])
+ end
+
+ it "returns an empty array when the current issue is closed already" do
+ expect(closed_issue.closed_by_merge_requests).to eq([])
+ end
+ end
+
it_behaves_like 'an editable mentionable' do
subject { create(:issue) }