Merge branch 'mark-pending-todos-as-done-when-approving-mr' into 'master'

Mark pending todos as done when approving a merge request

Closes #392 

See merge request !292

CHANGELOG-EE

@@ -2,6 +2,8 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.7.0 (unreleased)
 
+v 8.6.2 (unreleased)
+  - Mark pending todos as done when approving a merge request
 
 v 8.6.1
   - Only rename the `light_logo` column in the `appearances` table if its not there yet. !290

app/services/merge_requests/approval_service.rb

@@ -5,6 +5,7 @@ module MergeRequests
 
       if approval.save
         create_approval_note(merge_request)
+        mark_pending_todos_as_done(merge_request)
 
         if merge_request.approvals_left.zero?
           execute_hooks(merge_request, 'approved')
@@ -17,5 +18,9 @@ module MergeRequests
     def create_approval_note(merge_request)
       SystemNoteService.approve_mr(merge_request, current_user)
     end
+
+    def mark_pending_todos_as_done(merge_request)
+      todo_service.mark_pending_todos_as_done(merge_request, current_user)
+    end
   end
 end

spec/services/merge_requests/approval_service_spec.rb

@@ -3,35 +3,46 @@ require 'rails_helper'
 describe MergeRequests::ApprovalService, services: true do
   describe '#execute' do
     let(:user)          { build_stubbed(:user) }
-    let(:project)       { build_stubbed(:empty_project) }
     let(:merge_request) { build_stubbed(:merge_request) }
+    let(:project)       { merge_request.project }
+    let!(:todo)         { create(:todo, user: user, project: project, target: merge_request) }
+
+    subject(:service) { described_class.new(project, user) }
 
     context 'with invalid approval' do
-      it 'does not create an approval note' do
-        allow(merge_request.approvals).
-          to receive(:new).and_return(double(save: false))
-        service = described_class.new(double, double)
+      before do
+        allow(merge_request.approvals).to receive(:new).and_return(double(save: false))
+      end
 
+      it 'does not create an approval note' do
         expect(SystemNoteService).not_to receive(:approve_mr)
 
         service.execute(merge_request)
       end
+
+      it 'does not mark pending todos as done' do
+        service.execute(merge_request)
+
+        expect(todo.reload).to be_pending
+      end
     end
 
     context 'with valid approval' do
       it 'creates an approval note' do
-        service = described_class.new(project, user)
-
         expect(SystemNoteService).to receive(:approve_mr).with(merge_request, user)
 
         service.execute(merge_request)
       end
 
+      it 'marks pending todos as done' do
+        service.execute(merge_request)
+
+        expect(todo.reload).to be_done
+      end
+
       context 'with remaining approvals' do
         it 'does not fire a webhook' do
           expect(merge_request).to receive(:approvals_left).and_return(5)
-
-          service = described_class.new(project, user)
           expect(service).not_to receive(:execute_hooks)
 
           service.execute(merge_request)
@@ -41,8 +52,6 @@ describe MergeRequests::ApprovalService, services: true do
       context 'with required approvals' do
         it 'fires a webhook' do
           expect(merge_request).to receive(:approvals_left).and_return(0)
-
-          service = described_class.new(project, user)
           expect(service).to receive(:execute_hooks).with(merge_request, 'approved')
 
           service.execute(merge_request)