Refactor similar code for Issue and MR update service

Signed-off-by: Dmitriy Zaporozhets <dmitriy.zaporozhets@gmail.com>

app/services/issuable_base_service.rb

@@ -28,6 +28,9 @@ class IssuableBaseService < BaseService
   end
 
   def filter_params(issuable_ability_name = :issue)
+    params[:assignee_id]  = "" if params[:assignee_id] == IssuableFinder::NONE
+    params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE
+
     ability = :"admin_#{issuable_ability_name}"
 
     unless can?(current_user, ability, project)
@@ -36,4 +39,36 @@ class IssuableBaseService < BaseService
       params.delete(:assignee_id)
     end
   end
+
+  def update(issuable)
+    change_state(issuable)
+    filter_params
+    old_labels = issuable.labels.to_a
+
+    if params.present? && issuable.update_attributes(params.merge(updated_by: current_user))
+      issuable.reset_events_cache
+
+      if issuable.labels != old_labels
+        create_labels_note(
+          issuable,
+          issuable.labels - old_labels,
+          old_labels - issuable.labels)
+      end
+
+      handle_changes(issuable)
+      issuable.create_new_cross_references!(current_user)
+      execute_hooks(issuable, 'update')
+    end
+
+    issuable
+  end
+
+  def change_state(issuable)
+    case params.delete(:state_event)
+    when 'reopen'
+      reopen_service.new(project, current_user, {}).execute(issuable)
+    when 'close'
+      close_service.new(project, current_user, {}).execute(issuable)
+    end
+  end
 end

app/services/issues/update_service.rb

@@ -1,33 +1,7 @@
 module Issues
   class UpdateService < Issues::BaseService
     def execute(issue)
-      case params.delete(:state_event)
-      when 'reopen'
-        Issues::ReopenService.new(project, current_user, {}).execute(issue)
-      when 'close'
-        Issues::CloseService.new(project, current_user, {}).execute(issue)
-      end
-
-      params[:assignee_id]  = "" if params[:assignee_id] == IssuableFinder::NONE
-      params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE
-
-      filter_params
-      old_labels = issue.labels.to_a
-
-      if params.present? && issue.update_attributes(params.merge(updated_by: current_user))
-        issue.reset_events_cache
-
-        if issue.labels != old_labels
-          create_labels_note(
-            issue, issue.labels - old_labels, old_labels - issue.labels)
-        end
-
-        handle_changes(issue)
-        issue.create_new_cross_references!(current_user)
-        execute_hooks(issue, 'update')
-      end
-
-      issue
+      update(issue)
     end
 
     def handle_changes(issue)
@@ -44,5 +18,13 @@ module Issues
         create_title_change_note(issue, issue.previous_changes['title'].first)
       end
     end
+
+    def reopen_service
+      Issues::ReopenService
+    end
+
+    def close_service
+      Issues::CloseService
+    end
   end
 end

app/services/merge_requests/update_service.rb

@@ -11,36 +11,7 @@ module MergeRequests
       params.except!(:target_project_id)
       params.except!(:source_branch)
 
-      case params.delete(:state_event)
-      when 'reopen'
-        MergeRequests::ReopenService.new(project, current_user, {}).execute(merge_request)
-      when 'close'
-        MergeRequests::CloseService.new(project, current_user, {}).execute(merge_request)
-      end
-
-      params[:assignee_id]  = "" if params[:assignee_id] == IssuableFinder::NONE
-      params[:milestone_id] = "" if params[:milestone_id] == IssuableFinder::NONE
-
-      filter_params
-      old_labels = merge_request.labels.to_a
-
-      if params.present? && merge_request.update_attributes(params.merge(updated_by: current_user))
-        merge_request.reset_events_cache
-
-        if merge_request.labels != old_labels
-          create_labels_note(
-            merge_request,
-            merge_request.labels - old_labels,
-            old_labels - merge_request.labels
-          )
-        end
-
-        handle_changes(merge_request)
-        merge_request.create_new_cross_references!(current_user)
-        execute_hooks(merge_request, 'update')
-      end
-
-      merge_request
+      update(merge_request)
     end
 
     def handle_changes(merge_request)
@@ -68,5 +39,13 @@ module MergeRequests
         merge_request.mark_as_unchecked
       end
     end
+
+    def reopen_service
+      MergeRequests::ReopenService
+    end
+
+    def close_service
+      MergeRequests::CloseService
+    end
   end
 end