diff options
Diffstat (limited to 'app/services/merge_requests')
26 files changed, 200 insertions, 300 deletions
diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index b8d817a15f3..dcc4cf4bb1e 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -10,14 +10,27 @@ module MergeRequests return success unless save_approval(approval) reset_approvals_cache(merge_request) - create_event(merge_request) - stream_audit_event(merge_request) - create_approval_note(merge_request) - mark_pending_todos_as_done(merge_request) - execute_approval_hooks(merge_request, current_user) - remove_attention_requested(merge_request) merge_request_activity_counter.track_approve_mr_action(user: current_user, merge_request: merge_request) + # Approval side effects (things not required to be done immediately but + # should happen after a successful approval) should be done asynchronously + # utilizing the `Gitlab::EventStore`. + # + # Workers can subscribe to the `MergeRequests::ApprovedEvent`. + if Feature.enabled?(:async_after_approval, project) + Gitlab::EventStore.publish( + MergeRequests::ApprovedEvent.new( + data: { current_user_id: current_user.id, merge_request_id: merge_request.id } + ) + ) + else + create_event(merge_request) + stream_audit_event(merge_request) + create_approval_note(merge_request) + mark_pending_todos_as_done(merge_request) + execute_approval_hooks(merge_request, current_user) + end + success end @@ -27,21 +40,22 @@ module MergeRequests current_user.can?(:approve_merge_request, merge_request) end + def save_approval(approval) + Approval.safe_ensure_unique do + approval.save + end + end + def reset_approvals_cache(merge_request) merge_request.approvals.reset end - def execute_approval_hooks(merge_request, current_user) - # Only one approval is required for a merge request to be approved - notification_service.async.approve_mr(merge_request, current_user) - - execute_hooks(merge_request, 'approved') + def create_event(merge_request) + event_service.approve_mr(merge_request, current_user) end - def save_approval(approval) - Approval.safe_ensure_unique do - approval.save - end + def stream_audit_event(merge_request) + # Defined in EE end def create_approval_note(merge_request) @@ -52,12 +66,11 @@ module MergeRequests todo_service.resolve_todos_for_target(merge_request, current_user) end - def create_event(merge_request) - event_service.approve_mr(merge_request, current_user) - end + def execute_approval_hooks(merge_request, current_user) + # Only one approval is required for a merge request to be approved + notification_service.async.approve_mr(merge_request, current_user) - def stream_audit_event(merge_request) - # Defined in EE + execute_hooks(merge_request, 'approved') end end end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 9bd38478796..bda8dc64ac0 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -61,10 +61,6 @@ module MergeRequests merge_request_activity_counter.track_users_review_requested(users: new_reviewers) merge_request_activity_counter.track_reviewers_changed_action(user: current_user) bulk_update_reviewers_state(merge_request, new_reviewers) - - unless new_reviewers.include?(current_user) - remove_attention_requested(merge_request) - end end def cleanup_environments(merge_request) @@ -252,20 +248,6 @@ module MergeRequests Milestones::MergeRequestsCountService.new(milestone).delete_cache end - def remove_all_attention_requests(merge_request) - return unless current_user.mr_attention_requests_enabled? - - users = merge_request.reviewers + merge_request.assignees - - ::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, users: users.uniq).execute - end - - def remove_attention_requested(merge_request) - return unless current_user.mr_attention_requests_enabled? - - ::MergeRequests::RemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: current_user).execute - end - def bulk_update_assignees_state(merge_request, new_assignees) return unless current_user.mr_attention_requests_enabled? return if new_assignees.empty? diff --git a/app/services/merge_requests/bulk_remove_attention_requested_service.rb b/app/services/merge_requests/bulk_remove_attention_requested_service.rb deleted file mode 100644 index 774f2c2ee35..00000000000 --- a/app/services/merge_requests/bulk_remove_attention_requested_service.rb +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class BulkRemoveAttentionRequestedService < MergeRequests::BaseService - attr_accessor :merge_request - attr_accessor :users - - def initialize(project:, current_user:, merge_request:, users:) - super(project: project, current_user: current_user) - - @merge_request = merge_request - @users = users - end - - # rubocop: disable CodeReuse/ActiveRecord - def execute - return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - - merge_request.merge_request_assignees.where(user_id: users).update_all(state: :reviewed) - merge_request.merge_request_reviewers.where(user_id: users).update_all(state: :reviewed) - - users.each { |user| user.invalidate_attention_requested_count } - - success - end - # rubocop: enable CodeReuse/ActiveRecord - end -end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index e9b253129b4..f83b14c7269 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -17,7 +17,6 @@ module MergeRequests create_note(merge_request) notification_service.async.close_mr(merge_request, current_user) todo_service.close_merge_request(merge_request, current_user) - remove_all_attention_requests(merge_request) execute_hooks(merge_request, 'close') invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers) merge_request.update_project_counter_caches diff --git a/app/services/merge_requests/create_approval_event_service.rb b/app/services/merge_requests/create_approval_event_service.rb new file mode 100644 index 00000000000..1678bf31139 --- /dev/null +++ b/app/services/merge_requests/create_approval_event_service.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module MergeRequests + class CreateApprovalEventService < MergeRequests::BaseService + def execute(merge_request) + event_service.approve_mr(merge_request, current_user) + end + end +end + +MergeRequests::CreateApprovalEventService.prepend_mod diff --git a/app/services/merge_requests/create_pipeline_service.rb b/app/services/merge_requests/create_pipeline_service.rb index c6a91a3b61e..4f20ade2a42 100644 --- a/app/services/merge_requests/create_pipeline_service.rb +++ b/app/services/merge_requests/create_pipeline_service.rb @@ -50,7 +50,8 @@ module MergeRequests end def can_create_pipeline_in_target_project?(merge_request) - can?(current_user, :create_pipeline, merge_request.target_project) && + merge_request.target_project.ci_allow_fork_pipelines_to_run_in_parent_project? && + can?(current_user, :create_pipeline, merge_request.target_project) && can_update_source_branch_in_target_project?(merge_request) end diff --git a/app/services/merge_requests/execute_approval_hooks_service.rb b/app/services/merge_requests/execute_approval_hooks_service.rb new file mode 100644 index 00000000000..7beeb9ea3f9 --- /dev/null +++ b/app/services/merge_requests/execute_approval_hooks_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module MergeRequests + class ExecuteApprovalHooksService < MergeRequests::BaseService + def execute(merge_request) + # Only one approval is required for a merge request to be approved + notification_service.async.approve_mr(merge_request, current_user) + execute_hooks(merge_request, 'approved') + end + end +end + +MergeRequests::ExecuteApprovalHooksService.prepend_mod diff --git a/app/services/merge_requests/handle_assignees_change_service.rb b/app/services/merge_requests/handle_assignees_change_service.rb index 78c93d10f2a..87cd6544406 100644 --- a/app/services/merge_requests/handle_assignees_change_service.rb +++ b/app/services/merge_requests/handle_assignees_change_service.rb @@ -22,10 +22,6 @@ module MergeRequests merge_request_activity_counter.track_assignees_changed_action(user: current_user) execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks] - - unless new_assignees.include?(current_user) - remove_attention_requested(merge_request) - end end private diff --git a/app/services/merge_requests/mergeability/check_base_service.rb b/app/services/merge_requests/mergeability/check_base_service.rb index d5ddcb4b828..e614a7c27fe 100644 --- a/app/services/merge_requests/mergeability/check_base_service.rb +++ b/app/services/merge_requests/mergeability/check_base_service.rb @@ -24,12 +24,12 @@ module MergeRequests private - def success(*args) - Gitlab::MergeRequests::Mergeability::CheckResult.success(*args) + def success(**args) + Gitlab::MergeRequests::Mergeability::CheckResult.success(payload: args) end - def failure(*args) - Gitlab::MergeRequests::Mergeability::CheckResult.failed(*args) + def failure(**args) + Gitlab::MergeRequests::Mergeability::CheckResult.failed(payload: args) end end end diff --git a/app/services/merge_requests/mergeability/check_broken_status_service.rb b/app/services/merge_requests/mergeability/check_broken_status_service.rb index 9a54a4292c8..6fe4eb4a57f 100644 --- a/app/services/merge_requests/mergeability/check_broken_status_service.rb +++ b/app/services/merge_requests/mergeability/check_broken_status_service.rb @@ -4,7 +4,7 @@ module MergeRequests class CheckBrokenStatusService < CheckBaseService def execute if merge_request.broken? - failure + failure(reason: failure_reason) else success end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :broken_status + end end end end diff --git a/app/services/merge_requests/mergeability/check_ci_status_service.rb b/app/services/merge_requests/mergeability/check_ci_status_service.rb index c0ef5ba1c30..9e09b513c57 100644 --- a/app/services/merge_requests/mergeability/check_ci_status_service.rb +++ b/app/services/merge_requests/mergeability/check_ci_status_service.rb @@ -6,7 +6,7 @@ module MergeRequests if merge_request.mergeable_ci_state? success else - failure + failure(reason: failure_reason) end end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :ci_must_pass + end end end end diff --git a/app/services/merge_requests/mergeability/check_discussions_status_service.rb b/app/services/merge_requests/mergeability/check_discussions_status_service.rb index 9b4eab9d399..3421d96e8ae 100644 --- a/app/services/merge_requests/mergeability/check_discussions_status_service.rb +++ b/app/services/merge_requests/mergeability/check_discussions_status_service.rb @@ -6,7 +6,7 @@ module MergeRequests if merge_request.mergeable_discussions_state? success else - failure + failure(reason: failure_reason) end end @@ -17,6 +17,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :discussions_not_resolved + end end end end diff --git a/app/services/merge_requests/mergeability/check_draft_status_service.rb b/app/services/merge_requests/mergeability/check_draft_status_service.rb index bc940e2116d..a1524317155 100644 --- a/app/services/merge_requests/mergeability/check_draft_status_service.rb +++ b/app/services/merge_requests/mergeability/check_draft_status_service.rb @@ -5,7 +5,7 @@ module MergeRequests class CheckDraftStatusService < CheckBaseService def execute if merge_request.draft? - failure + failure(reason: failure_reason) else success end @@ -18,6 +18,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :draft_status + end end end end diff --git a/app/services/merge_requests/mergeability/check_open_status_service.rb b/app/services/merge_requests/mergeability/check_open_status_service.rb index 361af946e3f..29f3d0d3ccb 100644 --- a/app/services/merge_requests/mergeability/check_open_status_service.rb +++ b/app/services/merge_requests/mergeability/check_open_status_service.rb @@ -7,7 +7,7 @@ module MergeRequests if merge_request.open? success else - failure + failure(reason: failure_reason) end end @@ -18,6 +18,12 @@ module MergeRequests def cacheable? false end + + private + + def failure_reason + :not_open + end end end end diff --git a/app/services/merge_requests/mergeability/run_checks_service.rb b/app/services/merge_requests/mergeability/run_checks_service.rb index 1d4b96b3090..68f842b3322 100644 --- a/app/services/merge_requests/mergeability/run_checks_service.rb +++ b/app/services/merge_requests/mergeability/run_checks_service.rb @@ -10,36 +10,50 @@ module MergeRequests end def execute - merge_request.mergeability_checks.each_with_object([]) do |check_class, results| + @results = merge_request.mergeability_checks.each_with_object([]) do |check_class, result_hash| check = check_class.new(merge_request: merge_request, params: params) next if check.skip? check_result = run_check(check) - results << check_result + result_hash << check_result - break results if check_result.failed? + break result_hash if check_result.failed? end + + self + end + + def success? + raise 'Execute needs to be called before' if results.nil? + + results.all?(&:success?) + end + + def failure_reason + raise 'Execute needs to be called before' if results.nil? + + results.find(&:failed?)&.payload&.fetch(:reason) end private - attr_reader :merge_request, :params + attr_reader :merge_request, :params, :results def run_check(check) return check.execute unless Feature.enabled?(:mergeability_caching, merge_request.project) return check.execute unless check.cacheable? - cached_result = results.read(merge_check: check) + cached_result = cached_results.read(merge_check: check) return cached_result if cached_result.respond_to?(:status) check.execute.tap do |result| - results.write(merge_check: check, result_hash: result.to_hash) + cached_results.write(merge_check: check, result_hash: result.to_hash) end end - def results - strong_memoize(:results) do + def cached_results + strong_memoize(:cached_results) do Gitlab::MergeRequests::Mergeability::ResultsStore.new(merge_request: merge_request) end end diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb index 30531fcc17b..1ce44f465cd 100644 --- a/app/services/merge_requests/mergeability_check_service.rb +++ b/app/services/merge_requests/mergeability_check_service.rb @@ -78,8 +78,8 @@ module MergeRequests lease_key = "mergeability_check:#{merge_request.id}" lease_opts = { - ttl: 1.minute, - retries: retry_lease ? 10 : 0, + ttl: 1.minute, + retries: retry_lease ? 10 : 0, sleep_sec: retry_lease ? 1.second : 0 } diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index 286c082ac8a..9fca2b0d19e 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -28,7 +28,6 @@ module MergeRequests notification_service.merge_mr(merge_request, current_user) invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers) merge_request.update_project_counter_caches - remove_all_attention_requests(merge_request) delete_non_latest_diffs(merge_request) cancel_review_app_jobs!(merge_request) cleanup_environments(merge_request) diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb index 076fe8c3b21..ef251f121ae 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -105,7 +105,7 @@ module MergeRequests project: project, current_user: current_user, params: merge_request.attributes.merge(assignees: merge_request.assignees, - label_ids: merge_request.label_ids) + label_ids: merge_request.label_ids) ).execute end diff --git a/app/services/merge_requests/remove_approval_service.rb b/app/services/merge_requests/remove_approval_service.rb index d9bb17a7b1b..52628729519 100644 --- a/app/services/merge_requests/remove_approval_service.rb +++ b/app/services/merge_requests/remove_approval_service.rb @@ -17,7 +17,6 @@ module MergeRequests reset_approvals_cache(merge_request) create_note(merge_request) merge_request_activity_counter.track_unapprove_mr_action(user: current_user) - remove_attention_requested(merge_request) end success diff --git a/app/services/merge_requests/remove_attention_requested_service.rb b/app/services/merge_requests/remove_attention_requested_service.rb deleted file mode 100644 index 8a410fda691..00000000000 --- a/app/services/merge_requests/remove_attention_requested_service.rb +++ /dev/null @@ -1,50 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class RemoveAttentionRequestedService < MergeRequests::BaseService - attr_accessor :merge_request, :user - - def initialize(project:, current_user:, merge_request:, user:) - super(project: project, current_user: current_user) - - @merge_request = merge_request - @user = user - end - - def execute - return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - - if reviewer || assignee - return success if reviewer&.reviewed? || assignee&.reviewed? - - update_state(reviewer) - update_state(assignee) - - user.invalidate_attention_requested_count - create_remove_attention_request_note - - success - else - error("User is not a reviewer or assignee of the merge request") - end - end - - private - - def assignee - @assignee ||= merge_request.find_assignee(user) - end - - def reviewer - @reviewer ||= merge_request.find_reviewer(user) - end - - def update_state(reviewer_or_assignee) - reviewer_or_assignee&.update(state: :reviewed) - end - - def create_remove_attention_request_note - SystemNoteService.remove_attention_request(merge_request, merge_request.project, current_user, user) - end - end -end diff --git a/app/services/merge_requests/reopen_service.rb b/app/services/merge_requests/reopen_service.rb index 4612688f78b..d2247a6d4c1 100644 --- a/app/services/merge_requests/reopen_service.rb +++ b/app/services/merge_requests/reopen_service.rb @@ -20,8 +20,6 @@ module MergeRequests merge_request.cache_merge_request_closes_issues!(current_user) merge_request.cleanup_schedule&.destroy merge_request.update_column(:merge_ref_sha, nil) - - users.each { |user| user.invalidate_attention_requested_count } end merge_request diff --git a/app/services/merge_requests/request_attention_service.rb b/app/services/merge_requests/request_attention_service.rb deleted file mode 100644 index 07e9996f87b..00000000000 --- a/app/services/merge_requests/request_attention_service.rb +++ /dev/null @@ -1,60 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class RequestAttentionService < MergeRequests::BaseService - attr_accessor :merge_request, :user - - def initialize(project:, current_user:, merge_request:, user:) - super(project: project, current_user: current_user) - - @merge_request = merge_request - @user = user - end - - def execute - return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - - if reviewer || assignee - return success if reviewer&.attention_requested? || assignee&.attention_requested? - - update_state(reviewer) - update_state(assignee) - - user.invalidate_attention_requested_count - create_attention_request_note - notity_user - - if current_user.id != user.id - remove_attention_requested(merge_request) - end - - success - else - error("User is not a reviewer or assignee of the merge request") - end - end - - private - - def notity_user - notification_service.async.attention_requested_of_merge_request(merge_request, current_user, user) - todo_service.create_attention_requested_todo(merge_request, current_user, user) - end - - def create_attention_request_note - SystemNoteService.request_attention(merge_request, merge_request.project, current_user, user) - end - - def assignee - @assignee ||= merge_request.find_assignee(user) - end - - def reviewer - @reviewer ||= merge_request.find_reviewer(user) - end - - def update_state(reviewer_or_assignee) - reviewer_or_assignee&.update(state: :attention_requested, updated_state_by: current_user) - end - end -end diff --git a/app/services/merge_requests/toggle_attention_requested_service.rb b/app/services/merge_requests/toggle_attention_requested_service.rb deleted file mode 100644 index 64cdcd725a2..00000000000 --- a/app/services/merge_requests/toggle_attention_requested_service.rb +++ /dev/null @@ -1,68 +0,0 @@ -# frozen_string_literal: true - -module MergeRequests - class ToggleAttentionRequestedService < MergeRequests::BaseService - attr_accessor :merge_request, :user - - def initialize(project:, current_user:, merge_request:, user:) - super(project: project, current_user: current_user) - - @merge_request = merge_request - @user = user - end - - def execute - return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) - - if reviewer || assignee - update_state(reviewer) - update_state(assignee) - - user.invalidate_attention_requested_count - - if reviewer&.attention_requested? || assignee&.attention_requested? - create_attention_request_note - notity_user - - if current_user.id != user.id - remove_attention_requested(merge_request) - end - else - create_remove_attention_request_note - end - - success - else - error("User is not a reviewer or assignee of the merge request") - end - end - - private - - def notity_user - notification_service.async.attention_requested_of_merge_request(merge_request, current_user, user) - todo_service.create_attention_requested_todo(merge_request, current_user, user) - end - - def create_attention_request_note - SystemNoteService.request_attention(merge_request, merge_request.project, current_user, user) - end - - def create_remove_attention_request_note - SystemNoteService.remove_attention_request(merge_request, merge_request.project, current_user, user) - end - - def assignee - merge_request.find_assignee(user) - end - - def reviewer - merge_request.find_reviewer(user) - end - - def update_state(reviewer_or_assignee) - reviewer_or_assignee&.update(state: reviewer_or_assignee&.attention_requested? ? :reviewed : :attention_requested, - updated_state_by: current_user) - end - end -end diff --git a/app/services/merge_requests/update_assignees_service.rb b/app/services/merge_requests/update_assignees_service.rb index 5b23f69ac4a..a6b0235c525 100644 --- a/app/services/merge_requests/update_assignees_service.rb +++ b/app/services/merge_requests/update_assignees_service.rb @@ -11,7 +11,7 @@ module MergeRequests old_assignees = merge_request.assignees.to_a old_ids = old_assignees.map(&:id) - new_ids = new_assignee_ids(merge_request) + new_ids = new_user_ids(merge_request, update_attrs[:assignee_ids], :assignees) return merge_request if merge_request.errors.any? return merge_request if new_ids.size != update_attrs[:assignee_ids].size @@ -32,27 +32,8 @@ module MergeRequests private - def new_assignee_ids(merge_request) - # prime the cache - prevent N+1 lookup during authorization loop. - user_ids = update_attrs[:assignee_ids] - return [] if user_ids.empty? - - merge_request.project.team.max_member_access_for_user_ids(user_ids) - User.id_in(user_ids).map do |user| - if user.can?(:read_merge_request, merge_request) - user.id - else - merge_request.errors.add( - :assignees, - "Cannot assign #{user.to_reference} to #{merge_request.to_reference}" - ) - nil - end - end.compact - end - def assignee_ids - params.fetch(:assignee_ids).reject { _1 == 0 }.first(1) + filter_sentinel_values(params.fetch(:assignee_ids)).first(1) end def params diff --git a/app/services/merge_requests/update_reviewers_service.rb b/app/services/merge_requests/update_reviewers_service.rb new file mode 100644 index 00000000000..8e974d75676 --- /dev/null +++ b/app/services/merge_requests/update_reviewers_service.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +module MergeRequests + class UpdateReviewersService < UpdateService + def execute(merge_request) + return merge_request unless current_user&.can?(:update_merge_request, merge_request) + + old_reviewers = merge_request.reviewers.to_a + old_ids = old_reviewers.map(&:id) + new_ids = new_user_ids(merge_request, update_attrs[:reviewer_ids], :reviewers) + + return merge_request if merge_request.errors.any? + return merge_request if new_ids.size != update_attrs[:reviewer_ids].size + return merge_request if old_ids.to_set == new_ids.to_set # no-change + + merge_request.update!(update_attrs.merge(reviewer_ids: new_ids)) + handle_reviewers_change(merge_request, old_reviewers) + resolve_todos_for(merge_request) + execute_reviewers_hooks(merge_request, old_reviewers) + + merge_request + end + + private + + def reviewer_ids + filter_sentinel_values(params.fetch(:reviewer_ids)).first(1) + end + + def update_attrs + @attrs ||= { updated_by: current_user, reviewer_ids: reviewer_ids } + end + + def execute_reviewers_hooks(merge_request, old_reviewers) + execute_hooks( + merge_request, + 'update', + old_associations: { reviewers: old_reviewers } + ) + end + end +end + +MergeRequests::UpdateReviewersService.prepend_mod diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 603da4ef535..0902b5195a1 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -155,11 +155,7 @@ module MergeRequests def resolve_todos(merge_request, old_labels, old_assignees, old_reviewers) return unless has_changes?(merge_request, old_labels: old_labels, old_assignees: old_assignees, old_reviewers: old_reviewers) - service_user = current_user - - merge_request.run_after_commit_or_now do - ::MergeRequests::ResolveTodosService.new(merge_request, service_user).async_execute - end + resolve_todos_for(merge_request) end def handle_target_branch_change(merge_request) @@ -296,6 +292,36 @@ module MergeRequests def add_time_spent_service @add_time_spent_service ||= ::MergeRequests::AddSpentTimeService.new(project: project, current_user: current_user, params: params) end + + def new_user_ids(merge_request, user_ids, attribute) + # prime the cache - prevent N+1 lookup during authorization loop. + return [] if user_ids.empty? + + merge_request.project.team.max_member_access_for_user_ids(user_ids) + User.id_in(user_ids).map do |user| + if user.can?(:read_merge_request, merge_request) + user.id + else + merge_request.errors.add( + attribute, + "Cannot assign #{user.to_reference} to #{merge_request.to_reference}" + ) + nil + end + end.compact + end + + def resolve_todos_for(merge_request) + service_user = current_user + + merge_request.run_after_commit_or_now do + ::MergeRequests::ResolveTodosService.new(merge_request, service_user).async_execute + end + end + + def filter_sentinel_values(param) + param.reject { _1 == 0 } + end end end |