1 files changed, 165 insertions, 46 deletions
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index 3df5d9412f8..5e6ff40e8cf 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -126,6 +126,12 @@ describe API::Commits do
         end
       end
 
+      context "with empty ref_name parameter" do
+        let(:route) { "/projects/#{project_id}/repository/commits?ref_name=" }
+
+        it_behaves_like 'project commits'
+      end
+
       context "path optional parameter" do
         it "returns project commits matching provided path parameter" do
           path = 'files/ruby/popen.rb'
@@ -281,7 +287,7 @@ describe API::Commits do
       end
 
       it 'does not increment the usage counters using access token authentication' do
-        expect(::Gitlab::WebIdeCommitsCounter).not_to receive(:increment)
+        expect(::Gitlab::UsageDataCounters::WebIdeCounter).not_to receive(:increment_commits_count)
 
         post api(url, user), params: valid_c_params
       end
@@ -320,67 +326,132 @@ describe API::Commits do
         end
       end
 
-      context 'when the API user is a guest' do
+      context 'when committing to a new branch' do
         def last_commit_id(project, branch_name)
           project.repository.find_branch(branch_name)&.dereferenced_target&.id
         end
 
-        let(:public_project) { create(:project, :public, :repository) }
-        let!(:url) { "/projects/#{public_project.id}/repository/commits" }
-        let(:guest) { create(:user).tap { |u| public_project.add_guest(u) } }
+        before do
+          valid_c_params[:start_branch] = 'master'
+          valid_c_params[:branch] = 'patch'
+        end
 
-        it 'returns a 403' do
-          post api(url, guest), params: valid_c_params
+        context 'when the API user is a guest' do
+          let(:public_project) { create(:project, :public, :repository) }
+          let(:url) { "/projects/#{public_project.id}/repository/commits" }
+          let(:guest) { create(:user).tap { |u| public_project.add_guest(u) } }
 
-          expect(response).to have_gitlab_http_status(403)
-        end
+          it 'returns a 403' do
+            post api(url, guest), params: valid_c_params
 
-        context 'when start_project is provided' do
-          context 'when posting to a forked project the user owns' do
-            let!(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: true) }
-            let!(:url) { "/projects/#{forked_project.id}/repository/commits" }
+            expect(response).to have_gitlab_http_status(403)
+          end
 
-            before do
-              valid_c_params[:start_branch]  = "master"
-              valid_c_params[:branch]        = "patch"
-            end
+          context 'when start_project is provided' do
+            context 'when posting to a forked project the user owns' do
+              let(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: true) }
+              let(:url) { "/projects/#{forked_project.id}/repository/commits" }
+
+              context 'identified by Integer (id)' do
+                before do
+                  valid_c_params[:start_project] = public_project.id
+                end
+
+                it 'adds a new commit to forked_project and returns a 201' do
+                  expect_request_with_status(201) { post api(url, guest), params: valid_c_params }
+                    .to change { last_commit_id(forked_project, valid_c_params[:branch]) }
+                    .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) }
+                end
+              end
 
-            context 'identified by Integer (id)' do
-              before do
-                valid_c_params[:start_project] = public_project.id
+              context 'identified by String (full_path)' do
+                before do
+                  valid_c_params[:start_project] = public_project.full_path
+                end
+
+                it 'adds a new commit to forked_project and returns a 201' do
+                  expect_request_with_status(201) { post api(url, guest), params: valid_c_params }
+                    .to change { last_commit_id(forked_project, valid_c_params[:branch]) }
+                    .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) }
+                end
+              end
+
+              context 'when branch already exists' do
+                before do
+                  valid_c_params.delete(:start_branch)
+                  valid_c_params[:branch] = 'master'
+                  valid_c_params[:start_project] = public_project.id
+                end
+
+                it 'returns a 400' do
+                  post api(url, guest), params: valid_c_params
+
+                  expect(response).to have_gitlab_http_status(400)
+                  expect(json_response['message']).to eq("A branch called 'master' already exists. Switch to that branch in order to make changes")
+                end
+
+                context 'when force is set to true' do
+                  before do
+                    valid_c_params[:force] = true
+                  end
+
+                  it 'adds a new commit to forked_project and returns a 201' do
+                    expect_request_with_status(201) { post api(url, guest), params: valid_c_params }
+                      .to change { last_commit_id(forked_project, valid_c_params[:branch]) }
+                      .and not_change { last_commit_id(public_project, valid_c_params[:branch]) }
+                  end
+                end
               end
 
-              it 'adds a new commit to forked_project and returns a 201' do
-                expect { post api(url, guest), params: valid_c_params }
-                  .to change { last_commit_id(forked_project, valid_c_params[:branch]) }
-                  .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) }
+              context 'when start_sha is also provided' do
+                let(:forked_project) { fork_project(public_project, guest, namespace: guest.namespace, repository: false) }
+                let(:start_sha) { public_project.repository.commit.parent.sha }
 
-                expect(response).to have_gitlab_http_status(201)
+                before do
+                  # initialize an empty repository to force fetching from the original project
+                  forked_project.repository.create_if_not_exists
+
+                  valid_c_params[:start_project] = public_project.id
+                  valid_c_params[:start_sha] = start_sha
+                  valid_c_params.delete(:start_branch)
+                end
+
+                it 'fetches the start_sha from the original project to use as parent commit and returns a 201' do
+                  expect_request_with_status(201) { post api(url, guest), params: valid_c_params }
+                    .to change { last_commit_id(forked_project, valid_c_params[:branch]) }
+                    .and not_change { last_commit_id(forked_project, 'master') }
+
+                  last_commit = forked_project.repository.find_branch(valid_c_params[:branch]).dereferenced_target
+                  expect(last_commit.parent_id).to eq(start_sha)
+                end
               end
             end
 
