Move functionality for resolving discussions into a concern

6 files changed, 54 insertions, 42 deletions

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
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,