Refactor changing files in web UI

15 files changed, 45 insertions, 45 deletions

diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb
index 3e9f272a0d8..0fd09d156c4 100644
--- a/spec/controllers/projects/blob_controller_spec.rb
+++ b/spec/controllers/projects/blob_controller_spec.rb
@@ -106,7 +106,7 @@ describe Projects::BlobController do
         namespace_id: project.namespace,
         project_id: project,
         id: 'master/CHANGELOG',
-        target_branch: 'master',
+        branch_name: 'master',
         content: 'Added changes',
         commit_message: 'Update CHANGELOG'
       }
@@ -178,7 +178,7 @@ describe Projects::BlobController do
 
       context 'when editing on the original repository' do
         it "redirects to forked project new merge request" do
-          default_params[:target_branch] = "fork-test-1"
+          default_params[:branch_name] = "fork-test-1"
           default_params[:create_merge_request] = 1
 
           put :update, default_params
diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb
index ab94e292e48..a43dad5756d 100644
--- a/spec/controllers/projects/tree_controller_spec.rb
+++ b/spec/controllers/projects/tree_controller_spec.rb
@@ -97,29 +97,29 @@ describe Projects::TreeController do
            project_id: project,
            id: 'master',
            dir_name: path,
-           target_branch: target_branch,
+           branch_name: branch_name,
            commit_message: 'Test commit message')
     end
 
     context 'successful creation' do
       let(:path) { 'files/new_dir'}
-      let(:target_branch) { 'master-test'}
+      let(:branch_name) { 'master-test'}
 
       it 'redirects to the new directory' do
         expect(subject).
-            to redirect_to("/#{project.path_with_namespace}/tree/#{target_branch}/#{path}")
+            to redirect_to("/#{project.path_with_namespace}/tree/#{branch_name}/#{path}")
         expect(flash[:notice]).to eq('The directory has been successfully created.')
       end
     end
 
     context 'unsuccessful creation' do
       let(:path) { 'README.md' }
-      let(:target_branch) { 'master'}
+      let(:branch_name) { 'master'}
 
       it 'does not allow overwriting of existing files' do
         expect(subject).
             to redirect_to("/#{project.path_with_namespace}/tree/master")
-        expect(flash[:alert]).to eq('Directory already exists as a file')
+        expect(flash[:alert]).to eq('A file with this name already exists')
       end
     end
   end
diff --git a/spec/features/projects/blobs/user_create_spec.rb b/spec/features/projects/blobs/user_create_spec.rb
index fa1a753afcb..6ea149956fe 100644
--- a/spec/features/projects/blobs/user_create_spec.rb
+++ b/spec/features/projects/blobs/user_create_spec.rb
@@ -77,7 +77,7 @@ feature 'New blob creation', feature: true, js: true do
         project,
         user,
         start_branch: 'master',
-        target_branch: 'master',
+        branch_name: 'master',
         commit_message: 'Create file',
         file_path: 'feature.rb',
         file_content: content
@@ -87,7 +87,7 @@ feature 'New blob creation', feature: true, js: true do
     end
 
     scenario 'shows error message' do
-      expect(page).to have_content('Your changes could not be committed because a file with the same name already exists')
+      expect(page).to have_content('A file with this name already exists')
       expect(page).to have_content('New file')
       expect(page).to have_content('NextFeature')
     end
diff --git a/spec/features/projects/files/creating_a_file_spec.rb b/spec/features/projects/files/creating_a_file_spec.rb
index 5d7bd3dc4ce..de6905f2b58 100644
--- a/spec/features/projects/files/creating_a_file_spec.rb
+++ b/spec/features/projects/files/creating_a_file_spec.rb
@@ -29,16 +29,16 @@ feature 'User wants to create a file', feature: true do
 
   scenario 'directory name contains Chinese characters' do
     submit_new_file(file_name: '中文/测试.md')
-    expect(page).to have_content 'The file has been successfully created.'
+    expect(page).to have_content 'The file has been successfully created'
   end
 
   scenario 'file name contains invalid characters' do
     submit_new_file(file_name: '\\')
