Commit 63749fae authored by Phil Hughes's avatar Phil Hughes

Merge branch 'blackst0ne-squash-and-merge-in-gitlab-core-ce-ee' into 'master'

Bring the `squash and merge` feature to CE

See merge request gitlab-org/gitlab-ee!5727
parents 1436800b 4219efa7
/*
The squash-before-merge button is EE only, but it's located right in the middle
of the readyToMerge state component template.
If we didn't declare this component in CE, we'd need to maintain a separate copy
of the readyToMergeState template in EE, which is pretty big and likely to change.
Instead, in CE, we declare the component, but it's hidden and is configured to do nothing.
In EE, the configuration extends this object to add a functioning squash-before-merge
button.
*/
<script>
export default {};
import Icon from '~/vue_shared/components/icon.vue';
import eventHub from '~/vue_merge_request_widget/event_hub';
import tooltip from '~/vue_shared/directives/tooltip';
export default {
components: {
Icon,
},
directives: {
tooltip,
},
props: {
mr: {
type: Object,
required: true,
},
isMergeButtonDisabled: {
type: Boolean,
required: true,
},
},
data() {
return {
squashBeforeMerge: this.mr.squash,
};
},
methods: {
updateSquashModel() {
eventHub.$emit('MRWidgetUpdateSquash', this.squashBeforeMerge);
},
},
};
</script>
<template>
<div class="accept-control inline">
<label class="merge-param-checkbox">
<input
type="checkbox"
name="squash"
class="qa-squash-checkbox"
:disabled="isMergeButtonDisabled"
v-model="squashBeforeMerge"
@change="updateSquashModel"
/>
{{ __('Squash commits') }}
</label>
<a
:href="mr.squashBeforeMergeHelpPath"
data-title="About this feature"
data-placement="bottom"
target="_blank"
rel="noopener noreferrer nofollow"
data-container="body"
v-tooltip
>
<icon
name="question-o"
/>
</a>
</div>
</template>
......@@ -6,11 +6,13 @@ import MergeRequest from '../../../merge_request';
import Flash from '../../../flash';
import statusIcon from '../mr_widget_status_icon.vue';
import eventHub from '../../event_hub';
import SquashBeforeMerge from './mr_widget_squash_before_merge.vue';
export default {
name: 'ReadyToMerge',
components: {
statusIcon,
'squash-before-merge': SquashBeforeMerge,
},
props: {
mr: { type: Object, required: true },
......@@ -105,6 +107,12 @@ export default {
return this.mr.approvalsRequired ? !this.mr.isApproved : false;
},
},
created() {
eventHub.$on('MRWidgetUpdateSquash', this.handleUpdateSquash);
},
beforeDestroy() {
eventHub.$off('MRWidgetUpdateSquash', this.handleUpdateSquash);
},
methods: {
shouldShowMergeControls() {
return this.mr.isMergeAllowed || this.shouldShowMergeWhenPipelineSucceedsText;
......@@ -132,13 +140,9 @@ export default {
commit_message: this.commitMessage,
merge_when_pipeline_succeeds: this.setToMergeWhenPipelineSucceeds,
should_remove_source_branch: this.removeSourceBranch === true,
squash: this.mr.squash,
};
// Only truthy in EE extension of this component
if (this.setAdditionalParams) {
this.setAdditionalParams(options);
}
this.isMakingRequest = true;
this.service.merge(options)
.then(res => res.data)
......@@ -158,6 +162,9 @@ export default {
new Flash('Something went wrong. Please try again.'); // eslint-disable-line
});
},
handleUpdateSquash(val) {
this.mr.squash = val;
},
initiateMergePolling() {
simplePoll((continuePolling, stopPolling) => {
this.handleMergePolling(continuePolling, stopPolling);
......
......@@ -26,7 +26,7 @@ export { default as ConflictsState } from './components/states/mr_widget_conflic
export { default as NothingToMergeState } from './components/states/nothing_to_merge.vue';
export { default as MissingBranchState } from './components/states/mr_widget_missing_branch.vue';
export { default as NotAllowedState } from './components/states/mr_widget_not_allowed.vue';
export { default as ReadyToMergeState } from 'ee/vue_merge_request_widget/components/states/mr_widget_ready_to_merge.vue';
export { default as ReadyToMergeState } from './components/states/ready_to_merge.vue';
export { default as ShaMismatchState } from './components/states/sha_mismatch.vue';
export { default as UnresolvedDiscussionsState } from './components/states/unresolved_discussions.vue';
export { default as PipelineBlockedState } from './components/states/mr_widget_pipeline_blocked.vue';
......@@ -40,7 +40,7 @@ export { default as MRWidgetService } from 'ee/vue_merge_request_widget/services
export { default as eventHub } from './event_hub';
export { default as getStateKey } from 'ee/vue_merge_request_widget/stores/get_state_key';
export { default as stateMaps } from 'ee/vue_merge_request_widget/stores/state_maps';
export { default as SquashBeforeMerge } from 'ee/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue';
export { default as SquashBeforeMerge } from './components/states/mr_widget_squash_before_merge.vue';
export { default as notify } from '../lib/utils/notify';
export { default as SourceBranchRemovalStatus } from './components/source_branch_removal_status.vue';
......
......@@ -15,8 +15,10 @@ export default class MergeRequestStore {
const currentUser = data.current_user;
const pipelineStatus = data.pipeline ? data.pipeline.details.status : null;
// EE specific
this.squash = data.squash;
this.squashBeforeMergeHelpPath = this.squashBeforeMergeHelpPath ||
data.squash_before_merge_help_path;
this.enableSquashBeforeMerge = this.enableSquashBeforeMerge || true;
this.title = data.title;
this.targetBranch = data.target_branch;
......
......@@ -26,6 +26,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
:source_branch,
:source_project_id,
:state_event,
:squash,
:target_branch,
:target_project_id,
:task_num,
......
......@@ -257,7 +257,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
end
def merge_params_attributes
[:should_remove_source_branch, :commit_message]
[:should_remove_source_branch, :commit_message, :squash]
end
def merge_when_pipeline_succeeds_active?
......
......@@ -137,8 +137,9 @@ module MergeRequestsHelper
{
merge_when_pipeline_succeeds: true,
should_remove_source_branch: true,
sha: merge_request.diff_head_sha
}.merge(merge_params_ee(merge_request))
sha: merge_request.diff_head_sha,
squash: merge_request.squash
}
end
def tab_link_for(merge_request, tab, options = {}, &block)
......@@ -189,8 +190,4 @@ module MergeRequestsHelper
current_user.fork_of(project)
end
end
def merge_params_ee(merge_request)
{ squash: merge_request.squash }
end
end
......@@ -1173,4 +1173,11 @@ class MergeRequest < ActiveRecord::Base
maintainer_push_possible? &&
Ability.allowed?(user, :push_code, source_project)
end
def squash_in_progress?
# The source project can be deleted
return false unless source_project
source_project.repository.squash_in_progress?(id)
end
end
......@@ -1014,6 +1014,14 @@ class Repository
blob.data
end
def squash(user, merge_request)
raw.squash(user, merge_request.id, branch: merge_request.target_branch,
start_sha: merge_request.diff_start_sha,
end_sha: merge_request.diff_head_sha,
author: merge_request.author,
message: merge_request.title)
end
private
# TODO Generice finder, later split this on finders by Ref or Oid
......
......@@ -12,6 +12,7 @@ class MergeRequestWidgetEntity < IssuableEntity
expose :merge_when_pipeline_succeeds
expose :source_branch
expose :source_project_id
expose :squash
expose :target_branch
expose :target_project_id
expose :allow_maintainer_to_push
......@@ -23,7 +24,6 @@ class MergeRequestWidgetEntity < IssuableEntity
# EE-specific
expose :approvals_before_merge
expose :squash
expose :rebase_commit_sha
expose :rebase_in_progress?, as: :rebase_in_progress
......
......@@ -36,6 +36,19 @@ module MergeRequests
handle_merge_error(log_message: e.message, save_message_on_model: true)
end
def source
return merge_request.diff_head_sha unless merge_request.squash
squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute(merge_request)
case squash_result[:status]
when :success
squash_result[:squash_sha]
when :error
raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
end
end
private
def error_check!
......@@ -118,9 +131,5 @@ module MergeRequests
def merge_request_info
merge_request.to_reference(full: true)
end
def source
@source ||= @merge_request.diff_head_sha
end
end
end
require 'securerandom'
module MergeRequests
class SquashService < MergeRequests::WorkingCopyBaseService
def execute(merge_request)
......@@ -10,14 +8,10 @@ module MergeRequests
end
def squash
if merge_request.commits_count <= 1
if merge_request.commits_count < 2
return success(squash_sha: merge_request.diff_head_sha)
end
unless project.feature_available?(:merge_request_squash)
return error('License does not allow squashing')
end
if merge_request.squash_in_progress?
return error('Squash task canceled: another squash is already in progress.')
end
......
......@@ -20,13 +20,12 @@
window.gl = window.gl || {};
window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget')}
window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
// Append static, server-generated data not included in merge request entity (EE-Only)
// Object.assign would be useful here, but it blows up Phantom.js in tests
window.gl.mrWidgetData.is_geo_secondary_node = '#{Gitlab::Geo.secondary?}' === 'true';
window.gl.mrWidgetData.geo_secondary_help_path = '#{help_page_path("/gitlab-geo/configuration.md")}';
window.gl.mrWidgetData.enable_squash_before_merge = '#{@merge_request.project.feature_available?(:merge_request_squash)}' === 'true';
window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
window.gl.mrWidgetData.sast_help_path = '#{help_page_path("user/project/merge_requests/sast")}';
window.gl.mrWidgetData.sast_container_help_path = '#{help_page_path("user/project/merge_requests/container_scanning")}';
window.gl.mrWidgetData.dast_help_path = '#{help_page_path("user/project/merge_requests/dast")}';
......
......@@ -12,4 +12,11 @@
= check_box_tag 'merge_request[force_remove_source_branch]', '1', issuable.force_remove_source_branch?
Remove source branch when merge request is accepted.
= render 'shared/issuable/form/ee/squash_merge_param', issuable: issuable
.form-group
.col-sm-10.col-sm-offset-2
.checkbox
= label_tag 'merge_request[squash]' do
= hidden_field_tag 'merge_request[squash]', '0', id: nil
= check_box_tag 'merge_request[squash]', '1', issuable.squash
Squash commits when merge request is accepted.
= link_to 'About this feature', help_page_path('user/project/merge_requests/squash_and_merge')
---
title: Add `Squash and merge` to GitLab Core (CE)
merge_request: 18956
author: "@blackst0ne"
type: added
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddSquashToMergeRequests < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
unless column_exists?(:merge_requests, :squash)
add_column_with_default :merge_requests, :squash, :boolean, default: false, allow_null: false
end
end
def down
remove_column :merge_requests, :squash if column_exists?(:merge_requests, :squash)
end
end
......@@ -1617,7 +1617,6 @@ ActiveRecord::Schema.define(version: 20180524132016) do
t.text "title_html"
t.text "description_html"
t.integer "time_estimate"
t.boolean "squash", default: false, null: false
t.integer "cached_markdown_version"
t.datetime "last_edited_at"
t.integer "last_edited_by_id"
......@@ -1626,6 +1625,7 @@ ActiveRecord::Schema.define(version: 20180524132016) do
t.boolean "discussion_locked"
t.integer "latest_merge_request_diff_id"
t.boolean "allow_maintainer_to_push"
t.boolean "squash", default: false, null: false
end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
......
......@@ -107,6 +107,7 @@ Parameters:
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"time_stats": {
"time_estimate": 0,
......@@ -535,21 +536,20 @@ Creates a new merge request.
POST /projects/:id/merge_requests
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `source_branch` | string | yes | The source branch |
| `target_branch` | string | yes | The target branch |
| `title` | string | yes | Title of MR |
| `assignee_id` | integer | no | Assignee user ID |
| `description` | string | no | Description of MR |
| `target_project_id` | integer | no | The target project (numeric id) |
| `labels` | string | no | Labels for MR as a comma-separated list |
| `milestone_id` | integer | no | The global ID of a milestone |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `approvals_before_merge` | integer| no | Number of approvals required before this can be merged (see below) |
| `squash` | boolean| no | Squash commits into a single commit when merging |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `source_branch` | string | yes | The source branch |
| `target_branch` | string | yes | The target branch |
| `title` | string | yes | Title of MR |
| `assignee_id` | integer | no | Assignee user ID |
| `description` | string | no | Description of MR |
| `target_project_id` | integer | no | The target project (numeric id) |
| `labels` | string | no | Labels for MR as a comma-separated list |
| `milestone_id` | integer | no | The ID of a milestone |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
| `squash` | boolean | no | Squash commits into a single commit when merging |
If `approvals_before_merge` is not provided, it inherits the value from the
target project. If it is provided, then the following conditions must hold in
......@@ -634,21 +634,21 @@ Updates an existing merge request. You can change the target branch, title, or e
PUT /projects/:id/merge_requests/:merge_request_iid
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `merge_request_iid` | integer | yes | The ID of a merge request |
| `target_branch` | string | no | The target branch |
| `title` | string | no | Title of MR |
| `assignee_id` | integer | no | The ID of the user to assign the merge request to. Set to `0` or provide an empty value to unassign all assignees. |
| `milestone_id` | integer | no | The global ID of a milestone to assign the merge request to. Set to `0` or provide an empty value to unassign a milestone.|
| `labels` | string | no | Comma-separated label names for a merge request. Set to an empty string to unassign all labels. |
| `description` | string | no | Description of MR |
| `state_event` | string | no | New state (close/reopen) |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `squash` | boolean| no | Squash commits into a single commit when merging |
| `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `merge_request_iid` | integer | yes | The ID of a merge request |
| `target_branch` | string | no | The target branch |
| `title` | string | no | Title of MR |
| `assignee_id` | integer | no | The ID of the user to assign the merge request to. Set to `0` or provide an empty value to unassign all assignees. |
| `milestone_id` | integer | no | The ID of a milestone to assign the merge request to. Set to `0` or provide an empty value to unassign a milestone.|
| `labels` | string | no | Comma-separated label names for a merge request. Set to an empty string to unassign all labels. |
| `description` | string | no | Description of MR |
| `state_event` | string | no | New state (close/reopen) |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `squash` | boolean | no | Squash commits into a single commit when merging |
| `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
Must include at least one non-required attribute from above.
......@@ -1227,7 +1227,7 @@ Example response:
"changes_count": "1",
"should_remove_source_branch": true,
"force_remove_source_branch": false,
"squash": true,
"squash": false,
"web_url": "http://example.com/example/example/merge_requests/1"
},
"target_url": "https://gitlab.example.com/gitlab-org/gitlab-ci/merge_requests/7",
......
......@@ -29,6 +29,7 @@ With GitLab merge requests, you can:
- 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
- [Create new merge requests by email](#create-new-merge-requests-by-email)
- Allow maintainers of the target project to push directly to the fork by [allowing edits from maintainers](maintainer_access.md)
- [Squash and merge](squash_and_merge.md) for a cleaner commit history
With **[GitLab Enterprise Edition][ee]**, you can also:
......
# Squash and merge **[STARTER]**
# Squash and merge
> [Introduced][ee-1024] in [GitLab Starter][ee] 8.17.
> [Introduced][ee-1024] in [GitLab Starter][ee] 8.17, and in [GitLab CE][ce] [11.0][ce-18956].
Combine all commits of your merge request into one and retain a clean history.
......@@ -18,11 +18,11 @@ Into a single commit on merge:
![A squashed commit followed by a merge commit][squashed-commit]
The squashed commit's commit message is the merge request title. And note that
The squashed commit's commit message is the merge request title. And note that
the squashed commit is still followed by a merge commit, as the merge
method for this example repository uses a merge commit. Squashing also works
with the fast-forward merge strategy, see
[squashing and fast-forward merge](#squashing-and-fast-forward-merge) for more
[squashing and fast-forward merge](#squash-and-fast-forward-merge) for more
details.
## Use cases
......@@ -30,7 +30,7 @@ details.
When working on a feature branch, you sometimes want to commit your current
progress, but don't really care about the commit messages. Those 'work in
progress commits' don't necessarily contain important information and as such
you'd rather not include them in your target branch.
you'd rather not include them in your target branch.
With squash and merge, when the merge request is ready to be merged,
all you have to do is enable squashing before you press merge to join
......@@ -69,10 +69,12 @@ so a merge request may need to be rebased before squashing, even though
squashing can itself be considered equivalent to rebasing.
[ee-1024]: https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/1024
[ce-18956]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/18956
[mr-commits]: img/squash_mr_commits.png
[squashed-commit]: img/squash_squashed_commit.png
[squash-edit-form]: img/squash_edit_form.png
[squash-mr-widget]: img/squash_mr_widget.png
[ff-merge]: fast_forward_merge.md#enabling-fast-forward-merges
[ce]: https://about.gitlab.com/products/
[ee]: https://about.gitlab.com/products/
[revert]: revert_changes.md
<script>
import eventHub from '~/vue_merge_request_widget/event_hub';
import ReadyToMergeState from '~/vue_merge_request_widget/components/states/ready_to_merge.vue';
import SquashBeforeMerge from './mr_widget_squash_before_merge.vue';
export default {
name: 'ReadyToMerge',
components: {
SquashBeforeMerge,
},
extends: ReadyToMergeState,
data() {
return {
additionalParams: {
squash: this.mr.squash,
},
};
},
created() {
eventHub.$on('MRWidgetUpdateSquash', val => {
this.additionalParams.squash = val;
});
},
methods: {
// called in CE super component before form submission
setAdditionalParams(options) {
if (this.additionalParams) {
Object.assign(options, this.additionalParams);
}
},
},
};
</script>
<script>
import eventHub from '~/vue_merge_request_widget/event_hub';
import CESquashBeforeMerge from '~/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue';
export default {
extends: CESquashBeforeMerge,
props: {
mr: {
type: Object,
required: true,
},
isMergeButtonDisabled: {
type: Boolean,
required: true,
},
},
data() {
return {
squashBeforeMerge: this.mr.squash,
};
},
methods: {
updateSquashModel() {
eventHub.$emit('MRWidgetUpdateSquash', this.squashBeforeMerge);
},
},
};
</script>
<template>
<div class="accept-control inline">
<label class="merge-param-checkbox">
<input
type="checkbox"
name="squash"
class="qa-squash-checkbox"
:disabled="isMergeButtonDisabled"
v-model="squashBeforeMerge"
@change="updateSquashModel"
/>
Squash commits
</label>
<a
:href="mr.squashBeforeMergeHelpPath"
data-title="About this feature"
data-toggle="tooltip"
data-placement="bottom"
target="_blank"
rel="noopener noreferrer nofollow"
data-container="body"
>
<i
class="fa fa-question-circle"
aria-hidden="true"
>
</i>
</a>
</div>
</template>
......@@ -26,18 +26,11 @@ export default class MergeRequestStore extends CEMergeRequestStore {
setData(data) {
this.initGeo(data);
this.initSquashBeforeMerge(data);
this.initApprovals(data);
super.setData(data);
}
initSquashBeforeMerge(data) {
this.squashBeforeMergeHelpPath =
this.squashBeforeMergeHelpPath || data.squash_before_merge_help_path;
this.enableSquashBeforeMerge = this.enableSquashBeforeMerge || data.enable_squash_before_merge;
}
initGeo(data) {
this.isGeoSecondaryNode = this.isGeoSecondaryNode || data.is_geo_secondary_node;
this.geoSecondaryHelpPath = this.geoSecondaryHelpPath || data.geo_secondary_help_path;
......
......@@ -25,7 +25,6 @@ module EE
:approver_group_ids,
:approver_ids
)
attrs << :squash if project.feature_available?(:merge_request_squash)
attrs
end
......
......@@ -64,13 +64,6 @@ module EE
end
end
end
def merge_params_attributes
attrs = super
attrs << :squash if project.feature_available?(:merge_request_squash)
attrs
end
end
end
end
......@@ -48,18 +48,6 @@ module EE
delegate :expose_dast_data?, to: :head_pipeline, allow_nil: true
end
def squash_in_progress?
# The source project can be deleted
return false unless source_project
source_project.repository.squash_in_progress?(id)
end
def squash
super && project.feature_available?(:merge_request_squash)
end
alias_method :squash?, :squash
def supports_weight?
false
end
......
......@@ -25,13 +25,5 @@ module EE
expire_branch_cache if exists?
expire_content_cache
end
def squash(user, merge_request)
raw.squash(user, merge_request.id, branch: merge_request.target_branch,
start_sha: merge_request.diff_start_sha,
end_sha: merge_request.diff_head_sha,
author: merge_request.author,
message: merge_request.title)
end
end
end
......@@ -21,7 +21,6 @@ class License < ActiveRecord::Base
ldap_group_sync
member_lock
merge_request_approvers
merge_request_squash
multiple_ldap_servers
multiple_issue_assignees
multiple_project_issue_boards
......@@ -94,7 +93,6 @@ class License < ActiveRecord::Base
issue_weights
jenkins_integration
merge_request_approvers
merge_request_squash
multiple_issue_assignees
multiple_project_issue_boards
multiple_group_issue_boards
......
......@@ -10,19 +10,6 @@ module EE
super
end
def source
return merge_request.diff_head_sha unless merge_request.squash
squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute(merge_request)
case squash_result[:status]
when :success
squash_result[:squash_sha]
when :error
raise ::MergeRequests::MergeService::MergeError, squash_result[:message]
end
end
def hooks_validation_pass?(merge_request)
# handle_merge_error needs this. We should move that to a separate
# object instead of relying on the order of method calls.
......
- issuable = local_assigns.fetch(:issuable)
- if issuable.project.feature_available?(:merge_request_squash)
.form-group.row
.col-sm-10.offset-sm-2
= label_tag 'merge_request[squash]' do
= hidden_field_tag 'merge_request[squash]', '0', id: nil
= check_box_tag 'merge_request[squash]', '1', issuable.squash
Squash commits when merge request is accepted.
= link_to 'About this feature', help_page_path('user/project/merge_requests/squash_and_merge')
- elsif show_promotions? && show_callout?('promote_squash_commits_dismissed')
.form-group.row
.col-sm-10.offset-sm-2
= render 'shared/promotions/promote_squash_commits'
.user-callout.promotion-callout.prepend-top-10#promote_squash_commits{ data: { uid: 'promote_squash_commits_dismissed' } }
.bordered-box.content-block
%button.btn.btn-default.close.js-close-callout{ type: 'button', 'aria-label' => 'Dismiss merge request promotion' }
= icon('times', class: 'dismiss-icon', 'aria-hidden' => 'true')
.user-callout-copy
%h4
- if Gitlab::CurrentSettings.should_check_namespace_plan?
Upgrade your plan to improve Merge Requests with Squash Commit.
- else
Improve Merge Requests with Squash Commit and GitLab Enterprise Edition.
%p
Squashing lets you tidy up the commit history of a branch when accepting a merge request. It applies all of the changes in the merge request as a single commit, and then merges that commit using the merge method set for the project.
= link_to 'Read more', help_page_path('user/project/merge_requests/squash_and_merge.html'), target: '_blank'
= render 'shared/promotions/promotion_link_project'
# rubocop:disable Migration/AddColumnWithDefaultToLargeTable
# rubocop:disable Migration/UpdateLargeTable
class AddSquashToMergeRequests < ActiveRecord::Migration
class AddSquashToMergeRequestsEE < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
disable_ddl_transaction!
DOWNTIME = false
def up
add_column_with_default :merge_requests, :squash, :boolean, default: false, allow_null: false
unless column_exists?(:merge_requests, :squash)
add_column_with_default :merge_requests, :squash, :boolean, default: false, allow_null: false
end
end
def down
remove_column :merge_requests, :squash
remove_column :merge_requests, :squash if column_exists?(:merge_requests, :squash)
end
end
......@@ -62,7 +62,6 @@ module EE
prepended do
expose :approvals_before_merge
expose :squash, if: ->(mr, _) { mr.project.feature_available?(:merge_request_squash) }
end
end
......
......@@ -3,29 +3,10 @@ module EE
module MergeRequests
extend ActiveSupport::Concern
class_methods do
def update_params_at_least_one_of
super.push(*%i[
squash
])
end
end
prepended do
helpers do
params :merge_params_ee do
optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge'
end
params :optional_params_ee do
optional :approvals_before_merge, type: Integer, desc: 'Number of approvals required before this can be merged'
use :merge_params_ee
end
def update_merge_request_ee(merge_request)
if params[:squash] && merge_request.project.feature_available?(:merge_request_squash)
merge_request.update(squash: params[:squash])
end
end
end
......
......@@ -315,49 +315,6 @@ describe Projects::MergeRequestsController do
end
end
describe 'POST merge' do
let(:base_params) do
{
namespace_id: project.namespace,
project_id: project,
id: merge_request.iid,
squash: false,
format: 'json'
}
end
context 'when the sha parameter matches the source SHA' do
def merge_with_sha(params = {})
post :merge, base_params.merge(sha: merge_request.diff_head_sha).merge(params)
end
context 'when squash is passed as 1' do
it 'updates the squash attribute on the MR to true' do
merge_request.update(squash: false)
merge_with_sha(squash: '1')
expect(merge_request.reload.squash).to be_truthy
end
it 'merges even when squash is unavailable' do
stub_licensed_features(merge_request_squash: false)
merge_with_sha(squash: '1')
expect(merge_request.reload.squash).to be_falsey
end
end
context 'when squash is passed as 0' do
it 'updates the squash attribute on the MR to false' do
merge_request.update(squash: true)
merge_with_sha(squash: '0')
expect(merge_request.reload.squash).to be_falsey
end
end
end
end
describe 'POST #rebase' do
def post_rebase
post :rebase, namespace_id: project.namespace, project_id: project, id: merge_request
......
......@@ -161,34 +161,6 @@ describe 'Promotions', :js do
end
end
describe 'for squash commits', :js do
before do
allow(License).to receive(:current).and_return(nil)
stub_application_setting(check_namespace_plan: false)
project.add_master(user)
sign_in(user)
end
it 'should appear in new MR page' do
visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' })
expect(find('#promote_squash_commits')).to have_content 'Improve Merge Requests with Squash Commit and GitLab Enterprise Edition.'
end
it 'does not show when cookie is set' do
visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' })
within('#promote_squash_commits') do
find('.close').click
end
visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: 'feature' })
expect(page).not_to have_selector('#promote_squash_commits')
end
end
describe 'for burndown charts', :js do
before do
stub_application_setting(check_namespace_plan: true)
......
......@@ -14,85 +14,6 @@ describe MergeRequest do
it { is_expected.to have_many(:approved_by_users) }
end
describe '#squash_in_progress?' do
shared_examples 'checking whether a squash is in progress' do
let(:repo_path) { subject.source_project.repository.path }
let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") }
before do
system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{squash_path} master))
end
it 'returns true when there is a current squash directory' do
expect(subject.squash_in_progress?).to be_truthy
end
it 'returns false when there is no squash directory' do
FileUtils.rm_rf(squash_path)
expect(subject.squash_in_progress?).to be_falsey
end
it 'returns false when the squash directory has expired' do
time = 20.minutes.ago.to_time
File.utime(time, time, squash_path)
expect(subject.squash_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)
expect(subject.squash_in_progress?).to be_falsey
end
end
context 'when Gitaly squash_in_progress is enabled' do
it_behaves_like 'checking whether a squash is in progress'
end
context 'when Gitaly squash_in_progress is disabled', :disable_gitaly do
it_behaves_like 'checking whether a squash is in progress'
end
end
describe '#squash?' do
let(:merge_request) { build(:merge_request, squash: squash) }
subject { merge_request.squash? }
context 'unlicensed' do
before do
stub_licensed_features(merge_request_squash: false)
end
context 'disabled in database' do
let(:squash) { false }
it { is_expected.to be_falsy }
end
context 'enabled in database' do
let(:squash) { true }
it { is_expected.to be_falsy }
end
end
context 'licensed' do
context 'disabled in database' do
let(:squash) { false }
it { is_expected.to be_falsy }
end
context 'licensed' do
let(:squash) { true }
it { is_expected.to be_truthy }
end
end
end
describe '#approvals_before_merge' do
where(:license_value, :db_value, :expected) do
true | 5 | 5
......
require 'spec_helper'
describe MergeRequests::SquashService do
let(:service) { described_class.new(project, user, {}) }
let(:user) { project.owner }
let(:project) { create(:project, :repository) }
let(:repository) { project.repository.raw }
let(:log_error) { "Failed to squash merge request #{merge_request.to_reference(full: true)}:" }
let(:squash_dir_path) do
File.join(Gitlab.config.shared.path, 'tmp/squash', repository.gl_repository, merge_request.id.to_s)
end
let(:merge_request_with_one_commit) do
create(:merge_request,
source_branch: 'feature', source_project: project,
target_branch: 'master', target_project: project)
end
let(:merge_request_with_only_new_files) do
create(:merge_request,
source_branch: 'video', source_project: project,
target_branch: 'master', target_project: project)
end
let(:merge_request_with_large_files) do
create(:merge_request,
source_branch: 'squash-large-files', source_project: project,
target_branch: 'master', target_project: project)
end
shared_examples 'the squash succeeds' do
it 'returns the squashed commit SHA' do
result = service.execute(merge_request)
expect(result).to match(status: :success, squash_sha: a_string_matching(/\h{40}/))
expect(result[:squash_sha]).not_to eq(merge_request.diff_head_sha)
end
it 'cleans up the temporary directory' do
service.execute(merge_request)
expect(File.exist?(squash_dir_path)).to be(false)
end
it 'does not keep the branch push event' do
expect { service.execute(merge_request) }.not_to change { Event.count }
end
context 'the squashed commit' do
let(:squash_sha) { service.execute(merge_request)[:squash_sha] }
let(:squash_commit) { project.repository.commit(squash_sha) }
it 'copies the author info and message from the merge request' do
expect(squash_commit.author_name).to eq(merge_request.author.name)
expect(squash_commit.author_email).to eq(merge_request.author.email)
# Commit messages have a trailing newline, but titles don't.
expect(squash_commit.message.chomp).to eq(merge_request.title)
end
it 'sets the current user as the committer' do
expect(squash_commit.committer_name).to eq(user.name.chomp('.'))
expect(squash_commit.committer_email).to eq(user.email)
end
it 'has the same diff as the merge request, but a different SHA' do
rugged = project.repository.rugged
mr_diff = rugged.diff(merge_request.diff_base_sha, merge_request.diff_head_sha)
squash_diff = rugged.diff(merge_request.diff_start_sha, squash_sha)
expect(squash_diff.patch.length).to eq(mr_diff.patch.length)
expect(squash_commit.sha).not_to eq(merge_request.diff_head_sha)
end
end
end
describe '#execute' do
context 'when there is only one commit in the merge request' do
it 'returns that commit SHA' do
result = service.execute(merge_request_with_one_commit)
expect(result).to match(status: :success, squash_sha: merge_request_with_one_commit.diff_head_sha)
end
it 'does not perform any git actions' do
expect(repository).not_to receive(:popen)
service.execute(merge_request_with_one_commit)
end
end
context 'when squashing only new files' do
let(:merge_request) { merge_request_with_only_new_files }
include_examples 'the squash succeeds'
end
context 'when squashing with files too large to display' do
let(:merge_request) { merge_request_with_large_files }
include_examples 'the squash succeeds'
end
context 'squashing is unlicensed' do
before do
stub_licensed_features(merge_request_squash: false)
end
subject { service.execute(merge_request_with_only_new_files) }
it { is_expected.to match(status: :error, message: a_string_including('License')) }
end
context 'git errors' do
let(:merge_request) { merge_request_with_only_new_files }
let(:error) { 'A test error' }
context 'with gitaly enabled' do
before do
allow(repository.gitaly_operation_client).to receive(:user_squash)
.and_raise(Gitlab::Git::Repository::GitError, error)
end
it 'logs the stage and output' do
expect(service).to receive(:log_error).with(log_error)
expect(service).to receive(:log_error).with(error)
service.execute(merge_request)
end
it 'returns an error' do
expect(service.execute(merge_request)).to match(status: :error,
message: a_string_including('squash'))
end
end
context 'with Gitaly disabled', :skip_gitaly_mock do
stages = {
'add worktree for squash' => 'worktree',
'configure sparse checkout' => 'config',
'get files in diff' => 'diff --name-only',
'check out target branch' => 'checkout',
'apply patch' => 'diff --binary',
'commit squashed changes' => 'commit',
'get SHA of squashed commit' => 'rev-parse'
}
stages.each do |stage, command|
context "when the #{stage} stage fails" do
before do
git_command = a_collection_containing_exactly(
a_string_starting_with("#{Gitlab.config.git.bin_path} #{command}")
).or(
a_collection_starting_with([Gitlab.config.git.bin_path] + command.split)
)
allow(repository).to receive(:popen).and_return(['', 0])
allow(repository).to receive(:popen).with(git_command, anything, anything, anything).and_return([error, 1])
end
it 'logs the stage and output' do
expect(service).to receive(:log_error).with(log_error)
expect(service).to receive(:log_error).with(error)
service.execute(merge_request)
end
it 'returns an error' do
expect(service.execute(merge_request)).to match(status: :error,
message: a_string_including('squash'))
end
it 'cleans up the temporary directory' do
expect(File.exist?(squash_dir_path)).to be(false)
service.execute(merge_request)
end
end
end
end
end
context 'when any other exception is thrown' do
let(:merge_request) { merge_request_with_only_new_files }
let(:error) { 'A test error' }
before do
allow(merge_request).to receive(:commits_count).and_raise(error)
end
it 'logs the MR reference and exception' do
expect(service).to receive(:log_error).with(a_string_including("#{project.full_path}#{merge_request.to_reference}"))
expect(service).to receive(:log_error).with(error)
service.execute(merge_request)
end
it 'returns an error' do
expect(service.execute(merge_request)).to match(status: :error,
message: a_string_including('squash'))
end
it 'cleans up the temporary directory' do
service.execute(merge_request)
expect(File.exist?(squash_dir_path)).to be(false)
end
end
end
end
......@@ -570,6 +570,8 @@ module API
expose :time_stats, using: 'API::Entities::IssuableTimeStats' do |merge_request|
merge_request
end
expose :squash
end
class MergeRequest < MergeRequestBasic
......
......@@ -10,12 +10,6 @@ module API
helpers do
params :optional_params_ee do
end
params :merge_params_ee do
end
def update_merge_request_ee(merge_request)
end
end
def self.update_params_at_least_one_of
......@@ -29,6 +23,7 @@ module API
target_branch
title
discussion_locked
squash
]
end
......@@ -148,6 +143,7 @@ module API
optional :labels, type: String, desc: 'Comma-separated list of label names'
optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging'
optional :allow_maintainer_to_push, type: Boolean, desc: 'Whether a maintainer of the target project can push to the source project'
optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge'
use :optional_params_ee
end
......@@ -310,8 +306,7 @@ module API
optional :merge_when_pipeline_succeeds, type: Boolean,
desc: 'When true, this merge request will be merged when the pipeline succeeds'
optional :sha, type: String, desc: 'When present, must have the HEAD SHA of the source branch'
use :merge_params_ee
optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge'
end
put ':id/merge_requests/:merge_request_iid/merge' do
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab-ce/issues/42317')
......@@ -329,7 +324,7 @@ module API
check_sha_param!(params, merge_request)
update_merge_request_ee(merge_request)
merge_request.update(squash: params[:squash]) if params[:squash]
merge_params = {
commit_message: params[:merge_commit_message],
......
......@@ -139,7 +139,7 @@ module API
expose :should_remove_source_branch?, as: :should_remove_source_branch
expose :force_remove_source_branch?, as: :force_remove_source_branch
expose :squash, if: ->(mr, _) { mr.project.feature_available?(:merge_request_squash) }
expose :squash
expose :web_url do |merge_request, options|
Gitlab::UrlBuilder.build(merge_request)
......
......@@ -216,9 +216,7 @@ module API
render_api_error!("SHA does not match HEAD of source branch: #{merge_request.diff_head_sha}", 409)
end
if params[:squash] && merge_request.project.feature_available?(:merge_request_squash)
merge_request.update(squash: params[:squash])
end
merge_request.update(squash: params[:squash]) if params[:squash]
merge_params = {
commit_message: params[:merge_commit_message],
......
......@@ -8,19 +8,7 @@ module QA
view 'app/assets/javascripts/vue_merge_request_widget/components/states/sha_mismatch.vue' do
element :head_mismatch, "The source branch HEAD has recently changed."
end
view 'ee/app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue' do
element :squash_checkbox
end
end
end
def mark_to_squash
wait(reload: true) do
has_css?(element_selector_css(:squash_checkbox))
end
click_element :squash_checkbox
end
end
end
......
......@@ -18,6 +18,10 @@ module QA
element :no_fast_forward_message, 'Fast-forward merge is not possible'
end
view 'app/assets/javascripts/vue_merge_request_widget/components/states/mr_widget_squash_before_merge.vue' do
element :squash_checkbox
end
def rebase!
click_element :mr_rebase_button
......@@ -43,6 +47,14 @@ module QA
has_text?('The changes were merged into')
end
end
def mark_to_squash
wait(reload: true) do
has_css?(element_selector_css(:squash_checkbox))
end
click_element :squash_checkbox
end
end
end
end
......
......@@ -315,8 +315,8 @@ describe Projects::MergeRequestsController do
end
context 'when the sha parameter matches the source SHA' do
def merge_with_sha
post :merge, base_params.merge(sha: merge_request.diff_head_sha)
def merge_with_sha(params = {})
post :merge, base_params.merge(sha: merge_request.diff_head_sha).merge(params)
end
it 'returns :success' do
......@@ -331,6 +331,24 @@ describe Projects::MergeRequestsController do
merge_with_sha
end
context 'when squash is passed as 1' do
it 'updates the squash attribute on the MR to true' do
merge_request.update(squash: false)
merge_with_sha(squash: '1')
expect(merge_request.reload.squash).to be_truthy
end
end
context 'when squash is passed as 0' do
it 'updates the squash attribute on the MR to false' do
merge_request.update(squash: true)
merge_with_sha(squash: '0')
expect(merge_request.reload.squash).to be_falsey
end
end
context 'when the pipeline succeeds is passed' do
let!(:head_pipeline) do
create(:ci_empty_pipeline, project: project, sha: merge_request.diff_head_sha, ref: merge_request.source_branch, head_pipeline_of: merge_request)
......
require 'spec_helper'
feature 'Squashing merge requests', :js do
feature 'User squashes a merge request', :js do
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:source_branch) { 'csv' }
......@@ -121,24 +121,4 @@ feature 'Squashing merge requests', :js do
include_examples 'no squash'
end
end
context 'squash is unlicensed' do
let(:merge_request) { create(:merge_request, source_project: project, target_project: project, source_branch: source_branch, target_branch: 'master', squash: true) }
before do
stub_licensed_features(merge_request_squash: false)
end
it 'does not show squash option when creating MR' do
visit project_new_merge_request_path(project, merge_request: { target_branch: 'master', source_branch: source_branch })
expect(page).to have_no_field('merge_request[squash]')
end
it 'does not show squash option when viewing MR' do
visit project_merge_request_path(project, merge_request)
expect(page).to have_no_field('squash')
end
end
end
......@@ -114,10 +114,10 @@
"rebase_in_progress": { "type": "boolean" },
"can_push_to_source_branch": { "type": "boolean" },
"rebase_path": { "type": ["string", "null"] },
"squash": { "type": "boolean" },
// EE-specific
"approvals_before_merge": { "type": ["integer", "null"] },
"squash": { "type": "boolean" },
"approved": { "type": "boolean" },
"approvals_path": { "type": ["string", "null"] },
"codeclimate": {
......
......@@ -14,6 +14,65 @@ describe MergeRequest do
it { is_expected.to have_many(:merge_request_diffs) }
end
describe '#squash_in_progress?' do
shared_examples 'checking whether a squash is in progress' do
let(:repo_path) { subject.source_project.repository.path }
let(:squash_path) { File.join(repo_path, "gitlab-worktree", "squash-#{subject.id}") }
before do
system(*%W(#{Gitlab.config.git.bin_path} -C #{repo_path} worktree add --detach #{squash_path} master))
end
it 'returns true when there is a current squash directory' do
expect(subject.squash_in_progress?).to be_truthy
end
it 'returns false when there is no squash directory' do
FileUtils.rm_rf(squash_path)
expect(subject.squash_in_progress?).to be_falsey
end
it 'returns false when the squash directory has expired' do
time = 20.minutes.ago.to_time
File.utime(time, time, squash_path)
expect(subject.squash_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)
expect(subject.squash_in_progress?).to be_falsey
end
end
context 'when Gitaly squash_in_progress is enabled' do
it_behaves_like 'checking whether a squash is in progress'
end
context 'when Gitaly squash_in_progress is disabled', :disable_gitaly do
it_behaves_like 'checking whether a squash is in progress'
end
end
describe '#squash?' do
let(:merge_request) { build(:merge_request, squash: squash) }
subject { merge_request.squash? }
context 'disabled in database' do
let(:squash) { false }
it { is_expected.to be_falsy }
end
context 'enabled in database' do
let(:squash) { true }
it { is_expected.to be_truthy }
end
end
describe 'modules' do
subject { described_class }
......
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