Commit 6ced2f12 authored by Robert Speicher's avatar Robert Speicher

Merge branch '42751-rename-mr-maintainer-push' into 'master'

Rephrase Merge Request Maintainer Edit

See merge request gitlab-org/gitlab-ce!19061
parents cb770872 0d44f4d5
...@@ -265,10 +265,10 @@ export default { ...@@ -265,10 +265,10 @@ export default {
/> />
<section <section
v-if="mr.maintainerEditAllowed" v-if="mr.allowCollaboration"
class="mr-info-list mr-links" class="mr-info-list mr-links"
> >
{{ s__("mrWidget|Allows edits from maintainers") }} {{ s__("mrWidget|Allows commits from members who can merge to the target branch") }}
</section> </section>
<mr-widget-related-links <mr-widget-related-links
......
...@@ -83,7 +83,7 @@ export default class MergeRequestStore { ...@@ -83,7 +83,7 @@ export default class MergeRequestStore {
this.canBeMerged = data.can_be_merged || false; this.canBeMerged = data.can_be_merged || false;
this.isMergeAllowed = data.mergeable || false; this.isMergeAllowed = data.mergeable || false;
this.mergeOngoing = data.merge_ongoing; this.mergeOngoing = data.merge_ongoing;
this.maintainerEditAllowed = data.allow_maintainer_to_push; this.allowCollaboration = data.allow_collaboration;
// Cherry-pick and Revert actions related // Cherry-pick and Revert actions related
this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false; this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false;
......
...@@ -15,7 +15,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont ...@@ -15,7 +15,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
def merge_request_params_attributes def merge_request_params_attributes
[ [
:allow_maintainer_to_push, :allow_collaboration,
:assignee_id, :assignee_id,
:description, :description,
:force_remove_source_branch, :force_remove_source_branch,
......
...@@ -126,8 +126,8 @@ module MergeRequestsHelper ...@@ -126,8 +126,8 @@ module MergeRequestsHelper
link_to(url[merge_request.project, merge_request], data: data_attrs, &block) link_to(url[merge_request.project, merge_request], data: data_attrs, &block)
end end
def allow_maintainer_push_unavailable_reason(merge_request) def allow_collaboration_unavailable_reason(merge_request)
return if merge_request.can_allow_maintainer_to_push?(current_user) return if merge_request.can_allow_collaboration?(current_user)
minimum_visibility = [merge_request.target_project.visibility_level, minimum_visibility = [merge_request.target_project.visibility_level,
merge_request.source_project.visibility_level].min merge_request.source_project.visibility_level].min
......
...@@ -1125,21 +1125,21 @@ class MergeRequest < ActiveRecord::Base ...@@ -1125,21 +1125,21 @@ class MergeRequest < ActiveRecord::Base
project.merge_requests.merged.where(author_id: author_id).empty? project.merge_requests.merged.where(author_id: author_id).empty?
end end
def allow_maintainer_to_push def allow_collaboration
maintainer_push_possible? && super collaborative_push_possible? && super
end end
alias_method :allow_maintainer_to_push?, :allow_maintainer_to_push alias_method :allow_collaboration?, :allow_collaboration
def maintainer_push_possible? def collaborative_push_possible?
source_project.present? && for_fork? && source_project.present? && for_fork? &&
target_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE && target_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE &&
source_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE && source_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE &&
!ProtectedBranch.protected?(source_project, source_branch) !ProtectedBranch.protected?(source_project, source_branch)
end end
def can_allow_maintainer_to_push?(user) def can_allow_collaboration?(user)
maintainer_push_possible? && collaborative_push_possible? &&
Ability.allowed?(user, :push_code, source_project) Ability.allowed?(user, :push_code, source_project)
end end
......
...@@ -1968,18 +1968,18 @@ class Project < ActiveRecord::Base ...@@ -1968,18 +1968,18 @@ class Project < ActiveRecord::Base
.limit(1) .limit(1)
.select(1) .select(1)
source_of_merge_requests.opened source_of_merge_requests.opened
.where(allow_maintainer_to_push: true) .where(allow_collaboration: true)
.where('EXISTS (?)', developer_access_exists) .where('EXISTS (?)', developer_access_exists)
end end
def branch_allows_maintainer_push?(user, branch_name) def branch_allows_collaboration?(user, branch_name)
return false unless user return false unless user
cache_key = "user:#{user.id}:#{branch_name}:branch_allows_push" cache_key = "user:#{user.id}:#{branch_name}:branch_allows_push"
memoized_results = strong_memoize(:branch_allows_maintainer_push) do memoized_results = strong_memoize(:branch_allows_collaboration) do
Hash.new do |result, cache_key| Hash.new do |result, cache_key|
result[cache_key] = fetch_branch_allows_maintainer_push?(user, branch_name) result[cache_key] = fetch_branch_allows_collaboration?(user, branch_name)
end end
end end
...@@ -2121,18 +2121,18 @@ class Project < ActiveRecord::Base ...@@ -2121,18 +2121,18 @@ class Project < ActiveRecord::Base
raise ex raise ex
end end
def fetch_branch_allows_maintainer_push?(user, branch_name) def fetch_branch_allows_collaboration?(user, branch_name)
check_access = -> do check_access = -> do
next false if empty_repo? next false if empty_repo?
merge_request = source_of_merge_requests.opened merge_request = source_of_merge_requests.opened
.where(allow_maintainer_to_push: true) .where(allow_collaboration: true)
.find_by(source_branch: branch_name) .find_by(source_branch: branch_name)
merge_request&.can_be_merged_by?(user) merge_request&.can_be_merged_by?(user)
end end
if RequestStore.active? if RequestStore.active?
RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_maintainer_push") do RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_collaboration") do
check_access.call check_access.call
end end
else else
......
...@@ -14,8 +14,8 @@ module Ci ...@@ -14,8 +14,8 @@ module Ci
@subject.triggered_by?(@user) @subject.triggered_by?(@user)
end end
condition(:branch_allows_maintainer_push) do condition(:branch_allows_collaboration) do
@subject.project.branch_allows_maintainer_push?(@user, @subject.ref) @subject.project.branch_allows_collaboration?(@user, @subject.ref)
end end
rule { protected_ref }.policy do rule { protected_ref }.policy do
...@@ -25,7 +25,7 @@ module Ci ...@@ -25,7 +25,7 @@ module Ci
rule { can?(:admin_build) | (can?(:update_build) & owner_of_job) }.enable :erase_build rule { can?(:admin_build) | (can?(:update_build) & owner_of_job) }.enable :erase_build
rule { can?(:public_access) & branch_allows_maintainer_push }.policy do rule { can?(:public_access) & branch_allows_collaboration }.policy do
enable :update_build enable :update_build
enable :update_commit_status enable :update_commit_status
end end
......
...@@ -4,13 +4,13 @@ module Ci ...@@ -4,13 +4,13 @@ module Ci
condition(:protected_ref) { ref_protected?(@user, @subject.project, @subject.tag?, @subject.ref) } condition(:protected_ref) { ref_protected?(@user, @subject.project, @subject.tag?, @subject.ref) }
condition(:branch_allows_maintainer_push) do condition(:branch_allows_collaboration) do
@subject.project.branch_allows_maintainer_push?(@user, @subject.ref) @subject.project.branch_allows_collaboration?(@user, @subject.ref)
end end
rule { protected_ref }.prevent :update_pipeline rule { protected_ref }.prevent :update_pipeline
rule { can?(:public_access) & branch_allows_maintainer_push }.policy do rule { can?(:public_access) & branch_allows_collaboration }.policy do
enable :update_pipeline enable :update_pipeline
end end
......
...@@ -13,7 +13,7 @@ class MergeRequestWidgetEntity < IssuableEntity ...@@ -13,7 +13,7 @@ class MergeRequestWidgetEntity < IssuableEntity
expose :squash expose :squash
expose :target_branch expose :target_branch
expose :target_project_id expose :target_project_id
expose :allow_maintainer_to_push expose :allow_collaboration
expose :should_be_rebased?, as: :should_be_rebased expose :should_be_rebased?, as: :should_be_rebased
expose :ff_only_enabled do |merge_request| expose :ff_only_enabled do |merge_request|
......
...@@ -38,8 +38,8 @@ module MergeRequests ...@@ -38,8 +38,8 @@ module MergeRequests
def filter_params(merge_request) def filter_params(merge_request)
super super
unless merge_request.can_allow_maintainer_to_push?(current_user) unless merge_request.can_allow_collaboration?(current_user)
params.delete(:allow_maintainer_to_push) params.delete(:allow_collaboration)
end end
end end
......
...@@ -12,9 +12,9 @@ ...@@ -12,9 +12,9 @@
= _('Contribution') = _('Contribution')
.col-sm-10 .col-sm-10
.form-check .form-check
= form.check_box :allow_maintainer_to_push, disabled: !issuable.can_allow_maintainer_to_push?(current_user), class: 'form-check-input' = form.check_box :allow_collaboration, disabled: !issuable.can_allow_collaboration?(current_user), class: 'form-check-input'
= form.label :allow_maintainer_to_push, class: 'form-check-label' do = form.label :allow_collaboration, class: 'form-check-label' do
= _('Allow edits from maintainers.') = _('Allow commits from members who can merge to the target branch.')
= link_to 'About this feature', help_page_path('user/project/merge_requests/maintainer_access') = link_to 'About this feature', help_page_path('user/project/merge_requests/allow_collaboration')
.form-text.text-muted .form-text.text-muted
= allow_maintainer_push_unavailable_reason(issuable) = allow_collaboration_unavailable_reason(issuable)
- unless can?(current_user, :push_code, @project) - unless can?(current_user, :push_code, @project)
.inline.prepend-left-10 .inline.prepend-left-10
- if @project.branch_allows_maintainer_push?(current_user, selected_branch) - if @project.branch_allows_collaboration?(current_user, selected_branch)
= commit_in_single_accessible_branch = commit_in_single_accessible_branch
- else - else
= commit_in_fork_help = commit_in_fork_help
---
title: Rephrasing Merge Request's 'allow edits from maintainer' functionality
merge_request: 19061
author:
type: deprecated
class RenameMergeRequestsAllowMaintainerToPush < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
rename_column_concurrently :merge_requests, :allow_maintainer_to_push, :allow_collaboration
end
def down
cleanup_concurrent_column_rename :merge_requests, :allow_collaboration, :allow_maintainer_to_push
end
end
class CleanupMergeRequestsAllowMaintainerToPushRename < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
cleanup_concurrent_column_rename :merge_requests, :allow_maintainer_to_push, :allow_collaboration
end
def down
rename_column_concurrently :merge_requests, :allow_collaboration, :allow_maintainer_to_push
end
end
...@@ -1229,7 +1229,7 @@ ActiveRecord::Schema.define(version: 20180529152628) do ...@@ -1229,7 +1229,7 @@ ActiveRecord::Schema.define(version: 20180529152628) do
t.boolean "discussion_locked" t.boolean "discussion_locked"
t.integer "latest_merge_request_diff_id" t.integer "latest_merge_request_diff_id"
t.string "rebase_commit_sha" t.string "rebase_commit_sha"
t.boolean "allow_maintainer_to_push" t.boolean "allow_collaboration"
t.boolean "squash", default: false, null: false t.boolean "squash", default: false, null: false
end end
......
...@@ -651,7 +651,8 @@ POST /projects/:id/merge_requests ...@@ -651,7 +651,8 @@ POST /projects/:id/merge_requests
| `labels` | string | no | Labels for MR as a comma-separated list | | `labels` | string | no | Labels for MR as a comma-separated list |
| `milestone_id` | integer | no | The global ID of a milestone | | `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 | | `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 | | `allow_collaboration` | boolean | no | Allow commits from members who can merge to the target branch |
| `allow_maintainer_to_push` | boolean | no | Deprecated, see allow_collaboration |
| `squash` | boolean | no | Squash commits into a single commit when merging | | `squash` | boolean | no | Squash commits into a single commit when merging |
```json ```json
...@@ -709,6 +710,7 @@ POST /projects/:id/merge_requests ...@@ -709,6 +710,7 @@ POST /projects/:id/merge_requests
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false, "discussion_locked": false,
"allow_collaboration": false,
"allow_maintainer_to_push": false, "allow_maintainer_to_push": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
...@@ -741,7 +743,8 @@ PUT /projects/:id/merge_requests/:merge_request_iid ...@@ -741,7 +743,8 @@ PUT /projects/:id/merge_requests/:merge_request_iid
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging | | `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 | | `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. | | `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 | | `allow_collaboration` | boolean | no | Allow commits from members who can merge to the target branch |
| `allow_maintainer_to_push` | boolean | no | Deprecated, see allow_collaboration |
Must include at least one non-required attribute from above. Must include at least one non-required attribute from above.
...@@ -799,6 +802,7 @@ Must include at least one non-required attribute from above. ...@@ -799,6 +802,7 @@ Must include at least one non-required attribute from above.
"squash": false, "squash": false,
"web_url": "http://example.com/example/example/merge_requests/1", "web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false, "discussion_locked": false,
"allow_collaboration": false,
"allow_maintainer_to_push": false, "allow_maintainer_to_push": false,
"time_stats": { "time_stats": {
"time_estimate": 0, "time_estimate": 0,
......
# Allow collaboration on merge requests across forks
> [Introduced][ce-17395] in GitLab 10.6.
This feature is available for merge requests across forked projects that are
publicly accessible. It makes it easier for members of projects to
collaborate on merge requests across forks.
When enabled for a merge request, members with merge access to the target
branch of the project will be granted write permissions to the source branch
of the merge request.
The feature can only be enabled by users who already have push access to the
source project, and only lasts while the merge request is open.
Enable this functionality while creating or editing a merge request:
![Enable collaboration](./img/allow_collaboration.png)
[ce-17395]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17395
...@@ -28,7 +28,7 @@ With GitLab merge requests, you can: ...@@ -28,7 +28,7 @@ With GitLab merge requests, you can:
- Enable [fast-forward merge requests](#fast-forward-merge-requests) - Enable [fast-forward merge requests](#fast-forward-merge-requests)
- Enable [semi-linear history merge requests](#semi-linear-history-merge-requests) as another security layer to guarantee the pipeline is passing in the target branch - 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) - [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) - [Allow collaboration](allow_collaboration.md) so members of the target project can push directly to the fork
- [Squash and merge](squash_and_merge.md) for a cleaner commit history - [Squash and merge](squash_and_merge.md) for a cleaner commit history
With **[GitLab Enterprise Edition][ee]**, you can also: With **[GitLab Enterprise Edition][ee]**, you can also:
......
# Allow maintainer pushes for merge requests across forks This document was moved to [another location](allow_collaboration.md).
> [Introduced][ce-17395] in GitLab 10.6.
This feature is available for merge requests across forked projects that are
publicly accessible. It makes it easier for maintainers of projects to
collaborate on merge requests across forks.
When enabled for a merge request, members with merge access to the target
branch of the project will be granted write permissions to the source branch
of the merge request.
The feature can only be enabled by users who already have push access to the
source project, and only lasts while the merge request is open.
Enable this functionality while creating a merge request:
![Enable maintainer edits](./img/allow_maintainer_push.png)
[ce-17395]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/17395
...@@ -559,7 +559,9 @@ module API ...@@ -559,7 +559,9 @@ module API
expose :discussion_locked expose :discussion_locked
expose :should_remove_source_branch?, as: :should_remove_source_branch expose :should_remove_source_branch?, as: :should_remove_source_branch
expose :force_remove_source_branch?, as: :force_remove_source_branch expose :force_remove_source_branch?, as: :force_remove_source_branch
expose :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? } expose :allow_collaboration, if: -> (merge_request, _) { merge_request.for_fork? }
# Deprecated
expose :allow_collaboration, as: :allow_maintainer_to_push, if: -> (merge_request, _) { merge_request.for_fork? }
expose :web_url do |merge_request, options| expose :web_url do |merge_request, options|
Gitlab::UrlBuilder.build(merge_request) Gitlab::UrlBuilder.build(merge_request)
......
...@@ -162,7 +162,8 @@ module API ...@@ -162,7 +162,8 @@ module API
optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request' optional :milestone_id, type: Integer, desc: 'The ID of a milestone to assign the merge request'
optional :labels, type: String, desc: 'Comma-separated list of label names' optional :labels, type: String, desc: 'Comma-separated list of label names'
optional :remove_source_branch, type: Boolean, desc: 'Remove source branch when merging' 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 :allow_collaboration, type: Boolean, desc: 'Allow commits from members who can merge to the target branch'
optional :allow_maintainer_to_push, type: Boolean, as: :allow_collaboration, desc: '[deprecated] See allow_collaboration'
optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge' optional :squash, type: Grape::API::Boolean, desc: 'When true, the commits will be squashed into a single commit on merge'
use :optional_params_ee use :optional_params_ee
......
...@@ -65,7 +65,7 @@ module Gitlab ...@@ -65,7 +65,7 @@ module Gitlab
return false unless can_access_git? return false unless can_access_git?
return false unless project return false unless project
return false if !user.can?(:push_code, project) && !project.branch_allows_maintainer_push?(user, ref) return false if !user.can?(:push_code, project) && !project.branch_allows_collaboration?(user, ref)
if protected?(ProtectedBranch, project, ref) if protected?(ProtectedBranch, project, ref)
protected_branch_accessible_to?(ref, action: :push) protected_branch_accessible_to?(ref, action: :push)
......
...@@ -331,7 +331,7 @@ msgstr "" ...@@ -331,7 +331,7 @@ msgstr ""
msgid "All features are enabled for blank projects, from templates, or when importing, but you can disable them afterward in the project settings." msgid "All features are enabled for blank projects, from templates, or when importing, but you can disable them afterward in the project settings."
msgstr "" msgstr ""
msgid "Allow edits from maintainers." msgid "Allow commits from members who can merge to the target branch."
msgstr "" msgstr ""
msgid "Allow rendering of PlantUML diagrams in Asciidoc documents." msgid "Allow rendering of PlantUML diagrams in Asciidoc documents."
...@@ -4894,7 +4894,7 @@ msgstr "" ...@@ -4894,7 +4894,7 @@ msgstr ""
msgid "mrWidget|%{metricsLinkStart} Memory %{metricsLinkEnd} usage is %{emphasisStart} unchanged %{emphasisEnd} at %{memoryFrom}MB" msgid "mrWidget|%{metricsLinkStart} Memory %{metricsLinkEnd} usage is %{emphasisStart} unchanged %{emphasisEnd} at %{memoryFrom}MB"
msgstr "" msgstr ""
msgid "mrWidget|Allows edits from maintainers" msgid "mrWidget|Allows commits from members who can merge to the target branch"
msgstr "" msgstr ""
msgid "mrWidget|Cancel automatic merge" msgid "mrWidget|Cancel automatic merge"
......
...@@ -14,7 +14,7 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js ...@@ -14,7 +14,7 @@ describe 'a maintainer edits files on a source-branch of an MR from a fork', :js
source_branch: 'fix', source_branch: 'fix',
target_branch: 'master', target_branch: 'master',
author: author, author: author,
allow_maintainer_to_push: true) allow_collaboration: true)
end end
before do before do
......
require 'spec_helper' require 'spec_helper'
describe 'create a merge request that allows maintainers to push', :js do describe 'create a merge request, allowing commits from members who can merge to the target branch', :js do
include ProjectForksHelper include ProjectForksHelper
let(:user) { create(:user) } let(:user) { create(:user) }
let(:target_project) { create(:project, :public, :repository) } let(:target_project) { create(:project, :public, :repository) }
...@@ -21,16 +21,16 @@ describe 'create a merge request that allows maintainers to push', :js do ...@@ -21,16 +21,16 @@ describe 'create a merge request that allows maintainers to push', :js do
sign_in(user) sign_in(user)
end end
it 'allows setting maintainer push possible' do it 'allows setting possible' do
visit_new_merge_request visit_new_merge_request
check 'Allow edits from maintainers' check 'Allow commits from members who can merge to the target branch'
click_button 'Submit merge request' click_button 'Submit merge request'
wait_for_requests wait_for_requests
expect(page).to have_content('Allows edits from maintainers') expect(page).to have_content('Allows commits from members who can merge to the target branch')
end end
it 'shows a message when one of the projects is private' do it 'shows a message when one of the projects is private' do
...@@ -57,12 +57,12 @@ describe 'create a merge request that allows maintainers to push', :js do ...@@ -57,12 +57,12 @@ describe 'create a merge request that allows maintainers to push', :js do
visit_new_merge_request visit_new_merge_request
expect(page).not_to have_content('Allows edits from maintainers') expect(page).not_to have_content('Allows commits from members who can merge to the target branch')
end end
end end
context 'when a maintainer tries to edit the option' do context 'when a member who can merge tries to edit the option' do
let(:maintainer) { create(:user) } let(:member) { create(:user) }
let(:merge_request) do let(:merge_request) do
create(:merge_request, create(:merge_request,
source_project: source_project, source_project: source_project,
...@@ -71,15 +71,15 @@ describe 'create a merge request that allows maintainers to push', :js do ...@@ -71,15 +71,15 @@ describe 'create a merge request that allows maintainers to push', :js do
end end
before do before do
target_project.add_master(maintainer) target_project.add_master(member)
sign_in(maintainer) sign_in(member)
end end
it 'it hides the option from maintainers' do it 'it hides the option from members' do
visit edit_project_merge_request_path(target_project, merge_request) visit edit_project_merge_request_path(target_project, merge_request)
expect(page).not_to have_content('Allows edits from maintainers') expect(page).not_to have_content('Allows commits from members who can merge to the target branch')
end end
end end
end end
...@@ -13,6 +13,7 @@ ...@@ -13,6 +13,7 @@
"assignee_id": { "type": ["integer", "null"] }, "assignee_id": { "type": ["integer", "null"] },
"subscribed": { "type": ["boolean", "null"] }, "subscribed": { "type": ["boolean", "null"] },
"participants": { "type": "array" }, "participants": { "type": "array" },
"allow_collaboration": { "type": "boolean"},
"allow_maintainer_to_push": { "type": "boolean"} "allow_maintainer_to_push": { "type": "boolean"}
}, },
"additionalProperties": false "additionalProperties": false
......
...@@ -31,7 +31,7 @@ ...@@ -31,7 +31,7 @@
"source_project_id": { "type": "integer" }, "source_project_id": { "type": "integer" },
"target_branch": { "type": "string" }, "target_branch": { "type": "string" },
"target_project_id": { "type": "integer" }, "target_project_id": { "type": "integer" },
"allow_maintainer_to_push": { "type": "boolean"}, "allow_collaboration": { "type": "boolean"},
"metrics": { "metrics": {
"oneOf": [ "oneOf": [
{ "type": "null" }, { "type": "null" },
......
...@@ -82,6 +82,7 @@ ...@@ -82,6 +82,7 @@
"human_time_estimate": { "type": ["string", "null"] }, "human_time_estimate": { "type": ["string", "null"] },
"human_total_time_spent": { "type": ["string", "null"] } "human_total_time_spent": { "type": ["string", "null"] }
}, },
"allow_collaboration": { "type": ["boolean", "null"] },
"allow_maintainer_to_push": { "type": ["boolean", "null"] } "allow_maintainer_to_push": { "type": ["boolean", "null"] }
}, },
"required": [ "required": [
......
...@@ -170,7 +170,7 @@ MergeRequest: ...@@ -170,7 +170,7 @@ MergeRequest:
- last_edited_by_id - last_edited_by_id
- head_pipeline_id - head_pipeline_id
- discussion_locked - discussion_locked
- allow_maintainer_to_push - allow_collaboration
MergeRequestDiff: MergeRequestDiff:
- id - id
- state - state
......
...@@ -142,7 +142,7 @@ describe Gitlab::UserAccess do ...@@ -142,7 +142,7 @@ describe Gitlab::UserAccess do
target_project: canonical_project, target_project: canonical_project,
source_project: project, source_project: project,
source_branch: 'awesome-feature', source_branch: 'awesome-feature',
allow_maintainer_to_push: true allow_collaboration: true
) )
end end
......
...@@ -2237,25 +2237,25 @@ describe MergeRequest do ...@@ -2237,25 +2237,25 @@ describe MergeRequest do
end end
end end
describe '#allow_maintainer_to_push' do describe '#allow_collaboration' do
let(:merge_request) do let(:merge_request) do
build(:merge_request, source_branch: 'fixes', allow_maintainer_to_push: true) build(:merge_request, source_branch: 'fixes', allow_collaboration: true)
end end
it 'is false when pushing by a maintainer is not possible' do it 'is false when pushing by a maintainer is not possible' do
expect(merge_request).to receive(:maintainer_push_possible?) { false } expect(merge_request).to receive(:collaborative_push_possible?) { false }
expect(merge_request.allow_maintainer_to_push).to be_falsy expect(merge_request.allow_collaboration).to be_falsy
end end
it 'is true when pushing by a maintainer is possible' do it 'is true when pushing by a maintainer is possible' do
expect(merge_request).to receive(:maintainer_push_possible?) { true } expect(merge_request).to receive(:collaborative_push_possible?) { true }
expect(merge_request.allow_maintainer_to_push).to be_truthy expect(merge_request.allow_collaboration).to be_truthy
end end
end end
describe '#maintainer_push_possible?' do describe '#collaborative_push_possible?' do
let(:merge_request) do let(:merge_request) do
build(:merge_request, source_branch: 'fixes') build(:merge_request, source_branch: 'fixes')
end end
...@@ -2267,14 +2267,14 @@ describe MergeRequest do ...@@ -2267,14 +2267,14 @@ describe MergeRequest do
it 'does not allow maintainer to push if the source project is the same as the target' do 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) merge_request.target_project = merge_request.source_project = create(:project, :public)
expect(merge_request.maintainer_push_possible?).to be_falsy expect(merge_request.collaborative_push_possible?).to be_falsy
end end
it 'allows maintainer to push when both source and target are public' do it 'allows maintainer to push when both source and target are public' do
merge_request.target_project = build(:project, :public) merge_request.target_project = build(:project, :public)
merge_request.source_project = build(:project, :public) merge_request.source_project = build(:project, :public)
expect(merge_request.maintainer_push_possible?).to be_truthy expect(merge_request.collaborative_push_possible?).to be_truthy
end end
it 'is not available for protected branches' do it 'is not available for protected branches' do
...@@ -2285,11 +2285,11 @@ describe MergeRequest do ...@@ -2285,11 +2285,11 @@ describe MergeRequest do
.with(merge_request.source_project, 'fixes') .with(merge_request.source_project, 'fixes')
.and_return(true) .and_return(true)
expect(merge_request.maintainer_push_possible?).to be_falsy expect(merge_request.collaborative_push_possible?).to be_falsy
end end
end end
describe '#can_allow_maintainer_to_push?' do describe '#can_allow_collaboration?' do
let(:target_project) { create(:project, :public) } let(:target_project) { create(:project, :public) }
let(:source_project) { fork_project(target_project) } let(:source_project) { fork_project(target_project) }
let(:merge_request) do let(:merge_request) do
...@@ -2301,17 +2301,17 @@ describe MergeRequest do ...@@ -2301,17 +2301,17 @@ describe MergeRequest do
let(:user) { create(:user) } let(:user) { create(:user) }
before do before do
allow(merge_request).to receive(:maintainer_push_possible?) { true } allow(merge_request).to receive(:collaborative_push_possible?) { true }
end end
it 'is false if the user does not have push access to the source project' do 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 expect(merge_request.can_allow_collaboration?(user)).to be_falsy
end end
it 'is true when the user has push access to the source project' do it 'is true when the user has push access to the source project' do
source_project.add_developer(user) source_project.add_developer(user)
expect(merge_request.can_allow_maintainer_to_push?(user)).to be_truthy expect(merge_request.can_allow_collaboration?(user)).to be_truthy
end end
end end
......
...@@ -3583,7 +3583,7 @@ describe Project do ...@@ -3583,7 +3583,7 @@ describe Project do
target_branch: 'target-branch', target_branch: 'target-branch',
source_project: project, source_project: project,
source_branch: 'awesome-feature-1', source_branch: 'awesome-feature-1',
allow_maintainer_to_push: true allow_collaboration: true
) )
end end
...@@ -3620,9 +3620,9 @@ describe Project do ...@@ -3620,9 +3620,9 @@ describe Project do
end end
end end
describe '#branch_allows_maintainer_push?' do describe '#branch_allows_collaboration_push?' do
it 'allows access if the user can merge the merge request' do it 'allows access if the user can merge the merge request' do
expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) expect(project.branch_allows_collaboration?(user, 'awesome-feature-1'))
.to be_truthy .to be_truthy
end end
...@@ -3630,7 +3630,7 @@ describe Project do ...@@ -3630,7 +3630,7 @@ describe Project do
guest = create(:user) guest = create(:user)
target_project.add_guest(guest) target_project.add_guest(guest)
expect(project.branch_allows_maintainer_push?(guest, 'awesome-feature-1')) expect(project.branch_allows_collaboration?(guest, 'awesome-feature-1'))
.to be_falsy .to be_falsy
end end
...@@ -3640,31 +3640,31 @@ describe Project do ...@@ -3640,31 +3640,31 @@ describe Project do
target_branch: 'target-branch', target_branch: 'target-branch',
source_project: project, source_project: project,
source_branch: 'rejected-feature-1', source_branch: 'rejected-feature-1',
allow_maintainer_to_push: true) allow_collaboration: true)
expect(project.branch_allows_maintainer_push?(user, 'rejected-feature-1')) expect(project.branch_allows_collaboration?(user, 'rejected-feature-1'))
.to be_falsy .to be_falsy
end end
it 'does not allow access if the user cannot merge the merge request' do 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') create(:protected_branch, :masters_can_push, project: target_project, name: 'target-branch')
expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1')) expect(project.branch_allows_collaboration?(user, 'awesome-feature-1'))
.to be_falsy .to be_falsy
end end
it 'caches the result' do it 'caches the result' do
control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') }
expect { 3.times { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } } expect { 3.times { project.branch_allows_collaboration?(user, 'awesome-feature-1') } }
.not_to exceed_query_limit(control) .not_to exceed_query_limit(control)
end end
context 'when the requeststore is active', :request_store do context 'when the requeststore is active', :request_store do
it 'only queries per project across instances' do it 'only queries per project across instances' do
control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } control = ActiveRecord::QueryRecorder.new { project.branch_allows_collaboration?(user, 'awesome-feature-1') }
expect { 2.times { described_class.find(project.id).branch_allows_maintainer_push?(user, 'awesome-feature-1') } } expect { 2.times { described_class.find(project.id).branch_allows_collaboration?(user, 'awesome-feature-1') } }
.not_to exceed_query_limit(control).with_threshold(2) .not_to exceed_query_limit(control).with_threshold(2)
end end
end end
......
...@@ -101,7 +101,7 @@ describe Ci::BuildPolicy do ...@@ -101,7 +101,7 @@ describe Ci::BuildPolicy do
it 'enables update_build if user is maintainer' do it 'enables update_build if user is maintainer' do
allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false)
allow_any_instance_of(Project).to receive(:branch_allows_maintainer_push?).and_return(true) allow_any_instance_of(Project).to receive(:branch_allows_collaboration?).and_return(true)
expect(policy).to be_allowed :update_build expect(policy).to be_allowed :update_build
expect(policy).to be_allowed :update_commit_status expect(policy).to be_allowed :update_commit_status
......
...@@ -69,7 +69,7 @@ describe Ci::PipelinePolicy, :models do ...@@ -69,7 +69,7 @@ describe Ci::PipelinePolicy, :models do
it 'enables update_pipeline if user is maintainer' do it 'enables update_pipeline if user is maintainer' do
allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false)
allow_any_instance_of(Project).to receive(:branch_allows_maintainer_push?).and_return(true) allow_any_instance_of(Project).to receive(:branch_allows_collaboration?).and_return(true)
expect(policy).to be_allowed :update_pipeline expect(policy).to be_allowed :update_pipeline
end end
......
...@@ -400,7 +400,7 @@ describe ProjectPolicy do ...@@ -400,7 +400,7 @@ describe ProjectPolicy do
:merge_request, :merge_request,
target_project: target_project, target_project: target_project,
source_project: project, source_project: project,
allow_maintainer_to_push: true allow_collaboration: true
) )
end end
let(:maintainer_abilities) do let(:maintainer_abilities) do
......
...@@ -386,12 +386,13 @@ describe API::MergeRequests do ...@@ -386,12 +386,13 @@ describe API::MergeRequests do
source_project: forked_project, source_project: forked_project,
target_project: project, target_project: project,
source_branch: 'fixes', source_branch: 'fixes',
allow_maintainer_to_push: true) allow_collaboration: true)
end end
it 'includes the `allow_maintainer_to_push` field' do it 'includes the `allow_collaboration` field' do
get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user) get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
expect(json_response['allow_collaboration']).to be_truthy
expect(json_response['allow_maintainer_to_push']).to be_truthy expect(json_response['allow_maintainer_to_push']).to be_truthy
end end
end end
...@@ -654,11 +655,12 @@ describe API::MergeRequests do ...@@ -654,11 +655,12 @@ describe API::MergeRequests do
expect(response).to have_gitlab_http_status(400) expect(response).to have_gitlab_http_status(400)
end end
it 'allows setting `allow_maintainer_to_push`' do it 'allows setting `allow_collaboration`' do
post api("/projects/#{forked_project.id}/merge_requests", user2), post api("/projects/#{forked_project.id}/merge_requests", user2),
title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master", title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master",
author: user2, target_project_id: project.id, allow_maintainer_to_push: true author: user2, target_project_id: project.id, allow_collaboration: true
expect(response).to have_gitlab_http_status(201) expect(response).to have_gitlab_http_status(201)
expect(json_response['allow_collaboration']).to be_truthy
expect(json_response['allow_maintainer_to_push']).to be_truthy expect(json_response['allow_maintainer_to_push']).to be_truthy
end end
......
...@@ -280,12 +280,12 @@ describe Ci::RetryPipelineService, '#execute' do ...@@ -280,12 +280,12 @@ describe Ci::RetryPipelineService, '#execute' do
source_project: forked_project, source_project: forked_project,
target_project: project, target_project: project,
source_branch: 'fixes', source_branch: 'fixes',
allow_maintainer_to_push: true) allow_collaboration: true)
create_build('rspec 1', :failed, 1) create_build('rspec 1', :failed, 1)
end end
it 'allows to retry failed pipeline' do it 'allows to retry failed pipeline' do
allow_any_instance_of(Project).to receive(:fetch_branch_allows_maintainer_push?).and_return(true) allow_any_instance_of(Project).to receive(:fetch_branch_allows_collaboration?).and_return(true)
allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false) allow_any_instance_of(Project).to receive(:empty_repo?).and_return(false)
service.execute(pipeline) service.execute(pipeline)
......
...@@ -547,7 +547,7 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -547,7 +547,7 @@ describe MergeRequests::UpdateService, :mailer do
let(:closed_issuable) { create(:closed_merge_request, source_project: project) } let(:closed_issuable) { create(:closed_merge_request, source_project: project) }
end end
context 'setting `allow_maintainer_to_push`' do context 'setting `allow_collaboration`' do
let(:target_project) { create(:project, :public) } let(:target_project) { create(:project, :public) }
let(:source_project) { fork_project(target_project) } let(:source_project) { fork_project(target_project) }
let(:user) { create(:user) } let(:user) { create(:user) }
...@@ -562,23 +562,23 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -562,23 +562,23 @@ describe MergeRequests::UpdateService, :mailer do
allow(ProtectedBranch).to receive(:protected?).with(source_project, 'fixes') { false } allow(ProtectedBranch).to receive(:protected?).with(source_project, 'fixes') { false }
end end
it 'does not allow a maintainer of the target project to set `allow_maintainer_to_push`' do it 'does not allow a maintainer of the target project to set `allow_collaboration`' do
target_project.add_developer(user) target_project.add_developer(user)
update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') update_merge_request(allow_collaboration: true, title: 'Updated title')
expect(merge_request.title).to eq('Updated title') expect(merge_request.title).to eq('Updated title')
expect(merge_request.allow_maintainer_to_push).to be_falsy expect(merge_request.allow_collaboration).to be_falsy
end end
it 'is allowed by a user that can push to the source and can update the merge request' do it 'is allowed by a user that can push to the source and can update the merge request' do
merge_request.update!(assignee: user) merge_request.update!(assignee: user)
source_project.add_developer(user) source_project.add_developer(user)
update_merge_request(allow_maintainer_to_push: true, title: 'Updated title') update_merge_request(allow_collaboration: true, title: 'Updated title')
expect(merge_request.title).to eq('Updated title') expect(merge_request.title).to eq('Updated title')
expect(merge_request.allow_maintainer_to_push).to be_truthy expect(merge_request.allow_collaboration).to be_truthy
end end
end end
end end
......
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