Commit 6eeb69fc authored by Filipa Lacerda's avatar Filipa Lacerda

Merge branch 'jprovazn-rebase' into 'master'

Backport 'Rebase' feature from EE to CE

Closes #40301

See merge request gitlab-org/gitlab-ce!16071
parents 7d6a0a21 27a75ea1
<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 './stores/mr_widget_store';
......
......@@ -10,6 +10,7 @@ import {
MergedState,
ClosedState,
MergingState,
RebaseState,
WipState,
ArchivedState,
ConflictsState,
......@@ -79,6 +80,7 @@ export default {
ciEnvironmentsStatusPath: store.ciEnvironmentsStatusPath,
statusPath: store.statusPath,
mergeActionsContentPath: store.mergeActionsContentPath,
rebasePath: store.rebasePath,
};
return new MRWidgetService(endpoints);
},
......@@ -232,6 +234,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;
}
......
......@@ -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 {};
......
......@@ -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 {
......
......@@ -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
......@@ -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)
......
......@@ -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
......
......@@ -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))
......
......@@ -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
......@@ -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
......
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
---
title: Allow user to rebase merge requests.
merge_request:
author:
type: added
......@@ -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' }
......
class AddRebaseCommitShaToMergeRequests < ActiveRecord::Migration
DOWNTIME = false
def change
add_column :merge_requests, :rebase_commit_sha, :string
end
end
......@@ -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
......
......@@ -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**.
![Fast forward merge request](img/ff_merge_mr.png)
![Fast forward merge request](img/ff_merge_rebase.png)
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.
......
......@@ -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
......@@ -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
......
......@@ -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
......@@ -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" }
......
......@@ -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
}
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);
});
});
});
......@@ -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
......@@ -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
......
......@@ -404,4 +404,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
......@@ -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
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
......@@ -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
......
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
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