diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-03-07 21:51:55 +0300 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-03-07 21:51:55 +0300 |
commit | ace3bcdda42ce7c9031d938f8dd53f207121654e (patch) | |
tree | d66e4ae1cffe79ea5e535cf38e3e2cbc64816d27 /spec | |
parent | d179f002d9a51ba3e082b5ae5943c0f31c694e4e (diff) | |
parent | 22198466ca79b5f41282198f363a81fdb2ab3f2f (diff) |
Merge branch 'master' into 'ce-3839-ci-cd-only-github-projects-fe'
# Conflicts:
# locale/gitlab.pot
Diffstat (limited to 'spec')
23 files changed, 672 insertions, 35 deletions
diff --git a/spec/controllers/projects/milestones_controller_spec.rb b/spec/controllers/projects/milestones_controller_spec.rb index 00cf464ec5b..306094f7ffb 100644 --- a/spec/controllers/projects/milestones_controller_spec.rb +++ b/spec/controllers/projects/milestones_controller_spec.rb @@ -98,10 +98,8 @@ describe Projects::MilestonesController do it 'shows group milestone' do post :promote, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid - group_milestone = assigns(:milestone) - - expect(response).to redirect_to(group_milestone_path(project.group, group_milestone.iid)) - expect(flash[:notice]).to eq('Milestone has been promoted to group milestone.') + expect(flash[:notice]).to eq("#{milestone.title} promoted to group milestone") + expect(response).to redirect_to(project_milestones_path(project)) end end diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb new file mode 100644 index 00000000000..a3323da1b1f --- /dev/null +++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb @@ -0,0 +1,44 @@ +require 'spec_helper' + +describe 'a maintainer edits files on a source-branch of an MR from a fork', :js do + include ProjectForksHelper + let(:user) { create(:user, username: 'the-maintainer') } + let(:target_project) { create(:project, :public, :repository) } + let(:author) { create(:user, username: 'mr-authoring-machine') } + let(:source_project) { fork_project(target_project, author, repository: true) } + + let(:merge_request) do + create(:merge_request, + source_project: source_project, + target_project: target_project, + source_branch: 'fix', + target_branch: 'master', + author: author, + allow_maintainer_to_push: true) + end + + before do + target_project.add_master(user) + sign_in(user) + + visit project_merge_request_path(target_project, merge_request) + click_link 'Changes' + wait_for_requests + first('.js-file-title').click_link 'Edit' + wait_for_requests + end + + it 'mentions commits will go to the source branch' do + expect(page).to have_content('Your changes can be committed to fix because a merge request is open.') + end + + it 'allows committing to the source branch' do + find('.ace_text-input', visible: false).send_keys('Updated the readme') + + click_button 'Commit changes' + wait_for_requests + + expect(page).to have_content('Your changes have been successfully committed') + expect(page).to have_content('Updated the readme') + end +end 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/features/projects/user_creates_files_spec.rb b/spec/features/projects/user_creates_files_spec.rb index 7a935dd2477..8993533676b 100644 --- a/spec/features/projects/user_creates_files_spec.rb +++ b/spec/features/projects/user_creates_files_spec.rb @@ -133,13 +133,20 @@ describe 'User creates files' do before do project2.add_reporter(user) visit(project2_tree_path_root_ref) - end - it 'creates and commit new file in forked project', :js do find('.add-to-tree').click click_link('New file') + end + + it 'shows a message saying the file will be committed in a fork' do + message = "A new branch will be created in your fork and a new merge request will be started." + expect(page).to have_content(message) + end + + it 'creates and commit new file in forked project', :js do expect(page).to have_selector('.file-editor') + expect(page).to have_content find('#editor') execute_script("ace.edit('editor').setValue('*.rbca')") 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/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json index e86176e5316..0dc2eabec5d 100644 --- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json +++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json @@ -80,7 +80,8 @@ "total_time_spent": { "type": "integer" }, "human_time_estimate": { "type": ["string", "null"] }, "human_total_time_spent": { "type": ["string", "null"] } - } + }, + "allow_maintainer_to_push": { "type": ["boolean", "null"] } }, "required": [ "id", "iid", "project_id", "title", "description", diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb index d3b1be599dd..ccac6e29447 100644 --- a/spec/helpers/tree_helper_spec.rb +++ b/spec/helpers/tree_helper_spec.rb @@ -62,4 +62,13 @@ describe TreeHelper do end end end + + describe '#commit_in_single_accessible_branch' do + it 'escapes HTML from the branch name' do + helper.instance_variable_set(:@branch_name, "<script>alert('escape me!');</script>") + escaped_branch_name = '<script>alert('escape me!');</script>' + + expect(helper.commit_in_single_accessible_branch).to include(escaped_branch_name) + end + end end diff --git a/spec/javascripts/pages/labels/components/promote_label_modal_spec.js b/spec/javascripts/pages/labels/components/promote_label_modal_spec.js new file mode 100644 index 00000000000..ba2e07f02f7 --- /dev/null +++ b/spec/javascripts/pages/labels/components/promote_label_modal_spec.js @@ -0,0 +1,88 @@ +import Vue from 'vue'; +import promoteLabelModal from '~/pages/projects/labels/components/promote_label_modal.vue'; +import eventHub from '~/pages/projects/labels/event_hub'; +import axios from '~/lib/utils/axios_utils'; +import mountComponent from '../../../helpers/vue_mount_component_helper'; + +describe('Promote label modal', () => { + let vm; + const Component = Vue.extend(promoteLabelModal); + const labelMockData = { + labelTitle: 'Documentation', + labelColor: '#5cb85c', + labelTextColor: '#ffffff', + url: `${gl.TEST_HOST}/dummy/promote/labels`, + }; + + describe('Modal title and description', () => { + beforeEach(() => { + vm = mountComponent(Component, labelMockData); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('contains the proper description', () => { + expect(vm.text).toContain('Promoting this label will make it available for all projects inside the group'); + }); + + it('contains a label span with the color', () => { + const labelFromTitle = vm.$el.querySelector('.modal-header .label.color-label'); + + expect(labelFromTitle.style.backgroundColor).not.toBe(null); + expect(labelFromTitle.textContent).toContain(vm.labelTitle); + }); + }); + + describe('When requesting a label promotion', () => { + beforeEach(() => { + vm = mountComponent(Component, { + ...labelMockData, + }); + spyOn(eventHub, '$emit'); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('redirects when a label is promoted', (done) => { + const responseURL = `${gl.TEST_HOST}/dummy/endpoint`; + spyOn(axios, 'post').and.callFake((url) => { + expect(url).toBe(labelMockData.url); + expect(eventHub.$emit).toHaveBeenCalledWith('promoteLabelModal.requestStarted', labelMockData.url); + return Promise.resolve({ + request: { + responseURL, + }, + }); + }); + + vm.onSubmit() + .then(() => { + expect(eventHub.$emit).toHaveBeenCalledWith('promoteLabelModal.requestFinished', { labelUrl: labelMockData.url, successful: true }); + }) + .then(done) + .catch(done.fail); + }); + + it('displays an error if promoting a label failed', (done) => { + const dummyError = new Error('promoting label failed'); + dummyError.response = { status: 500 }; + spyOn(axios, 'post').and.callFake((url) => { + expect(url).toBe(labelMockData.url); + expect(eventHub.$emit).toHaveBeenCalledWith('promoteLabelModal.requestStarted', labelMockData.url); + return Promise.reject(dummyError); + }); + + vm.onSubmit() + .catch((error) => { + expect(error).toBe(dummyError); + expect(eventHub.$emit).toHaveBeenCalledWith('promoteLabelModal.requestFinished', { labelUrl: labelMockData.url, successful: false }); + }) + .then(done) + .catch(done.fail); + }); + }); +}); diff --git a/spec/javascripts/pages/milestones/shared/components/promote_milestone_modal_spec.js b/spec/javascripts/pages/milestones/shared/components/promote_milestone_modal_spec.js new file mode 100644 index 00000000000..bf044fe8fb5 --- /dev/null +++ b/spec/javascripts/pages/milestones/shared/components/promote_milestone_modal_spec.js @@ -0,0 +1,83 @@ +import Vue from 'vue'; +import promoteMilestoneModal from '~/pages/milestones/shared/components/promote_milestone_modal.vue'; +import eventHub from '~/pages/milestones/shared/event_hub'; +import axios from '~/lib/utils/axios_utils'; +import mountComponent from '../../../../helpers/vue_mount_component_helper'; + +describe('Promote milestone modal', () => { + let vm; + const Component = Vue.extend(promoteMilestoneModal); + const milestoneMockData = { + milestoneTitle: 'v1.0', + url: `${gl.TEST_HOST}/dummy/promote/milestones`, + }; + + describe('Modal title and description', () => { + beforeEach(() => { + vm = mountComponent(Component, milestoneMockData); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('contains the proper description', () => { + expect(vm.text).toContain('Promoting this milestone will make it available for all projects inside the group.'); + }); + + it('contains the correct title', () => { + expect(vm.title).toEqual('Promote v1.0 to group milestone?'); + }); + }); + + describe('When requesting a milestone promotion', () => { + beforeEach(() => { + vm = mountComponent(Component, { + ...milestoneMockData, + }); + spyOn(eventHub, '$emit'); + }); + + afterEach(() => { + vm.$destroy(); + }); + + it('redirects when a milestone is promoted', (done) => { + const responseURL = `${gl.TEST_HOST}/dummy/endpoint`; + spyOn(axios, 'post').and.callFake((url) => { + expect(url).toBe(milestoneMockData.url); + expect(eventHub.$emit).toHaveBeenCalledWith('promoteMilestoneModal.requestStarted', milestoneMockData.url); + return Promise.resolve({ + request: { + responseURL, + }, + }); + }); + + vm.onSubmit() + .then(() => { + expect(eventHub.$emit).toHaveBeenCalledWith('promoteMilestoneModal.requestFinished', { milestoneUrl: milestoneMockData.url, successful: true }); + }) + .then(done) + .catch(done.fail); + }); + + it('displays an error if promoting a milestone failed', (done) => { + const dummyError = new Error('promoting milestone failed'); + dummyError.response = { status: 500 }; + spyOn(axios, 'post').and.callFake((url) => { + expect(url).toBe(milestoneMockData.url); + expect(eventHub.$emit).toHaveBeenCalledWith('promoteMilestoneModal.requestStarted', milestoneMockData.url); + return Promise.reject(dummyError); + }); + + vm.onSubmit() + .catch((error) => { + expect(error).toBe(dummyError); + expect(eventHub.$emit).toHaveBeenCalledWith('promoteMilestoneModal.requestFinished', { milestoneUrl: milestoneMockData.url, successful: false }); + }) + .then(done) + .catch(done.fail); + }); + }); +}); 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/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index b49ddbfc780..48e9902027c 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -30,9 +30,10 @@ describe Gitlab::Checks::ChangeAccess do end end - context 'when the user is not allowed to push code' do + context 'when the user is not allowed to push to the repo' do it 'raises an error' do expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false) + expect(user_access).to receive(:can_push_to_branch?).with('master').and_return(false) expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.') end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index b20cc34dd5c..bece82e531a 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -278,6 +278,7 @@ project: - custom_attributes - lfs_file_locks - project_badges +- source_of_merge_requests award_emoji: - awardable - user 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/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb index 7280acb6c82..40c8286b1b9 100644 --- a/spec/lib/gitlab/user_access_spec.rb +++ b/spec/lib/gitlab/user_access_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Gitlab::UserAccess do + include ProjectForksHelper + let(:access) { described_class.new(user, project: project) } let(:project) { create(:project, :repository) } let(:user) { create(:user) } @@ -118,6 +120,39 @@ describe Gitlab::UserAccess do end end + describe 'allowing pushes to maintainers of forked projects' do + let(:canonical_project) { create(:project, :public, :repository) } + let(:project) { fork_project(canonical_project, create(:user), repository: true) } + + before do + create( + :merge_request, + target_project: canonical_project, + source_project: project, + source_branch: 'awesome-feature', + allow_maintainer_to_push: true + ) + end + + it 'allows users that have push access to the canonical project to push to the MR branch' do + canonical_project.add_developer(user) + + expect(access.can_push_to_branch?('awesome-feature')).to be_truthy + end + + it 'does not allow the user to push to other branches' do + canonical_project.add_developer(user) + + expect(access.can_push_to_branch?('master')).to be_falsey + end + + it 'does not allow the user to push if he does not have push access to the canonical project' do + canonical_project.add_guest(user) + + expect(access.can_push_to_branch?('awesome-feature')).to be_falsey + end + end + describe 'merge to protected branch if allowed for developers' do before do @branch = create :protected_branch, :developers_can_merge, project: project 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/models/project_spec.rb b/spec/models/project_spec.rb index b1c9e6754b9..e970cd7dfdb 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -1,6 +1,8 @@ require 'spec_helper' describe Project do + include ProjectForksHelper + describe 'associations' do it { is_expected.to belong_to(:group) } it { is_expected.to belong_to(:namespace) } @@ -3378,4 +3380,103 @@ describe Project do end end end + + context 'with cross project merge requests' do + let(:user) { create(:user) } + let(:target_project) { create(:project, :repository) } + let(:project) { fork_project(target_project, nil, repository: true) } + let!(:merge_request) do + create( + :merge_request, + target_project: target_project, + target_branch: 'target-branch', + source_project: project, + source_branch: 'awesome-feature-1', + allow_maintainer_to_push: true + ) + end + + before do + target_project.add_developer(user) + end + + describe '#merge_requests_allowing_push_to_user' do + it 'returns open merge requests for which the user has developer access to the target project' do + expect(project.merge_requests_allowing_push_to_user(user)).to include(merge_request) + end + + it 'does not include closed merge requests' do + merge_request.close + + expect(project.merge_requests_allowing_push_to_user(user)).to be_empty + end + + it 'does not include merge requests for guest users' do + guest = create(:user) + target_project.add_guest(guest) + + expect(project.merge_requests_allowing_push_to_user(guest)).to be_empty + end + + it 'does not include the merge request for other users' do + other_user = create(:user) + + expect(project.merge_requests_allowing_push_to_user(other_user)).to be_empty + end + + it 'is empty when no user is passed' do + expect(project.merge_requests_allowing_push_to_user(nil)).to be_empty + end + end + + describe '#branch_allows_maintainer_push?' do + it 'allows access if the user can merge the merge request' do + expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) + .to be_truthy + end + + it 'does not allow guest users access' do + guest = create(:user) + target_project.add_guest(guest) + + expect(project.branch_allows_maintainer_push?(guest, 'awesome-feature-1')) + .to be_falsy + end + + it 'does not allow access to branches for which the merge request was closed' do + create(:merge_request, :closed, + target_project: target_project, + target_branch: 'target-branch', + source_project: project, + source_branch: 'rejected-feature-1', + allow_maintainer_to_push: true) + + expect(project.branch_allows_maintainer_push?(user, 'rejected-feature-1')) + .to be_falsy + end + + it 'does not allow access if the user cannot merge the merge request' do + create(:protected_branch, :masters_can_push, project: target_project, name: 'target-branch') + + expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) + .to be_falsy + end + + it 'caches the result' do + control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } + + expect { 3.times { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } } + .not_to exceed_query_limit(control) + end + + context 'when the requeststore is active', :request_store do + it 'only queries per project across instances' do + control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } + + expect { 2.times { described_class.find(project.id).branch_allows_maintainer_push?(user, 'awesome-feature-1') } } + .not_to exceed_query_limit(control).with_threshold(2) + end + end + end + end end diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 129344f105f..ea76e604153 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -308,4 +308,41 @@ describe ProjectPolicy do it_behaves_like 'project policies as master' it_behaves_like 'project policies as owner' it_behaves_like 'project policies as admin' + + context 'when a public project has merge requests allowing access' do + include ProjectForksHelper + let(:user) { create(:user) } + let(:target_project) { create(:project, :public) } + let(:project) { fork_project(target_project) } + let!(:merge_request) do + create( + :merge_request, + target_project: target_project, + source_project: project, + allow_maintainer_to_push: true + ) + end + let(:maintainer_abilities) do + %w(create_build update_build create_pipeline update_pipeline) + end + + subject { described_class.new(user, project) } + + it 'does not allow pushing code' do + expect_disallowed(*maintainer_abilities) + end + + it 'allows pushing if the user is a member with push access to the target project' do + target_project.add_developer(user) + + expect_allowed(*maintainer_abilities) + end + + it 'dissallows abilities to a maintainer if the merge request was closed' do + target_project.add_developer(user) + merge_request.close! + + expect_disallowed(*maintainer_abilities) + end + end end diff --git a/spec/requests/api/internal_spec.rb b/spec/requests/api/internal_spec.rb index 827f4c04324..ca0aac87ba9 100644 --- a/spec/requests/api/internal_spec.rb +++ b/spec/requests/api/internal_spec.rb @@ -335,21 +335,8 @@ describe API::Internal do end context "git push" do - context "gitaly disabled", :disable_gitaly do - it "has the correct payload" do - push(key, project) - - expect(response).to have_gitlab_http_status(200) - expect(json_response["status"]).to be_truthy - expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) - expect(json_response["gl_repository"]).to eq("project-#{project.id}") - expect(json_response["gitaly"]).to be_nil - expect(user).not_to have_an_activity_record - end - end - - context "gitaly enabled" do - it "has the correct payload" do + context 'project as namespace/project' do + it do push(key, project) expect(response).to have_gitlab_http_status(200) @@ -365,17 +352,6 @@ describe API::Internal do expect(user).not_to have_an_activity_record end end - - context 'project as namespace/project' do - it do - push(key, project) - - expect(response).to have_gitlab_http_status(200) - expect(json_response["status"]).to be_truthy - expect(json_response["repository_path"]).to eq(project.repository.path_to_repo) - expect(json_response["gl_repository"]).to eq("project-#{project.id}") - end - end end end diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb index 484322752c0..3764aec0c71 100644 --- a/spec/requests/api/merge_requests_spec.rb +++ b/spec/requests/api/merge_requests_spec.rb @@ -616,6 +616,25 @@ describe API::MergeRequests do expect(json_response['changes_count']).to eq('5+') end end + + context 'for forked projects' do + let(:user2) { create(:user) } + let(:project) { create(:project, :public, :repository) } + let(:forked_project) { fork_project(project, user2, repository: true) } + let(:merge_request) do + create(:merge_request, + source_project: forked_project, + target_project: project, + source_branch: 'fixes', + allow_maintainer_to_push: true) + end + + it 'includes the `allow_maintainer_to_push` field' do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) + + expect(json_response['allow_maintainer_to_push']).to be_truthy + end + end end describe 'GET /projects/:id/merge_requests/:merge_request_iid/participants' do @@ -815,6 +834,7 @@ describe API::MergeRequests do context 'forked projects' do let!(:user2) { create(:user) } + let(:project) { create(:project, :public, :repository) } let!(:forked_project) { fork_project(project, user2, repository: true) } let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) } @@ -872,6 +892,14 @@ describe API::MergeRequests do expect(response).to have_gitlab_http_status(400) end + it 'allows setting `allow_maintainer_to_push`' do + post api("/projects/#{forked_project.id}/merge_requests", user2), + title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", + author: user2, target_project_id: project.id, allow_maintainer_to_push: true + expect(response).to have_gitlab_http_status(201) + expect(json_response['allow_maintainer_to_push']).to be_truthy + end + context 'when target_branch and target_project_id is specified' do let(:params) do { title: 'Test merge_request', 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 diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb index 44b32df0395..3b098320ad7 100644 --- a/spec/views/projects/tree/show.html.haml_spec.rb +++ b/spec/views/projects/tree/show.html.haml_spec.rb @@ -13,6 +13,7 @@ describe 'projects/tree/show' do allow(view).to receive(:can?).and_return(true) allow(view).to receive(:can_collaborate_with_project?).and_return(true) + allow(view).to receive_message_chain('user_access.can_push_to_branch?').and_return(true) allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings) end |