From d9c82d679fd622aead99aeb90369361a05e02a36 Mon Sep 17 00:00:00 2001 From: Douwe Maan Date: Fri, 18 Dec 2015 10:03:34 +0100 Subject: Automatically fork a project when not allowed to edit a file. --- .../javascripts/blob/blob_file_dropzone.js.coffee | 2 +- app/assets/javascripts/new_commit_form.js.coffee | 2 +- app/controllers/concerns/creates_commit.rb | 103 +++++++++++++++++++++ .../concerns/creates_merge_request_for_commit.rb | 28 ------ app/controllers/projects/blob_controller.rb | 94 ++++--------------- app/controllers/projects/forks_controller.rb | 19 +++- app/controllers/projects/imports_controller.rb | 24 +++-- app/controllers/projects/tree_controller.rb | 38 ++------ app/helpers/blob_helper.rb | 102 +++++++++++++++----- app/helpers/tree_helper.rb | 41 ++++++-- app/models/repository.rb | 59 ++++++------ app/services/create_branch_service.rb | 17 +++- app/services/files/base_service.rb | 26 +++--- app/services/files/create_service.rb | 2 +- app/views/projects/_commit_button.html.haml | 4 + app/views/projects/blob/_actions.html.haml | 13 +-- app/views/projects/blob/_new_dir.html.haml | 4 + app/views/projects/blob/_upload.html.haml | 5 + app/views/projects/blob/edit.html.haml | 2 +- app/views/projects/blob/show.html.haml | 2 +- app/views/projects/buttons/_dropdown.html.haml | 20 +++- app/views/projects/diffs/_file.html.haml | 3 +- app/views/projects/forks/new.html.haml | 1 - app/views/projects/tree/_tree_content.html.haml | 2 +- app/views/projects/tree/_tree_header.html.haml | 76 ++++++++++----- app/views/shared/_new_commit_form.html.haml | 28 +++--- features/steps/project/source/browse_files.rb | 2 +- lib/api/files.rb | 2 +- spec/controllers/projects/tree_controller_spec.rb | 4 +- 29 files changed, 452 insertions(+), 273 deletions(-) create mode 100644 app/controllers/concerns/creates_commit.rb delete mode 100644 app/controllers/concerns/creates_merge_request_for_commit.rb diff --git a/app/assets/javascripts/blob/blob_file_dropzone.js.coffee b/app/assets/javascripts/blob/blob_file_dropzone.js.coffee index 195f8b11e5d..9df932817f6 100644 --- a/app/assets/javascripts/blob/blob_file_dropzone.js.coffee +++ b/app/assets/javascripts/blob/blob_file_dropzone.js.coffee @@ -35,7 +35,7 @@ class @BlobFileDropzone return this.on 'sending', (file, xhr, formData) -> - formData.append('new_branch', form.find('.js-new-branch').val()) + formData.append('target_branch', form.find('.js-target-branch').val()) formData.append('create_merge_request', form.find('.js-create-merge-request').val()) formData.append('commit_message', form.find('.js-commit-message').val()) return diff --git a/app/assets/javascripts/new_commit_form.js.coffee b/app/assets/javascripts/new_commit_form.js.coffee index 3c7b776155f..03f0f51acfa 100644 --- a/app/assets/javascripts/new_commit_form.js.coffee +++ b/app/assets/javascripts/new_commit_form.js.coffee @@ -1,6 +1,6 @@ class @NewCommitForm constructor: (form) -> - @newBranch = form.find('.js-new-branch') + @newBranch = form.find('.js-target-branch') @originalBranch = form.find('.js-original-branch') @createMergeRequest = form.find('.js-create-merge-request') @createMergeRequestContainer = form.find('.js-create-merge-request-container') diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb new file mode 100644 index 00000000000..62127a09081 --- /dev/null +++ b/app/controllers/concerns/creates_commit.rb @@ -0,0 +1,103 @@ +module CreatesCommit + extend ActiveSupport::Concern + + def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil) + set_commit_variables + + commit_params = @commit_params.merge( + source_project: @project, + source_branch: @ref, + target_branch: @target_branch + ) + + result = service.new(@tree_edit_project, current_user, commit_params).execute + + if result[:status] == :success + flash[:notice] = success_notice || "Your changes have been successfully committed." + + if create_merge_request? + success_path = new_merge_request_path + target = different_project? ? "project" : "branch" + flash[:notice] << " You can now submit a merge request to get this change into the original #{target}." + end + + respond_to do |format| + format.html { redirect_to success_path } + format.json { render json: { message: "success", filePath: success_path } } + end + else + flash[:alert] = result[:message] + respond_to do |format| + format.html do + if failure_view + render failure_view + else + redirect_to failure_path + end + end + format.json { render json: { message: "failed", filePath: failure_path } } + end + end + end + + def authorize_edit_tree! + return if can?(current_user, :push_code, project) + return if current_user && current_user.already_forked?(project) + + access_denied! + end + + private + + def new_merge_request_path + new_namespace_project_merge_request_path( + @mr_source_project.namespace, + @mr_source_project, + merge_request: { + source_project_id: @mr_source_project.id, + target_project_id: @mr_target_project.id, + source_branch: @mr_source_branch, + target_branch: @mr_target_branch + } + ) + end + + def different_project? + @mr_source_project != @mr_target_project + end + + def different_branch? + @mr_source_branch != @mr_target_branch || different_project? + end + + def create_merge_request? + params[:create_merge_request].present? && different_branch? + end + + def set_commit_variables + @mr_source_branch = @target_branch + + if can?(current_user, :push_code, @project) + # Edit file in this project + @tree_edit_project = @project + @mr_source_project = @project + + if @project.forked? + # Merge request from this project to fork origin + @mr_target_project = @project.forked_from_project + @mr_target_branch = @mr_target_project.repository.root_ref + else + # Merge request to this project + @mr_target_project = @project + @mr_target_branch = @ref + end + else + # Edit file in fork + @tree_edit_project = current_user.fork_of(@project) + # Merge request from fork to this project + @mr_source_project = @tree_edit_project + @mr_target_project = @project + @mr_target_branch = @mr_target_project.repository.root_ref + end + end +end diff --git a/app/controllers/concerns/creates_merge_request_for_commit.rb b/app/controllers/concerns/creates_merge_request_for_commit.rb deleted file mode 100644 index c7527822158..00000000000 --- a/app/controllers/concerns/creates_merge_request_for_commit.rb +++ /dev/null @@ -1,28 +0,0 @@ -module CreatesMergeRequestForCommit - extend ActiveSupport::Concern - - def new_merge_request_path - if @project.forked? - target_project = @project.forked_from_project || @project - target_branch = target_project.repository.root_ref - else - target_project = @project - target_branch = @ref - end - - new_namespace_project_merge_request_path( - @project.namespace, - @project, - merge_request: { - source_project_id: @project.id, - target_project_id: target_project.id, - source_branch: @new_branch, - target_branch: target_branch - } - ) - end - - def create_merge_request? - params[:create_merge_request] && @new_branch != @ref - end -end diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 62163682936..c56a3497bb2 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -1,7 +1,7 @@ # Controller for viewing a file's blame class Projects::BlobController < Projects::ApplicationController include ExtractsPath - include CreatesMergeRequestForCommit + include CreatesCommit include ActionView::Helpers::SanitizeHelper # Raised when given an invalid file path @@ -9,21 +9,21 @@ class Projects::BlobController < Projects::ApplicationController before_action :require_non_empty_project, except: [:new, :create] before_action :authorize_download_code! - before_action :authorize_push_code!, only: [:destroy, :create] + before_action :authorize_edit_tree!, only: [:new, :create, :edit, :update, :destroy] before_action :assign_blob_vars 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] def new commit unless @repository.empty? end def create - create_commit(Files::CreateService, success_path: after_create_path, + create_commit(Files::CreateService, success_notice: "The file has been successfully created.", + success_path: namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @file_path)), failure_view: :new, failure_path: namespace_project_new_blob_path(@project.namespace, @project, @ref)) end @@ -36,6 +36,14 @@ class Projects::BlobController < Projects::ApplicationController end def update + after_edit_path = + if from_merge_request && @target_branch == @ref + diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) + + "#file-path-#{hexdigest(@path)}" + else + namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @path)) + end + create_commit(Files::UpdateService, success_path: after_edit_path, failure_view: :edit, failure_path: namespace_project_blob_path(@project.namespace, @project, @id)) @@ -50,15 +58,10 @@ class Projects::BlobController < Projects::ApplicationController end def destroy - result = Files::DeleteService.new(@project, current_user, @commit_params).execute - - if result[:status] == :success - flash[:notice] = "Your changes have been successfully committed" - redirect_to after_destroy_path - else - flash[:alert] = result[:message] - render :show - end + create_commit(Files::DeleteService, success_notice: "The file has been successfully deleted.", + success_path: namespace_project_tree_path(@project.namespace, @project, @target_branch), + failure_view: :show, + failure_path: namespace_project_blob_path(@project.namespace, @project, @id)) end def diff @@ -108,74 +111,13 @@ class Projects::BlobController < Projects::ApplicationController render_404 end - def create_commit(service, success_path:, failure_view:, failure_path:) - result = service.new(@project, current_user, @commit_params).execute - - if result[:status] == :success - flash[:notice] = "Your changes have been successfully committed" - respond_to do |format| - format.html { redirect_to success_path } - format.json { render json: { message: "success", filePath: success_path } } - end - else - flash[:alert] = result[:message] - respond_to do |format| - format.html { render failure_view } - format.json { render json: { message: "failed", filePath: failure_path } } - end - end - end - - def after_create_path - @after_create_path ||= - if create_merge_request? - new_merge_request_path - else - namespace_project_blob_path(@project.namespace, @project, File.join(@new_branch, @file_path)) - end - end - - def after_edit_path - @after_edit_path ||= - if create_merge_request? - new_merge_request_path - elsif from_merge_request && @new_branch == @ref - diffs_namespace_project_merge_request_path(from_merge_request.target_project.namespace, from_merge_request.target_project, from_merge_request) + - "#file-path-#{hexdigest(@path)}" - else - namespace_project_blob_path(@project.namespace, @project, File.join(@new_branch, @path)) - end - end - - def after_destroy_path - @after_destroy_path ||= - if create_merge_request? - new_merge_request_path - else - namespace_project_tree_path(@project.namespace, @project, @new_branch) - end - end - def from_merge_request # If blob edit was initiated from merge request page @from_merge_request ||= MergeRequest.find_by(id: params[:from_merge_request_id]) end - def sanitized_new_branch_name - sanitize(strip_tags(params[:new_branch])) - end - def editor_variables - @current_branch = @ref - - @new_branch = - if params[:new_branch].present? - sanitized_new_branch_name - elsif ::Gitlab::GitAccess.new(current_user, @project).can_push_to_branch?(@ref) - @ref - else - @repository.next_patch_branch - end + @target_branch = params[:target_branch] @file_path = if action_name.to_s == 'create' @@ -194,8 +136,6 @@ class Projects::BlobController < Projects::ApplicationController @commit_params = { file_path: @file_path, - current_branch: @current_branch, - target_branch: @new_branch, commit_message: params[:commit_message], file_content: params[:content], file_content_encoding: params[:encoding] diff --git a/app/controllers/projects/forks_controller.rb b/app/controllers/projects/forks_controller.rb index 8a785076bb7..51181b8042e 100644 --- a/app/controllers/projects/forks_controller.rb +++ b/app/controllers/projects/forks_controller.rb @@ -13,16 +13,25 @@ class Projects::ForksController < Projects::ApplicationController @forked_project = ::Projects::ForkService.new(project, current_user, namespace: namespace).execute if @forked_project.saved? && @forked_project.forked? + continue_params[:notice] ||= "The project was successfully forked." + if @forked_project.import_in_progress? - redirect_to namespace_project_import_path(@forked_project.namespace, @forked_project) + redirect_to namespace_project_import_path(@forked_project.namespace, @forked_project, continue: continue_params) else - redirect_to( - namespace_project_path(@forked_project.namespace, @forked_project), - notice: 'Project was successfully forked.' - ) + if continue_params + redirect_to continue_params[:to], notice: continue_params[:notice] + else + redirect_to namespace_project_path(@forked_project.namespace, @forked_project) + end end else render :error end end + + private + + def continue_params + params[:continue].permit(:to, :notice, :notice_now) + end end diff --git a/app/controllers/projects/imports_controller.rb b/app/controllers/projects/imports_controller.rb index fb8788f0818..e9c9edd3a3c 100644 --- a/app/controllers/projects/imports_controller.rb +++ b/app/controllers/projects/imports_controller.rb @@ -1,7 +1,7 @@ class Projects::ImportsController < Projects::ApplicationController # Authorize before_action :authorize_admin_project! - before_action :require_no_repo + before_action :require_no_repo, except: :show before_action :redirect_if_progress, except: :show def new @@ -24,21 +24,31 @@ class Projects::ImportsController < Projects::ApplicationController end def show - unless @project.import_in_progress? - if @project.import_finished? - redirect_to(project_path(@project)) and return + if @project.repository_exists? || @project.import_finished? + if continue_params + redirect_to continue_params[:to], notice: continue_params[:notice] else - redirect_to(new_namespace_project_import_path(@project.namespace, - @project)) and return + redirect_to project_path(@project) end + elsif @project.import_failed? + redirect_to new_namespace_project_import_path(@project.namespace, @project) + else + if continue_params && continue_params[:notice_now] + flash.now[:notice] = continue_params[:notice_now] + end + # Render end end private + def continue_params + @continue_params ||= params[:continue].permit(:to, :notice, :notice_now) + end + def require_no_repo if @project.repository_exists? && !@project.import_in_progress? - redirect_to(namespace_project_path(@project.namespace, @project)) and return + redirect_to(namespace_project_path(@project.namespace, @project)) end end diff --git a/app/controllers/projects/tree_controller.rb b/app/controllers/projects/tree_controller.rb index 8f272ad1281..4f78bde2d2d 100644 --- a/app/controllers/projects/tree_controller.rb +++ b/app/controllers/projects/tree_controller.rb @@ -1,14 +1,14 @@ # Controller for viewing a repository's file structure class Projects::TreeController < Projects::ApplicationController include ExtractsPath - include CreatesMergeRequestForCommit + include CreatesCommit include ActionView::Helpers::SanitizeHelper before_action :require_non_empty_project, except: [:new, :create] before_action :assign_ref_vars before_action :assign_dir_vars, only: [:create_dir] before_action :authorize_download_code! - before_action :authorize_push_code!, only: [:create_dir] + before_action :authorize_edit_tree!, only: [:create_dir] def show return render_404 unless @repository.commit(@ref) @@ -34,44 +34,20 @@ class Projects::TreeController < Projects::ApplicationController def create_dir return render_404 unless @commit_params.values.all? - begin - result = Files::CreateDirService.new(@project, current_user, @commit_params).execute - message = result[:message] - rescue => e - message = e.to_s - end - - if result && result[:status] == :success - flash[:notice] = "The directory has been successfully created" - respond_to do |format| - format.html { redirect_to after_create_dir_path } - end - else - flash[:alert] = message - respond_to do |format| - format.html { redirect_to namespace_project_blob_path(@project.namespace, @project, @new_branch) } - end - end + create_commit(Files::CreateDirService, success_notice: "The directory has been successfully created.", + success_path: namespace_project_blob_path(@project.namespace, @project, File.join(@target_branch, @dir_name)), + failure_path: namespace_project_tree_path(@project.namespace, @project, @ref)) end private def assign_dir_vars - @new_branch = params[:new_branch].present? ? sanitize(strip_tags(params[:new_branch])) : @ref + @target_branch = params[:target_branch] + @dir_name = File.join(@path, params[:dir_name]) @commit_params = { file_path: @dir_name, - current_branch: @ref, - target_branch: @new_branch, commit_message: params[:commit_message], } end - - def after_create_dir_path - if create_merge_request? - new_merge_request_path - else - namespace_project_blob_path(@project.namespace, @project, File.join(@new_branch, @dir_name)) - end - end end diff --git a/app/helpers/blob_helper.rb b/app/helpers/blob_helper.rb index 68e5d5be600..81b1f34cdf8 100644 --- a/app/helpers/blob_helper.rb +++ b/app/helpers/blob_helper.rb @@ -22,32 +22,92 @@ module BlobHelper %w(credits changelog news copying copyright license authors) end - def edit_blob_link(project, ref, path, options = {}) - blob = - begin - project.repository.blob_at(ref, path) - rescue - nil - end - - return unless blob && blob.text? && blob_editable?(blob) - - text = 'Edit' - after = options[:after] || '' + def edit_blob_link(project = @project, ref = @ref, path = @path, options = {}) + return unless current_user + + blob = project.repository.blob_at(ref, path) rescue nil + + return unless blob && blob_text_editable?(blob) + from_mr = options[:from_merge_request_id] link_opts = {} link_opts[:from_merge_request_id] = from_mr if from_mr - cls = 'btn btn-small' - link_to(text, - namespace_project_edit_blob_path(project.namespace, project, - tree_join(ref, path), - link_opts), - class: cls - ) + after.html_safe + + edit_path = namespace_project_edit_blob_path(project.namespace, project, + tree_join(ref, path), + link_opts) + + if !on_top_of_branch? + button_tag "Edit", class: "btn btn-default disabled has_tooltip", title: "You can only edit files when you are on a branch", data: {container: 'body'} + elsif can_edit_blob?(blob) + link_to "Edit", edit_path, class: 'btn btn-small' + elsif can?(current_user, :fork_project, project) + continue_params = { + to: edit_path, + notice: edit_in_new_fork_notice, + notice_now: edit_in_new_fork_notice_now + } + fork_path = namespace_project_fork_path(project.namespace, project, namespace_key: current_user.namespace.id, + continue: continue_params) + + link_to "Edit", fork_path, class: 'btn btn-small', method: :post + end + end + + def modify_file_link(project = @project, ref = @ref, path = @path, label:, action:, btn_class:, modal_type:) + return unless current_user + + blob = project.repository.blob_at(ref, path) rescue nil + + return unless blob + + if !on_top_of_branch? + button_tag label, class: "btn btn-#{btn_class} disabled has_tooltip", title: "You can only #{action} files when you are on a branch", data: {container: 'body'} + elsif can_edit_blob?(blob) + button_tag label, class: "btn btn-#{btn_class}", 'data-target' => "#modal-#{modal_type}-blob", 'data-toggle' => 'modal' + elsif can?(current_user, :fork_project, project) + continue_params = { + to: request.fullpath, + notice: edit_in_new_fork_notice + " Try to #{action} this file again.", + notice_now: edit_in_new_fork_notice_now + } + fork_path = namespace_project_fork_path(project.namespace, project, namespace_key: current_user.namespace.id, + continue: continue_params) + + link_to label, fork_path, class: "btn btn-#{btn_class}", method: :post + end + end + + def replace_blob_link(project = @project, ref = @ref, path = @path) + modify_file_link( + project, + ref, + path, + label: "Replace", + action: "replace", + btn_class: "default", + modal_type: "upload" + ) + end + + def delete_blob_link(project = @project, ref = @ref, path = @path) + modify_file_link( + project, + ref, + path, + label: "Delete", + action: "delete", + btn_class: "remove", + modal_type: "remove" + ) + end + + def blob_text_editable?(blob) + blob.text? && !blob.lfs_pointer? end - def blob_editable?(blob, project = @project, ref = @ref) - !blob.lfs_pointer? && allowed_tree_edit?(project, ref) + def can_edit_blob?(blob, project = @project, ref = @ref) + !blob.lfs_pointer? && can_edit_tree?(project, ref) end def leave_edit_message diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index f448dd0ab61..2ad7c80dae0 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -50,24 +50,49 @@ module TreeHelper project.repository.branch_names.include?(ref) end - def allowed_tree_edit?(project = nil, ref = nil) + def can_edit_tree?(project = nil, ref = nil) project ||= @project ref ||= @ref + return false unless on_top_of_branch?(project, ref) - can?(current_user, :push_code, project) + can?(current_user, :push_code, project) || + (current_user && current_user.already_forked?(project)) end def tree_edit_branch(project = @project, ref = @ref) - if allowed_tree_edit?(project, ref) - if can_push_branch?(project, ref) - ref - else - project.repository.next_patch_branch - end + return unless can_edit_tree?(project, ref) + + if can_push_branch?(project, ref) + ref + else + project = tree_edit_project(project) + project.repository.next_patch_branch + end + end + + def tree_edit_project(project = @project) + if can?(current_user, :push_code, project) + project + elsif current_user && current_user.already_forked?(project) + current_user.fork_of(project) end end + def edit_in_new_fork_notice_now + "You're not allowed to make changes to this project directly." + + " A fork of this project is being created that you can make changes in, so you can submit a merge request." + end + + def edit_in_new_fork_notice + "You're not allowed to make changes to this project directly." + + " A fork of this project has been created that you can make changes in, so you can submit a merge request." + end + + def commit_in_fork_help + "A new branch will be created in your fork and a new merge request will be started." + end + def tree_breadcrumbs(tree, max_links = 2) if @path.present? part_path = "" diff --git a/app/models/repository.rb b/app/models/repository.rb index 2c25f4ce451..9f688e3b45b 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -592,47 +592,54 @@ class Repository Gitlab::Popen.popen(args, path_to_repo) end - def commit_with_hooks(current_user, branch) - oldrev = Gitlab::Git::BLANK_SHA - ref = Gitlab::Git::BRANCH_REF_PREFIX + branch - was_empty = empty? - - # Create temporary ref + def with_tmp_ref(oldrev = nil) random_string = SecureRandom.hex tmp_ref = "refs/tmp/#{random_string}/head" - unless was_empty - oldrev = find_branch(branch).target + if oldrev && !Gitlab::Git.blank_ref?(oldrev) rugged.references.create(tmp_ref, oldrev) end # Make commit in tmp ref - newrev = yield(tmp_ref) + yield(tmp_ref) + ensure + rugged.references.delete(tmp_ref) rescue nil + end + + def commit_with_hooks(current_user, branch) + oldrev = Gitlab::Git::BLANK_SHA + ref = Gitlab::Git::BRANCH_REF_PREFIX + branch + was_empty = empty? - unless newrev - raise CommitError.new('Failed to create commit') + unless was_empty + oldrev = find_branch(branch).target end - GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do - if was_empty - # Create branch - rugged.references.create(ref, newrev) - else - # Update head - current_head = find_branch(branch).target + with_tmp_ref(oldrev) do |tmp_ref| + # Make commit in tmp ref + newrev = yield(tmp_ref) + + unless newrev + raise CommitError.new('Failed to create commit') + end - # Make sure target branch was not changed during pre-receive hook - if current_head == oldrev - rugged.references.update(ref, newrev) + GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do + if was_empty + # Create branch + rugged.references.create(ref, newrev) else - raise CommitError.new('Commit was rejected because branch received new push') + # Update head + current_head = find_branch(branch).target + + # Make sure target branch was not changed during pre-receive hook + if current_head == oldrev + rugged.references.update(ref, newrev) + else + raise CommitError.new('Commit was rejected because branch received new push') + end end end end - rescue GitHooksService::PreReceiveError - # Remove tmp ref and return error to user - rugged.references.delete(tmp_ref) - raise end private diff --git a/app/services/create_branch_service.rb b/app/services/create_branch_service.rb index de18f3bc556..84e141f5fd8 100644 --- a/app/services/create_branch_service.rb +++ b/app/services/create_branch_service.rb @@ -1,7 +1,7 @@ require_relative 'base_service' class CreateBranchService < BaseService - def execute(branch_name, ref) + def execute(branch_name, ref, source_project: @project) valid_branch = Gitlab::GitRefValidator.validate(branch_name) if valid_branch == false return error('Branch name invalid') @@ -13,7 +13,20 @@ class CreateBranchService < BaseService return error('Branch already exists') end - new_branch = repository.add_branch(current_user, branch_name, ref) + new_branch = nil + if source_project != @project + repository.with_tmp_ref do |tmp_ref| + repository.fetch_ref( + source_project.repository.path_to_repo, + "refs/heads/#{ref}", + tmp_ref + ) + + new_branch = repository.add_branch(current_user, branch_name, tmp_ref) + end + else + new_branch = repository.add_branch(current_user, branch_name, ref) + end if new_branch push_data = build_push_data(project, current_user, new_branch) diff --git a/app/services/files/base_service.rb b/app/services/files/base_service.rb index 9a67b160940..0326a8823e9 100644 --- a/app/services/files/base_service.rb +++ b/app/services/files/base_service.rb @@ -3,8 +3,10 @@ module Files class ValidationError < StandardError; end def execute - @current_branch = params[:current_branch] + @source_project = params[:source_project] || @project + @source_branch = params[:source_branch] @target_branch = params[:target_branch] + @commit_message = params[:commit_message] @file_path = params[:file_path] @file_content = if params[:file_content_encoding] == 'base64' @@ -16,8 +18,8 @@ module Files # Validate parameters validate - # Create new branch if it different from current_branch - if @target_branch != @current_branch + # Create new branch if it different from source_branch + if different_branch? create_target_branch end @@ -26,18 +28,14 @@ module Files else error("Something went wrong. Your changes were not committed") end - rescue Repository::CommitError, GitHooksService::PreReceiveError, ValidationError => ex + rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, ValidationError => ex error(ex.message) end private - def current_branch - @current_branch ||= params[:current_branch] - end - - def target_branch - @target_branch ||= params[:target_branch] + def different_branch? + @source_branch != @target_branch || @source_project != @project end def raise_error(message) @@ -52,11 +50,11 @@ module Files end unless project.empty_repo? - unless repository.branch_names.include?(@current_branch) + unless @source_project.repository.branch_names.include?(@source_branch) raise_error("You can only create or edit files when you are on a branch") end - if @current_branch != @target_branch + if different_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 @@ -65,10 +63,10 @@ module Files end def create_target_branch - result = CreateBranchService.new(project, current_user).execute(@target_branch, @current_branch) + result = CreateBranchService.new(project, current_user).execute(@target_branch, @source_branch, source_project: @source_project) unless result[:status] == :success - raise_error("Something went wrong when we tried to create #{@target_branch} for you") + raise_error("Something went wrong when we tried to create #{@target_branch} for you: #{result[:message]}") end end end diff --git a/app/services/files/create_service.rb b/app/services/files/create_service.rb index 2348920cc58..e4cde4a2fd8 100644 --- a/app/services/files/create_service.rb +++ b/app/services/files/create_service.rb @@ -26,7 +26,7 @@ module Files unless project.empty_repo? @file_path.slice!(0) if @file_path.start_with?('/') - blob = repository.blob_at_branch(@current_branch, @file_path) + blob = repository.blob_at_branch(@source_branch, @file_path) if blob raise_error("Your changes could not be committed because a file with the same name already exists") diff --git a/app/views/projects/_commit_button.html.haml b/app/views/projects/_commit_button.html.haml index 2fd3d9e1be4..640612ca433 100644 --- a/app/views/projects/_commit_button.html.haml +++ b/app/views/projects/_commit_button.html.haml @@ -2,3 +2,7 @@ = button_tag 'Commit Changes', class: 'btn commit-btn js-commit-button btn-create' = link_to 'Cancel', cancel_path, class: 'btn btn-cancel', data: {confirm: leave_edit_message} + + - unless can?(current_user, :push_code, @project) + .inline.prepend-left-10 + = commit_in_fork_help diff --git a/app/views/projects/blob/_actions.html.haml b/app/views/projects/blob/_actions.html.haml index b1df8d19938..caefd911a2a 100644 --- a/app/views/projects/blob/_actions.html.haml +++ b/app/views/projects/blob/_actions.html.haml @@ -14,13 +14,8 @@ = link_to 'Permalink', namespace_project_blob_path(@project.namespace, @project, tree_join(@commit.sha, @path)), class: 'btn btn-sm' -- if blob_editable?(@blob) +- if current_user .btn-group{ role: "group" } - = edit_blob_link(@project, @ref, @path) - %button.btn.btn-default{ 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal' } Replace - %button.btn.btn-remove{ 'data-target' => '#modal-remove-blob', 'data-toggle' => 'modal' } Delete -- elsif !on_top_of_branch? - .btn-group{ role: "group" } - %button.btn.btn-default.disabled.has_tooltip{title: "You can only edit files when you are on a branch.", data: {container: 'body'}} Edit - %button.btn.btn-default.disabled.has_tooltip{title: "You can only replace files when you are on a branch.", data: {container: 'body'}} Replace - %button.btn.btn-remove.disabled.has_tooltip{title: "You can only delete files when you are on a branch.", data: {container: 'body'}} Delete + = edit_blob_link + = replace_blob_link + = delete_blob_link diff --git a/app/views/projects/blob/_new_dir.html.haml b/app/views/projects/blob/_new_dir.html.haml index fc6c9f5fd09..084608bbba3 100644 --- a/app/views/projects/blob/_new_dir.html.haml +++ b/app/views/projects/blob/_new_dir.html.haml @@ -17,5 +17,9 @@ = submit_tag "Create directory", class: 'btn btn-create' = link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal" + - unless can?(current_user, :push_code, @project) + .inline.prepend-left-10 + = commit_in_fork_help + :javascript new NewCommitForm($('.js-create-dir-form')) diff --git a/app/views/projects/blob/_upload.html.haml b/app/views/projects/blob/_upload.html.haml index ecc90a30e78..676924dc6ca 100644 --- a/app/views/projects/blob/_upload.html.haml +++ b/app/views/projects/blob/_upload.html.haml @@ -20,6 +20,11 @@ = button_tag button_title, class: 'btn btn-small btn-create btn-upload-file', id: 'submit-all' = link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal" + - unless can?(current_user, :push_code, @project) + .inline.prepend-left-10 + = commit_in_fork_help + + :javascript disableButtonIfEmptyField($('.js-upload-blob-form').find('.js-commit-message'), '.btn-upload-file'); new BlobFileDropzone($('.js-upload-blob-form'), '#{method}'); diff --git a/app/views/projects/blob/edit.html.haml b/app/views/projects/blob/edit.html.haml index a47fe7ede80..09fa148b129 100644 --- a/app/views/projects/blob/edit.html.haml +++ b/app/views/projects/blob/edit.html.haml @@ -20,7 +20,7 @@ = hidden_field_tag 'last_commit', @last_commit = hidden_field_tag 'content', '', id: "file-content" = hidden_field_tag 'from_merge_request_id', params[:from_merge_request_id] - = render 'projects/commit_button', ref: @ref, cancel_path: @after_edit_path + = render 'projects/commit_button', ref: @ref, cancel_path: namespace_project_blob_path(@project.namespace, @project, @id) :javascript blob = new EditBlob(gon.relative_url_root + "#{Gitlab::Application.config.assets.prefix}", "#{@blob.language.try(:ace_mode)}") diff --git a/app/views/projects/blob/show.html.haml b/app/views/projects/blob/show.html.haml index 3f8d11ed8c8..6988039b6c7 100644 --- a/app/views/projects/blob/show.html.haml +++ b/app/views/projects/blob/show.html.haml @@ -6,7 +6,7 @@ %div#tree-holder.tree-holder = render 'blob', blob: @blob -- if blob_editable?(@blob) +- if can_edit_blob?(@blob) = render 'projects/blob/remove' - title = "Replace #{@blob.name}" diff --git a/app/views/projects/buttons/_dropdown.html.haml b/app/views/projects/buttons/_dropdown.html.haml index b277b765b6b..1f639fecc30 100644 --- a/app/views/projects/buttons/_dropdown.html.haml +++ b/app/views/projects/buttons/_dropdown.html.haml @@ -18,10 +18,11 @@ = link_to new_namespace_project_snippet_path(@project.namespace, @project) do = icon('file-text-o fw') New snippet + - if can?(current_user, :push_code, @project) %li.divider %li - = link_to namespace_project_new_blob_path(@project.namespace, @project, @project.default_branch || 'master'), title: 'New file' do + = link_to namespace_project_new_blob_path(@project.namespace, @project, @project.default_branch || 'master') do = icon('file fw') New file %li @@ -32,3 +33,20 @@ = link_to new_namespace_project_tag_path(@project.namespace, @project) do = icon('tags fw') New tag + - elsif current_user && current_user.already_forked?(@project) + %li.divider + %li + = link_to namespace_project_new_blob_path(@project.namespace, @project, @project.default_branch || 'master') do + = icon('file fw') + New file + - elsif can?(current_user, :fork_project, @project) + %li.divider + %li + - continue_params = { to: namespace_project_new_blob_path(@project.namespace, @project, @project.default_branch || 'master'), + notice: edit_in_new_fork_notice, + notice_now: edit_in_new_fork_notice_now } + - fork_path = namespace_project_fork_path(@project.namespace, @project, namespace_key: current_user.namespace.id, + continue: continue_params) + = link_to fork_path, method: :post do + = icon('file fw') + New file diff --git a/app/views/projects/diffs/_file.html.haml b/app/views/projects/diffs/_file.html.haml index 327e7d9245a..9c6d7b46429 100644 --- a/app/views/projects/diffs/_file.html.haml +++ b/app/views/projects/diffs/_file.html.haml @@ -32,7 +32,8 @@ - if editable_diff?(diff_file) = edit_blob_link(@merge_request.source_project, @merge_request.source_branch, diff_file.new_path, - after: ' ', from_merge_request_id: @merge_request.id) + from_merge_request_id: @merge_request.id) +   = view_file_btn(diff_commit.id, diff_file, project) diff --git a/app/views/projects/forks/new.html.haml b/app/views/projects/forks/new.html.haml index f0b0a11c04a..8a2c027a455 100644 --- a/app/views/projects/forks/new.html.haml +++ b/app/views/projects/forks/new.html.haml @@ -43,4 +43,3 @@ %i.fa.fa-spinner.fa-spin Forking repository %p Please wait a moment, this page will automatically refresh when ready. - diff --git a/app/views/projects/tree/_tree_content.html.haml b/app/views/projects/tree/_tree_content.html.haml index 1bc90edd8f0..1927883513a 100644 --- a/app/views/projects/tree/_tree_content.html.haml +++ b/app/views/projects/tree/_tree_content.html.haml @@ -29,7 +29,7 @@ - if tree.readme = render "projects/tree/readme", readme: tree.readme -- if allowed_tree_edit? +- if can_edit_tree? = render 'projects/blob/upload', title: 'Upload New File', placeholder: 'Upload new file', button_title: 'Upload file', form_path: namespace_project_create_blob_path(@project.namespace, @project, @id), method: :post = render 'projects/blob/new_dir' diff --git a/app/views/projects/tree/_tree_header.html.haml b/app/views/projects/tree/_tree_header.html.haml index cefe33e581f..6167006f947 100644 --- a/app/views/projects/tree/_tree_header.html.haml +++ b/app/views/projects/tree/_tree_header.html.haml @@ -11,26 +11,60 @@ = link_to truncate(title, length: 40), namespace_project_tree_path(@project.namespace, @project, path) - else = link_to title, '#' - - if allowed_tree_edit? + + - if current_user %li - %span.dropdown - %a.dropdown-toggle.btn.btn-sm.add-to-tree{href: '#', "data-toggle" => "dropdown"} + - if !on_top_of_branch? + %span.btn.btn-sm.add-to-tree.disabled.has_tooltip{title: "You can only add files when you are on a branch", data: {container: 'body'}} = icon('plus') - %ul.dropdown-menu - %li - = link_to namespace_project_new_blob_path(@project.namespace, @project, @id), title: 'Create file', id: 'new-file-link' do - = icon('pencil fw') - Create file - %li - = link_to '#modal-upload-blob', { 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal'} do - = icon('file fw') - Upload file - %li.divider - %li - = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal'} do - = icon('folder fw') - New directory - - elsif !on_top_of_branch? - %li - %span.btn.btn-sm.add-to-tree.disabled.has_tooltip{title: "You can only add files when you are on a branch.", data: {container: 'body'}} - = icon('plus') + - elsif can_edit_tree? + %span.dropdown + %a.dropdown-toggle.btn.btn-sm.add-to-tree{href: '#', "data-toggle" => "dropdown"} + = icon('plus') + %ul.dropdown-menu + %li + = link_to namespace_project_new_blob_path(@project.namespace, @project, @id) do + = icon('pencil fw') + Create file + %li + = link_to '#modal-upload-blob', { 'data-target' => '#modal-upload-blob', 'data-toggle' => 'modal'} do + = icon('file fw') + Upload file + %li.divider + %li + = link_to '#modal-create-new-dir', { 'data-target' => '#modal-create-new-dir', 'data-toggle' => 'modal'} do + = icon('folder fw') + New directory + - elsif can?(current_user, :fork_project, @project) + %span.dropdown + %a.dropdown-toggle.btn.btn-sm.add-to-tree{href: '#', "data-toggle" => "dropdown"} + = icon('plus') + %ul.dropdown-menu + %li + - continue_params = { to: namespace_project_new_blob_path(@project.namespace, @project, @id), + notice: edit_in_new_fork_notice, + notice_now: edit_in_new_fork_notice_now } + - fork_path = namespace_project_fork_path(@project.namespace, @project, namespace_key: current_user.namespace.id, + continue: continue_params) + = link_to fork_path, method: :post do + = icon('pencil fw') + Create file + %li + - continue_params = { to: request.fullpath, + notice: edit_in_new_fork_notice + " Try to upload a file again.", + notice_now: edit_in_new_fork_notice_now } + - fork_path = namespace_project_fork_path(@project.namespace, @project, namespace_key: current_user.namespace.id, + continue: continue_params) + = link_to fork_path, method: :post do + = icon('file fw') + Upload file + %li.divider + %li + - continue_params = { to: request.fullpath, + notice: edit_in_new_fork_notice + " Try to create a new directory again.", + notice_now: edit_in_new_fork_notice_now } + - fork_path = namespace_project_fork_path(@project.namespace, @project, namespace_key: current_user.namespace.id, + continue: continue_params) + = link_to fork_path, method: :post do + = icon('folder fw') + New directory diff --git a/app/views/shared/_new_commit_form.html.haml b/app/views/shared/_new_commit_form.html.haml index 111219f2064..0c8ac48bb58 100644 --- a/app/views/shared/_new_commit_form.html.haml +++ b/app/views/shared/_new_commit_form.html.haml @@ -1,16 +1,22 @@ = render 'shared/commit_message_container', placeholder: placeholder -- unless @project.empty_repo? - .form-group.branch - = label_tag 'new_branch', 'Target branch', class: 'control-label' - .col-sm-10 - = text_field_tag 'new_branch', @new_branch || tree_edit_branch, required: true, class: "form-control js-new-branch" +- if @project.empty_repo? + = hidden_field_tag 'target_branch', @ref +- else + - if can?(current_user, :push_code, @project) + .form-group.branch + = label_tag 'target_branch', 'Target branch', class: 'control-label' + .col-sm-10 + = text_field_tag 'target_branch', @target_branch || tree_edit_branch, required: true, class: "form-control js-target-branch" - .js-create-merge-request-container - .checkbox - - nonce = SecureRandom.hex - = label_tag "create_merge_request-#{nonce}" do - = check_box_tag 'create_merge_request', 1, true, class: 'js-create-merge-request', id: "create_merge_request-#{nonce}" - Start a new merge request with these changes + .js-create-merge-request-container + .checkbox + - nonce = SecureRandom.hex + = label_tag "create_merge_request-#{nonce}" do + = check_box_tag 'create_merge_request', 1, true, class: 'js-create-merge-request', id: "create_merge_request-#{nonce}" + Start a new merge request with these changes + - else + = hidden_field_tag 'target_branch', @target_branch || tree_edit_branch + = hidden_field_tag 'create_merge_request', 1 = hidden_field_tag 'original_branch', @ref, class: 'js-original-branch' diff --git a/features/steps/project/source/browse_files.rb b/features/steps/project/source/browse_files.rb index 0c6df18ce2e..da8a07c2b4a 100644 --- a/features/steps/project/source/browse_files.rb +++ b/features/steps/project/source/browse_files.rb @@ -75,7 +75,7 @@ class Spinach::Features::ProjectSourceBrowseFiles < Spinach::FeatureSteps end step 'I fill the new branch name' do - fill_in :new_branch, with: 'new_branch_name', visible: true + fill_in :target_branch, with: 'new_branch_name', visible: true end step 'I fill the new file name with an illegal name' do diff --git a/lib/api/files.rb b/lib/api/files.rb index a7a768f8895..8ad2c1883c7 100644 --- a/lib/api/files.rb +++ b/lib/api/files.rb @@ -7,7 +7,7 @@ module API def commit_params(attrs) { file_path: attrs[:file_path], - current_branch: attrs[:branch_name], + source_branch: attrs[:branch_name], target_branch: attrs[:branch_name], commit_message: attrs[:commit_message], file_content: attrs[:content], diff --git a/spec/controllers/projects/tree_controller_spec.rb b/spec/controllers/projects/tree_controller_spec.rb index a474574c6e5..a4366e378c7 100644 --- a/spec/controllers/projects/tree_controller_spec.rb +++ b/spec/controllers/projects/tree_controller_spec.rb @@ -98,7 +98,7 @@ describe Projects::TreeController do project_id: project.to_param, id: 'master', dir_name: path, - new_branch: target_branch, + target_branch: target_branch, commit_message: 'Test commit message') end @@ -109,7 +109,7 @@ describe Projects::TreeController do it 'redirects to the new directory' do expect(subject). to redirect_to("/#{project.path_with_namespace}/blob/#{target_branch}/#{path}") - expect(flash[:notice]).to eq('The directory has been successfully created') + expect(flash[:notice]).to eq('The directory has been successfully created.') end end -- cgit v1.2.1