diff options
Diffstat (limited to 'app/services/merge_requests')
14 files changed, 158 insertions, 16 deletions
diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index 77564521d45..d2c83f82ff8 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -2,13 +2,22 @@ module MergeRequests class AfterCreateService < MergeRequests::BaseService + include Gitlab::Utils::StrongMemoize + def execute(merge_request) + prepare_for_mergeability(merge_request) if early_prepare_for_mergeability?(merge_request) prepare_merge_request(merge_request) - merge_request.mark_as_unchecked if merge_request.preparing? + mark_as_unchecked(merge_request) unless early_prepare_for_mergeability?(merge_request) end private + def prepare_for_mergeability(merge_request) + create_pipeline_for(merge_request, current_user) + merge_request.update_head_pipeline + mark_as_unchecked(merge_request) + end + def prepare_merge_request(merge_request) event_service.open_mr(merge_request, current_user) @@ -17,8 +26,10 @@ module MergeRequests notification_service.new_merge_request(merge_request, current_user) - create_pipeline_for(merge_request, current_user) - merge_request.update_head_pipeline + unless early_prepare_for_mergeability?(merge_request) + create_pipeline_for(merge_request, current_user) + merge_request.update_head_pipeline + end merge_request.diffs(include_stats: false).write_cache merge_request.create_cross_references!(current_user) @@ -37,6 +48,16 @@ module MergeRequests def link_lfs_objects(merge_request) LinkLfsObjectsService.new(project: merge_request.target_project).execute(merge_request) end + + def early_prepare_for_mergeability?(merge_request) + strong_memoize("early_prepare_for_mergeability_#{merge_request.target_project_id}".to_sym) do + Feature.enabled?(:early_prepare_for_mergeability, merge_request.target_project) + end + end + + def mark_as_unchecked(merge_request) + merge_request.mark_as_unchecked if merge_request.preparing? + end end end diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index 62e599e3e27..3f39b2742c6 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -14,6 +14,7 @@ module MergeRequests create_approval_note(merge_request) mark_pending_todos_as_done(merge_request) execute_approval_hooks(merge_request, current_user) + remove_attention_requested(merge_request, current_user) merge_request_activity_counter.track_approve_mr_action(user: current_user) success diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 0a652c58aab..d744881549a 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -58,6 +58,8 @@ module MergeRequests new_reviewers = merge_request.reviewers - old_reviewers merge_request_activity_counter.track_users_review_requested(users: new_reviewers) merge_request_activity_counter.track_reviewers_changed_action(user: current_user) + + remove_attention_requested(merge_request, current_user) end def cleanup_environments(merge_request) @@ -238,6 +240,18 @@ module MergeRequests Milestones::MergeRequestsCountService.new(milestone).delete_cache end + + def remove_all_attention_requests(merge_request) + return unless merge_request.attention_requested_enabled? + + ::MergeRequests::BulkRemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request).execute + end + + def remove_attention_requested(merge_request, user) + return unless merge_request.attention_requested_enabled? + + ::MergeRequests::RemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: user).execute + end end end diff --git a/app/services/merge_requests/bulk_remove_attention_requested_service.rb b/app/services/merge_requests/bulk_remove_attention_requested_service.rb new file mode 100644 index 00000000000..dd2ff741ba6 --- /dev/null +++ b/app/services/merge_requests/bulk_remove_attention_requested_service.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module MergeRequests + class BulkRemoveAttentionRequestedService < MergeRequests::BaseService + attr_accessor :merge_request + + def initialize(project:, current_user:, merge_request:) + super(project: project, current_user: current_user) + + @merge_request = merge_request + end + + def execute + return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) + + merge_request.merge_request_assignees.update_all(state: :reviewed) + merge_request.merge_request_reviewers.update_all(state: :reviewed) + + success + end + end +end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index f83b14c7269..e9b253129b4 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -17,6 +17,7 @@ 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_pipeline_service.rb b/app/services/merge_requests/create_pipeline_service.rb index 6b032545230..9d7f8393ba5 100644 --- a/app/services/merge_requests/create_pipeline_service.rb +++ b/app/services/merge_requests/create_pipeline_service.rb @@ -48,7 +48,7 @@ module MergeRequests end def can_create_pipeline_in_target_project?(merge_request) - if Gitlab::Ci::Features.disallow_to_create_merge_request_pipelines_in_target_project?(merge_request.target_project) + if ::Feature.enabled?(:ci_disallow_to_create_merge_request_pipelines_in_target_project, merge_request.target_project) merge_request.for_same_project? else can?(current_user, :create_pipeline, merge_request.target_project) && diff --git a/app/services/merge_requests/handle_assignees_change_service.rb b/app/services/merge_requests/handle_assignees_change_service.rb index 87cd6544406..1d9f7ab59f4 100644 --- a/app/services/merge_requests/handle_assignees_change_service.rb +++ b/app/services/merge_requests/handle_assignees_change_service.rb @@ -22,6 +22,8 @@ module MergeRequests merge_request_activity_counter.track_assignees_changed_action(user: current_user) execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks] + + remove_attention_requested(merge_request, current_user) end private diff --git a/app/services/merge_requests/outdated_discussion_diff_lines_service.rb b/app/services/merge_requests/outdated_discussion_diff_lines_service.rb index a2de5a32963..a3d94e888df 100644 --- a/app/services/merge_requests/outdated_discussion_diff_lines_service.rb +++ b/app/services/merge_requests/outdated_discussion_diff_lines_service.rb @@ -14,13 +14,23 @@ module MergeRequests end def execute - end_position = position.line_range["end"] - diff_line_index = diff_lines.find_index do |l| - if end_position["new_line"] - l.new_line == end_position["new_line"] - elsif end_position["old_line"] - l.old_line == end_position["old_line"] + line_position = position.line_range["end"] || position.line_range["start"] + found_line = false + diff_line_index = -1 + diff_lines.each_with_index do |l, i| + if found_line + if !l.type + break + elsif l.type == 'new' + diff_line_index = i + break + end + else + # Find the old line + found_line = l.old_line == line_position["new_line"] end + + diff_line_index = i end initial_line_index = [diff_line_index - OVERFLOW_LINES_COUNT, 0].max last_line_index = [diff_line_index + OVERFLOW_LINES_COUNT, diff_lines.length].min diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index ea3071b3c2d..e475b57e4a2 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -28,6 +28,7 @@ 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/rebase_service.rb b/app/services/merge_requests/rebase_service.rb index 9423194c01d..d1f45b4b49c 100644 --- a/app/services/merge_requests/rebase_service.rb +++ b/app/services/merge_requests/rebase_service.rb @@ -4,7 +4,7 @@ module MergeRequests class RebaseService < MergeRequests::BaseService REBASE_ERROR = 'Rebase failed. Please rebase locally' - attr_reader :merge_request + attr_reader :merge_request, :rebase_error def execute(merge_request, skip_ci: false) @merge_request = merge_request @@ -13,7 +13,7 @@ module MergeRequests if rebase success else - error(REBASE_ERROR) + error(rebase_error) end end @@ -22,11 +22,23 @@ module MergeRequests true rescue StandardError => e - log_error(exception: e, message: REBASE_ERROR, save_message_on_model: true) + set_rebase_error(e) + log_error(exception: e, message: rebase_error, save_message_on_model: true) false ensure merge_request.update_column(:rebase_jid, nil) end + + private + + def set_rebase_error(exception) + @rebase_error = + if exception.is_a?(Gitlab::Git::PreReceiveError) + "Something went wrong during the rebase pre-receive hook: #{exception.message}." + else + REBASE_ERROR + end + end end end diff --git a/app/services/merge_requests/remove_attention_requested_service.rb b/app/services/merge_requests/remove_attention_requested_service.rb new file mode 100644 index 00000000000..b727c24415e --- /dev/null +++ b/app/services/merge_requests/remove_attention_requested_service.rb @@ -0,0 +1,41 @@ +# 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 + update_state(reviewer) + update_state(assignee) + + success + else + error("User is not a reviewer or assignee of the merge request") + end + end + + private + + 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: :reviewed) + end + end +end diff --git a/app/services/merge_requests/resolved_discussion_notification_service.rb b/app/services/merge_requests/resolved_discussion_notification_service.rb index 03ded1512f9..6afd760386e 100644 --- a/app/services/merge_requests/resolved_discussion_notification_service.rb +++ b/app/services/merge_requests/resolved_discussion_notification_service.rb @@ -6,6 +6,7 @@ module MergeRequests return unless merge_request.discussions_resolved? SystemNoteService.resolve_all_discussions(merge_request, project, current_user) + execute_hooks(merge_request, 'update') notification_service.async.resolve_all_discussions(merge_request, current_user) end end diff --git a/app/services/merge_requests/squash_service.rb b/app/services/merge_requests/squash_service.rb index 102f78c6a9b..0600fd1d740 100644 --- a/app/services/merge_requests/squash_service.rb +++ b/app/services/merge_requests/squash_service.rb @@ -5,7 +5,7 @@ module MergeRequests def execute # If performing a squash would result in no change, then # immediately return a success message without performing a squash - if merge_request.commits_count < 2 && message.nil? + if merge_request.commits_count == 1 && message == merge_request.first_commit.safe_message return success(squash_sha: merge_request.diff_head_sha) end @@ -17,7 +17,7 @@ module MergeRequests private def squash! - squash_sha = repository.squash(current_user, merge_request, message || merge_request.default_squash_commit_message) + squash_sha = repository.squash(current_user, merge_request, message) success(squash_sha: squash_sha) rescue StandardError => e @@ -39,7 +39,7 @@ module MergeRequests end def message - params[:squash_commit_message].presence + params[:squash_commit_message].presence || merge_request.default_squash_commit_message 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 index 66c5d6fce5d..d9f81ac310f 100644 --- a/app/services/merge_requests/toggle_attention_requested_service.rb +++ b/app/services/merge_requests/toggle_attention_requested_service.rb @@ -19,7 +19,14 @@ module MergeRequests update_state(assignee) if reviewer&.attention_requested? || assignee&.attention_requested? + create_attention_request_note notity_user + + if current_user.id != user.id + remove_attention_requested(merge_request, current_user) + end + else + create_remove_attention_request_note end success @@ -31,9 +38,18 @@ module MergeRequests 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 |