diff --git a/CHANGELOG b/CHANGELOG index 4093b2e151e..3f2a69835b5 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -102,6 +102,7 @@ v 8.7.0 (unreleased) v 8.6.7 (unreleased) - Fix vulnerability that made it possible to enumerate private projects belonging to group + - Add support to cherry-pick any commit into any branch in the web interface (Minqi Pan) v 8.6.6 - Expire the exists cache before deletion to ensure project dir actually exists (Stan Hu). !3413 diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 576fa3cedb2..4d64a2d9884 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -12,7 +12,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] + before_action :authorize_edit_tree!, only: [:revert, :cherry_pick] def show apply_diff_view_cookie! @@ -60,27 +60,32 @@ class Projects::CommitController < Projects::ApplicationController end def revert - assign_revert_commit_vars + assign_change_commit_vars(@commit.revert_branch_name) 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) + create_commit(Commits::RevertService, success_notice: "The #{@commit.change_type_title} has been successfully reverted.", + success_path: successful_change_path, failure_path: failed_change_path) + end + + def cherry_pick + assign_change_commit_vars(@commit.cherry_pick_branch_name) + + return render_404 if @target_branch.blank? + + create_commit(Commits::CherryPickService, success_notice: "The #{@commit.change_type_title} has been successfully cherry-picked.", + success_path: successful_change_path, failure_path: failed_change_path) end private - def revert_type_title - @commit.merged_merge_request ? 'merge request' : 'commit' - end - - def successful_revert_path + def successful_change_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 + def failed_change_path return referenced_merge_request_url if @commit.merged_merge_request namespace_project_commit_url(@project.namespace, @project, params[:id]) @@ -111,14 +116,13 @@ class Projects::CommitController < Projects::ApplicationController @statuses = ci_commit.statuses if ci_commit end - def assign_revert_commit_vars + def assign_change_commit_vars(mr_source_branch) @commit = project.commit(params[:id]) @target_branch = params[:target_branch] - @mr_source_branch = @commit.revert_branch_name + @mr_source_branch = mr_source_branch @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 diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index 5394347bd15..b59c3982edd 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -126,12 +126,10 @@ module CommitsHelper 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" + tooltip = "Revert this #{commit.change_type_title} 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', 'data-container' => 'body', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class}" - end + link_to 'Revert', '#modal-revert-commit', 'data-toggle' => 'modal', 'data-container' => 'body', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class} has-tooltip" elsif can?(current_user, :fork_project, @project) continue_params = { to: continue_to_path, @@ -146,11 +144,24 @@ module CommitsHelper end end - def revert_commit_type(commit) - if commit.merged_merge_request - 'merge request' - else - 'commit' + def cherry_pick_commit_link(commit, continue_to_path, btn_class: nil) + return unless current_user + + tooltip = "Cherry-pick this #{commit.change_type_title} in a new merge request" + + if can_collaborate_with_project? + link_to 'Cherry-pick', '#modal-cherry-pick-commit', 'data-toggle' => 'modal', 'data-container' => 'body', title: tooltip, class: "btn btn-default btn-grouped btn-#{btn_class} has-tooltip" + elsif can?(current_user, :fork_project, @project) + continue_params = { + to: continue_to_path, + notice: edit_in_new_fork_notice + ' Try to cherry-pick 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 'Cherry-pick', fork_path, class: 'btn btn-grouped btn-close', method: :post, 'data-toggle' => 'tooltip', 'data-container' => 'body', title: tooltip end end diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb index 4920ca5af6e..dbedf417fa5 100644 --- a/app/helpers/tree_helper.rb +++ b/app/helpers/tree_helper.rb @@ -66,7 +66,7 @@ module TreeHelper ref else project = tree_edit_project(project) - project.repository.next_patch_branch + project.repository.next_branch('patch') end end diff --git a/app/models/commit.rb b/app/models/commit.rb index d1f07ccd55c..b406a4dd8d2 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -218,6 +218,10 @@ class Commit def revert_branch_name "revert-#{short_id}" end + + def cherry_pick_branch_name + project.repository.next_branch("cherry-pick-#{short_id}", mild: true) + end def revert_description if merged_merge_request @@ -253,6 +257,10 @@ class Commit end.any? { |commit_ref| commit_ref.reverts_commit?(self) } end + def change_type_title + merged_merge_request ? 'merge request' : 'commit' + end + private def repo_changes diff --git a/app/models/repository.rb b/app/models/repository.rb index 589756f8531..c6d7a439c05 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -549,15 +549,18 @@ class Repository commit(sha) end - def next_patch_branch - patch_branch_ids = self.branch_names.map do |n| - result = n.match(/\Apatch-([0-9]+)\z/) + def next_branch(name, opts={}) + branch_ids = self.branch_names.map do |n| + next 1 if n == name + result = n.match(/\A#{name}-([0-9]+)\z/) result[1].to_i if result end.compact - highest_patch_branch_id = patch_branch_ids.max || 0 + highest_branch_id = branch_ids.max || 0 - "patch-#{highest_patch_branch_id + 1}" + return name if opts[:mild] && 0 == highest_branch_id + + "#{name}-#{highest_branch_id + 1}" end # Remove archives older than 2 hours @@ -760,6 +763,28 @@ class Repository end end + def cherry_pick(user, commit, base_branch, cherry_pick_tree_id = nil) + source_sha = find_branch(base_branch).target + cherry_pick_tree_id ||= check_cherry_pick_content(commit, base_branch) + + return false unless cherry_pick_tree_id + + commit_with_hooks(user, base_branch) do |ref| + committer = user_to_committer(user) + source_sha = Rugged::Commit.create(rugged, + message: commit.message, + author: { + email: commit.author_email, + name: commit.author_name, + time: commit.authored_date + }, + committer: committer, + tree: cherry_pick_tree_id, + parents: [rugged.lookup(source_sha)], + update_ref: ref) + end + end + def check_revert_content(commit, base_branch) source_sha = find_branch(base_branch).target args = [commit.id, source_sha] @@ -774,6 +799,20 @@ class Repository tree_id end + def check_cherry_pick_content(commit, base_branch) + source_sha = find_branch(base_branch).target + args = [commit.id, source_sha] + args << 1 if commit.merge_commit? + + cherry_pick_index = rugged.cherrypick_commit(*args) + return false if cherry_pick_index.conflicts? + + tree_id = cherry_pick_index.write_tree(rugged) + return false unless diff_exists?(source_sha, tree_id) + + tree_id + end + def diff_exists?(sha1, sha2) rugged.diff(sha1, sha2).size > 0 end diff --git a/app/services/commits/change_service.rb b/app/services/commits/change_service.rb new file mode 100644 index 00000000000..6b69cb53b2c --- /dev/null +++ b/app/services/commits/change_service.rb @@ -0,0 +1,47 @@ +module Commits + class ChangeService < ::BaseService + class ValidationError < StandardError; end + class ChangeError < 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? + + check_push_permissions unless @create_merge_request + commit + rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, + ValidationError, ChangeError => ex + error(ex.message) + end + + def commit + raise NotImplementedError + end + + private + + def check_push_permissions + allowed = ::Gitlab::GitAccess.new(current_user, 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 create_target_branch(new_branch) + # Temporary branch exists and contains the change commit + return success if repository.find_branch(new_branch) + + result = CreateBranchService.new(@project, current_user) + .execute(new_branch, @target_branch, source_project: @source_project) + + if result[:status] == :error + raise ChangeError, "There was an error creating the source branch: #{result[:message]}" + end + end + end +end diff --git a/app/services/commits/cherry_pick_service.rb b/app/services/commits/cherry_pick_service.rb new file mode 100644 index 00000000000..f9a4efa7182 --- /dev/null +++ b/app/services/commits/cherry_pick_service.rb @@ -0,0 +1,19 @@ +module Commits + class CherryPickService < ChangeService + def commit + cherry_pick_into = @create_merge_request ? @commit.cherry_pick_branch_name : @target_branch + cherry_pick_tree_id = repository.check_cherry_pick_content(@commit, @target_branch) + + if cherry_pick_tree_id + create_target_branch(cherry_pick_into) if @create_merge_request + + repository.cherry_pick(current_user, @commit, cherry_pick_into, cherry_pick_tree_id) + success + else + error_msg = "Sorry, we cannot cherry-pick this #{@commit.change_type_title} automatically. + It may have already been cherry-picked, or a more recent commit may have updated some of its content." + raise ChangeError, error_msg + end + end + end +end diff --git a/app/services/commits/revert_service.rb b/app/services/commits/revert_service.rb index a3c950ede1f..c7de9f6f35e 100644 --- a/app/services/commits/revert_service.rb +++ b/app/services/commits/revert_service.rb @@ -1,21 +1,5 @@ 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? - - check_push_permissions unless @create_merge_request - commit - rescue Repository::CommitError, Gitlab::Git::Repository::InvalidBlobName, GitHooksService::PreReceiveError, - ValidationError, ReversionError => ex - error(ex.message) - end - + class RevertService < ChangeService def commit revert_into = @create_merge_request ? @commit.revert_branch_name : @target_branch revert_tree_id = repository.check_revert_content(@commit, @target_branch) @@ -26,34 +10,10 @@ module Commits repository.revert(current_user, @commit, revert_into, revert_tree_id) success else - error_msg = "Sorry, we cannot revert this #{params[:revert_type_title]} automatically. + error_msg = "Sorry, we cannot revert this #{@commit.change_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 + raise ChangeError, error_msg end end - - private - - def create_target_branch(new_branch) - # Temporary branch exists and contains the revert commit - return success if repository.find_branch(new_branch) - - result = CreateBranchService.new(@project, current_user) - .execute(new_branch, @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 check_push_permissions - allowed = ::Gitlab::GitAccess.new(current_user, project).can_push_to_branch?(@target_branch) - - unless allowed - raise ValidationError.new('You are not allowed to push into this branch') - end - - true - end end end diff --git a/app/views/projects/commit/_revert.html.haml b/app/views/projects/commit/_change.html.haml similarity index 62% rename from app/views/projects/commit/_revert.html.haml rename to app/views/projects/commit/_change.html.haml index 52ca3ed5b14..44ef1fdbbe3 100644 --- a/app/views/projects/commit/_revert.html.haml +++ b/app/views/projects/commit/_change.html.haml @@ -1,13 +1,21 @@ -#modal-revert-commit.modal +- case type.to_s +- when 'revert' + - label = 'Revert' + - target_label = 'Revert in branch' +- when 'cherry-pick' + - label = 'Cherry-pick' + - target_label = 'Pick into branch' + +.modal{id: "modal-#{type}-commit"} .modal-dialog .modal-content .modal-header %a.close{href: "#", "data-dismiss" => "modal"} × - %h3.page-title== Revert this #{revert_commit_type(commit)} + %h3.page-title== #{label} this #{commit.change_type_title} .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_tag send("#{type.underscore}_namespace_project_commit_path", @project.namespace, @project, commit.id), method: :post, remote: false, class: 'form-horizontal js-#{type}-form js-requires-input' do .form-group.branch - = label_tag 'target_branch', 'Revert in branch', class: 'control-label' + = label_tag 'target_branch', target_label, 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) @@ -20,7 +28,7 @@ - else = hidden_field_tag 'create_merge_request', 1 .form-actions - = submit_tag "Revert", class: 'btn btn-create' + = submit_tag label, class: 'btn btn-create' = link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal" - unless can?(current_user, :push_code, @project) @@ -28,4 +36,4 @@ = commit_in_fork_help :javascript - new NewCommitForm($('.js-create-dir-form')) + new NewCommitForm($('.js-#{type}-form')) diff --git a/app/views/projects/commit/_commit_box.html.haml b/app/views/projects/commit/_commit_box.html.haml index 71995fcc487..d6c9e54e657 100644 --- a/app/views/projects/commit/_commit_box.html.haml +++ b/app/views/projects/commit/_commit_box.html.haml @@ -18,6 +18,7 @@ Browse Files - unless @commit.has_been_reverted?(current_user) = revert_commit_link(@commit, namespace_project_commit_path(@project.namespace, @project, @commit.id)) + = cherry_pick_commit_link(@commit, namespace_project_commit_path(@project.namespace, @project, @commit.id)) %div %p diff --git a/app/views/projects/commit/show.html.haml b/app/views/projects/commit/show.html.haml index 21e186120c3..e550af7888a 100644 --- a/app/views/projects/commit/show.html.haml +++ b/app/views/projects/commit/show.html.haml @@ -13,4 +13,5 @@ diff_refs: @diff_refs = render "projects/notes/notes_with_form" - if can_collaborate_with_project? - = render "projects/commit/revert", commit: @commit, title: @commit.title + - %w(revert cherry-pick).each do |type| + = render "projects/commit/change", type: type, 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 7125d7d9d1c..65da3712a24 100644 --- a/app/views/projects/merge_requests/_show.html.haml +++ b/app/views/projects/merge_requests/_show.html.haml @@ -87,7 +87,9 @@ = 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 + = render "projects/commit/change", type: 'revert', commit: @merge_request.merge_commit, title: @merge_request.title +- if @merge_request.merge_commit + = render "projects/commit/change", type: 'cherry-pick', commit: @merge_request.merge_commit, title: @merge_request.title :javascript var merge_request; diff --git a/app/views/projects/merge_requests/widget/_merged_buttons.haml b/app/views/projects/merge_requests/widget/_merged_buttons.haml index 85a3a6ba9e2..361acf7d27f 100644 --- a/app/views/projects/merge_requests/widget/_merged_buttons.haml +++ b/app/views/projects/merge_requests/widget/_merged_buttons.haml @@ -9,3 +9,4 @@ 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') + = cherry_pick_commit_link(@merge_request.merge_commit, namespace_project_merge_request_path(@project.namespace, @project, @merge_request), btn_class: 'sm') diff --git a/config/routes.rb b/config/routes.rb index 46a25262844..b9f30ede524 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -549,6 +549,7 @@ Rails.application.routes.draw do post :cancel_builds post :retry_builds post :revert + post :cherry_pick end end diff --git a/doc/intro/README.md b/doc/intro/README.md index fecbbe6317b..ab298d3808e 100644 --- a/doc/intro/README.md +++ b/doc/intro/README.md @@ -25,6 +25,7 @@ Create merge requests and review code. - [Automatically close issues from merge requests](../customization/issue_closing.md) - [Automatically merge when your builds succeed](../workflow/merge_when_build_succeeds.md) - [Revert any commit](../workflow/revert_changes.md) +- [Cherry-pick any commit](../workflow/cherry_pick_changes.md) ## Test and Deploy diff --git a/doc/workflow/README.md b/doc/workflow/README.md index 25893f948ea..9efe41308dc 100644 --- a/doc/workflow/README.md +++ b/doc/workflow/README.md @@ -20,6 +20,7 @@ - [Milestones](milestones.md) - [Merge Requests](merge_requests.md) - [Revert changes](revert_changes.md) +- [Cherry-pick changes](cherry_pick_changes.md) - ["Work In Progress" Merge Requests](wip_merge_requests.md) - [Merge When Build Succeeds](merge_when_build_succeeds.md) - [Manage large binaries with Git LFS](lfs/manage_large_binaries_with_git_lfs.md) diff --git a/doc/workflow/cherry_pick_changes.md b/doc/workflow/cherry_pick_changes.md new file mode 100644 index 00000000000..b0ca0879643 --- /dev/null +++ b/doc/workflow/cherry_pick_changes.md @@ -0,0 +1,52 @@ +# Cherry-pick changes + +_**Note:** This feature was [introduced][ce-3514] in GitLab 8.7._ + +--- + +GitLab implements Git's powerful feature to [cherry-pick any commit][git-cherry-pick] +with introducing a **Cherry-pick** button in Merge Requests and commit details. + +## Cherry-picking a Merge Request + +After the Merge Request has been merged, a **Cherry-pick** button will be available +to cherry-pick the changes introduced by that Merge Request: + +![Cherry-pick Merge Request](img/cherry_pick_changes_mr.png) + +--- + +You can cherry-pick the changes directly into the selected branch or you can opt to +create a new Merge Request with the cherry-pick changes: + +![Cherry-pick Merge Request modal](img/cherry_pick_changes_mr_modal.png) + +## Cherry-picking a Commit + +You can cherry-pick a Commit from the Commit details page: + +![Cherry-pick commit](img/cherry_pick_changes_commit.png) + +--- + +Similar to cherry-picking a Merge Request, you can opt to cherry-pick the changes +directly into the target branch or create a new Merge Request to cherry-pick the +changes: + +![Cherry-pick commit modal](img/cherry_pick_changes_commit_modal.png) + +--- + +Please note that when cherry-picking merge commits, the mainline will always be the +first parent. If you want to use a different mainline then you need to do that +from the command line. + +Here is a quick example to cherry-pick a merge commit using the second parent as the +mainline: + +```bash +git cherry-pick -m 2 7a39eb0 +``` + +[ce-3514]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/3514 "Cherry-pick button Merge Request" +[git-cherry-pick]: https://git-scm.com/docs/git-cherry-pick "Git cherry-pick documentation" diff --git a/doc/workflow/img/cherry_pick_changes_commit.png b/doc/workflow/img/cherry_pick_changes_commit.png new file mode 100644 index 00000000000..ae91d2cae53 Binary files /dev/null and b/doc/workflow/img/cherry_pick_changes_commit.png differ diff --git a/doc/workflow/img/cherry_pick_changes_commit_modal.png b/doc/workflow/img/cherry_pick_changes_commit_modal.png new file mode 100644 index 00000000000..f502f87677a Binary files /dev/null and b/doc/workflow/img/cherry_pick_changes_commit_modal.png differ diff --git a/doc/workflow/img/cherry_pick_changes_mr.png b/doc/workflow/img/cherry_pick_changes_mr.png new file mode 100644 index 00000000000..59c610e620b Binary files /dev/null and b/doc/workflow/img/cherry_pick_changes_mr.png differ diff --git a/doc/workflow/img/cherry_pick_changes_mr_modal.png b/doc/workflow/img/cherry_pick_changes_mr_modal.png new file mode 100644 index 00000000000..96a80f4726d Binary files /dev/null and b/doc/workflow/img/cherry_pick_changes_mr_modal.png differ diff --git a/spec/controllers/commit_controller_spec.rb b/spec/controllers/commit_controller_spec.rb index f09e4fcb154..cf5c606c723 100644 --- a/spec/controllers/commit_controller_spec.rb +++ b/spec/controllers/commit_controller_spec.rb @@ -4,6 +4,8 @@ describe Projects::CommitController do let(:project) { create(:project) } let(:user) { create(:user) } let(:commit) { project.commit("master") } + let(:master_pickable_sha) { '7d3b0f7cff5f37573aea97cebfd5692ea1689924' } + let(:master_pickable_commit) { project.commit(master_pickable_sha) } before do sign_in(user) @@ -192,4 +194,53 @@ describe Projects::CommitController do end end end + + describe '#cherry_pick' do + context 'when target branch is not provided' do + it 'should render the 404 page' do + post(:cherry_pick, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + id: master_pickable_commit.id) + + expect(response).not_to be_success + expect(response.status).to eq(404) + end + end + + context 'when the cherry-pick was successful' do + it 'should redirect to the commits page' do + post(:cherry_pick, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: master_pickable_commit.id) + + expect(response).to redirect_to namespace_project_commits_path(project.namespace, project, 'master') + expect(flash[:notice]).to eq('The commit has been successfully cherry-picked.') + end + end + + context 'when the cherry_pick failed' do + before do + post(:cherry_pick, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: master_pickable_commit.id) + end + + it 'should redirect to the commit page' do + # Cherry-picking a commit that has been already cherry-picked. + post(:cherry_pick, + namespace_id: project.namespace.to_param, + project_id: project.to_param, + target_branch: 'master', + id: master_pickable_commit.id) + + expect(response).to redirect_to namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) + expect(flash[:alert]).to match('Sorry, we cannot cherry-pick this commit automatically.') + end + end + end end diff --git a/spec/features/project/commits/cherry_pick_spec.rb b/spec/features/project/commits/cherry_pick_spec.rb new file mode 100644 index 00000000000..0559b02f321 --- /dev/null +++ b/spec/features/project/commits/cherry_pick_spec.rb @@ -0,0 +1,67 @@ +require 'spec_helper' + +describe 'Cherry-pick Commits' do + let(:project) { create(:project) } + let(:master_pickable_commit) { project.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } + let(:master_pickable_merge) { project.commit('e56497bb5f03a90a51293fc6d516788730953899') } + + + before do + login_as :user + project.team << [@user, :master] + visit namespace_project_commits_path(project.namespace, project, project.repository.root_ref, { limit: 5 }) + end + + context "I cherry-pick a commit" do + it do + visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) + find("a[href='#modal-cherry-pick-commit']").click + page.within('#modal-cherry-pick-commit') do + uncheck 'create_merge_request' + click_button 'Cherry-pick' + end + expect(page).to have_content('The commit has been successfully cherry-picked.') + end + end + + context "I cherry-pick a merge commit" do + it do + visit namespace_project_commit_path(project.namespace, project, master_pickable_merge.id) + find("a[href='#modal-cherry-pick-commit']").click + page.within('#modal-cherry-pick-commit') do + uncheck 'create_merge_request' + click_button 'Cherry-pick' + end + expect(page).to have_content('The commit has been successfully cherry-picked.') + end + end + + context "I cherry-pick a commit that was previously cherry-picked" do + it do + visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) + find("a[href='#modal-cherry-pick-commit']").click + page.within('#modal-cherry-pick-commit') do + uncheck 'create_merge_request' + click_button 'Cherry-pick' + end + visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) + find("a[href='#modal-cherry-pick-commit']").click + page.within('#modal-cherry-pick-commit') do + uncheck 'create_merge_request' + click_button 'Cherry-pick' + end + expect(page).to have_content('Sorry, we cannot cherry-pick this commit automatically.') + end + end + + context "I cherry-pick a commit in a new merge request" do + it do + visit namespace_project_commit_path(project.namespace, project, master_pickable_commit.id) + find("a[href='#modal-cherry-pick-commit']").click + page.within('#modal-cherry-pick-commit') do + click_button 'Cherry-pick' + end + expect(page).to have_content('The commit has been successfully cherry-picked. You can now submit a merge request to get this change into the original branch.') + end + end +end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index f30a21e79ae..35d7dcd8aea 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -541,6 +541,41 @@ describe Repository, models: true do end end + describe '#cherry_pick' do + let(:conflict_commit) { repository.commit('c642fe9b8b9f28f9225d7ea953fe14e74748d53b') } + let(:pickable_commit) { repository.commit('7d3b0f7cff5f37573aea97cebfd5692ea1689924') } + let(:pickable_merge) { repository.commit('e56497bb5f03a90a51293fc6d516788730953899') } + + context 'when there is a conflict' do + it 'should abort the operation' do + expect(repository.cherry_pick(user, conflict_commit, 'master')).to eq(false) + end + end + + context 'when commit was already cherry-picked' do + it 'should abort the operation' do + repository.cherry_pick(user, pickable_commit, 'master') + + expect(repository.cherry_pick(user, pickable_commit, 'master')).to eq(false) + end + end + + context 'when commit can be cherry-picked' do + it 'should cherry-pick the changes' do + expect(repository.cherry_pick(user, pickable_commit, 'master')).to be_truthy + end + end + + context 'cherry-picking a merge commit' do + it 'should cherry-pick the changes' do + expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).to be_nil + + repository.cherry_pick(user, pickable_merge, 'master') + expect(repository.blob_at_branch('master', 'foo/bar/.gitkeep')).not_to be_nil + end + end + end + describe '#before_delete' do describe 'when a repository does not exist' do before do diff --git a/spec/support/repo_helpers.rb b/spec/support/repo_helpers.rb index aa8258d6dad..73f375c481b 100644 --- a/spec/support/repo_helpers.rb +++ b/spec/support/repo_helpers.rb @@ -42,7 +42,7 @@ Signed-off-by: Dmitriy Zaporozhets eos ) end - + def another_sample_commit OpenStruct.new( id: "e56497bb5f03a90a51293fc6d516788730953899",