-    expect(page).to have_content 'Your changes could not be committed, because the file name can contain only'
+    expect(page).to have_content 'Path can contain only'
   end
 
   scenario 'file name contains directory traversal' do
     submit_new_file(file_name: '../README.md')
-    expect(page).to have_content 'Your changes could not be committed, because the file name cannot include directory traversal.'
+    expect(page).to have_content 'Path cannot include directory traversal'
   end
 end
diff --git a/spec/features/projects/files/editing_a_file_spec.rb b/spec/features/projects/files/editing_a_file_spec.rb
index 3e544316f28..4da34108b46 100644
--- a/spec/features/projects/files/editing_a_file_spec.rb
+++ b/spec/features/projects/files/editing_a_file_spec.rb
@@ -8,7 +8,7 @@ feature 'User wants to edit a file', feature: true do
   let(:commit_params) do
     {
       start_branch: project.default_branch,
-      target_branch: project.default_branch,
+      branch_name: project.default_branch,
       commit_message: "Committing First Update",
       file_path: ".gitignore",
       file_content: "First Update",
diff --git a/spec/features/projects/view_on_env_spec.rb b/spec/features/projects/view_on_env_spec.rb
index ce5c5f21167..34c6a10950f 100644
--- a/spec/features/projects/view_on_env_spec.rb
+++ b/spec/features/projects/view_on_env_spec.rb
@@ -25,7 +25,7 @@ describe 'View on environment', js: true do
         project,
         user,
         start_branch: branch_name,
-        target_branch: branch_name,
+        branch_name: branch_name,
         commit_message: "Add .gitlab/route-map.yml",
         file_path: '.gitlab/route-map.yml',
         file_content: route_map
@@ -36,7 +36,7 @@ describe 'View on environment', js: true do
         project,
         user,
         start_branch: branch_name,
-        target_branch: branch_name,
+        branch_name: branch_name,
         commit_message: "Update feature",
         file_path: file_path,
         file_content: "# Noop"
diff --git a/spec/lib/gitlab/diff/position_tracer_spec.rb b/spec/lib/gitlab/diff/position_tracer_spec.rb
index 994995b57b8..c166f83664a 100644
--- a/spec/lib/gitlab/diff/position_tracer_spec.rb
+++ b/spec/lib/gitlab/diff/position_tracer_spec.rb
@@ -100,7 +100,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
       project,
       current_user,
       start_branch: branch_name,
-      target_branch: branch_name,
+      branch_name: branch_name,
       commit_message: "Create file",
       file_path: file_name,
       file_content: content
@@ -113,7 +113,7 @@ describe Gitlab::Diff::PositionTracer, lib: true do
       project,
       current_user,
       start_branch: branch_name,
-      target_branch: branch_name,
+      branch_name: branch_name,
       commit_message: "Update file",
       file_path: file_name,
       file_content: content
@@ -122,11 +122,11 @@ describe Gitlab::Diff::PositionTracer, lib: true do
   end
 
   def delete_file(branch_name, file_name)
-    Files::DestroyService.new(
+    Files::DeleteService.new(
       project,
       current_user,
       start_branch: branch_name,
-      target_branch: branch_name,
+      branch_name: branch_name,
       commit_message: "Delete file",
       file_path: file_name
     ).execute
diff --git a/spec/lib/gitlab/git/index_spec.rb b/spec/lib/gitlab/git/index_spec.rb
index 07d71f6777d..21b71654251 100644
--- a/spec/lib/gitlab/git/index_spec.rb
+++ b/spec/lib/gitlab/git/index_spec.rb
@@ -33,7 +33,7 @@ describe Gitlab::Git::Index, seed_helper: true do
       end
 
       it 'raises an error' do
-        expect { index.create(options) }.to raise_error('Filename already exists')
+        expect { index.create(options) }.to raise_error('A file with this name already exists')
       end
     end
 
@@ -89,7 +89,7 @@ describe Gitlab::Git::Index, seed_helper: true do
       end
 
       it 'raises an error' do
-        expect { index.create_dir(options) }.to raise_error('Directory already exists as a file')
+        expect { index.create_dir(options) }.to raise_error('A file with this name already exists')
       end
     end
 
@@ -99,7 +99,7 @@ describe Gitlab::Git::Index, seed_helper: true do
       end
 
       it 'raises an error' do
-        expect { index.create_dir(options) }.to raise_error('Directory already exists')
+        expect { index.create_dir(options) }.to raise_error('A directory with this name already exists')
       end
     end
   end
@@ -118,7 +118,7 @@ describe Gitlab::Git::Index, seed_helper: true do
       end
 
       it 'raises an error' do
-        expect { index.update(options) }.to raise_error("File doesn't exist")
+        expect { index.update(options) }.to raise_error("A file with this name doesn't exist")
       end
     end
 
@@ -156,7 +156,15 @@ describe Gitlab::Git::Index, seed_helper: true do
       it 'raises an error' do
         options[:previous_path] = 'documents/story.txt'
 
-        expect { index.move(options) }.to raise_error("File doesn't exist")
+        expect { index.move(options) }.to raise_error("A file with this name doesn't exist")
+      end
+    end
+
+    context 'when a file at the new path already exists' do
+      it 'raises an error' do
+        options[:file_path] = 'CHANGELOG'
+
+        expect { index.move(options) }.to raise_error("A file with this name already exists")
       end
     end
 
@@ -203,7 +211,7 @@ describe Gitlab::Git::Index, seed_helper: true do
       end
 
       it 'raises an error' do
-        expect { index.delete(options) }.to raise_error("File doesn't exist")
+        expect { index.delete(options) }.to raise_error("A file with this name doesn't exist")
       end
     end
 
diff --git a/spec/lib/gitlab/git_access_spec.rb b/spec/lib/gitlab/git_access_spec.rb
index 703b41f95ac..d8b72615fab 100644
--- a/spec/lib/gitlab/git_access_spec.rb
+++ b/spec/lib/gitlab/git_access_spec.rb
@@ -211,7 +211,7 @@ describe Gitlab::GitAccess, lib: true do
         target_branch = project.repository.lookup('feature')
         source_branch = project.repository.create_file(
           user,
-          'John Doe',
+          'filename',
           'This is the file content',
           message: 'This is a good commit message',
           branch_name: unprotected_branch)
diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb
index ba45e2d758c..127cd8c78d8 100644
--- a/spec/lib/gitlab/regex_spec.rb
+++ b/spec/lib/gitlab/regex_spec.rb
@@ -32,12 +32,6 @@ describe Gitlab::Regex, lib: true do
     it { is_expected.to match('foo@bar') }
   end
 
-  describe '.file_path_regex' do
-    subject { described_class.file_path_regex }
-
-    it { is_expected.to match('foo@/bar') }
-  end
-
   describe '.environment_slug_regex' do
     subject { described_class.environment_slug_regex }
 
diff --git a/spec/requests/api/commits_spec.rb b/spec/requests/api/commits_spec.rb
index a10d876ffad..42dbab586cd 100644
--- a/spec/requests/api/commits_spec.rb
+++ b/spec/requests/api/commits_spec.rb
@@ -599,8 +599,7 @@ describe API::Commits, api: true  do
         post api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user), branch: 'markdown'
 
         expect(response).to have_http_status(400)
-        expect(json_response['message']).to eq('Sorry, we cannot cherry-pick this commit automatically.
-                     A cherry-pick may have already been performed with this commit, or a more recent commit may have updated some of its content.')
+        expect(json_response['message']).to include('Sorry, we cannot cherry-pick this commit automatically.')
       end
 
       it 'returns 400 if you are not allowed to push to the target branch' do
diff --git a/spec/requests/api/files_spec.rb b/spec/requests/api/files_spec.rb
index 8012530f139..6db2faed76b 100644
--- a/spec/requests/api/files_spec.rb
+++ b/spec/requests/api/files_spec.rb
@@ -205,7 +205,7 @@ describe API::Files, api: true  do
 
     it "returns a 400 if editor fails to create file" do
       allow_any_instance_of(Repository).to receive(:create_file).
-        and_return(false)
+        and_raise(Repository::CommitError, 'Cannot create file')
 
       post api(route("any%2Etxt"), user), valid_params
 
@@ -299,8 +299,8 @@ describe API::Files, api: true  do
       expect(response).to have_http_status(400)
     end
 
-    it "returns a 400 if fails to create file" do
-      allow_any_instance_of(Repository).to receive(:delete_file).and_return(false)
+    it "returns a 400 if fails to delete file" do
+      allow_any_instance_of(Repository).to receive(:delete_file).and_raise(Repository::CommitError, 'Cannot delete file')
 
       delete api(route(file_path), user), valid_params
 
diff --git a/spec/requests/api/v3/commits_spec.rb b/spec/requests/api/v3/commits_spec.rb
index adba3a787aa..0a28cb9bddb 100644
--- a/spec/requests/api/v3/commits_spec.rb
+++ b/spec/requests/api/v3/commits_spec.rb
@@ -485,8 +485,7 @@ describe API::V3::Commits, api: true do
         post v3_api("/projects/#{project.id}/repository/commits/#{master_pickable_commit.id}/cherry_pick", user), branch: 'markdown'
 
         expect(response).to have_http_status(400)
-        expect(json_response['message']).to eq('Sorry, we cannot cherry-pick this commit automatically.
-                     A cherry-pick may have already been performed with this commit, or a more recent commit may have updated some of its content.')
+        expect(json_response['message']).to include('Sorry, we cannot cherry-pick this commit automatically.')
       end
 
       it 'returns 400 if you are not allowed to push to the target branch' do
diff --git a/spec/requests/api/v3/files_spec.rb b/spec/requests/api/v3/files_spec.rb
index 349fd6b3415..c45e2028e1d 100644
--- a/spec/requests/api/v3/files_spec.rb
+++ b/spec/requests/api/v3/files_spec.rb
@@ -129,7 +129,7 @@ describe API::V3::Files, api: true  do
 
     it "returns a 400 if editor fails to create file" do
       allow_any_instance_of(Repository).to receive(:create_file).
-        and_return(false)
+        and_raise(Repository::CommitError, 'Cannot create file')
 
       post v3_api("/projects/#{project.id}/repository/files", user), valid_params
 
@@ -229,8 +229,8 @@ describe API::V3::Files, api: true  do
       expect(response).to have_http_status(400)
     end
 
-    it "returns a 400 if fails to create file" do
-      allow_any_instance_of(Repository).to receive(:delete_file).and_return(false)
+    it "returns a 400 if fails to delete file" do
+      allow_any_instance_of(Repository).to receive(:delete_file).and_raise(Repository::CommitError, 'Cannot delete file')
 
       delete v3_api("/projects/#{project.id}/repository/files", user), valid_params
 
diff --git a/spec/services/files/update_service_spec.rb b/spec/services/files/update_service_spec.rb
index 26aa5b432d4..16bca66766a 100644
--- a/spec/services/files/update_service_spec.rb
+++ b/spec/services/files/update_service_spec.rb
@@ -7,7 +7,7 @@ describe Files::UpdateService do
   let(:user) { create(:user) }
   let(:file_path) { 'files/ruby/popen.rb' }
   let(:new_contents) { 'New Content' }
-  let(:target_branch) { project.default_branch }
+  let(:branch_name) { project.default_branch }
   let(:last_commit_sha) { nil }
 
   let(:commit_params) do
@@ -19,7 +19,7 @@ describe Files::UpdateService do
       last_commit_sha: last_commit_sha,
       start_project: project,
       start_branch: project.default_branch,
-      target_branch: target_branch
+      branch_name: branch_name
     }
   end
 
@@ -73,7 +73,7 @@ describe Files::UpdateService do
     end
 
     context 'when target branch is different than source branch' do
-      let(:target_branch) { "#{project.default_branch}-new" }
+      let(:branch_name) { "#{project.default_branch}-new" }
 
       it 'fires hooks only once' do
         expect(GitHooksService).to receive(:new).once.and_call_original