diff --git a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
index 7c0bbdd403f919b86b3ee7c2d8e4dd03a1f37a7c..fecbfec22149ca4e117efcbe32fb1345dd300d76 100644
--- a/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
+++ b/app/assets/javascripts/vue_merge_request_widget/services/mr_widget_service.js
@@ -37,6 +37,10 @@ export default class MRWidgetService {
return axios.get(this.endpoints.mergeActionsContentPath);
}
+ rebase() {
+ return axios.post(this.endpoints.rebasePath);
+ }
+
static stopEnvironment(url) {
return axios.post(url);
}
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js
index 2bace3311c89511d229abf29f8a1b24bcc17b3bc..f7f0c1b6cb7a3b98e56d1d4091c04fca15143ebc 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/get_state_key.js
@@ -25,6 +25,8 @@ export default function deviseState(data) {
return this.mergeError ? stateKey.autoMergeFailed : stateKey.mergeWhenPipelineSucceeds;
} else if (!this.canMerge) {
return stateKey.notAllowedToMerge;
+ } else if (this.shouldBeRebased) {
+ return stateKey.rebase;
} else if (this.canBeMerged) {
return stateKey.readyToMerge;
}
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index 474c17ec1337c72ba3ca83b2ad66e9d9d451fae5..ed004b3bb081c10e7cb78c7586c50c884dbad2f4 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -26,6 +26,7 @@ export default class MergeRequestStore {
this.divergedCommitsCount = data.diverged_commits_count;
this.pipeline = data.pipeline || {};
this.deployments = this.deployments || data.deployments || [];
+ this.initRebase(data);
if (data.issues_links) {
const links = data.issues_links;
@@ -124,6 +125,13 @@ export default class MergeRequestStore {
return this.state === stateKey.nothingToMerge;
}
+ initRebase(data) {
+ this.canPushToSourceBranch = data.can_push_to_source_branch;
+ this.rebaseInProgress = data.rebase_in_progress;
+ this.approvalsLeft = !data.approved;
+ this.rebasePath = data.rebase_path;
+ }
+
static buildMetrics(metrics) {
if (!metrics) {
return {};
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
index de980c175fb6c2ac434e746aa47e6f4f2dcf7ac9..29d5bd4a1dac347be809d8269e086b644fa1739c 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/state_maps.js
@@ -17,6 +17,7 @@ const stateToComponentMap = {
failedToMerge: 'mr-widget-failed-to-merge',
autoMergeFailed: 'mr-widget-auto-merge-failed',
shaMismatch: 'mr-widget-sha-mismatch',
+ rebase: 'mr-widget-rebase',
};
const statesToShowHelpWidget = [
@@ -29,6 +30,7 @@ const statesToShowHelpWidget = [
'pipelineFailed',
'pipelineBlocked',
'autoMergeFailed',
+ 'rebase',
];
export const stateKey = {
@@ -46,6 +48,7 @@ export const stateKey = {
mergeWhenPipelineSucceeds: 'mergeWhenPipelineSucceeds',
notAllowedToMerge: 'notAllowedToMerge',
readyToMerge: 'readyToMerge',
+ rebase: 'rebase',
};
export default {
diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb
index 6b59c8461a34f99cd99af93ba9331344512a5ed0..2e8a738b6d91af6e26d759a7d9eba82afcb61901 100644
--- a/app/controllers/projects/merge_requests_controller.rb
+++ b/app/controllers/projects/merge_requests_controller.rb
@@ -10,6 +10,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
before_action :authorize_update_issuable!, only: [:close, :edit, :update, :remove_wip, :sort]
before_action :set_issuables_index, only: [:index]
before_action :authenticate_user!, only: [:assign_related_issues]
+ before_action :check_user_can_push_to_source_branch!, only: [:rebase]
def index
@merge_requests = @issuables
@@ -223,6 +224,12 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
render json: environments
end
+ def rebase
+ RebaseWorker.perform_async(@merge_request.id, current_user.id)
+
+ render nothing: true, status: 200
+ end
+
protected
alias_method :subscribable_resource, :merge_request
@@ -322,4 +329,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
@finder_type = MergeRequestsFinder
super
end
+
+ def check_user_can_push_to_source_branch!
+ return access_denied! unless @merge_request.source_branch_exists?
+
+ access_check = ::Gitlab::UserAccess
+ .new(current_user, project: @merge_request.source_project)
+ .can_push_to_branch?(@merge_request.source_branch)
+
+ access_denied! unless access_check
+ end
end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index c39789b047d100ba12f8a51744052a05a663f782..ef58816937c8b3cbc66098a63dfb6d145b4457fd 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -156,6 +156,13 @@ class MergeRequest < ActiveRecord::Base
'!'
end
+ def rebase_in_progress?
+ # The source project can be deleted
+ return false unless source_project
+
+ source_project.repository.rebase_in_progress?(id)
+ end
+
# Use this method whenever you need to make sure the head_pipeline is synced with the
# branch head commit, for example checking if a merge request can be merged.
# For more information check: https://gitlab.com/gitlab-org/gitlab-ce/issues/40004
@@ -607,7 +614,7 @@ class MergeRequest < ActiveRecord::Base
check_if_can_be_merged
- can_be_merged?
+ can_be_merged? && !should_be_rebased?
end
def mergeable_state?(skip_ci_check: false)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index b1fd981965c1c44e6f48882136e84604e35bcee6..4bedcbfb6a270371d2a4a8ad7a51522a51a81c87 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -1099,6 +1099,13 @@ class Repository
@project.repository_storage_path
end
+ def rebase(user, merge_request)
+ raw.rebase(user, merge_request.id, branch: merge_request.source_branch,
+ branch_sha: merge_request.source_branch_sha,
+ remote_repository: merge_request.target_project.repository.raw,
+ remote_branch: merge_request.target_branch)
+ end
+
private
# TODO Generice finder, later split this on finders by Ref or Oid
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index ab4c87c016979f5475235d84c365ee1f69b79f25..c6806b7cc26d4a6c76d8dd0a522fe47eeeb6a81b 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -76,6 +76,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end
end
+ def rebase_path
+ if !rebase_in_progress? && should_be_rebased? && user_can_push_to_source_branch?
+ rebase_project_merge_request_path(project, merge_request)
+ end
+ end
+
def target_branch_tree_path
if target_branch_exists?
project_tree_path(project, target_branch)
@@ -152,6 +158,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
user_can_collaborate_with_project? && can_be_cherry_picked?
end
+ def can_push_to_source_branch?
+ source_branch_exists? && user_can_push_to_source_branch?
+ end
+
private
def conflicts
@@ -174,6 +184,14 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end.sort.to_sentence
end
+ def user_can_push_to_source_branch?
+ return false unless source_branch_exists?
+
+ ::Gitlab::UserAccess
+ .new(current_user, project: source_project)
+ .can_push_to_branch?(source_branch)
+ end
+
def user_can_collaborate_with_project?
can?(current_user, :push_code, project) ||
(current_user && current_user.already_forked?(project))
diff --git a/app/serializers/merge_request_basic_entity.rb b/app/serializers/merge_request_basic_entity.rb
index d54a6516aedf3733fe1f3af848c2f29a40780c20..e4aec977f016c9aeb2c16dea4ef5e024562ab518 100644
--- a/app/serializers/merge_request_basic_entity.rb
+++ b/app/serializers/merge_request_basic_entity.rb
@@ -4,4 +4,5 @@ class MergeRequestBasicEntity < IssuableSidebarEntity
expose :merge_error
expose :state
expose :source_branch_exists?, as: :source_branch_exists
+ expose :rebase_in_progress?, as: :rebase_in_progress
end
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index e905e6876c2ea64e422e857b14e0267866ee4110..48cd2317f46eff15d1b20ea418ac0a34f96b0cb4 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -23,6 +23,16 @@ class MergeRequestWidgetEntity < IssuableEntity
MergeRequestMetricsEntity.new(metrics).as_json
end
+ expose :rebase_commit_sha
+ expose :rebase_in_progress?, as: :rebase_in_progress
+
+ expose :can_push_to_source_branch do |merge_request|
+ presenter(merge_request).can_push_to_source_branch?
+ end
+ expose :rebase_path do |merge_request|
+ presenter(merge_request).rebase_path
+ end
+
# User entities
expose :merge_user, using: UserEntity
diff --git a/app/services/merge_requests/rebase_service.rb b/app/services/merge_requests/rebase_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..0d5a25fa28e2b95dd4f850e7ea6802dc39638130
--- /dev/null
+++ b/app/services/merge_requests/rebase_service.rb
@@ -0,0 +1,30 @@
+module MergeRequests
+ class RebaseService < MergeRequests::WorkingCopyBaseService
+ def execute(merge_request)
+ @merge_request = merge_request
+
+ if rebase
+ success
+ else
+ error('Failed to rebase. Should be done manually')
+ end
+ end
+
+ def rebase
+ if merge_request.rebase_in_progress?
+ log_error('Rebase task canceled: Another rebase is already in progress', save_message_on_model: true)
+ return false
+ end
+
+ rebase_sha = repository.rebase(current_user, merge_request)
+
+ merge_request.update_attributes(rebase_commit_sha: rebase_sha)
+
+ true
+ rescue => e
+ log_error('Failed to rebase branch:')
+ log_error(e.message, save_message_on_model: true)
+ false
+ end
+ end
+end
diff --git a/app/services/merge_requests/working_copy_base_service.rb b/app/services/merge_requests/working_copy_base_service.rb
new file mode 100644
index 0000000000000000000000000000000000000000..186e05bf9665fcb82dcfcc5508e1196275d0097a
--- /dev/null
+++ b/app/services/merge_requests/working_copy_base_service.rb
@@ -0,0 +1,24 @@
+module MergeRequests
+ class WorkingCopyBaseService < MergeRequests::BaseService
+ attr_reader :merge_request
+
+ def source_project
+ @source_project ||= merge_request.source_project
+ end
+
+ def target_project
+ @target_project ||= merge_request.target_project
+ end
+
+ def log_error(message, save_message_on_model: false)
+ Gitlab::GitLogger.error("#{self.class.name} error (#{merge_request.to_reference(full: true)}): #{message}")
+
+ merge_request.update(merge_error: message) if save_message_on_model
+ end
+
+ # Don't try to print expensive instance variables.
+ def inspect
+ "#<#{self.class} #{merge_request.to_reference(full: true)}>"
+ end
+ end
+end
diff --git a/app/views/projects/_merge_request_fast_forward_settings.html.haml b/app/views/projects/_merge_request_fast_forward_settings.html.haml
index 9d357293a2f0cc7ff9db12eeacbf5b4bf96f6847..8129c72feb218aa9dc3abe004bd3f5cba2b40ce5 100644
--- a/app/views/projects/_merge_request_fast_forward_settings.html.haml
+++ b/app/views/projects/_merge_request_fast_forward_settings.html.haml
@@ -10,4 +10,4 @@
No merge commits are created and all merges are fast-forwarded, which means that merging is only allowed if the branch could be fast-forwarded.
%br
%span.descr
- When fast-forward merge is not possible, the user must first rebase locally.
+ When fast-forward merge is not possible, the user is given the option to rebase.
diff --git a/app/views/projects/_merge_request_rebase_settings.html.haml b/app/views/projects/_merge_request_rebase_settings.html.haml
index c52e09573a635dc27345ccaff42bbfa86fd93eb2..54e0b73d24c3fab8ed1c7cf91153e325803722e4 100644
--- a/app/views/projects/_merge_request_rebase_settings.html.haml
+++ b/app/views/projects/_merge_request_rebase_settings.html.haml
@@ -10,4 +10,4 @@
This way you could make sure that if this merge request would build, after merging to target branch it would also build.
%br
%span.descr
- When fast-forward merge is not possible, the user must first rebase locally.
+ When fast-forward merge is not possible, the user is given the option to rebase.
diff --git a/app/workers/all_queues.yml b/app/workers/all_queues.yml
index 268b7028fd9e2058564c4e20bfb1fdb8a44e4332..fafd9e5ef0055de2bc2e800778204153561e9758 100644
--- a/app/workers/all_queues.yml
+++ b/app/workers/all_queues.yml
@@ -89,6 +89,7 @@
- project_service
- propagate_service_template
- reactive_caching
+- rebase
- repository_fork
- repository_import
- storage_migrator
diff --git a/app/workers/rebase_worker.rb b/app/workers/rebase_worker.rb
new file mode 100644
index 0000000000000000000000000000000000000000..090987778a2d28f87b4c6d41a203cfb035d8cfed
--- /dev/null
+++ b/app/workers/rebase_worker.rb
@@ -0,0 +1,12 @@
+class RebaseWorker
+ include ApplicationWorker
+
+ def perform(merge_request_id, current_user_id)
+ current_user = User.find(current_user_id)
+ merge_request = MergeRequest.find(merge_request_id)
+
+ MergeRequests::RebaseService
+ .new(merge_request.source_project, current_user)
+ .execute(merge_request)
+ end
+end
diff --git a/changelogs/unreleased/40301-rebase.yml b/changelogs/unreleased/40301-rebase.yml
new file mode 100644
index 0000000000000000000000000000000000000000..1c0fc0cd8ae5bd65897d660e53a45c054742bba4
--- /dev/null
+++ b/changelogs/unreleased/40301-rebase.yml
@@ -0,0 +1,5 @@
+---
+title: Allow user to rebase merge requests.
+merge_request:
+author:
+type: added
diff --git a/config/routes/project.rb b/config/routes/project.rb
index c3ad53a387fa860ba5123e962d620f13f42ae4e9..1354c4c55374f16ae259705a404b784bb6beecb8 100644
--- a/config/routes/project.rb
+++ b/config/routes/project.rb
@@ -96,6 +96,7 @@ constraints(ProjectUrlConstrainer.new) do
post :toggle_subscription
post :remove_wip
post :assign_related_issues
+ post :rebase
scope constraints: { format: nil }, action: :show do
get :commits, defaults: { tab: 'commits' }
diff --git a/db/migrate/20171230123729_add_rebase_commit_sha_to_merge_requests.rb b/db/migrate/20171230123729_add_rebase_commit_sha_to_merge_requests.rb
new file mode 100644
index 0000000000000000000000000000000000000000..2ce156fa92e493c99ec02392781b80d2e80ec550
--- /dev/null
+++ b/db/migrate/20171230123729_add_rebase_commit_sha_to_merge_requests.rb
@@ -0,0 +1,7 @@
+class AddRebaseCommitShaToMergeRequests < ActiveRecord::Migration
+ DOWNTIME = false
+
+ def change
+ add_column :merge_requests, :rebase_commit_sha, :string
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 778d66f16b009f2580bfe3afd5ab00975f39de87..740e80ccfd48e92300ff1579844fd60015a3df74 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20171229225929) do
+ActiveRecord::Schema.define(version: 20171230123729) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -1099,6 +1099,7 @@ ActiveRecord::Schema.define(version: 20171229225929) do
t.string "merge_jid"
t.boolean "discussion_locked"
t.integer "latest_merge_request_diff_id"
+ t.string "rebase_commit_sha"
end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
diff --git a/doc/user/project/merge_requests/fast_forward_merge.md b/doc/user/project/merge_requests/fast_forward_merge.md
index 085170d9f03fb028fa853393c9f3f9bdeaca2ae8..3cd91a185e3c3d150cc46a14e8ab9c4728c9792f 100644
--- a/doc/user/project/merge_requests/fast_forward_merge.md
+++ b/doc/user/project/merge_requests/fast_forward_merge.md
@@ -9,7 +9,7 @@ When the fast-forward merge ([`--ff-only`][ffonly]) setting is enabled, no merge
commits will be created and all merges are fast-forwarded, which means that
merging is only allowed if the branch could be fast-forwarded.
-When a fast-forward merge is not possible, the user must rebase the branch manually.
+When a fast-forward merge is not possible, the user is given the option to rebase.
## Use cases
@@ -25,7 +25,7 @@ merge commits. In such cases, the fast-forward merge is the perfect candidate.
Now, when you visit the merge request page, you will be able to accept it
**only if a fast-forward merge is possible**.
-
+
If the target branch is ahead of the source branch, you need to rebase the
source branch locally before you will be able to do a fast-forward merge.
diff --git a/doc/user/project/merge_requests/img/ff_merge_mr.png b/doc/user/project/merge_requests/img/ff_merge_mr.png
deleted file mode 100644
index 241cc99034305316ae8f29c234f5c83d7d2d62c4..0000000000000000000000000000000000000000
Binary files a/doc/user/project/merge_requests/img/ff_merge_mr.png and /dev/null differ
diff --git a/doc/user/project/merge_requests/img/ff_merge_rebase.png b/doc/user/project/merge_requests/img/ff_merge_rebase.png
new file mode 100644
index 0000000000000000000000000000000000000000..f6139f189ced3f004d4f60e49a774d0466c7cb54
Binary files /dev/null and b/doc/user/project/merge_requests/img/ff_merge_rebase.png differ
diff --git a/features/project/ff_merge_requests.feature b/features/project/ff_merge_requests.feature
index 995e52f9332abb79c38bbb2b57067bd5fb9bacc1..39035d551d155225c407e89b41a2e4c1cac8d831 100644
--- a/features/project/ff_merge_requests.feature
+++ b/features/project/ff_merge_requests.feature
@@ -22,3 +22,20 @@ Feature: Project Ff Merge Requests
Then I should see ff-only merge button
When I accept this merge request
Then I should see merged request
+
+ @javascript
+ Scenario: I do rebase before ff-only merge
+ Given ff merge enabled
+ And rebase before merge enabled
+ When I visit merge request page "Bug NS-05"
+ Then I should see rebase button
+ When I press rebase button
+ Then I should see rebase in progress message
+
+ @javascript
+ Scenario: I do rebase before regular merge
+ Given rebase before merge enabled
+ When I visit merge request page "Bug NS-05"
+ Then I should see rebase button
+ When I press rebase button
+ Then I should see rebase in progress message
diff --git a/features/steps/project/ff_merge_requests.rb b/features/steps/project/ff_merge_requests.rb
index d68fe71e16e3c363df1acdbc9538c3ea9cf4d95c..27efcfd65b6d93d43da2b655e41fc5f3b50077c7 100644
--- a/features/steps/project/ff_merge_requests.rb
+++ b/features/steps/project/ff_merge_requests.rb
@@ -17,6 +17,10 @@ class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps
author: project.users.first)
end
+ step 'merge request is mergeable' do
+ expect(page).to have_button 'Merge'
+ end
+
step 'I should see ff-only merge button' do
expect(page).to have_content "Fast-forward merge without a merge commit"
expect(page).to have_button 'Merge'
@@ -45,6 +49,10 @@ class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps
project.save!
end
+ step 'I should see rebase button' do
+ expect(page).to have_button "Rebase"
+ end
+
step 'merge request "Bug NS-05" is rebased' do
merge_request.source_branch = 'flatten-dir'
merge_request.target_branch = 'improve/awesome'
@@ -59,6 +67,20 @@ class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps
merge_request.save!
end
+ step 'rebase before merge enabled' do
+ project = merge_request.target_project
+ project.merge_requests_rebase_enabled = true
+ project.save!
+ end
+
+ step 'I press rebase button' do
+ click_button "Rebase"
+ end
+
+ step "I should see rebase in progress message" do
+ expect(page).to have_content("Rebase in progress")
+ end
+
def merge_request
@merge_request ||= MergeRequest.find_by!(title: "Bug NS-05")
end
diff --git a/lib/gitlab/git/operation_service.rb b/lib/gitlab/git/operation_service.rb
index ef5bdbaf8191e75127ac90387a80fa762af4415b..3fb0e2eed931ddc5b04f473f2dccc9459cd10de6 100644
--- a/lib/gitlab/git/operation_service.rb
+++ b/lib/gitlab/git/operation_service.rb
@@ -97,6 +97,11 @@ module Gitlab
end
end
+ def update_branch(branch_name, newrev, oldrev)
+ ref = Gitlab::Git::BRANCH_REF_PREFIX + branch_name
+ update_ref_in_hooks(ref, newrev, oldrev)
+ end
+
private
# Returns [newrev, should_run_after_create, should_run_after_create_branch]
diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb
index 45c424af8c4cf8a53fe6cb94684de4089ef2234e..c8cc6b374f669c346d4a4afc86443f9122e7a554 100644
--- a/spec/controllers/projects/merge_requests_controller_spec.rb
+++ b/spec/controllers/projects/merge_requests_controller_spec.rb
@@ -684,4 +684,62 @@ describe Projects::MergeRequestsController do
format: :json
end
end
+
+ describe 'POST #rebase' do
+ let(:viewer) { user }
+
+ def post_rebase
+ post :rebase, namespace_id: project.namespace, project_id: project, id: merge_request
+ end
+
+ def expect_rebase_worker_for(user)
+ expect(RebaseWorker).to receive(:perform_async).with(merge_request.id, user.id)
+ end
+
+ context 'successfully' do
+ it 'enqeues a RebaseWorker' do
+ expect_rebase_worker_for(viewer)
+
+ post_rebase
+
+ expect(response.status).to eq(200)
+ end
+ end
+
+ context 'with a forked project' do
+ let(:fork_project) { create(:project, :repository, forked_from_project: project) }
+ let(:fork_owner) { fork_project.owner }
+
+ before do
+ merge_request.update!(source_project: fork_project)
+ fork_project.add_reporter(user)
+ end
+
+ context 'user cannot push to source branch' do
+ it 'returns 404' do
+ expect_rebase_worker_for(viewer).never
+
+ post_rebase
+
+ expect(response.status).to eq(404)
+ end
+ end
+
+ context 'user can push to source branch' do
+ before do
+ project.add_reporter(fork_owner)
+
+ sign_in(fork_owner)
+ end
+
+ it 'returns 200' do
+ expect_rebase_worker_for(fork_owner)
+
+ post_rebase
+
+ expect(response.status).to eq(200)
+ end
+ end
+ end
+ end
end
diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json
index 995f13381adb36fbda15778ad4ed7881e5fda6dd..f1199468d53d0e841fcbd64af374a7be151a23dc 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_basic.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json
@@ -9,6 +9,7 @@
"human_time_estimate": { "type": ["string", "null"] },
"human_total_time_spent": { "type": ["string", "null"] },
"merge_error": { "type": ["string", "null"] },
+ "rebase_in_progress": { "type": "boolean" },
"assignee_id": { "type": ["integer", "null"] },
"subscribed": { "type": ["boolean", "null"] },
"participants": { "type": "array" }
diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json
index 9de27bee751e945015a4ded3d7c43b229e997cf8..7f662098216a90d2bffab8c1f95362576c59af9c 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_widget.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json
@@ -103,7 +103,11 @@
"remove_source_branch": { "type": ["boolean", "null"] },
"merge_ongoing": { "type": "boolean" },
"ff_only_enabled": { "type": ["boolean", false] },
- "should_be_rebased": { "type": "boolean" }
+ "should_be_rebased": { "type": "boolean" },
+ "rebase_commit_sha": { "type": ["string", "null"] },
+ "rebase_in_progress": { "type": "boolean" },
+ "can_push_to_source_branch": { "type": "boolean" },
+ "rebase_path": { "type": ["string", "null"] }
},
"additionalProperties": false
}
diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js
new file mode 100644
index 0000000000000000000000000000000000000000..66ecaa316c8dacdb0b0ee63ee6bb3322c7794e27
--- /dev/null
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_rebase_spec.js
@@ -0,0 +1,115 @@
+import Vue from 'vue';
+import eventHub from '~/vue_merge_request_widget/event_hub';
+import component from '~/vue_merge_request_widget/components/states/mr_widget_rebase.vue';
+import mountComponent from '../../helpers/vue_mount_component_helper';
+
+describe('Merge request widget rebase component', () => {
+ let Component;
+ let vm;
+ beforeEach(() => {
+ Component = Vue.extend(component);
+ });
+
+ afterEach(() => {
+ vm.$destroy();
+ });
+
+ describe('While rebasing', () => {
+ it('should show progress message', () => {
+ vm = mountComponent(Component, {
+ mr: { rebaseInProgress: true },
+ service: {},
+ });
+
+ expect(
+ vm.$el.querySelector('.rebase-state-find-class-convention span').textContent.trim(),
+ ).toContain('Rebase in progress');
+ });
+ });
+
+ describe('With permissions', () => {
+ beforeEach(() => {
+ vm = mountComponent(Component, {
+ mr: {
+ rebaseInProgress: false,
+ canPushToSourceBranch: true,
+ },
+ service: {},
+ });
+ });
+
+ it('it should render rebase button and warning message', () => {
+ const text = vm.$el.querySelector('.rebase-state-find-class-convention span').textContent.trim();
+ expect(text).toContain('Fast-forward merge is not possible.');
+ expect(text).toContain('Rebase the source branch onto the target branch or merge target');
+ expect(text).toContain('branch into source branch to allow this merge request to be merged.');
+ });
+
+ it('it should render error message when it fails', (done) => {
+ vm.rebasingError = 'Something went wrong!';
+
+ Vue.nextTick(() => {
+ expect(
+ vm.$el.querySelector('.rebase-state-find-class-convention span').textContent.trim(),
+ ).toContain('Something went wrong!');
+ done();
+ });
+ });
+ });
+
+ describe('Without permissions', () => {
+ it('should render a message explaining user does not have permissions', () => {
+ vm = mountComponent(Component, {
+ mr: {
+ rebaseInProgress: false,
+ canPushToSourceBranch: false,
+ targetBranch: 'foo',
+ },
+ service: {},
+ });
+
+ const text = vm.$el.querySelector('.rebase-state-find-class-convention span').textContent.trim();
+
+ expect(text).toContain('Fast-forward merge is not possible.');
+ expect(text).toContain('Rebase the source branch onto');
+ expect(text).toContain('foo');
+ expect(text).toContain('to allow this merge request to be merged.');
+ });
+ });
+
+ describe('methods', () => {
+ it('checkRebaseStatus', (done) => {
+ spyOn(eventHub, '$emit');
+ vm = mountComponent(Component, {
+ mr: {},
+ service: {
+ rebase() {
+ return Promise.resolve();
+ },
+ poll() {
+ return Promise.resolve({
+ data: {
+ rebase_in_progress: false,
+ merge_error: null,
+ },
+ });
+ },
+ },
+ });
+
+ vm.rebase();
+
+ // Wait for the rebase request
+ vm.$nextTick()
+ // Wait for the polling request
+ .then(vm.$nextTick())
+ // Wait for the eventHub to be called
+ .then(vm.$nextTick())
+ .then(() => {
+ expect(eventHub.$emit).toHaveBeenCalledWith('MRWidgetUpdateRequested');
+ })
+ .then(done)
+ .catch(done.fail);
+ });
+ });
+});
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index d8ebd46faab0eea8f5445824f75b9b26fa9ba38e..07b3e1c175812b533a96ea31d12f4a35ad33977d 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -1903,4 +1903,50 @@ describe MergeRequest do
end
end
end
+
+ describe '#should_be_rebased?' do
+ let(:project) { create(:project, :repository) }
+
+ it 'returns false for the same source and target branches' do
+ merge_request = create(:merge_request, source_project: project, target_project: project)
+
+ expect(merge_request.should_be_rebased?).to be_falsey
+ end
+ end
+
+ describe '#rebase_in_progress?' do
+ # Create merge request and project before we stub file calls
+ before do
+ subject
+ end
+
+ it 'returns true when there is a current rebase directory' do
+ allow(File).to receive(:exist?).and_return(true)
+ allow(File).to receive(:mtime).and_return(Time.now)
+
+ expect(subject.rebase_in_progress?).to be_truthy
+ end
+
+ it 'returns false when there is no rebase directory' do
+ allow(File).to receive(:exist?).and_return(false)
+
+ expect(subject.rebase_in_progress?).to be_falsey
+ end
+
+ it 'returns false when the rebase directory has expired' do
+ allow(File).to receive(:exist?).and_return(true)
+ allow(File).to receive(:mtime).and_return(20.minutes.ago)
+
+ expect(subject.rebase_in_progress?).to be_falsey
+ end
+
+ it 'returns false when the source project has been removed' do
+ allow(subject).to receive(:source_project).and_return(nil)
+ allow(File).to receive(:exist?).and_return(true)
+ allow(File).to receive(:mtime).and_return(Time.now)
+
+ expect(File).not_to have_received(:exist?)
+ expect(subject.rebase_in_progress?).to be_falsey
+ end
+ end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 3c2ed043b82648aa3f319e5bf4785b8fb0e92409..13e5345ee4ce42890a4fc429129b3e4b493dd7f8 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -418,14 +418,21 @@ describe Project do
end
describe '#merge_method' do
- it 'returns "ff" merge_method when ff is enabled' do
- project = build(:project, merge_requests_ff_only_enabled: true)
- expect(project.merge_method).to be :ff
+ using RSpec::Parameterized::TableSyntax
+
+ where(:ff, :rebase, :method) do
+ true | true | :ff
+ true | false | :ff
+ false | true | :rebase_merge
+ false | false | :merge
end
- it 'returns "merge" merge_method when ff is disabled' do
- project = build(:project, merge_requests_ff_only_enabled: false)
- expect(project.merge_method).to be :merge
+ with_them do
+ let(:project) { build(:project, merge_requests_rebase_enabled: rebase, merge_requests_ff_only_enabled: ff) }
+
+ subject { project.merge_method }
+
+ it { is_expected.to eq(method) }
end
end
diff --git a/spec/presenters/merge_request_presenter_spec.rb b/spec/presenters/merge_request_presenter_spec.rb
index 969c4753f33e9f47c31d7f7b84c520bfe6e25514..e3b37739e8ed4700842dd09aae7499d75fd3a6a8 100644
--- a/spec/presenters/merge_request_presenter_spec.rb
+++ b/spec/presenters/merge_request_presenter_spec.rb
@@ -404,4 +404,67 @@ describe MergeRequestPresenter do
.to eq("
#{resource.source_branch}")
end
end
+
+ describe '#rebase_path' do
+ before do
+ allow(resource).to receive(:rebase_in_progress?) { rebase_in_progress }
+ allow(resource).to receive(:should_be_rebased?) { should_be_rebased }
+
+ allow_any_instance_of(Gitlab::UserAccess::RequestCacheExtension)
+ .to receive(:can_push_to_branch?)
+ .with(resource.source_branch)
+ .and_return(can_push_to_branch)
+ end
+
+ subject do
+ described_class.new(resource, current_user: user).rebase_path
+ end
+
+ context 'when can rebase' do
+ let(:rebase_in_progress) { false }
+ let(:can_push_to_branch) { true }
+ let(:should_be_rebased) { true }
+
+ before do
+ allow(resource).to receive(:source_branch_exists?) { true }
+ end
+
+ it 'returns path' do
+ is_expected
+ .to eq("/#{project.full_path}/merge_requests/#{resource.iid}/rebase")
+ end
+ end
+
+ context 'when cannot rebase' do
+ context 'when rebase in progress' do
+ let(:rebase_in_progress) { true }
+ let(:can_push_to_branch) { true }
+ let(:should_be_rebased) { true }
+
+ it 'returns nil' do
+ is_expected.to be_nil
+ end
+ end
+
+ context 'when user cannot merge' do
+ let(:rebase_in_progress) { false }
+ let(:can_push_to_branch) { false }
+ let(:should_be_rebased) { true }
+
+ it 'returns nil' do
+ is_expected.to be_nil
+ end
+ end
+
+ context 'should not be rebased' do
+ let(:rebase_in_progress) { false }
+ let(:can_push_to_branch) { true }
+ let(:should_be_rebased) { false }
+
+ it 'returns nil' do
+ is_expected.to be_nil
+ end
+ end
+ end
+ end
end
diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index e25552eb0d83c3475aa1d945a5e40806ce65e68c..80a271ba7fbc317b7e92cd51e79053a529f2cd02 100644
--- a/spec/serializers/merge_request_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -190,4 +190,20 @@ describe MergeRequestWidgetEntity do
end
end
end
+
+ describe 'when source project is deleted' do
+ let(:project) { create(:project, :repository) }
+ let(:fork_project) { create(:project, :repository, forked_from_project: project) }
+ let(:merge_request) { create(:merge_request, source_project: fork_project, target_project: project) }
+
+ it 'returns a blank rebase_path' do
+ allow(merge_request).to receive(:should_be_rebased?).and_return(true)
+ fork_project.destroy
+ merge_request.reload
+
+ entity = described_class.new(merge_request, request: request).as_json
+
+ expect(entity[:rebase_path]).to be_nil
+ end
+ end
end
diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..d1b37cdd073687348d4eac67072ced776023ef0f
--- /dev/null
+++ b/spec/services/merge_requests/rebase_service_spec.rb
@@ -0,0 +1,134 @@
+require 'spec_helper'
+
+describe MergeRequests::RebaseService do
+ include ProjectForksHelper
+
+ let(:user) { create(:user) }
+ let(:merge_request) do
+ create(:merge_request,
+ source_branch: 'feature_conflict',
+ target_branch: 'master')
+ end
+ let(:project) { merge_request.project }
+ let(:repository) { project.repository.raw }
+
+ subject(:service) { described_class.new(project, user, {}) }
+
+ before do
+ project.add_master(user)
+ end
+
+ describe '#execute' do
+ context 'when another rebase is already in progress' do
+ before do
+ allow(merge_request).to receive(:rebase_in_progress?).and_return(true)
+ end
+
+ it 'saves the error message' do
+ subject.execute(merge_request)
+
+ expect(merge_request.reload.merge_error).to eq 'Rebase task canceled: Another rebase is already in progress'
+ end
+
+ it 'returns an error' do
+ expect(service.execute(merge_request)).to match(status: :error,
+ message: 'Failed to rebase. Should be done manually')
+ end
+ end
+
+ context 'when unexpected error occurs' do
+ before do
+ allow(repository).to receive(:run_git!).and_raise('Something went wrong')
+ end
+
+ it 'saves the error message' do
+ subject.execute(merge_request)
+
+ expect(merge_request.reload.merge_error).to eq 'Something went wrong'
+ end
+
+ it 'returns an error' do
+ expect(service.execute(merge_request)).to match(status: :error,
+ message: 'Failed to rebase. Should be done manually')
+ end
+ end
+
+ context 'with git command failure' do
+ before do
+ allow(repository).to receive(:run_git!).and_raise(Gitlab::Git::Repository::GitError, 'Something went wrong')
+ end
+
+ it 'saves the error message' do
+ subject.execute(merge_request)
+
+ expect(merge_request.reload.merge_error).to eq 'Something went wrong'
+ end
+
+ it 'returns an error' do
+ expect(service.execute(merge_request)).to match(status: :error,
+ message: 'Failed to rebase. Should be done manually')
+ end
+ end
+
+ context 'valid params' do
+ before do
+ service.execute(merge_request)
+ end
+
+ it 'rebases source branch' do
+ parent_sha = merge_request.source_project.repository.commit(merge_request.source_branch).parents.first.sha
+ target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha
+ expect(parent_sha).to eq(target_branch_sha)
+ end
+
+ it 'records the new SHA on the merge request' do
+ head_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha
+ expect(merge_request.reload.rebase_commit_sha).to eq(head_sha)
+ end
+
+ it 'logs correct author and commiter' do
+ head_commit = merge_request.source_project.repository.commit(merge_request.source_branch)
+
+ expect(head_commit.author_email).to eq('dmitriy.zaporozhets@gmail.com')
+ expect(head_commit.author_name).to eq('Dmitriy Zaporozhets')
+ expect(head_commit.committer_email).to eq(user.email)
+ expect(head_commit.committer_name).to eq(user.name)
+ end
+
+ context 'git commands' do
+ it 'sets GL_REPOSITORY env variable when calling git commands' do
+ expect(repository).to receive(:popen).exactly(3)
+ .with(anything, anything, hash_including('GL_REPOSITORY'))
+ .and_return(['', 0])
+
+ service.execute(merge_request)
+ end
+ end
+
+ context 'fork' do
+ let(:forked_project) do
+ fork_project(project, user, repository: true)
+ end
+
+ let(:merge_request_from_fork) do
+ forked_project.repository.create_file(
+ user,
+ 'new-file-to-target',
+ '',
+ message: 'Add new file to target',
+ branch_name: 'master')
+
+ create(:merge_request,
+ source_branch: 'master', source_project: forked_project,
+ target_branch: 'master', target_project: project)
+ end
+
+ it 'rebases source branch' do
+ parent_sha = forked_project.repository.commit(merge_request_from_fork.source_branch).parents.first.sha
+ target_branch_sha = project.repository.commit(merge_request_from_fork.target_branch).sha
+ expect(parent_sha).to eq(target_branch_sha)
+ end
+ end
+ end
+ end
+end
diff --git a/spec/views/projects/merge_requests/show.html.haml_spec.rb b/spec/views/projects/merge_requests/show.html.haml_spec.rb
index 28d54c2fb77a317656658b52ce6d34b27b00e934..264e0ce0b4067e1ceb66e6648d04992e2dd06ddf 100644
--- a/spec/views/projects/merge_requests/show.html.haml_spec.rb
+++ b/spec/views/projects/merge_requests/show.html.haml_spec.rb
@@ -54,6 +54,8 @@ describe 'projects/merge_requests/show.html.haml' do
it 'closes the merge request if the source project does not exist' do
closed_merge_request.update_attributes(state: 'open')
forked_project.destroy
+ # Reload merge request so MergeRequest#source_project turns to `nil`
+ closed_merge_request.reload
render
diff --git a/spec/workers/rebase_worker_spec.rb b/spec/workers/rebase_worker_spec.rb
new file mode 100644
index 0000000000000000000000000000000000000000..20aff020dbb64322e80288d555b2755bae3dfdd6
--- /dev/null
+++ b/spec/workers/rebase_worker_spec.rb
@@ -0,0 +1,27 @@
+require 'spec_helper'
+
+describe RebaseWorker, '#perform' do
+ context 'when rebasing an MR from a fork where upstream has protected branches' do
+ let(:upstream_project) { create(:project, :repository) }
+ let(:fork_project) { create(:project, :repository) }
+
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: fork_project,
+ source_branch: 'feature_conflict',
+ target_project: upstream_project,
+ target_branch: 'master')
+ end
+
+ before do
+ create(:forked_project_link, forked_to_project: fork_project, forked_from_project: upstream_project)
+ end
+
+ it 'sets the correct project for running hooks' do
+ expect(MergeRequests::RebaseService)
+ .to receive(:new).with(fork_project, merge_request.author).and_call_original
+
+ subject.perform(merge_request, merge_request.author)
+ end
+ end
+end