Commit 275d7b38 authored by Nick Thomas's avatar Nick Thomas

Introduce namespace license checks for rebase before merge

parent 051fbfb8
...@@ -4,12 +4,13 @@ module EE ...@@ -4,12 +4,13 @@ module EE
extend ActiveSupport::Concern extend ActiveSupport::Concern
prepended do prepended do
# This module is prepended to `Projects::MergeRequstController`, which # This module is prepended to `Projects::MergeRequestController`, which
# already calls `before_action :merge_request, only: [...]`. Calling it # already calls `before_action :merge_request, only: [...]`. Calling it
# again here would *replace* the restriction, rather than extending it. # again here would *replace* the restriction, rather than extending it.
before_action(only: [:approve, :approvals, :unapprove, :rebase]) { merge_request } before_action(only: [:approve, :approvals, :unapprove, :rebase]) { merge_request }
before_action :set_suggested_approvers, only: [:new, :new_diffs, :edit] before_action :set_suggested_approvers, only: [:new, :new_diffs, :edit]
before_action :check_merge_request_rebase_available!, only: [:rebase]
end end
def rebase def rebase
......
...@@ -5,7 +5,9 @@ module EE ...@@ -5,7 +5,9 @@ module EE
end end
def should_be_rebased? def should_be_rebased?
self.project.ff_merge_must_be_possible? && !ff_merge_possible? project.feature_available?(:merge_request_rebase) &&
project.ff_merge_must_be_possible? &&
!ff_merge_possible?
end end
def rebase_dir_path def rebase_dir_path
......
...@@ -10,6 +10,7 @@ class License < ActiveRecord::Base ...@@ -10,6 +10,7 @@ class License < ActiveRecord::Base
ELASTIC_SEARCH_FEATURE = 'GitLab_ElasticSearch'.freeze ELASTIC_SEARCH_FEATURE = 'GitLab_ElasticSearch'.freeze
RELATED_ISSUES_FEATURE = 'RelatedIssues'.freeze RELATED_ISSUES_FEATURE = 'RelatedIssues'.freeze
EXPORT_ISSUES_FEATURE = 'GitLab_ExportIssues'.freeze EXPORT_ISSUES_FEATURE = 'GitLab_ExportIssues'.freeze
MERGE_REQUEST_REBASE_FEATURE = 'GitLab_MergeRequestRebase'.freeze
FEATURE_CODES = { FEATURE_CODES = {
geo: GEO_FEATURE, geo: GEO_FEATURE,
...@@ -21,7 +22,8 @@ class License < ActiveRecord::Base ...@@ -21,7 +22,8 @@ class License < ActiveRecord::Base
# Features that make sense to Namespace: # Features that make sense to Namespace:
deploy_board: DEPLOY_BOARD_FEATURE, deploy_board: DEPLOY_BOARD_FEATURE,
file_lock: FILE_LOCK_FEATURE, file_lock: FILE_LOCK_FEATURE,
export_issues: EXPORT_ISSUES_FEATURE export_issues: EXPORT_ISSUES_FEATURE,
merge_request_rebase: MERGE_REQUEST_REBASE_FEATURE
}.freeze }.freeze
STARTER_PLAN = 'starter'.freeze STARTER_PLAN = 'starter'.freeze
...@@ -32,7 +34,8 @@ class License < ActiveRecord::Base ...@@ -32,7 +34,8 @@ class License < ActiveRecord::Base
EES_FEATURES = [ EES_FEATURES = [
{ ELASTIC_SEARCH_FEATURE => 1 }, { ELASTIC_SEARCH_FEATURE => 1 },
{ RELATED_ISSUES_FEATURE => 1 }, { RELATED_ISSUES_FEATURE => 1 },
{ EXPORT_ISSUES_FEATURE => 1 } { EXPORT_ISSUES_FEATURE => 1 },
{ MERGE_REQUEST_REBASE_FEATURE => 1 }
].freeze ].freeze
EEP_FEATURES = [ EEP_FEATURES = [
...@@ -65,7 +68,8 @@ class License < ActiveRecord::Base ...@@ -65,7 +68,8 @@ class License < ActiveRecord::Base
{ AUDITOR_USER_FEATURE => 1 }, { AUDITOR_USER_FEATURE => 1 },
{ SERVICE_DESK_FEATURE => 1 }, { SERVICE_DESK_FEATURE => 1 },
{ OBJECT_STORAGE_FEATURE => 1 }, { OBJECT_STORAGE_FEATURE => 1 },
{ EXPORT_ISSUES_FEATURE => 1 } { EXPORT_ISSUES_FEATURE => 1 },
{ MERGE_REQUEST_REBASE_FEATURE => 1 }
].freeze ].freeze
FEATURES_BY_PLAN = { FEATURES_BY_PLAN = {
......
...@@ -20,9 +20,10 @@ ...@@ -20,9 +20,10 @@
%span.descr %span.descr
A merge commit is created for every merge, but merging is only allowed if fast-forward merge is possible. 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. This way you could make sure that if this merge request would build, after merging to target branch it would also build.
%br - if project.feature_available?(:merge_request_rebase)
%span.descr %br
When fast-forward merge is not possible, the user is given the option to rebase. %span.descr
When fast-forward merge is not possible, the user is given the option to rebase.
.radio .radio
= label_tag :project_merge_method_ff do = label_tag :project_merge_method_ff do
...@@ -31,9 +32,10 @@ ...@@ -31,9 +32,10 @@
%br %br
%span.descr %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. 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 - if project.feature_available?(:merge_request_rebase)
%span.descr %br
When fast-forward merge is not possible, the user is given the option to rebase. %span.descr
When fast-forward merge is not possible, the user is given the option to rebase.
.form-group .form-group
= form.label :merge_requests_template, class: 'label-light' do = form.label :merge_requests_template, class: 'label-light' do
......
---
title: Introduce namespace license checks for rebase before merge
merge_request: 2200
author:
require 'spec_helper' require 'spec_helper'
describe Projects::MergeRequestsController do describe Projects::MergeRequestsController do
let(:project) { create(:project) } let(:project) { create(:project) }
let(:user) { project.owner }
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
let(:user) { project.owner }
let(:viewer) { user }
before do before do
sign_in(user) sign_in(viewer)
end end
describe 'POST #create' do describe 'POST #create' do
...@@ -347,4 +348,63 @@ describe Projects::MergeRequestsController do ...@@ -347,4 +348,63 @@ describe Projects::MergeRequestsController do
end end
end end
end end
describe 'POST #rebase' do
def post_rebase
post :rebase, namespace_id: project.namespace, project_id: project, id: merge_request
end
def expect_rebase_worker
expect(RebaseWorker).to receive(:perform_async).with(merge_request.id, viewer.id)
end
context 'successfully' do
it 'enqeues a RebaseWorker' do
expect_rebase_worker
post_rebase
expect(response.status).to eq(200)
end
end
context 'approvals pending' do
let(:project) { create(:project, approvals_before_merge: 1) }
it 'returns 404' do
expect_rebase_worker.never
post_rebase
expect(response.status).to eq(404)
end
end
context 'user cannot merge' do
let(:viewer) { create(:user) }
before do
project.add_reporter(viewer)
end
it 'returns 404' do
expect_rebase_worker.never
post_rebase
expect(response.status).to eq(404)
end
end
context 'rebase unavailable in license' do
it 'returns 404' do
stub_licensed_features(merge_request_rebase: false)
expect_rebase_worker.never
post_rebase
expect(response.status).to eq(404)
end
end
end
end end
require 'spec_helper' require 'spec_helper'
describe MergeRequest, models: true do describe MergeRequest, models: true do
subject { create(:merge_request) } let(:project) { create(:project) }
subject(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
describe '#should_be_rebased?' do
subject { merge_request.should_be_rebased? }
context 'project forbids rebase' do
it { is_expected.to be_falsy }
end
context 'project allows rebase' do
let(:project) { create(:project, merge_requests_rebase_enabled: true) }
it 'returns false when the project feature is unavailable' do
expect(merge_request.target_project).to receive(:feature_available?).with(:merge_request_rebase).and_return(false)
is_expected.to be_falsy
end
it 'returns true when the project feature is available' do
expect(merge_request.target_project).to receive(:feature_available?).with(:merge_request_rebase).and_return(true)
is_expected.to be_truthy
end
end
end
describe '#rebase_in_progress?' do describe '#rebase_in_progress?' do
it 'returns true when there is a current rebase directory' do it 'returns true when there is a current rebase directory' do
......
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