diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index d76643ff875..537506542b3 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -64,16 +64,15 @@ class Projects::IssuesController < Projects::ApplicationController params[:issue] ||= ActionController::Parameters.new( assignee_id: "" ) - build_params = issue_params.merge( 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 = service.execute + @merge_request_for_resolving_discussions = service.merge_request_for_resolving_discussions + @discussion_to_resolve = service.discussions_to_resolve.first if params[:discussion_to_resolve] respond_with(@issue) end diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb index 058c398df02..ee1b40db718 100644 --- a/app/services/issues/base_service.rb +++ b/app/services/issues/base_service.rb @@ -1,13 +1,5 @@ module Issues class BaseService < ::IssuableBaseService - attr_reader :merge_request_for_resolving_discussions_iid, :discussion_to_resolve_id - def initialize(*args) - super - - @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) issue_data = issue.to_hook_data(current_user) issue_url = Gitlab::UrlBuilder.build(issue) @@ -15,25 +7,6 @@ module Issues issue_data end - def 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 - 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 def execute_hooks(issue, action = 'open') diff --git a/app/services/issues/build_service.rb b/app/services/issues/build_service.rb index 1bcc5abd3f2..5f303b73d16 100644 --- a/app/services/issues/build_service.rb +++ b/app/services/issues/build_service.rb @@ -1,6 +1,9 @@ module Issues class BuildService < Issues::BaseService + include ResolveDiscussions + def execute + filter_resolve_discussion_params @issue = project.issues.new(issue_params) end diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 007c8354c8c..f40cb4a8fa0 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -1,9 +1,11 @@ module Issues class CreateService < Issues::BaseService include SpamCheckService + include ResolveDiscussions def execute filter_spam_check_params + filter_resolve_discussion_params issue_attributes = params.merge( merge_request_for_resolving_discussions: merge_request_for_resolving_discussions_iid, @@ -28,15 +30,6 @@ module Issues resolve_discussions_with_issue(issuable) end - def resolve_discussions_with_issue(issue) - return if discussions_to_resolve.empty? - - Discussions::ResolveService.new(project, current_user, - merge_request: merge_request_for_resolving_discussions, - follow_up_issue: issue). - execute(discussions_to_resolve) - end - private def user_agent_detail_service diff --git a/app/services/issues/resolve_discussions.rb b/app/services/issues/resolve_discussions.rb new file mode 100644 index 00000000000..62a89ae4c0b --- /dev/null +++ b/app/services/issues/resolve_discussions.rb @@ -0,0 +1,35 @@ +module Issues + module ResolveDiscussions + attr_reader :merge_request_for_resolving_discussions_iid, :discussion_to_resolve_id + + def filter_resolve_discussion_params + @merge_request_for_resolving_discussions_iid ||= params.delete(:merge_request_for_resolving_discussions) + @discussion_to_resolve_id ||= params.delete(:discussion_to_resolve) + end + + def resolve_discussions_with_issue(issue) + return if discussions_to_resolve.empty? + + Discussions::ResolveService.new(project, current_user, + merge_request: merge_request_for_resolving_discussions, + follow_up_issue: issue). + execute(discussions_to_resolve) + end + + def 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 + 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 + end +end diff --git a/spec/services/issues/base_service_spec.rb b/spec/services/issues/resolve_discussions_spec.rb similarity index 89% rename from spec/services/issues/base_service_spec.rb rename to spec/services/issues/resolve_discussions_spec.rb index 0cef09917ee..332927d10ad 100644 --- a/spec/services/issues/base_service_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -1,6 +1,15 @@ require 'spec_helper.rb' -describe Issues::BaseService, services: true do +class DummyService < Issues::BaseService + include ::Issues::ResolveDiscussions + + def initialize(*args) + super + filter_resolve_discussion_params + end +end + +describe DummyService, services: true do let(:project) { create(:project) } let(:user) { create(:user) } @@ -46,11 +55,11 @@ describe Issues::BaseService, services: true do end it "contains only unresolved discussions" do - second_discussion = Discussion.new([create(:diff_note_on_merge_request, :resolved, + _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,