Use the IssuableBaseService lifecycle hooks to cache MergeRequestsClosingIssues

- Instead of overriding `create` and `update` in `MergeRequests::BaseService`
- Get all merge request service specs passing

app/models/issue.rb

@@ -23,7 +23,7 @@ class Issue < ActiveRecord::Base
 
   has_many :events, as: :target, dependent: :destroy
 
-  has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues'
+  has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all
 
   validates :project, presence: true
 

app/models/merge_request.rb

@@ -16,7 +16,7 @@ class MergeRequest < ActiveRecord::Base
 
   has_many :events, as: :target, dependent: :destroy
 
-  has_many :merge_requests_closing_issues, class_name:  'MergeRequestsClosingIssues'
+  has_many :merge_requests_closing_issues, class_name: 'MergeRequestsClosingIssues', dependent: :delete_all
 
   serialize :merge_params, Hash
 

app/services/issuable_base_service.rb

@@ -157,6 +157,10 @@ class IssuableBaseService < BaseService
     # To be overridden by subclasses
   end
 
+  def after_update(issuable)
+    # To be overridden by subclasses
+  end
+
   def update_issuable(issuable, attributes)
     issuable.with_transaction_returning_status do
       issuable.update(attributes.merge(updated_by: current_user))
@@ -182,6 +186,7 @@ class IssuableBaseService < BaseService
       end
 
       handle_changes(issuable, old_labels: old_labels, old_mentioned_users: old_mentioned_users)
+      after_update(issuable)
       issuable.create_new_cross_references!(current_user)
       execute_hooks(issuable, 'update')
     end

app/services/merge_requests/base_service.rb

@@ -35,18 +35,6 @@ module MergeRequests
       end
     end
 
-    def create(merge_request)
-      merge_request = super(merge_request)
-      merge_request.cache_merge_request_closes_issues!(current_user)
-      merge_request
-    end
-
-    def update(merge_request)
-      merge_request = super(merge_request)
-      merge_request.cache_merge_request_closes_issues!(current_user)
-      merge_request
-    end
-
     private
 
     def filter_params

app/services/merge_requests/create_service.rb

@@ -20,6 +20,7 @@ module MergeRequests
       event_service.open_mr(issuable, current_user)
       notification_service.new_merge_request(issuable, current_user)
       todo_service.new_merge_request(issuable, current_user)
+      issuable.cache_merge_request_closes_issues!(current_user)
     end
   end
 end

app/services/merge_requests/update_service.rb

@@ -77,5 +77,9 @@ module MergeRequests
     def close_service
       MergeRequests::CloseService
     end
+
+    def after_update(issuable)
+      issuable.cache_merge_request_closes_issues!(current_user)
+    end
   end
 end

spec/services/merge_requests/create_service_spec.rb

@@ -108,7 +108,7 @@ describe MergeRequests::CreateService, services: true do
         allow(service).to receive(:execute_hooks)
         merge_request = service.execute
 
-        expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue])
+        expect(merge_request.reload.closes_issues(user)).to match_array([first_issue, second_issue])
       end
     end
   end

spec/services/merge_requests/refresh_service_spec.rb

@@ -200,7 +200,7 @@ describe MergeRequests::RefreshService, services: true do
         allow(refresh_service).to receive(:execute_hooks)
         refresh_service.execute(@oldrev, @newrev, 'refs/heads/feature')
 
-        expect(merge_request.reload.closes_issues).to eq([issue])
+        expect(merge_request.reload.closes_issues(@user)).to eq([issue])
       end
     end
 

spec/services/merge_requests/update_service_spec.rb

@@ -274,7 +274,7 @@ describe MergeRequests::UpdateService, services: true do
         allow(service).to receive(:execute_hooks)
         service.execute(merge_request)
 
-        expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue])
+        expect(merge_request.reload.closes_issues(user)).to match_array([first_issue, second_issue])
       end
 
       it 'removes `MergeRequestsClosingIssues` records when issues are not closed anymore' do
@@ -288,13 +288,13 @@ describe MergeRequests::UpdateService, services: true do
 
         merge_request = MergeRequests::CreateService.new(project, user, opts).execute
 
-        expect(merge_request.reload.closes_issues).to match_array([first_issue, second_issue])
+        expect(merge_request.reload.closes_issues(user)).to match_array([first_issue, second_issue])
 
         service = described_class.new(project, user, description: "not closing any issues")
         allow(service).to receive(:execute_hooks)
         service.execute(merge_request.reload)
 
-        expect(merge_request.reload.closes_issues).to be_empty
+        expect(merge_request.reload.closes_issues(user)).to be_empty
       end
     end
   end