Commit 71280599 authored by Jan Provaznik's avatar Jan Provaznik

Backport 'Rebase' feature from EE to CE

When a project uses fast-forward merging strategy user has
to rebase MRs to target branch before it can be merged.
Now user can do rebase in UI by clicking 'Rebase' button
instead of doing rebase locally.

This feature was already present in EE, this is only backport
of the feature to CE.

Rebase license check is now removed.

Related to !16071
Related to #40301
parent 81d9e72b
<script>
import simplePoll from '../../../lib/utils/simple_poll';
import eventHub from '../../event_hub';
import statusIcon from '../mr_widget_status_icon';
import loadingIcon from '../../../vue_shared/components/loading_icon.vue';
import Flash from '../../../flash';
export default {
name: 'MRWidgetRebase',
props: {
mr: {
type: Object,
required: true,
},
service: {
type: Object,
required: true,
},
},
components: {
statusIcon,
loadingIcon,
},
data() {
return {
isMakingRequest: false,
rebasingError: null,
};
},
computed: {
status() {
if (this.mr.rebaseInProgress || this.isMakingRequest) {
return 'loading';
}
if (!this.mr.canPushToSourceBranch && !this.mr.rebaseInProgress) {
return 'warning';
}
return 'success';
},
showDisabledButton() {
return ['failed', 'loading'].includes(this.status);
},
},
methods: {
rebase() {
this.isMakingRequest = true;
this.rebasingError = null;
this.service.rebase()
.then(() => {
simplePoll(this.checkRebaseStatus);
})
.catch((error) => {
this.rebasingError = error.merge_error;
this.isMakingRequest = false;
Flash('Something went wrong. Please try again.');
});
},
checkRebaseStatus(continuePolling, stopPolling) {
this.service.poll()
.then(res => res.data)
.then((res) => {
if (res.rebase_in_progress) {
continuePolling();
} else {
this.isMakingRequest = false;
if (res.merge_error && res.merge_error.length) {
this.rebasingError = res.merge_error;
Flash('Something went wrong. Please try again.');
}
eventHub.$emit('MRWidgetUpdateRequested');
stopPolling();
}
})
.catch(() => {
this.isMakingRequest = false;
Flash('Something went wrong. Please try again.');
stopPolling();
});
},
},
};
</script>
<template>
<div class="mr-widget-body media">
<status-icon
:status="status"
:show-disabled-button="showDisabledButton"
/>
<div class="rebase-state-find-class-convention media media-body space-children">
<template v-if="mr.rebaseInProgress || isMakingRequest">
<span class="bold">
Rebase in progress
</span>
</template>
<template v-if="!mr.rebaseInProgress && !mr.canPushToSourceBranch">
<span class="bold">
Fast-forward merge is not possible.
Rebase the source branch onto
<span class="label-branch">{{mr.targetBranch}}</span>
to allow this merge request to be merged.
</span>
</template>
<template v-if="!mr.rebaseInProgress && mr.canPushToSourceBranch && !isMakingRequest">
<div class="accept-merge-holder clearfix js-toggle-container accept-action media space-children">
<button
type="button"
class="btn btn-sm btn-reopen btn-success"
:disabled="isMakingRequest"
@click="rebase">
<loading-icon v-if="isMakingRequest" />
Rebase
</button>
<span
v-if="!rebasingError"
class="bold">
Fast-forward merge is not possible.
Rebase the source branch onto the target branch or merge target
branch into source branch to allow this merge request to be merged.
</span>
<span
v-else
class="bold danger">
{{rebasingError}}
</span>
</div>
</template>
</div>
</div>
</template>
......@@ -32,6 +32,7 @@ export { default as UnresolvedDiscussionsState } from './components/states/mr_wi
export { default as PipelineBlockedState } from './components/states/mr_widget_pipeline_blocked';
export { default as PipelineFailedState } from './components/states/mr_widget_pipeline_failed';
export { default as MergeWhenPipelineSucceedsState } from './components/states/mr_widget_merge_when_pipeline_succeeds';
export { default as RebaseState } from './components/states/mr_widget_rebase.vue';
export { default as AutoMergeFailed } from './components/states/mr_widget_auto_merge_failed';
export { default as CheckingState } from './components/states/mr_widget_checking';
export { default as MRWidgetStore } from 'ee/vue_merge_request_widget/stores/mr_widget_store';
......
......@@ -10,6 +10,7 @@ import {
MergedState,
ClosedState,
MergingState,
RebaseState,
WipState,
ArchivedState,
ConflictsState,
......@@ -230,6 +231,7 @@ export default {
'mr-widget-pipeline-failed': PipelineFailedState,
'mr-widget-merge-when-pipeline-succeeds': MergeWhenPipelineSucceedsState,
'mr-widget-auto-merge-failed': AutoMergeFailed,
'mr-widget-rebase': RebaseState,
},
template: `
<div class="mr-state-widget prepend-top-default">
......
......@@ -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);
}
......
......@@ -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;
}
......
......@@ -28,6 +28,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;
......@@ -126,6 +127,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 {};
......
......@@ -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 {
......
......@@ -12,6 +12,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
......@@ -226,6 +227,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
......@@ -331,4 +338,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
......@@ -161,6 +161,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
......
......@@ -1154,6 +1154,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
......
......@@ -22,6 +22,7 @@ class MergeRequestWidgetEntity < IssuableEntity
# EE-specific
expose :approvals_before_merge
expose :squash
expose :rebase_commit_sha
expose :rebase_in_progress?, as: :rebase_in_progress
......
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
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
......@@ -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.
......@@ -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.
......@@ -89,6 +89,7 @@
- project_service
- propagate_service_template
- reactive_caching
- rebase
- repository_fork
- repository_import
- storage_migrator
......
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
......@@ -105,6 +105,7 @@ constraints(ProjectUrlConstrainer.new) do
post :remove_wip
post :assign_related_issues
post :rebase
scope constraints: { format: nil }, action: :show do
get :commits, defaults: { tab: 'commits' }
......
......@@ -685,4 +685,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
......@@ -105,14 +105,14 @@
"merge_ongoing": { "type": "boolean" },
"ff_only_enabled": { "type": ["boolean", false] },
"should_be_rebased": { "type": "boolean" },
// EE-specific
"rebase_commit_sha": { "type": ["string", "null"] },
"approvals_before_merge": { "type": ["integer", "null"] },
"squash": { "type": "boolean" },
"rebase_in_progress": { "type": "boolean" },
"can_push_to_source_branch": { "type": "boolean" },
"rebase_path": { "type": ["string", "null"] },
// EE-specific
"approvals_before_merge": { "type": ["integer", "null"] },
"squash": { "type": "boolean" },
"approved": { "type": "boolean" },
"approvals_path": { "type": ["string", "null"] },
"codeclimate": {
......
import Vue from 'vue';
import eventHub from '~/vue_merge_request_widget/event_hub';
import component from 'ee/vue_merge_request_widget/components/states/mr_widget_rebase.vue';
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', () => {
......
......@@ -2346,4 +2346,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
......@@ -536,4 +536,67 @@ describe MergeRequestPresenter do
.to eq("<a href=\"/#{resource.source_project.full_path}/tree/#{resource.source_branch}\">#{resource.source_branch}</a>")
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment