diff options
Diffstat (limited to 'app')
| -rw-r--r-- | app/assets/stylesheets/pages/merge_requests.scss | 2 | ||||
| -rw-r--r-- | app/controllers/application_controller.rb | 9 | ||||
| -rw-r--r-- | app/controllers/concerns/creates_commit.rb | 53 | ||||
| -rw-r--r-- | app/controllers/projects/commit_controller.rb | 44 | ||||
| -rw-r--r-- | app/helpers/commits_helper.rb | 31 | ||||
| -rw-r--r-- | app/helpers/tree_helper.rb | 3 | ||||
| -rw-r--r-- | app/models/commit.rb | 38 | ||||
| -rw-r--r-- | app/models/merge_request.rb | 9 | ||||
| -rw-r--r-- | app/models/repository.rb | 37 | ||||
| -rw-r--r-- | app/services/commits/revert_service.rb | 58 | ||||
| -rw-r--r-- | app/services/merge_requests/build_service.rb | 2 | ||||
| -rw-r--r-- | app/services/merge_requests/merge_service.rb | 3 | ||||
| -rw-r--r-- | app/views/projects/commit/_commit_box.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/commit/_revert.html.haml | 31 | ||||
| -rw-r--r-- | app/views/projects/commit/show.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/merge_requests/_show.html.haml | 2 | ||||
| -rw-r--r-- | app/views/projects/merge_requests/widget/_merged.html.haml | 14 | ||||
| -rw-r--r-- | app/views/projects/merge_requests/widget/_merged_buttons.haml | 11 |
18 files changed, 321 insertions, 30 deletions
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss index 6b497cd56ed..12fb030760c 100644 --- a/app/assets/stylesheets/pages/merge_requests.scss +++ b/app/assets/stylesheets/pages/merge_requests.scss @@ -237,3 +237,5 @@ } } } + + diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 2c329b60a19..fb74919ea23 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -25,7 +25,7 @@ class ApplicationController < ActionController::Base helper_method :abilities, :can?, :current_application_settings helper_method :import_sources_enabled?, :github_import_enabled?, :github_import_configured?, :gitlab_import_enabled?, :gitlab_import_configured?, :bitbucket_import_enabled?, :bitbucket_import_configured?, :gitorious_import_enabled?, :google_code_import_enabled?, :fogbugz_import_enabled?, :git_import_enabled? - helper_method :repository + helper_method :repository, :can_collaborate_with_project? rescue_from Encoding::CompatibilityError do |exception| log_exception(exception) @@ -410,6 +410,13 @@ class ApplicationController < ActionController::Base current_user.nil? && root_path == request.path end + def can_collaborate_with_project?(project = nil) + project ||= @project + + can?(current_user, :push_code, project) || + (current_user && current_user.already_forked?(project)) + end + private def set_default_sort diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb index b9eb0a22f88..787416c17ab 100644 --- a/app/controllers/concerns/creates_commit.rb +++ b/app/controllers/concerns/creates_commit.rb @@ -13,17 +13,11 @@ module CreatesCommit 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 + update_flash_notice(success_notice) respond_to do |format| - format.html { redirect_to success_path } - format.json { render json: { message: "success", filePath: success_path } } + format.html { redirect_to final_success_path(success_path) } + format.json { render json: { message: "success", filePath: final_success_path(success_path) } } end else flash[:alert] = result[:message] @@ -41,14 +35,32 @@ module CreatesCommit end def authorize_edit_tree! - return if can?(current_user, :push_code, project) - return if current_user && current_user.already_forked?(project) + return if can_collaborate_with_project? access_denied! end private + def update_flash_notice(success_notice) + flash[:notice] = success_notice || "Your changes have been successfully committed." + + if create_merge_request? + if merge_request_exists? + flash[:notice] = nil + else + target = different_project? ? "project" : "branch" + flash[:notice] << " You can now submit a merge request to get this change into the original #{target}." + end + end + end + + def final_success_path(success_path) + return success_path unless create_merge_request? + + merge_request_exists? ? existing_merge_request_path : new_merge_request_path + end + def new_merge_request_path new_namespace_project_merge_request_path( @mr_source_project.namespace, @@ -62,6 +74,19 @@ module CreatesCommit ) end + def existing_merge_request_path + namespace_project_merge_request_path(@mr_target_project.namespace, @mr_target_project, @merge_request) + end + + def merge_request_exists? + return @merge_request if defined?(@merge_request) + + @merge_request = @mr_target_project.merge_requests.opened.find_by( + source_branch: @mr_source_branch, + target_branch: @mr_target_branch + ) + end + def different_project? @mr_source_project != @mr_target_project end @@ -75,7 +100,7 @@ module CreatesCommit end def set_commit_variables - @mr_source_branch = @target_branch + @mr_source_branch ||= @target_branch if can?(current_user, :push_code, @project) # Edit file in this project @@ -89,7 +114,7 @@ module CreatesCommit else # Merge request to this project @mr_target_project = @project - @mr_target_branch = @ref + @mr_target_branch ||= @ref end else # Edit file in fork @@ -97,7 +122,7 @@ module CreatesCommit # Merge request from fork to this project @mr_source_project = @tree_edit_project @mr_target_project = @project - @mr_target_branch = @ref + @mr_target_branch ||= @ref end end end diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 36951b91372..97d31a4229a 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -2,6 +2,8 @@ # # Not to be confused with CommitsController, plural. class Projects::CommitController < Projects::ApplicationController + include CreatesCommit + # Authorize before_action :require_non_empty_project before_action :authorize_download_code!, except: [:cancel_builds, :retry_builds] @@ -9,6 +11,7 @@ class Projects::CommitController < Projects::ApplicationController before_action :authorize_read_commit_status!, only: [:builds] before_action :commit before_action :define_show_vars, only: [:show, :builds] + before_action :authorize_edit_tree!, only: [:revert] def show apply_diff_view_cookie! @@ -55,8 +58,37 @@ class Projects::CommitController < Projects::ApplicationController render layout: false end + def revert + assign_revert_commit_vars + + return render_404 if @target_branch.blank? + + create_commit(Commits::RevertService, success_notice: "The #{revert_type_title} has been successfully reverted.", + success_path: successful_revert_path, failure_path: failed_revert_path) + end + private + def revert_type_title + @commit.merged_merge_request ? 'merge request' : 'commit' + end + + def successful_revert_path + return referenced_merge_request_url if @commit.merged_merge_request + + namespace_project_commits_url(@project.namespace, @project, @target_branch) + end + + def failed_revert_path + return referenced_merge_request_url if @commit.merged_merge_request + + namespace_project_commit_url(@project.namespace, @project, params[:id]) + end + + def referenced_merge_request_url + namespace_project_merge_request_url(@project.namespace, @project, @commit.merged_merge_request) + end + def commit @commit ||= @project.commit(params[:id]) end @@ -79,4 +111,16 @@ class Projects::CommitController < Projects::ApplicationController @statuses = ci_commit.statuses if ci_commit end + + def assign_revert_commit_vars + @commit = project.commit(params[:id]) + @target_branch = params[:target_branch] + @mr_source_branch = @commit.revert_branch_name + @mr_target_branch = @target_branch + @commit_params = { + commit: @commit, + revert_type_title: revert_type_title, + create_merge_request: params[:create_merge_request].present? || different_project? + } + end end diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 1d14ee52cfc..7ff539118d3 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -123,6 +123,37 @@ module CommitsHelper ) end + def revert_commit_link(commit, continue_to_path, btn_class: nil) + return unless current_user + + tooltip = "Revert this #{revert_commit_type(commit)} in a new merge request" + + if can_collaborate_with_project? + content_tag :span, 'data-toggle' => 'modal', 'data-target' => '#modal-revert-commit' do + link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'tooltip', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class}" + end + elsif can?(current_user, :fork_project, @project) + continue_params = { + to: continue_to_path, + notice: edit_in_new_fork_notice + ' Try to revert this commit again.', + notice_now: edit_in_new_fork_notice_now + } + fork_path = namespace_project_forks_path(@project.namespace, @project, + namespace_key: current_user.namespace.id, + continue: continue_params) + + link_to 'Revert', fork_path, class: 'btn btn-grouped btn-close', method: :post, 'data-toggle' => 'tooltip', title: tooltip + end + end + + def revert_commit_type(commit) + if commit.merged_merge_request + 'merge request' + else + 'commit' + end + end + protected # Private: Returns a link to a person. If the person has a matching user and diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index 2ad7c80dae0..4920ca5af6e 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -56,8 +56,7 @@ module TreeHelper return false unless on_top_of_branch?(project, ref) - can?(current_user, :push_code, project) || - (current_user && current_user.already_forked?(project)) + can_collaborate_with_project?(project) end def tree_edit_branch(project = @project, ref = @ref) diff --git a/app/models/commit.rb b/app/models/commit.rb index 23b771aebb7..b99abb540ea 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -215,6 +215,44 @@ class Commit ci_commit.try(:status) || :not_found end + def revert_branch_name + "revert-#{short_id}" + end + + def revert_description + if merged_merge_request + "This reverts merge request #{merged_merge_request.to_reference}" + else + "This reverts commit #{sha}" + end + end + + def revert_message + %Q{Revert "#{title}"\n\n#{revert_description}} + end + + def reverts_commit?(commit) + description.include?(commit.revert_description) + end + + def merge_commit? + parents.size > 1 + end + + def merged_merge_request + return @merged_merge_request if defined?(@merged_merge_request) + + @merged_merge_request = project.merge_requests.find_by(merge_commit_sha: id) if merge_commit? + end + + def has_been_reverted?(current_user = nil, noteable = self) + Gitlab::ReferenceExtractor.lazily do + noteable.notes.system.flat_map do |note| + note.all_references(current_user).commits + end + end.any? { |commit_ref| commit_ref.reverts_commit?(self) } + end + private def repo_changes diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index ea2b0e075f6..1543ef311d7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -24,6 +24,7 @@ # merge_params :text # merge_when_build_succeeds :boolean default(FALSE), not null # merge_user_id :integer +# merge_commit_sha :string # require Rails.root.join("app/models/commit") @@ -532,4 +533,12 @@ class MergeRequest < ActiveRecord::Base [diff_base_commit, last_commit] end + + def merge_commit + @merge_commit ||= project.commit(merge_commit_sha) if merge_commit_sha + end + + def can_be_reverted?(current_user = nil) + merge_commit && !merge_commit.has_been_reverted?(current_user, self) + end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 73aa67d46ec..be30a3b0906 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -619,6 +619,34 @@ class Repository end end + def revert(user, commit, base_branch, target_branch = nil) + source_sha = find_branch(base_branch).target + target_branch ||= base_branch + args = [commit.id, source_sha] + args << { mainline: 1 } if commit.merge_commit? + + revert_index = rugged.revert_commit(*args) + return false if revert_index.conflicts? + + tree_id = revert_index.write_tree(rugged) + return false unless diff_exists?(source_sha, tree_id) + + commit_with_hooks(user, target_branch) do |ref| + committer = user_to_committer(user) + source_sha = Rugged::Commit.create(rugged, + message: commit.revert_message, + author: committer, + committer: committer, + tree: tree_id, + parents: [rugged.lookup(source_sha)], + update_ref: ref) + end + end + + def diff_exists?(sha1, sha2) + rugged.diff(sha1, sha2).size > 0 + end + def merged_to_root_ref?(branch_name) branch_commit = commit(branch_name) root_ref_commit = commit(root_ref) @@ -700,10 +728,11 @@ class Repository oldrev = Gitlab::Git::BLANK_SHA ref = Gitlab::Git::BRANCH_REF_PREFIX + branch + target_branch = find_branch(branch) was_empty = empty? - unless was_empty - oldrev = find_branch(branch).target + if !was_empty && target_branch + oldrev = target_branch.target end with_tmp_ref(oldrev) do |tmp_ref| @@ -715,7 +744,7 @@ class Repository end GitHooksService.new.execute(current_user, path_to_repo, oldrev, newrev, ref) do - if was_empty + if was_empty || !target_branch # Create branch rugged.references.create(ref, newrev) else @@ -730,6 +759,8 @@ class Repository end end end + + newrev end end diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb new file mode 100644 index 00000000000..43d1c766e35 --- /dev/null +++ b/app/services/commits/revert_service.rb @@ -0,0 +1,58 @@ +module Commits + class RevertService < ::BaseService + class ValidationError < StandardError; end + class ReversionError < StandardError; end + + def execute + @source_project = params[:source_project] || @project + @target_branch = params[:target_branch] + @commit = params[:commit] + @create_merge_request = params[:create_merge_request].present? + + validate and commit + rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, + ValidationError, ReversionError => ex + error(ex.message) + end + + def commit + revert_into = @create_merge_request ? @commit.revert_branch_name : @target_branch + + if @create_merge_request + # Temporary branch exists and contains the revert commit + return success if repository.find_branch(revert_into) + + create_target_branch + end + + unless repository.revert(current_user, @commit, revert_into) + error_msg = "Sorry, we cannot revert this #{params[:revert_type_title]} automatically. + It may have already been reverted, or a more recent commit may have updated some of its content." + raise ReversionError, error_msg + end + + success + end + + private + + def create_target_branch + result = CreateBranchService.new(@project, current_user) + .execute(@commit.revert_branch_name, @target_branch, source_project: @source_project) + + if result[:status] == :error + raise ReversionError, "There was an error creating the source branch: #{result[:message]}" + end + 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 + + true + end + end +end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index a9b29f9654d..c0700d953dd 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -56,7 +56,7 @@ module MergeRequests if commits && commits.count == 1 commit = commits.first merge_request.title = commit.title - merge_request.description = commit.description.try(:strip) + merge_request.description ||= commit.description.try(:strip) else merge_request.title = merge_request.source_branch.titleize.humanize end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index e8bef250d8b..9a58383b398 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -34,7 +34,8 @@ module MergeRequests committer: committer } - repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options) + commit_id = repository.merge(current_user, merge_request.source_sha, merge_request.target_branch, options) + merge_request.update(merge_commit_sha: commit_id) rescue StandardError => e merge_request.update(merge_error: "Something went wrong during merge") Rails.logger.error(e.message) diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index bbe820b8842..71995fcc487 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -16,6 +16,8 @@ = link_to namespace_project_tree_path(@project.namespace, @project, @commit), class: "btn btn-grouped" do = icon('files-o') Browse Files + - unless @commit.has_been_reverted?(current_user) + = revert_commit_link(@commit, namespace_project_commit_path(@project.namespace, @project, @commit.id)) %div %p diff --git a/app/views/projects/commit/_revert.html.haml b/app/views/projects/commit/_revert.html.haml new file mode 100644 index 00000000000..52ca3ed5b14 --- /dev/null +++ b/app/views/projects/commit/_revert.html.haml @@ -0,0 +1,31 @@ +#modal-revert-commit.modal + .modal-dialog + .modal-content + .modal-header + %a.close{href: "#", "data-dismiss" => "modal"} × + %h3.page-title== Revert this #{revert_commit_type(commit)} + .modal-body + = form_tag revert_namespace_project_commit_path(@project.namespace, @project, commit.id), method: :post, remote: false, class: 'form-horizontal js-create-dir-form js-requires-input' do + .form-group.branch + = label_tag 'target_branch', 'Revert in branch', class: 'control-label' + .col-sm-10 + = select_tag "target_branch", grouped_options_refs, class: "select2 select2-sm js-target-branch" + - if can?(current_user, :push_code, @project) + .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 <strong>new merge request</strong> with these changes + - else + = hidden_field_tag 'create_merge_request', 1 + .form-actions + = submit_tag "Revert", 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/commit/show.html.haml b/app/views/projects/commit/show.html.haml index 05dbe5ebea4..21e186120c3 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -12,3 +12,5 @@ = render "projects/diffs/diffs", diffs: @diffs, project: @project, diff_refs: @diff_refs = render "projects/notes/notes_with_form" +- if can_collaborate_with_project? + = render "projects/commit/revert", commit: @commit, title: @commit.title diff --git a/app/views/projects/merge_requests/_show.html.haml b/app/views/projects/merge_requests/_show.html.haml index da67645bc2b..648512e5379 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -85,6 +85,8 @@ = spinner = render 'shared/issuable/sidebar', issuable: @merge_request +- if @merge_request.can_be_reverted? + = render "projects/commit/revert", commit: @merge_request.merge_commit, title: @merge_request.title :javascript var merge_request; diff --git a/app/views/projects/merge_requests/widget/_merged.html.haml b/app/views/projects/merge_requests/widget/_merged.html.haml index d1d602eecdc..3abae9f0bf6 100644 --- a/app/views/projects/merge_requests/widget/_merged.html.haml +++ b/app/views/projects/merge_requests/widget/_merged.html.haml @@ -8,20 +8,18 @@ #{time_ago_with_tooltip(@merge_request.merge_event.created_at)} %div - if !@merge_request.source_branch_exists? || (params[:delete_source] == 'true') - The changes were merged into - #{link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch"}. - The source branch has been removed. - + %p + The changes were merged into + #{link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch"}. + The source branch has been removed. + = render 'projects/merge_requests/widget/merged_buttons' - elsif @merge_request.can_remove_source_branch?(current_user) .remove_source_branch_widget %p The changes were merged into #{link_to @merge_request.target_branch, namespace_project_commits_path(@project.namespace, @project, @merge_request.target_branch), class: "label-branch"}. You can remove the source branch now. - = link_to namespace_project_branch_path(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request.source_branch), remote: true, method: :delete, class: "btn btn-primary btn-sm remove_source_branch" do - %i.fa.fa-times - Remove Source Branch - + = render 'projects/merge_requests/widget/merged_buttons', source_branch_exists: true .remove_source_branch_widget.failed.hide %p Failed to remove source branch '#{@merge_request.source_branch}'. diff --git a/app/views/projects/merge_requests/widget/_merged_buttons.haml b/app/views/projects/merge_requests/widget/_merged_buttons.haml new file mode 100644 index 00000000000..85a3a6ba9e2 --- /dev/null +++ b/app/views/projects/merge_requests/widget/_merged_buttons.haml @@ -0,0 +1,11 @@ +- source_branch_exists = local_assigns.fetch(:source_branch_exists, false) +- mr_can_be_reverted = @merge_request.can_be_reverted? + +- if source_branch_exists || mr_can_be_reverted + .btn-group + - if source_branch_exists + = link_to namespace_project_branch_path(@merge_request.source_project.namespace, @merge_request.source_project, @merge_request.source_branch), remote: true, method: :delete, class: "btn btn-default btn-grouped btn-sm remove_source_branch" do + = icon('trash-o') + Remove Source Branch + - if mr_can_be_reverted + = revert_commit_link(@merge_request.merge_commit, namespace_project_merge_request_path(@project.namespace, @project, @merge_request), btn_class: 'sm') |