Merge branch...

Merge branch '799-viewing-a-mr-with-approvals-with-an-anonymous-user-results-in-error-500' into 'master'

Don't check if anonymous user can update MR

Closes #799. Interestingly, I think previously we showed the 'Approve MR' button to anonymous users when an MR had approvals from anyone allowed, but we didn't blow up with a 500 error.

See merge request !582

app/models/merge_request.rb

@@ -623,6 +623,7 @@ class MergeRequest < ActiveRecord::Base
   end
 
   def can_approve?(user)
+    return false unless user
     return true if approvers_left.include?(user)
     return false if user == author
     return false unless user.can?(:update_merge_request, self)

spec/models/merge_request_spec.rb

@@ -781,6 +781,10 @@ describe MergeRequest, models: true do
           it 'does not allow the approver to approve the MR' do
             expect(merge_request.can_approve?(author)).to be_falsey
           end
+
+          it 'does not allow a logged-out user to approve the MR' do
+            expect(merge_request.can_approve?(nil)).to be_falsey
+          end
         end
 
         context 'when that approver is not the MR author' do
@@ -796,6 +800,10 @@ describe MergeRequest, models: true do
           it 'allows the approver to approve the MR' do
             expect(merge_request.can_approve?(approver)).to be_truthy
           end
+
+          it 'does not allow a logged-out user to approve the MR' do
+            expect(merge_request.can_approve?(nil)).to be_falsey
+          end
         end
       end
     end
@@ -834,6 +842,10 @@ describe MergeRequest, models: true do
             expect(merge_request.can_approve?(reporter)).to be_falsey
             expect(merge_request.can_approve?(stranger)).to be_falsey
           end
+
+          it 'does not allow a logged-out user to approve the MR' do
+            expect(merge_request.can_approve?(nil)).to be_falsey
+          end
         end
 
         context 'when that approver is not the MR author' do
@@ -850,6 +862,7 @@ describe MergeRequest, models: true do
             expect(merge_request.can_approve?(developer)).to be_falsey
             expect(merge_request.can_approve?(reporter)).to be_falsey
             expect(merge_request.can_approve?(stranger)).to be_falsey
+            expect(merge_request.can_approve?(nil)).to be_falsey
           end
         end
       end
@@ -876,6 +889,10 @@ describe MergeRequest, models: true do
             expect(merge_request.can_approve?(approver)).to be_truthy
           end
 
+          it 'does not allow a logged-out user to approve the MR' do
+            expect(merge_request.can_approve?(nil)).to be_falsey
+          end
+
           context 'when all of the valid approvers have approved the MR' do
             before do
               create(:approval, user: approver, merge_request: merge_request)
@@ -900,6 +917,7 @@ describe MergeRequest, models: true do
 
               expect(merge_request.can_approve?(reporter)).to be_falsey
               expect(merge_request.can_approve?(stranger)).to be_falsey
+              expect(merge_request.can_approve?(nil)).to be_falsey
             end
           end
         end
@@ -923,6 +941,7 @@ describe MergeRequest, models: true do
             expect(merge_request.can_approve?(author)).to be_falsey
             expect(merge_request.can_approve?(reporter)).to be_falsey
             expect(merge_request.can_approve?(stranger)).to be_falsey
+            expect(merge_request.can_approve?(nil)).to be_falsey
           end
         end
       end