Merge branch 'hanloong/gitlab-ce-remove-forks-from-projects-settings' into 'master'

Add ability to remove the fork relationship from project settings

![Screen_Shot_2015-10-18_at_12.37.24](/uploads/676571642a4c90f7f286280d714599a3/Screen_Shot_2015-10-18_at_12.37.24.png)
![Screen_Shot_2015-10-18_at_12.37.28](/uploads/1a069ecfc4cd3b5438772a9c3f04b6fc/Screen_Shot_2015-10-18_at_12.37.28.png)

Replaces !1579.

Fixes #2578.

See merge request !1636

CHANGELOG

@@ -3,6 +3,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 v 8.2.0 (unreleased)
   - Show last project commit to default branch on project home page
   - Highlight comment based on anchor in URL
+  - Adds ability to remove the forked relationship from project settings screen. (Han Loong Liauw)
 v 8.1.0 (unreleased)
   - Fix bug preventing mentioned issued from being closed when MR is merged using fast-forward merge.

app/controllers/projects_controller.rb

@@ -8,7 +8,7 @@ class ProjectsController < ApplicationController
   before_action :assign_ref_vars, :tree, only: [:show], if: :repo_exists?
   # Authorize
-  before_action :authorize_admin_project!, only: [:edit, :update, :destroy, :transfer, :archive, :unarchive]
+  before_action :authorize_admin_project!, only: [:edit, :update]
   before_action :event_filter, only: [:show, :activity]
   layout :determine_layout
@@ -59,6 +59,8 @@ class ProjectsController < ApplicationController
   end
   def transfer
+    return access_denied! unless can?(current_user, :change_namespace, @project)
+
     namespace = Namespace.find_by(id: params[:new_namespace_id])
     ::Projects::TransferService.new(project, current_user).execute(namespace)
@@ -67,6 +69,15 @@ class ProjectsController < ApplicationController
     end
   end
+  def remove_fork
+    return access_denied! unless can?(current_user, :remove_fork_project, @project)
+
+    if @project.forked?
+      @project.forked_project_link.destroy
+      flash[:notice] = 'The fork relationship has been removed.'
+    end
+  end
+
   def activity
     respond_to do |format|
       format.html
@@ -142,6 +153,7 @@ class ProjectsController < ApplicationController
   def archive
     return access_denied! unless can?(current_user, :archive_project, @project)
+
     @project.archive!
     respond_to do |format|
@@ -151,6 +163,7 @@ class ProjectsController < ApplicationController
   def unarchive
     return access_denied! unless can?(current_user, :archive_project, @project)
+
     @project.unarchive!
     respond_to do |format|

app/helpers/projects_helper.rb

@@ -70,6 +70,10 @@ module ProjectsHelper
     "You are going to transfer #{project.name_with_namespace} to another owner. Are you ABSOLUTELY sure?"
   end
+  def remove_fork_project_message(project)
+    "You are going to remove the fork relationship to source project #{@project.forked_from_project.name_with_namespace}.  Are you ABSOLUTELY sure?"
+  end
+
   def project_nav_tabs
     @nav_tabs ||= get_project_nav_tabs(@project, current_user)
   end

app/models/ability.rb

@@ -189,7 +189,8 @@ class Ability
         :change_visibility_level,
         :rename_project,
         :remove_project,
-        :archive_project
+        :archive_project,
+        :remove_fork_project
       ]
     end

app/views/projects/edit.html.haml

@@ -189,6 +189,21 @@
       - else
         .nothing-here-block Only the project owner can transfer a project
+      - if @project.forked?
+        - if can?(current_user, :remove_fork_project, @project)
+          = form_for([@project.namespace.becomes(Namespace), @project], url: remove_fork_namespace_project_path(@project.namespace, @project), method: :delete, remote: true, html: { class: 'transfer-project form-horizontal' }) do |f|
+            .panel.panel-default.panel.panel-danger
+              .panel-heading Remove fork relationship
+              .panel-body
+                %p
+                  This will remove the fork relationship to source project
+                  #{link_to @project.forked_from_project.name_with_namespace, project_path(@project.forked_from_project)}.
+                  %br
+                  %strong Once removed, the fork relationship cannot be restored and you will no longer be able to send merge requests to the source.
+                = button_to 'Remove fork relationship', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_fork_project_message(@project) }
+        - else
+          .nothing-here-block Only the project owner can remove the fork relationship.
+
       - if can?(current_user, :remove_project, @project)
         .panel.panel-default.panel.panel-danger
           .panel-heading Remove project
