Refactor changing files in web UI

4 files changed, 81 insertions, 49 deletions

diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb
index 1297a792259..a48d6a976f0 100644
--- a/app/services/commits/change_service.rb
+++ b/app/services/commits/change_service.rb
@@ -1,69 +1,27 @@
 module Commits
-  class ChangeService < ::BaseService
-    ValidationError = Class.new(StandardError)
-    ChangeError = Class.new(StandardError)
+  class ChangeService < Commits::CreateService
+    def initialize(*args)
+      super
 
-    def execute
-      @start_project = params[:start_project] || @project
-      @start_branch = params[:start_branch]
-      @target_branch = params[:target_branch]
       @commit = params[:commit]
-
-      check_push_permissions
-
-      commit
-    rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError,
-           ValidationError, ChangeError => ex
-      error(ex.message)
     end
 
     private
 
-    def commit
-      raise NotImplementedError
-    end
-
     def commit_change(action)
       raise NotImplementedError unless repository.respond_to?(action)
 
-      validate_target_branch if different_branch?
-
       repository.public_send(
         action,
         current_user,
         @commit,
-        @target_branch,
+        @branch_name,
         start_project: @start_project,
         start_branch_name: @start_branch)
-
-      success
     rescue Repository::CreateTreeError
       error_msg = "Sorry, we cannot #{action.to_s.dasherize} this #{@commit.change_type_title(current_user)} automatically.
-                     A #{action.to_s.dasherize} may have already been performed with this #{@commit.change_type_title(current_user)}, or a more recent commit may have updated some of its content."
+                   This #{@commit.change_type_title(current_user)} may already have been #{action.to_s.dasherize}ed, or a more recent commit may have updated some of its content."
       raise ChangeError, error_msg
     end
-
-    def check_push_permissions
-      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@target_branch)
-
-      unless allowed
-        raise ValidationError.new('You are not allowed to push into this branch')
-      end
-
-      true
-    end
-
-    def validate_target_branch
-      result = ValidateNewBranchService.new(@project, current_user)
-        .execute(@target_branch)
-
-      if result[:status] == :error
-        raise ChangeError, "There was an error creating the source branch: #{result[:message]}"
-      end
-    end
-
-    def different_branch?
-      @start_branch != @target_branch || @start_project != @project
-    end
   end
 end
diff --git a/app/services/commits/cherry_pick_service.rb b/app/services/commits/cherry_pick_service.rb
index 605cca36f9c..320e229560d 100644
--- a/app/services/commits/cherry_pick_service.rb
+++ b/app/services/commits/cherry_pick_service.rb
@@ -1,6 +1,6 @@
 module Commits
   class CherryPickService < ChangeService
-    def commit
+    def create_commit!
       commit_change(:cherry_pick)
     end
   end
diff --git a/app/services/commits/create_service.rb b/app/services/commits/create_service.rb
new file mode 100644
index 00000000000..c58f04a252b
--- /dev/null
+++ b/app/services/commits/create_service.rb
@@ -0,0 +1,74 @@
+module Commits
+  class CreateService < ::BaseService
+    ValidationError = Class.new(StandardError)
+    ChangeError = Class.new(StandardError)
+
+    def initialize(*args)
+      super
+
+      @start_project = params[:start_project] || @project
+      @start_branch = params[:start_branch]
+      @branch_name = params[:branch_name]
+    end
+
+    def execute
+      validate!
+
+      new_commit = create_commit!
+
+      success(result: new_commit)
+    rescue ValidationError, ChangeError, Gitlab::Git::Index::IndexError, Repository::CommitError, GitHooksService::PreReceiveError => ex
+      error(ex.message)
+    end
+
+    private
+
+    def create_commit!
+      raise NotImplementedError
+    end
+
+    def raise_error(message)
+      raise ValidationError, message
+    end
+
+    def different_branch?
+      @start_branch != @branch_name || @start_project != @project
+    end
+
+    def validate!
+      validate_permissions!
+      validate_on_branch!
+      validate_branch_existance!
+
+      validate_new_branch_name! if different_branch?
+    end
+
+    def validate_permissions!
+      allowed = ::Gitlab::UserAccess.new(current_user, project: project).can_push_to_branch?(@branch_name)
+
+      unless allowed
+        raise_error("You are not allowed to push into this branch")
+      end
+    end
+
+    def validate_on_branch!
+      if !@start_project.empty_repo? && !@start_project.repository.branch_exists?(@start_branch)
+        raise_error('You can only create or edit files when you are on a branch')
+      end
+    end
+
+    def validate_branch_existance!
+      if !project.empty_repo? && different_branch? && repository.branch_exists?(@branch_name)
+        raise_error("A branch called '#{@branch_name}' already exists. Switch to that branch in order to make changes")
+      end
+    end
+
+    def validate_new_branch_name!
+      result = ValidateNewBranchService.new(project, current_user).execute(@branch_name)
+
+      if result[:status] == :error
+        raise_error("Something went wrong when we tried to create '#{@branch_name}' for you: #{result[:message]}")
+      end
+    end
+  end
+end
diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb
index addd55cb32f..dc27399e047 100644
--- a/app/services/commits/revert_service.rb
+++ b/app/services/commits/revert_service.rb
@@ -1,6 +1,6 @@
 module Commits
   class RevertService < ChangeService
-    def commit
+    def create_commit!
       commit_change(:revert)
     end
   end