diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-02-23 18:30:37 +0100 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-03-07 15:12:31 +0100 |
commit | 792ab0631c85098fbf92e727b77158fb9dae5219 (patch) | |
tree | 390bd2b69c2de7f229f4d073ad60282b473312d7 /spec | |
parent | 9ba0052cdd5ae12ea6b1edd8d25867e0fef25afc (diff) | |
download | gitlab-ce-792ab0631c85098fbf92e727b77158fb9dae5219.tar.gz |
Allow a user to select `allow maintainer to push`
When a project is not private, and the source branch not protected the
user can now select the option to allow maintainers to push to this
branch
Diffstat (limited to 'spec')
8 files changed, 228 insertions, 1 deletions
diff --git a/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb b/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb new file mode 100644 index 00000000000..eb41d7de8ed --- /dev/null +++ b/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' + +describe 'create a merge request that allows maintainers to push', :js do + include ProjectForksHelper + let(:user) { create(:user) } + let(:target_project) { create(:project, :public, :repository) } + let(:source_project) { fork_project(target_project, user, repository: true, namespace: user.namespace) } + + def visit_new_merge_request + visit project_new_merge_request_path( + source_project, + merge_request: { + source_project_id: source_project.id, + target_project_id: target_project.id, + source_branch: 'fix', + target_branch: 'master' + }) + end + + before do + sign_in(user) + end + + it 'allows setting maintainer push possible' do + visit_new_merge_request + + check 'Allow edits from maintainers' + + click_button 'Submit merge request' + + wait_for_requests + + expect(page).to have_content('Allows edits from maintainers') + end + + it 'shows a message when one of the projects is private' do + source_project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + visit_new_merge_request + + expect(page).to have_content('Not available for private projects') + end + + it 'shows a message when the source branch is protected' do + create(:protected_branch, project: source_project, name: 'fix') + + visit_new_merge_request + + expect(page).to have_content('Not available for protected branches') + end + + context 'when the merge request is being created within the same project' do + let(:source_project) { target_project } + + it 'hides the checkbox if the merge request is being created within the same project' do + target_project.add_developer(user) + + visit_new_merge_request + + expect(page).not_to have_content('Allows edits from maintainers') + end + end + + context 'when a maintainer tries to edit the option' do + let(:maintainer) { create(:user) } + let(:merge_request) do + create(:merge_request, + source_project: source_project, + target_project: target_project, + source_branch: 'fixes') + end + + before do + target_project.add_master(maintainer) + + sign_in(maintainer) + end + + it 'it hides the option from maintainers' do + visit edit_project_merge_request_path(target_project, merge_request) + + expect(page).not_to have_content('Allows edits from maintainers') + end + end +end diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json index f1199468d53..46031961cca 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_basic.json +++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json @@ -12,7 +12,8 @@ "rebase_in_progress": { "type": "boolean" }, "assignee_id": { "type": ["integer", "null"] }, "subscribed": { "type": ["boolean", "null"] }, - "participants": { "type": "array" } + "participants": { "type": "array" }, + "allow_maintainer_to_push": { "type": "boolean"} }, "additionalProperties": false } diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json index cfbeec58a45..a622bf88b13 100644 --- a/spec/fixtures/api/schemas/entities/merge_request_widget.json +++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json @@ -30,6 +30,7 @@ "source_project_id": { "type": "integer" }, "target_branch": { "type": "string" }, "target_project_id": { "type": "integer" }, + "allow_maintainer_to_push": { "type": "boolean"}, "metrics": { "oneOf": [ { "type": "null" }, diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js new file mode 100644 index 00000000000..65b3f721281 --- /dev/null +++ b/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js @@ -0,0 +1,23 @@ +import Vue from 'vue'; +import maintainerEditComponent from '~/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue'; +import mountComponent from 'spec/helpers/vue_mount_component_helper'; + +describe('MRWidgetAuthor', () => { + let vm; + + beforeEach(() => { + const Component = Vue.extend(maintainerEditComponent); + + vm = mountComponent(Component, { + maintainerEditAllowed: true, + }); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('renders the message when maintainers are allowed to edit', () => { + expect(vm.$el.textContent.trim()).toEqual('Allows edits from maintainers'); + }); +}); diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js index 18ba34b55a5..ebe151ac3b1 100644 --- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js +++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js @@ -349,6 +349,7 @@ describe('mrWidgetOptions', () => { expect(comps['mr-widget-pipeline-blocked']).toBeDefined(); expect(comps['mr-widget-pipeline-failed']).toBeDefined(); expect(comps['mr-widget-merge-when-pipeline-succeeds']).toBeDefined(); + expect(comps['mr-widget-maintainer-edit']).toBeDefined(); }); }); diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index ddcbb7a0033..0b938892da5 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -168,6 +168,7 @@ MergeRequest: - last_edited_by_id - head_pipeline_id - discussion_locked +- allow_maintainer_to_push MergeRequestDiff: - id - state diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index 243eeddc7a8..7986aa31e16 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2084,4 +2084,82 @@ describe MergeRequest do it_behaves_like 'checking whether a rebase is in progress' end end + + describe '#allow_maintainer_to_push' do + let(:merge_request) do + build(:merge_request, source_branch: 'fixes', allow_maintainer_to_push: true) + end + + it 'is false when pushing by a maintainer is not possible' do + expect(merge_request).to receive(:maintainer_push_possible?) { false } + + expect(merge_request.allow_maintainer_to_push).to be_falsy + end + + it 'is true when pushing by a maintainer is possible' do + expect(merge_request).to receive(:maintainer_push_possible?) { true } + + expect(merge_request.allow_maintainer_to_push).to be_truthy + end + end + + describe '#maintainer_push_possible?' do + let(:merge_request) do + build(:merge_request, source_branch: 'fixes') + end + + before do + allow(ProtectedBranch).to receive(:protected?) { false } + end + + it 'does not allow maintainer to push if the source project is the same as the target' do + merge_request.target_project = merge_request.source_project = create(:project, :public) + + expect(merge_request.maintainer_push_possible?).to be_falsy + end + + it 'allows maintainer to push when both source and target are public' do + merge_request.target_project = build(:project, :public) + merge_request.source_project = build(:project, :public) + + expect(merge_request.maintainer_push_possible?).to be_truthy + end + + it 'is not available for protected branches' do + merge_request.target_project = build(:project, :public) + merge_request.source_project = build(:project, :public) + + expect(ProtectedBranch).to receive(:protected?) + .with(merge_request.source_project, 'fixes') + .and_return(true) + + expect(merge_request.maintainer_push_possible?).to be_falsy + end + end + + describe '#can_allow_maintainer_to_push?' do + let(:target_project) { create(:project, :public) } + let(:source_project) { fork_project(target_project) } + let(:merge_request) do + create(:merge_request, + source_project: source_project, + source_branch: 'fixes', + target_project: target_project) + end + let(:user) { create(:user) } + + before do + allow(merge_request).to receive(:maintainer_push_possible?) { true } + end + + it 'is false if the user does not have push access to the source project' do + expect(merge_request.can_allow_maintainer_to_push?(user)).to be_falsy + end + + it 'is true when the user has push access to the source project' do + source_project.add_developer(user) + + expect(merge_request.can_allow_maintainer_to_push?(user)).to be_truthy + end + end end diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index c31259239ee..5279ea6164e 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe MergeRequests::UpdateService, :mailer do + include ProjectForksHelper + let(:project) { create(:project, :repository) } let(:user) { create(:user) } let(:user2) { create(:user) } @@ -538,5 +540,40 @@ describe MergeRequests::UpdateService, :mailer do let(:open_issuable) { merge_request } let(:closed_issuable) { create(:closed_merge_request, source_project: project) } end + + context 'setting `allow_maintainer_to_push`' do + let(:target_project) { create(:project, :public) } + let(:source_project) { fork_project(target_project) } + let(:user) { create(:user) } + let(:merge_request) do + create(:merge_request, + source_project: source_project, + source_branch: 'fixes', + target_project: target_project) + end + + before do + allow(ProtectedBranch).to receive(:protected?).with(source_project, 'fixes') { false } + end + + it 'does not allow a maintainer of the target project to set `allow_maintainer_to_push`' do + target_project.add_developer(user) + + update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') + + expect(merge_request.title).to eq('Updated title') + expect(merge_request.allow_maintainer_to_push).to be_falsy + end + + it 'is allowed by a user that can push to the source and can update the merge request' do + merge_request.update!(assignee: user) + source_project.add_developer(user) + + update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') + + expect(merge_request.title).to eq('Updated title') + expect(merge_request.allow_maintainer_to_push).to be_truthy + end + end end end |