diff --git a/CHANGELOG b/CHANGELOG index 699b44f00d7..f8daa6d246c 100644 --- a/CHANGELOG +++ b/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. diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index bb2df275b77..73200396ecc 100644 --- a/app/controllers/projects_controller.rb +++ b/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| diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index dd5e3828da2..5301c2ccf76 100644 --- a/app/helpers/projects_helper.rb +++ b/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 diff --git a/app/models/ability.rb b/app/models/ability.rb index 38bc2086683..b72178fa126 100644 --- a/app/models/ability.rb +++ b/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 diff --git a/app/views/projects/edit.html.haml b/app/views/projects/edit.html.haml index 1882a82fba5..afbf88b5507 100644 --- a/app/views/projects/edit.html.haml +++ b/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 diff --git a/app/views/projects/remove_fork.js.haml b/app/views/projects/remove_fork.js.haml new file mode 100644 index 00000000000..17b9fecfeb1 --- /dev/null +++ b/app/views/projects/remove_fork.js.haml @@ -0,0 +1,2 @@ +:plain + location.href = "#{edit_namespace_project_path(@project.namespace, @project)}"; diff --git a/config/routes.rb b/config/routes.rb index 3dbe2c4dfcc..f6812c9280a 100644 --- a/config/routes.rb +++ b/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 diff --git a/lib/api/projects.rb b/lib/api/projects.rb index c2fb36b4143..67ee66a2058 100644 --- a/lib/api/projects.rb +++ b/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 diff --git a/spec/controllers/projects_controller_spec.rb b/spec/controllers/projects_controller_spec.rb index b7ec5e48e85..4460bf12f96 100644 --- a/spec/controllers/projects_controller_spec.rb +++ b/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 diff --git a/spec/features/projects_spec.rb b/spec/features/projects_spec.rb index aac93b17a38..09fcff2444a 100644 --- a/spec/features/projects_spec.rb +++ b/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 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