rework the spec

spec/ee/spec/controllers/projects/merge_requests_controller_spec.rb

@@ -5,12 +5,12 @@ shared_examples 'approvals' do
     JSON.parse(response.body)
   end
 
-  let(:approver) { create(:user) }
+  let!(:approver) { create(:approver, target: project) }
+  let!(:user_approver) { create(:approver, target: project, user: user) }
 
   before do
     merge_request.update_attribute :approvals_before_merge, 2
-    project.team << [approver, :developer]
-    project.approver_ids = [user, approver].map(&:id).join(',')
+    project.team << [approver.user, :developer]
   end
 
   describe 'approve' do
@@ -23,20 +23,23 @@ shared_examples 'approvals' do
     end
 
     it 'approves the merge request' do
+      approvals = json_response
+
       expect(response).to be_success
-      expect(json_response['approvals_left']).to eq 1
-      expect(json_response['approved_by'].size).to eq 1
-      expect(json_response['approved_by'][0]['user']['username']).to eq user.username
-      expect(json_response['user_has_approved']).to be true
-      expect(json_response['user_can_approve']).to be false
-      expect(json_response['suggested_approvers'].size).to eq 1
-      expect(json_response['suggested_approvers'][0]['username']).to eq approver.username
+      expect(approvals['approvals_left']).to eq 1
+      expect(approvals['approved_by'].size).to eq 1
+      expect(approvals['approved_by'][0]['user']['username']).to eq user.username
+      expect(approvals['user_has_approved']).to be true
+      expect(approvals['user_can_approve']).to be false
+      expect(approvals['suggested_approvers'].size).to eq 1
+      expect(approvals['suggested_approvers'][0]['username']).to eq approver.user.username
     end
   end
 
   describe 'approvals' do
+    let!(:approval) { create(:approval, merge_request: merge_request, user: approver.user) }
+
     before do
-      merge_request.approvals.create(user: approver)
       get :approvals,
           namespace_id: project.namespace.to_param,
           project_id: project.to_param,
@@ -45,20 +48,23 @@ shared_examples 'approvals' do
     end
 
     it 'shows approval information' do
+      approvals = json_response
+
       expect(response).to be_success
-      expect(json_response['approvals_left']).to eq 1
-      expect(json_response['approved_by'].size).to eq 1
-      expect(json_response['approved_by'][0]['user']['username']).to eq approver.username
-      expect(json_response['user_has_approved']).to be false
-      expect(json_response['user_can_approve']).to be true
-      expect(json_response['suggested_approvers'].size).to eq 1
-      expect(json_response['suggested_approvers'][0]['username']).to eq user.username
+      expect(approvals['approvals_left']).to eq 1
+      expect(approvals['approved_by'].size).to eq 1
+      expect(approvals['approved_by'][0]['user']['username']).to eq approver.user.username
+      expect(approvals['user_has_approved']).to be false
+      expect(approvals['user_can_approve']).to be true
+      expect(approvals['suggested_approvers'].size).to eq 1
+      expect(approvals['suggested_approvers'][0]['username']).to eq user.username
     end
   end
 
   describe 'unapprove' do
+    let!(:approval) { create(:approval, merge_request: merge_request, user: user) }
+
     before do
-      merge_request.approvals.create(user: user)
       delete :unapprove,
              namespace_id: project.namespace.to_param,
              project_id: project.to_param,
@@ -67,12 +73,14 @@ shared_examples 'approvals' do
     end
 
     it 'unapproves the merge request' do
+      approvals = json_response
+
       expect(response).to be_success
-      expect(json_response['approvals_left']).to eq 2
-      expect(json_response['approved_by']).to be_empty
-      expect(json_response['user_has_approved']).to be false
-      expect(json_response['user_can_approve']).to be true
-      expect(json_response['suggested_approvers'].size).to eq 2
+      expect(approvals['approvals_left']).to eq 2
+      expect(approvals['approved_by']).to be_empty
+      expect(approvals['user_has_approved']).to be false
+      expect(approvals['user_can_approve']).to be true
+      expect(approvals['suggested_approvers'].size).to eq 2
     end
   end
 end
@@ -90,10 +98,14 @@ describe Projects::MergeRequestsController do
   it_behaves_like 'approvals'
 
   describe 'PUT update' do
+    before do
+      project.update_attributes(approvals_before_merge: 2)
+    end
+
     def update_merge_request(params = {})
       post :update,
-           namespace_id: project.namespace.to_param,
-           project_id: project.to_param,
+           namespace_id: merge_request.target_project.namespace.to_param,
+           project_id: merge_request.target_project.to_param,
            id: merge_request.iid,
            merge_request: params
     end
@@ -128,7 +140,7 @@ describe Projects::MergeRequestsController do
         let(:new_approver_group) { create(:approver_group) }
 
         before do
-          project.team << [new_approver, :developer]
+          project.add_developer(new_approver)
           project.update_attributes(disable_overriding_approvers_per_merge_request: true)
         end
 
@@ -152,7 +164,7 @@ describe Projects::MergeRequestsController do
       end
     end
 
-    context 'the approvals_before_merge param' do
+    shared_examples 'approvals_before_merge param' do
       before do
         project.update_attributes(approvals_before_merge: 2)
       end
@@ -164,12 +176,12 @@ describe Projects::MergeRequestsController do
           end
 
           it 'sets the param to nil' do
-            expect(merge_request.reload.approvals_before_merge).to eq(nil)
+            expect(merge_request.approvals_before_merge).to eq(nil)
           end
 
           it 'updates the merge request' do
-            expect(merge_request.reload).to be_valid
-            expect(response).to redirect_to(project_merge_request_path(project, merge_request))
+            expect(merge_request).to be_valid
+            expect(response).to redirect_to(project_merge_request_path(merge_request.target_project, merge_request))
           end
         end
 
@@ -184,7 +196,7 @@ describe Projects::MergeRequestsController do
 
           it 'updates the merge request' do
             expect(merge_request.reload).to be_valid
-            expect(response).to redirect_to(project_merge_request_path(project, merge_request))
+            expect(response).to redirect_to(project_merge_request_path(merge_request.target_project, merge_request))
           end
         end
 
@@ -199,7 +211,7 @@ describe Projects::MergeRequestsController do
 
           it 'updates the merge request' do
             expect(merge_request.reload).to be_valid
-            expect(response).to redirect_to(project_merge_request_path(project, merge_request))
+            expect(response).to redirect_to(project_merge_request_path(merge_request.target_project, merge_request))
           end
         end
       end
@@ -220,7 +232,7 @@ describe Projects::MergeRequestsController do
 
           it 'updates the merge request' do
             expect(merge_request.reload).to be_valid
-            expect(response).to redirect_to(project_merge_request_path(project, merge_request))
+            expect(response).to redirect_to(project_merge_request_path(merge_request.target_project, merge_request))
           end
         end
 
@@ -235,7 +247,7 @@ describe Projects::MergeRequestsController do
 
           it 'updates the merge request' do
             expect(merge_request.reload).to be_valid
-            expect(response).to redirect_to(project_merge_request_path(project, merge_request))
+            expect(response).to redirect_to(project_merge_request_path(merge_request.target_project, merge_request))
           end
         end
 
@@ -250,7 +262,7 @@ describe Projects::MergeRequestsController do
 
           it 'updates the merge request' do
             expect(merge_request.reload).to be_valid
-            expect(response).to redirect_to(project_merge_request_path(project, merge_request))
+            expect(response).to redirect_to(project_merge_request_path(merge_request.target_project, merge_request))
           end
         end
 
@@ -265,11 +277,36 @@ describe Projects::MergeRequestsController do
 
           it 'updates the merge request' do
             expect(merge_request.reload).to be_valid
-            expect(response).to redirect_to(project_merge_request_path(project, merge_request))
+            expect(response).to redirect_to(project_merge_request_path(merge_request.target_project, merge_request))
           end
         end
       end
     end
+
+    context 'when the MR targets the project' do
+      it_behaves_like 'approvals_before_merge param'
+    end
+
+    context 'when the project is a fork' do
+      let(:upstream) { create(:project, :repository) }
+      let(:project) { create(:project, :repository, forked_from_project: upstream) }
+
+      before do
+        project.add_developer(user)
+        upstream.add_developer(user)
+        upstream.update_attributes(approvals_before_merge: 2)
+      end
+
+      context 'when the MR target upstream' do
+        let(:merge_request) { create(:merge_request, title: 'This is targeting upstream', source_project: project, target_project: upstream) }
+
+        it_behaves_like 'approvals_before_merge param'
+      end
+
+      context 'when the MR target the fork' do
+        it_behaves_like 'approvals_before_merge param'
+      end
+    end
   end
 
   describe 'POST merge' do