@@ -201,7 +216,8 @@
               = button_to 'Remove project', '#', class: "btn btn-remove js-confirm-danger", data: { "confirm-danger-message" => remove_project_message(@project) }
       - else
-        .nothing-here-block Only project owner can remove a project
+        .nothing-here-block Only the project owner can remove a project.
+
 .save-project-loader.hide
   .center

app/views/projects/remove_fork.js.haml

+:plain
+    location.href = "#{edit_namespace_project_path(@project.namespace, @project)}";

config/routes.rb

@@ -378,6 +378,7 @@ Gitlab::Application.routes.draw do
               [:new, :create, :index], path: "/") do
       member do
         put :transfer
+        delete :remove_fork
         post :archive
         post :unarchive
         post :toggle_star

lib/api/projects.rb

@@ -246,8 +246,8 @@ module API
       # Example Request:
       #  DELETE /projects/:id/fork
       delete ":id/fork" do
-        authenticated_as_admin!
-        unless user_project.forked_project_link.nil?
+        authorize! :remove_fork_project, user_project
+        if user_project.forked?
           user_project.forked_project_link.destroy
         end
       end

spec/controllers/projects_controller_spec.rb

@@ -90,4 +90,50 @@ describe ProjectsController do
       expect(user.starred?(public_project)).to be_falsey
     end
   end
+
+  describe "DELETE remove_fork" do
+    context 'when signed in' do
+      before do
+        sign_in(user)
+      end
+
+      context 'with forked project' do
+        let(:project_fork) { create(:project, namespace: user.namespace) }
+
+        before do
+          create(:forked_project_link, forked_to_project: project_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)
+
+          expect(project_fork.forked?).to be_falsey
+          expect(flash[:notice]).to eq('The fork relationship has been removed.')
+          expect(response).to render_template(:remove_fork)
+        end
+      end
+
+      context 'when project not forked' do
+        let(:unforked_project) { create(:project, namespace: user.namespace) }
+
+        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
+      delete(:remove_fork,
+          namespace_id: project.namespace.to_param,
+          id: project.to_param, format: :js)
+      expect(response.status).to eq(401)
+    end
+  end
 end

spec/features/projects_spec.rb

@@ -34,6 +34,27 @@ feature 'Project', feature: true do
     end
   end
+  describe 'remove forked relationship', js: true do
+    let(:user)    { create(:user) }
+    let(:project) { create(:project, namespace: user.namespace) }
+
+    before do
+      login_with user
+      create(:forked_project_link, forked_to_project: project)
+      visit edit_namespace_project_path(project.namespace, project)
+    end
+
+    it 'should remove fork' do
+      expect(page).to have_content 'Remove fork relationship'
+
+      remove_with_confirm('Remove fork relationship', project.path)
+
+      expect(page).to have_content 'The fork relationship has been removed.'
+      expect(project.forked?).to be_falsey
+      expect(page).not_to have_content 'Remove fork relationship'
+    end
+  end
+
   describe 'removal', js: true do
     let(:user)    { create(:user) }
     let(:project) { create(:project, namespace: user.namespace) }
@@ -45,13 +66,13 @@ feature 'Project', feature: true do
     end
     it 'should remove project' do
-      expect { remove_project }.to change {Project.count}.by(-1)
+      expect { remove_with_confirm('Remove project', project.path) }.to change {Project.count}.by(-1)
     end
   end
-  def remove_project
-    click_button "Remove project"
-    fill_in 'confirm_name_input', with: project.path
+  def remove_with_confirm(button_text, confirm_with)
+    click_button button_text
+    fill_in 'confirm_name_input', with: confirm_with
     click_button 'Confirm'
   end
 end

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
-      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
+      context 'when users belong to project group' do
+        let(:project_fork_target) { create(:project, group: create(:group)) }
+        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