kotovalexarian-likes-gitlab
/
gitlab-org--gitlab-foss
1
0
Fork 0
Code Releases Activity

Merge branch 'bvl-allow-maintainer-to-push' into 'master' 

Allow maintainers to push forks of a project for branches that have open MRs

Closes #22292

See merge request gitlab-org/gitlab-ce!17395
This commit is contained in:
Douwe Maan 2018-03-07 18:43:53 +00:00
parent 6718509773 cfa9d1ed6b
commit e85f4697cc
58 changed files with 912 additions and 119 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
app/assets/javascripts/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue Normal file
View File

@ -0,0 +1,19 @@
<script>
export default {
name: 'MRWidgetMaintainerEdit',
props: {
maintainerEditAllowed: {
type: Boolean,
default: false,
required: false,
},
},
};
</script>
<template>
<section class="mr-info-list mr-maintainer-edit">
<p v-if="maintainerEditAllowed">
{{ s__("mrWidget|Allows edits from maintainers") }}
</p>
</section>
</template>

1
app/assets/javascripts/vue_merge_request_widget/dependencies.js
View File

@ -15,6 +15,7 @@ export { default as WidgetHeader } from './components/mr_widget_header.vue';
export { default as WidgetMergeHelp } from './components/mr_widget_merge_help.vue';
export { default as WidgetPipeline } from './components/mr_widget_pipeline.vue';
export { default as WidgetDeployment } from './components/mr_widget_deployment';
export { default as WidgetMaintainerEdit } from './components/mr_widget_maintainer_edit.vue';
export { default as WidgetRelatedLinks } from './components/mr_widget_related_links.vue';
export { default as MergedState } from './components/states/mr_widget_merged.vue';
export { default as FailedToMerge } from './components/states/mr_widget_failed_to_merge.vue';

7
app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
View File

