diff options
author | Yorick Peterse <yorick@yorickpeterse.com> | 2019-10-30 17:22:45 +0300 |
---|---|---|
committer | Yorick Peterse <yorick@yorickpeterse.com> | 2019-10-30 17:22:45 +0300 |
commit | ad8eea383406037a207c80421e6e4bfa357f8044 (patch) | |
tree | 396b89ad72b9d7e35fab26c6ee22c978a12defbb /app/services | |
parent | 228d752ff09362002cc904d28edee7d63cc3cef2 (diff) | |
parent | b0f939a79fe16ff760d6e589c8f9cd71c0fa1da7 (diff) |
Merge dev.gitlab.org@master into GitLab.com@master
Diffstat (limited to 'app/services')
-rw-r--r-- | app/services/auto_merge/base_service.rb | 7 | ||||
-rw-r--r-- | app/services/concerns/merge_requests/assigns_merge_params.rb | 24 | ||||
-rw-r--r-- | app/services/error_tracking/list_projects_service.rb | 9 | ||||
-rw-r--r-- | app/services/merge_requests/base_service.rb | 14 | ||||
-rw-r--r-- | app/services/merge_requests/build_service.rb | 2 | ||||
-rw-r--r-- | app/services/merge_requests/create_service.rb | 1 | ||||
-rw-r--r-- | app/services/merge_requests/update_service.rb | 4 | ||||
-rw-r--r-- | app/services/notification_service.rb | 2 | ||||
-rw-r--r-- | app/services/projects/operations/update_service.rb | 6 | ||||
-rw-r--r-- | app/services/projects/participants_service.rb | 57 | ||||
-rw-r--r-- | app/services/projects/transfer_service.rb | 2 |
11 files changed, 113 insertions, 15 deletions
diff --git a/app/services/auto_merge/base_service.rb b/app/services/auto_merge/base_service.rb index e06659a39cd..e08b4ac2260 100644 --- a/app/services/auto_merge/base_service.rb +++ b/app/services/auto_merge/base_service.rb @@ -3,12 +3,13 @@ module AutoMerge class BaseService < ::BaseService include Gitlab::Utils::StrongMemoize + include MergeRequests::AssignsMergeParams def execute(merge_request) - merge_request.merge_params.merge!(params) + assign_allowed_merge_params(merge_request, params.merge(auto_merge_strategy: strategy)) + merge_request.auto_merge_enabled = true merge_request.merge_user = current_user - merge_request.auto_merge_strategy = strategy return :failed unless merge_request.save @@ -21,7 +22,7 @@ module AutoMerge end def update(merge_request) - merge_request.merge_params.merge!(params) + assign_allowed_merge_params(merge_request, params.merge(auto_merge_strategy: strategy)) return :failed unless merge_request.save diff --git a/app/services/concerns/merge_requests/assigns_merge_params.rb b/app/services/concerns/merge_requests/assigns_merge_params.rb new file mode 100644 index 00000000000..bd870d9a1e7 --- /dev/null +++ b/app/services/concerns/merge_requests/assigns_merge_params.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module MergeRequests + module AssignsMergeParams + def self.included(klass) + raise "#{self} can not be included in #{klass} without implementing #current_user" unless klass.method_defined?(:current_user) + end + + def assign_allowed_merge_params(merge_request, merge_params) + known_merge_params = merge_params.to_h.with_indifferent_access.slice(*MergeRequest::KNOWN_MERGE_PARAMS) + + # Not checking `MergeRequest#can_remove_source_branch` as that includes + # other checks that aren't needed here. + known_merge_params.delete(:force_remove_source_branch) unless current_user.can?(:push_code, merge_request.source_project) + + merge_request.merge_params.merge!(known_merge_params) + + # Delete the known params now that they're assigned, so we don't try to + # assign them through an `#assign_attributes` later. + # They could be coming in as strings or symbols + merge_params.to_h.with_indifferent_access.except!(*MergeRequest::KNOWN_MERGE_PARAMS) + end + end +end diff --git a/app/services/error_tracking/list_projects_service.rb b/app/services/error_tracking/list_projects_service.rb index 8d08f0cda94..92d4ef85ecf 100644 --- a/app/services/error_tracking/list_projects_service.rb +++ b/app/services/error_tracking/list_projects_service.rb @@ -32,7 +32,7 @@ module ErrorTracking project_slug: 'proj' ) - setting.token = params[:token] + setting.token = token(setting) setting.enabled = true end end @@ -40,5 +40,12 @@ module ErrorTracking def can_read? can?(current_user, :read_sentry_issue, project) end + + def token(setting) + # Use param token if not masked, otherwise use database token + return params[:token] unless /\A\*+\z/.match?(params[:token]) + + setting.token + end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 7d4227e4a41..aacc3d6831e 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -2,6 +2,8 @@ module MergeRequests class BaseService < ::IssuableBaseService + include MergeRequests::AssignsMergeParams + def create_note(merge_request, state = merge_request.state) SystemNoteService.change_status(merge_request, merge_request.target_project, current_user, state, nil) end @@ -29,6 +31,18 @@ module MergeRequests private + def create(merge_request) + self.params = assign_allowed_merge_params(merge_request, params) + + super + end + + def update(merge_request) + self.params = assign_allowed_merge_params(merge_request, params) + + super + end + def handle_wip_event(merge_request) if wip_event = params.delete(:wip_event) # We update the title that is provided in the params or we use the mr title diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 06ee25eac2a..456cc589477 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -24,6 +24,8 @@ module MergeRequests merge_request.source_project.remove_source_branch_after_merge? end + self.params = assign_allowed_merge_params(merge_request, params) + filter_params(merge_request) # merge_request.assign_attributes(...) below is a Rails diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 1c730232abb..9a37a0330fc 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -9,7 +9,6 @@ module MergeRequests merge_request.target_project = @project merge_request.source_project = @source_project merge_request.source_branch = params[:source_branch] - merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) create(merge_request) end diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index ae678d4c036..7c9abb12b6e 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -16,10 +16,6 @@ module MergeRequests params.delete(:force_remove_source_branch) end - if params.has_key?(:force_remove_source_branch) - merge_request.merge_params['force_remove_source_branch'] = params.delete(:force_remove_source_branch) - end - handle_wip_event(merge_request) update_task_event(merge_request) || update(merge_request) end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index b56b2cf14e3..1709474a6c7 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -281,7 +281,7 @@ class NotificationService end def send_new_note_notifications(note) - notify_method = "note_#{note.to_ability_name}_email".to_sym + notify_method = "note_#{note.noteable_ability_name}_email".to_sym recipients = NotificationRecipientService.build_new_note_recipients(note) recipients.each do |recipient| diff --git a/app/services/projects/operations/update_service.rb b/app/services/projects/operations/update_service.rb index 64519501ff4..0ca89664304 100644 --- a/app/services/projects/operations/update_service.rb +++ b/app/services/projects/operations/update_service.rb @@ -36,15 +36,17 @@ module Projects organization_slug: settings.dig(:project, :organization_slug) ) - { + params = { error_tracking_setting_attributes: { api_url: api_url, - token: settings[:token], enabled: settings[:enabled], project_name: settings.dig(:project, :name), organization_name: settings.dig(:project, :organization_name) } } + params[:error_tracking_setting_attributes][:token] = settings[:token] unless /\A\*+\z/.match?(settings[:token]) # Don't update token if we receive masked value + + params end def grafana_integration_params diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index 7080f388e53..1cd81fe37c7 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -7,16 +7,69 @@ module Projects def execute(noteable) @noteable = noteable - participants = noteable_owner + participants_in_noteable + all_members + groups + project_members + participants = + noteable_owner + + participants_in_noteable + + all_members + + groups + + project_members + participants.uniq end def project_members - @project_members ||= sorted(project.team.members) + @project_members ||= sorted(get_project_members) + end + + def get_project_members + members = Member.from_union([project_members_through_ancestral_groups, + project_members_through_invited_groups, + individual_project_members]) + + User.id_in(members.select(:user_id)) end def all_members [{ username: "all", name: "All Project and Group Members", count: project_members.count }] end + + private + + def project_members_through_invited_groups + groups_with_ancestors_ids = Gitlab::ObjectHierarchy + .new(visible_groups) + .base_and_ancestors + .pluck_primary_key + + GroupMember + .active_without_invites_and_requests + .with_source_id(groups_with_ancestors_ids) + end + + def visible_groups + visible_groups = project.invited_groups + + unless project_owner? + visible_groups = visible_groups.public_or_visible_to_user(current_user) + end + + visible_groups + end + + def project_members_through_ancestral_groups + project.group.present? ? project.group.members_with_parents : Member.none + end + + def individual_project_members + project.project_members + end + + def project_owner? + if project.group.present? + project.group.owners.include?(current_user) + else + project.namespace.owner == current_user + end + end end end diff --git a/app/services/projects/transfer_service.rb b/app/services/projects/transfer_service.rb index 4b3aca58dd7..525fc18b312 100644 --- a/app/services/projects/transfer_service.rb +++ b/app/services/projects/transfer_service.rb @@ -98,7 +98,7 @@ module Projects @new_namespace && can?(current_user, :change_namespace, project) && @new_namespace.id != project.namespace_id && - current_user.can?(:create_projects, @new_namespace) + current_user.can?(:transfer_projects, @new_namespace) end def update_namespace_and_visibility(to_namespace) |