-            context 'identified by String (full_path)' do
+            context 'when the target project is not part of the fork network of start_project' do
+              let(:unrelated_project) { create(:project, :public, :repository, creator: guest) }
+              let(:url) { "/projects/#{unrelated_project.id}/repository/commits" }
+
               before do
-                valid_c_params[:start_project] = public_project.full_path
+                valid_c_params[:start_branch] = 'master'
+                valid_c_params[:branch] = 'patch'
+                valid_c_params[:start_project] = public_project.id
               end
 
-              it 'adds a new commit to forked_project and returns a 201' do
-                expect { post api(url, guest), params: valid_c_params }
-                  .to change { last_commit_id(forked_project, valid_c_params[:branch]) }
-                  .and not_change { last_commit_id(public_project, valid_c_params[:start_branch]) }
+              it 'returns a 403' do
+                post api(url, guest), params: valid_c_params
 
-                expect(response).to have_gitlab_http_status(201)
+                expect(response).to have_gitlab_http_status(403)
               end
             end
           end
 
-          context 'when the target project is not part of the fork network of start_project' do
-            let(:unrelated_project) { create(:project, :public, :repository, creator: guest) }
-            let!(:url) { "/projects/#{unrelated_project.id}/repository/commits" }
+          context 'when posting to a forked project the user does not have write access' do
+            let(:forked_project) { fork_project(public_project, user, namespace: user.namespace, repository: true) }
+            let(:url) { "/projects/#{forked_project.id}/repository/commits" }
 
             before do
-              valid_c_params[:start_branch]  = "master"
-              valid_c_params[:branch]        = "patch"
+              valid_c_params[:start_branch] = 'master'
+              valid_c_params[:branch] = 'patch'
               valid_c_params[:start_project] = public_project.id
             end
 
@@ -392,20 +463,68 @@ describe API::Commits do
           end
         end
 
-        context 'when posting to a forked project the user does not have write access' do
-          let!(:forked_project) { fork_project(public_project, user, namespace: user.namespace, repository: true) }
-          let!(:url) { "/projects/#{forked_project.id}/repository/commits" }
+        context 'when start_sha is provided' do
+          let(:start_sha) { project.repository.commit.parent.sha }
 
           before do
-            valid_c_params[:start_branch]  = "master"
-            valid_c_params[:branch]        = "patch"
-            valid_c_params[:start_project] = public_project.id
+            valid_c_params[:start_sha] = start_sha
+            valid_c_params.delete(:start_branch)
           end
 
-          it 'returns a 403' do
-            post api(url, guest), params: valid_c_params
+          it 'returns a 400 if start_branch is also provided' do
+            valid_c_params[:start_branch] = 'master'
+            post api(url, user), params: valid_c_params
 
-            expect(response).to have_gitlab_http_status(403)
+            expect(response).to have_gitlab_http_status(400)
+            expect(json_response['error']).to eq('start_branch, start_sha are mutually exclusive')
+          end
+
+          it 'returns a 400 if branch already exists' do
+            valid_c_params[:branch] = 'master'
+            post api(url, user), params: valid_c_params
+
+            expect(response).to have_gitlab_http_status(400)
+            expect(json_response['message']).to eq("A branch called 'master' already exists. Switch to that branch in order to make changes")
+          end
+
+          it 'returns a 400 if start_sha does not exist' do
+            valid_c_params[:start_sha] = '1' * 40
+            post api(url, user), params: valid_c_params
+
+            expect(response).to have_gitlab_http_status(400)
+            expect(json_response['message']).to eq("Cannot find start_sha '#{valid_c_params[:start_sha]}'")
+          end
+
+          it 'returns a 400 if start_sha is not a full SHA' do
+            valid_c_params[:start_sha] = start_sha.slice(0, 7)
+            post api(url, user), params: valid_c_params
+
+            expect(response).to have_gitlab_http_status(400)
+            expect(json_response['message']).to eq("Invalid start_sha '#{valid_c_params[:start_sha]}'")
+          end
+
+          it 'uses the start_sha as parent commit and returns a 201' do
+            expect_request_with_status(201) { post api(url, user), params: valid_c_params }
+              .to change { last_commit_id(project, valid_c_params[:branch]) }
+              .and not_change { last_commit_id(project, 'master') }
+
+            last_commit = project.repository.find_branch(valid_c_params[:branch]).dereferenced_target
+            expect(last_commit.parent_id).to eq(start_sha)
+          end
+
+          context 'when force is set to true and branch already exists' do
+            before do
+              valid_c_params[:force] = true
+              valid_c_params[:branch] = 'master'
+            end
+
+            it 'uses the start_sha as parent commit and returns a 201' do
+              expect_request_with_status(201) { post api(url, user), params: valid_c_params }
+                .to change { last_commit_id(project, valid_c_params[:branch]) }
+
+              last_commit = project.repository.find_branch(valid_c_params[:branch]).dereferenced_target
+              expect(last_commit.parent_id).to eq(start_sha)
+            end
           end
         end
       end