@ -6,6 +6,7 @@ import {
WidgetMergeHelp,
WidgetPipeline,
WidgetDeployment,
WidgetMaintainerEdit,
WidgetRelatedLinks,
MergedState,
ClosedState,
@ -211,6 +212,7 @@ export default {
'mr-widget-merge-help': WidgetMergeHelp,
'mr-widget-pipeline': WidgetPipeline,
'mr-widget-deployment': WidgetDeployment,
'mr-widget-maintainer-edit': WidgetMaintainerEdit,
'mr-widget-related-links': WidgetRelatedLinks,
'mr-widget-merged': MergedState,
'mr-widget-closed': ClosedState,
@ -251,11 +253,12 @@ export default {
:is="componentName"
:mr="mr"
:service="service" />
<mr-widget-maintainer-edit
:maintainerEditAllowed="mr.maintainerEditAllowed" />
<mr-widget-related-links
v-if="shouldRenderRelatedLinks"
:state="mr.state"
:related-links="mr.relatedLinks"
/>
:related-links="mr.relatedLinks" />
</div>
<div
class="mr-widget-footer"

1
app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
View File

@ -76,6 +76,7 @@ export default class MergeRequestStore {
this.canBeMerged = data.can_be_merged || false;
this.isMergeAllowed = data.mergeable || false;
this.mergeOngoing = data.merge_ongoing;
this.maintainerEditAllowed = data.allow_maintainer_to_push;
// Cherry-pick and Revert actions related
this.canCherryPickInCurrentMR = currentUser.can_cherry_pick_on_current_merge_request || false;

3
app/assets/stylesheets/pages/merge_requests.scss
View File

@ -453,7 +453,8 @@
}
}
.mr-links {
.mr-links,
.mr-maintainer-edit {
padding-left: $status-icon-size + $status-icon-margin;
}

8
app/controllers/concerns/creates_commit.rb
View File

@ -4,7 +4,7 @@ module CreatesCommit
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def create_commit(service, success_path:, failure_path:, failure_view: nil, success_notice: nil)
if can?(current_user, :push_code, @project)
if user_access(@project).can_push_to_branch?(branch_name_or_ref)
@project_to_commit_into = @project
@branch_name ||= @ref
else
@ -50,7 +50,7 @@ module CreatesCommit
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def authorize_edit_tree!
return if can_collaborate_with_project?
return if can_collaborate_with_project?(project, ref: branch_name_or_ref)
access_denied!
end
@ -123,4 +123,8 @@ module CreatesCommit
params[:create_merge_request].present? &&
(different_project? || @start_branch != @branch_name) # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
def branch_name_or_ref
@branch_name || @ref # rubocop:disable Gitlab/ModuleWithInstanceVariables
end
end

12
app/controllers/projects/application_controller.rb
View File

@ -6,7 +6,7 @@ class Projects::ApplicationController < ApplicationController
before_action :repository
layout 'project'
helper_method :repository, :can_collaborate_with_project?
helper_method :repository, :can_collaborate_with_project?, :user_access
private
@ -31,11 +31,12 @@ class Projects::ApplicationController < ApplicationController
@repository ||= project.repository
end
def can_collaborate_with_project?(project = nil)
def can_collaborate_with_project?(project = nil, ref: nil)
project ||= @project
can?(current_user, :push_code, project) ||
(current_user && current_user.already_forked?(project))
(current_user && current_user.already_forked?(project)) ||
user_access(project).can_push_to_branch?(ref)
end
def authorize_action!(action)
@ -90,4 +91,9 @@ class Projects::ApplicationController < ApplicationController
def check_issues_available!
return render_404 unless @project.feature_available?(:issues, current_user)
end
def user_access(project)
@user_access ||= {}
@user_access[project] ||= Gitlab::UserAccess.new(current_user, project: project)
end
end

8
app/controllers/projects/blob_controller.rb
View File

@ -9,8 +9,12 @@ class Projects::BlobController < Projects::ApplicationController
before_action :require_non_empty_project, except: [:new, :create]
before_action :authorize_download_code!
before_action :authorize_edit_tree!, only: [:new, :create, :update, :destroy]
# We need to assign the blob vars before `authorize_edit_tree!` so we can
# validate access to a specific ref.
before_action :assign_blob_vars
before_action :authorize_edit_tree!, only: [:new, :create, :update, :destroy]
before_action :commit, except: [:new, :create]
before_action :blob, except: [:new, :create]
before_action :require_branch_head, only: [:edit, :update]
@ -46,7 +50,7 @@ class Projects::BlobController < Projects::ApplicationController
end
def edit
if can_collaborate_with_project?
if can_collaborate_with_project?(project, ref: @ref)
blob.load_all_data!
else
redirect_to action: 'show'

1
app/controllers/projects/merge_requests/application_controller.rb
View File

@ -15,6 +15,7 @@ class Projects::MergeRequests::ApplicationController < Projects::ApplicationCont
def merge_request_params_attributes
[
:allow_maintainer_to_push,
:assignee_id,
:description,
:force_remove_source_branch,

13
app/helpers/merge_requests_helper.rb
View File

@ -125,6 +125,19 @@ module MergeRequestsHelper
link_to(url[merge_request.project, merge_request], data: data_attrs, &block)
end
def allow_maintainer_push_unavailable_reason(merge_request)
return if merge_request.can_allow_maintainer_to_push?(current_user)
minimum_visibility = [merge_request.target_project.visibility_level,
merge_request.source_project.visibility_level].min
if minimum_visibility < Gitlab::VisibilityLevel::INTERNAL
_('Not available for private projects')
elsif ProtectedBranch.protected?(merge_request.source_project, merge_request.source_branch)
_('Not available for protected branches')
end
end
def merge_params_ee(merge_request)
{}
end

21
app/helpers/tree_helper.rb
View File

@ -49,15 +49,13 @@ module TreeHelper
return false unless on_top_of_branch?(project, ref)
can_collaborate_with_project?(project)
can_collaborate_with_project?(project, ref: ref)
end
def tree_edit_branch(project = @project, ref = @ref)
return unless can_edit_tree?(project, ref)
project = project.present(current_user: current_user)
if project.can_current_user_push_to_branch?(ref)
if user_access(project).can_push_to_branch?(ref)
ref
else
project = tree_edit_project(project)
@ -88,7 +86,16 @@ module TreeHelper
end
def commit_in_fork_help
"A new branch will be created in your fork and a new merge request will be started."
_("A new branch will be created in your fork and a new merge request will be started.")
end
def commit_in_single_accessible_branch
branch_name = html_escape(selected_branch)
message = _("Your changes can be committed to %{branch_name} because a merge "\
"request is open.") % { branch_name: "<strong>#{branch_name}</strong>" }
message.html_safe
end
def path_breadcrumbs(max_links = 6)
@ -125,4 +132,8 @@ module TreeHelper
return tree.name
end
end
def selected_branch
@branch_name || tree_edit_branch
end
end

20
app/models/merge_request.rb
View File

@ -865,7 +865,7 @@ class MergeRequest < ActiveRecord::Base
def can_be_merged_by?(user)
access = ::Gitlab::UserAccess.new(user, project: project)
access.can_push_to_branch?(target_branch) || access.can_merge_to_branch?(target_branch)
access.can_update_branch?(target_branch)
end
def can_be_merged_via_command_line_by?(user)
@ -1087,4 +1087,22 @@ class MergeRequest < ActiveRecord::Base
project.merge_requests.merged.where(author_id: author_id).empty?
end
def allow_maintainer_to_push
maintainer_push_possible? && super
end
alias_method :allow_maintainer_to_push?, :allow_maintainer_to_push
def maintainer_push_possible?
source_project.present? && for_fork? &&
target_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE &&
source_project.visibility_level > Gitlab::VisibilityLevel::PRIVATE &&
!ProtectedBranch.protected?(source_project, source_branch)
end
def can_allow_maintainer_to_push?(user)
maintainer_push_possible? &&
Ability.allowed?(user, :push_code, source_project)
end
end

46
app/models/project.rb
View File

@ -150,6 +150,7 @@ class Project < ActiveRecord::Base
# Merge Requests for target project should be removed with it
has_many :merge_requests, foreign_key: 'target_project_id'
has_many :source_of_merge_requests, foreign_key: 'source_project_id', class_name: 'MergeRequest'
has_many :issues
has_many :labels, class_name: 'ProjectLabel'
has_many :services
@ -1799,6 +1800,33 @@ class Project < ActiveRecord::Base
Badge.where("id IN (#{union.to_sql})") # rubocop:disable GitlabSecurity/SqlInjection
end
def merge_requests_allowing_push_to_user(user)
return MergeRequest.none unless user
developer_access_exists = user.project_authorizations
.where('access_level >= ? ', Gitlab::Access::DEVELOPER)
.where('project_authorizations.project_id = merge_requests.target_project_id')
.limit(1)
.select(1)
source_of_merge_requests.opened
.where(allow_maintainer_to_push: true)
.where('EXISTS (?)', developer_access_exists)
end
def branch_allows_maintainer_push?(user, branch_name)
return false unless user
cache_key = "user:#{user.id}:#{branch_name}:branch_allows_push"
memoized_results = strong_memoize(:branch_allows_maintainer_push) do
Hash.new do |result, cache_key|
result[cache_key] = fetch_branch_allows_maintainer_push?(user, branch_name)
end
end
memoized_results[cache_key]
end
private
def storage
@ -1921,4 +1949,22 @@ class Project < ActiveRecord::Base
raise ex
end
def fetch_branch_allows_maintainer_push?(user, branch_name)
check_access = -> do
merge_request = source_of_merge_requests.opened
.where(allow_maintainer_to_push: true)
.find_by(source_branch: branch_name)
merge_request&.can_be_merged_by?(user)
end
if RequestStore.active?
RequestStore.fetch("project-#{id}:branch-#{branch_name}:user-#{user.id}:branch_allows_maintainer_push") do
check_access.call
end
else
check_access.call
end
end
end

14
app/policies/project_policy.rb
View File

@ -61,6 +61,11 @@ class ProjectPolicy < BasePolicy
desc "Project has request access enabled"
condition(:request_access_enabled, scope: :subject) { project.request_access_enabled }
desc "Has merge requests allowing pushes to user"
condition(:has_merge_requests_allowing_pushes, scope: :subject) do
project.merge_requests_allowing_push_to_user(user).any?
end
features = %w[
merge_requests
issues
@ -291,6 +296,15 @@ class ProjectPolicy < BasePolicy
prevent :read_issue
end
# These rules are included to allow maintainers of projects to push to certain
# to run pipelines for the branches they have access to.
rule { can?(:public_access) & has_merge_requests_allowing_pushes }.policy do
enable :create_build
enable :update_build
enable :create_pipeline
enable :update_pipeline
end
private
def team_member?

19
app/presenters/merge_request_presenter.rb
View File

@ -78,7 +78,7 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end
def rebase_path
if !rebase_in_progress? && should_be_rebased? && user_can_push_to_source_branch?
if !rebase_in_progress? && should_be_rebased? && can_push_to_source_branch?
rebase_project_merge_request_path(project, merge_request)
end
end
@ -160,7 +160,11 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end
def can_push_to_source_branch?
source_branch_exists? && user_can_push_to_source_branch?
return false unless source_branch_exists?
!!::Gitlab::UserAccess
.new(current_user, project: source_project)
.can_push_to_branch?(source_branch)
end
private
@ -191,17 +195,10 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated
end.sort.to_sentence
end
def user_can_push_to_source_branch?
return false unless source_branch_exists?
::Gitlab::UserAccess
.new(current_user, project: source_project)
.can_push_to_branch?(source_branch)
end
def user_can_collaborate_with_project?
can?(current_user, :push_code, project) ||
(current_user && current_user.already_forked?(project))
(current_user && current_user.already_forked?(project)) ||
can_push_to_source_branch?
end
def user_can_fork_project?

6
app/serializers/merge_request_widget_entity.rb
View File

@ -11,6 +11,7 @@ class MergeRequestWidgetEntity < IssuableEntity
expose :source_project_id
expose :target_branch
expose :target_project_id
expose :allow_maintainer_to_push
expose :should_be_rebased?, as: :should_be_rebased
expose :ff_only_enabled do |merge_request|
@ -29,6 +30,7 @@ class MergeRequestWidgetEntity < IssuableEntity
expose :can_push_to_source_branch do |merge_request|
presenter(merge_request).can_push_to_source_branch?
end
expose :rebase_path do |merge_request|
presenter(merge_request).rebase_path
end
@ -136,8 +138,8 @@ class MergeRequestWidgetEntity < IssuableEntity
end
expose :new_blob_path do |merge_request|
if can?(current_user, :push_code, merge_request.project)
project_new_blob_path(merge_request.project, merge_request.source_branch)
if presenter(merge_request).can_push_to_source_branch?
project_new_blob_path(merge_request.source_project, merge_request.source_branch)
end
end

2
app/services/ci/create_pipeline_service.rb
View File

@ -81,7 +81,7 @@ module Ci
end
def related_merge_requests
MergeRequest.opened.where(source_project: pipeline.project, source_branch: pipeline.ref)
pipeline.project.source_of_merge_requests.opened.where(source_branch: pipeline.ref)
end
end
end

8
app/services/merge_requests/base_service.rb
View File

@ -35,6 +35,14 @@ module MergeRequests
end
end
def filter_params(merge_request)
super
unless merge_request.can_allow_maintainer_to_push?(current_user)
params.delete(:allow_maintainer_to_push)
end
end
def merge_request_metrics_service(merge_request)
MergeRequestMetricsService.new(merge_request.metrics)
end

1
app/services/merge_requests/build_service.rb
View File

@ -6,6 +6,7 @@ module MergeRequests
@params_issue_iid = params.delete(:issue_iid)
self.merge_request = MergeRequest.new(params)
merge_request.author = current_user
merge_request.compare_commits = []
merge_request.source_project = find_source_project
merge_request.target_project = find_target_project

4
app/views/projects/_commit_button.html.haml
View File

@ -3,6 +3,4 @@
= link_to 'Cancel', cancel_path,
class: 'btn btn-cancel', data: {confirm: leave_edit_message}
- unless can?(current_user, :push_code, @project)
.inline.prepend-left-10
= commit_in_fork_help
= render 'shared/projects/edit_information'

4
app/views/projects/blob/_new_dir.html.haml
View File

@ -17,6 +17,4 @@
= submit_tag _("Create directory"), class: 'btn btn-create'
= link_to "Cancel", '#', class: "btn btn-cancel", "data-dismiss" => "modal"
- unless can?(current_user, :push_code, @project)
.inline.prepend-left-10
= commit_in_fork_help
= render 'shared/projects/edit_information'

4
app/views/projects/blob/_upload.html.haml
View File

@ -24,6 +24,4 @@
= button_title
= link_to _("Cancel"), '#', class: "btn btn-cancel", "data-dismiss" => "modal"
- unless can?(current_user, :push_code, @project)
.inline.prepend-left-10
= commit_in_fork_help
= render 'shared/projects/edit_information'

4
app/views/projects/commit/_change.html.haml
View File

@ -35,6 +35,4 @@
= submit_tag label, class: 'btn btn-create'
= link_to _("Cancel"), '#', class: "btn btn-cancel", "data-dismiss" => "modal"
- unless can?(current_user, :push_code, @project)
.inline.prepend-left-10
= commit_in_fork_help
= render 'shared/projects/edit_information'

9
app/views/shared/_new_commit_form.html.haml
View File

@ -1,3 +1,6 @@
- project = @project.present(current_user: current_user)
- branch_name = selected_branch
= render 'shared/commit_message_container', placeholder: placeholder
- if @project.empty_repo?
@ -7,12 +10,14 @@
.form-group.branch
= label_tag 'branch_name', _('Target Branch'), class: 'control-label'
.col-sm-10
= text_field_tag 'branch_name', @branch_name || tree_edit_branch, required: true, class: "form-control js-branch-name ref-name"
= text_field_tag 'branch_name', branch_name, required: true, class: "form-control js-branch-name ref-name"
.js-create-merge-request-container
= render 'shared/new_merge_request_checkbox'
- elsif project.can_current_user_push_to_branch?(branch_name)
= hidden_field_tag 'branch_name', branch_name
- else
= hidden_field_tag 'branch_name', @branch_name || tree_edit_branch
= hidden_field_tag 'branch_name', branch_name
= hidden_field_tag 'create_merge_request', 1
= hidden_field_tag 'original_branch', @ref, class: 'js-original-branch'

2
app/views/shared/issuable/_form.html.haml
View File

@ -33,6 +33,8 @@
= render 'shared/issuable/form/merge_params', issuable: issuable
= render 'shared/issuable/form/contribution', issuable: issuable, form: form
- if @merge_request_to_resolve_discussions_of
.form-group
.col-sm-10.col-sm-offset-2

20
app/views/shared/issuable/form/_contribution.html.haml Normal file
View File

@ -0,0 +1,20 @@
- issuable = local_assigns.fetch(:issuable)
- form = local_assigns.fetch(:form)
- return unless issuable.is_a?(MergeRequest)
- return unless issuable.for_fork?
- return unless can?(current_user, :push_code, issuable.source_project)
%hr
.form-group
.control-label
= _('Contribution')
.col-sm-10
.checkbox
= form.label :allow_maintainer_to_push do
= form.check_box :allow_maintainer_to_push, disabled: !issuable.can_allow_maintainer_to_push?(current_user)
= _('Allow edits from maintainers')
= link_to 'About this feature', help_page_path('user/project/merge_requests/maintainer_access')
.help-block
= allow_maintainer_push_unavailable_reason(issuable)

6
app/views/shared/projects/_edit_information.html.haml Normal file
View File

@ -0,0 +1,6 @@
- unless can?(current_user, :push_code, @project)
.inline.prepend-left-10
- if @project.branch_allows_maintainer_push?(current_user, selected_branch)
= commit_in_single_accessible_branch
- else
= commit_in_fork_help

5
changelogs/unreleased/bvl-allow-maintainer-to-push.yml Normal file
View File

@ -0,0 +1,5 @@
---
title: Allow maintainers to push to forks of their projects when a merge request is open
merge_request: 17395
author:
type: added

18
db/migrate/20180221151752_add_allow_maintainer_to_push_to_merge_requests.rb Normal file
View File

@ -0,0 +1,18 @@
# See http://doc.gitlab.com/ce/development/migration_style_guide.html
# for more information on how to write migrations for GitLab.
class AddAllowMaintainerToPushToMergeRequests < ActiveRecord::Migration
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
disable_ddl_transaction!
def up
add_column :merge_requests, :allow_maintainer_to_push, :boolean
end
def down
remove_column :merge_requests, :allow_maintainer_to_push
end
end

1
db/schema.rb
View File

@ -1145,6 +1145,7 @@ ActiveRecord::Schema.define(version: 20180307012445) do
t.boolean "discussion_locked"
t.integer "latest_merge_request_diff_id"
t.string "rebase_commit_sha"
t.boolean "allow_maintainer_to_push"
end
add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree

62
doc/api/merge_requests.md
View File

@ -529,18 +529,19 @@ Creates a new merge request.
POST /projects/:id/merge_requests
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `source_branch` | string | yes | The source branch |
| `target_branch` | string | yes | The target branch |
| `title` | string | yes | Title of MR |
| `assignee_id` | integer | no | Assignee user ID |
| `description` | string | no | Description of MR |
| `target_project_id` | integer | no | The target project (numeric id) |
| `labels` | string | no | Labels for MR as a comma-separated list |
| `milestone_id` | integer | no | The ID of a milestone |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `source_branch` | string | yes | The source branch |
| `target_branch` | string | yes | The target branch |
| `title` | string | yes | Title of MR |
| `assignee_id` | integer | no | Assignee user ID |
| `description` | string | no | Description of MR |
| `target_project_id` | integer | no | The target project (numeric id) |
| `labels` | string | no | Labels for MR as a comma-separated list |
| `milestone_id` | integer | no | The ID of a milestone |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
```json
{
@ -548,7 +549,7 @@ POST /projects/:id/merge_requests
"iid": 1,
"target_branch": "master",
"source_branch": "test1",
"project_id": 3,
"project_id": 4,
"title": "test1",
"state": "opened",
"upvotes": 0,
@ -569,7 +570,7 @@ POST /projects/:id/merge_requests
"state": "active",
"created_at": "2012-04-29T08:46:00Z"
},
"source_project_id": 4,
"source_project_id": 3,
"target_project_id": 4,
"labels": [ ],
"description": "fixed login page css paddings",
@ -596,6 +597,7 @@ POST /projects/:id/merge_requests
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"allow_maintainer_to_push": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,
@ -613,19 +615,20 @@ Updates an existing merge request. You can change the target branch, title, or e
PUT /projects/:id/merge_requests/:merge_request_iid
```
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `merge_request_iid` | integer | yes | The ID of a merge request |
| `target_branch` | string | no | The target branch |
| `title` | string | no | Title of MR |
| `assignee_id` | integer | no | The ID of the user to assign the merge request to. Set to `0` or provide an empty value to unassign all assignees. |
| `milestone_id` | integer | no | The ID of a milestone to assign the merge request to. Set to `0` or provide an empty value to unassign a milestone.|
| `labels` | string | no | Comma-separated label names for a merge request. Set to an empty string to unassign all labels. |
| `description` | string | no | Description of MR |
| `state_event` | string | no | New state (close/reopen) |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `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. |
| Attribute | Type | Required | Description |
| --------- | ---- | -------- | ----------- |
| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
| `merge_request_iid` | integer | yes | The ID of a merge request |
| `target_branch` | string | no | The target branch |
| `title` | string | no | Title of MR |
| `assignee_id` | integer | no | The ID of the user to assign the merge request to. Set to `0` or provide an empty value to unassign all assignees. |
| `milestone_id` | integer | no | The ID of a milestone to assign the merge request to. Set to `0` or provide an empty value to unassign a milestone.|
| `labels` | string | no | Comma-separated label names for a merge request. Set to an empty string to unassign all labels. |
| `description` | string | no | Description of MR |
| `state_event` | string | no | New state (close/reopen) |
| `remove_source_branch` | boolean | no | Flag indicating if a merge request should remove the source branch when merging |
| `discussion_locked` | boolean | no | Flag indicating if the merge request's discussion is locked. If the discussion is locked only project members can add, edit or resolve comments. |
| `allow_maintainer_to_push` | boolean | no | Whether or not a maintainer of the target project can push to the source branch |
Must include at least one non-required attribute from above.
@ -634,7 +637,7 @@ Must include at least one non-required attribute from above.
"id": 1,
"iid": 1,
"target_branch": "master",
"project_id": 3,
"project_id": 4,
"title": "test1",
"state": "opened",
"upvotes": 0,
@ -655,7 +658,7 @@ Must include at least one non-required attribute from above.
"state": "active",
"created_at": "2012-04-29T08:46:00Z"
},
"source_project_id": 4,
"source_project_id": 3,
"target_project_id": 4,
"labels": [ ],
"description": "description1",
@ -682,6 +685,7 @@ Must include at least one non-required attribute from above.
"force_remove_source_branch": false,
"web_url": "http://example.com/example/example/merge_requests/1",
"discussion_locked": false,
"allow_maintainer_to_push": false,
"time_stats": {
"time_estimate": 0,
"total_time_spent": 0,

BIN
doc/user/project/merge_requests/img/allow_maintainer_push.png Normal file
View File

Binary file not shown.
Side by Side

After

Width:  |  Height:  |  Size: 491 KiB

1
doc/user/project/merge_requests/index.md
View File

@ -28,6 +28,7 @@ With GitLab merge requests, you can:
- 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
- [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)
With **[GitLab Enterprise Edition][ee]**, you can also:

13
doc/user/project/merge_requests/maintainer_access.md Normal file
View File

@ -0,0 +1,13 @@
# Allow maintainer pushes for merge requests accross forks
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 enabling this feature for a merge request, you give can give members with push access to the target project rights to edit files on 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)

1
lib/api/entities.rb
View File

@ -547,6 +547,7 @@ module API
expose :discussion_locked
expose :should_remove_source_branch?, as: :should_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 :web_url do |merge_request, options|
Gitlab::UrlBuilder.build(merge_request)

1
lib/api/merge_requests.rb
View File

@ -144,6 +144,7 @@ module API
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 :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'
use :optional_params_ee
end

7
lib/gitlab/checks/change_access.rb
View File

@ -47,7 +47,7 @@ module Gitlab
protected
def push_checks
if user_access.cannot_do_action?(:push_code)
unless can_push?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:push_code]
end
end
@ -183,6 +183,11 @@ module Gitlab
def commits
@commits ||= project.repository.new_commits(newrev)
end
def can_push?
user_access.can_do_action?(:push_code) ||
user_access.can_push_to_branch?(branch_name)
end
end
end
end

4
lib/gitlab/user_access.rb
View File

@ -68,8 +68,10 @@ module Gitlab
return true if project.user_can_push_to_empty_repo?(user)
protected_branch_accessible_to?(ref, action: :push)
elsif user.can?(:push_code, project)
true
else
user.can?(:push_code, project)
project.branch_allows_maintainer_push?(user, ref)
end
end

151
locale/gitlab.pot
View File

@ -8,8 +8,8 @@ msgid ""
msgstr ""
"Project-Id-Version: gitlab 1.0.0\n"
"Report-Msgid-Bugs-To: \n"
"POT-Creation-Date: 2018-03-05 13:02-0600\n"
"PO-Revision-Date: 2018-03-05 13:02-0600\n"
"POT-Creation-Date: 2018-03-06 17:36+0100\n"
"PO-Revision-Date: 2018-03-06 17:36+0100\n"
"Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
"Language-Team: LANGUAGE <LL@li.org>\n"
"Language: \n"
@ -102,6 +102,9 @@ msgstr ""
msgid "A collection of graphs regarding Continuous Integration"
msgstr ""
msgid "A new branch will be created in your fork and a new merge request will be started."
msgstr ""
msgid "A project is where you house your files (repository), plan your work (issues), and publish your documentation (wiki), %{among_other_things_link}."
msgstr ""
@ -207,6 +210,9 @@ msgstr ""
msgid "All features are enabled for blank projects, from templates, or when importing, but you can disable them afterward in the project settings."
msgstr ""
msgid "Allow edits from maintainers"
msgstr ""
msgid "Allows you to add and manage Kubernetes clusters."
msgstr ""
@ -288,9 +294,6 @@ msgstr ""
msgid "Are you sure you want to delete this pipeline schedule?"
msgstr ""
msgid "Are you sure you want to discard your changes?"
msgstr ""
msgid "Are you sure you want to reset registration token?"
msgstr ""
@ -392,9 +395,6 @@ msgstr[1] ""
msgid "Branch <strong>%{branch_name}</strong> was created. To set up auto deploy, choose a GitLab CI Yaml template and commit your changes. %{link_to_autodeploy_doc}"
msgstr ""
msgid "Branch has changed"
msgstr ""
msgid "Branch is already taken"
msgstr ""
@ -410,6 +410,15 @@ msgstr ""
msgid "Branches"
msgstr ""
msgid "Branches|Active"
msgstr ""
msgid "Branches|Active branches"
msgstr ""
msgid "Branches|All"
msgstr ""
msgid "Branches|Cant find HEAD commit for this branch"
msgstr ""
@ -455,12 +464,39 @@ msgstr ""
msgid "Branches|Only a project master or owner can delete a protected branch"
msgstr ""
msgid "Branches|Protected branches can be managed in %{project_settings_link}"
msgid "Branches|Overview"
msgstr ""
msgid "Branches|Protected branches can be managed in %{project_settings_link}."
msgstr ""
msgid "Branches|Show active branches"
msgstr ""
msgid "Branches|Show all branches"
msgstr ""
msgid "Branches|Show more active branches"
msgstr ""
msgid "Branches|Show more stale branches"
msgstr ""
msgid "Branches|Show overview of the branches"
msgstr ""
msgid "Branches|Show stale branches"
msgstr ""
msgid "Branches|Sort by"
msgstr ""
msgid "Branches|Stale"
msgstr ""
msgid "Branches|Stale branches"
msgstr ""
msgid "Branches|The default branch cannot be deleted"
msgstr ""
@ -512,9 +548,6 @@ msgstr ""
msgid "Cancel"
msgstr ""
msgid "Cancel edit"
msgstr ""
msgid "Cannot modify managed Kubernetes cluster"
msgstr ""
@ -662,6 +695,9 @@ msgstr ""
msgid "Clone repository"
msgstr ""
msgid "Close"
msgstr ""
msgid "ClusterIntegration|%{appList} was successfully installed on your Kubernetes cluster"
msgstr ""
@ -1077,6 +1113,9 @@ msgstr ""
msgid "ContainerRegistry|With the Docker Container Registry integrated into GitLab, every project can have its own space to store its Docker images."
msgstr ""
msgid "Contribution"
msgstr ""
msgid "Contribution guide"
msgstr ""
@ -1128,9 +1167,6 @@ msgstr ""
msgid "Create empty bare repository"
msgstr ""
msgid "Create file"
msgstr ""
msgid "Create lists from labels. Issues with that label appear in that list."
msgstr ""
@ -1140,15 +1176,6 @@ msgstr ""
msgid "Create merge request and branch"
msgstr ""
msgid "Create new branch"
msgstr ""
msgid "Create new directory"
msgstr ""
msgid "Create new file"
msgstr ""
msgid "Create new label"
msgstr ""
@ -1238,9 +1265,6 @@ msgstr ""
msgid "Directory name"
msgstr ""
msgid "Discard changes"
msgstr ""
msgid "Dismiss Cycle Analytics introduction box"
msgstr ""
@ -1421,9 +1445,6 @@ msgstr ""
msgid "Fields on this page are now uneditable, you can configure"
msgstr ""
msgid "File name"
msgstr ""
msgid "Files"
msgstr ""
@ -1439,6 +1460,9 @@ msgstr ""
msgid "Find file"
msgstr ""
msgid "Finished"
msgstr ""
msgid "FirstPushedBy|First"
msgstr ""
@ -1495,6 +1519,9 @@ msgstr ""
msgid "Gitaly Servers"
msgstr ""
msgid "Go back"
msgstr ""
msgid "Go to your fork"
msgstr ""
@ -1701,6 +1728,15 @@ msgstr ""
msgid "LFSStatus|Enabled"
msgstr ""
msgid "Label"
msgstr ""
msgid "LabelSelect|%{firstLabelName} +%{remainingLabelCount} more"
msgstr ""
msgid "LabelSelect|%{labelsString}, and %{remainingLabelCount} more"
msgstr ""
msgid "Labels"
msgstr ""
@ -1760,9 +1796,6 @@ msgstr ""
msgid "Leave project"
msgstr ""
msgid "Loading the GitLab IDE..."
msgstr ""
msgid "Lock"
msgstr ""
@ -1945,6 +1978,12 @@ msgstr ""
msgid "Not available"
msgstr ""
msgid "Not available for private projects"
msgstr ""
msgid "Not available for protected branches"
msgstr ""
msgid "Not confidential"
msgstr ""
@ -2071,6 +2110,9 @@ msgstr ""
msgid "Password"
msgstr ""
msgid "Pending"
msgstr ""
msgid "Pipeline"
msgstr ""
@ -2149,9 +2191,30 @@ msgstr ""
msgid "Pipelines|Build with confidence"
msgstr ""
msgid "Pipelines|CI Lint"
msgstr ""
msgid "Pipelines|Clear Runner Caches"
msgstr ""
msgid "Pipelines|Get started with Pipelines"
msgstr ""
msgid "Pipelines|Loading Pipelines"
msgstr ""
msgid "Pipelines|Run Pipeline"
msgstr ""
msgid "Pipelines|There are currently no %{scope} pipelines."
msgstr ""
msgid "Pipelines|There are currently no pipelines."
msgstr ""
msgid "Pipelines|This project is not currently set up to run pipelines."
msgstr ""
msgid "Pipeline|Retry pipeline"
msgstr ""
@ -2404,6 +2467,9 @@ msgstr ""
msgid "Public - The project can be accessed without any authentication."
msgstr ""
msgid "Push access to this project is necessary in order to enable this option"
msgstr ""
msgid "Push events"
msgstr ""
@ -2490,6 +2556,9 @@ msgstr ""
msgid "Revert this merge request"
msgstr ""
msgid "Running"
msgstr ""
msgid "SSH Keys"
msgstr ""
@ -2511,6 +2580,9 @@ msgstr ""
msgid "Scheduling Pipelines"
msgstr ""
msgid "Search"
msgstr ""
msgid "Search branches and tags"
msgstr ""
@ -3278,9 +3350,6 @@ msgstr ""
msgid "We want to be sure it is you, please confirm you are not a robot."
msgstr ""
msgid "Web IDE"
msgstr ""
msgid "Wiki"
msgstr ""
@ -3395,13 +3464,13 @@ msgstr ""
msgid "You are going to remove %{group_name}. Removed groups CANNOT be restored! Are you ABSOLUTELY sure?"
msgstr ""
msgid "You are going to remove %{project_name_with_namespace}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?"
msgid "You are going to remove %{project_full_name}. Removed project CANNOT be restored! Are you ABSOLUTELY sure?"
msgstr ""
msgid "You are going to remove the fork relationship to source project %{forked_from_project}. Are you ABSOLUTELY sure?"
msgstr ""
msgid "You are going to transfer %{project_name_with_namespace} to another owner. Are you ABSOLUTELY sure?"
msgid "You are going to transfer %{project_full_name} to another owner. Are you ABSOLUTELY sure?"
msgstr ""
msgid "You are on a read-only GitLab instance."
@ -3467,6 +3536,9 @@ msgstr ""
msgid "Your Kubernetes cluster information on this page is still editable, but you are advised to disable and reconfigure"
msgstr ""
msgid "Your changes can be committed to %{branch_name} because a merge request is open."
msgstr ""
msgid "Your comment will not be visible to the public."
msgstr ""
@ -3513,6 +3585,9 @@ msgstr[1] ""
msgid "mrWidget| Please restore it or use a different %{missingBranchName} branch"
msgstr ""
msgid "mrWidget|Allows edits from maintainers"
msgstr ""
msgid "mrWidget|Cancel automatic merge"
msgstr ""

44
spec/features/merge_request/maintainer_edits_fork_spec.rb Normal file
View File

@ -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

85
spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb Normal file
View File

@ -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

13
spec/features/projects/user_creates_files_spec.rb
View File

@ -133,13 +133,20 @@ describe 'User creates files' do
before do
project2.add_reporter(user)
visit(project2_tree_path_root_ref)
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
find('.add-to-tree').click
click_link('New file')
expect(page).to have_selector('.file-editor')
expect(page).to have_content
find('#editor')
execute_script("ace.edit('editor').setValue('*.rbca')")

3
spec/fixtures/api/schemas/entities/merge_request_basic.json vendored
View File

@ -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
}

1
spec/fixtures/api/schemas/entities/merge_request_widget.json vendored
View File

@ -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" },

3
spec/fixtures/api/schemas/public_api/v4/merge_requests.json vendored
View File

@ -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",

9
spec/helpers/tree_helper_spec.rb
View File

@ -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 = '&lt;script&gt;alert(&#39;escape me!&#39;);&lt;/script&gt;'
expect(helper.commit_in_single_accessible_branch).to include(escaped_branch_name)
end
end
end

23
spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js Normal file
View File

@ -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');
});
});

1
spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
View File

@ -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();
});
});

3
spec/lib/gitlab/checks/change_access_spec.rb
View File

@ -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

1
spec/lib/gitlab/import_export/all_models.yml
View File

@ -278,6 +278,7 @@ project:
- custom_attributes
- lfs_file_locks
- project_badges
- source_of_merge_requests
award_emoji:
- awardable
- user

1
spec/lib/gitlab/import_export/safe_model_attributes.yml
View File

@ -168,6 +168,7 @@ MergeRequest:
- last_edited_by_id
- head_pipeline_id
- discussion_locked
- allow_maintainer_to_push
MergeRequestDiff:
- id
- state

35
spec/lib/gitlab/user_access_spec.rb
View File

@ -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

78
spec/models/merge_request_spec.rb
View File

@ -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

101
spec/models/project_spec.rb
View File

@ -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

37
spec/policies/project_policy_spec.rb
View File

@ -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

28
spec/requests/api/merge_requests_spec.rb
View File

@ -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',

37
spec/services/merge_requests/update_service_spec.rb
View File

@ -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

1
spec/views/projects/tree/show.html.haml_spec.rb
View File

@ -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