Made suggested content changes based on MR Review

Changed the authentication method for removing fork through API
Reflected changes to new auth method in API specs

3 files changed, 51 insertions, 31 deletions

diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb
index 626b56cc789..e963e913512 100644
--- a/spec/controllers/projects_controller_spec.rb
+++ b/spec/controllers/projects_controller_spec.rb
@@ -72,9 +72,12 @@ describe ProjectsController do
       context 'with forked project' do
         let(:project_fork) { create(:project, namespace: user.namespace) }
 
-        it 'should remove fork from project' do
+        before do
           create(:forked_project_link, forked_to_project: project_fork)
-          put(:remove_fork,
+        end
+
+        it 'should remove fork from project' do
+          delete(:remove_fork,
               namespace_id: project_fork.namespace.to_param,
               id: project_fork.to_param, format: :js)
 
@@ -84,19 +87,22 @@ describe ProjectsController do
         end
       end
 
-      it 'should do nothing if project was not forked' do
-        unforked_project = create(:project, namespace: user.namespace)
-        put(:remove_fork,
-            namespace_id: unforked_project.namespace.to_param,
-            id: unforked_project.to_param, format: :js)
+      context 'when project not forked' do
+        let(:unforked_project) { create(:project, namespace: user.namespace) }
 
-        expect(flash[:notice]).to be_nil
-        expect(response).to render_template(:remove_fork)
+        it 'should do nothing if project was not forked' do
+          delete(:remove_fork,
+              namespace_id: unforked_project.namespace.to_param,
+              id: unforked_project.to_param, format: :js)
+
+          expect(flash[:notice]).to be_nil
+          expect(response).to render_template(:remove_fork)
+        end
       end
     end
 
     it "does nothing if user is not signed in" do
-      put(:remove_fork,
+      delete(:remove_fork,
           namespace_id: project.namespace.to_param,
           id: project.to_param, format: :js)
       expect(response.status).to eq(401)
diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb
index df0dcb2bb21..f3d51641ece 100644
--- a/spec/features/projects_spec.rb
+++ b/spec/features/projects_spec.rb
@@ -45,13 +45,13 @@ feature 'Project', feature: true do
     end
 
     it 'should remove fork' do
-      expect(page).to have_content 'Remove forked relationship'
+      expect(page).to have_content 'Remove fork relationship'
 
-      remove_with_confirm('Remove forked relationship', project.path)
+      remove_with_confirm('Remove fork relationship', project.path)
 
       expect(page).to have_content 'Fork relationship has been removed.'
       expect(project.forked?).to be_falsey
-      expect(page).not_to have_content 'Remove forked relationship'
+      expect(page).not_to have_content 'Remove fork relationship'
     end
   end
 
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 580bbec77d1..e9de9e0826d 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -606,28 +606,42 @@ describe API::API, api: true  do
 
     describe 'DELETE /projects/:id/fork' do
 
-      it "shouldn't available for non admin users" do
+      it "shouldn't be visible to users outside group" do
         delete api("/projects/#{project_fork_target.id}/fork", user)
-        expect(response.status).to eq(403)
+        expect(response.status).to eq(404)
       end
 
-      it 'should make forked project unforked' do
-        post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin)
-        project_fork_target.reload
-        expect(project_fork_target.forked_from_project).not_to be_nil
-        expect(project_fork_target.forked?).to be_truthy
-        delete api("/projects/#{project_fork_target.id}/fork", admin)
-        expect(response.status).to eq(200)
-        project_fork_target.reload
-        expect(project_fork_target.forked_from_project).to be_nil
-        expect(project_fork_target.forked?).not_to be_truthy
-      end
+      context 'when users belong to project group' do
+        let(:project_fork_target) { create(:project, group: create(:group)) }
 
-      it 'should be idempotent if not forked' do
-        expect(project_fork_target.forked_from_project).to be_nil
-        delete api("/projects/#{project_fork_target.id}/fork", admin)
-        expect(response.status).to eq(200)
-        expect(project_fork_target.reload.forked_from_project).to be_nil
+        before do
+          project_fork_target.group.add_owner user
+          project_fork_target.group.add_developer user2
+        end
+
+        it 'should be forbidden to non-owner users' do
+          delete api("/projects/#{project_fork_target.id}/fork", user2)
+          expect(response.status).to eq(403)
+        end
+
+        it 'should make forked project unforked' do
+          post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin)
+          project_fork_target.reload
+          expect(project_fork_target.forked_from_project).not_to be_nil
+          expect(project_fork_target.forked?).to be_truthy
+          delete api("/projects/#{project_fork_target.id}/fork", admin)
+          expect(response.status).to eq(200)
+          project_fork_target.reload
+          expect(project_fork_target.forked_from_project).to be_nil
+          expect(project_fork_target.forked?).not_to be_truthy
+        end
+
+        it 'should be idempotent if not forked' do
+          expect(project_fork_target.forked_from_project).to be_nil
+          delete api("/projects/#{project_fork_target.id}/fork", admin)
+          expect(response.status).to eq(200)
+          expect(project_fork_target.reload.forked_from_project).to be_nil
+        end
       end
     end
   end