summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorBob Van Landuyt <bob@vanlanduyt.co>2017-02-27 10:23:36 +0100
committerBob Van Landuyt <bob@gitlab.com>2017-03-13 08:27:51 +0100
commitf86928953d2d79f40f10813a6e244c1da0779d16 (patch)
tree7396889bce42589175ec3c2fa314fdf621ef73b1
parent51253b2dd0bb073d271ddcbd172f7c204d4639db (diff)
downloadgitlab-ce-f86928953d2d79f40f10813a6e244c1da0779d16.tar.gz
Always require MR-iid for resolving discussions
And deduplicate the finding of MR's & discussions. Now the searching is done in the service, istead of the controller & the API.
-rw-r--r--app/controllers/projects/issues_controller.rb36
-rw-r--r--app/finders/notes_finder.rb4
-rw-r--r--app/services/issues/base_service.rb36
-rw-r--r--app/services/issues/create_service.rb4
-rw-r--r--app/views/discussions/_new_issue_for_discussion.html.haml2
-rw-r--r--app/views/discussions/_notes.html.haml2
-rw-r--r--app/views/shared/issuable/_form.html.haml26
-rw-r--r--doc/api/issues.md6
-rw-r--r--lib/api/issues.rb13
-rw-r--r--spec/controllers/projects/issues_controller_spec.rb13
-rw-r--r--spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb4
-rw-r--r--spec/finders/notes_finder_spec.rb22
-rw-r--r--spec/requests/api/issues_spec.rb1
-rw-r--r--spec/services/issues/base_service_spec.rb90
-rw-r--r--spec/services/issues/build_service_spec.rb19
-rw-r--r--spec/services/issues/create_service_spec.rb4
-rw-r--r--spec/support/features/resolving_discussions_in_issues_shared_examples.rb6
17 files changed, 101 insertions, 187 deletions
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb
index d7eb73d7ff5..d76643ff875 100644
--- a/app/controllers/projects/issues_controller.rb
+++ b/app/controllers/projects/issues_controller.rb
@@ -66,11 +66,14 @@ class Projects::IssuesController < Projects::ApplicationController
)
build_params = issue_params.merge(
- merge_request_for_resolving_discussions: merge_request_for_resolving_discussions,
- discussion_to_resolve: discussion_to_resolve
+ merge_request_for_resolving_discussions: params[:merge_request_for_resolving_discussions],
+ discussion_to_resolve: params[:discussion_to_resolve]
)
+ service = Issues::BuildService.new(project, current_user, build_params)
+ @merge_request_for_resolving_discussions = service.merge_request_for_resolving_discussions
+ @discussion_to_resolve = service.discussions_to_resolve.first if params[:discussion_to_resolve]
- @issue = @noteable = Issues::BuildService.new(project, current_user, build_params).execute
+ @issue = @noteable = service.execute
respond_with(@issue)
end
@@ -99,12 +102,11 @@ class Projects::IssuesController < Projects::ApplicationController
end
def create
- create_params = issue_params.
- merge(
- merge_request_for_resolving_discussions: merge_request_for_resolving_discussions,
- discussion_to_resolve: discussion_to_resolve
- ).
- merge(spammable_params)
+ create_params = issue_params.merge(spammable_params).merge(
+ merge_request_for_resolving_discussions: params[:merge_request_for_resolving_discussions],
+ discussion_to_resolve: params[:discussion_to_resolve]
+ )
+
@issue = Issues::CreateService.new(project, current_user, create_params).execute
respond_to do |format|
@@ -192,21 +194,6 @@ class Projects::IssuesController < Projects::ApplicationController
alias_method :awardable, :issue
alias_method :spammable, :issue
- def merge_request_for_resolving_discussions
- return unless merge_request_iid = params[:merge_request_for_resolving_discussions]
-
- @merge_request_for_resolving_discussions ||= MergeRequestsFinder.new(current_user, project_id: project.id).
- execute.
- find_by(iid: merge_request_iid)
- end
-
- def discussion_to_resolve
- return unless discussion_id = params[:discussion_to_resolve]
-
- @discussion_to_resolve ||= NotesFinder.new(project, current_user, discussion_id: discussion_id).
- first_discussion
- end
-
def authorize_read_issue!
return render_404 unless can?(current_user, :read_issue, @issue)
end
@@ -253,6 +240,7 @@ class Projects::IssuesController < Projects::ApplicationController
def issue_params
params.require(:issue).permit(
:title, :assignee_id, :position, :description, :confidential,
+ :discussion_to_resolve, :merge_request_for_resolving_discussions,
:milestone_id, :due_date, :state_event, :task_num, :lock_version, label_ids: []
)
end
diff --git a/app/finders/notes_finder.rb b/app/finders/notes_finder.rb
index 6d0ff1798ce..98c66dfa021 100644
--- a/app/finders/notes_finder.rb
+++ b/app/finders/notes_finder.rb
@@ -27,10 +27,6 @@ class NotesFinder
@notes
end
- def first_discussion
- execute.discussions.first
- end
-
private
def init_collection
diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb
index 3aa842eb9a8..058c398df02 100644
--- a/app/services/issues/base_service.rb
+++ b/app/services/issues/base_service.rb
@@ -1,12 +1,11 @@
module Issues
class BaseService < ::IssuableBaseService
- attr_reader :merge_request_for_resolving_discussions, :discussion_to_resolve
-
+ attr_reader :merge_request_for_resolving_discussions_iid, :discussion_to_resolve_id
def initialize(*args)
super
- @merge_request_for_resolving_discussions ||= params.delete(:merge_request_for_resolving_discussions)
- @discussion_to_resolve ||= params.delete(:discussion_to_resolve)
+ @merge_request_for_resolving_discussions_iid ||= params.delete(:merge_request_for_resolving_discussions)
+ @discussion_to_resolve_id ||= params.delete(:discussion_to_resolve)
end
def hook_data(issue, action)
@@ -17,25 +16,22 @@ module Issues
end
def merge_request_for_resolving_discussions
- @merge_request_for_resolving_discussions ||= discussion_to_resolve.try(:noteable)
- end
-
- def for_all_discussions_in_a_merge_request?
- discussion_to_resolve.nil? && merge_request_for_resolving_discussions
- end
-
- def for_single_discussion?
- discussion_to_resolve && discussion_to_resolve.noteable == merge_request_for_resolving_discussions
+ @merge_request_for_resolving_discussions ||= MergeRequestsFinder.new(current_user, project_id: project.id).
+ execute.
+ find_by(iid: merge_request_for_resolving_discussions_iid)
end
def discussions_to_resolve
- @discussions_to_resolve ||= if for_all_discussions_in_a_merge_request?
- merge_request_for_resolving_discussions.resolvable_discussions
- elsif for_single_discussion?
- Array(discussion_to_resolve)
- else
- []
- end
+ return [] unless merge_request_for_resolving_discussions
+
+ @discussions_to_resolve ||= NotesFinder.new(project, current_user, {
+ discussion_id: discussion_to_resolve_id,
+ target_type: MergeRequest.name.underscore,
+ target_id: merge_request_for_resolving_discussions.id
+ }).
+ execute.
+ discussions.
+ select(&:to_be_resolved?)
end
private
diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb
index d2cc70ed0dc..007c8354c8c 100644
--- a/app/services/issues/create_service.rb
+++ b/app/services/issues/create_service.rb
@@ -6,8 +6,8 @@ module Issues
filter_spam_check_params
issue_attributes = params.merge(
- merge_request_for_resolving_discussions: merge_request_for_resolving_discussions,
- discussion_to_resolve: discussion_to_resolve
+ merge_request_for_resolving_discussions: merge_request_for_resolving_discussions_iid,
+ discussion_to_resolve: discussion_to_resolve_id
)
@issue = BuildService.new(project, current_user, issue_attributes).execute
diff --git a/app/views/discussions/_new_issue_for_discussion.html.haml b/app/views/discussions/_new_issue_for_discussion.html.haml
index de28ded9820..c21ec0ecc3d 100644
--- a/app/views/discussions/_new_issue_for_discussion.html.haml
+++ b/app/views/discussions/_new_issue_for_discussion.html.haml
@@ -5,4 +5,4 @@
.btn.btn-default.discussion-create-issue-btn.has-tooltip{ title: "Resolve this discussion in a new issue",
"aria-label" => "Resolve this discussion in a new issue",
"data-container" => "body" }
- = link_to custom_icon('icon_mr_issue'), new_namespace_project_issue_path(@project.namespace, @project, discussion_to_resolve: discussion.id), title: "Resolve this discussion in a new issue", class: 'new-issue-for-discussion'
+ = link_to custom_icon('icon_mr_issue'), new_namespace_project_issue_path(@project.namespace, @project, merge_request_for_resolving_discussions: merge_request.iid, discussion_to_resolve: discussion.id), title: "Resolve this discussion in a new issue", class: 'new-issue-for-discussion'
diff --git a/app/views/discussions/_notes.html.haml b/app/views/discussions/_notes.html.haml
index 4a1e93cec3a..2789391819c 100644
--- a/app/views/discussions/_notes.html.haml
+++ b/app/views/discussions/_notes.html.haml
@@ -12,7 +12,7 @@
= render "discussions/resolve_all", discussion: discussion
- if discussion.for_merge_request?
.btn-group.discussion-actions
- = render "discussions/new_issue_for_discussion", discussion: discussion
+ = render "discussions/new_issue_for_discussion", discussion: discussion, merge_request: discussion.noteable
= render "discussions/jump_to_next", discussion: discussion
- else
= link_to_reply_discussion(discussion)
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index e93fe0f6cb5..a10f9caf3c1 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -50,30 +50,20 @@
.col-sm-10.col-sm-offset-2
= icon('exclamation-triangle')
- if @merge_request_for_resolving_discussions.discussions_can_be_resolved_by?(current_user)
- Creating this issue will mark all discussions in
- = link_to @merge_request_for_resolving_discussions.to_reference, merge_request_path(@merge_request_for_resolving_discussions)
- as resolved.
= hidden_field_tag 'merge_request_for_resolving_discussions', @merge_request_for_resolving_discussions.iid
+ - if @discussion_to_resolve
+ = hidden_field_tag 'discussion_to_resolve', @discussion_to_resolve.id
+ Creating this issue will mark the discussion at
+ = link_to @discussion_to_resolve.noteable.to_reference, Gitlab::UrlBuilder.build(@discussion_to_resolve.first_note)
+ - else
+ Creating this issue will mark all discussions in
+ = link_to @merge_request_for_resolving_discussions.to_reference, merge_request_path(@merge_request_for_resolving_discussions)
+ as resolved.
- else
You can't automatically mark all discussions in
= link_to @merge_request_for_resolving_discussions.to_reference, merge_request_path(@merge_request_for_resolving_discussions)
as resolved. Ask someone with sufficient rights to resolve the them.
-- if @discussion_to_resolve
- .form-group
- .col-sm-10.col-sm-offset-2
- = icon('exclamation-triangle')
- - if @discussion_to_resolve.can_resolve?(current_user)
- Creating this issue will mark the discussion at
- = link_to @discussion_to_resolve.noteable.to_reference, Gitlab::UrlBuilder.build(@discussion_to_resolve.first_note)
- as resolved.
- = hidden_field_tag 'discussion_to_resolve', @discussion_to_resolve.id
- - else
- You can't automatically mark the discussion at
- = link_to @discussion_to_resolve.noteable.to_reference, Gitlab::UrlBuilder.build(@discussion_to_resolve.first_note)
- as resolved. Ask someone with sufficient rights to resolve it.
-
-
- is_footer = !(issuable.is_a?(MergeRequest) && issuable.new_record?)
.row-content-block{ class: (is_footer ? "footer-block" : "middle-block") }
.pull-right
diff --git a/doc/api/issues.md b/doc/api/issues.md
index ddc304ea128..05fb9739ed1 100644
--- a/doc/api/issues.md
+++ b/doc/api/issues.md
@@ -329,8 +329,8 @@ Creates a new project issue.
POST /projects/:id/issues
```
-| Attribute | Type | Required | Description |
-| --------- | ---- | -------- | ----------- |
+| Attribute | Type | Required | Description |
+| --------- | ---- | -------- | ----------- |
| `id` | integer | yes | The ID of a project |
| `title` | string | yes | The title of an issue |
| `description` | string | no | The description of an issue |
@@ -341,7 +341,7 @@ POST /projects/:id/issues
| `created_at` | string | no | Date time string, ISO 8601 formatted, e.g. `2016-03-11T03:45:40Z` (requires admin or project owner rights) |
| `due_date` | string | no | Date time string in the format YEAR-MONTH-DAY, e.g. `2016-03-11` |
| `merge_request_for_resolving_discussions` | integer | no | The IID of a merge request in which to resolve all issues. This will fill the issue with a default description and mark all discussions as resolved. When passing a description or title, these values will take precedence over the default values. |
-| `discussion_to_resolve` | string | no | The ID of a discussion to resolve. This will fill in the issue with a default description and mark the discussion as resolved. |
+| `discussion_to_resolve` | string | no | The ID of a discussion to resolve. This will fill in the issue with a default description and mark the discussion as resolved. Use in combination with `merge_request_for_resolving_discussions`. |
```bash
curl --request POST --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/4/issues?title=Issues%20with%20auth&labels=bug
diff --git a/lib/api/issues.rb b/lib/api/issues.rb
index 9aac3c8fe1a..162e13a313d 100644
--- a/lib/api/issues.rb
+++ b/lib/api/issues.rb
@@ -119,7 +119,7 @@ module API
optional :merge_request_for_resolving_discussions, type: Integer,
desc: 'The IID of a merge request for which to resolve discussions'
optional :discussion_to_resolve, type: String,
- desc: 'The ID of a discussion to resolve'
+ desc: 'The ID of a discussion to resolve, also pass `merge_request_for_resolving_discussions`'
use :issue_params
end
post ':id/issues' do
@@ -130,17 +130,6 @@ module API
issue_params = declared_params(include_missing: false)
- if merge_request_iid = params[:merge_request_for_resolving_discussions]
- issue_params[:merge_request_for_resolving_discussions] = MergeRequestsFinder.new(current_user, project_id: user_project.id).
- execute.
- find_by(iid: merge_request_iid)
- end
-
- if discussion_id = params[:discussion_to_resolve]
- issue_params[:discussion_to_resolve] = NotesFinder.new(user_project, current_user, discussion_id: discussion_id).
- first_discussion
- end
-
issue = ::Issues::CreateService.new(user_project,
current_user,
issue_params.merge(request: request, api: true)).execute
diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb
index 336e3b50a72..46e2f4cef29 100644
--- a/spec/controllers/projects/issues_controller_spec.rb
+++ b/spec/controllers/projects/issues_controller_spec.rb
@@ -113,7 +113,7 @@ describe Projects::IssuesController do
it 'fills in an issue for a discussion' do
note = create(:note_on_merge_request, project: project)
- get :new, namespace_id: project.namespace.path, project_id: project, discussion_to_resolve: note.discussion_id
+ get :new, namespace_id: project.namespace.path, project_id: project, merge_request_for_resolving_discussions: note.noteable.iid, discussion_to_resolve: note.discussion_id
expect(assigns(:issue).title).not_to be_empty
expect(assigns(:issue).description).not_to be_empty
@@ -474,8 +474,8 @@ describe Projects::IssuesController do
{ merge_request_for_resolving_discussions: merge_request.iid }
end
- def post_issue(issue_params)
- post :create, namespace_id: project.namespace.to_param, project_id: project, issue: issue_params, merge_request_for_resolving_discussions: merge_request.iid
+ def post_issue(issue_params, other_params: {})
+ post :create, { namespace_id: project.namespace.to_param, project_id: project, issue: issue_params, merge_request_for_resolving_discussions: merge_request.iid }.merge(other_params)
end
it 'creates an issue for the project' do
@@ -494,6 +494,13 @@ describe Projects::IssuesController do
expect(discussion.resolved?).to eq(true)
end
+
+ it "resolves a single discussion" do
+ post_issue(other_params: { discussion_to_resolve: discussion.id })
+ discussion.first_note.reload
+
+ expect(discussion.resolved?).to eq(true)
+ end
end
context 'Akismet is enabled' do
diff --git a/spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb b/spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb
index 6cf67639b7e..61b5b226c03 100644
--- a/spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb
+++ b/spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb
@@ -42,14 +42,14 @@ feature 'Resolve an open discussion in a merge request by creating an issue', fe
end
it 'has a link to create a new issue for a discussion' do
- new_issue_link = new_namespace_project_issue_path(project.namespace, project, discussion_to_resolve: discussion.id)
+ new_issue_link = new_namespace_project_issue_path(project.namespace, project, discussion_to_resolve: discussion.id, merge_request_for_resolving_discussions: merge_request.iid)
expect(page).to have_link 'Resolve this discussion in a new issue', href: new_issue_link
end
context 'creating the issue' do
before do
- click_link 'Resolve this discussion in a new issue', href: new_namespace_project_issue_path(project.namespace, project, discussion_to_resolve: discussion.id)
+ click_link 'Resolve this discussion in a new issue', href: new_namespace_project_issue_path(project.namespace, project, discussion_to_resolve: discussion.id, merge_request_for_resolving_discussions: merge_request.iid)
end
it 'has a hidden field for the discussion' do
diff --git a/spec/finders/notes_finder_spec.rb b/spec/finders/notes_finder_spec.rb
index 5fc2e3dd527..2c72f7839a6 100644
--- a/spec/finders/notes_finder_spec.rb
+++ b/spec/finders/notes_finder_spec.rb
@@ -157,28 +157,6 @@ describe NotesFinder do
end
end
- describe "#first_discussion" do
- let!(:discussion_note) { create(:note_on_merge_request, project: project) }
-
- it 'returns a discussion' do
- discussion = described_class.new(project, user, discussion_id: discussion_note.discussion_id).first_discussion
-
- expect(discussion).is_a? Discussion
- end
-
- it 'includes the notes in the discussion' do
- discussion = described_class.new(project, user, discussion_id: discussion_note.discussion_id).first_discussion
-
- expect(discussion.notes).to include(discussion_note)
- end
-
- it 'returns nil when no notes are found' do
- discussion = described_class.new(project, user, discussion_id: 'non-existant').first_discussion
-
- expect(discussion).to be(nil)
- end
- end
-
describe '.search' do
let(:project) { create(:empty_project, :public) }
let(:note) { create(:note_on_issue, note: 'WoW', project: project) }
diff --git a/spec/requests/api/issues_spec.rb b/spec/requests/api/issues_spec.rb
index 4d52ab2aaa2..26997ac0107 100644
--- a/spec/requests/api/issues_spec.rb
+++ b/spec/requests/api/issues_spec.rb
@@ -951,6 +951,7 @@ describe API::Issues, api: true do
before do
post api("/projects/#{project.id}/issues", user),
title: 'New Issue',
+ merge_request_for_resolving_discussions: merge_request.iid,
discussion_to_resolve: discussion.id
end
diff --git a/spec/services/issues/base_service_spec.rb b/spec/services/issues/base_service_spec.rb
index bfeefe3bcc5..0cef09917ee 100644
--- a/spec/services/issues/base_service_spec.rb
+++ b/spec/services/issues/base_service_spec.rb
@@ -13,71 +13,19 @@ describe Issues::BaseService, services: true do
let(:merge_request) { discussion.noteable }
let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "other") }
- describe "#for_single_discussion" do
- it "is true when only a discussion is passed" do
- service = described_class.new(project, user, discussion_to_resolve: discussion)
-
- expect(service.for_single_discussion?).to be_truthy
- end
-
- it "is true when matching merge request and discussion are passed" do
- service = described_class.new(
- project,
- user,
- discussion_to_resolve: discussion,
- merge_request_for_resolving_discussions: merge_request
- )
-
- expect(service.for_single_discussion?).to be_truthy
- end
-
- it "is false when a discussion and another merge request are passed" do
- service = described_class.new(
- project,
- user,
- discussion_to_resolve: discussion,
- merge_request_for_resolving_discussions: other_merge_request
- )
-
- expect(service.for_single_discussion?).to be_falsy
- end
- end
-
- describe "#for_all_discussions_in_a_merge_request" do
- it "is true when only a merge request is passed" do
- service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request)
-
- expect(service.for_all_discussions_in_a_merge_request?).to be_truthy
- end
-
- it "is false when matching merge request and discussion are passed" do
- service = described_class.new(
- project,
- user,
- discussion_to_resolve: discussion,
- merge_request_for_resolving_discussions: merge_request
- )
-
- expect(service.for_all_discussions_in_a_merge_request?).to be_falsy
- end
- end
-
describe "#discussions_to_resolve" do
- it "only contains a single discussion when only a discussion is passed" do
- service = described_class.new(project, user, discussion_to_resolve: discussion)
-
- expect(service.discussions_to_resolve).to contain_exactly(discussion)
- end
-
- it "is contains a single discussion when matching merge request and discussion are passed" do
+ it "contains a single discussion when matching merge request and discussion are passed" do
service = described_class.new(
project,
user,
- discussion_to_resolve: discussion,
- merge_request_for_resolving_discussions: merge_request
+ discussion_to_resolve: discussion.id,
+ merge_request_for_resolving_discussions: merge_request.iid
)
+ # We need to compare discussion id's because the Discussion-objects are rebuilt
+ # which causes the object-id's not to be different.
+ discussion_ids = service.discussions_to_resolve.map(&:id)
- expect(service.discussions_to_resolve).to contain_exactly(discussion)
+ expect(discussion_ids).to contain_exactly(discussion.id)
end
it "contains all discussions when only a merge request is passed" do
@@ -88,7 +36,7 @@ describe Issues::BaseService, services: true do
service = described_class.new(
project,
user,
- merge_request_for_resolving_discussions: merge_request
+ merge_request_for_resolving_discussions: merge_request.iid
)
# We need to compare discussion id's because the Discussion-objects are rebuilt
# which causes the object-id's not to be different.
@@ -97,12 +45,30 @@ describe Issues::BaseService, services: true do
expect(discussion_ids).to contain_exactly(discussion.id, second_discussion.id)
end
+ it "contains only unresolved discussions" do
+ second_discussion = Discussion.new([create(:diff_note_on_merge_request, :resolved,
+ noteable: merge_request,
+ project: merge_request.target_project,
+ line_number: 15,
+ )])
+ service = described_class.new(
+ project,
+ user,
+ merge_request_for_resolving_discussions: merge_request.iid
+ )
+ # We need to compare discussion id's because the Discussion-objects are rebuilt
+ # which causes the object-id's not to be different.
+ discussion_ids = service.discussions_to_resolve.map(&:id)
+
+ expect(discussion_ids).to contain_exactly(discussion.id)
+ end
+
it "is empty when a discussion and another merge request are passed" do
service = described_class.new(
project,
user,
- discussion_to_resolve: discussion,
- merge_request_for_resolving_discussions: other_merge_request
+ discussion_to_resolve: discussion.id,
+ merge_request_for_resolving_discussions: other_merge_request.iid
)
expect(service.discussions_to_resolve).to be_empty
diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb
index cca7cdbdd2d..76417279923 100644
--- a/spec/services/issues/build_service_spec.rb
+++ b/spec/services/issues/build_service_spec.rb
@@ -10,20 +10,17 @@ describe Issues::BuildService, services: true do
context 'for a single discussion' do
describe '#execute' do
- it 'references the noteable title in the issue title' do
- merge_request = create(:merge_request, title: "Hello world", source_project: project)
- discussion = Discussion.new([create(:note_on_merge_request, project: project, noteable: merge_request)])
- service = described_class.new(project, user, discussion_to_resolve: discussion)
+ let(:merge_request) { create(:merge_request, title: "Hello world", source_project: project) }
+ let(:discussion) { Discussion.new([create(:diff_note_on_merge_request, project: project, noteable: merge_request, note: "Almost done")]) }
+ let(:service) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request.iid, discussion_to_resolve: discussion.id) }
+ it 'references the noteable title in the issue title' do
issue = service.execute
expect(issue.title).to include('Hello world')
end
it 'adds the note content to the description' do
- discussion = Discussion.new([create(:note_on_merge_request, project: project, note: "Almost done")])
- service = described_class.new(project, user, discussion_to_resolve: discussion)
-
issue = service.execute
expect(issue.description).to include('Almost done')
@@ -33,12 +30,12 @@ describe Issues::BuildService, services: true do
context 'for discussions in a merge request' do
let(:merge_request) { create(:merge_request_with_diff_notes, source_project: project) }
- let(:issue) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute }
+ let(:issue) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request.iid).execute }
describe '#items_for_discussions' do
it 'has an item for each discussion' do
create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, line_number: 13)
- service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request)
+ service = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request.iid)
service.execute
@@ -47,7 +44,7 @@ describe Issues::BuildService, services: true do
end
describe '#item_for_discussion' do
- let(:service) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request) }
+ let(:service) { described_class.new(project, user, merge_request_for_resolving_discussions: merge_request.iid) }
it 'mentions the author of the note' do
discussion = Discussion.new([create(:diff_note_on_merge_request, author: create(:user, username: 'author'))])
@@ -125,7 +122,7 @@ describe Issues::BuildService, services: true do
describe '#execute' do
it 'mentions the merge request in the description' do
- issue = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request).execute
+ issue = described_class.new(project, user, merge_request_for_resolving_discussions: merge_request.iid).execute
expect(issue.description).to include("Review the conversation in #{merge_request.to_reference}")
end
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 4a5780f257a..77a984df2d5 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -150,7 +150,7 @@ describe Issues::CreateService, services: true do
end
describe 'for a single discussion' do
- let(:opts) { { discussion_to_resolve: discussion } }
+ let(:opts) { { discussion_to_resolve: discussion.id, merge_request_for_resolving_discussions: merge_request.iid } }
it 'resolves the discussion' do
described_class.new(project, user, opts).execute
@@ -186,7 +186,7 @@ describe Issues::CreateService, services: true do
end
describe 'for a merge request' do
- let(:opts) { { merge_request_for_resolving_discussions: merge_request } }
+ let(:opts) { { merge_request_for_resolving_discussions: merge_request.iid } }
it 'resolves the discussion' do
described_class.new(project, user, opts).execute
diff --git a/spec/support/features/resolving_discussions_in_issues_shared_examples.rb b/spec/support/features/resolving_discussions_in_issues_shared_examples.rb
index 8712e688d33..669b306d94e 100644
--- a/spec/support/features/resolving_discussions_in_issues_shared_examples.rb
+++ b/spec/support/features/resolving_discussions_in_issues_shared_examples.rb
@@ -22,4 +22,10 @@ shared_examples 'creating an issue for a discussion' do
expect(discussion.resolved?).to eq(true)
end
+
+ it 'has a hidden field for the merge request' do
+ merge_request_field = find('#merge_request_for_resolving_discussions', visible: false)
+
+ expect(merge_request_field.value).to eq(merge_request.iid.to_s)
+ end
end