diff options
Diffstat (limited to 'app/services/merge_requests')
-rw-r--r-- | app/services/merge_requests/base_service.rb | 32 | ||||
-rw-r--r-- | app/services/merge_requests/cleanup_refs_service.rb | 75 | ||||
-rw-r--r-- | app/services/merge_requests/close_service.rb | 3 | ||||
-rw-r--r-- | app/services/merge_requests/create_pipeline_service.rb | 12 | ||||
-rw-r--r-- | app/services/merge_requests/merge_service.rb | 5 | ||||
-rw-r--r-- | app/services/merge_requests/post_merge_service.rb | 3 | ||||
-rw-r--r-- | app/services/merge_requests/refresh_service.rb | 130 | ||||
-rw-r--r-- | app/services/merge_requests/update_service.rb | 9 |
8 files changed, 198 insertions, 71 deletions
diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 7e301f311e9..abc3f99797d 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -23,6 +23,8 @@ module MergeRequests merge_data = hook_data(merge_request, action, old_rev: old_rev, old_associations: old_associations) merge_request.project.execute_hooks(merge_data, :merge_request_hooks) merge_request.project.execute_services(merge_data, :merge_request_hooks) + + enqueue_jira_connect_messages_for(merge_request) end def cleanup_environments(merge_request) @@ -52,6 +54,14 @@ module MergeRequests private + def enqueue_jira_connect_messages_for(merge_request) + return unless project.jira_subscription_exists? + + if Atlassian::JiraIssueKeyExtractor.has_keys?(merge_request.title, merge_request.description) + JiraConnect::SyncMergeRequestWorker.perform_async(merge_request.id) + end + end + def create(merge_request) self.params = assign_allowed_merge_params(merge_request, params) @@ -87,6 +97,28 @@ module MergeRequests unless merge_request.can_allow_collaboration?(current_user) params.delete(:allow_collaboration) end + + filter_reviewer(merge_request) + end + + def filter_reviewer(merge_request) + return if params[:reviewer_ids].blank? + + unless can_admin_issuable?(merge_request) && merge_request.allows_reviewers? + params.delete(:reviewer_ids) + + return + end + + reviewer_ids = params[:reviewer_ids].select { |reviewer_id| user_can_read?(merge_request, reviewer_id) } + + if params[:reviewer_ids].map(&:to_s) == [IssuableFinder::Params::NONE] + params[:reviewer_ids] = [] + elsif reviewer_ids.any? + params[:reviewer_ids] = reviewer_ids + else + params.delete(:reviewer_ids) + end end def merge_request_metrics_service(merge_request) diff --git a/app/services/merge_requests/cleanup_refs_service.rb b/app/services/merge_requests/cleanup_refs_service.rb new file mode 100644 index 00000000000..0f03f5f09b4 --- /dev/null +++ b/app/services/merge_requests/cleanup_refs_service.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +module MergeRequests + class CleanupRefsService + include BaseServiceUtility + + TIME_THRESHOLD = 14.days + + attr_reader :merge_request + + def self.schedule(merge_request) + MergeRequestCleanupRefsWorker.perform_in(TIME_THRESHOLD, merge_request.id) + end + + def initialize(merge_request) + @merge_request = merge_request + @repository = merge_request.project.repository + @ref_path = merge_request.ref_path + @merge_ref_path = merge_request.merge_ref_path + @ref_head_sha = @repository.commit(merge_request.ref_path).id + @merge_ref_sha = merge_request.merge_ref_head&.id + end + + def execute + return error("Merge request has not been closed nor merged for #{TIME_THRESHOLD.inspect}.") unless eligible? + + # Ensure that commit shas of refs are kept around so we won't lose them when GC runs. + keep_around + + return error('Failed to create keep around refs.') unless kept_around? + return error('Failed to cache merge ref sha.') unless cache_merge_ref_sha + + delete_refs + success + end + + private + + attr_reader :repository, :ref_path, :merge_ref_path, :ref_head_sha, :merge_ref_sha + + def eligible? + return met_time_threshold?(merge_request.metrics&.latest_closed_at) if merge_request.closed? + + merge_request.merged? && met_time_threshold?(merge_request.metrics&.merged_at) + end + + def met_time_threshold?(attr) + attr.nil? || attr.to_i <= TIME_THRESHOLD.ago.to_i + end + + def kept_around? + service = Gitlab::Git::KeepAround.new(repository) + + [ref_head_sha, merge_ref_sha].compact.all? do |sha| + service.kept_around?(sha) + end + end + + def keep_around + repository.keep_around(ref_head_sha, merge_ref_sha) + end + + def cache_merge_ref_sha + return true if merge_ref_sha.nil? + + # Caching the merge ref sha is needed before we delete the merge ref so + # we can still show the merge ref diff (via `MergeRequest#merge_ref_head`) + merge_request.update_column(:merge_ref_sha, merge_ref_sha) + end + + def delete_refs + repository.delete_refs(ref_path, merge_ref_path) + end + end +end diff --git a/app/services/merge_requests/close_service.rb b/app/services/merge_requests/close_service.rb index c2174d2a130..b0a7face594 100644 --- a/app/services/merge_requests/close_service.rb +++ b/app/services/merge_requests/close_service.rb @@ -2,6 +2,8 @@ module MergeRequests class CloseService < MergeRequests::BaseService + include RemovesRefs + def execute(merge_request, commit = nil) return merge_request unless can?(current_user, :update_merge_request, merge_request) @@ -19,6 +21,7 @@ module MergeRequests merge_request.update_project_counter_caches cleanup_environments(merge_request) abort_auto_merge(merge_request, 'merge request was closed') + cleanup_refs(merge_request) end merge_request diff --git a/app/services/merge_requests/create_pipeline_service.rb b/app/services/merge_requests/create_pipeline_service.rb index f9352f10fea..46c4c102091 100644 --- a/app/services/merge_requests/create_pipeline_service.rb +++ b/app/services/merge_requests/create_pipeline_service.rb @@ -48,12 +48,18 @@ module MergeRequests end def can_create_pipeline_in_target_project?(merge_request) - if Gitlab::Ci::Features.allow_to_create_merge_request_pipelines_in_target_project?(merge_request.target_project) - can?(current_user, :create_pipeline, merge_request.target_project) - else + if Gitlab::Ci::Features.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) && + can_update_source_branch_in_target_project?(merge_request) end end + + def can_update_source_branch_in_target_project?(merge_request) + ::Gitlab::UserAccess.new(current_user, container: merge_request.target_project) + .can_update_branch?(merge_request.source_branch_ref) + end end end diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 961a7cb1ef6..437e87dadf7 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -10,13 +10,14 @@ module MergeRequests class MergeService < MergeRequests::MergeBaseService delegate :merge_jid, :state, to: :@merge_request - def execute(merge_request) + def execute(merge_request, options = {}) if project.merge_requests_ff_only_enabled && !self.is_a?(FfMergeService) FfMergeService.new(project, current_user, params).execute(merge_request) return end @merge_request = merge_request + @options = options validate! @@ -55,7 +56,7 @@ module MergeRequests error = if @merge_request.should_be_rebased? 'Only fast-forward merge is allowed for your project. Please update your source branch' - elsif !@merge_request.mergeable? + elsif !@merge_request.mergeable?(skip_discussions_check: @options[:skip_discussions_check]) 'Merge request is not mergeable' elsif !@merge_request.squash && project.squash_always? 'This project requires squashing commits when merge requests are accepted.' diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index fdf8f442297..1c78fca3c26 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -7,6 +7,8 @@ module MergeRequests # and execute all hooks and notifications # class PostMergeService < MergeRequests::BaseService + include RemovesRefs + def execute(merge_request) merge_request.mark_as_merged close_issues(merge_request) @@ -20,6 +22,7 @@ module MergeRequests delete_non_latest_diffs(merge_request) cancel_review_app_jobs!(merge_request) cleanup_environments(merge_request) + cleanup_refs(merge_request) end private diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 56a91fa0305..405b8fe9c9e 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -2,6 +2,7 @@ module MergeRequests class RefreshService < MergeRequests::BaseService + include Gitlab::Utils::StrongMemoize attr_reader :push def execute(oldrev, newrev, ref) @@ -23,25 +24,37 @@ module MergeRequests post_merge_manually_merged link_forks_lfs_objects reload_merge_requests - outdate_suggestions - refresh_pipelines_on_merge_requests - abort_auto_merges + + merge_requests_for_source_branch.each do |mr| + outdate_suggestions(mr) + refresh_pipelines_on_merge_requests(mr) + abort_auto_merges(mr) + mark_pending_todos_done(mr) + end + abort_ff_merge_requests_with_when_pipeline_succeeds - mark_pending_todos_done cache_merge_requests_closing_issues - # Leave a system note if a branch was deleted/added - if @push.branch_added? || @push.branch_removed? - comment_mr_branch_presence_changed - end + merge_requests_for_source_branch.each do |mr| + # Leave a system note if a branch was deleted/added + if branch_added_or_removed? + comment_mr_branch_presence_changed(mr) + end - notify_about_push - mark_mr_as_wip_from_commits - execute_mr_web_hooks + notify_about_push(mr) + mark_mr_as_wip_from_commits(mr) + execute_mr_web_hooks(mr) + end true end + def branch_added_or_removed? + strong_memoize(:branch_added_or_removed) do + @push.branch_added? || @push.branch_removed? + end + end + def close_upon_missing_source_branch_ref # MergeRequest#reload_diff ignores not opened MRs. This means it won't # create an `empty` diff for `closed` MRs without a source branch, keeping @@ -140,25 +153,22 @@ module MergeRequests merge_request.source_branch == @push.branch_name end - def outdate_suggestions - outdate_service = Suggestions::OutdateService.new + def outdate_suggestions(merge_request) + outdate_service.execute(merge_request) + end - merge_requests_for_source_branch.each do |merge_request| - outdate_service.execute(merge_request) - end + def outdate_service + @outdate_service ||= Suggestions::OutdateService.new end - def refresh_pipelines_on_merge_requests - merge_requests_for_source_branch.each do |merge_request| - create_pipeline_for(merge_request, current_user) - UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) - end + def refresh_pipelines_on_merge_requests(merge_request) + create_pipeline_for(merge_request, current_user) + + UpdateHeadPipelineForMergeRequestWorker.perform_async(merge_request.id) end - def abort_auto_merges - merge_requests_for_source_branch.each do |merge_request| - abort_auto_merge(merge_request, 'source branch was updated') - end + def abort_auto_merges(merge_request) + abort_auto_merge(merge_request, 'source branch was updated') end def abort_ff_merge_requests_with_when_pipeline_succeeds @@ -187,10 +197,8 @@ module MergeRequests .with_auto_merge_enabled end - def mark_pending_todos_done - merge_requests_for_source_branch.each do |merge_request| - todo_service.merge_request_push(merge_request, @current_user) - end + def mark_pending_todos_done(merge_request) + todo_service.merge_request_push(merge_request, @current_user) end def find_new_commits @@ -218,62 +226,54 @@ module MergeRequests end # Add comment about branches being deleted or added to merge requests - def comment_mr_branch_presence_changed + def comment_mr_branch_presence_changed(merge_request) presence = @push.branch_added? ? :add : :delete - merge_requests_for_source_branch.each do |merge_request| - SystemNoteService.change_branch_presence( - merge_request, merge_request.project, @current_user, - :source, @push.branch_name, presence) - end + SystemNoteService.change_branch_presence( + merge_request, merge_request.project, @current_user, + :source, @push.branch_name, presence) end # Add comment about pushing new commits to merge requests and send nofitication emails - def notify_about_push + def notify_about_push(merge_request) return unless @commits.present? - merge_requests_for_source_branch.each do |merge_request| - mr_commit_ids = Set.new(merge_request.commit_shas) + mr_commit_ids = Set.new(merge_request.commit_shas) - new_commits, existing_commits = @commits.partition do |commit| - mr_commit_ids.include?(commit.id) - end + new_commits, existing_commits = @commits.partition do |commit| + mr_commit_ids.include?(commit.id) + end - SystemNoteService.add_commits(merge_request, merge_request.project, - @current_user, new_commits, - existing_commits, @push.oldrev) + SystemNoteService.add_commits(merge_request, merge_request.project, + @current_user, new_commits, + existing_commits, @push.oldrev) - notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits) - end + notification_service.push_to_merge_request(merge_request, @current_user, new_commits: new_commits, existing_commits: existing_commits) end - def mark_mr_as_wip_from_commits + def mark_mr_as_wip_from_commits(merge_request) return unless @commits.present? - merge_requests_for_source_branch.each do |merge_request| - commit_shas = merge_request.commit_shas + commit_shas = merge_request.commit_shas - wip_commit = @commits.detect do |commit| - commit.work_in_progress? && commit_shas.include?(commit.sha) - end + wip_commit = @commits.detect do |commit| + commit.work_in_progress? && commit_shas.include?(commit.sha) + end - if wip_commit && !merge_request.work_in_progress? - merge_request.update(title: merge_request.wip_title) - SystemNoteService.add_merge_request_wip_from_commit( - merge_request, - merge_request.project, - @current_user, - wip_commit - ) - end + if wip_commit && !merge_request.work_in_progress? + merge_request.update(title: merge_request.wip_title) + SystemNoteService.add_merge_request_wip_from_commit( + merge_request, + merge_request.project, + @current_user, + wip_commit + ) end end # Call merge request webhook with update branches - def execute_mr_web_hooks - merge_requests_for_source_branch.each do |merge_request| - execute_hooks(merge_request, 'update', old_rev: @push.oldrev) - end + def execute_mr_web_hooks(merge_request) + execute_hooks(merge_request, 'update', old_rev: @push.oldrev) end # If the merge requests closes any issues, save this information in the diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index cf02158b629..1468bfd6bb6 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -24,8 +24,9 @@ module MergeRequests old_labels = old_associations.fetch(:labels, []) old_mentioned_users = old_associations.fetch(:mentioned_users, []) old_assignees = old_associations.fetch(:assignees, []) + old_reviewers = old_associations.fetch(:reviewers, []) - if has_changes?(merge_request, old_labels: old_labels, old_assignees: old_assignees) + if has_changes?(merge_request, old_labels: old_labels, old_assignees: old_assignees, old_reviewers: old_reviewers) todo_service.resolve_todos_for_target(merge_request, current_user) end @@ -44,6 +45,8 @@ module MergeRequests handle_assignees_change(merge_request, old_assignees) if merge_request.assignees != old_assignees + handle_reviewers_change(merge_request, old_reviewers) if merge_request.reviewers != old_reviewers + if merge_request.previous_changes.include?('target_branch') || merge_request.previous_changes.include?('source_branch') merge_request.mark_as_unchecked @@ -108,6 +111,10 @@ module MergeRequests todo_service.reassigned_assignable(merge_request, current_user, old_assignees) end + def handle_reviewers_change(merge_request, old_reviewers) + todo_service.reassigned_reviewable(merge_request, current_user, old_reviewers) + end + def create_branch_change_note(issuable, branch_type, old_branch, new_branch) SystemNoteService.change_branch( issuable, issuable.project, current_user, branch_type, |