summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDouwe Maan <douwe@gitlab.com>2018-03-07 18:43:53 +0000
committerDouwe Maan <douwe@gitlab.com>2018-03-07 18:43:53 +0000
commite85f4697cce4f0040f235bd8f2fefce1daa9e8bf (patch)
tree29a5992d0bd311c535efc25d25acb31826d83d79
parent67185097734bb88979df020123cf7327bc5d32c5 (diff)
parentcfa9d1ed6b870dbc635148219b42d73c382eb90a (diff)
downloadgitlab-ce-e85f4697cce4f0040f235bd8f2fefce1daa9e8bf.tar.gz
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
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue19
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/dependencies.js1
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js7
-rw-r--r--app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js1
-rw-r--r--app/assets/stylesheets/pages/merge_requests.scss3
-rw-r--r--app/controllers/concerns/creates_commit.rb8
-rw-r--r--app/controllers/projects/application_controller.rb12
-rw-r--r--app/controllers/projects/blob_controller.rb8
-rw-r--r--app/controllers/projects/merge_requests/application_controller.rb1
-rw-r--r--app/helpers/merge_requests_helper.rb13
-rw-r--r--app/helpers/tree_helper.rb21
-rw-r--r--app/models/merge_request.rb20
-rw-r--r--app/models/project.rb46
-rw-r--r--app/policies/project_policy.rb14
-rw-r--r--app/presenters/merge_request_presenter.rb19
-rw-r--r--app/serializers/merge_request_widget_entity.rb6
-rw-r--r--app/services/ci/create_pipeline_service.rb2
-rw-r--r--app/services/merge_requests/base_service.rb8
-rw-r--r--app/services/merge_requests/build_service.rb1
-rw-r--r--app/views/projects/_commit_button.html.haml4
-rw-r--r--app/views/projects/blob/_new_dir.html.haml4
-rw-r--r--app/views/projects/blob/_upload.html.haml4
-rw-r--r--app/views/projects/commit/_change.html.haml4
-rw-r--r--app/views/shared/_new_commit_form.html.haml9
-rw-r--r--app/views/shared/issuable/_form.html.haml2
-rw-r--r--app/views/shared/issuable/form/_contribution.html.haml20
-rw-r--r--app/views/shared/projects/_edit_information.html.haml6
-rw-r--r--changelogs/unreleased/bvl-allow-maintainer-to-push.yml5
-rw-r--r--db/migrate/20180221151752_add_allow_maintainer_to_push_to_merge_requests.rb18
-rw-r--r--db/schema.rb1
-rw-r--r--doc/api/merge_requests.md62
-rw-r--r--doc/user/project/merge_requests/img/allow_maintainer_push.pngbin0 -> 502758 bytes
-rw-r--r--doc/user/project/merge_requests/index.md1
-rw-r--r--doc/user/project/merge_requests/maintainer_access.md13
-rw-r--r--lib/api/entities.rb1
-rw-r--r--lib/api/merge_requests.rb1
-rw-r--r--lib/gitlab/checks/change_access.rb7
-rw-r--r--lib/gitlab/user_access.rb4
-rw-r--r--locale/gitlab.pot151
-rw-r--r--spec/features/merge_request/maintainer_edits_fork_spec.rb44
-rw-r--r--spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb85
-rw-r--r--spec/features/projects/user_creates_files_spec.rb11
-rw-r--r--spec/fixtures/api/schemas/entities/merge_request_basic.json3
-rw-r--r--spec/fixtures/api/schemas/entities/merge_request_widget.json1
-rw-r--r--spec/fixtures/api/schemas/public_api/v4/merge_requests.json3
-rw-r--r--spec/helpers/tree_helper_spec.rb9
-rw-r--r--spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js23
-rw-r--r--spec/javascripts/vue_mr_widget/mr_widget_options_spec.js1
-rw-r--r--spec/lib/gitlab/checks/change_access_spec.rb3
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml1
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml1
-rw-r--r--spec/lib/gitlab/user_access_spec.rb35
-rw-r--r--spec/models/merge_request_spec.rb78
-rw-r--r--spec/models/project_spec.rb101
-rw-r--r--spec/policies/project_policy_spec.rb37
-rw-r--r--spec/requests/api/merge_requests_spec.rb28
-rw-r--r--spec/services/merge_requests/update_service_spec.rb37
-rw-r--r--spec/views/projects/tree/show.html.haml_spec.rb1
58 files changed, 911 insertions, 118 deletions
diff --git a/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue
new file mode 100644
index 00000000000..7a69cce695e
--- /dev/null
+++ b/app/assets/javascripts/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue
@@ -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>
diff --git a/app/assets/javascripts/vue_merge_request_widget/dependencies.js b/app/assets/javascripts/vue_merge_request_widget/dependencies.js
index edb3baa39e4..a1bc28873df 100644
--- a/app/assets/javascripts/vue_merge_request_widget/dependencies.js
+++ b/app/assets/javascripts/vue_merge_request_widget/dependencies.js
@@ -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';
diff --git a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
index 797f0f6ec0f..df3eb86f35c 100644
--- a/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
+++ b/app/assets/javascripts/vue_merge_request_widget/mr_widget_options.js
@@ -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"
diff --git a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
index 9a750ce42bd..5d07bcf1bb9 100644
--- a/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
+++ b/app/assets/javascripts/vue_merge_request_widget/stores/mr_widget_store.js
@@ -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;
diff --git a/app/assets/stylesheets/pages/merge_requests.scss b/app/assets/stylesheets/pages/merge_requests.scss
index f887a11004f..f8708a2c0a3 100644
--- a/app/assets/stylesheets/pages/merge_requests.scss
+++ b/app/assets/stylesheets/pages/merge_requests.scss
@@ -453,7 +453,8 @@
}
}
-.mr-links {
+.mr-links,
+.mr-maintainer-edit {
padding-left: $status-icon-size + $status-icon-margin;
}
diff --git a/app/controllers/concerns/creates_commit.rb b/app/controllers/concerns/creates_commit.rb
index 6f4fdcdaa4f..b26a76d2b62 100644
--- a/app/controllers/concerns/creates_commit.rb
+++ b/app/controllers/concerns/creates_commit.rb
@@ -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
diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb
index 6025a40348b..6d9b42a2c04 100644
--- a/app/controllers/projects/application_controller.rb
+++ b/app/controllers/projects/application_controller.rb
@@ -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
diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb
index 405726c017c..0c1c286a0a4 100644
--- a/app/controllers/projects/blob_controller.rb
+++ b/app/controllers/projects/blob_controller.rb
@@ -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'
diff --git a/app/controllers/projects/merge_requests/application_controller.rb b/app/controllers/projects/merge_requests/application_controller.rb
index 793ae03fb88..67d4ea2ca8f 100644
--- a/app/controllers/projects/merge_requests/application_controller.rb
+++ b/app/controllers/projects/merge_requests/application_controller.rb
@@ -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,
diff --git a/app/helpers/merge_requests_helper.rb b/app/helpers/merge_requests_helper.rb
index ce57422f45d..fb4fe1c40b7 100644
--- a/app/helpers/merge_requests_helper.rb
+++ b/app/helpers/merge_requests_helper.rb
@@ -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
diff --git a/app/helpers/tree_helper.rb b/app/helpers/tree_helper.rb
index f6a6d9bebde..b64be89c181 100644
--- a/app/helpers/tree_helper.rb
+++ b/app/helpers/tree_helper.rb
@@ -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
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 9a7e66a9cbb..c2bae379a94 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -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
diff --git a/app/models/project.rb b/app/models/project.rb
index 5cd1da43645..5f9d9785d64 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -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
diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb
index 3b0550b4dd6..57ab0c23dcd 100644
--- a/app/policies/project_policy.rb
+++ b/app/policies/project_policy.rb
@@ -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?
diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb
index 08ae49562c7..9f3f2637183 100644
--- a/app/presenters/merge_request_presenter.rb
+++ b/app/presenters/merge_request_presenter.rb
@@ -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?
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index a3ebec0efc6..4a812e39ee1 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -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
diff --git a/app/services/ci/create_pipeline_service.rb b/app/services/ci/create_pipeline_service.rb
index c8b112132b3..3b3d9239086 100644
--- a/app/services/ci/create_pipeline_service.rb
+++ b/app/services/ci/create_pipeline_service.rb
@@ -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
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb
index 23262b62615..231ab76fde4 100644
--- a/app/services/merge_requests/base_service.rb
+++ b/app/services/merge_requests/base_service.rb
@@ -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
diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb
index 4b186d93772..a98bbdf74dd 100644
--- a/app/services/merge_requests/build_service.rb
+++ b/app/services/merge_requests/build_service.rb
@@ -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
diff --git a/app/views/projects/_commit_button.html.haml b/app/views/projects/_commit_button.html.haml
index b55dc3dce5c..b387e38c1a6 100644
--- a/app/views/projects/_commit_button.html.haml
+++ b/app/views/projects/_commit_button.html.haml
@@ -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'
diff --git a/app/views/projects/blob/_new_dir.html.haml b/app/views/projects/blob/_new_dir.html.haml
index 5d48a35dc4c..48ff66900be 100644
--- a/app/views/projects/blob/_new_dir.html.haml
+++ b/app/views/projects/blob/_new_dir.html.haml
@@ -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'
diff --git a/app/views/projects/blob/_upload.html.haml b/app/views/projects/blob/_upload.html.haml
index f1324c61500..182d02376bf 100644
--- a/app/views/projects/blob/_upload.html.haml
+++ b/app/views/projects/blob/_upload.html.haml
@@ -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'
diff --git a/app/views/projects/commit/_change.html.haml b/app/views/projects/commit/_change.html.haml
index 93407956f56..21e4664d4e4 100644
--- a/app/views/projects/commit/_change.html.haml
+++ b/app/views/projects/commit/_change.html.haml
@@ -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'
diff --git a/app/views/shared/_new_commit_form.html.haml b/app/views/shared/_new_commit_form.html.haml
index 0a4a24ae807..9221fd1e025 100644
--- a/app/views/shared/_new_commit_form.html.haml
+++ b/app/views/shared/_new_commit_form.html.haml
@@ -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'
diff --git a/app/views/shared/issuable/_form.html.haml b/app/views/shared/issuable/_form.html.haml
index 6dfabd7ba4c..4c8f03f1498 100644
--- a/app/views/shared/issuable/_form.html.haml
+++ b/app/views/shared/issuable/_form.html.haml
@@ -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
diff --git a/app/views/shared/issuable/form/_contribution.html.haml b/app/views/shared/issuable/form/_contribution.html.haml
new file mode 100644
index 00000000000..0f2d313a5cc
--- /dev/null
+++ b/app/views/shared/issuable/form/_contribution.html.haml
@@ -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)
diff --git a/app/views/shared/projects/_edit_information.html.haml b/app/views/shared/projects/_edit_information.html.haml
new file mode 100644
index 00000000000..ec9dc8f62c2
--- /dev/null
+++ b/app/views/shared/projects/_edit_information.html.haml
@@ -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
diff --git a/changelogs/unreleased/bvl-allow-maintainer-to-push.yml b/changelogs/unreleased/bvl-allow-maintainer-to-push.yml
new file mode 100644
index 00000000000..a3fefc2889a
--- /dev/null
+++ b/changelogs/unreleased/bvl-allow-maintainer-to-push.yml
@@ -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
diff --git a/db/migrate/20180221151752_add_allow_maintainer_to_push_to_merge_requests.rb b/db/migrate/20180221151752_add_allow_maintainer_to_push_to_merge_requests.rb
new file mode 100644
index 00000000000..81acfbc3655
--- /dev/null
+++ b/db/migrate/20180221151752_add_allow_maintainer_to_push_to_merge_requests.rb
@@ -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
diff --git a/db/schema.rb b/db/schema.rb
index 387b15f8f30..b93e6fa594b 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -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
diff --git a/doc/api/merge_requests.md b/doc/api/merge_requests.md
index 25b0807eb18..b9a4f661777 100644
--- a/doc/api/merge_requests.md
+++ b/doc/api/merge_requests.md
@@ -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,
diff --git a/doc/user/project/merge_requests/img/allow_maintainer_push.png b/doc/user/project/merge_requests/img/allow_maintainer_push.png
new file mode 100644
index 00000000000..e73cc1ab5a5
--- /dev/null
+++ b/doc/user/project/merge_requests/img/allow_maintainer_push.png
Binary files differ
diff --git a/doc/user/project/merge_requests/index.md b/doc/user/project/merge_requests/index.md
index d3220598933..10d67729734 100644
--- a/doc/user/project/merge_requests/index.md
+++ b/doc/user/project/merge_requests/index.md
@@ -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:
diff --git a/doc/user/project/merge_requests/maintainer_access.md b/doc/user/project/merge_requests/maintainer_access.md
new file mode 100644
index 00000000000..1ade5db1587
--- /dev/null
+++ b/doc/user/project/merge_requests/maintainer_access.md
@@ -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)
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 4555184095c..16147ee90c9 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -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)
diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb
index ead1bb7957b..3264a26f7d2 100644
--- a/lib/api/merge_requests.rb
+++ b/lib/api/merge_requests.rb
@@ -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
diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb
index 3ce5f807989..51ba09aa129 100644
--- a/lib/gitlab/checks/change_access.rb
+++ b/lib/gitlab/checks/change_access.rb
@@ -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
diff --git a/lib/gitlab/user_access.rb b/lib/gitlab/user_access.rb
index 91b8bb2a83f..fd68b9f2b48 100644
--- a/lib/gitlab/user_access.rb
+++ b/lib/gitlab/user_access.rb
@@ -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
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 8a2176a4d72..bb7aaa2fb2f 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -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 ""
diff --git a/spec/features/merge_request/maintainer_edits_fork_spec.rb b/spec/features/merge_request/maintainer_edits_fork_spec.rb
new file mode 100644
index 00000000000..a3323da1b1f
--- /dev/null
+++ b/spec/features/merge_request/maintainer_edits_fork_spec.rb
@@ -0,0 +1,44 @@
+require 'spec_helper'
+
+describe 'a maintainer edits files on a source-branch of an MR from a fork', :js do
+ include ProjectForksHelper
+ let(:user) { create(:user, username: 'the-maintainer') }
+ let(:target_project) { create(:project, :public, :repository) }
+ let(:author) { create(:user, username: 'mr-authoring-machine') }
+ let(:source_project) { fork_project(target_project, author, repository: true) }
+
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: source_project,
+ target_project: target_project,
+ source_branch: 'fix',
+ target_branch: 'master',
+ author: author,
+ allow_maintainer_to_push: true)
+ end
+
+ before do
+ target_project.add_master(user)
+ sign_in(user)
+
+ visit project_merge_request_path(target_project, merge_request)
+ click_link 'Changes'
+ wait_for_requests
+ first('.js-file-title').click_link 'Edit'
+ wait_for_requests
+ end
+
+ it 'mentions commits will go to the source branch' do
+ expect(page).to have_content('Your changes can be committed to fix because a merge request is open.')
+ end
+
+ it 'allows committing to the source branch' do
+ find('.ace_text-input', visible: false).send_keys('Updated the readme')
+
+ click_button 'Commit changes'
+ wait_for_requests
+
+ expect(page).to have_content('Your changes have been successfully committed')
+ expect(page).to have_content('Updated the readme')
+ end
+end
diff --git a/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb b/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb
new file mode 100644
index 00000000000..eb41d7de8ed
--- /dev/null
+++ b/spec/features/merge_request/user_allows_a_maintainer_to_push_spec.rb
@@ -0,0 +1,85 @@
+require 'spec_helper'
+
+describe 'create a merge request that allows maintainers to push', :js do
+ include ProjectForksHelper
+ let(:user) { create(:user) }
+ let(:target_project) { create(:project, :public, :repository) }
+ let(:source_project) { fork_project(target_project, user, repository: true, namespace: user.namespace) }
+
+ def visit_new_merge_request
+ visit project_new_merge_request_path(
+ source_project,
+ merge_request: {
+ source_project_id: source_project.id,
+ target_project_id: target_project.id,
+ source_branch: 'fix',
+ target_branch: 'master'
+ })
+ end
+
+ before do
+ sign_in(user)
+ end
+
+ it 'allows setting maintainer push possible' do
+ visit_new_merge_request
+
+ check 'Allow edits from maintainers'
+
+ click_button 'Submit merge request'
+
+ wait_for_requests
+
+ expect(page).to have_content('Allows edits from maintainers')
+ end
+
+ it 'shows a message when one of the projects is private' do
+ source_project.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE)
+
+ visit_new_merge_request
+
+ expect(page).to have_content('Not available for private projects')
+ end
+
+ it 'shows a message when the source branch is protected' do
+ create(:protected_branch, project: source_project, name: 'fix')
+
+ visit_new_merge_request
+
+ expect(page).to have_content('Not available for protected branches')
+ end
+
+ context 'when the merge request is being created within the same project' do
+ let(:source_project) { target_project }
+
+ it 'hides the checkbox if the merge request is being created within the same project' do
+ target_project.add_developer(user)
+
+ visit_new_merge_request
+
+ expect(page).not_to have_content('Allows edits from maintainers')
+ end
+ end
+
+ context 'when a maintainer tries to edit the option' do
+ let(:maintainer) { create(:user) }
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: source_project,
+ target_project: target_project,
+ source_branch: 'fixes')
+ end
+
+ before do
+ target_project.add_master(maintainer)
+
+ sign_in(maintainer)
+ end
+
+ it 'it hides the option from maintainers' do
+ visit edit_project_merge_request_path(target_project, merge_request)
+
+ expect(page).not_to have_content('Allows edits from maintainers')
+ end
+ end
+end
diff --git a/spec/features/projects/user_creates_files_spec.rb b/spec/features/projects/user_creates_files_spec.rb
index 7a935dd2477..8993533676b 100644
--- a/spec/features/projects/user_creates_files_spec.rb
+++ b/spec/features/projects/user_creates_files_spec.rb
@@ -133,13 +133,20 @@ describe 'User creates files' do
before do
project2.add_reporter(user)
visit(project2_tree_path_root_ref)
- end
- it 'creates and commit new file in forked project', :js do
find('.add-to-tree').click
click_link('New file')
+ end
+
+ it 'shows a message saying the file will be committed in a fork' do
+ message = "A new branch will be created in your fork and a new merge request will be started."
+ expect(page).to have_content(message)
+ end
+
+ it 'creates and commit new file in forked project', :js do
expect(page).to have_selector('.file-editor')
+ expect(page).to have_content
find('#editor')
execute_script("ace.edit('editor').setValue('*.rbca')")
diff --git a/spec/fixtures/api/schemas/entities/merge_request_basic.json b/spec/fixtures/api/schemas/entities/merge_request_basic.json
index f1199468d53..46031961cca 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_basic.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_basic.json
@@ -12,7 +12,8 @@
"rebase_in_progress": { "type": "boolean" },
"assignee_id": { "type": ["integer", "null"] },
"subscribed": { "type": ["boolean", "null"] },
- "participants": { "type": "array" }
+ "participants": { "type": "array" },
+ "allow_maintainer_to_push": { "type": "boolean"}
},
"additionalProperties": false
}
diff --git a/spec/fixtures/api/schemas/entities/merge_request_widget.json b/spec/fixtures/api/schemas/entities/merge_request_widget.json
index cfbeec58a45..a622bf88b13 100644
--- a/spec/fixtures/api/schemas/entities/merge_request_widget.json
+++ b/spec/fixtures/api/schemas/entities/merge_request_widget.json
@@ -30,6 +30,7 @@
"source_project_id": { "type": "integer" },
"target_branch": { "type": "string" },
"target_project_id": { "type": "integer" },
+ "allow_maintainer_to_push": { "type": "boolean"},
"metrics": {
"oneOf": [
{ "type": "null" },
diff --git a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json
index e86176e5316..0dc2eabec5d 100644
--- a/spec/fixtures/api/schemas/public_api/v4/merge_requests.json
+++ b/spec/fixtures/api/schemas/public_api/v4/merge_requests.json
@@ -80,7 +80,8 @@
"total_time_spent": { "type": "integer" },
"human_time_estimate": { "type": ["string", "null"] },
"human_total_time_spent": { "type": ["string", "null"] }
- }
+ },
+ "allow_maintainer_to_push": { "type": ["boolean", "null"] }
},
"required": [
"id", "iid", "project_id", "title", "description",
diff --git a/spec/helpers/tree_helper_spec.rb b/spec/helpers/tree_helper_spec.rb
index d3b1be599dd..ccac6e29447 100644
--- a/spec/helpers/tree_helper_spec.rb
+++ b/spec/helpers/tree_helper_spec.rb
@@ -62,4 +62,13 @@ describe TreeHelper do
end
end
end
+
+ describe '#commit_in_single_accessible_branch' do
+ it 'escapes HTML from the branch name' do
+ helper.instance_variable_set(:@branch_name, "<script>alert('escape me!');</script>")
+ escaped_branch_name = '&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
diff --git a/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js b/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js
new file mode 100644
index 00000000000..65b3f721281
--- /dev/null
+++ b/spec/javascripts/vue_mr_widget/components/mr_widget_maintainer_edit_spec.js
@@ -0,0 +1,23 @@
+import Vue from 'vue';
+import maintainerEditComponent from '~/vue_merge_request_widget/components/mr_widget_maintainer_edit.vue';
+import mountComponent from 'spec/helpers/vue_mount_component_helper';
+
+describe('MRWidgetAuthor', () => {
+ let vm;
+
+ beforeEach(() => {
+ const Component = Vue.extend(maintainerEditComponent);
+
+ vm = mountComponent(Component, {
+ maintainerEditAllowed: true,
+ });
+ });
+
+ afterEach(() => {
+ vm.$destroy();
+ });
+
+ it('renders the message when maintainers are allowed to edit', () => {
+ expect(vm.$el.textContent.trim()).toEqual('Allows edits from maintainers');
+ });
+});
diff --git a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
index 18ba34b55a5..ebe151ac3b1 100644
--- a/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
+++ b/spec/javascripts/vue_mr_widget/mr_widget_options_spec.js
@@ -349,6 +349,7 @@ describe('mrWidgetOptions', () => {
expect(comps['mr-widget-pipeline-blocked']).toBeDefined();
expect(comps['mr-widget-pipeline-failed']).toBeDefined();
expect(comps['mr-widget-merge-when-pipeline-succeeds']).toBeDefined();
+ expect(comps['mr-widget-maintainer-edit']).toBeDefined();
});
});
diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb
index b49ddbfc780..48e9902027c 100644
--- a/spec/lib/gitlab/checks/change_access_spec.rb
+++ b/spec/lib/gitlab/checks/change_access_spec.rb
@@ -30,9 +30,10 @@ describe Gitlab::Checks::ChangeAccess do
end
end
- context 'when the user is not allowed to push code' do
+ context 'when the user is not allowed to push to the repo' do
it 'raises an error' do
expect(user_access).to receive(:can_do_action?).with(:push_code).and_return(false)
+ expect(user_access).to receive(:can_push_to_branch?).with('master').and_return(false)
expect { subject.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, 'You are not allowed to push code to this project.')
end
diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml
index b20cc34dd5c..bece82e531a 100644
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -278,6 +278,7 @@ project:
- custom_attributes
- lfs_file_locks
- project_badges
+- source_of_merge_requests
award_emoji:
- awardable
- user
diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml
index ddcbb7a0033..0b938892da5 100644
--- a/spec/lib/gitlab/import_export/safe_model_attributes.yml
+++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml
@@ -168,6 +168,7 @@ MergeRequest:
- last_edited_by_id
- head_pipeline_id
- discussion_locked
+- allow_maintainer_to_push
MergeRequestDiff:
- id
- state
diff --git a/spec/lib/gitlab/user_access_spec.rb b/spec/lib/gitlab/user_access_spec.rb
index 7280acb6c82..40c8286b1b9 100644
--- a/spec/lib/gitlab/user_access_spec.rb
+++ b/spec/lib/gitlab/user_access_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe Gitlab::UserAccess do
+ include ProjectForksHelper
+
let(:access) { described_class.new(user, project: project) }
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
@@ -118,6 +120,39 @@ describe Gitlab::UserAccess do
end
end
+ describe 'allowing pushes to maintainers of forked projects' do
+ let(:canonical_project) { create(:project, :public, :repository) }
+ let(:project) { fork_project(canonical_project, create(:user), repository: true) }
+
+ before do
+ create(
+ :merge_request,
+ target_project: canonical_project,
+ source_project: project,
+ source_branch: 'awesome-feature',
+ allow_maintainer_to_push: true
+ )
+ end
+
+ it 'allows users that have push access to the canonical project to push to the MR branch' do
+ canonical_project.add_developer(user)
+
+ expect(access.can_push_to_branch?('awesome-feature')).to be_truthy
+ end
+
+ it 'does not allow the user to push to other branches' do
+ canonical_project.add_developer(user)
+
+ expect(access.can_push_to_branch?('master')).to be_falsey
+ end
+
+ it 'does not allow the user to push if he does not have push access to the canonical project' do
+ canonical_project.add_guest(user)
+
+ expect(access.can_push_to_branch?('awesome-feature')).to be_falsey
+ end
+ end
+
describe 'merge to protected branch if allowed for developers' do
before do
@branch = create :protected_branch, :developers_can_merge, project: project
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index 243eeddc7a8..7986aa31e16 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -2084,4 +2084,82 @@ describe MergeRequest do
it_behaves_like 'checking whether a rebase is in progress'
end
end
+
+ describe '#allow_maintainer_to_push' do
+ let(:merge_request) do
+ build(:merge_request, source_branch: 'fixes', allow_maintainer_to_push: true)
+ end
+
+ it 'is false when pushing by a maintainer is not possible' do
+ expect(merge_request).to receive(:maintainer_push_possible?) { false }
+
+ expect(merge_request.allow_maintainer_to_push).to be_falsy
+ end
+
+ it 'is true when pushing by a maintainer is possible' do
+ expect(merge_request).to receive(:maintainer_push_possible?) { true }
+
+ expect(merge_request.allow_maintainer_to_push).to be_truthy
+ end
+ end
+
+ describe '#maintainer_push_possible?' do
+ let(:merge_request) do
+ build(:merge_request, source_branch: 'fixes')
+ end
+
+ before do
+ allow(ProtectedBranch).to receive(:protected?) { false }
+ end
+
+ it 'does not allow maintainer to push if the source project is the same as the target' do
+ merge_request.target_project = merge_request.source_project = create(:project, :public)
+
+ expect(merge_request.maintainer_push_possible?).to be_falsy
+ end
+
+ it 'allows maintainer to push when both source and target are public' do
+ merge_request.target_project = build(:project, :public)
+ merge_request.source_project = build(:project, :public)
+
+ expect(merge_request.maintainer_push_possible?).to be_truthy
+ end
+
+ it 'is not available for protected branches' do
+ merge_request.target_project = build(:project, :public)
+ merge_request.source_project = build(:project, :public)
+
+ expect(ProtectedBranch).to receive(:protected?)
+ .with(merge_request.source_project, 'fixes')
+ .and_return(true)
+
+ expect(merge_request.maintainer_push_possible?).to be_falsy
+ end
+ end
+
+ describe '#can_allow_maintainer_to_push?' do
+ let(:target_project) { create(:project, :public) }
+ let(:source_project) { fork_project(target_project) }
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: source_project,
+ source_branch: 'fixes',
+ target_project: target_project)
+ end
+ let(:user) { create(:user) }
+
+ before do
+ allow(merge_request).to receive(:maintainer_push_possible?) { true }
+ end
+
+ it 'is false if the user does not have push access to the source project' do
+ expect(merge_request.can_allow_maintainer_to_push?(user)).to be_falsy
+ end
+
+ it 'is true when the user has push access to the source project' do
+ source_project.add_developer(user)
+
+ expect(merge_request.can_allow_maintainer_to_push?(user)).to be_truthy
+ end
+ end
end
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index b1c9e6754b9..e970cd7dfdb 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe Project do
+ include ProjectForksHelper
+
describe 'associations' do
it { is_expected.to belong_to(:group) }
it { is_expected.to belong_to(:namespace) }
@@ -3378,4 +3380,103 @@ describe Project do
end
end
end
+
+ context 'with cross project merge requests' do
+ let(:user) { create(:user) }
+ let(:target_project) { create(:project, :repository) }
+ let(:project) { fork_project(target_project, nil, repository: true) }
+ let!(:merge_request) do
+ create(
+ :merge_request,
+ target_project: target_project,
+ target_branch: 'target-branch',
+ source_project: project,
+ source_branch: 'awesome-feature-1',
+ allow_maintainer_to_push: true
+ )
+ end
+
+ before do
+ target_project.add_developer(user)
+ end
+
+ describe '#merge_requests_allowing_push_to_user' do
+ it 'returns open merge requests for which the user has developer access to the target project' do
+ expect(project.merge_requests_allowing_push_to_user(user)).to include(merge_request)
+ end
+
+ it 'does not include closed merge requests' do
+ merge_request.close
+
+ expect(project.merge_requests_allowing_push_to_user(user)).to be_empty
+ end
+
+ it 'does not include merge requests for guest users' do
+ guest = create(:user)
+ target_project.add_guest(guest)
+
+ expect(project.merge_requests_allowing_push_to_user(guest)).to be_empty
+ end
+
+ it 'does not include the merge request for other users' do
+ other_user = create(:user)
+
+ expect(project.merge_requests_allowing_push_to_user(other_user)).to be_empty
+ end
+
+ it 'is empty when no user is passed' do
+ expect(project.merge_requests_allowing_push_to_user(nil)).to be_empty
+ end
+ end
+
+ describe '#branch_allows_maintainer_push?' do
+ it 'allows access if the user can merge the merge request' do
+ expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1'))
+ .to be_truthy
+ end
+
+ it 'does not allow guest users access' do
+ guest = create(:user)
+ target_project.add_guest(guest)
+
+ expect(project.branch_allows_maintainer_push?(guest, 'awesome-feature-1'))
+ .to be_falsy
+ end
+
+ it 'does not allow access to branches for which the merge request was closed' do
+ create(:merge_request, :closed,
+ target_project: target_project,
+ target_branch: 'target-branch',
+ source_project: project,
+ source_branch: 'rejected-feature-1',
+ allow_maintainer_to_push: true)
+
+ expect(project.branch_allows_maintainer_push?(user, 'rejected-feature-1'))
+ .to be_falsy
+ end
+
+ it 'does not allow access if the user cannot merge the merge request' do
+ create(:protected_branch, :masters_can_push, project: target_project, name: 'target-branch')
+
+ expect(project.branch_allows_maintainer_push?(user, 'awesome-feature-1'))
+ .to be_falsy
+ end
+
+ it 'caches the result' do
+ control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') }
+
+ expect { 3.times { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') } }
+ .not_to exceed_query_limit(control)
+ end
+
+ context 'when the requeststore is active', :request_store do
+ it 'only queries per project across instances' do
+ control = ActiveRecord::QueryRecorder.new { project.branch_allows_maintainer_push?(user, 'awesome-feature-1') }
+
+ expect { 2.times { described_class.find(project.id).branch_allows_maintainer_push?(user, 'awesome-feature-1') } }
+ .not_to exceed_query_limit(control).with_threshold(2)
+ end
+ end
+ end
+ end
end
diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb
index 129344f105f..ea76e604153 100644
--- a/spec/policies/project_policy_spec.rb
+++ b/spec/policies/project_policy_spec.rb
@@ -308,4 +308,41 @@ describe ProjectPolicy do
it_behaves_like 'project policies as master'
it_behaves_like 'project policies as owner'
it_behaves_like 'project policies as admin'
+
+ context 'when a public project has merge requests allowing access' do
+ include ProjectForksHelper
+ let(:user) { create(:user) }
+ let(:target_project) { create(:project, :public) }
+ let(:project) { fork_project(target_project) }
+ let!(:merge_request) do
+ create(
+ :merge_request,
+ target_project: target_project,
+ source_project: project,
+ allow_maintainer_to_push: true
+ )
+ end
+ let(:maintainer_abilities) do
+ %w(create_build update_build create_pipeline update_pipeline)
+ end
+
+ subject { described_class.new(user, project) }
+
+ it 'does not allow pushing code' do
+ expect_disallowed(*maintainer_abilities)
+ end
+
+ it 'allows pushing if the user is a member with push access to the target project' do
+ target_project.add_developer(user)
+
+ expect_allowed(*maintainer_abilities)
+ end
+
+ it 'dissallows abilities to a maintainer if the merge request was closed' do
+ target_project.add_developer(user)
+ merge_request.close!
+
+ expect_disallowed(*maintainer_abilities)
+ end
+ end
end
diff --git a/spec/requests/api/merge_requests_spec.rb b/spec/requests/api/merge_requests_spec.rb
index 484322752c0..3764aec0c71 100644
--- a/spec/requests/api/merge_requests_spec.rb
+++ b/spec/requests/api/merge_requests_spec.rb
@@ -616,6 +616,25 @@ describe API::MergeRequests do
expect(json_response['changes_count']).to eq('5+')
end
end
+
+ context 'for forked projects' do
+ let(:user2) { create(:user) }
+ let(:project) { create(:project, :public, :repository) }
+ let(:forked_project) { fork_project(project, user2, repository: true) }
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: forked_project,
+ target_project: project,
+ source_branch: 'fixes',
+ allow_maintainer_to_push: true)
+ end
+
+ it 'includes the `allow_maintainer_to_push` field' do
+ get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}", user)
+
+ expect(json_response['allow_maintainer_to_push']).to be_truthy
+ end
+ end
end
describe 'GET /projects/:id/merge_requests/:merge_request_iid/participants' do
@@ -815,6 +834,7 @@ describe API::MergeRequests do
context 'forked projects' do
let!(:user2) { create(:user) }
+ let(:project) { create(:project, :public, :repository) }
let!(:forked_project) { fork_project(project, user2, repository: true) }
let!(:unrelated_project) { create(:project, namespace: create(:user).namespace, creator_id: user2.id) }
@@ -872,6 +892,14 @@ describe API::MergeRequests do
expect(response).to have_gitlab_http_status(400)
end
+ it 'allows setting `allow_maintainer_to_push`' do
+ post api("/projects/#{forked_project.id}/merge_requests", user2),
+ title: 'Test merge_request', source_branch: "feature_conflict", target_branch: "master",
+ author: user2, target_project_id: project.id, allow_maintainer_to_push: true
+ expect(response).to have_gitlab_http_status(201)
+ expect(json_response['allow_maintainer_to_push']).to be_truthy
+ end
+
context 'when target_branch and target_project_id is specified' do
let(:params) do
{ title: 'Test merge_request',
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index c31259239ee..5279ea6164e 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -1,6 +1,8 @@
require 'spec_helper'
describe MergeRequests::UpdateService, :mailer do
+ include ProjectForksHelper
+
let(:project) { create(:project, :repository) }
let(:user) { create(:user) }
let(:user2) { create(:user) }
@@ -538,5 +540,40 @@ describe MergeRequests::UpdateService, :mailer do
let(:open_issuable) { merge_request }
let(:closed_issuable) { create(:closed_merge_request, source_project: project) }
end
+
+ context 'setting `allow_maintainer_to_push`' do
+ let(:target_project) { create(:project, :public) }
+ let(:source_project) { fork_project(target_project) }
+ let(:user) { create(:user) }
+ let(:merge_request) do
+ create(:merge_request,
+ source_project: source_project,
+ source_branch: 'fixes',
+ target_project: target_project)
+ end
+
+ before do
+ allow(ProtectedBranch).to receive(:protected?).with(source_project, 'fixes') { false }
+ end
+
+ it 'does not allow a maintainer of the target project to set `allow_maintainer_to_push`' do
+ target_project.add_developer(user)
+
+ update_merge_request(allow_maintainer_to_push: true, title: 'Updated title')
+
+ expect(merge_request.title).to eq('Updated title')
+ expect(merge_request.allow_maintainer_to_push).to be_falsy
+ end
+
+ it 'is allowed by a user that can push to the source and can update the merge request' do
+ merge_request.update!(assignee: user)
+ source_project.add_developer(user)
+
+ update_merge_request(allow_maintainer_to_push: true, title: 'Updated title')
+
+ expect(merge_request.title).to eq('Updated title')
+ expect(merge_request.allow_maintainer_to_push).to be_truthy
+ end
+ end
end
end
diff --git a/spec/views/projects/tree/show.html.haml_spec.rb b/spec/views/projects/tree/show.html.haml_spec.rb
index 44b32df0395..3b098320ad7 100644
--- a/spec/views/projects/tree/show.html.haml_spec.rb
+++ b/spec/views/projects/tree/show.html.haml_spec.rb
@@ -13,6 +13,7 @@ describe 'projects/tree/show' do
allow(view).to receive(:can?).and_return(true)
allow(view).to receive(:can_collaborate_with_project?).and_return(true)
+ allow(view).to receive_message_chain('user_access.can_push_to_branch?').and_return(true)
allow(view).to receive(:current_application_settings).and_return(Gitlab::CurrentSettings.current_application_settings)
end