diff options
author | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-08-21 10:03:32 +0000 |
---|---|---|
committer | Grzegorz Bizon <grzegorz@gitlab.com> | 2018-08-21 10:03:32 +0000 |
commit | 5dac2937945cd3099ddf83ca7771b8f7ec3efac7 (patch) | |
tree | 73ccf7f50631472788ec672290324eefc60935a9 | |
parent | c0dcf8ce3c1e3d3ee2cb0e73eee615060a864714 (diff) | |
parent | f4f321b9023d5aba76541f442fe1061d70186f27 (diff) | |
download | gitlab-ce-5dac2937945cd3099ddf83ca7771b8f7ec3efac7.tar.gz |
Merge branch 'tc-api-fork-owners' into 'master'
Allow project owners to set up forking relation through API
Closes #40550
See merge request gitlab-org/gitlab-ce!18104
-rw-r--r-- | app/services/projects/fork_service.rb | 8 | ||||
-rw-r--r-- | changelogs/unreleased/tc-api-fork-owners.yml | 5 | ||||
-rw-r--r-- | doc/api/projects.md | 9 | ||||
-rw-r--r-- | lib/api/projects.rb | 2 | ||||
-rw-r--r-- | spec/requests/api/projects_spec.rb | 108 | ||||
-rw-r--r-- | spec/services/projects/fork_service_spec.rb | 8 |
6 files changed, 103 insertions, 37 deletions
diff --git a/app/services/projects/fork_service.rb b/app/services/projects/fork_service.rb index 33ad2120a75..cbbb88a9410 100644 --- a/app/services/projects/fork_service.rb +++ b/app/services/projects/fork_service.rb @@ -17,6 +17,14 @@ module Projects link_fork_network(fork_to_project) + # A forked project stores its LFS objects in the `forked_from_project`. + # So the LFS objects become inaccessible, and therefore delete them from + # the database so they'll get cleaned up. + # + # TODO: refactor this to get the correct lfs objects when implementing + # https://gitlab.com/gitlab-org/gitlab-ce/issues/39769 + fork_to_project.lfs_objects_projects.delete_all + fork_to_project end diff --git a/changelogs/unreleased/tc-api-fork-owners.yml b/changelogs/unreleased/tc-api-fork-owners.yml new file mode 100644 index 00000000000..feaa3c1705e --- /dev/null +++ b/changelogs/unreleased/tc-api-fork-owners.yml @@ -0,0 +1,5 @@ +--- +title: Allow project owners to set up forking relation through API +merge_request: 18104 +author: +type: changed diff --git a/doc/api/projects.md b/doc/api/projects.md index bda4164ee92..0936ff52dae 100644 --- a/doc/api/projects.md +++ b/doc/api/projects.md @@ -1413,12 +1413,17 @@ DELETE /projects/:id/hooks/:hook_id Note the JSON response differs if the hook is available or not. If the project hook is available before it is returned in the JSON response or an empty response is returned. -## Admin fork relation +## Fork relationship -Allows modification of the forked relationship between existing projects. Available only for admins. +Allows modification of the forked relationship between existing projects. Available only for project owners and admins. ### Create a forked from/to relation between existing projects +CAUTION: **Warning:** +This will destroy the LFS objects stored in the fork. +So to retain the LFS objects, make sure you've pulled them **before** creating the fork relation, +and push them again **after** creating the fork relation. + ``` POST /projects/:id/fork/:forked_from_id ``` diff --git a/lib/api/projects.rb b/lib/api/projects.rb index 5738bf220c6..2801ae918c6 100644 --- a/lib/api/projects.rb +++ b/lib/api/projects.rb @@ -386,7 +386,7 @@ module API requires :forked_from_id, type: String, desc: 'The ID of the project it was forked from' end post ":id/fork/:forked_from_id" do - authenticated_as_admin! + authorize! :admin_project, user_project fork_from_project = find_project!(params[:forked_from_id]) diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb index c249c881db5..881eff5d09d 100644 --- a/spec/requests/api/projects_spec.rb +++ b/spec/requests/api/projects_spec.rb @@ -402,7 +402,7 @@ describe API::Projects do context 'and with min_access_level' do before do - project2.add_master(user2) + project2.add_maintainer(user2) project3.add_developer(user2) project4.add_reporter(user2) end @@ -1121,47 +1121,87 @@ describe API::Projects do describe 'fork management' do let(:project_fork_target) { create(:project) } let(:project_fork_source) { create(:project, :public) } + let(:private_project_fork_source) { create(:project, :private) } describe 'POST /projects/:id/fork/:forked_from_id' do - let(:new_project_fork_source) { create(:project, :public) } + context 'user is a developer' do + before do + project_fork_target.add_developer(user) + end - it "is not available for non admin users" do - post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", user) - expect(response).to have_gitlab_http_status(403) - end + it 'denies project to be forked from an existing project' do + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", user) - it 'allows project to be forked from an existing project' do - expect(project_fork_target.forked?).not_to be_truthy - post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) - expect(response).to have_gitlab_http_status(201) - project_fork_target.reload - expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id) - expect(project_fork_target.forked_project_link).not_to be_nil - expect(project_fork_target.forked?).to be_truthy + expect(response).to have_gitlab_http_status(403) + end end it 'refreshes the forks count cache' do expect(project_fork_source.forks_count).to be_zero + end + + context 'user is maintainer' do + before do + project_fork_target.add_maintainer(user) + end - post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) + it 'allows project to be forked from an existing project' do + expect(project_fork_target).not_to be_forked - expect(project_fork_source.forks_count).to eq(1) - end + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", user) + project_fork_target.reload - it 'fails if forked_from project which does not exist' do - post api("/projects/#{project_fork_target.id}/fork/9999", admin) - expect(response).to have_gitlab_http_status(404) + expect(response).to have_gitlab_http_status(201) + expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id) + expect(project_fork_target.forked_project_link).to be_present + expect(project_fork_target).to be_forked + end + + it 'denies project to be forked from a private project' do + post api("/projects/#{project_fork_target.id}/fork/#{private_project_fork_source.id}", user) + + expect(response).to have_gitlab_http_status(404) + end end - it 'fails with 409 if already forked' 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.id).to eq(project_fork_source.id) - post api("/projects/#{project_fork_target.id}/fork/#{new_project_fork_source.id}", admin) - expect(response).to have_gitlab_http_status(409) - project_fork_target.reload - expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id) - expect(project_fork_target.forked?).to be_truthy + context 'user is admin' do + it 'allows project to be forked from an existing project' do + expect(project_fork_target).not_to be_forked + + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) + + expect(response).to have_gitlab_http_status(201) + end + + it 'allows project to be forked from a private project' do + post api("/projects/#{project_fork_target.id}/fork/#{private_project_fork_source.id}", admin) + + expect(response).to have_gitlab_http_status(201) + end + + it 'refreshes the forks count cachce' do + expect do + post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin) + end.to change(project_fork_source, :forks_count).by(1) + end + + it 'fails if forked_from project which does not exist' do + post api("/projects/#{project_fork_target.id}/fork/9999", admin) + expect(response).to have_gitlab_http_status(404) + end + + it 'fails with 409 if already forked' do + other_project_fork_source = create(:project, :public) + + Projects::ForkService.new(project_fork_source, admin).execute(project_fork_target) + + post api("/projects/#{project_fork_target.id}/fork/#{other_project_fork_source.id}", admin) + project_fork_target.reload + + expect(response).to have_gitlab_http_status(409) + expect(project_fork_target.forked_from_project.id).to eq(project_fork_source.id) + expect(project_fork_target).to be_forked + end end end @@ -1183,8 +1223,8 @@ describe API::Projects do before 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 + expect(project_fork_target.forked_from_project).to be_present + expect(project_fork_target).to be_forked end it 'makes forked project unforked' do @@ -1193,7 +1233,7 @@ describe API::Projects do expect(response).to have_gitlab_http_status(204) project_fork_target.reload expect(project_fork_target.forked_from_project).to be_nil - expect(project_fork_target.forked?).not_to be_truthy + expect(project_fork_target).not_to be_forked end it_behaves_like '412 response' do @@ -1228,8 +1268,8 @@ describe API::Projects do before do post api("/projects/#{private_fork.id}/fork/#{project_fork_source.id}", admin) private_fork.reload - expect(private_fork.forked_from_project).not_to be_nil - expect(private_fork.forked?).to be_truthy + expect(private_fork.forked_from_project).to be_present + expect(private_fork).to be_forked project_fork_source.reload expect(project_fork_source.forks.length).to eq(1) expect(project_fork_source.forks).to include(private_fork) diff --git a/spec/services/projects/fork_service_spec.rb b/spec/services/projects/fork_service_spec.rb index f89f9b54f53..947cb61038d 100644 --- a/spec/services/projects/fork_service_spec.rb +++ b/spec/services/projects/fork_service_spec.rb @@ -264,6 +264,14 @@ describe Projects::ForkService do expect(fork_from_project.forks_count).to eq(1) end + + it 'leaves no LFS objects dangling' do + create(:lfs_objects_project, project: fork_to_project) + + expect { subject.execute(fork_to_project) } + .to change { fork_to_project.lfs_objects_projects.count } + .to(0) + end end end end |