From d4cfa0bf766bb99cb6b8e600faee1e90678816b4 Mon Sep 17 00:00:00 2001 From: Dmitriy Zaporozhets Date: Tue, 11 Aug 2015 11:49:09 +0200 Subject: [PATCH] Revert "Refactor web editor" This reverts commit dfccb06dda344819989fa8d6a9a3c56c5ca0b65f. Signed-off-by: Dmitriy Zaporozhets --- Gemfile.lock | 4 ++ app/controllers/projects/blob_controller.rb | 60 ++++++++-------- app/services/files/base_service.rb | 77 ++------------------- app/services/files/create_service.rb | 50 ++++++++++--- app/services/files/delete_service.rb | 32 ++++++++- app/services/files/update_service.rb | 42 ++++++++++- app/views/projects/blob/new.html.haml | 11 ++- 7 files changed, 156 insertions(+), 120 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 53d21b26e97..7f8db0606e5 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -783,7 +783,11 @@ DEPENDENCIES gitlab-grack (~> 2.0.2) gitlab-linguist (~> 3.0.1) gitlab_emoji (~> 0.1) +<<<<<<< HEAD gitlab_git (~> 7.2.6) +======= + gitlab_git (~> 7.2.2) +>>>>>>> parent of dfccb06... Refactor web editor gitlab_meta (= 7.0) gitlab_omniauth-ldap (= 1.2.1) gollum-lib (~> 4.0.2) diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 100d3d3b317..b762518d377 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -13,20 +13,27 @@ class Projects::BlobController < Projects::ApplicationController before_action :commit, except: [:new, :create] before_action :blob, except: [:new, :create] before_action :from_merge_request, only: [:edit, :update] - before_action :require_branch_head, only: [:edit, :update] - before_action :editor_variables, except: [:show, :preview, :diff] before_action :after_edit_path, only: [:edit, :update] + before_action :require_branch_head, only: [:edit, :update] def new commit unless @repository.empty? end def create - result = Files::CreateService.new(@project, current_user, @commit_params).execute + file_path = File.join(@path, File.basename(params[:file_name])) + result = Files::CreateService.new( + @project, + current_user, + params.merge(new_branch: sanitized_new_branch_name), + @ref, + file_path + ).execute if result[:status] == :success flash[:notice] = "Your changes have been successfully committed" - redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @file_path)) + ref = sanitized_new_branch_name.presence || @ref + redirect_to namespace_project_blob_path(@project.namespace, @project, File.join(ref, file_path)) else flash[:alert] = result[:message] render :new @@ -41,10 +48,22 @@ class Projects::BlobController < Projects::ApplicationController end def update - result = Files::UpdateService.new(@project, current_user, @commit_params).execute + result = Files::UpdateService. + new( + @project, + current_user, + params.merge(new_branch: sanitized_new_branch_name), + @ref, + @path + ).execute if result[:status] == :success flash[:notice] = "Your changes have been successfully committed" + + if from_merge_request + from_merge_request.reload_code + end + redirect_to after_edit_path else flash[:alert] = result[:message] @@ -61,11 +80,12 @@ class Projects::BlobController < Projects::ApplicationController end def destroy - result = Files::DeleteService.new(@project, current_user, @commit_params).execute + result = Files::DeleteService.new(@project, current_user, params, @ref, @path).execute if result[:status] == :success flash[:notice] = "Your changes have been successfully committed" - redirect_to namespace_project_tree_path(@project.namespace, @project, @target_branch) + redirect_to namespace_project_tree_path(@project.namespace, @project, + @ref) else flash[:alert] = result[:message] render :show @@ -115,6 +135,7 @@ class Projects::BlobController < Projects::ApplicationController @id = params[:id] @ref, @path = extract_ref(@id) + rescue InvalidPathError not_found! end @@ -124,8 +145,8 @@ class Projects::BlobController < Projects::ApplicationController if from_merge_request diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) + "#file-path-#{hexdigest(@path)}" - elsif @target_branch.present? - namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path)) + elsif sanitized_new_branch_name.present? + namespace_project_blob_path(@project.namespace, @project, File.join(sanitized_new_branch_name, @path)) else namespace_project_blob_path(@project.namespace, @project, @id) end @@ -139,25 +160,4 @@ class Projects::BlobController < Projects::ApplicationController def sanitized_new_branch_name @new_branch ||= sanitize(strip_tags(params[:new_branch])) end - - def editor_variables - @current_branch = @ref - @target_branch = (sanitized_new_branch_name || @ref) - - @file_path = - if action_name.to_s == 'create' - File.join(@path, File.basename(params[:file_name])) - else - @path - end - - @commit_params = { - file_path: @file_path, - current_branch: @current_branch, - target_branch: @target_branch, - commit_message: params[:commit_message], - file_content: params[:content], - file_content_encoding: params[:encoding] - } - end end diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 646784f2d9d..4d02752454e 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -1,34 +1,11 @@ module Files class BaseService < ::BaseService - class ValidationError < StandardError; end + attr_reader :ref, :path - def execute - @current_branch = params[:current_branch] - @target_branch = params[:target_branch] - @commit_message = params[:commit_message] - @file_path = params[:file_path] - @file_content = if params[:file_content_encoding] == 'base64' - Base64.decode64(params[:file_content]) - else - params[:file_content] - end - - # Validate parameters - validate - - # Create new branch if it different from current_branch - if @target_branch != @current_branch - create_target_branch - end - - if sha = commit - after_commit(sha, @target_branch) - success - else - error("Something went wrong. Your changes were not committed") - end - rescue ValidationError => ex - error(ex.message) + def initialize(project, user, params, ref, path = nil) + @project, @current_user, @params = project, user, params.dup + @ref = ref + @path = path end private @@ -37,51 +14,11 @@ module Files project.repository end - def after_commit(sha, branch) + def after_commit(sha) commit = repository.commit(sha) - full_ref = "#{Gitlab::Git::BRANCH_REF_PREFIX}#{branch}" + full_ref = 'refs/heads/' + (params[:new_branch] || ref) old_sha = commit.parent_id || Gitlab::Git::BLANK_SHA GitPushService.new.execute(project, current_user, old_sha, sha, full_ref) end - - def current_branch - @current_branch ||= params[:current_branch] - end - - def target_branch - @target_branch ||= params[:target_branch] - end - - def raise_error(message) - raise ValidationError.new(message) - end - - def validate - allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch) - - unless allowed - raise_error("You are not allowed to push into this branch") - end - - unless project.empty_repo? - unless repository.branch_names.include?(@current_branch) - raise_error("You can only create files if you are on top of a branch") - end - - if @current_branch != @target_branch - if repository.branch_names.include?(@target_branch) - raise_error("Branch with such name already exists. You need to switch to this branch in order to make changes") - end - end - end - end - - def create_target_branch - result = CreateBranchService.new(project, current_user).execute(@target_branch, @current_branch) - - unless result[:status] == :success - raise_error("Something went wrong when we tried to create #{@target_branch} for you") - end - end end end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 91d715b2d63..0a80455bc6b 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -2,28 +2,58 @@ require_relative "base_service" module Files class CreateService < Files::BaseService - def commit - repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) - end + def execute + allowed = Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) - def validate - super + unless allowed + return error("You are not allowed to create file in this branch") + end - file_name = File.basename(@file_path) + file_name = File.basename(path) + file_path = path unless file_name =~ Gitlab::Regex.file_name_regex - raise_error( + return error( 'Your changes could not be committed, because the file name ' + Gitlab::Regex.file_name_regex_message ) end - unless project.empty_repo? - blob = repository.blob_at_branch(@current_branch, @file_path) + if project.empty_repo? + # everything is ok because repo does not have a commits yet + else + unless repository.branch_names.include?(ref) + return error("You can only create files if you are on top of a branch") + end + + blob = repository.blob_at_branch(ref, file_path) if blob - raise_error("Your changes could not be committed, because file with such name exists") + return error("Your changes could not be committed, because file with such name exists") + end + end + + content = + if params[:encoding] == 'base64' + Base64.decode64(params[:content]) + else + params[:content] end + + sha = repository.commit_file( + current_user, + file_path, + content, + params[:commit_message], + params[:new_branch] || ref + ) + + + if sha + after_commit(sha) + success + else + error("Your changes could not be committed, because the file has been changed") end end end diff --git a/app/services/files/delete_service.rb b/app/services/files/delete_service.rb index 27c881c3430..2281777604c 100644 --- a/app/services/files/delete_service.rb +++ b/app/services/files/delete_service.rb @@ -2,8 +2,36 @@ require_relative "base_service" module Files class DeleteService < Files::BaseService - def commit - repository.remove_file(current_user, @file_path, @commit_message, @target_branch) + def execute + allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) + + unless allowed + return error("You are not allowed to push into this branch") + end + + unless repository.branch_names.include?(ref) + return error("You can only create files if you are on top of a branch") + end + + blob = repository.blob_at_branch(ref, path) + + unless blob + return error("You can only edit text files") + end + + sha = repository.remove_file( + current_user, + path, + params[:commit_message], + ref + ) + + if sha + after_commit(sha) + success + else + error("Your changes could not be committed, because the file has been changed") + end end end end diff --git a/app/services/files/update_service.rb b/app/services/files/update_service.rb index a20903c6f02..013cc1ee322 100644 --- a/app/services/files/update_service.rb +++ b/app/services/files/update_service.rb @@ -2,8 +2,46 @@ require_relative "base_service" module Files class UpdateService < Files::BaseService - def commit - repository.commit_file(current_user, @file_path, @file_content, @commit_message, @target_branch) + def execute + allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(ref) + + unless allowed + return error("You are not allowed to push into this branch") + end + + unless repository.branch_names.include?(ref) + return error("You can only create files if you are on top of a branch") + end + + blob = repository.blob_at_branch(ref, path) + + unless blob + return error("You can only edit text files") + end + + content = + if params[:encoding] == 'base64' + Base64.decode64(params[:content]) + else + params[:content] + end + + sha = repository.commit_file( + current_user, + path, + content, + params[:commit_message], + params[:new_branch] || ref + ) + + after_commit(sha) + success + rescue Gitlab::Satellite::CheckoutFailed => ex + error("Your changes could not be committed because ref '#{ref}' could not be checked out", 400) + rescue Gitlab::Satellite::CommitFailed => ex + error("Your changes could not be committed. Maybe there was nothing to commit?", 409) + rescue Gitlab::Satellite::PushFailed => ex + error("Your changes could not be committed. Maybe the file was changed by another process?", 409) end end end diff --git a/app/views/projects/blob/new.html.haml b/app/views/projects/blob/new.html.haml index 7c2a4fece94..dac984f8c31 100644 --- a/app/views/projects/blob/new.html.haml +++ b/app/views/projects/blob/new.html.haml @@ -6,12 +6,11 @@ = render 'shared/commit_message_container', params: params, placeholder: 'Add new file' - - unless @project.empty_repo? - .form-group.branch - = label_tag 'branch', class: 'control-label' do - Branch - .col-sm-10 - = text_field_tag 'new_branch', @ref, class: "form-control" + .form-group.branch + = label_tag 'branch', class: 'control-label' do + Branch + .col-sm-10 + = text_field_tag 'new_branch', @ref, class: "form-control" = hidden_field_tag 'content', '', id: 'file-content' = render 'projects/commit_button', ref: @ref, -- GitLab