diff options
| -rw-r--r-- | app/controllers/projects/issues_controller.rb | 36 | ||||
| -rw-r--r-- | app/finders/notes_finder.rb | 4 | ||||
| -rw-r--r-- | app/services/issues/base_service.rb | 36 | ||||
| -rw-r--r-- | app/services/issues/create_service.rb | 4 | ||||
| -rw-r--r-- | app/views/discussions/_new_issue_for_discussion.html.haml | 2 | ||||
| -rw-r--r-- | app/views/discussions/_notes.html.haml | 2 | ||||
| -rw-r--r-- | app/views/shared/issuable/_form.html.haml | 26 | ||||
| -rw-r--r-- | doc/api/issues.md | 6 | ||||
| -rw-r--r-- | lib/api/issues.rb | 13 | ||||
| -rw-r--r-- | spec/controllers/projects/issues_controller_spec.rb | 13 | ||||
| -rw-r--r-- | spec/features/issues/create_issue_for_single_discussion_in_merge_request.rb | 4 | ||||
| -rw-r--r-- | spec/finders/notes_finder_spec.rb | 22 | ||||
| -rw-r--r-- | spec/requests/api/issues_spec.rb | 1 | ||||
| -rw-r--r-- | spec/services/issues/base_service_spec.rb | 90 | ||||
| -rw-r--r-- | spec/services/issues/build_service_spec.rb | 19 | ||||
| -rw-r--r-- | spec/services/issues/create_service_spec.rb | 4 | ||||
| -rw-r--r-- | spec/support/features/resolving_discussions_in_issues_shared_examples.rb | 6 |
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 |