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

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

changelogs/unreleased/tc-api-fork-owners.yml

+---
+title: Allow project owners to set up forking relation through API
+merge_request: 18104
+author:
+type: changed

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
-Allows modification of the forked relationship between existing projects. Available only for admins.
+## Fork relationship
+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
 ```

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])

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) }
-      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 '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
+      context 'user is a developer' do
+        before do
+          project_fork_target.add_developer(user)
+        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)
+          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
-        post api("/projects/#{project_fork_target.id}/fork/#{project_fork_source.id}", admin)
-        expect(project_fork_source.forks_count).to eq(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
+
+      context 'user is maintainer' do
+        before do
+          project_fork_target.add_maintainer(user)
+        end
+        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}", user)
+          project_fork_target.reload
+          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)

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