Commit 0ac06c89 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'ff_port_from_ee' into 'master'

Move Fast-Forward Merge to CE

See merge request gitlab-org/gitlab-ce!14272
parents 75f257ad e559b851
...@@ -27,7 +27,7 @@ export default { ...@@ -27,7 +27,7 @@ export default {
<button <button
v-if="showDisabledButton" v-if="showDisabledButton"
type="button" type="button"
class="btn btn-success btn-sm" class="js-disabled-merge-button btn btn-success btn-sm"
disabled="true"> disabled="true">
Merge Merge
</button> </button>
......
...@@ -10,8 +10,17 @@ export default { ...@@ -10,8 +10,17 @@ export default {
}, },
template: ` template: `
<div class="mr-widget-body media"> <div class="mr-widget-body media">
<status-icon status="failed" showDisabledButton /> <status-icon
status="failed"
showDisabledButton />
<div class="media-body space-children"> <div class="media-body space-children">
<span
v-if="mr.shouldBeRebased"
class="bold">
Fast-forward merge is not possible.
To merge this request, first rebase locally.
</span>
<template v-else>
<span class="bold"> <span class="bold">
There are merge conflicts<span v-if="!mr.canMerge">.</span> There are merge conflicts<span v-if="!mr.canMerge">.</span>
<span v-if="!mr.canMerge"> <span v-if="!mr.canMerge">
...@@ -21,16 +30,17 @@ export default { ...@@ -21,16 +30,17 @@ export default {
<a <a
v-if="mr.canMerge && mr.conflictResolutionPath" v-if="mr.canMerge && mr.conflictResolutionPath"
:href="mr.conflictResolutionPath" :href="mr.conflictResolutionPath"
class="btn btn-default btn-xs js-resolve-conflicts-button"> class="js-resolve-conflicts-button btn btn-default btn-xs">
Resolve conflicts Resolve conflicts
</a> </a>
<a <a
v-if="mr.canMerge" v-if="mr.canMerge"
class="btn btn-default btn-xs js-merge-locally-button" class="js-merge-locally-button btn btn-default btn-xs"
data-toggle="modal" data-toggle="modal"
href="#modal_merge_info"> href="#modal_merge_info">
Merge locally Merge locally
</a> </a>
</template>
</div> </div>
</div> </div>
`, `,
......
...@@ -284,10 +284,16 @@ export default { ...@@ -284,10 +284,16 @@ export default {
:mr="mr" :mr="mr"
:is-merge-button-disabled="isMergeButtonDisabled" /> :is-merge-button-disabled="isMergeButtonDisabled" />
<span
v-if="mr.ffOnlyEnabled"
class="js-fast-forward-message">
Fast-forward merge without a merge commit
</span>
<button <button
v-else
@click="toggleCommitMessageEditor" @click="toggleCommitMessageEditor"
:disabled="isMergeButtonDisabled" :disabled="isMergeButtonDisabled"
class="btn btn-default btn-xs" class="js-modify-commit-message-button btn btn-default btn-xs"
type="button"> type="button">
Modify commit message Modify commit message
</button> </button>
......
...@@ -57,6 +57,8 @@ export default class MergeRequestStore { ...@@ -57,6 +57,8 @@ export default class MergeRequestStore {
this.onlyAllowMergeIfPipelineSucceeds = data.only_allow_merge_if_pipeline_succeeds || false; this.onlyAllowMergeIfPipelineSucceeds = data.only_allow_merge_if_pipeline_succeeds || false;
this.mergeWhenPipelineSucceeds = data.merge_when_pipeline_succeeds || false; this.mergeWhenPipelineSucceeds = data.merge_when_pipeline_succeeds || false;
this.mergePath = data.merge_path; this.mergePath = data.merge_path;
this.ffOnlyEnabled = data.ff_only_enabled;
this.shouldBeRebased = !!data.should_be_rebased;
this.statusPath = data.status_path; this.statusPath = data.status_path;
this.emailPatchesPath = data.email_patches_path; this.emailPatchesPath = data.email_patches_path;
this.plainDiffPath = data.plain_diff_path; this.plainDiffPath = data.plain_diff_path;
......
...@@ -344,6 +344,7 @@ class ProjectsController < Projects::ApplicationController ...@@ -344,6 +344,7 @@ class ProjectsController < Projects::ApplicationController
:tag_list, :tag_list,
:visibility_level, :visibility_level,
:template_name, :template_name,
:merge_method,
project_feature_attributes: %i[ project_feature_attributes: %i[
builds_access_level builds_access_level
......
...@@ -524,6 +524,14 @@ class MergeRequest < ActiveRecord::Base ...@@ -524,6 +524,14 @@ class MergeRequest < ActiveRecord::Base
true true
end end
def ff_merge_possible?
project.repository.ancestor?(target_branch_sha, diff_head_sha)
end
def should_be_rebased?
project.ff_merge_must_be_possible? && !ff_merge_possible?
end
def can_cancel_merge_when_pipeline_succeeds?(current_user) def can_cancel_merge_when_pipeline_succeeds?(current_user)
can_be_merged_by?(current_user) || self.author == current_user can_be_merged_by?(current_user) || self.author == current_user
end end
......
...@@ -1566,6 +1566,34 @@ class Project < ActiveRecord::Base ...@@ -1566,6 +1566,34 @@ class Project < ActiveRecord::Base
persisted? && path_changed? persisted? && path_changed?
end end
def merge_method
if self.merge_requests_ff_only_enabled
:ff
elsif self.merge_requests_rebase_enabled
:rebase_merge
else
:merge
end
end
def merge_method=(method)
case method.to_s
when "ff"
self.merge_requests_ff_only_enabled = true
self.merge_requests_rebase_enabled = true
when "rebase_merge"
self.merge_requests_ff_only_enabled = false
self.merge_requests_rebase_enabled = true
when "merge"
self.merge_requests_ff_only_enabled = false
self.merge_requests_rebase_enabled = false
end
end
def ff_merge_must_be_possible?
self.merge_requests_ff_only_enabled || self.merge_requests_rebase_enabled
end
def migrate_to_hashed_storage! def migrate_to_hashed_storage!
return if hashed_storage? return if hashed_storage?
......
...@@ -850,6 +850,25 @@ class Repository ...@@ -850,6 +850,25 @@ class Repository
end end
end end
def ff_merge(user, source, target_branch, merge_request: nil)
our_commit = rugged.branches[target_branch].target
their_commit =
if source.is_a?(Gitlab::Git::Commit)
source.raw_commit
else
rugged.lookup(source)
end
raise 'Invalid merge target' if our_commit.nil?
raise 'Invalid merge source' if their_commit.nil?
with_branch(user, target_branch) do |start_commit|
merge_request&.update(in_progress_merge_commit_sha: their_commit.oid)
their_commit.oid
end
end
def revert( def revert(
user, commit, branch_name, message, user, commit, branch_name, message,
start_branch_name: nil, start_project: project) start_branch_name: nil, start_project: project)
......
...@@ -13,6 +13,11 @@ class MergeRequestEntity < IssuableEntity ...@@ -13,6 +13,11 @@ class MergeRequestEntity < IssuableEntity
expose :target_branch expose :target_branch
expose :target_project_id expose :target_project_id
expose :should_be_rebased?, as: :should_be_rebased
expose :ff_only_enabled do |merge_request|
merge_request.project.merge_requests_ff_only_enabled
end
# Events # Events
expose :merge_event, using: EventEntity expose :merge_event, using: EventEntity
expose :closed_event, using: EventEntity expose :closed_event, using: EventEntity
......
module MergeRequests
# MergeService class
#
# Do git fast-forward merge and in case of success
# mark merge request as merged and execute all hooks and notifications
# Executed when you do fast-forward merge via GitLab UI
#
class FfMergeService < MergeRequests::MergeService
private
def commit
repository.ff_merge(current_user,
source,
merge_request.target_branch,
merge_request: merge_request)
rescue Gitlab::Git::HooksService::PreReceiveError => e
raise MergeError, e.message
rescue StandardError => e
raise MergeError, "Something went wrong during merge: #{e.message}"
ensure
merge_request.update(in_progress_merge_commit_sha: nil)
end
end
end
...@@ -11,6 +11,11 @@ module MergeRequests ...@@ -11,6 +11,11 @@ module MergeRequests
attr_reader :merge_request, :source attr_reader :merge_request, :source
def execute(merge_request) def execute(merge_request)
if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService)
FfMergeService.new(project, current_user, params).execute(merge_request)
return
end
@merge_request = merge_request @merge_request = merge_request
unless @merge_request.mergeable? unless @merge_request.mergeable?
......
- form = local_assigns.fetch(:form)
- project = local_assigns.fetch(:project)
.radio
= label_tag :project_merge_method_ff do
= form.radio_button :merge_method, :ff, class: "js-merge-method-radio"
%strong Fast-forward merge
%br
%span.descr
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.
- form = local_assigns.fetch(:form)
.radio
= label_tag :project_merge_method_rebase_merge do
= form.radio_button :merge_method, :rebase_merge, class: "js-merge-method-radio"
%strong Merge commit with semi-linear history
%br
%span.descr
A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible.
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.
- form = local_assigns.fetch(:form) - form = local_assigns.fetch(:form)
.form-group
= label_tag :merge_method_merge, class: 'label-light' do
Merge method
.radio
= label_tag :project_merge_method_merge do
= form.radio_button :merge_method, :merge, class: "js-merge-method-radio"
%strong Merge commit
%br
%span.descr
A merge commit is created for every merge, and merging is allowed as long as there are no conflicts.
= render 'merge_request_rebase_settings', form: form
= render 'merge_request_fast_forward_settings', project: @project, form: form
= render 'projects/merge_request_merge_settings', form: form = render 'projects/merge_request_merge_settings', form: form
---
title: Move Custom merge methods from EE
merge_request:
author:
type: added
# rubocop:disable all
class AddMergeRequestRebaseEnabledToProjects < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
def up
add_column_with_default(:projects, :merge_requests_rebase_enabled, :boolean, default: false)
end
def down
remove_column(:projects, :merge_requests_rebase_enabled)
end
end
# rubocop:disable all
class AddFastForwardOptionToProject < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
# Set this constant to true if this migration requires downtime.
DOWNTIME = false
disable_ddl_transaction!
def add
add_column_with_default(:projects, :merge_requests_ff_only_enabled, :boolean, default: false)
end
def down
remove_column(:projects, :merge_requests_ff_only_enabled)
end
end
...@@ -1217,6 +1217,8 @@ ActiveRecord::Schema.define(version: 20170928100231) do ...@@ -1217,6 +1217,8 @@ ActiveRecord::Schema.define(version: 20170928100231) do
t.integer "storage_version", limit: 2 t.integer "storage_version", limit: 2
t.boolean "resolve_outdated_diff_discussions" t.boolean "resolve_outdated_diff_discussions"
t.boolean "repository_read_only" t.boolean "repository_read_only"
t.boolean "merge_requests_ff_only_enabled", default: false
t.boolean "merge_requests_rebase_enabled", default: false, null: false
end end
add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree add_index "projects", ["ci_id"], name: "index_projects_on_ci_id", using: :btree
......
# Fast-forward merge requests
Retain a linear Git history and a way to accept merge requests without
creating merge commits.
## Overview
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.
## Use cases
Sometimes, a workflow policy might mandate a clean commit history without
merge commits. In such cases, the fast-forward merge is the perfect candidate.
## Enabling fast-forward merges
1. Navigate to your project's **Settings** and search for the 'Merge method'
1. Select the **Fast-forward merge** option
1. Hit **Save changes** for the changes to take effect
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)
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.
![Fast forward merge rebase locally](img/ff_merge_rebase_locally.png)
[ffonly]: https://git-scm.com/docs/git-merge#git-merge---ff-only
...@@ -23,12 +23,14 @@ With GitLab merge requests, you can: ...@@ -23,12 +23,14 @@ With GitLab merge requests, you can:
- Organize your issues and merge requests consistently throughout the project with [labels](../../project/labels.md) - Organize your issues and merge requests consistently throughout the project with [labels](../../project/labels.md)
- Add a time estimation and the time spent with that merge request with [Time Tracking](../../../workflow/time_tracking.html#time-tracking) - Add a time estimation and the time spent with that merge request with [Time Tracking](../../../workflow/time_tracking.html#time-tracking)
- [Resolve merge conflicts from the UI](#resolve-conflicts) - [Resolve merge conflicts from the UI](#resolve-conflicts)
- Enable [fast-forward merge requests](#fast-forward-merge-requests)
- Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch
With **[GitLab Enterprise Edition][ee]**, you can also: With **[GitLab Enterprise Edition][ee]**, you can also:
- View the deployment process across projects with [Multi-Project Pipeline Graphs](https://docs.gitlab.com/ee/ci/multi_project_pipeline_graphs.html#multi-project-pipeline-graphs) (available only in GitLab Enterprise Edition Premium) - View the deployment process across projects with [Multi-Project Pipeline Graphs](https://docs.gitlab.com/ee/ci/multi_project_pipeline_graphs.html#multi-project-pipeline-graphs) (available only in GitLab Enterprise Edition Premium)
- Request [approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html) from your managers (available in GitLab Enterprise Edition Starter) - Request [approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html) from your managers (available in GitLab Enterprise Edition Starter)
- Enable [fast-forward merge requests](https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html) (available in GitLab Enterprise Edition Starter)
- [Squash and merge](https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html) for a cleaner commit history (available in GitLab Enterprise Edition Starter) - [Squash and merge](https://docs.gitlab.com/ee/user/project/merge_requests/squash_and_merge.html) for a cleaner commit history (available in GitLab Enterprise Edition Starter)
- Enable [semi-linear history merge requests](https://docs.gitlab.com/ee/user/project/merge_requests/index.html#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch (available in GitLab Enterprise Edition Starter) - Enable [semi-linear history merge requests](https://docs.gitlab.com/ee/user/project/merge_requests/index.html#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch (available in GitLab Enterprise Edition Starter)
- Analise the impact of your changes with [Code Quality reports](https://docs.gitlab.com/ee/user/project/merge_requests/code_quality_diff.html) (available in GitLab Enterprise Edition Starter) - Analise the impact of your changes with [Code Quality reports](https://docs.gitlab.com/ee/user/project/merge_requests/code_quality_diff.html) (available in GitLab Enterprise Edition Starter)
...@@ -89,6 +91,22 @@ in a merged merge requests or a commit. ...@@ -89,6 +91,22 @@ in a merged merge requests or a commit.
[Learn more about cherry-picking changes.](cherry_pick_changes.md) [Learn more about cherry-picking changes.](cherry_pick_changes.md)
## Semi-linear history merge requests
A merge commit is created for every merge, but the branch is only merged if
a fast-forward merge is possible. This ensures that if the merge request build
succeeded, the target branch build will also succeed after merging.
Navigate to a project's settings, select the **Merge commit with semi-linear
history** option under **Merge Requests: Merge method** and save your changes.
## Fast-forward merge requests
If you prefer a linear Git history and a way to accept merge requests without
creating merge commits, you can configure this on a per-project basis.
[Read more about fast-forward merge requests.](fast_forward_merge.md)
## Merge when pipeline succeeds ## Merge when pipeline succeeds
When reviewing a merge request that looks ready to merge but still has one or When reviewing a merge request that looks ready to merge but still has one or
......
...@@ -23,7 +23,7 @@ Add an [issue description template](../description_templates.md#description-temp ...@@ -23,7 +23,7 @@ Add an [issue description template](../description_templates.md#description-temp
Set up your project's merge request settings: Set up your project's merge request settings:
- Set up the merge request method (merge commit, [fast-forward merge](https://docs.gitlab.com/ee/user/project/merge_requests/fast_forward_merge.html#fast-forward-merge-requests)). _Fast-forward is available in [GitLab Enterprise Edition Starter](https://about.gitlab.com/gitlab-ee/)._ - Set up the merge request method (merge commit, [fast-forward merge](../merge_requests/fast_forward_merge.html)).
- Merge request [description templates](../description_templates.md#description-templates). - Merge request [description templates](../description_templates.md#description-templates).
- Enable [merge request approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html#merge-request-approvals), _available in [GitLab Enterprise Edition Starter](https://about.gitlab.com/gitlab-ee/)_. - Enable [merge request approvals](https://docs.gitlab.com/ee/user/project/merge_requests/merge_request_approvals.html#merge-request-approvals), _available in [GitLab Enterprise Edition Starter](https://about.gitlab.com/gitlab-ee/)_.
- Enable [merge only of pipeline succeeds](../merge_requests/merge_when_pipeline_succeeds.md). - Enable [merge only of pipeline succeeds](../merge_requests/merge_when_pipeline_succeeds.md).
......
...@@ -36,6 +36,7 @@ ...@@ -36,6 +36,7 @@
- [Revert changes in the UI](../user/project/merge_requests/revert_changes.md) - [Revert changes in the UI](../user/project/merge_requests/revert_changes.md)
- [Merge requests versions](../user/project/merge_requests/versions.md) - [Merge requests versions](../user/project/merge_requests/versions.md)
- ["Work In Progress" merge requests](../user/project/merge_requests/work_in_progress_merge_requests.md) - ["Work In Progress" merge requests](../user/project/merge_requests/work_in_progress_merge_requests.md)
- [Fast-forward merge requests](../user/project/merge_requests/fast_forward_merge.md)
- [Manage large binaries with Git LFS](lfs/manage_large_binaries_with_git_lfs.md) - [Manage large binaries with Git LFS](lfs/manage_large_binaries_with_git_lfs.md)
- [Importing from SVN, GitHub, Bitbucket, etc](importing/README.md) - [Importing from SVN, GitHub, Bitbucket, etc](importing/README.md)
- [Todos](todos.md) - [Todos](todos.md)
......
Feature: Project Ff Merge Requests
Background:
Given I sign in as a user
And I own project "Shop"
And project "Shop" have "Bug NS-05" open merge request with diffs inside
And merge request "Bug NS-05" is mergeable
@javascript
Scenario: I do ff-only merge for rebased branch
Given ff merge enabled
And merge request "Bug NS-05" is rebased
When I visit merge request page "Bug NS-05"
Then I should see ff-only merge button
When I accept this merge request
Then I should see merged request
@javascript
Scenario: I do ff-only merge for merged branch
Given ff merge enabled
And merge request "Bug NS-05" merged target
When I visit merge request page "Bug NS-05"
Then I should see ff-only merge button
When I accept this merge request
Then I should see merged request
class Spinach::Features::ProjectFfMergeRequests < Spinach::FeatureSteps
include SharedAuthentication
include SharedIssuable
include SharedProject
include SharedNote
include SharedPaths
include SharedMarkdown
include SharedDiffNote
include SharedUser
include WaitForRequests
step 'project "Shop" have "Bug NS-05" open merge request with diffs inside' do
create(:merge_request_with_diffs,
title: "Bug NS-05",
source_project: project,
target_project: project,
author: project.users.first)
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'
end
step 'merge request "Bug NS-05" is mergeable' do
merge_request.mark_as_mergeable
end
step 'I accept this merge request' do
page.within '.mr-state-widget' do
click_button "Merge"
end
end
step 'I should see merged request' do
page.within '.status-box' do
expect(page).to have_content "Merged"
wait_for_requests
end
end
step 'ff merge enabled' do
project = merge_request.target_project
project.merge_requests_ff_only_enabled = true
project.save!
end
step 'merge request "Bug NS-05" is rebased' do
merge_request.source_branch = 'flatten-dir'
merge_request.target_branch = 'improve/awesome'
merge_request.reload_diff
merge_request.save!
end
step 'merge request "Bug NS-05" merged target' do
merge_request.source_branch = 'merged-target'
merge_request.target_branch = 'improve/awesome'
merge_request.reload_diff
merge_request.save!
end
def merge_request
@merge_request ||= MergeRequest.find_by!(title: "Bug NS-05")
end
end
...@@ -289,6 +289,24 @@ describe ProjectsController do ...@@ -289,6 +289,24 @@ describe ProjectsController do
end end
end end
it 'updates Fast Forward Merge attributes' do
controller.instance_variable_set(:@project, project)
params = {
merge_method: :ff
}
put :update,
namespace_id: project.namespace,
id: project.id,
project: params
expect(response).to have_http_status(302)
params.each do |param, value|
expect(project.public_send(param)).to eq(value)
end
end
def update_project(**parameters) def update_project(**parameters)
put :update, put :update,
namespace_id: project.namespace.path, namespace_id: project.namespace.path,
......
...@@ -202,6 +202,28 @@ describe 'Merge request', :js do ...@@ -202,6 +202,28 @@ describe 'Merge request', :js do
end end
end end
context 'view merge request where fast-forward merge is not possible' do
before do
project.update(merge_requests_ff_only_enabled: true)
merge_request.update(
merge_user: merge_request.author,
merge_status: :cannot_be_merged
)
visit project_merge_request_path(project, merge_request)
end
it 'shows information about the merge error' do
# Wait for the `ci_status` and `merge_check` requests
wait_for_requests
page.within('.mr-widget-body') do
expect(page).to have_content('Fast-forward merge is not possible')
end
end
end
context 'merge error' do context 'merge error' do
before do before do
allow_any_instance_of(Repository).to receive(:merge).and_return(false) allow_any_instance_of(Repository).to receive(:merge).and_return(false)
......
...@@ -32,6 +32,32 @@ describe 'Edit Project Settings' do ...@@ -32,6 +32,32 @@ describe 'Edit Project Settings' do
end end
end end
describe 'Merge request settings section' do
it 'shows "Merge commit" strategy' do
visit edit_project_path(project)
page.within '.merge-requests-feature' do
expect(page).to have_content 'Merge commit'
end
end
it 'shows "Merge commit with semi-linear history " strategy' do
visit edit_project_path(project)
page.within '.merge-requests-feature' do
expect(page).to have_content 'Merge commit with semi-linear history'
end
end
it 'shows "Fast-forward merge" strategy' do
visit edit_project_path(project)
page.within '.merge-requests-feature' do
expect(page).to have_content 'Fast-forward merge'
end
end
end
describe 'Rename repository section' do describe 'Rename repository section' do
context 'with invalid characters' do context 'with invalid characters' do
it 'shows errors for invalid project path/name' do it 'shows errors for invalid project path/name' do
......
...@@ -96,7 +96,9 @@ ...@@ -96,7 +96,9 @@
"remove_wip_path": { "type": "string" }, "remove_wip_path": { "type": "string" },
"commits_count": { "type": "integer" }, "commits_count": { "type": "integer" },
"remove_source_branch": { "type": ["boolean", "null"] }, "remove_source_branch": { "type": ["boolean", "null"] },
"merge_ongoing": { "type": "boolean" } "merge_ongoing": { "type": "boolean" },
"ff_only_enabled": { "type": ["boolean", false] },
"should_be_rebased": { "type": "boolean" }
}, },
"additionalProperties": false "additionalProperties": false
} }
import Vue from 'vue'; import Vue from 'vue';
import conflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts'; import conflictsComponent from '~/vue_merge_request_widget/components/states/mr_widget_conflicts';
import mountComponent from '../../../helpers/vue_mount_component_helper';
const ConflictsComponent = Vue.extend(conflictsComponent);
const path = '/conflicts'; const path = '/conflicts';
const createComponent = () => {
const Component = Vue.extend(conflictsComponent);
return new Component({
el: document.createElement('div'),
propsData: {
mr: {
canMerge: true,
conflictResolutionPath: path,
},
},
});
};
describe('MRWidgetConflicts', () => { describe('MRWidgetConflicts', () => {
describe('props', () => { describe('props', () => {
...@@ -27,43 +16,89 @@ describe('MRWidgetConflicts', () => { ...@@ -27,43 +16,89 @@ describe('MRWidgetConflicts', () => {
}); });
describe('template', () => { describe('template', () => {
it('should have correct elements', () => { describe('when allowed to merge', () => {
const el = createComponent().$el; let vm;
const resolveButton = el.querySelector('.js-resolve-conflicts-button');
const mergeButton = el.querySelector('.mr-widget-body .btn'); beforeEach(() => {
const mergeLocallyButton = el.querySelector('.js-merge-locally-button'); vm = mountComponent(ConflictsComponent, {
mr: {
expect(el.textContent).toContain('There are merge conflicts'); canMerge: true,
expect(el.textContent).not.toContain('ask someone with write access'); conflictResolutionPath: path,
expect(el.querySelector('.btn-success').disabled).toBeTruthy(); },
});
});
afterEach(() => {
vm.$destroy();
});
it('should tell you about conflicts without bothering other people', () => {
expect(vm.$el.textContent).toContain('There are merge conflicts');
expect(vm.$el.textContent).not.toContain('ask someone with write access');
});
it('should allow you to resolve the conflicts', () => {
const resolveButton = vm.$el.querySelector('.js-resolve-conflicts-button');
expect(resolveButton.textContent).toContain('Resolve conflicts'); expect(resolveButton.textContent).toContain('Resolve conflicts');
expect(resolveButton.getAttribute('href')).toEqual(path); expect(resolveButton.getAttribute('href')).toEqual(path);
});
it('should have merge buttons', () => {
const mergeButton = vm.$el.querySelector('.js-disabled-merge-button');
const mergeLocallyButton = vm.$el.querySelector('.js-merge-locally-button');
expect(mergeButton.textContent).toContain('Merge'); expect(mergeButton.textContent).toContain('Merge');
expect(mergeButton.disabled).toBeTruthy();
expect(mergeButton.classList.contains('btn-success')).toEqual(true);
expect(mergeLocallyButton.textContent).toContain('Merge locally'); expect(mergeLocallyButton.textContent).toContain('Merge locally');
}); });
});
describe('when user does not have permission to merge', () => { describe('when user does not have permission to merge', () => {
let vm; let vm;
beforeEach(() => { beforeEach(() => {
vm = createComponent(); vm = mountComponent(ConflictsComponent, {
vm.mr.canMerge = false; mr: {
canMerge: false,
},
});
}); });
it('should show proper message', (done) => { afterEach(() => {
Vue.nextTick(() => { vm.$destroy();
});
it('should show proper message', () => {
expect(vm.$el.textContent).toContain('ask someone with write access'); expect(vm.$el.textContent).toContain('ask someone with write access');
done(); });
it('should not have action buttons', () => {
expect(vm.$el.querySelector('.js-disabled-merge-button')).toBeDefined();
expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toBeNull();
expect(vm.$el.querySelector('.js-merge-locally-button')).toBeNull();
});
});
describe('when fast-forward or semi-linear merge enabled', () => {
let vm;
beforeEach(() => {
vm = mountComponent(ConflictsComponent, {
mr: {
shouldBeRebased: true,
},
}); });
}); });
it('should not have action buttons', (done) => { afterEach(() => {
Vue.nextTick(() => { vm.$destroy();
expect(vm.$el.querySelectorAll('.btn').length).toBe(1);
expect(vm.$el.querySelector('.js-resolve-conflicts-button')).toEqual(null);
expect(vm.$el.querySelector('.js-merge-locally-button')).toEqual(null);
done();
}); });
it('should tell you to rebase locally', () => {
expect(vm.$el.textContent).toContain('Fast-forward merge is not possible.');
expect(vm.$el.textContent).toContain('To merge this request, first rebase locally');
}); });
}); });
}); });
......
...@@ -182,36 +182,6 @@ describe('MRWidgetReadyToMerge', () => { ...@@ -182,36 +182,6 @@ describe('MRWidgetReadyToMerge', () => {
expect(vm.isMergeButtonDisabled).toBeTruthy(); expect(vm.isMergeButtonDisabled).toBeTruthy();
}); });
}); });
describe('Remove source branch checkbox', () => {
describe('when user can merge but cannot delete branch', () => {
it('isRemoveSourceBranchButtonDisabled should be true', () => {
expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true);
});
it('should be disabled in the rendered output', () => {
const checkboxElement = vm.$el.querySelector('#remove-source-branch-input');
expect(checkboxElement.getAttribute('disabled')).toBe('disabled');
});
});
describe('when user can merge and can delete branch', () => {
beforeEach(() => {
this.customVm = createComponent({
mr: { canRemoveSourceBranch: true },
});
});
it('isRemoveSourceBranchButtonDisabled should be false', () => {
expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false);
});
it('should be enabled in rendered output', () => {
const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input');
expect(checkboxElement.getAttribute('disabled')).toBeNull();
});
});
});
}); });
describe('methods', () => { describe('methods', () => {
...@@ -467,4 +437,54 @@ describe('MRWidgetReadyToMerge', () => { ...@@ -467,4 +437,54 @@ describe('MRWidgetReadyToMerge', () => {
}); });
}); });
}); });
describe('Remove source branch checkbox', () => {
describe('when user can merge but cannot delete branch', () => {
it('isRemoveSourceBranchButtonDisabled should be true', () => {
expect(vm.isRemoveSourceBranchButtonDisabled).toBe(true);
});
it('should be disabled in the rendered output', () => {
const checkboxElement = vm.$el.querySelector('#remove-source-branch-input');
expect(checkboxElement.getAttribute('disabled')).toBe('disabled');
});
});
describe('when user can merge and can delete branch', () => {
beforeEach(() => {
this.customVm = createComponent({
mr: { canRemoveSourceBranch: true },
});
});
it('isRemoveSourceBranchButtonDisabled should be false', () => {
expect(this.customVm.isRemoveSourceBranchButtonDisabled).toBe(false);
});
it('should be enabled in rendered output', () => {
const checkboxElement = this.customVm.$el.querySelector('#remove-source-branch-input');
expect(checkboxElement.getAttribute('disabled')).toBeNull();
});
});
});
describe('Commit message area', () => {
it('when using merge commits, should show "Modify commit message" button', () => {
const customVm = createComponent({
mr: { ffOnlyEnabled: false },
});
expect(customVm.$el.querySelector('.js-fast-forward-message')).toBeNull();
expect(customVm.$el.querySelector('.js-modify-commit-message-button')).toBeDefined();
});
it('when fast-forward merge is enabled, only show fast-forward message', () => {
const customVm = createComponent({
mr: { ffOnlyEnabled: true },
});
expect(customVm.$el.querySelector('.js-fast-forward-message')).toBeDefined();
expect(customVm.$el.querySelector('.js-modify-commit-message-button')).toBeNull();
});
});
}); });
...@@ -412,6 +412,8 @@ Project: ...@@ -412,6 +412,8 @@ Project:
- last_repository_updated_at - last_repository_updated_at
- ci_config_path - ci_config_path
- delete_error - delete_error
- merge_requests_ff_only_enabled
- merge_requests_rebase_enabled
Author: Author:
- name - name
ProjectFeature: ProjectFeature:
......
...@@ -408,6 +408,18 @@ describe Project do ...@@ -408,6 +408,18 @@ describe Project do
end end
end 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
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
end
end
describe '#repository_storage_path' do describe '#repository_storage_path' do
let(:project) { create(:project, repository_storage: 'custom') } let(:project) { create(:project, repository_storage: 'custom') }
......
...@@ -1345,6 +1345,34 @@ describe Repository do ...@@ -1345,6 +1345,34 @@ describe Repository do
end end
end end
describe '#ff_merge' do
before do
repository.add_branch(user, 'ff-target', 'feature~5')
end
it 'merges the code and return the commit id' do
merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'ff-target', source_project: project)
merge_commit_id = repository.ff_merge(user,
merge_request.diff_head_sha,
merge_request.target_branch,
merge_request: merge_request)
merge_commit = repository.commit(merge_commit_id)
expect(merge_commit).to be_present
expect(repository.blob_at(merge_commit.id, 'files/ruby/feature.rb')).to be_present
end
it 'sets the `in_progress_merge_commit_sha` flag for the given merge request' do
merge_request = create(:merge_request, source_branch: 'feature', target_branch: 'ff-target', source_project: project)
merge_commit_id = repository.ff_merge(user,
merge_request.diff_head_sha,
merge_request.target_branch,
merge_request: merge_request)
expect(merge_request.in_progress_merge_commit_sha).to eq(merge_commit_id)
end
end
describe '#revert' do describe '#revert' do
let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') } let(:new_image_commit) { repository.commit('33f3729a45c02fc67d00adb1b8bca394b0e761d9') }
let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') } let(:update_image_commit) { repository.commit('2f63565e7aac07bcdadb654e253078b727143ec4') }
......
...@@ -47,7 +47,8 @@ describe MergeRequestEntity do ...@@ -47,7 +47,8 @@ describe MergeRequestEntity do
:cancel_merge_when_pipeline_succeeds_path, :cancel_merge_when_pipeline_succeeds_path,
:create_issue_to_resolve_discussions_path, :create_issue_to_resolve_discussions_path,
:source_branch_path, :target_branch_commits_path, :source_branch_path, :target_branch_commits_path,
:target_branch_tree_path, :commits_count, :merge_ongoing) :target_branch_tree_path, :commits_count, :merge_ongoing,
:ff_only_enabled)
end end
it 'has email_patches_path' do it 'has email_patches_path' do
......
require 'spec_helper'
describe MergeRequests::FfMergeService do
let(:user) { create(:user) }
let(:user2) { create(:user) }
let(:merge_request) do
create(:merge_request,
source_branch: 'flatten-dir',
target_branch: 'improve/awesome',
assignee: user2)
end
let(:project) { merge_request.project }
before do
project.team << [user, :master]
project.team << [user2, :developer]
end
describe '#execute' do
context 'valid params' do
let(:service) { described_class.new(project, user, {}) }
before do
allow(service).to receive(:execute_hooks)
perform_enqueued_jobs do
service.execute(merge_request)
end
end
it "does not create merge commit" do
source_branch_sha = merge_request.source_project.repository.commit(merge_request.source_branch).sha
target_branch_sha = merge_request.target_project.repository.commit(merge_request.target_branch).sha
expect(source_branch_sha).to eq(target_branch_sha)
end
it { expect(merge_request).to be_valid }
it { expect(merge_request).to be_merged }
it 'sends email to user2 about merge of new merge_request' do
email = ActionMailer::Base.deliveries.last
expect(email.to.first).to eq(user2.email)
expect(email.subject).to include(merge_request.title)
end
it 'creates system note about merge_request merge' do
note = merge_request.notes.last
expect(note.note).to include 'merged'
end
end
context "error handling" do
let(:service) { described_class.new(project, user, commit_message: 'Awesome message') }
before do
allow(Rails.logger).to receive(:error)
end
it 'logs and saves error if there is an exception' do
error_message = 'error message'
allow(service).to receive(:repository).and_raise("error message")
allow(service).to receive(:execute_hooks)
service.execute(merge_request)
expect(merge_request.merge_error).to include(error_message)
expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
end
it 'logs and saves error if there is an PreReceiveError exception' do
error_message = 'error message'
allow(service).to receive(:repository).and_raise(Gitlab::Git::HooksService::PreReceiveError, error_message)
allow(service).to receive(:execute_hooks)
service.execute(merge_request)
expect(merge_request.merge_error).to include(error_message)
expect(Rails.logger).to have_received(:error).with(a_string_matching(error_message))
end
end
end
end
...@@ -17,6 +17,7 @@ module TestEnv ...@@ -17,6 +17,7 @@ module TestEnv
'feature_conflict' => 'bb5206f', 'feature_conflict' => 'bb5206f',
'fix' => '48f0be4', 'fix' => '48f0be4',
'improve/awesome' => '5937ac0', 'improve/awesome' => '5937ac0',
'merged-target' => '21751bf',
'markdown' => '0ed8c6c', 'markdown' => '0ed8c6c',
'lfs' => 'be93687', 'lfs' => 'be93687',
'master' => 'b83d6e3', 'master' => 'b83d6e3',
......
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