diff options
195 files changed, 592 insertions, 531 deletions
diff --git a/.rubocop_manual_todo.yml b/.rubocop_manual_todo.yml index 90ec92d0cde..e8f149230a9 100644 --- a/.rubocop_manual_todo.yml +++ b/.rubocop_manual_todo.yml @@ -2094,6 +2094,7 @@ Gitlab/NamespacedClass: - 'app/services/auto_merge_service.rb' - 'app/services/base_container_service.rb' - 'app/services/base_count_service.rb' + - 'app/services/base_project_service.rb' - 'app/services/base_renderer.rb' - 'app/services/base_service.rb' - 'app/services/bulk_create_integration_service.rb' diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 37eff37899c..929e60a9e77 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -61,7 +61,7 @@ module IssuableActions end def destroy - Issuable::DestroyService.new(issuable.project, current_user).execute(issuable) + Issuable::DestroyService.new(project: issuable.project, current_user: current_user).execute(issuable) name = issuable.human_class_name flash[:notice] = "The #{name} was successfully deleted." diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index c196147692d..01a6de76ba5 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -113,7 +113,7 @@ class Projects::IssuesController < Projects::ApplicationController discussion_to_resolve: params[:discussion_to_resolve], confidential: !!Gitlab::Utils.to_boolean(issue_params[:confidential]) ) - service = ::Issues::BuildService.new(project, current_user, build_params) + service = ::Issues::BuildService.new(project: project, current_user: current_user, params: build_params) @issue = @noteable = service.execute @@ -133,7 +133,7 @@ class Projects::IssuesController < Projects::ApplicationController discussion_to_resolve: params[:discussion_to_resolve] ) - service = ::Issues::CreateService.new(project, current_user, create_params) + service = ::Issues::CreateService.new(project: project, current_user: current_user, params: create_params) @issue = service.execute create_vulnerability_issue_feedback(issue) @@ -160,7 +160,7 @@ class Projects::IssuesController < Projects::ApplicationController new_project = Project.find(params[:move_to_project_id]) return render_404 unless issue.can_move?(current_user, new_project) - @issue = ::Issues::UpdateService.new(project, current_user, target_project: new_project).execute(issue) + @issue = ::Issues::UpdateService.new(project: project, current_user: current_user, params: { target_project: new_project }).execute(issue) end respond_to do |format| @@ -174,7 +174,7 @@ class Projects::IssuesController < Projects::ApplicationController end def reorder - service = ::Issues::ReorderService.new(project, current_user, reorder_params) + service = ::Issues::ReorderService.new(project: project, current_user: current_user, params: reorder_params) if service.execute(issue) head :ok @@ -185,7 +185,7 @@ class Projects::IssuesController < Projects::ApplicationController def related_branches @related_branches = ::Issues::RelatedBranchesService - .new(project, current_user) + .new(project: project, current_user: current_user) .execute(issue) .map { |branch| branch.merge(link: branch_link(branch)) } @@ -213,7 +213,7 @@ class Projects::IssuesController < Projects::ApplicationController def create_merge_request create_params = params.slice(:branch_name, :ref).merge(issue_iid: issue.iid) create_params[:target_project_id] = params[:target_project_id] - result = ::MergeRequests::CreateFromIssueService.new(project, current_user, create_params).execute + result = ::MergeRequests::CreateFromIssueService.new(project: project, current_user: current_user, mr_params: create_params).execute if result[:status] == :success render json: MergeRequestCreateSerializer.new.represent(result[:merge_request]) @@ -334,7 +334,7 @@ class Projects::IssuesController < Projects::ApplicationController def update_service update_params = issue_params.merge(spammable_params) - ::Issues::UpdateService.new(project, current_user, update_params) + ::Issues::UpdateService.new(project: project, current_user: current_user, params: update_params) end def finder_type diff --git a/app/controllers/projects/merge_requests/creations_controller.rb b/app/controllers/projects/merge_requests/creations_controller.rb index 5e60d5b2b84..9f1e2d8236a 100644 --- a/app/controllers/projects/merge_requests/creations_controller.rb +++ b/app/controllers/projects/merge_requests/creations_controller.rb @@ -19,7 +19,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap end def create - @merge_request = ::MergeRequests::CreateService.new(project, current_user, merge_request_params).execute + @merge_request = ::MergeRequests::CreateService.new(project: project, current_user: current_user, params: merge_request_params).execute if @merge_request.valid? incr_count_webide_merge_request @@ -93,7 +93,7 @@ class Projects::MergeRequests::CreationsController < Projects::MergeRequests::Ap # Gitaly N+1 issue: https://gitlab.com/gitlab-org/gitlab-foss/issues/58096 Gitlab::GitalyClient.allow_n_plus_1_calls do - @merge_request = ::MergeRequests::BuildService.new(project, current_user, merge_request_params.merge(diff_options: diff_options)).execute + @merge_request = ::MergeRequests::BuildService.new(project: project, current_user: current_user, params: merge_request_params.merge(diff_options: diff_options)).execute end end diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 5b7c3cddcda..d1d83f35d5f 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -245,7 +245,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end def update - @merge_request = ::MergeRequests::UpdateService.new(project, current_user, merge_request_update_params).execute(@merge_request) + @merge_request = ::MergeRequests::UpdateService.new(project: project, current_user: current_user, params: merge_request_update_params).execute(@merge_request) respond_to do |format| format.html do @@ -274,7 +274,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo def remove_wip @merge_request = ::MergeRequests::UpdateService - .new(project, current_user, wip_event: 'unwip') + .new(project: project, current_user: current_user, params: { wip_event: 'unwip' }) .execute(@merge_request) render json: serialize_widget(@merge_request) @@ -309,7 +309,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end def assign_related_issues - result = ::MergeRequests::AssignIssuesService.new(project, current_user, merge_request: @merge_request).execute + result = ::MergeRequests::AssignIssuesService.new(project: project, current_user: current_user, params: { merge_request: @merge_request }).execute case result[:count] when 0 @@ -421,7 +421,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo return :failed end - merge_service = ::MergeRequests::MergeService.new(@project, current_user, merge_params) + merge_service = ::MergeRequests::MergeService.new(project: @project, current_user: current_user, params: merge_params) unless merge_service.hooks_validation_pass?(@merge_request) return :hook_validation_error diff --git a/app/graphql/mutations/concerns/mutations/assignable.rb b/app/graphql/mutations/concerns/mutations/assignable.rb index d3ab0a1779a..e214a57500c 100644 --- a/app/graphql/mutations/concerns/mutations/assignable.rb +++ b/app/graphql/mutations/concerns/mutations/assignable.rb @@ -33,9 +33,9 @@ module Mutations def assign!(resource, users, operation_mode) update_service_class.new( - resource.project, - current_user, - assignee_ids: assignee_ids(resource, users, operation_mode) + project: resource.project, + current_user: current_user, + params: { assignee_ids: assignee_ids(resource, users, operation_mode) } ).execute(resource) end diff --git a/app/graphql/mutations/issues/create.rb b/app/graphql/mutations/issues/create.rb index 1bb75a1108b..3a57e2434a5 100644 --- a/app/graphql/mutations/issues/create.rb +++ b/app/graphql/mutations/issues/create.rb @@ -73,7 +73,7 @@ module Mutations project = authorized_find!(project_path) params = build_create_issue_params(attributes.merge(author_id: current_user.id)) - issue = ::Issues::CreateService.new(project, current_user, params).execute + issue = ::Issues::CreateService.new(project: project, current_user: current_user, params: params).execute if issue.spam? issue.errors.add(:base, 'Spam detected.') diff --git a/app/graphql/mutations/issues/move.rb b/app/graphql/mutations/issues/move.rb index 0f2af99bf61..cb4f0f42b38 100644 --- a/app/graphql/mutations/issues/move.rb +++ b/app/graphql/mutations/issues/move.rb @@ -18,7 +18,7 @@ module Mutations target_project = resolve_project(full_path: target_project_path).sync begin - moved_issue = ::Issues::MoveService.new(source_project, current_user).execute(issue, target_project) + moved_issue = ::Issues::MoveService.new(project: source_project, current_user: current_user).execute(issue, target_project) rescue ::Issues::MoveService::MoveError => error errors = error.message end diff --git a/app/graphql/mutations/issues/set_confidential.rb b/app/graphql/mutations/issues/set_confidential.rb index 75befddc261..8e88b31d9ed 100644 --- a/app/graphql/mutations/issues/set_confidential.rb +++ b/app/graphql/mutations/issues/set_confidential.rb @@ -14,7 +14,7 @@ module Mutations issue = authorized_find!(project_path: project_path, iid: iid) project = issue.project - ::Issues::UpdateService.new(project, current_user, confidential: confidential) + ::Issues::UpdateService.new(project: project, current_user: current_user, params: { confidential: confidential }) .execute(issue) { diff --git a/app/graphql/mutations/issues/set_due_date.rb b/app/graphql/mutations/issues/set_due_date.rb index 22a72e201ba..9cefac96b25 100644 --- a/app/graphql/mutations/issues/set_due_date.rb +++ b/app/graphql/mutations/issues/set_due_date.rb @@ -23,7 +23,7 @@ module Mutations issue = authorized_find!(project_path: project_path, iid: iid) project = issue.project - ::Issues::UpdateService.new(project, current_user, due_date: due_date) + ::Issues::UpdateService.new(project: project, current_user: current_user, params: { due_date: due_date }) .execute(issue) { diff --git a/app/graphql/mutations/issues/set_locked.rb b/app/graphql/mutations/issues/set_locked.rb index 611226e48ad..3a696a64dad 100644 --- a/app/graphql/mutations/issues/set_locked.rb +++ b/app/graphql/mutations/issues/set_locked.rb @@ -13,7 +13,7 @@ module Mutations def resolve(project_path:, iid:, locked:) issue = authorized_find!(project_path: project_path, iid: iid) - ::Issues::UpdateService.new(issue.project, current_user, discussion_locked: locked) + ::Issues::UpdateService.new(project: issue.project, current_user: current_user, params: { discussion_locked: locked }) .execute(issue) { diff --git a/app/graphql/mutations/issues/set_severity.rb b/app/graphql/mutations/issues/set_severity.rb index bc386e07178..778563ba053 100644 --- a/app/graphql/mutations/issues/set_severity.rb +++ b/app/graphql/mutations/issues/set_severity.rb @@ -12,7 +12,7 @@ module Mutations issue = authorized_find!(project_path: project_path, iid: iid) project = issue.project - ::Issues::UpdateService.new(project, current_user, severity: severity) + ::Issues::UpdateService.new(project: project, current_user: current_user, params: { severity: severity }) .execute(issue) { diff --git a/app/graphql/mutations/issues/update.rb b/app/graphql/mutations/issues/update.rb index 71b0c685faf..eb16b7b38d0 100644 --- a/app/graphql/mutations/issues/update.rb +++ b/app/graphql/mutations/issues/update.rb @@ -31,7 +31,7 @@ module Mutations issue = authorized_find!(project_path: project_path, iid: iid) project = issue.project - ::Issues::UpdateService.new(project, current_user, args).execute(issue) + ::Issues::UpdateService.new(project: project, current_user: current_user, params: args).execute(issue) { issue: issue, diff --git a/app/graphql/mutations/merge_requests/accept.rb b/app/graphql/mutations/merge_requests/accept.rb index da94dcd8890..9994f793a01 100644 --- a/app/graphql/mutations/merge_requests/accept.rb +++ b/app/graphql/mutations/merge_requests/accept.rb @@ -47,7 +47,7 @@ module Mutations merge_request = authorized_find!(project_path: project_path, iid: iid) project = merge_request.target_project merge_params = args.compact.with_indifferent_access - merge_service = ::MergeRequests::MergeService.new(project, current_user, merge_params) + merge_service = ::MergeRequests::MergeService.new(project: project, current_user: current_user, params: merge_params) if error = validate(merge_request, merge_service, merge_params) return { merge_request: merge_request, errors: [error] } diff --git a/app/graphql/mutations/merge_requests/create.rb b/app/graphql/mutations/merge_requests/create.rb index 9ac8f70be95..4849c198677 100644 --- a/app/graphql/mutations/merge_requests/create.rb +++ b/app/graphql/mutations/merge_requests/create.rb @@ -42,7 +42,7 @@ module Mutations project = authorized_find!(project_path) params = attributes.merge(author_id: current_user.id) - merge_request = ::MergeRequests::CreateService.new(project, current_user, params).execute + merge_request = ::MergeRequests::CreateService.new(project: project, current_user: current_user, params: params).execute { merge_request: merge_request.valid? ? merge_request : nil, diff --git a/app/graphql/mutations/merge_requests/reviewer_rereview.rb b/app/graphql/mutations/merge_requests/reviewer_rereview.rb index f6f4881654e..d1d5118e271 100644 --- a/app/graphql/mutations/merge_requests/reviewer_rereview.rb +++ b/app/graphql/mutations/merge_requests/reviewer_rereview.rb @@ -15,7 +15,7 @@ module Mutations def resolve(project_path:, iid:, user:) merge_request = authorized_find!(project_path: project_path, iid: iid) - result = ::MergeRequests::RequestReviewService.new(merge_request.project, current_user).execute(merge_request, user) + result = ::MergeRequests::RequestReviewService.new(project: merge_request.project, current_user: current_user).execute(merge_request, user) { merge_request: merge_request, diff --git a/app/graphql/mutations/merge_requests/set_draft.rb b/app/graphql/mutations/merge_requests/set_draft.rb index 54775f87ce8..80006c6f70e 100644 --- a/app/graphql/mutations/merge_requests/set_draft.rb +++ b/app/graphql/mutations/merge_requests/set_draft.rb @@ -16,7 +16,7 @@ module Mutations merge_request = authorized_find!(project_path: project_path, iid: iid) project = merge_request.project - ::MergeRequests::UpdateService.new(project, current_user, wip_event: wip_event(draft)) + ::MergeRequests::UpdateService.new(project: project, current_user: current_user, params: { wip_event: wip_event(draft) }) .execute(merge_request) { diff --git a/app/graphql/mutations/merge_requests/set_labels.rb b/app/graphql/mutations/merge_requests/set_labels.rb index 712c68c9425..a77c2731a05 100644 --- a/app/graphql/mutations/merge_requests/set_labels.rb +++ b/app/graphql/mutations/merge_requests/set_labels.rb @@ -9,14 +9,14 @@ module Mutations [::Types::GlobalIDType[Label]], required: true, description: <<~DESC - The Label IDs to set. Replaces existing labels by default. + The Label IDs to set. Replaces existing labels by default. DESC argument :operation_mode, Types::MutationOperationModeEnum, required: false, description: <<~DESC - Changes the operation mode. Defaults to REPLACE. + Changes the operation mode. Defaults to REPLACE. DESC def resolve(project_path:, iid:, label_ids:, operation_mode: Types::MutationOperationModeEnum.enum[:replace]) @@ -38,7 +38,7 @@ module Mutations :label_ids end - ::MergeRequests::UpdateService.new(project, current_user, attribute_name => label_ids) + ::MergeRequests::UpdateService.new(project: project, current_user: current_user, params: { attribute_name => label_ids }) .execute(merge_request) { diff --git a/app/graphql/mutations/merge_requests/set_locked.rb b/app/graphql/mutations/merge_requests/set_locked.rb index c49d5186a03..e9e607551a6 100644 --- a/app/graphql/mutations/merge_requests/set_locked.rb +++ b/app/graphql/mutations/merge_requests/set_locked.rb @@ -9,14 +9,14 @@ module Mutations GraphQL::BOOLEAN_TYPE, required: true, description: <<~DESC - Whether or not to lock the merge request. + Whether or not to lock the merge request. DESC def resolve(project_path:, iid:, locked:) merge_request = authorized_find!(project_path: project_path, iid: iid) project = merge_request.project - ::MergeRequests::UpdateService.new(project, current_user, discussion_locked: locked) + ::MergeRequests::UpdateService.new(project: project, current_user: current_user, params: { discussion_locked: locked }) .execute(merge_request) { diff --git a/app/graphql/mutations/merge_requests/set_milestone.rb b/app/graphql/mutations/merge_requests/set_milestone.rb index abcb1bda1f3..ed5139c4af9 100644 --- a/app/graphql/mutations/merge_requests/set_milestone.rb +++ b/app/graphql/mutations/merge_requests/set_milestone.rb @@ -10,14 +10,14 @@ module Mutations required: false, loads: Types::MilestoneType, description: <<~DESC - The milestone to assign to the merge request. + The milestone to assign to the merge request. DESC def resolve(project_path:, iid:, milestone: nil) merge_request = authorized_find!(project_path: project_path, iid: iid) project = merge_request.project - ::MergeRequests::UpdateService.new(project, current_user, milestone: milestone) + ::MergeRequests::UpdateService.new(project: project, current_user: current_user, params: { milestone: milestone }) .execute(merge_request) { diff --git a/app/graphql/mutations/merge_requests/set_wip.rb b/app/graphql/mutations/merge_requests/set_wip.rb index beb042ce93f..6f52b240840 100644 --- a/app/graphql/mutations/merge_requests/set_wip.rb +++ b/app/graphql/mutations/merge_requests/set_wip.rb @@ -16,7 +16,7 @@ module Mutations merge_request = authorized_find!(project_path: project_path, iid: iid) project = merge_request.project - ::MergeRequests::UpdateService.new(project, current_user, wip_event: wip_event(merge_request, wip)) + ::MergeRequests::UpdateService.new(project: project, current_user: current_user, params: { wip_event: wip_event(merge_request, wip) }) .execute(merge_request) { diff --git a/app/graphql/mutations/merge_requests/update.rb b/app/graphql/mutations/merge_requests/update.rb index 6a94d2f37b2..246e468c34c 100644 --- a/app/graphql/mutations/merge_requests/update.rb +++ b/app/graphql/mutations/merge_requests/update.rb @@ -29,7 +29,7 @@ module Mutations attributes = args.compact ::MergeRequests::UpdateService - .new(merge_request.project, current_user, attributes) + .new(project: merge_request.project, current_user: current_user, params: attributes) .execute(merge_request) errors = errors_on_object(merge_request) diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 34cab691709..751dc4b762d 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -1741,7 +1741,7 @@ class MergeRequest < ApplicationRecord if project.resolve_outdated_diff_discussions? MergeRequests::ResolvedDiscussionNotificationService - .new(project, current_user) + .new(project: project, current_user: current_user) .execute(self) end end diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index f03ea5d129a..7d0fa9e2f8a 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -151,11 +151,12 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated def assign_to_closing_issues_link # rubocop: disable CodeReuse/ServiceClass - issues = MergeRequests::AssignIssuesService.new(project, - current_user, - merge_request: merge_request, - closes_issues: closing_issues - ).assignable_issues + issues = MergeRequests::AssignIssuesService.new(project: project, + current_user: current_user, + params: { + merge_request: merge_request, + closes_issues: closing_issues + }).assignable_issues path = assign_related_issues_project_merge_request_path(project, merge_request) if issues.present? if issues.count > 1 diff --git a/app/services/base_container_service.rb b/app/services/base_container_service.rb index 6852237dc25..ee15763ce65 100644 --- a/app/services/base_container_service.rb +++ b/app/services/base_container_service.rb @@ -1,6 +1,13 @@ # frozen_string_literal: true -# Base class, scoped by container (project or group) +# Base class, scoped by container (project or group). +# +# New or existing services which only require project as a container +# should subclass BaseProjectService. +# +# If you require a different but specific, non-polymorphic container (such +# as group), consider creating a new subclass such as BaseGroupService, +# and update the related comment at the top of the original BaseService. class BaseContainerService include BaseServiceUtility diff --git a/app/services/base_project_service.rb b/app/services/base_project_service.rb new file mode 100644 index 00000000000..fb466e61673 --- /dev/null +++ b/app/services/base_project_service.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +# Base class, scoped by project +class BaseProjectService < ::BaseContainerService + attr_accessor :project + + def initialize(project:, current_user: nil, params: {}) + super(container: project, current_user: current_user, params: params) + + @project = project + end + + delegate :repository, to: :project +end diff --git a/app/services/base_service.rb b/app/services/base_service.rb index 20dfeb67815..7ab87a1af09 100644 --- a/app/services/base_service.rb +++ b/app/services/base_service.rb @@ -6,9 +6,12 @@ # and existing service will use these one by one. # After all are migrated, we can remove this class. # -# TODO: New services should consider inheriting from -# BaseContainerService, or create new base class: -# https://gitlab.com/gitlab-org/gitlab/-/issues/216672 +# New services should consider inheriting from: +# +# - BaseContainerService for services scoped by container (project or group) +# - BaseProjectService for services scoped to projects +# +# or, create a new base class and update this comment. class BaseService include BaseServiceUtility diff --git a/app/services/boards/issues/create_service.rb b/app/services/boards/issues/create_service.rb index d036556f22a..0639acfb399 100644 --- a/app/services/boards/issues/create_service.rb +++ b/app/services/boards/issues/create_service.rb @@ -30,7 +30,7 @@ module Boards end def create_issue(params) - ::Issues::CreateService.new(project, current_user, params).execute + ::Issues::CreateService.new(project: project, current_user: current_user, params: params).execute end end end diff --git a/app/services/boards/issues/move_service.rb b/app/services/boards/issues/move_service.rb index 05f9ccb6172..959a7fa3ad2 100644 --- a/app/services/boards/issues/move_service.rb +++ b/app/services/boards/issues/move_service.rb @@ -52,7 +52,7 @@ module Boards end def update(issue, issue_modification_params) - ::Issues::UpdateService.new(issue.project, current_user, issue_modification_params).execute(issue) + ::Issues::UpdateService.new(project: issue.project, current_user: current_user, params: issue_modification_params).execute(issue) end def reposition_parent diff --git a/app/services/ci/retry_build_service.rb b/app/services/ci/retry_build_service.rb index d5e64828c1f..55801fea6c7 100644 --- a/app/services/ci/retry_build_service.rb +++ b/app/services/ci/retry_build_service.rb @@ -18,7 +18,7 @@ module Ci AfterRequeueJobService.new(project, current_user).execute(build) ::MergeRequests::AddTodoWhenBuildFailsService - .new(project, current_user) + .new(project: project, current_user: current_user) .close(new_build) end end diff --git a/app/services/ci/retry_pipeline_service.rb b/app/services/ci/retry_pipeline_service.rb index e1b27c6f7fd..5cc6b89bfef 100644 --- a/app/services/ci/retry_pipeline_service.rb +++ b/app/services/ci/retry_pipeline_service.rb @@ -29,7 +29,7 @@ module Ci pipeline.reset_source_bridge!(current_user) ::MergeRequests::AddTodoWhenBuildFailsService - .new(project, current_user) + .new(project: project, current_user: current_user) .close_all(pipeline) Ci::ProcessPipelineService diff --git a/app/services/concerns/alert_management/alert_processing.rb b/app/services/concerns/alert_management/alert_processing.rb index 9690d703408..98d255dec27 100644 --- a/app/services/concerns/alert_management/alert_processing.rb +++ b/app/services/concerns/alert_management/alert_processing.rb @@ -56,7 +56,7 @@ module AlertManagement return if issue.blank? || issue.closed? ::Issues::CloseService - .new(project, User.alert_bot) + .new(project: project, current_user: User.alert_bot) .execute(issue, system_note: false) SystemNoteService.auto_resolve_prometheus_alert(issue, project, User.alert_bot) if issue.reset.closed? diff --git a/app/services/discussions/resolve_service.rb b/app/services/discussions/resolve_service.rb index 91c3cf136a4..3b733023eae 100644 --- a/app/services/discussions/resolve_service.rb +++ b/app/services/discussions/resolve_service.rb @@ -44,7 +44,7 @@ module Discussions Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter .track_resolve_thread_action(user: current_user) - MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) + MergeRequests::ResolvedDiscussionNotificationService.new(project: project, current_user: current_user).execute(merge_request) end SystemNoteService.discussion_continued_in_issue(discussion, project, current_user, follow_up_issue) if follow_up_issue diff --git a/app/services/draft_notes/publish_service.rb b/app/services/draft_notes/publish_service.rb index 82917241347..d73c3417a8b 100644 --- a/app/services/draft_notes/publish_service.rb +++ b/app/services/draft_notes/publish_service.rb @@ -24,7 +24,7 @@ module DraftNotes create_note_from_draft(draft) draft.delete - MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) + MergeRequests::ResolvedDiscussionNotificationService.new(project: project, current_user: current_user).execute(merge_request) end def publish_draft_notes @@ -41,7 +41,7 @@ module DraftNotes set_reviewed notification_service.async.new_review(review) - MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(merge_request) + MergeRequests::ResolvedDiscussionNotificationService.new(project: project, current_user: current_user).execute(merge_request) end def create_note_from_draft(draft) @@ -68,7 +68,7 @@ module DraftNotes end def set_reviewed - ::MergeRequests::MarkReviewerReviewedService.new(project, current_user).execute(merge_request) + ::MergeRequests::MarkReviewerReviewedService.new(project: project, current_user: current_user).execute(merge_request) end end end diff --git a/app/services/error_tracking/issue_update_service.rb b/app/services/error_tracking/issue_update_service.rb index b8235678d1d..2f8bbfddef0 100644 --- a/app/services/error_tracking/issue_update_service.rb +++ b/app/services/error_tracking/issue_update_service.rb @@ -35,7 +35,7 @@ module ErrorTracking def close_issue(issue) Issues::CloseService - .new(project, current_user) + .new(project: project, current_user: current_user) .execute(issue, system_note: false) end diff --git a/app/services/git/process_ref_changes_service.rb b/app/services/git/process_ref_changes_service.rb index d039ed00efc..6f348ff9e0b 100644 --- a/app/services/git/process_ref_changes_service.rb +++ b/app/services/git/process_ref_changes_service.rb @@ -77,7 +77,7 @@ module Git def merge_request_branches_for(ref_type, changes) return [] if ref_type == :tag - MergeRequests::PushedBranchesService.new(project, current_user, changes: changes).execute + MergeRequests::PushedBranchesService.new(project: project, current_user: current_user, params: { changes: changes }).execute end end end diff --git a/app/services/incident_management/incidents/create_service.rb b/app/services/incident_management/incidents/create_service.rb index cff288d602b..7497ee00d74 100644 --- a/app/services/incident_management/incidents/create_service.rb +++ b/app/services/incident_management/incidents/create_service.rb @@ -15,11 +15,13 @@ module IncidentManagement def execute issue = Issues::CreateService.new( - project, - current_user, - title: title, - description: description, - issue_type: ISSUE_TYPE + project: project, + current_user: current_user, + params: { + title: title, + description: description, + issue_type: ISSUE_TYPE + } ).execute return error(issue.errors.full_messages.to_sentence, issue) unless issue.valid? diff --git a/app/services/issuable/bulk_update_service.rb b/app/services/issuable/bulk_update_service.rb index 824415215ec..cd32cd78728 100644 --- a/app/services/issuable/bulk_update_service.rb +++ b/app/services/issuable/bulk_update_service.rb @@ -57,7 +57,7 @@ module Issuable items.each do |issuable| next unless can?(current_user, :"update_#{type}", issuable) - update_class.new(issuable.issuing_parent, current_user, params).execute(issuable) + update_class.new(**update_class.constructor_container_arg(issuable.issuing_parent), current_user: current_user, params: params).execute(issuable) end items diff --git a/app/services/issuable/clone/base_service.rb b/app/services/issuable/clone/base_service.rb index 3d73a142f31..f8a9eb3ece5 100644 --- a/app/services/issuable/clone/base_service.rb +++ b/app/services/issuable/clone/base_service.rb @@ -65,7 +65,7 @@ module Issuable end def close_issue - close_service = Issues::CloseService.new(old_project, current_user) + close_service = Issues::CloseService.new(project: old_project, current_user: current_user) close_service.execute(original_entity, notifications: false, system_note: false) end diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb index f69dde4caa8..aedd0c377c6 100644 --- a/app/services/issuable/common_system_notes_service.rb +++ b/app/services/issuable/common_system_notes_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Issuable - class CommonSystemNotesService < ::BaseService + class CommonSystemNotesService < ::BaseProjectService attr_reader :issuable def execute(issuable, old_labels: [], old_milestone: nil, is_update: true) diff --git a/app/services/issuable/import_csv/base_service.rb b/app/services/issuable/import_csv/base_service.rb index 5a2665285de..27dbc8b3cc4 100644 --- a/app/services/issuable/import_csv/base_service.rb +++ b/app/services/issuable/import_csv/base_service.rb @@ -68,7 +68,7 @@ module Issuable end def create_issuable(attributes) - create_issuable_class.new(@project, @user, attributes).execute + create_issuable_class.new(project: @project, current_user: @user, params: attributes).execute end def email_results_to_user diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 02d005fb569..099e0d81bc9 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -1,11 +1,21 @@ # frozen_string_literal: true -class IssuableBaseService < BaseService +class IssuableBaseService < ::BaseProjectService private + def self.constructor_container_arg(value) + # TODO: Dynamically determining the type of a constructor arg based on the class is an antipattern, + # but the root cause is that Epics::BaseService has some issues that inheritance may not be the + # appropriate pattern. See more details in comments at the top of Epics::BaseService#initialize. + # Follow on issue to address this: + # https://gitlab.com/gitlab-org/gitlab/-/issues/328438 + + { project: value } + end + attr_accessor :params, :skip_milestone_email - def initialize(project, user = nil, params = {}) + def initialize(project:, current_user: nil, params: {}) super @skip_milestone_email = @params.delete(:skip_milestone_email) @@ -343,9 +353,13 @@ class IssuableBaseService < BaseService def change_state(issuable) case params.delete(:state_event) when 'reopen' - reopen_service.new(project, current_user, {}).execute(issuable) + service_class = reopen_service when 'close' - close_service.new(project, current_user, {}).execute(issuable) + service_class = close_service + end + + if service_class + service_class.new(**service_class.constructor_container_arg(project), current_user: current_user).execute(issuable) end end @@ -406,7 +420,7 @@ class IssuableBaseService < BaseService end def create_system_notes(issuable, **options) - Issuable::CommonSystemNotesService.new(project, current_user).execute(issuable, **options) + Issuable::CommonSystemNotesService.new(project: project, current_user: current_user).execute(issuable, **options) end def associations_before_update(issuable) diff --git a/app/services/issues/clone_service.rb b/app/services/issues/clone_service.rb index fc1cde5fabc..6df32f1104c 100644 --- a/app/services/issues/clone_service.rb +++ b/app/services/issues/clone_service.rb @@ -57,7 +57,7 @@ module Issues # Skip creation of system notes for existing attributes of the issue. The system notes of the old # issue are copied over so we don't want to end up with duplicate notes. - CreateService.new(target_project, current_user, new_params).execute(skip_system_notes: true) + CreateService.new(project: target_project, current_user: current_user, params: new_params).execute(skip_system_notes: true) end def queue_copy_designs diff --git a/app/services/issues/create_service.rb b/app/services/issues/create_service.rb index 951dad486cb..1f4efeb1a8a 100644 --- a/app/services/issues/create_service.rb +++ b/app/services/issues/create_service.rb @@ -8,7 +8,7 @@ module Issues @request = params.delete(:request) @spam_params = Spam::SpamActionService.filter_spam_params!(params, @request) - @issue = BuildService.new(project, current_user, params).execute + @issue = BuildService.new(project: project, current_user: current_user, params: params).execute filter_resolve_discussion_params diff --git a/app/services/issues/duplicate_service.rb b/app/services/issues/duplicate_service.rb index feb496542c8..d150f0e5917 100644 --- a/app/services/issues/duplicate_service.rb +++ b/app/services/issues/duplicate_service.rb @@ -10,7 +10,7 @@ module Issues create_issue_duplicate_note(duplicate_issue, canonical_issue) create_issue_canonical_note(canonical_issue, duplicate_issue) - close_service.new(project, current_user, {}).execute(duplicate_issue) + close_service.new(project: project, current_user: current_user).execute(duplicate_issue) duplicate_issue.update(duplicated_to: canonical_issue) relate_two_issues(duplicate_issue, canonical_issue) diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb index ace1434356b..e49123a2993 100644 --- a/app/services/issues/move_service.rb +++ b/app/services/issues/move_service.rb @@ -61,7 +61,7 @@ module Issues # Skip creation of system notes for existing attributes of the issue. The system notes of the old # issue are copied over so we don't want to end up with duplicate notes. - CreateService.new(@target_project, @current_user, new_params).execute(skip_system_notes: true) + CreateService.new(project: @target_project, current_user: @current_user, params: new_params).execute(skip_system_notes: true) end def queue_copy_designs diff --git a/app/services/issues/related_branches_service.rb b/app/services/issues/related_branches_service.rb index 98d8412102f..8b08c1f8ddb 100644 --- a/app/services/issues/related_branches_service.rb +++ b/app/services/issues/related_branches_service.rb @@ -30,7 +30,7 @@ module Issues def branches_with_merge_request_for(issue) Issues::ReferencedMergeRequestsService - .new(project, current_user) + .new(project: project, current_user: current_user) .referenced_merge_requests(issue) .map(&:source_branch) end diff --git a/app/services/issues/reorder_service.rb b/app/services/issues/reorder_service.rb index c82ad6ea501..9c5fbec7d8e 100644 --- a/app/services/issues/reorder_service.rb +++ b/app/services/issues/reorder_service.rb @@ -21,7 +21,7 @@ module Issues end def update(issue, attrs) - ::Issues::UpdateService.new(project, current_user, attrs).execute(issue) + ::Issues::UpdateService.new(project: project, current_user: current_user, params: attrs).execute(issue) rescue ActiveRecord::RecordNotFound false end diff --git a/app/services/issues/update_service.rb b/app/services/issues/update_service.rb index bd3f80773ac..d9371e331c1 100644 --- a/app/services/issues/update_service.rb +++ b/app/services/issues/update_service.rb @@ -122,7 +122,7 @@ module Issues canonical_issue = IssuesFinder.new(current_user).find_by(id: canonical_issue_id) if canonical_issue - Issues::DuplicateService.new(project, current_user).execute(issue, canonical_issue) + Issues::DuplicateService.new(project: project, current_user: current_user).execute(issue, canonical_issue) end end # rubocop: enable CodeReuse/ActiveRecord @@ -135,7 +135,7 @@ module Issues target_project != issue.project update(issue) - Issues::MoveService.new(project, current_user).execute(issue, target_project) + Issues::MoveService.new(project: project, current_user: current_user).execute(issue, target_project) end private @@ -151,14 +151,14 @@ module Issues # we've pre-empted this from running in #execute, so let's go ahead and update the Issue now. update(issue) - Issues::CloneService.new(project, current_user).execute(issue, target_project, with_notes: with_notes) + Issues::CloneService.new(project: project, current_user: current_user).execute(issue, target_project, with_notes: with_notes) end def create_merge_request_from_quick_action create_merge_request_params = params.delete(:create_merge_request) return unless create_merge_request_params - MergeRequests::CreateFromIssueService.new(project, current_user, create_merge_request_params).execute + MergeRequests::CreateFromIssueService.new(project: project, current_user: current_user, mr_params: create_merge_request_params).execute end def handle_milestone_change(issue) diff --git a/app/services/issues/zoom_link_service.rb b/app/services/issues/zoom_link_service.rb index 1384e2f83b2..ef48134dec4 100644 --- a/app/services/issues/zoom_link_service.rb +++ b/app/services/issues/zoom_link_service.rb @@ -2,10 +2,10 @@ module Issues class ZoomLinkService < Issues::BaseService - def initialize(issue, user) - super(issue.project, user) + def initialize(project:, current_user:, params:) + super - @issue = issue + @issue = params.fetch(:issue) @added_meeting = ZoomMeeting.canonical_meeting(@issue) end diff --git a/app/services/merge_requests/after_create_service.rb b/app/services/merge_requests/after_create_service.rb index cce0d06b458..77564521d45 100644 --- a/app/services/merge_requests/after_create_service.rb +++ b/app/services/merge_requests/after_create_service.rb @@ -35,7 +35,7 @@ module MergeRequests end def link_lfs_objects(merge_request) - LinkLfsObjectsService.new(merge_request.target_project).execute(merge_request) + LinkLfsObjectsService.new(project: merge_request.target_project).execute(merge_request) end end end diff --git a/app/services/merge_requests/assign_issues_service.rb b/app/services/merge_requests/assign_issues_service.rb index e9107b9998e..f016c16e816 100644 --- a/app/services/merge_requests/assign_issues_service.rb +++ b/app/services/merge_requests/assign_issues_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module MergeRequests - class AssignIssuesService < BaseService + class AssignIssuesService < BaseProjectService def assignable_issues @assignable_issues ||= begin if current_user == merge_request.author @@ -16,7 +16,7 @@ module MergeRequests def execute assignable_issues.each do |issue| - Issues::UpdateService.new(issue.project, current_user, assignee_ids: [current_user.id]).execute(issue) + Issues::UpdateService.new(project: issue.project, current_user: current_user, params: { assignee_ids: [current_user.id] }).execute(issue) end { diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index 1072049738b..e94274aff9d 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -147,7 +147,7 @@ module MergeRequests if async MergeRequests::CreatePipelineWorker.perform_async(project.id, user.id, merge_request.id) else - MergeRequests::CreatePipelineService.new(project, user).execute(merge_request) + MergeRequests::CreatePipelineService.new(project: project, current_user: user).execute(merge_request) end end diff --git a/app/services/merge_requests/create_from_issue_service.rb b/app/services/merge_requests/create_from_issue_service.rb index e03043ef6a2..12fc828b194 100644 --- a/app/services/merge_requests/create_from_issue_service.rb +++ b/app/services/merge_requests/create_from_issue_service.rb @@ -2,16 +2,28 @@ module MergeRequests class CreateFromIssueService < MergeRequests::CreateService - def initialize(project, user, params) + # TODO: This constructor does not use the "params:" argument from the superclass, + # but instead has a custom "mr_params:" argument. This is because historically, + # prior to named arguments being introduced to the constructor, it never passed + # along the third positional argument when calling `super`. + # This should be changed, in order to be consistent (all subclasses should pass + # along all of the arguments to the superclass, otherwise it is probably not an + # "is a" relationship). However, we need to be sure that passing the params + # argument to `super` (especially target_project_id) will not cause any unexpected + # behavior in the superclass. Since the addition of the named arguments is + # intended to be a low-risk pure refactor, we will defer this fix + # to this follow-on issue: + # https://gitlab.com/gitlab-org/gitlab/-/issues/328726 + def initialize(project:, current_user:, mr_params: {}) # branch - the name of new branch # ref - the source of new branch. - @branch_name = params[:branch_name] - @issue_iid = params[:issue_iid] - @ref = params[:ref] - @target_project_id = params[:target_project_id] + @branch_name = mr_params[:branch_name] + @issue_iid = mr_params[:issue_iid] + @ref = mr_params[:ref] + @target_project_id = mr_params[:target_project_id] - super(project, user) + super(project: project, current_user: current_user) end def execute @@ -77,7 +89,7 @@ module MergeRequests end def merge_request - MergeRequests::BuildService.new(target_project, current_user, merge_request_params).execute + MergeRequests::BuildService.new(project: target_project, current_user: current_user, params: merge_request_params).execute end def merge_request_params diff --git a/app/services/merge_requests/get_urls_service.rb b/app/services/merge_requests/get_urls_service.rb index de3f2acdf63..7996fcb5273 100644 --- a/app/services/merge_requests/get_urls_service.rb +++ b/app/services/merge_requests/get_urls_service.rb @@ -1,13 +1,7 @@ # frozen_string_literal: true module MergeRequests - class GetUrlsService < BaseService - attr_reader :project - - def initialize(project) - @project = project - end - + class GetUrlsService < BaseProjectService def execute(changes) return [] unless project&.printing_merge_request_link_enabled diff --git a/app/services/merge_requests/link_lfs_objects_service.rb b/app/services/merge_requests/link_lfs_objects_service.rb index 191da594095..4981d3efcae 100644 --- a/app/services/merge_requests/link_lfs_objects_service.rb +++ b/app/services/merge_requests/link_lfs_objects_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module MergeRequests - class LinkLfsObjectsService < ::BaseService + class LinkLfsObjectsService < ::BaseProjectService def execute(merge_request, oldrev: merge_request.diff_base_sha, newrev: merge_request.diff_head_sha) return if merge_request.source_project == project return if no_changes?(oldrev, newrev) diff --git a/app/services/merge_requests/merge_base_service.rb b/app/services/merge_requests/merge_base_service.rb index 3d1b7616e76..3b9d3bccacf 100644 --- a/app/services/merge_requests/merge_base_service.rb +++ b/app/services/merge_requests/merge_base_service.rb @@ -61,7 +61,7 @@ module MergeRequests def squash_sha! params[:merge_request] = merge_request - squash_result = ::MergeRequests::SquashService.new(project, current_user, params).execute + squash_result = ::MergeRequests::SquashService.new(project: project, current_user: current_user, params: params).execute case squash_result[:status] when :success diff --git a/app/services/merge_requests/merge_service.rb b/app/services/merge_requests/merge_service.rb index 111975b76bc..5e7eee4f1c3 100644 --- a/app/services/merge_requests/merge_service.rb +++ b/app/services/merge_requests/merge_service.rb @@ -17,7 +17,7 @@ module MergeRequests 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) + FfMergeService.new(project: project, current_user: current_user, params: params).execute(merge_request) return end @@ -111,7 +111,7 @@ module MergeRequests def after_merge log_info("Post merge started on JID #{merge_jid} with state #{state}") - MergeRequests::PostMergeService.new(project, current_user).execute(merge_request) + MergeRequests::PostMergeService.new(project: project, current_user: current_user).execute(merge_request) log_info("Post merge finished on JID #{merge_jid} with state #{state}") if delete_source_branch? diff --git a/app/services/merge_requests/mergeability_check_service.rb b/app/services/merge_requests/mergeability_check_service.rb index 9fecab85cc1..3e294aeaa07 100644 --- a/app/services/merge_requests/mergeability_check_service.rb +++ b/app/services/merge_requests/mergeability_check_service.rb @@ -157,7 +157,7 @@ module MergeRequests def merge_to_ref params = { allow_conflicts: Feature.enabled?(:display_merge_conflicts_in_diff, project) } - result = MergeRequests::MergeToRefService.new(project, merge_request.author, params).execute(merge_request) + result = MergeRequests::MergeToRefService.new(project: project, current_user: merge_request.author, params: params).execute(merge_request) result[:status] == :success end diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index 1c9b802780e..ea3071b3c2d 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -44,7 +44,7 @@ module MergeRequests closed_issues = merge_request.visible_closing_issues_for(current_user) closed_issues.each do |issue| - Issues::CloseService.new(project, current_user).execute(issue, commit: merge_request) + Issues::CloseService.new(project: project, current_user: current_user).execute(issue, commit: merge_request) end end diff --git a/app/services/merge_requests/push_options_handler_service.rb b/app/services/merge_requests/push_options_handler_service.rb index 05ec87c7d60..cc1e08e1606 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -1,16 +1,16 @@ # frozen_string_literal: true module MergeRequests - class PushOptionsHandlerService + class PushOptionsHandlerService < ::BaseProjectService LIMIT = 10 - attr_reader :current_user, :errors, :changes, - :project, :push_options, :target_project + attr_reader :errors, :changes, + :push_options, :target_project + + def initialize(project:, current_user:, params: {}, changes:, push_options:) + super(project: project, current_user: current_user, params: params) - def initialize(project, current_user, changes, push_options) - @project = project @target_project = @project.default_merge_request_target - @current_user = current_user @changes = Gitlab::ChangesList.new(changes) @push_options = push_options @errors = [] @@ -95,16 +95,16 @@ module MergeRequests # Use BuildService to assign the standard attributes of a merge request merge_request = ::MergeRequests::BuildService.new( - project, - current_user, - create_params(branch) + project: project, + current_user: current_user, + params: create_params(branch) ).execute unless merge_request.errors.present? merge_request = ::MergeRequests::CreateService.new( - project, - current_user, - merge_request.attributes.merge(assignees: merge_request.assignees, + project: project, + current_user: current_user, + params: merge_request.attributes.merge(assignees: merge_request.assignees, label_ids: merge_request.label_ids) ).execute end @@ -114,9 +114,9 @@ module MergeRequests def update!(merge_request) merge_request = ::MergeRequests::UpdateService.new( - target_project, - current_user, - update_params(merge_request) + project: target_project, + current_user: current_user, + params: update_params(merge_request) ).execute(merge_request) collect_errors_from_merge_request(merge_request) unless merge_request.valid? diff --git a/app/services/merge_requests/refresh_service.rb b/app/services/merge_requests/refresh_service.rb index 463f9f3d6d1..d5e2595a9c6 100644 --- a/app/services/merge_requests/refresh_service.rb +++ b/app/services/merge_requests/refresh_service.rb @@ -62,7 +62,7 @@ module MergeRequests # the latest diff state as the last _valid_ one. merge_requests_for_source_branch.reject(&:source_branch_exists?).each do |mr| MergeRequests::CloseService - .new(mr.target_project, @current_user) + .new(project: mr.target_project, current_user: @current_user) .execute(mr) end end @@ -96,7 +96,7 @@ module MergeRequests merge_request.merge_commit_sha = analyzer.get_merge_commit(merge_request.diff_head_sha) MergeRequests::PostMergeService - .new(merge_request.target_project, @current_user) + .new(project: merge_request.target_project, current_user: @current_user) .execute(merge_request) end end @@ -109,7 +109,7 @@ module MergeRequests merge_requests_for_forks.find_each do |mr| LinkLfsObjectsService - .new(mr.target_project) + .new(project: mr.target_project) .execute(mr, oldrev: @push.oldrev, newrev: @push.newrev) end end diff --git a/app/services/merge_requests/retarget_chain_service.rb b/app/services/merge_requests/retarget_chain_service.rb index e8101e447d2..dab6e198979 100644 --- a/app/services/merge_requests/retarget_chain_service.rb +++ b/app/services/merge_requests/retarget_chain_service.rb @@ -24,9 +24,11 @@ module MergeRequests next unless can?(current_user, :update_merge_request, other_merge_request.source_project) ::MergeRequests::UpdateService - .new(other_merge_request.source_project, current_user, - target_branch: merge_request.target_branch, - target_branch_was_deleted: true) + .new(project: other_merge_request.source_project, current_user: current_user, + params: { + target_branch: merge_request.target_branch, + target_branch_was_deleted: true + }) .execute(other_merge_request) end end diff --git a/app/services/merge_requests/update_assignees_service.rb b/app/services/merge_requests/update_assignees_service.rb index 91797c31e85..420afac7de3 100644 --- a/app/services/merge_requests/update_assignees_service.rb +++ b/app/services/merge_requests/update_assignees_service.rb @@ -20,7 +20,7 @@ module MergeRequests # Defer the more expensive operations (handle_assignee_changes) to the background MergeRequests::HandleAssigneesChangeService - .new(project, current_user) + .new(project: project, current_user: current_user) .async_execute(merge_request, old_assignees, execute_hooks: true) merge_request diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 9522bdc7419..b613d88aee4 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -4,7 +4,7 @@ module MergeRequests class UpdateService < MergeRequests::BaseService extend ::Gitlab::Utils::Override - def initialize(project, user = nil, params = {}) + def initialize(project:, current_user: nil, params: {}) super @target_branch_was_deleted = @params.delete(:target_branch_was_deleted) @@ -222,7 +222,7 @@ module MergeRequests def handle_assignees_change(merge_request, old_assignees) MergeRequests::HandleAssigneesChangeService - .new(project, current_user) + .new(project: project, current_user: current_user) .async_execute(merge_request, old_assignees) end @@ -304,11 +304,11 @@ module MergeRequests def assignees_service @assignees_service ||= ::MergeRequests::UpdateAssigneesService - .new(project, current_user, params) + .new(project: project, current_user: current_user, params: params) end def add_time_spent_service - @add_time_spent_service ||= ::MergeRequests::AddSpentTimeService.new(project, current_user, params) + @add_time_spent_service ||= ::MergeRequests::AddSpentTimeService.new(project: project, current_user: current_user, params: params) end end end diff --git a/app/services/metrics/dashboard/update_dashboard_service.rb b/app/services/metrics/dashboard/update_dashboard_service.rb index d990e96ecb5..0574cb15e96 100644 --- a/app/services/metrics/dashboard/update_dashboard_service.rb +++ b/app/services/metrics/dashboard/update_dashboard_service.rb @@ -58,7 +58,7 @@ module Metrics target_branch: project.default_branch, title: params[:commit_message] } - merge_request = ::MergeRequests::CreateService.new(project, current_user, merge_request_params).execute + merge_request = ::MergeRequests::CreateService.new(project: project, current_user: current_user, params: merge_request_params).execute if merge_request.persisted? success(result.merge(merge_request: Gitlab::UrlBuilder.build(merge_request))) diff --git a/app/services/milestones/destroy_service.rb b/app/services/milestones/destroy_service.rb index 87c7a282081..2563f2f5390 100644 --- a/app/services/milestones/destroy_service.rb +++ b/app/services/milestones/destroy_service.rb @@ -7,11 +7,11 @@ module Milestones update_params = { milestone: nil, skip_milestone_email: true } milestone.issues.each do |issue| - Issues::UpdateService.new(parent, current_user, update_params).execute(issue) + Issues::UpdateService.new(project: parent, current_user: current_user, params: update_params).execute(issue) end milestone.merge_requests.each do |merge_request| - MergeRequests::UpdateService.new(parent, current_user, update_params).execute(merge_request) + MergeRequests::UpdateService.new(project: parent, current_user: current_user, params: update_params).execute(merge_request) end log_destroy_event_for(milestone) diff --git a/app/services/notes/quick_actions_service.rb b/app/services/notes/quick_actions_service.rb index 4bcd2586788..900ace24ab4 100644 --- a/app/services/notes/quick_actions_service.rb +++ b/app/services/notes/quick_actions_service.rb @@ -24,12 +24,12 @@ module Notes UPDATE_SERVICES end - def self.noteable_update_service(note) + def self.noteable_update_service_class(note) update_services[note.noteable_type] end def self.supported?(note) - !!noteable_update_service(note) + !!noteable_update_service_class(note) end def supported?(note) @@ -55,7 +55,21 @@ module Notes update_params[:spend_time][:note_id] = note.id end - self.class.noteable_update_service(note).new(note.resource_parent, current_user, update_params).execute(note.noteable) + noteable_update_service_class = self.class.noteable_update_service_class(note) + + # TODO: This conditional is necessary because we have not fully converted all possible + # noteable_update_service_class classes to use named arguments. See more details + # on the partial conversion at https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59182 + # Follow-on issue to address this is here: + # https://gitlab.com/gitlab-org/gitlab/-/issues/328734 + service = + if noteable_update_service_class.respond_to?(:constructor_container_arg) + noteable_update_service_class.new(**noteable_update_service_class.constructor_container_arg(note.resource_parent), current_user: current_user, params: update_params) + else + noteable_update_service_class.new(note.resource_parent, current_user, update_params) + end + + service.execute(note.noteable) end end end diff --git a/app/services/notes/resolve_service.rb b/app/services/notes/resolve_service.rb index cf24795f050..75ce9e27c5b 100644 --- a/app/services/notes/resolve_service.rb +++ b/app/services/notes/resolve_service.rb @@ -5,7 +5,7 @@ module Notes def execute(note) note.resolve!(current_user) - ::MergeRequests::ResolvedDiscussionNotificationService.new(project, current_user).execute(note.noteable) + ::MergeRequests::ResolvedDiscussionNotificationService.new(project: project, current_user: current_user).execute(note.noteable) end end end diff --git a/app/services/post_receive_service.rb b/app/services/post_receive_service.rb index 8d5cd82bcac..faacabbb16c 100644 --- a/app/services/post_receive_service.rb +++ b/app/services/post_receive_service.rb @@ -56,7 +56,7 @@ class PostReceiveService end service = ::MergeRequests::PushOptionsHandlerService.new( - project, user, changes, push_options + project: project, current_user: user, changes: changes, push_options: push_options ).execute if service.errors.present? @@ -72,7 +72,7 @@ class PostReceiveService def merge_request_urls return [] unless repository&.repo_type&.project? - ::MergeRequests::GetUrlsService.new(project).execute(params[:changes]) + ::MergeRequests::GetUrlsService.new(project: project).execute(params[:changes]) end private diff --git a/app/services/projects/unlink_fork_service.rb b/app/services/projects/unlink_fork_service.rb index 91632e50ba8..9eccc16a8b2 100644 --- a/app/services/projects/unlink_fork_service.rb +++ b/app/services/projects/unlink_fork_service.rb @@ -17,7 +17,7 @@ module Projects .from_and_to_forks(@project) merge_requests.find_each do |mr| - ::MergeRequests::CloseService.new(@project, @current_user).execute(mr) + ::MergeRequests::CloseService.new(project: @project, current_user: @current_user).execute(mr) log_info(message: "UnlinkForkService: Closed merge request", merge_request_id: mr.id) end diff --git a/app/workers/ci/merge_requests/add_todo_when_build_fails_worker.rb b/app/workers/ci/merge_requests/add_todo_when_build_fails_worker.rb index c6815b780b1..bd061b5f988 100644 --- a/app/workers/ci/merge_requests/add_todo_when_build_fails_worker.rb +++ b/app/workers/ci/merge_requests/add_todo_when_build_fails_worker.rb @@ -17,7 +17,7 @@ module Ci return unless job && project - ::MergeRequests::AddTodoWhenBuildFailsService.new(job.project, nil).execute(job) + ::MergeRequests::AddTodoWhenBuildFailsService.new(project: job.project).execute(job) end end end diff --git a/app/workers/issue_placement_worker.rb b/app/workers/issue_placement_worker.rb index d3995f1d89e..54a483a3871 100644 --- a/app/workers/issue_placement_worker.rb +++ b/app/workers/issue_placement_worker.rb @@ -33,7 +33,7 @@ class IssuePlacementWorker leftover = to_place.pop if to_place.count > QUERY_LIMIT Issue.move_nulls_to_end(to_place) - Issues::BaseService.new(nil).rebalance_if_needed(to_place.max_by(&:relative_position)) + Issues::BaseService.new(project: nil).rebalance_if_needed(to_place.max_by(&:relative_position)) IssuePlacementWorker.perform_async(nil, leftover.project_id) if leftover.present? rescue RelativePositioning::NoSpaceLeft => e Gitlab::ErrorTracking.log_exception(e, issue_id: issue_id, project_id: project_id) diff --git a/app/workers/merge_requests/assignees_change_worker.rb b/app/workers/merge_requests/assignees_change_worker.rb index d062336667b..fe39f20151f 100644 --- a/app/workers/merge_requests/assignees_change_worker.rb +++ b/app/workers/merge_requests/assignees_change_worker.rb @@ -21,7 +21,7 @@ class MergeRequests::AssigneesChangeWorker return if users.blank? ::MergeRequests::HandleAssigneesChangeService - .new(merge_request.target_project, current_user) + .new(project: merge_request.target_project, current_user: current_user) .execute(merge_request, users, execute_hooks: true) rescue ActiveRecord::RecordNotFound end diff --git a/app/workers/merge_requests/create_pipeline_worker.rb b/app/workers/merge_requests/create_pipeline_worker.rb index ce7f84f997b..a79a92a5419 100644 --- a/app/workers/merge_requests/create_pipeline_worker.rb +++ b/app/workers/merge_requests/create_pipeline_worker.rb @@ -23,7 +23,7 @@ module MergeRequests merge_request = MergeRequest.find_by_id(merge_request_id) return unless merge_request - MergeRequests::CreatePipelineService.new(project, user).execute(merge_request) + MergeRequests::CreatePipelineService.new(project: project, current_user: user).execute(merge_request) merge_request.update_head_pipeline end end diff --git a/app/workers/merge_requests/delete_source_branch_worker.rb b/app/workers/merge_requests/delete_source_branch_worker.rb index 2fc90082834..1ce3a99b298 100644 --- a/app/workers/merge_requests/delete_source_branch_worker.rb +++ b/app/workers/merge_requests/delete_source_branch_worker.rb @@ -19,7 +19,7 @@ class MergeRequests::DeleteSourceBranchWorker ::Branches::DeleteService.new(merge_request.source_project, user) .execute(merge_request.source_branch) - ::MergeRequests::RetargetChainService.new(merge_request.source_project, user) + ::MergeRequests::RetargetChainService.new(project: merge_request.source_project, current_user: user) .execute(merge_request) rescue ActiveRecord::RecordNotFound end diff --git a/app/workers/merge_requests/handle_assignees_change_worker.rb b/app/workers/merge_requests/handle_assignees_change_worker.rb index 338a0cfeffb..4c0500cd520 100644 --- a/app/workers/merge_requests/handle_assignees_change_worker.rb +++ b/app/workers/merge_requests/handle_assignees_change_worker.rb @@ -17,7 +17,7 @@ class MergeRequests::HandleAssigneesChangeWorker old_assignees = User.id_in(old_assignee_ids) ::MergeRequests::HandleAssigneesChangeService - .new(merge_request.target_project, user) + .new(project: merge_request.target_project, current_user: user) .execute(merge_request, old_assignees, options) rescue ActiveRecord::RecordNotFound end diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index 250fbc1b9b3..df5a7a904fc 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -23,7 +23,7 @@ class MergeWorker # rubocop:disable Scalability/IdempotentWorker return end - MergeRequests::MergeService.new(merge_request.target_project, current_user, params) + MergeRequests::MergeService.new(project: merge_request.target_project, current_user: current_user, params: params) .execute(merge_request) end end diff --git a/app/workers/new_issue_worker.rb b/app/workers/new_issue_worker.rb index bc56c3d0dec..a579b828354 100644 --- a/app/workers/new_issue_worker.rb +++ b/app/workers/new_issue_worker.rb @@ -20,7 +20,7 @@ class NewIssueWorker # rubocop:disable Scalability/IdempotentWorker issuable.create_cross_references!(user) Issues::AfterCreateService - .new(issuable.project, user) + .new(project: issuable.project, current_user: user) .execute(issuable) end diff --git a/app/workers/new_merge_request_worker.rb b/app/workers/new_merge_request_worker.rb index 77d6c020c67..574c73ad3b5 100644 --- a/app/workers/new_merge_request_worker.rb +++ b/app/workers/new_merge_request_worker.rb @@ -15,7 +15,7 @@ class NewMergeRequestWorker # rubocop:disable Scalability/IdempotentWorker return unless objects_found?(merge_request_id, user_id) MergeRequests::AfterCreateService - .new(issuable.target_project, user) + .new(project: issuable.target_project, current_user: user) .execute(issuable) end diff --git a/app/workers/process_commit_worker.rb b/app/workers/process_commit_worker.rb index ff76debee8c..54ffe8d3b10 100644 --- a/app/workers/process_commit_worker.rb +++ b/app/workers/process_commit_worker.rb @@ -53,7 +53,7 @@ class ProcessCommitWorker # therefore we use IssueCollection here and skip the authorization check in # Issues::CloseService#execute. IssueCollection.new(issues).updatable_by_user(user).each do |issue| - Issues::CloseService.new(project, author) + Issues::CloseService.new(project: project, current_user: author) .close_issue(issue, closed_via: commit) end end diff --git a/app/workers/rebase_worker.rb b/app/workers/rebase_worker.rb index f2be7dc4749..664905eb9e5 100644 --- a/app/workers/rebase_worker.rb +++ b/app/workers/rebase_worker.rb @@ -16,7 +16,7 @@ class RebaseWorker # rubocop:disable Scalability/IdempotentWorker merge_request = MergeRequest.find(merge_request_id) MergeRequests::RebaseService - .new(merge_request.source_project, current_user) + .new(project: merge_request.source_project, current_user: current_user) .execute(merge_request, skip_ci: skip_ci) end end diff --git a/app/workers/update_merge_requests_worker.rb b/app/workers/update_merge_requests_worker.rb index b6ec4d8a8ab..6f86a7e7e2f 100644 --- a/app/workers/update_merge_requests_worker.rb +++ b/app/workers/update_merge_requests_worker.rb @@ -19,7 +19,7 @@ class UpdateMergeRequestsWorker # rubocop:disable Scalability/IdempotentWorker user = User.find_by(id: user_id) return unless user - MergeRequests::RefreshService.new(project, user).execute(oldrev, newrev, ref) + MergeRequests::RefreshService.new(project: project, current_user: user).execute(oldrev, newrev, ref) end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/changelogs/unreleased/remove_flag_ci_pipeline_ensure_iid_on_skip.yml b/changelogs/unreleased/remove_flag_ci_pipeline_ensure_iid_on_skip.yml new file mode 100644 index 00000000000..89a55bd7ae3 --- /dev/null +++ b/changelogs/unreleased/remove_flag_ci_pipeline_ensure_iid_on_skip.yml @@ -0,0 +1,5 @@ +--- +title: Ensure iid is set before skipping ci pipeline +merge_request: 61231 +author: +type: performance diff --git a/config/feature_flags/development/ci_pipeline_ensure_iid_on_skip.yml b/config/feature_flags/development/ci_pipeline_ensure_iid_on_skip.yml deleted file mode 100644 index 43fb411edbf..00000000000 --- a/config/feature_flags/development/ci_pipeline_ensure_iid_on_skip.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: ci_pipeline_ensure_iid_on_skip -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/59342 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/327661 -milestone: '13.12' -type: development -group: group::code review -default_enabled: false diff --git a/db/fixtures/development/10_merge_requests.rb b/db/fixtures/development/10_merge_requests.rb index 8cda3eb51be..19bec2cef72 100644 --- a/db/fixtures/development/10_merge_requests.rb +++ b/db/fixtures/development/10_merge_requests.rb @@ -36,7 +36,7 @@ Gitlab::Seeder.quiet do break unless developer Sidekiq::Worker.skipping_transaction_check do - MergeRequests::CreateService.new(project, developer, params).execute + MergeRequests::CreateService.new(project: project, current_user: developer, params: params).execute rescue Repository::AmbiguousRefError # Ignore pipelines creation errors for now, we can doing that after # https://gitlab.com/gitlab-org/gitlab-foss/issues/55966. will be resolved. @@ -55,7 +55,7 @@ Gitlab::Seeder.quiet do title: 'Can be automatically merged' } Sidekiq::Worker.skipping_transaction_check do - MergeRequests::CreateService.new(project, User.admins.first, params).execute + MergeRequests::CreateService.new(project: project, current_user: User.admins.first, params: params).execute end print '.' @@ -65,7 +65,7 @@ Gitlab::Seeder.quiet do title: 'Cannot be automatically merged' } Sidekiq::Worker.skipping_transaction_check do - MergeRequests::CreateService.new(project, User.admins.first, params).execute + MergeRequests::CreateService.new(project: project, current_user: User.admins.first, params: params).execute end print '.' end diff --git a/lib/api/issues.rb b/lib/api/issues.rb index a78e1f8cc27..355b5ed3a1f 100644 --- a/lib/api/issues.rb +++ b/lib/api/issues.rb @@ -255,9 +255,9 @@ module API issue_params = convert_parameters_from_legacy_format(issue_params) begin - issue = ::Issues::CreateService.new(user_project, - current_user, - issue_params.merge(request: request, api: true)).execute + issue = ::Issues::CreateService.new(project: user_project, + current_user: current_user, + params: issue_params.merge(request: request, api: true)).execute if issue.spam? render_api_error!({ error: 'Spam detected' }, 400) @@ -298,9 +298,9 @@ module API update_params = convert_parameters_from_legacy_format(update_params) - issue = ::Issues::UpdateService.new(user_project, - current_user, - update_params).execute(issue) + issue = ::Issues::UpdateService.new(project: user_project, + current_user: current_user, + params: update_params).execute(issue) render_spam_error! if issue.spam? @@ -328,7 +328,7 @@ module API authorize! :update_issue, issue - if ::Issues::ReorderService.new(user_project, current_user, params).execute(issue) + if ::Issues::ReorderService.new(project: user_project, current_user: current_user, params: params).execute(issue) present issue, with: Entities::Issue, current_user: current_user, project: user_project else render_api_error!({ error: 'Unprocessable Entity' }, 422) @@ -354,7 +354,7 @@ module API not_found!('Project') unless new_project begin - issue = ::Issues::MoveService.new(user_project, current_user).execute(issue, new_project) + issue = ::Issues::MoveService.new(project: user_project, current_user: current_user).execute(issue, new_project) present issue, with: Entities::Issue, current_user: current_user, project: user_project rescue ::Issues::MoveService::MoveError => error render_api_error!(error.message, 400) @@ -374,7 +374,7 @@ module API authorize!(:destroy_issue, issue) destroy_conditionally!(issue) do |issue| - Issuable::DestroyService.new(user_project, current_user).execute(issue) + Issuable::DestroyService.new(project: user_project, current_user: current_user).execute(issue) end end # rubocop: enable CodeReuse/ActiveRecord @@ -388,7 +388,7 @@ module API get ':id/issues/:issue_iid/related_merge_requests' do issue = find_project_issue(params[:issue_iid]) - merge_requests = ::Issues::ReferencedMergeRequestsService.new(user_project, current_user) + merge_requests = ::Issues::ReferencedMergeRequestsService.new(project: user_project, current_user: current_user) .execute(issue) .first diff --git a/lib/api/merge_request_approvals.rb b/lib/api/merge_request_approvals.rb index c61c787472d..83150bb51ca 100644 --- a/lib/api/merge_request_approvals.rb +++ b/lib/api/merge_request_approvals.rb @@ -54,7 +54,7 @@ module API success = ::MergeRequests::ApprovalService - .new(user_project, current_user, params) + .new(project: user_project, current_user: current_user, params: params) .execute(merge_request) unauthorized! unless success @@ -67,7 +67,7 @@ module API merge_request = find_merge_request_with_access(params[:merge_request_iid], :approve_merge_request) success = ::MergeRequests::RemoveApprovalService - .new(user_project, current_user) + .new(project: user_project, current_user: current_user) .execute(merge_request) not_found! unless success diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 1bd62b5da3a..3dc1341d6cf 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -224,7 +224,7 @@ module API mr_params[:force_remove_source_branch] = mr_params.delete(:remove_source_branch) mr_params = convert_parameters_from_legacy_format(mr_params) - merge_request = ::MergeRequests::CreateService.new(user_project, current_user, mr_params).execute + merge_request = ::MergeRequests::CreateService.new(project: user_project, current_user: current_user, params: mr_params).execute handle_merge_request_errors!(merge_request) @@ -243,7 +243,7 @@ module API authorize!(:destroy_merge_request, merge_request) destroy_conditionally!(merge_request) do |merge_request| - Issuable::DestroyService.new(user_project, current_user).execute(merge_request) + Issuable::DestroyService.new(project: user_project, current_user: current_user).execute(merge_request) end end @@ -335,7 +335,7 @@ module API authorize!(:update_merge_request, merge_request) project = merge_request.target_project - result = ::MergeRequests::AddContextService.new(project, current_user, merge_request: merge_request, commits: commit_ids).execute + result = ::MergeRequests::AddContextService.new(project: project, current_user: current_user, params: { merge_request: merge_request, commits: commit_ids }).execute if result.instance_of?(Array) present result, with: Entities::Commit @@ -398,7 +398,7 @@ module API end post ':id/merge_requests/:merge_request_iid/pipelines', feature_category: :continuous_integration do pipeline = ::MergeRequests::CreatePipelineService - .new(user_project, current_user, allow_duplicate: true) + .new(project: user_project, current_user: current_user, params: { allow_duplicate: true }) .execute(find_merge_request_with_access(params[:merge_request_iid])) if pipeline.nil? @@ -439,7 +439,7 @@ module API ::MergeRequests::UpdateService end - merge_request = service.new(user_project, current_user, mr_params).execute(merge_request) + merge_request = service.new(project: user_project, current_user: current_user, params: mr_params).execute(merge_request) handle_merge_request_errors!(merge_request) @@ -489,7 +489,7 @@ module API if immediately_mergeable ::MergeRequests::MergeService - .new(merge_request.target_project, current_user, merge_params) + .new(project: merge_request.target_project, current_user: current_user, params: merge_params) .execute(merge_request) elsif automatically_mergeable AutoMergeService.new(merge_request.target_project, current_user, merge_params) diff --git a/lib/api/time_tracking_endpoints.rb b/lib/api/time_tracking_endpoints.rb index b1d9037f9d0..969122d7906 100644 --- a/lib/api/time_tracking_endpoints.rb +++ b/lib/api/time_tracking_endpoints.rb @@ -37,7 +37,7 @@ module API custom_params = declared_params(include_missing: false) custom_params.merge!(attrs) - issuable = update_service.new(user_project, current_user, custom_params).execute(load_issuable) + issuable = update_service.new(project: user_project, current_user: current_user, params: custom_params).execute(load_issuable) if issuable.valid? present issuable, with: Entities::IssuableTimeStats else diff --git a/lib/gitlab/ci/pipeline/chain/skip.rb b/lib/gitlab/ci/pipeline/chain/skip.rb index bea0c0ea115..e4e4f4f484a 100644 --- a/lib/gitlab/ci/pipeline/chain/skip.rb +++ b/lib/gitlab/ci/pipeline/chain/skip.rb @@ -12,12 +12,10 @@ module Gitlab def perform! if skipped? if @command.save_incompleted - if Feature.enabled?(:ci_pipeline_ensure_iid_on_skip, @pipeline.project, default_enabled: :yaml) - # Project iid must be called outside a transaction, so we ensure it is set here - # otherwise it may be set within the state transition transaction of the skip call - # which it will lock the InternalId row for the whole transaction - @pipeline.ensure_project_iid! - end + # Project iid must be called outside a transaction, so we ensure it is set here + # otherwise it may be set within the state transition transaction of the skip call + # which it will lock the InternalId row for the whole transaction + @pipeline.ensure_project_iid! @pipeline.skip end diff --git a/lib/gitlab/email/handler/create_issue_handler.rb b/lib/gitlab/email/handler/create_issue_handler.rb index 22fc8addcd9..e927a5641e5 100644 --- a/lib/gitlab/email/handler/create_issue_handler.rb +++ b/lib/gitlab/email/handler/create_issue_handler.rb @@ -56,10 +56,12 @@ module Gitlab def create_issue Issues::CreateService.new( - project, - author, - title: mail.subject, - description: message_including_reply + project: project, + current_user: author, + params: { + title: mail.subject, + description: message_including_reply + } ).execute end diff --git a/lib/gitlab/email/handler/create_merge_request_handler.rb b/lib/gitlab/email/handler/create_merge_request_handler.rb index e8071bcafd0..df12aea1988 100644 --- a/lib/gitlab/email/handler/create_merge_request_handler.rb +++ b/lib/gitlab/email/handler/create_merge_request_handler.rb @@ -61,7 +61,7 @@ module Gitlab private def build_merge_request - MergeRequests::BuildService.new(project, author, merge_request_params).execute + MergeRequests::BuildService.new(project: project, current_user: author, params: merge_request_params).execute end def create_merge_request @@ -78,7 +78,7 @@ module Gitlab if merge_request.errors.any? merge_request else - MergeRequests::CreateService.new(project, author).create(merge_request) + MergeRequests::CreateService.new(project: project, current_user: author).create(merge_request) end end diff --git a/lib/gitlab/email/handler/service_desk_handler.rb b/lib/gitlab/email/handler/service_desk_handler.rb index 14930e90c9e..cab3538a447 100644 --- a/lib/gitlab/email/handler/service_desk_handler.rb +++ b/lib/gitlab/email/handler/service_desk_handler.rb @@ -77,12 +77,14 @@ module Gitlab def create_issue! @issue = Issues::CreateService.new( - project, - User.support_bot, - title: mail.subject, - description: message_including_template, - confidential: true, - external_author: from_address + project: project, + current_user: User.support_bot, + params: { + title: mail.subject, + description: message_including_template, + confidential: true, + external_author: from_address + } ).execute raise InvalidIssueError unless @issue.persisted? diff --git a/lib/gitlab/quick_actions/issue_actions.rb b/lib/gitlab/quick_actions/issue_actions.rb index 012e495502f..ff17ecf8024 100644 --- a/lib/gitlab/quick_actions/issue_actions.rb +++ b/lib/gitlab/quick_actions/issue_actions.rb @@ -267,7 +267,7 @@ module Gitlab private def zoom_link_service - Issues::ZoomLinkService.new(quick_action_target, current_user) + Issues::ZoomLinkService.new(project: quick_action_target.project, current_user: current_user, params: { issue: quick_action_target }) end end end diff --git a/lib/gitlab/quick_actions/merge_request_actions.rb b/lib/gitlab/quick_actions/merge_request_actions.rb index 6a404c34044..f3c6315cd6a 100644 --- a/lib/gitlab/quick_actions/merge_request_actions.rb +++ b/lib/gitlab/quick_actions/merge_request_actions.rb @@ -148,7 +148,7 @@ module Gitlab quick_action_target.persisted? && quick_action_target.can_be_approved_by?(current_user) end command :approve do - success = MergeRequests::ApprovalService.new(quick_action_target.project, current_user).execute(quick_action_target) + success = MergeRequests::ApprovalService.new(project: quick_action_target.project, current_user: current_user).execute(quick_action_target) next unless success diff --git a/lib/gitlab/slash_commands/issue_close.rb b/lib/gitlab/slash_commands/issue_close.rb index 5fcc86e91c4..3dad7216983 100644 --- a/lib/gitlab/slash_commands/issue_close.rb +++ b/lib/gitlab/slash_commands/issue_close.rb @@ -29,7 +29,7 @@ module Gitlab private def close_issue(issue:) - Issues::CloseService.new(project, current_user).execute(issue) + Issues::CloseService.new(project: project, current_user: current_user).execute(issue) end def presenter(issue) diff --git a/lib/gitlab/slash_commands/issue_move.rb b/lib/gitlab/slash_commands/issue_move.rb index d2f1f130b38..0612663017c 100644 --- a/lib/gitlab/slash_commands/issue_move.rb +++ b/lib/gitlab/slash_commands/issue_move.rb @@ -29,7 +29,7 @@ module Gitlab return Gitlab::SlashCommands::Presenters::Access.new.not_found end - new_issue = Issues::MoveService.new(project, current_user) + new_issue = Issues::MoveService.new(project: project, current_user: current_user) .execute(old_issue, target_project) presenter(new_issue).present(old_issue) diff --git a/lib/gitlab/slash_commands/issue_new.rb b/lib/gitlab/slash_commands/issue_new.rb index 48379031537..99a056c97fc 100644 --- a/lib/gitlab/slash_commands/issue_new.rb +++ b/lib/gitlab/slash_commands/issue_new.rb @@ -33,7 +33,7 @@ module Gitlab private def create_issue(title:, description:) - Issues::CreateService.new(project, current_user, title: title, description: description).execute + Issues::CreateService.new(project: project, current_user: current_user, params: { title: title, description: description }).execute end def presenter(issue) diff --git a/lib/quality/seeders/issues.rb b/lib/quality/seeders/issues.rb index ae19e86546a..ea2db2aa5fe 100644 --- a/lib/quality/seeders/issues.rb +++ b/lib/quality/seeders/issues.rb @@ -30,7 +30,7 @@ module Quality labels: labels.join(',') } params[:closed_at] = params[:created_at] + rand(35).days if params[:state] == 'closed' - issue = ::Issues::CreateService.new(project, team.sample, params).execute + issue = ::Issues::CreateService.new(project: project, current_user: team.sample, params: params).execute if issue.persisted? created_issues_count += 1 diff --git a/package.json b/package.json index 087fd21e488..073f908069c 100644 --- a/package.json +++ b/package.json @@ -51,9 +51,9 @@ "@babel/preset-env": "^7.10.1", "@gitlab/at.js": "1.5.7", "@gitlab/favicon-overlay": "2.0.0", - "@gitlab/svgs": "1.192.0", + "@gitlab/svgs": "1.195.0", "@gitlab/tributejs": "1.0.0", - "@gitlab/ui": "29.18.0", + "@gitlab/ui": "29.21.0", "@gitlab/visual-review-tools": "1.6.1", "@rails/actioncable": "^6.0.3-4", "@rails/ujs": "^6.0.3-4", diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 41d3cac63a6..989f941caea 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -204,7 +204,7 @@ RSpec.describe Projects::MergeRequests::DiffsController do end it "correctly generates the right diff between versions" do - MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) + MergeRequests::MergeToRefService.new(project: project, current_user: merge_request.author).execute(merge_request) expect_next_instance_of(CompareService) do |service| expect(service).to receive(:execute).with( diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index a04bda9902b..d4c52e1c7ca 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -531,7 +531,7 @@ RSpec.describe Projects::MergeRequestsController do sha: merge_request.diff_head_sha, merge_request: merge_request } - expect_next_instance_of(MergeRequests::SquashService, project, user, expected_squash_params) do |squash_service| + expect_next_instance_of(MergeRequests::SquashService, project: project, current_user: user, params: expected_squash_params) do |squash_service| expect(squash_service).to receive(:execute).and_return({ status: :success, squash_sha: SecureRandom.hex(20) @@ -1831,7 +1831,7 @@ RSpec.describe Projects::MergeRequestsController do it 'calls MergeRequests::AssignIssuesService' do expect(MergeRequests::AssignIssuesService).to receive(:new) - .with(project, user, merge_request: merge_request) + .with(project: project, current_user: user, params: { merge_request: merge_request }) .and_return(double(execute: { count: 1 })) post_assign_issues diff --git a/spec/features/calendar_spec.rb b/spec/features/calendar_spec.rb index 0b73226268d..1281d890ef7 100644 --- a/spec/features/calendar_spec.rb +++ b/spec/features/calendar_spec.rb @@ -146,7 +146,7 @@ RSpec.describe 'Contributions Calendar', :js do describe '1 issue creation calendar activity' do before do - Issues::CreateService.new(contributed_project, user, issue_params).execute + Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params).execute end it_behaves_like 'a day with activity', contribution_count: 1 @@ -181,7 +181,7 @@ RSpec.describe 'Contributions Calendar', :js do push_code_contribution travel_to(Date.yesterday) do - Issues::CreateService.new(contributed_project, user, issue_params).execute + Issues::CreateService.new(project: contributed_project, current_user: user, params: issue_params).execute end end include_context 'visit user page' diff --git a/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb b/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb index 78c1b2a718e..35be21a646e 100644 --- a/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb +++ b/spec/features/merge_request/user_sees_cherry_pick_modal_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Merge request > User cherry-picks', :js do context 'Viewing a merged merge request' do before do - service = MergeRequests::MergeService.new(project, user, sha: merge_request.diff_head_sha) + service = MergeRequests::MergeService.new(project: project, current_user: user, params: { sha: merge_request.diff_head_sha }) perform_enqueued_jobs do service.execute(merge_request) diff --git a/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb b/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb index cbd68025b50..a764dd97878 100644 --- a/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb +++ b/spec/features/merge_request/user_sees_mr_from_deleted_forked_project_spec.rb @@ -15,7 +15,7 @@ RSpec.describe 'Merge request > User sees MR from deleted forked project', :js d end before do - MergeRequests::MergeService.new(project, user).execute(merge_request) + MergeRequests::MergeService.new(project: project, current_user: user).execute(merge_request) forked_project.destroy! sign_in(user) visit project_merge_request_path(project, merge_request) diff --git a/spec/features/merge_request/user_sees_pipelines_spec.rb b/spec/features/merge_request/user_sees_pipelines_spec.rb index a5047c8d550..2d8fe10b987 100644 --- a/spec/features/merge_request/user_sees_pipelines_spec.rb +++ b/spec/features/merge_request/user_sees_pipelines_spec.rb @@ -239,7 +239,7 @@ RSpec.describe 'Merge request > User sees pipelines', :js do threads << Thread.new do Sidekiq::Worker.skipping_transaction_check do - @merge_request = MergeRequests::CreateService.new(project, user, merge_request_params).execute + @merge_request = MergeRequests::CreateService.new(project: project, current_user: user, params: merge_request_params).execute end end diff --git a/spec/features/unsubscribe_links_spec.rb b/spec/features/unsubscribe_links_spec.rb index 966d90ab16b..b2d0f29808c 100644 --- a/spec/features/unsubscribe_links_spec.rb +++ b/spec/features/unsubscribe_links_spec.rb @@ -9,7 +9,7 @@ RSpec.describe 'Unsubscribe links', :sidekiq_might_not_need_inline do let(:author) { create(:user) } let(:project) { create(:project, :public) } let(:params) { { title: 'A bug!', description: 'Fix it!', assignees: [recipient] } } - let(:issue) { Issues::CreateService.new(project, author, params).execute } + let(:issue) { Issues::CreateService.new(project: project, current_user: author, params: params).execute } let(:mail) { ActionMailer::Base.deliveries.last } let(:body) { Capybara::Node::Simple.new(mail.default_part_body.to_s) } diff --git a/spec/features/users/user_browses_projects_on_user_page_spec.rb b/spec/features/users/user_browses_projects_on_user_page_spec.rb index 7d05b2ae27a..ded90be3924 100644 --- a/spec/features/users/user_browses_projects_on_user_page_spec.rb +++ b/spec/features/users/user_browses_projects_on_user_page_spec.rb @@ -125,7 +125,7 @@ RSpec.describe 'Users > User browses projects on user page', :js do end before do - Issues::CreateService.new(contributed_project, user, { title: 'Bug in old browser' }).execute + Issues::CreateService.new(project: contributed_project, current_user: user, params: { title: 'Bug in old browser' }).execute event = create(:push_event, project: contributed_project, author: user) create(:push_event_payload, event: event, commit_count: 3) end diff --git a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb index ae427eaf403..27af8d379ef 100644 --- a/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb +++ b/spec/lib/gitlab/ci/pipeline/chain/skip_spec.rb @@ -40,18 +40,6 @@ RSpec.describe Gitlab::Ci::Pipeline::Chain::Skip do step.perform! end - - context 'when the ci_pipeline_ensure_iid_on_save feature flag is off' do - before do - stub_feature_flags(ci_pipeline_ensure_iid_on_skip: false) - end - - it 'does not call ensure_project_iid explicitly' do - expect(pipeline).not_to receive(:ensure_project_iid!) - - step.perform! - end - end end context 'when pipeline has not been skipped' do diff --git a/spec/lib/gitlab/ci/templates/Jobs/build_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Jobs/build_gitlab_ci_yaml_spec.rb index 1f278048ad5..053499344e1 100644 --- a/spec/lib/gitlab/ci/templates/Jobs/build_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Jobs/build_gitlab_ci_yaml_spec.rb @@ -45,7 +45,7 @@ RSpec.describe 'Jobs/Build.gitlab-ci.yml' do end context 'on merge request' do - let(:service) { MergeRequests::CreatePipelineService.new(project, user) } + let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:pipeline) { service.execute(merge_request) } diff --git a/spec/lib/gitlab/ci/templates/Jobs/code_quality_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Jobs/code_quality_gitlab_ci_yaml_spec.rb index 0a76de82421..b23457315cc 100644 --- a/spec/lib/gitlab/ci/templates/Jobs/code_quality_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Jobs/code_quality_gitlab_ci_yaml_spec.rb @@ -45,7 +45,7 @@ RSpec.describe 'Jobs/Code-Quality.gitlab-ci.yml' do end context 'on merge request' do - let(:service) { MergeRequests::CreatePipelineService.new(project, user) } + let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:pipeline) { service.execute(merge_request) } diff --git a/spec/lib/gitlab/ci/templates/Jobs/deploy_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Jobs/deploy_gitlab_ci_yaml_spec.rb index 25c88c161ea..1d137ef89e1 100644 --- a/spec/lib/gitlab/ci/templates/Jobs/deploy_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Jobs/deploy_gitlab_ci_yaml_spec.rb @@ -208,7 +208,7 @@ RSpec.describe 'Jobs/Deploy.gitlab-ci.yml' do end context 'on merge request' do - let(:service) { MergeRequests::CreatePipelineService.new(project, user) } + let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:pipeline) { service.execute(merge_request) } diff --git a/spec/lib/gitlab/ci/templates/Jobs/test_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Jobs/test_gitlab_ci_yaml_spec.rb index b64959a9917..7fa8d906d07 100644 --- a/spec/lib/gitlab/ci/templates/Jobs/test_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Jobs/test_gitlab_ci_yaml_spec.rb @@ -45,7 +45,7 @@ RSpec.describe 'Jobs/Test.gitlab-ci.yml' do end context 'on merge request' do - let(:service) { MergeRequests::CreatePipelineService.new(project, user) } + let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:pipeline) { service.execute(merge_request) } diff --git a/spec/lib/gitlab/ci/templates/Verify/load_performance_testing_gitlab_ci_yaml_spec.rb b/spec/lib/gitlab/ci/templates/Verify/load_performance_testing_gitlab_ci_yaml_spec.rb index 03fa45fe0a1..e53d2f4f975 100644 --- a/spec/lib/gitlab/ci/templates/Verify/load_performance_testing_gitlab_ci_yaml_spec.rb +++ b/spec/lib/gitlab/ci/templates/Verify/load_performance_testing_gitlab_ci_yaml_spec.rb @@ -62,7 +62,7 @@ RSpec.describe 'Verify/Load-Performance-Testing.gitlab-ci.yml' do end context 'on merge request' do - let(:service) { MergeRequests::CreatePipelineService.new(project, user) } + let(:service) { MergeRequests::CreatePipelineService.new(project: project, current_user: user) } let(:merge_request) { create(:merge_request, :simple, source_project: project) } let(:pipeline) { service.execute(merge_request) } diff --git a/spec/lib/gitlab/slash_commands/presenters/issue_move_spec.rb b/spec/lib/gitlab/slash_commands/presenters/issue_move_spec.rb index 7b3440b40a7..7d36e67ddbf 100644 --- a/spec/lib/gitlab/slash_commands/presenters/issue_move_spec.rb +++ b/spec/lib/gitlab/slash_commands/presenters/issue_move_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Gitlab::SlashCommands::Presenters::IssueMove do let_it_be(:other_project) { create(:project) } let_it_be(:old_issue, reload: true) { create(:issue, project: project) } - let(:new_issue) { Issues::MoveService.new(project, user).execute(old_issue, other_project) } + let(:new_issue) { Issues::MoveService.new(project: project, current_user: user).execute(old_issue, other_project) } let(:attachment) { subject[:attachments].first } subject { described_class.new(new_issue).present(old_issue) } diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index c7850b0bc16..e51711c3895 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3653,7 +3653,7 @@ RSpec.describe Ci::Build do end describe 'state transition when build fails' do - let(:service) { ::MergeRequests::AddTodoWhenBuildFailsService.new(project, user) } + let(:service) { ::MergeRequests::AddTodoWhenBuildFailsService.new(project: project, current_user: user) } before do allow(::MergeRequests::AddTodoWhenBuildFailsService).to receive(:new).and_return(service) diff --git a/spec/models/concerns/noteable_spec.rb b/spec/models/concerns/noteable_spec.rb index a7117af81a2..38766d8decd 100644 --- a/spec/models/concerns/noteable_spec.rb +++ b/spec/models/concerns/noteable_spec.rb @@ -288,7 +288,7 @@ RSpec.describe Noteable do end before do - MergeRequests::MergeToRefService.new(merge_request.project, merge_request.author).execute(merge_request) + MergeRequests::MergeToRefService.new(project: merge_request.project, current_user: merge_request.author).execute(merge_request) Discussions::CaptureDiffNotePositionsService.new(merge_request).execute end diff --git a/spec/models/merge_request_diff_spec.rb b/spec/models/merge_request_diff_spec.rb index 0e843f0d9dc..4075eb96fc2 100644 --- a/spec/models/merge_request_diff_spec.rb +++ b/spec/models/merge_request_diff_spec.rb @@ -61,7 +61,7 @@ RSpec.describe MergeRequestDiff do let_it_be(:merge_head) do MergeRequests::MergeToRefService - .new(merge_request.project, merge_request.author) + .new(project: merge_request.project, current_user: merge_request.author) .execute(merge_request) merge_request.create_merge_head_diff diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb index b534c761d2a..84d4794df5e 100644 --- a/spec/models/merge_request_spec.rb +++ b/spec/models/merge_request_spec.rb @@ -2628,7 +2628,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'when the MR has been merged' do before do MergeRequests::MergeService - .new(subject.target_project, subject.author, { sha: subject.diff_head_sha }) + .new(project: subject.target_project, current_user: subject.author, params: { sha: subject.diff_head_sha }) .execute(subject) end @@ -4806,7 +4806,7 @@ RSpec.describe MergeRequest, factory_default: :keep do context 'when merge_ref_sha is not present' do let!(:result) do MergeRequests::MergeToRefService - .new(merge_request.project, merge_request.author) + .new(project: merge_request.project, current_user: merge_request.author) .execute(merge_request) end diff --git a/spec/models/project_services/microsoft_teams_service_spec.rb b/spec/models/project_services/microsoft_teams_service_spec.rb index 53ab63ef030..b7eb558b2e7 100644 --- a/spec/models/project_services/microsoft_teams_service_spec.rb +++ b/spec/models/project_services/microsoft_teams_service_spec.rb @@ -73,7 +73,7 @@ RSpec.describe MicrosoftTeamsService do context 'with issue events' do let(:opts) { { title: 'Awesome issue', description: 'please fix' } } let(:issues_sample_data) do - service = Issues::CreateService.new(project, user, opts) + service = Issues::CreateService.new(project: project, current_user: user, params: opts) issue = service.execute service.hook_data(issue, 'open') end @@ -96,7 +96,7 @@ RSpec.describe MicrosoftTeamsService do end let(:merge_sample_data) do - service = MergeRequests::CreateService.new(project, user, opts) + service = MergeRequests::CreateService.new(project: project, current_user: user, params: opts) merge_request = service.execute service.hook_data(merge_request, 'open') end diff --git a/spec/serializers/issue_entity_spec.rb b/spec/serializers/issue_entity_spec.rb index 82ea26fae40..76f8cf644c6 100644 --- a/spec/serializers/issue_entity_spec.rb +++ b/spec/serializers/issue_entity_spec.rb @@ -29,7 +29,7 @@ RSpec.describe IssueEntity do before do project.add_developer(member) public_project.add_developer(member) - Issues::MoveService.new(public_project, member).execute(issue, project) + Issues::MoveService.new(project: public_project, current_user: member).execute(issue, project) end context 'when user cannot read target project' do @@ -61,7 +61,7 @@ RSpec.describe IssueEntity do before do Issues::DuplicateService - .new(project, member) + .new(project: project, current_user: member) .execute(issue, new_issue) end diff --git a/spec/services/discussions/capture_diff_note_positions_service_spec.rb b/spec/services/discussions/capture_diff_note_positions_service_spec.rb index be53b02a4c1..25e5f549bee 100644 --- a/spec/services/discussions/capture_diff_note_positions_service_spec.rb +++ b/spec/services/discussions/capture_diff_note_positions_service_spec.rb @@ -55,7 +55,7 @@ RSpec.describe Discussions::CaptureDiffNotePositionsService do context 'and position of the discussion changed on target branch head' do it 'diff positions are created for the first notes of the discussions' do - MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) + MergeRequests::MergeToRefService.new(project: project, current_user: merge_request.author).execute(merge_request) service.execute verify_diff_note_position!(first_discussion_note, new_line: first_new_line) diff --git a/spec/services/draft_notes/publish_service_spec.rb b/spec/services/draft_notes/publish_service_spec.rb index f93622dc25a..2e1de367da3 100644 --- a/spec/services/draft_notes/publish_service_spec.rb +++ b/spec/services/draft_notes/publish_service_spec.rb @@ -202,7 +202,7 @@ RSpec.describe DraftNotes::PublishService do expect(newrev).to be_present # Generates new MR revision at DB level - refresh = MergeRequests::RefreshService.new(project, user) + refresh = MergeRequests::RefreshService.new(project: project, current_user: user) refresh.execute(oldrev, newrev, merge_request.source_branch_ref) expect { publish(draft: draft) }.to change { Suggestion.count }.by(1) diff --git a/spec/services/issuable/common_system_notes_service_spec.rb b/spec/services/issuable/common_system_notes_service_spec.rb index a988ab81754..1426ef2a1f6 100644 --- a/spec/services/issuable/common_system_notes_service_spec.rb +++ b/spec/services/issuable/common_system_notes_service_spec.rb @@ -23,7 +23,7 @@ RSpec.describe Issuable::CommonSystemNotesService do end it 'creates a resource label event' do - described_class.new(project, user).execute(issuable, old_labels: []) + described_class.new(project: project, current_user: user).execute(issuable, old_labels: []) event = issuable.reload.resource_label_events.last expect(event).not_to be_nil @@ -66,7 +66,7 @@ RSpec.describe Issuable::CommonSystemNotesService do context 'on issuable create' do let(:issuable) { build(:issue, project: project) } - subject { described_class.new(project, user).execute(issuable, old_labels: [], is_update: false) } + subject { described_class.new(project: project, current_user: user).execute(issuable, old_labels: [], is_update: false) } it 'does not create system note for title and description' do issuable.save! diff --git a/spec/services/issuable/destroy_service_spec.rb b/spec/services/issuable/destroy_service_spec.rb index fa4902e5237..21063539a4b 100644 --- a/spec/services/issuable/destroy_service_spec.rb +++ b/spec/services/issuable/destroy_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Issuable::DestroyService do let(:group) { create(:group, :public) } let(:project) { create(:project, :public, group: group) } - subject(:service) { described_class.new(project, user) } + subject(:service) { described_class.new(project: project, current_user: user) } describe '#execute' do context 'when issuable is an issue' do diff --git a/spec/services/issues/after_create_service_spec.rb b/spec/services/issues/after_create_service_spec.rb index bc9be3211d3..6b720d6e687 100644 --- a/spec/services/issues/after_create_service_spec.rb +++ b/spec/services/issues/after_create_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Issues::AfterCreateService do let_it_be(:milestone) { create(:milestone, project: project) } let_it_be(:issue) { create(:issue, project: project, author: current_user, milestone: milestone, assignee_ids: [assignee.id]) } - subject(:after_create_service) { described_class.new(project, current_user) } + subject(:after_create_service) { described_class.new(project: project, current_user: current_user) } describe '#execute' do it 'creates a pending todo for new assignee' do diff --git a/spec/services/issues/build_service_spec.rb b/spec/services/issues/build_service_spec.rb index 87b5f250d4b..d0f228fb3d9 100644 --- a/spec/services/issues/build_service_spec.rb +++ b/spec/services/issues/build_service_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Issues::BuildService do end def build_issue(issue_params = {}) - described_class.new(project, user, issue_params).execute + described_class.new(project: project, current_user: user, params: issue_params).execute end context 'for a single discussion' do @@ -41,7 +41,7 @@ RSpec.describe Issues::BuildService do describe '#items_for_discussions' do it 'has an item for each discussion' do create(:diff_note_on_merge_request, noteable: merge_request, project: merge_request.source_project, line_number: 13) - service = described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) + service = described_class.new(project: project, current_user: user, params: { merge_request_to_resolve_discussions_of: merge_request.iid }) service.execute @@ -50,7 +50,7 @@ RSpec.describe Issues::BuildService do end describe '#item_for_discussion' do - let(:service) { described_class.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } + let(:service) { described_class.new(project: project, current_user: user, params: { merge_request_to_resolve_discussions_of: merge_request.iid }) } it 'mentions the author of the note' do discussion = create(:diff_note_on_merge_request, author: create(:user, username: 'author')).to_discussion diff --git a/spec/services/issues/clone_service_spec.rb b/spec/services/issues/clone_service_spec.rb index 44180a322ca..abbcb1c1d48 100644 --- a/spec/services/issues/clone_service_spec.rb +++ b/spec/services/issues/clone_service_spec.rb @@ -22,7 +22,7 @@ RSpec.describe Issues::CloneService do let(:with_notes) { false } subject(:clone_service) do - described_class.new(old_project, user) + described_class.new(project: old_project, current_user: user) end shared_context 'user can clone issue' do diff --git a/spec/services/issues/close_service_spec.rb b/spec/services/issues/close_service_spec.rb index 8e02a07c0cd..f18c72d3991 100644 --- a/spec/services/issues/close_service_spec.rb +++ b/spec/services/issues/close_service_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Issues::CloseService do end describe '#execute' do - let(:service) { described_class.new(project, user) } + let(:service) { described_class.new(project: project, current_user: user) } it 'checks if the user is authorized to update the issue' do expect(service).to receive(:can?).with(user, :update_issue, issue) @@ -87,7 +87,7 @@ RSpec.describe Issues::CloseService do project.reload expect(project.external_issue_tracker).to receive(:close_issue) - described_class.new(project, user).close_issue(external_issue) + described_class.new(project: project, current_user: user).close_issue(external_issue) end end @@ -98,7 +98,7 @@ RSpec.describe Issues::CloseService do project.reload expect(project.external_issue_tracker).not_to receive(:close_issue) - described_class.new(project, user).close_issue(external_issue) + described_class.new(project: project, current_user: user).close_issue(external_issue) end end @@ -109,7 +109,7 @@ RSpec.describe Issues::CloseService do project.reload expect(project.external_issue_tracker).not_to receive(:close_issue) - described_class.new(project, user).close_issue(external_issue) + described_class.new(project: project, current_user: user).close_issue(external_issue) end end end @@ -117,7 +117,7 @@ RSpec.describe Issues::CloseService do context "closed by a merge request", :sidekiq_might_not_need_inline do subject(:close_issue) do perform_enqueued_jobs do - described_class.new(project, user).close_issue(issue, closed_via: closing_merge_request) + described_class.new(project: project, current_user: user).close_issue(issue, closed_via: closing_merge_request) end end @@ -186,7 +186,7 @@ RSpec.describe Issues::CloseService do context "closed by a commit", :sidekiq_might_not_need_inline do it 'mentions closure via a commit' do perform_enqueued_jobs do - described_class.new(project, user).close_issue(issue, closed_via: closing_commit) + described_class.new(project: project, current_user: user).close_issue(issue, closed_via: closing_commit) end email = ActionMailer::Base.deliveries.last @@ -200,7 +200,7 @@ RSpec.describe Issues::CloseService do it 'does not mention the commit id' do project.project_feature.update_attribute(:repository_access_level, ProjectFeature::DISABLED) perform_enqueued_jobs do - described_class.new(project, user).close_issue(issue, closed_via: closing_commit) + described_class.new(project: project, current_user: user).close_issue(issue, closed_via: closing_commit) end email = ActionMailer::Base.deliveries.last @@ -216,7 +216,7 @@ RSpec.describe Issues::CloseService do context "valid params" do subject(:close_issue) do perform_enqueued_jobs do - described_class.new(project, user).close_issue(issue) + described_class.new(project: project, current_user: user).close_issue(issue) end end @@ -325,7 +325,7 @@ RSpec.describe Issues::CloseService do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) - described_class.new(project, user).close_issue(issue) + described_class.new(project: project, current_user: user).close_issue(issue) end end @@ -336,7 +336,7 @@ RSpec.describe Issues::CloseService do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) - described_class.new(project, user).close_issue(issue) + described_class.new(project: project, current_user: user).close_issue(issue) end end diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb index 83c6373c335..9c84242d8ae 100644 --- a/spec/services/issues/create_service_spec.rb +++ b/spec/services/issues/create_service_spec.rb @@ -12,7 +12,7 @@ RSpec.describe Issues::CreateService do let_it_be(:assignee) { create(:user) } let_it_be(:milestone) { create(:milestone, project: project) } - let(:issue) { described_class.new(project, user, opts).execute } + let(:issue) { described_class.new(project: project, current_user: user, params: opts).execute } context 'when params are valid' do let_it_be(:labels) { create_pair(:label, project: project) } @@ -44,7 +44,7 @@ RSpec.describe Issues::CreateService do end context 'when skip_system_notes is true' do - let(:issue) { described_class.new(project, user, opts).execute(skip_system_notes: true) } + let(:issue) { described_class.new(project: project, current_user: user, params: opts).execute(skip_system_notes: true) } it 'does not call Issuable::CommonSystemNotesService' do expect(Issuable::CommonSystemNotesService).not_to receive(:new) @@ -96,7 +96,7 @@ RSpec.describe Issues::CreateService do end it 'filters out params that cannot be set without the :admin_issue permission' do - issue = described_class.new(project, guest, opts).execute + issue = described_class.new(project: project, current_user: guest, params: opts).execute expect(issue).to be_persisted expect(issue.title).to eq('Awesome issue') @@ -108,7 +108,7 @@ RSpec.describe Issues::CreateService do end it 'creates confidential issues' do - issue = described_class.new(project, guest, confidential: true).execute + issue = described_class.new(project: project, current_user: guest, params: { confidential: true }).execute expect(issue.confidential).to be_truthy end @@ -117,7 +117,7 @@ RSpec.describe Issues::CreateService do it 'moves the issue to the end, in an asynchronous worker' do expect(IssuePlacementWorker).to receive(:perform_async).with(be_nil, Integer) - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute end context 'when label belongs to project group' do @@ -204,7 +204,7 @@ RSpec.describe Issues::CreateService do it 'invalidates open issues counter for assignees when issue is assigned' do project.add_maintainer(assignee) - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute expect(assignee.assigned_open_issues_count).to eq 1 end @@ -230,7 +230,7 @@ RSpec.describe Issues::CreateService do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute end it 'executes confidential issue hooks when issue is confidential' do @@ -239,7 +239,7 @@ RSpec.describe Issues::CreateService do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute end context 'after_save callback to store_mentions' do @@ -283,7 +283,7 @@ RSpec.describe Issues::CreateService do it 'removes assignee when user id is invalid' do opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } - issue = described_class.new(project, user, opts).execute + issue = described_class.new(project: project, current_user: user, params: opts).execute expect(issue.assignees).to be_empty end @@ -291,7 +291,7 @@ RSpec.describe Issues::CreateService do it 'removes assignee when user id is 0' do opts = { title: 'Title', description: 'Description', assignee_ids: [0] } - issue = described_class.new(project, user, opts).execute + issue = described_class.new(project: project, current_user: user, params: opts).execute expect(issue.assignees).to be_empty end @@ -300,7 +300,7 @@ RSpec.describe Issues::CreateService do project.add_maintainer(assignee) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } - issue = described_class.new(project, user, opts).execute + issue = described_class.new(project: project, current_user: user, params: opts).execute expect(issue.assignees).to eq([assignee]) end @@ -318,7 +318,7 @@ RSpec.describe Issues::CreateService do project.update!(visibility_level: level) opts = { title: 'Title', description: 'Description', assignee_ids: [assignee.id] } - issue = described_class.new(project, user, opts).execute + issue = described_class.new(project: project, current_user: user, params: opts).execute expect(issue.assignees).to be_empty end @@ -328,7 +328,7 @@ RSpec.describe Issues::CreateService do end it_behaves_like 'issuable record that supports quick actions' do - let(:issuable) { described_class.new(project, user, params).execute } + let(:issuable) { described_class.new(project: project, current_user: user, params: params).execute } end context 'Quick actions' do @@ -368,14 +368,14 @@ RSpec.describe Issues::CreateService do let(:opts) { { discussion_to_resolve: discussion.id, merge_request_to_resolve_discussions_of: merge_request.iid } } it 'resolves the discussion' do - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute discussion.first_note.reload expect(discussion.resolved?).to be(true) end it 'added a system note to the discussion' do - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first @@ -383,17 +383,19 @@ RSpec.describe Issues::CreateService do end it 'assigns the title and description for the issue' do - issue = described_class.new(project, user, opts).execute + issue = described_class.new(project: project, current_user: user, params: opts).execute expect(issue.title).not_to be_nil expect(issue.description).not_to be_nil end it 'can set nil explicitly to the title and description' do - issue = described_class.new(project, user, - merge_request_to_resolve_discussions_of: merge_request, - description: nil, - title: nil).execute + issue = described_class.new(project: project, current_user: user, + params: { + merge_request_to_resolve_discussions_of: merge_request, + description: nil, + title: nil + }).execute expect(issue.description).to be_nil expect(issue.title).to be_nil @@ -404,14 +406,14 @@ RSpec.describe Issues::CreateService do let(:opts) { { merge_request_to_resolve_discussions_of: merge_request.iid } } it 'resolves the discussion' do - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute discussion.first_note.reload expect(discussion.resolved?).to be(true) end it 'added a system note to the discussion' do - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute reloaded_discussion = MergeRequest.find(merge_request.id).discussions.first @@ -419,17 +421,19 @@ RSpec.describe Issues::CreateService do end it 'assigns the title and description for the issue' do - issue = described_class.new(project, user, opts).execute + issue = described_class.new(project: project, current_user: user, params: opts).execute expect(issue.title).not_to be_nil expect(issue.description).not_to be_nil end it 'can set nil explicitly to the title and description' do - issue = described_class.new(project, user, - merge_request_to_resolve_discussions_of: merge_request, - description: nil, - title: nil).execute + issue = described_class.new(project: project, current_user: user, + params: { + merge_request_to_resolve_discussions_of: merge_request, + description: nil, + title: nil + }).execute expect(issue.description).to be_nil expect(issue.title).to be_nil @@ -454,7 +458,7 @@ RSpec.describe Issues::CreateService do end subject do - described_class.new(project, user, params) + described_class.new(project: project, current_user: user, params: params) end before do diff --git a/spec/services/issues/duplicate_service_spec.rb b/spec/services/issues/duplicate_service_spec.rb index 0b5bc3f32ef..0eb0bbb1480 100644 --- a/spec/services/issues/duplicate_service_spec.rb +++ b/spec/services/issues/duplicate_service_spec.rb @@ -10,7 +10,7 @@ RSpec.describe Issues::DuplicateService do let(:canonical_issue) { create(:issue, project: canonical_project) } let(:duplicate_issue) { create(:issue, project: duplicate_project) } - subject { described_class.new(duplicate_project, user, {}) } + subject { described_class.new(project: duplicate_project, current_user: user) } describe '#execute' do context 'when the issues passed are the same' do diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 2f29a2e2022..76588860957 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -20,7 +20,7 @@ RSpec.describe Issues::MoveService do end subject(:move_service) do - described_class.new(old_project, user) + described_class.new(project: old_project, current_user: user) end shared_context 'user can move issue' do diff --git a/spec/services/issues/referenced_merge_requests_service_spec.rb b/spec/services/issues/referenced_merge_requests_service_spec.rb index 1dfe88d9951..dc55ba8ebea 100644 --- a/spec/services/issues/referenced_merge_requests_service_spec.rb +++ b/spec/services/issues/referenced_merge_requests_service_spec.rb @@ -26,7 +26,7 @@ RSpec.describe Issues::ReferencedMergeRequestsService do let_it_be(:referencing_mr) { create_referencing_mr(source_project: project, source_branch: 'csv') } let_it_be(:referencing_mr_other_project) { create_referencing_mr(source_project: other_project, source_branch: 'csv') } - let(:service) { described_class.new(project, user) } + let(:service) { described_class.new(project: project, current_user: user) } describe '#execute' do it 'returns a list of sorted merge requests' do diff --git a/spec/services/issues/related_branches_service_spec.rb b/spec/services/issues/related_branches_service_spec.rb index c9c029bca4f..7a4bae7f852 100644 --- a/spec/services/issues/related_branches_service_spec.rb +++ b/spec/services/issues/related_branches_service_spec.rb @@ -8,7 +8,7 @@ RSpec.describe Issues::RelatedBranchesService do let(:user) { developer } - subject { described_class.new(issue.project, user) } + subject { described_class.new(project: issue.project, current_user: user) } before do issue.project.add_developer(developer) @@ -95,7 +95,7 @@ RSpec.describe Issues::RelatedBranchesService do merge_request.create_cross_references!(user) referenced_merge_requests = Issues::ReferencedMergeRequestsService - .new(issue.project, user) + .new(project: issue.project, current_user: user) .referenced_merge_requests(issue) expect(referenced_merge_requests).not_to be_empty diff --git a/spec/services/issues/reopen_service_spec.rb b/spec/services/issues/reopen_service_spec.rb index ffe74cca9cf..746a9105531 100644 --- a/spec/services/issues/reopen_service_spec.rb +++ b/spec/services/issues/reopen_service_spec.rb @@ -13,7 +13,7 @@ RSpec.describe Issues::ReopenService do project.add_guest(guest) perform_enqueued_jobs do - described_class.new(project, guest).execute(issue) + described_class.new(project: project, current_user: guest).execute(issue) end end @@ -33,11 +33,11 @@ RSpec.describe Issues::ReopenService do issue.assignees << user expect_any_instance_of(User).to receive(:invalidate_issue_cache_counts) - described_class.new(project, user).execute(issue) + described_class.new(project: project, current_user: user).execute(issue) end it 'refreshes the number of opened issues' do - service = described_class.new(project, user) + service = described_class.new(project: project, current_user: user) expect { service.execute(issue) } .to change { project.open_issues_count }.from(0).to(1) @@ -50,14 +50,14 @@ RSpec.describe Issues::ReopenService do expect(service).to receive(:delete_cache).and_call_original end - described_class.new(project, user).execute(issue) + described_class.new(project: project, current_user: user).execute(issue) end context 'issue is incident type' do let(:issue) { create(:incident, :closed, project: project) } let(:current_user) { user } - subject { described_class.new(project, user).execute(issue) } + subject { described_class.new(project: project, current_user: user).execute(issue) } it_behaves_like 'an incident management tracked event', :incident_management_incident_reopened end @@ -67,7 +67,7 @@ RSpec.describe Issues::ReopenService do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :issue_hooks) - described_class.new(project, user).execute(issue) + described_class.new(project: project, current_user: user).execute(issue) end end @@ -78,7 +78,7 @@ RSpec.describe Issues::ReopenService do expect(project).to receive(:execute_hooks).with(an_instance_of(Hash), :confidential_issue_hooks) expect(project).to receive(:execute_services).with(an_instance_of(Hash), :confidential_issue_hooks) - described_class.new(project, user).execute(issue) + described_class.new(project: project, current_user: user).execute(issue) end end end diff --git a/spec/services/issues/reorder_service_spec.rb b/spec/services/issues/reorder_service_spec.rb index 78b937a1caf..15668a3aa23 100644 --- a/spec/services/issues/reorder_service_spec.rb +++ b/spec/services/issues/reorder_service_spec.rb @@ -75,7 +75,7 @@ RSpec.describe Issues::ReorderService do match_params = { move_between_ids: [issue2.id, issue3.id], board_group_id: group.id } expect(Issues::UpdateService) - .to receive(:new).with(project, user, match_params) + .to receive(:new).with(project: project, current_user: user, params: match_params) .and_return(double(execute: build(:issue))) subject.execute(issue1) @@ -95,6 +95,6 @@ RSpec.describe Issues::ReorderService do end def service(params) - described_class.new(project, user, params) + described_class.new(project: project, current_user: user, params: params) end end diff --git a/spec/services/issues/resolve_discussions_spec.rb b/spec/services/issues/resolve_discussions_spec.rb index 927dd7ae3e7..1ac71b966bc 100644 --- a/spec/services/issues/resolve_discussions_spec.rb +++ b/spec/services/issues/resolve_discussions_spec.rb @@ -11,7 +11,7 @@ RSpec.describe Issues::ResolveDiscussions do DummyService.class_eval do include ::Issues::ResolveDiscussions - def initialize(*args) + def initialize(project:, current_user: nil, params: {}) super filter_resolve_discussion_params end @@ -26,7 +26,7 @@ RSpec.describe Issues::ResolveDiscussions do let(:other_merge_request) { create(:merge_request, source_project: project, source_branch: "fix") } describe "#merge_request_for_resolving_discussion" do - let(:service) { DummyService.new(project, user, merge_request_to_resolve_discussions_of: merge_request.iid) } + let(:service) { DummyService.new(project: project, current_user: user, params: { merge_request_to_resolve_discussions_of: merge_request.iid }) } it "finds the merge request" do expect(service.merge_request_to_resolve_discussions_of).to eq(merge_request) @@ -45,10 +45,12 @@ RSpec.describe Issues::ResolveDiscussions do describe "#discussions_to_resolve" do it "contains a single discussion when matching merge request and discussion are passed" do service = DummyService.new( - project, - user, - discussion_to_resolve: discussion.id, - merge_request_to_resolve_discussions_of: merge_request.iid + project: project, + current_user: user, + params: { + discussion_to_resolve: discussion.id, + merge_request_to_resolve_discussions_of: merge_request.iid + } ) # We need to compare discussion id's because the Discussion-objects are rebuilt # which causes the object-id's not to be different. @@ -63,9 +65,9 @@ RSpec.describe Issues::ResolveDiscussions do project: merge_request.target_project, line_number: 15)]) service = DummyService.new( - project, - user, - merge_request_to_resolve_discussions_of: merge_request.iid + project: project, + current_user: user, + params: { merge_request_to_resolve_discussions_of: merge_request.iid } ) # We need to compare discussion id's because the Discussion-objects are rebuilt # which causes the object-id's not to be different. @@ -81,9 +83,9 @@ RSpec.describe Issues::ResolveDiscussions do line_number: 15 )]) service = DummyService.new( - project, - user, - merge_request_to_resolve_discussions_of: merge_request.iid + project: project, + current_user: user, + params: { merge_request_to_resolve_discussions_of: merge_request.iid } ) # We need to compare discussion id's because the Discussion-objects are rebuilt # which causes the object-id's not to be different. @@ -94,10 +96,12 @@ RSpec.describe Issues::ResolveDiscussions do it "is empty when a discussion and another merge request are passed" do service = DummyService.new( - project, - user, - discussion_to_resolve: discussion.id, - merge_request_to_resolve_discussions_of: other_merge_request.iid + project: project, + current_user: user, + params: { + discussion_to_resolve: discussion.id, + merge_request_to_resolve_discussions_of: other_merge_request.iid + } ) expect(service.discussions_to_resolve).to be_empty diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 5d716d2a53e..2bb62f49bd0 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -41,7 +41,7 @@ RSpec.describe Issues::UpdateService, :mailer do end def update_issue(opts) - described_class.new(project, user, opts).execute(issue) + described_class.new(project: project, current_user: user, params: opts).execute(issue) end context 'valid params' do @@ -269,7 +269,7 @@ RSpec.describe Issues::UpdateService, :mailer do opts[:move_between_ids] = [issue_1.id, issue_2.id] opts[:board_group_id] = group.id - described_class.new(issue_3.project, user, opts).execute(issue_3) + described_class.new(project: issue_3.project, current_user: user, params: opts).execute(issue_3) expect(issue_2.relative_position).to be_between(issue_1.relative_position, issue_2.relative_position) end end @@ -283,9 +283,7 @@ RSpec.describe Issues::UpdateService, :mailer do it 'filters out params that cannot be set without the :admin_issue permission' do described_class.new( - project, - guest, - opts.merge( + project: project, current_user: guest, params: opts.merge( confidential: true, issue_type: 'test_case' ) @@ -658,7 +656,7 @@ RSpec.describe Issues::UpdateService, :mailer do opts = { label_ids: [label.id] } perform_enqueued_jobs do - @issue = described_class.new(project, user, opts).execute(issue) + @issue = described_class.new(project: project, current_user: user, params: opts).execute(issue) end should_email(subscriber) @@ -674,7 +672,7 @@ RSpec.describe Issues::UpdateService, :mailer do opts = { label_ids: [label.id, label2.id] } perform_enqueued_jobs do - @issue = described_class.new(project, user, opts).execute(issue) + @issue = described_class.new(project: project, current_user: user, params: opts).execute(issue) end should_not_email(subscriber) @@ -685,7 +683,7 @@ RSpec.describe Issues::UpdateService, :mailer do opts = { label_ids: [label2.id] } perform_enqueued_jobs do - @issue = described_class.new(project, user, opts).execute(issue) + @issue = described_class.new(project: project, current_user: user, params: opts).execute(issue) end should_not_email(subscriber) @@ -717,7 +715,7 @@ RSpec.describe Issues::UpdateService, :mailer do line_number: 1 } } - service = described_class.new(project, user, params) + service = described_class.new(project: project, current_user: user, params: params) expect(Spam::SpamActionService).not_to receive(:new) @@ -793,7 +791,7 @@ RSpec.describe Issues::UpdateService, :mailer do context 'updating labels' do let(:label3) { create(:label, project: project) } - let(:result) { described_class.new(project, user, params).execute(issue).reload } + let(:result) { described_class.new(project: project, current_user: user, params: params).execute(issue).reload } context 'when add_label_ids and label_ids are passed' do let(:params) { { label_ids: [label.id], add_label_ids: [label3.id] } } @@ -991,14 +989,14 @@ RSpec.describe Issues::UpdateService, :mailer do it 'raises an error for invalid move ids within a project' do opts = { move_between_ids: [9000, non_existing_record_id] } - expect { described_class.new(issue.project, user, opts).execute(issue) } + expect { described_class.new(project: issue.project, current_user: user, params: opts).execute(issue) } .to raise_error(ActiveRecord::RecordNotFound) end it 'raises an error for invalid move ids within a group' do opts = { move_between_ids: [9000, non_existing_record_id], board_group_id: create(:group).id } - expect { described_class.new(issue.project, user, opts).execute(issue) } + expect { described_class.new(project: issue.project, current_user: user, params: opts).execute(issue) } .to raise_error(ActiveRecord::RecordNotFound) end end @@ -1040,7 +1038,7 @@ RSpec.describe Issues::UpdateService, :mailer do it_behaves_like 'issuable record that supports quick actions' do let(:existing_issue) { create(:issue, project: project) } - let(:issuable) { described_class.new(project, user, params).execute(existing_issue) } + let(:issuable) { described_class.new(project: project, current_user: user, params: params).execute(existing_issue) } end end end diff --git a/spec/services/issues/zoom_link_service_spec.rb b/spec/services/issues/zoom_link_service_spec.rb index 8e8adc516cf..19db892fcae 100644 --- a/spec/services/issues/zoom_link_service_spec.rb +++ b/spec/services/issues/zoom_link_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Issues::ZoomLinkService do let_it_be(:issue) { create(:issue) } let(:project) { issue.project } - let(:service) { described_class.new(issue, user) } + let(:service) { described_class.new(project: project, current_user: user, params: { issue: issue }) } let(:zoom_link) { 'https://zoom.us/j/123456789' } before do diff --git a/spec/services/merge_requests/add_context_service_spec.rb b/spec/services/merge_requests/add_context_service_spec.rb index 27b46a9023c..448be27efe8 100644 --- a/spec/services/merge_requests/add_context_service_spec.rb +++ b/spec/services/merge_requests/add_context_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe MergeRequests::AddContextService do let(:commits) { ["874797c3a73b60d2187ed6e2fcabd289ff75171e"] } let(:raw_repository) { project.repository.raw } - subject(:service) { described_class.new(project, admin, merge_request: merge_request, commits: commits) } + subject(:service) { described_class.new(project: project, current_user: admin, params: { merge_request: merge_request, commits: commits }) } describe "#execute" do context "when admin mode is enabled", :enable_admin_mode do @@ -32,7 +32,7 @@ RSpec.describe MergeRequests::AddContextService do let(:user) { create(:user) } let(:merge_request1) { create(:merge_request, source_project: project, author: user) } - subject(:service) { described_class.new(project, user, merge_request: merge_request, commits: commits) } + subject(:service) { described_class.new(project: project, current_user: user, params: { merge_request: merge_request, commits: commits }) } it "doesn't add context commit" do subject.execute @@ -42,7 +42,7 @@ RSpec.describe MergeRequests::AddContextService do end context "when the commits array is empty" do - subject(:service) { described_class.new(project, admin, merge_request: merge_request, commits: []) } + subject(:service) { described_class.new(project: project, current_user: admin, params: { merge_request: merge_request, commits: [] }) } it "doesn't add context commit" do subject.execute diff --git a/spec/services/merge_requests/add_spent_time_service_spec.rb b/spec/services/merge_requests/add_spent_time_service_spec.rb index 42c692eb128..db3380e9582 100644 --- a/spec/services/merge_requests/add_spent_time_service_spec.rb +++ b/spec/services/merge_requests/add_spent_time_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe MergeRequests::AddSpentTimeService do let(:duration) { 1500 } let(:params) { { spend_time: { duration: duration, user_id: user.id } } } - let(:service) { described_class.new(project, user, params) } + let(:service) { described_class.new(project: project, current_user: user, params: params) } describe '#execute' do before do @@ -44,7 +44,7 @@ RSpec.describe MergeRequests::AddSpentTimeService do it 'is more efficient than using the full update-service' do other_mr = create(:merge_request, :simple, :unique_branches, source_project: project) - update_service = ::MergeRequests::UpdateService.new(project, user, params) + update_service = ::MergeRequests::UpdateService.new(project: project, current_user: user, params: params) other_mr.reload expect { service.execute(merge_request) } diff --git a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb index 6edaa91b8b2..8d1abe5ea89 100644 --- a/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb +++ b/spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe ::MergeRequests::AddTodoWhenBuildFailsService do let(:ref) { merge_request.source_branch } let(:service) do - described_class.new(project, user, commit_message: 'Awesome message') + described_class.new(project: project, current_user: user, params: { commit_message: 'Awesome message' }) end let(:todo_service) { spy('todo service') } diff --git a/spec/services/merge_requests/after_create_service_spec.rb b/spec/services/merge_requests/after_create_service_spec.rb index e1f28e32164..cbbd193a411 100644 --- a/spec/services/merge_requests/after_create_service_spec.rb +++ b/spec/services/merge_requests/after_create_service_spec.rb @@ -6,7 +6,7 @@ RSpec.describe MergeRequests::AfterCreateService do let_it_be(:merge_request) { create(:merge_request) } subject(:after_create_service) do - described_class.new(merge_request.target_project, merge_request.author) + described_class.new(project: merge_request.target_project, current_user: merge_request.author) end describe '#execute' do @@ -191,7 +191,7 @@ RSpec.describe MergeRequests::AfterCreateService do it 'calls MergeRequests::LinkLfsObjectsService#execute' do service = instance_spy(MergeRequests::LinkLfsObjectsService) - allow(MergeRequests::LinkLfsObjectsService).to receive(:new).with(merge_request.target_project).and_return(service) + allow(MergeRequests::LinkLfsObjectsService).to receive(:new).with(project: merge_request.target_project).and_return(service) execute_service diff --git a/spec/services/merge_requests/approval_service_spec.rb b/spec/services/merge_requests/approval_service_spec.rb index df9a98c5540..d30b2721a36 100644 --- a/spec/services/merge_requests/approval_service_spec.rb +++ b/spec/services/merge_requests/approval_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe MergeRequests::ApprovalService do let(:project) { merge_request.project } let!(:todo) { create(:todo, user: user, project: project, target: merge_request) } - subject(:service) { described_class.new(project, user) } + subject(:service) { described_class.new(project: project, current_user: user) } before do project.add_developer(user) diff --git a/spec/services/merge_requests/assign_issues_service_spec.rb b/spec/services/merge_requests/assign_issues_service_spec.rb index 6398e8c533e..b857f26c052 100644 --- a/spec/services/merge_requests/assign_issues_service_spec.rb +++ b/spec/services/merge_requests/assign_issues_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe MergeRequests::AssignIssuesService do let(:project) { create(:project, :public, :repository) } let(:issue) { create(:issue, project: project) } let(:merge_request) { create(:merge_request, :simple, source_project: project, author: user, description: "fixes #{issue.to_reference}") } - let(:service) { described_class.new(project, user, merge_request: merge_request) } + let(:service) { described_class.new(project: project, current_user: user, params: { merge_request: merge_request }) } before do project.add_developer(user) @@ -37,10 +37,12 @@ RSpec.describe MergeRequests::AssignIssuesService do it 'accepts precomputed data for closes_issues' do issue2 = create(:issue, project: project) - service2 = described_class.new(project, - user, - merge_request: merge_request, - closes_issues: [issue, issue2]) + service2 = described_class.new(project: project, + current_user: user, + params: { + merge_request: merge_request, + closes_issues: [issue, issue2] + }) expect(service2.assignable_issues.count).to eq 2 end @@ -52,10 +54,12 @@ RSpec.describe MergeRequests::AssignIssuesService do it 'ignores external issues' do external_issue = ExternalIssue.new('JIRA-123', project) service = described_class.new( - project, - user, - merge_request: merge_request, - closes_issues: [external_issue] + project: project, + current_user: user, + params: { + merge_request: merge_request, + closes_issues: [external_issue] + } ) expect(service.assignable_issues.count).to eq 0 diff --git a/spec/services/merge_requests/base_service_spec.rb b/spec/services/merge_requests/base_service_spec.rb index d8ba2bc43fb..7911392ef19 100644 --- a/spec/services/merge_requests/base_service_spec.rb +++ b/spec/services/merge_requests/base_service_spec.rb @@ -17,7 +17,7 @@ RSpec.describe MergeRequests::BaseService do } end - subject { MergeRequests::CreateService.new(project, project.owner, params) } + subject { MergeRequests::CreateService.new(project: project, current_user: project.owner, params: params) } describe '#execute_hooks' do shared_examples 'enqueues Jira sync worker' do diff --git a/spec/services/merge_requests/build_service_spec.rb b/spec/services/merge_requests/build_service_spec.rb index 8adf6d69f73..5a6a9df3f44 100644 --- a/spec/services/merge_requests/build_service_spec.rb +++ b/spec/services/merge_requests/build_service_spec.rb @@ -49,7 +49,7 @@ RSpec.describe MergeRequests::BuildService do end let(:service) do - described_class.new(project, user, params) + described_class.new(project: project, current_user: user, params: params) end before do @@ -100,7 +100,7 @@ RSpec.describe MergeRequests::BuildService do context 'with force_remove_source_branch parameter when the user is authorized' do let(:mr_params) { params.merge(force_remove_source_branch: '1') } let(:source_project) { fork_project(project, user) } - let(:merge_request) { described_class.new(project, user, mr_params).execute } + let(:merge_request) { described_class.new(project: project, current_user: user, params: mr_params).execute } before do project.add_reporter(user) diff --git a/spec/services/merge_requests/cleanup_refs_service_spec.rb b/spec/services/merge_requests/cleanup_refs_service_spec.rb index a1822a4d5ba..e8690ae5bf2 100644 --- a/spec/services/merge_requests/cleanup_refs_service_spec.rb +++ b/spec/services/merge_requests/cleanup_refs_service_spec.rb @@ -54,7 +54,7 @@ RSpec.describe MergeRequests::CleanupRefsService do context 'when merge request has merge ref' do before do MergeRequests::MergeToRefService - .new(merge_request.project, merge_request.author) + .new(project: merge_request.project, current_user: merge_request.author) .execute(merge_request) end diff --git a/spec/services/merge_requests/close_service_spec.rb b/spec/services/merge_requests/close_service_spec.rb index 48f56b3ec68..f6336a85a25 100644 --- a/spec/services/merge_requests/close_service_spec.rb +++ b/spec/services/merge_requests/close_service_spec.rb @@ -21,7 +21,7 @@ RSpec.describe MergeRequests::CloseService do it_behaves_like 'merge request reviewers cache counters invalidator' context 'valid params' do - let(:service) { described_class.new(project, user, {}) } + let(:service) { described_class.new(project: project, current_user: user) } before do allow(service).to receive(:execute_hooks) @@ -73,7 +73,7 @@ RSpec.describe MergeRequests::CloseService do expect(metrics_service).to receive(:close) - described_class.new(project, user, {}).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end it 'calls the merge request activity counter' do @@ -81,11 +81,11 @@ RSpec.describe MergeRequests::CloseService do .to receive(:track_close_mr_action) .with(user: user) - described_class.new(project, user, {}).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do - service = described_class.new(project, user, {}) + service = described_class.new(project: project, current_user: user) expect { service.execute(merge_request) } .to change { project.open_merge_requests_count }.from(1).to(0) @@ -96,19 +96,19 @@ RSpec.describe MergeRequests::CloseService do expect(service).to receive(:execute_for_merge_request).with(merge_request) end - described_class.new(project, user).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end it 'schedules CleanupRefsService' do expect(MergeRequests::CleanupRefsService).to receive(:schedule).with(merge_request) - described_class.new(project, user).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end context 'current user is not authorized to close merge request' do before do perform_enqueued_jobs do - @merge_request = described_class.new(project, guest).execute(merge_request) + @merge_request = described_class.new(project: project, current_user: guest).execute(merge_request) end end diff --git a/spec/services/merge_requests/create_from_issue_service_spec.rb b/spec/services/merge_requests/create_from_issue_service_spec.rb index 6528edfc8b7..749b30bff5f 100644 --- a/spec/services/merge_requests/create_from_issue_service_spec.rb +++ b/spec/services/merge_requests/create_from_issue_service_spec.rb @@ -11,8 +11,8 @@ RSpec.describe MergeRequests::CreateFromIssueService do let(:milestone_id) { create(:milestone, project: project).id } let(:issue) { create(:issue, project: project, milestone_id: milestone_id) } let(:custom_source_branch) { 'custom-source-branch' } - let(:service) { described_class.new(project, user, service_params) } - let(:service_with_custom_source_branch) { described_class.new(project, user, branch_name: custom_source_branch, **service_params) } + let(:service) { described_class.new(project: project, current_user: user, mr_params: service_params) } + let(:service_with_custom_source_branch) { described_class.new(project: project, current_user: user, mr_params: { branch_name: custom_source_branch, **service_params }) } before do project.add_developer(user) @@ -21,14 +21,14 @@ RSpec.describe MergeRequests::CreateFromIssueService do describe '#execute' do shared_examples_for 'a service that creates a merge request from an issue' do it 'returns an error when user can not create merge request on target project' do - result = described_class.new(project, create(:user), service_params).execute + result = described_class.new(project: project, current_user: create(:user), mr_params: service_params).execute expect(result[:status]).to eq(:error) expect(result[:message]).to eq('Not allowed to create merge request') end it 'returns an error with invalid issue iid' do - result = described_class.new(project, user, issue_iid: -1).execute + result = described_class.new(project: project, current_user: user, mr_params: { issue_iid: -1 }).execute expect(result[:status]).to eq(:error) expect(result[:message]).to eq('Invalid issue iid') @@ -123,7 +123,7 @@ RSpec.describe MergeRequests::CreateFromIssueService do end context 'when ref branch is set', :sidekiq_might_not_need_inline do - subject { described_class.new(project, user, ref: 'feature', **service_params).execute } + subject { described_class.new(project: project, current_user: user, mr_params: { ref: 'feature', **service_params }).execute } it 'sets the merge request source branch to the new issue branch' do expect(subject[:merge_request].source_branch).to eq(issue.to_branch_name) @@ -134,7 +134,7 @@ RSpec.describe MergeRequests::CreateFromIssueService do end context 'when the ref is a tag' do - subject { described_class.new(project, user, ref: 'v1.0.0', **service_params).execute } + subject { described_class.new(project: project, current_user: user, mr_params: { ref: 'v1.0.0', **service_params }).execute } it 'sets the merge request source branch to the new issue branch' do expect(subject[:merge_request].source_branch).to eq(issue.to_branch_name) @@ -150,7 +150,7 @@ RSpec.describe MergeRequests::CreateFromIssueService do end context 'when ref branch does not exist' do - subject { described_class.new(project, user, ref: 'no-such-branch', **service_params).execute } + subject { described_class.new(project: project, current_user: user, mr_params: { ref: 'no-such-branch', **service_params }).execute } it 'creates a merge request' do expect { subject }.to change(target_project.merge_requests, :count).by(1) diff --git a/spec/services/merge_requests/create_pipeline_service_spec.rb b/spec/services/merge_requests/create_pipeline_service_spec.rb index 3e2e940dc24..a0ac168f3d7 100644 --- a/spec/services/merge_requests/create_pipeline_service_spec.rb +++ b/spec/services/merge_requests/create_pipeline_service_spec.rb @@ -8,7 +8,7 @@ RSpec.describe MergeRequests::CreatePipelineService do let_it_be(:project, reload: true) { create(:project, :repository) } let_it_be(:user) { create(:user) } - let(:service) { described_class.new(project, actor, params) } + let(:service) { described_class.new(project: project, current_user: actor, params: params) } let(:actor) { user } let(:params) { {} } diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb index f2bc55103f0..b2351ab53bd 100644 --- a/spec/services/merge_requests/create_service_spec.rb +++ b/spec/services/merge_requests/create_service_spec.rb @@ -21,7 +21,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do } end - let(:service) { described_class.new(project, user, opts) } + let(:service) { described_class.new(project: project, current_user: user, params: opts) } let(:merge_request) { service.execute } before do @@ -347,12 +347,12 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do } end - let(:issuable) { described_class.new(project, user, params).execute } + let(:issuable) { described_class.new(project: project, current_user: user, params: params).execute } end context 'Quick actions' do context 'with assignee and milestone in params and command' do - let(:merge_request) { described_class.new(project, user, opts).execute } + let(:merge_request) { described_class.new(project: project, current_user: user, params: opts).execute } let(:milestone) { create(:milestone, project: project) } let(:opts) do @@ -390,7 +390,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it 'removes assignee_id when user id is invalid' do opts = { title: 'Title', description: 'Description', assignee_ids: [-1] } - merge_request = described_class.new(project, user, opts).execute + merge_request = described_class.new(project: project, current_user: user, params: opts).execute expect(merge_request.assignee_ids).to be_empty end @@ -398,7 +398,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it 'removes assignee_id when user id is 0' do opts = { title: 'Title', description: 'Description', assignee_ids: [0] } - merge_request = described_class.new(project, user, opts).execute + merge_request = described_class.new(project: project, current_user: user, params: opts).execute expect(merge_request.assignee_ids).to be_empty end @@ -407,7 +407,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do project.add_maintainer(user2) opts = { title: 'Title', description: 'Description', assignee_ids: [user2.id] } - merge_request = described_class.new(project, user, opts).execute + merge_request = described_class.new(project: project, current_user: user, params: opts).execute expect(merge_request.assignees).to eq([user2]) end @@ -426,7 +426,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it 'invalidates open merge request counter for assignees when merge request is assigned' do project.add_maintainer(user2) - described_class.new(project, user, opts).execute + described_class.new(project: project, current_user: user, params: opts).execute expect(user2.assigned_open_merge_requests_count).to eq 1 end @@ -445,7 +445,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do project.update!(visibility_level: level) opts = { title: 'Title', description: 'Description', assignee_ids: [user2.id] } - merge_request = described_class.new(project, user, opts).execute + merge_request = described_class.new(project: project, current_user: user, params: opts).execute expect(merge_request.assignee_id).to be_nil end @@ -473,7 +473,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end it 'raises an error' do - expect { described_class.new(project, user, opts).execute } + expect { described_class.new(project: project, current_user: user, params: opts).execute } .to raise_error Gitlab::Access::AccessDeniedError end end @@ -485,7 +485,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end it 'raises an error' do - expect { described_class.new(project, user, opts).execute } + expect { described_class.new(project: project, current_user: user, params: opts).execute } .to raise_error Gitlab::Access::AccessDeniedError end end @@ -497,7 +497,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end it 'creates the merge request', :sidekiq_might_not_need_inline do - merge_request = described_class.new(project, user, opts).execute + merge_request = described_class.new(project: project, current_user: user, params: opts).execute expect(merge_request).to be_persisted end @@ -505,7 +505,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do it 'does not create the merge request when the target project is archived' do target_project.update!(archived: true) - expect { described_class.new(project, user, opts).execute } + expect { described_class.new(project: project, current_user: user, params: opts).execute } .to raise_error Gitlab::Access::AccessDeniedError end end @@ -529,7 +529,7 @@ RSpec.describe MergeRequests::CreateService, :clean_gitlab_redis_shared_state do end it 'ignores source_project_id' do - merge_request = described_class.new(project, user, opts).execute + merge_request = described_class.new(project: project, current_user: user, params: opts).execute expect(merge_request.source_project_id).to eq(project.id) end diff --git a/spec/services/merge_requests/ff_merge_service_spec.rb b/spec/services/merge_requests/ff_merge_service_spec.rb index aec5a3b3fa3..24a1a8b3113 100644 --- a/spec/services/merge_requests/ff_merge_service_spec.rb +++ b/spec/services/merge_requests/ff_merge_service_spec.rb @@ -23,7 +23,7 @@ RSpec.describe MergeRequests::FfMergeService do describe '#execute' do context 'valid params' do - let(:service) { described_class.new(project, user, valid_merge_params) } + let(:service) { described_class.new(project: project, current_user: user, params: valid_merge_params) } def execute_ff_merge perform_enqueued_jobs do @@ -92,7 +92,7 @@ RSpec.describe MergeRequests::FfMergeService do end context 'error handling' do - let(:service) { described_class.new(project, user, valid_merge_params.merge(commit_message: 'Awesome message')) } + let(:service) { described_class.new(project: project, current_user: user, params: valid_merge_params.merge(commit_message: 'Awesome message')) } before do allow(Gitlab::AppLogger).to receive(:error) diff --git a/spec/services/merge_requests/get_urls_service_spec.rb b/spec/services/merge_requests/get_urls_service_spec.rb index 053752626dc..5f81e1728fa 100644 --- a/spec/services/merge_requests/get_urls_service_spec.rb +++ b/spec/services/merge_requests/get_urls_service_spec.rb @@ -6,7 +6,7 @@ RSpec.describe MergeRequests::GetUrlsService do include ProjectForksHelper let(:project) { create(:project, :public, :repository) } - let(:service) { described_class.new(project) } + let(:service) { described_class.new(project: project) } let(:source_branch) { "merge-test" } let(:new_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/-/merge_requests/new?merge_request%5Bsource_branch%5D=#{source_branch}" } let(:show_merge_request_url) { "http://#{Gitlab.config.gitlab.host}/#{project.full_path}/-/merge_requests/#{merge_request.iid}" } @@ -106,7 +106,7 @@ RSpec.describe MergeRequests::GetUrlsService do let!(:merge_request) { create(:merge_request, source_project: forked_project, target_project: project, source_branch: source_branch) } let(:changes) { existing_branch_changes } # Source project is now the forked one - let(:service) { described_class.new(forked_project) } + let(:service) { described_class.new(project: forked_project) } before do allow(forked_project).to receive(:empty_repo?).and_return(false) diff --git a/spec/services/merge_requests/handle_assignees_change_service_spec.rb b/spec/services/merge_requests/handle_assignees_change_service_spec.rb index cc595aab04b..cb4bda0442e 100644 --- a/spec/services/merge_requests/handle_assignees_change_service_spec.rb +++ b/spec/services/merge_requests/handle_assignees_change_service_spec.rb @@ -10,7 +10,7 @@ RSpec.describe MergeRequests::HandleAssigneesChangeService do let_it_be(:old_assignees) { create_list(:user, 3) } let(:options) { {} } - let(:service) { described_class.new(project, user) } + let(:service) { described_class.new(project: project, current_user: user) } before_all do project.add_maintainer(user) diff --git a/spec/services/merge_requests/link_lfs_objects_service_spec.rb b/spec/services/merge_requests/link_lfs_objects_service_spec.rb index c1765e3a2ab..2fb6bbaf02f 100644 --- a/spec/services/merge_requests/link_lfs_objects_service_spec.rb +++ b/spec/services/merge_requests/link_lfs_objects_service_spec.rb @@ -18,7 +18,7 @@ RSpec.describe MergeRequests::LinkLfsObjectsService, :sidekiq_inline do ) end - subject { described_class.new(target_project) } + subject { described_class.new(project: target_project) } shared_examples_for 'linking LFS objects' do context 'when source project is the same as target project' do diff --git a/spec/services/merge_requests/mark_reviewer_reviewed_service_spec.rb b/spec/services/merge_requests/mark_reviewer_reviewed_service_spec.rb index 1075f6f9034..4d7bd3d8800 100644 --- a/spec/services/merge_requests/mark_reviewer_reviewed_service_spec.rb +++ b/spec/services/merge_requests/mark_reviewer_reviewed_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe MergeRequests::MarkReviewerReviewedService do let(:merge_request) { create(:merge_request, reviewers: [current_user]) } let(:reviewer) { merge_request.merge_request_reviewers.find_by(user_id: current_user.id) } let(:project) { merge_request.project } - let(:service) { described_class.new(project, current_user) } + let(:service) { described_class.new(project: project, current_user: current_user) } let(:result) { service.execute(merge_request) } before do @@ -16,7 +16,7 @@ RSpec.describe MergeRequests::MarkReviewerReviewedService do describe '#execute' do describe 'invalid permissions' do - let(:service) { described_class.new(project, create(:user)) } + let(:service) { described_class.new(project: project, current_user: create(:user)) } it 'returns an error' do expect(result[:status]).to eq :error @@ -24,7 +24,7 @@ RSpec.describe MergeRequests::MarkReviewerReviewedService do end describe 'reviewer does not exist' do - let(:service) { described_class.new(project, create(:user)) } + let(:service) { described_class.new(project: project, current_user: create(:user)) } it 'returns an error' do expect(result[:status]).to eq :error diff --git a/spec/services/merge_requests/merge_service_spec.rb b/spec/services/merge_requests/merge_service_spec.rb index df7d9bbf13d..ac39fb59c62 100644 --- a/spec/services/merge_requests/merge_service_spec.rb +++ b/spec/services/merge_requests/merge_service_spec.rb @@ -17,7 +17,7 @@ RSpec.describe MergeRequests::MergeService do end describe '#execute' do - let(:service) { described_class.new(project, user, merge_params) } + let(:service) { described_class.new(project: project, current_user: user, params: merge_params) } let(:merge_params) do { commit_message: 'Awesome message', sha: merge_request.diff_head_sha } end @@ -228,7 +228,7 @@ RSpec.describe MergeRequests::MergeService do context 'source branch removal' do context 'when the source branch is protected' do let(:service) do - described_class.new(project, user, merge_params.merge('should_remove_source_branch' => true)) + described_class.new(project: project, current_user: user, params: merge_params.merge('should_remove_source_branch' => true)) end before do @@ -244,7 +244,7 @@ RSpec.describe MergeRequests::MergeService do context 'when the source branch is the default branch' do let(:service) do - described_class.new(project, user, merge_params.merge('should_remove_source_branch' => true)) + described_class.new(project: project, current_user: user, params: merge_params.merge('should_remove_source_branch' => true)) end before do @@ -270,7 +270,7 @@ RSpec.describe MergeRequests::MergeService do end context 'when the merger set the source branch not to be removed' do - let(:service) { described_class.new(project, user, merge_params.merge('should_remove_source_branch' => false)) } + let(:service) { described_class.new(project: project, current_user: user, params: merge_params.merge('should_remove_source_branch' => false)) } it 'does not delete the source branch' do expect(::MergeRequests::DeleteSourceBranchWorker).not_to receive(:perform_async) @@ -282,7 +282,7 @@ RSpec.describe MergeRequests::MergeService do context 'when MR merger set the source branch to be removed' do let(:service) do - described_class.new(project, user, merge_params.merge('should_remove_source_branch' => true)) + described_class.new(project: project, current_user: user, params: merge_params.merge('should_remove_source_branch' => true)) end it 'removes the source branch using the current user' do @@ -330,7 +330,7 @@ RSpec.describe MergeRequests::MergeService do unauthorized_user = create(:user) project.add_reporter(unauthorized_user) - service = described_class.new(project, unauthorized_user) + service = described_class.new(project: project, current_user: unauthorized_user) service.execute(merge_request) diff --git a/spec/services/merge_requests/merge_to_ref_service_spec.rb b/spec/services/merge_requests/merge_to_ref_service_spec.rb index 01dce0dfbce..bb764ff5672 100644 --- a/spec/services/merge_requests/merge_to_ref_service_spec.rb +++ b/spec/services/merge_requests/merge_to_ref_service_spec.rb @@ -74,7 +74,7 @@ RSpec.describe MergeRequests::MergeToRefService do describe '#execute' do let(:service) do - described_class.new(project, user, **params) + described_class.new(project: project, current_user: user, params: params) end let(:params) { { commit_message: 'Awesome message', should_remove_source_branch: true, sha: merge_request.diff_head_sha } } @@ -111,11 +111,11 @@ RSpec.describe MergeRequests::MergeToRefService do end let(:merge_ref_service) do - described_class.new(project, user, {}) + described_class.new(project: project, current_user: user) end let(:merge_service) do - MergeRequests::MergeService.new(project, user, { sha: merge_request.diff_head_sha }) + MergeRequests::MergeService.new(project: project, current_user: user, params: { sha: merge_request.diff_head_sha }) end context 'when merge commit' do diff --git a/spec/services/merge_requests/mergeability_check_service_spec.rb b/spec/services/merge_requests/mergeability_check_service_spec.rb index 32d7991e593..65599b7e046 100644 --- a/spec/services/merge_requests/mergeability_check_service_spec.rb +++ b/spec/services/merge_requests/mergeability_check_service_spec.rb @@ -232,7 +232,7 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar context 'when MR cannot be merged and has outdated merge ref' do before do - MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) + MergeRequests::MergeToRefService.new(project: project, current_user: merge_request.author).execute(merge_request) merge_request.mark_as_unmergeable! end @@ -332,7 +332,7 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar context 'when MR is mergeable but merge-ref is already updated' do before do - MergeRequests::MergeToRefService.new(project, merge_request.author).execute(merge_request) + MergeRequests::MergeToRefService.new(project: project, current_user: merge_request.author).execute(merge_request) merge_request.mark_as_mergeable! end @@ -361,7 +361,7 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar context 'merge with conflicts' do it 'calls MergeToRefService with true allow_conflicts param' do expect(MergeRequests::MergeToRefService).to receive(:new) - .with(project, merge_request.author, { allow_conflicts: true }).and_call_original + .with(project: project, current_user: merge_request.author, params: { allow_conflicts: true }).and_call_original subject end @@ -373,7 +373,7 @@ RSpec.describe MergeRequests::MergeabilityCheckService, :clean_gitlab_redis_shar it 'calls MergeToRefService with false allow_conflicts param' do expect(MergeRequests::MergeToRefService).to receive(:new) - .with(project, merge_request.author, { allow_conflicts: false }).and_call_original + .with(project: project, current_user: merge_request.author, params: { allow_conflicts: false }).and_call_original subject end diff --git a/spec/services/merge_requests/post_merge_service_spec.rb b/spec/services/merge_requests/post_merge_service_spec.rb index cc347904f49..14804aa33d4 100644 --- a/spec/services/merge_requests/post_merge_service_spec.rb +++ b/spec/services/merge_requests/post_merge_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe MergeRequests::PostMergeService do let_it_be(:merge_request, reload: true) { create(:merge_request, assignees: [user]) } let_it_be(:project) { merge_request.project } - subject { described_class.new(project, user).execute(merge_request) } + subject { described_class.new(project: project, current_user: user).execute(merge_request) } before do project.add_maintainer(user) diff --git a/spec/services/merge_requests/push_options_handler_service_spec.rb b/spec/services/merge_requests/push_options_handler_service_spec.rb index b5086ea3a82..87c3fc6a2d8 100644 --- a/spec/services/merge_requests/push_options_handler_service_spec.rb +++ b/spec/services/merge_requests/push_options_handler_service_spec.rb @@ -11,7 +11,7 @@ RSpec.describe MergeRequests::PushOptionsHandlerService do let_it_be(:user3) { create(:user, developer_projects: [project]) } let_it_be(:forked_project) { fork_project(project, user1, repository: true) } - let(:service) { described_class.new(project, user1, changes, push_options) } + let(:service) { described_class.new(project: project, current_user: user1, changes: changes, push_options: push_options) } let(:source_branch) { 'fix' } let(:target_branch) { 'feature' } let(:title) { 'my title' } diff --git a/spec/services/merge_requests/pushed_branches_service_spec.rb b/spec/services/merge_requests/pushed_branches_service_spec.rb index cd6af4c275e..59424263ec5 100644 --- a/spec/services/merge_requests/pushed_branches_service_spec.rb +++ b/spec/services/merge_requests/pushed_branches_service_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' RSpec.describe MergeRequests::PushedBranchesService do let(:project) { create(:project) } - let!(:service) { described_class.new(project, nil, changes: pushed_branches) } + let!(:service) { described_class.new(project: project, current_user: nil, params: { changes: pushed_branches }) } context 'when branches pushed' do let(:pushed_branches) do diff --git a/spec/services/merge_requests/rebase_service_spec.rb b/spec/services/merge_requests/rebase_service_spec.rb index 653fcf12a76..a46f3cf6148 100644 --- a/spec/services/merge_requests/rebase_service_spec.rb +++ b/spec/services/merge_requests/rebase_service_spec.rb @@ -18,7 +18,7 @@ RSpec.describe MergeRequests::RebaseService do let(:repository) { project.repository.raw } let(:skip_ci) { false } - subject(:service) { described_class.new(project, user, {}) } + subject(:service) { described_class.new(project: project, current_user: user) } before do project.add_maintainer(user) diff --git a/spec/services/merge_requests/refresh_service_spec.rb b/spec/services/merge_requests/refresh_service_spec.rb index 26f757d5ba0..6e6b4a91e0d 100644 --- a/spec/services/merge_requests/refresh_service_spec.rb +++ b/spec/services/merge_requests/refresh_service_spec.rb @@ -64,7 +64,7 @@ RSpec.describe MergeRequests::RefreshService do end context 'push to origin repo source branch' do - let(:refresh_service) { service.new(@project, @user) } + let(:refresh_service) { service.new(project: @project, current_user: @user) } let(:notification_service) { spy('notification_service') } before do @@ -187,7 +187,7 @@ RSpec.describe MergeRequests::RefreshService do context 'when pipeline exists for the source branch' do let!(:pipeline) { create(:ci_empty_pipeline, ref: @merge_request.source_branch, project: @project, sha: @commits.first.sha)} - subject { service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') } + subject { service.new(project: @project, current_user: @user).execute(@oldrev, @newrev, 'refs/heads/master') } it 'updates the head_pipeline_id for @merge_request', :sidekiq_might_not_need_inline do expect { subject }.to change { @merge_request.reload.head_pipeline_id }.from(nil).to(pipeline.id) @@ -203,7 +203,7 @@ RSpec.describe MergeRequests::RefreshService do stub_ci_pipeline_yaml_file(config) end - subject { service.new(project, @user).execute(@oldrev, @newrev, ref) } + subject { service.new(project: project, current_user: @user).execute(@oldrev, @newrev, ref) } let(:ref) { 'refs/heads/master' } let(:project) { @project } @@ -291,11 +291,11 @@ RSpec.describe MergeRequests::RefreshService do context "when MergeRequestUpdateWorker is retried by an exception" do it 'does not re-create a duplicate detached merge request pipeline' do expect do - service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') + service.new(project: @project, current_user: @user).execute(@oldrev, @newrev, 'refs/heads/master') end.to change { @merge_request.pipelines_for_merge_request.count }.by(1) expect do - service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/master') + service.new(project: @project, current_user: @user).execute(@oldrev, @newrev, 'refs/heads/master') end.not_to change { @merge_request.pipelines_for_merge_request.count } end end @@ -365,7 +365,7 @@ RSpec.describe MergeRequests::RefreshService do end context 'push to origin repo source branch' do - let(:refresh_service) { service.new(@project, @user) } + let(:refresh_service) { service.new(project: @project, current_user: @user) } let(:notification_service) { spy('notification_service') } before do @@ -397,7 +397,7 @@ RSpec.describe MergeRequests::RefreshService do context 'push to origin repo target branch', :sidekiq_might_not_need_inline do context 'when all MRs to the target branch had diffs' do before do - service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + service.new(project: @project, current_user: @user).execute(@oldrev, @newrev, 'refs/heads/feature') reload_mrs end @@ -426,7 +426,7 @@ RSpec.describe MergeRequests::RefreshService do # feature all along. empty_fork_merge_request.update_columns(target_branch: 'feature') - service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + service.new(project: @project, current_user: @user).execute(@oldrev, @newrev, 'refs/heads/feature') reload_mrs empty_fork_merge_request.reload end @@ -449,7 +449,7 @@ RSpec.describe MergeRequests::RefreshService do # Merge master -> feature branch @project.repository.merge(@user, @merge_request.diff_head_sha, @merge_request, 'Test message') commit = @project.repository.commit('feature') - service.new(@project, @user).execute(@oldrev, commit.id, 'refs/heads/feature') + service.new(project: @project, current_user: @user).execute(@oldrev, commit.id, 'refs/heads/feature') reload_mrs end @@ -467,7 +467,7 @@ RSpec.describe MergeRequests::RefreshService do end context 'push to fork repo source branch', :sidekiq_might_not_need_inline do - let(:refresh_service) { service.new(@fork_project, @user) } + let(:refresh_service) { service.new(project: @fork_project, current_user: @user) } def refresh allow(refresh_service).to receive(:execute_hooks) @@ -534,7 +534,7 @@ RSpec.describe MergeRequests::RefreshService do context 'push to fork repo target branch', :sidekiq_might_not_need_inline do describe 'changes to merge requests' do before do - service.new(@fork_project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + service.new(project: @fork_project, current_user: @user).execute(@oldrev, @newrev, 'refs/heads/feature') reload_mrs end @@ -551,7 +551,7 @@ RSpec.describe MergeRequests::RefreshService do describe 'merge request diff' do it 'does not reload the diff of the merge request made from fork' do expect do - service.new(@fork_project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + service.new(project: @fork_project, current_user: @user).execute(@oldrev, @newrev, 'refs/heads/feature') end.not_to change { @fork_merge_request.reload.merge_request_diff } end end @@ -582,28 +582,28 @@ RSpec.describe MergeRequests::RefreshService do it 'reloads a new diff for a push to the forked project' do expect do - service.new(@fork_project, @user).execute(@oldrev, first_commit, 'refs/heads/master') + service.new(project: @fork_project, current_user: @user).execute(@oldrev, first_commit, 'refs/heads/master') reload_mrs end.to change { forked_master_mr.merge_request_diffs.count }.by(1) end it 'reloads a new diff for a force push to the source branch' do expect do - service.new(@fork_project, @user).execute(@oldrev, force_push_commit, 'refs/heads/master') + service.new(project: @fork_project, current_user: @user).execute(@oldrev, force_push_commit, 'refs/heads/master') reload_mrs end.to change { forked_master_mr.merge_request_diffs.count }.by(1) end it 'reloads a new diff for a force push to the target branch' do expect do - service.new(@project, @user).execute(@oldrev, force_push_commit, 'refs/heads/master') + service.new(project: @project, current_user: @user).execute(@oldrev, force_push_commit, 'refs/heads/master') reload_mrs end.to change { forked_master_mr.merge_request_diffs.count }.by(1) end it 'reloads a new diff for a push to the target project that contains a commit in the MR' do expect do - service.new(@project, @user).execute(@oldrev, first_commit, 'refs/heads/master') + service.new(project: @project, current_user: @user).execute(@oldrev, first_commit, 'refs/heads/master') reload_mrs end.to change { forked_master_mr.merge_request_diffs.count }.by(1) end @@ -614,7 +614,7 @@ RSpec.describe MergeRequests::RefreshService do branch_name: 'master') expect do - service.new(@project, @user).execute(@newrev, new_commit, 'refs/heads/master') + service.new(project: @project, current_user: @user).execute(@newrev, new_commit, 'refs/heads/master') reload_mrs end.not_to change { forked_master_mr.merge_request_diffs.count } end @@ -623,7 +623,7 @@ RSpec.describe MergeRequests::RefreshService do context 'push to origin repo target branch after fork project was removed' do before do @fork_project.destroy! - service.new(@project, @user).execute(@oldrev, @newrev, 'refs/heads/feature') + service.new(project: @project, current_user: @user).execute(@oldrev, @newrev, 'refs/heads/feature') reload_mrs end @@ -639,7 +639,7 @@ RSpec.describe MergeRequests::RefreshService do end context 'push new branch that exists in a merge request' do - let(:refresh_service) { service.new(@fork_project, @user) } + let(:refresh_service) { service.new(project: @fork_project, current_user: @user) } it 'refreshes the merge request', :sidekiq_might_not_need_inline do expect(refresh_service).to receive(:execute_hooks) @@ -688,7 +688,7 @@ RSpec.describe MergeRequests::RefreshService do source_branch: 'close-by-commit', source_project: project) - refresh_service = service.new(project, user) + refresh_service = service.new(project: project, current_user: user) allow(refresh_service).to receive(:execute_hooks) refresh_service.execute(@oldrev, @newrev, 'refs/heads/close-by-commit') @@ -711,7 +711,7 @@ RSpec.describe MergeRequests::RefreshService do source_branch: 'close-by-commit', source_project: forked_project) - refresh_service = service.new(forked_project, user) + refresh_service = service.new(project: forked_project, current_user: user) allow(refresh_service).to receive(:execute_hooks) refresh_service.execute(@oldrev, @newrev, 'refs/heads/close-by-commit') @@ -722,7 +722,7 @@ RSpec.describe MergeRequests::RefreshService do end context 'marking the merge request as draft' do - let(:refresh_service) { service.new(@project, @user) } + let(:refresh_service) { service.new(project: @project, current_user: @user) } before do allow(refresh_service).to receive(:execute_hooks) @@ -802,7 +802,7 @@ RSpec.describe MergeRequests::RefreshService do end describe 'updating merge_commit' do - let(:service) { described_class.new(project, user) } + let(:service) { described_class.new(project: project, current_user: user) } let(:user) { create(:user) } let(:project) { create(:project, :repository) } @@ -890,7 +890,7 @@ RSpec.describe MergeRequests::RefreshService do end let(:auto_merge_strategy) { AutoMergeService::STRATEGY_MERGE_WHEN_PIPELINE_SUCCEEDS } - let(:refresh_service) { service.new(project, user) } + let(:refresh_service) { service.new(project: project, current_user: user) } before do target_project.merge_method = merge_method diff --git a/spec/services/merge_requests/reload_merge_head_diff_service_spec.rb b/spec/services/merge_requests/reload_merge_head_diff_service_spec.rb index 3152a4e3861..b333d4af6cf 100644 --- a/spec/services/merge_requests/reload_merge_head_diff_service_spec.rb +++ b/spec/services/merge_requests/reload_merge_head_diff_service_spec.rb @@ -10,7 +10,7 @@ RSpec.describe MergeRequests::ReloadMergeHeadDiffService do describe '#execute' do before do MergeRequests::MergeToRefService - .new(merge_request.project, merge_request.author) + .new(project: merge_request.project, current_user: merge_request.author) .execute(merge_request) end diff --git a/spec/services/merge_requests/remove_approval_service_spec.rb b/spec/services/merge_requests/remove_approval_service_spec.rb index 4ef2da290e1..ef6a0ec69bd 100644 --- a/spec/services/merge_requests/remove_approval_service_spec.rb +++ b/spec/services/merge_requests/remove_approval_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe MergeRequests::RemoveApprovalService do let(:merge_request) { create(:merge_request, source_project: project) } let!(:existing_approval) { create(:approval, merge_request: merge_request) } - subject(:service) { described_class.new(project, user) } + subject(:service) { described_class.new(project: project, current_user: user) } def execute! service.execute(merge_request) diff --git a/spec/services/merge_requests/reopen_service_spec.rb b/spec/services/merge_requests/reopen_service_spec.rb index 8541d597581..b9df31b6727 100644 --- a/spec/services/merge_requests/reopen_service_spec.rb +++ b/spec/services/merge_requests/reopen_service_spec.rb @@ -20,7 +20,7 @@ RSpec.describe MergeRequests::ReopenService do it_behaves_like 'merge request reviewers cache counters invalidator' context 'valid params' do - let(:service) { described_class.new(project, user, {}) } + let(:service) { described_class.new(project: project, current_user: user) } before do allow(service).to receive(:execute_hooks) @@ -65,7 +65,7 @@ RSpec.describe MergeRequests::ReopenService do it 'caches merge request closing issues' do expect(merge_request).to receive(:cache_merge_request_closes_issues!) - described_class.new(project, user, {}).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end it 'updates metrics' do @@ -78,7 +78,7 @@ RSpec.describe MergeRequests::ReopenService do expect(service).to receive(:reopen) - described_class.new(project, user, {}).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end it 'calls the merge request activity counter' do @@ -86,11 +86,11 @@ RSpec.describe MergeRequests::ReopenService do .to receive(:track_reopen_mr_action) .with(user: user) - described_class.new(project, user, {}).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end it 'refreshes the number of open merge requests for a valid MR' do - service = described_class.new(project, user, {}) + service = described_class.new(project: project, current_user: user) expect { service.execute(merge_request) } .to change { project.open_merge_requests_count }.from(0).to(1) @@ -99,7 +99,7 @@ RSpec.describe MergeRequests::ReopenService do context 'current user is not authorized to reopen merge request' do before do perform_enqueued_jobs do - @merge_request = described_class.new(project, guest).execute(merge_request) + @merge_request = described_class.new(project: project, current_user: guest).execute(merge_request) end end diff --git a/spec/services/merge_requests/request_review_service_spec.rb b/spec/services/merge_requests/request_review_service_spec.rb index 5cb4120852a..8bc31df605c 100644 --- a/spec/services/merge_requests/request_review_service_spec.rb +++ b/spec/services/merge_requests/request_review_service_spec.rb @@ -8,7 +8,7 @@ RSpec.describe MergeRequests::RequestReviewService do let(:merge_request) { create(:merge_request, reviewers: [user]) } let(:reviewer) { merge_request.find_reviewer(user) } let(:project) { merge_request.project } - let(:service) { described_class.new(project, current_user) } + let(:service) { described_class.new(project: project, current_user: current_user) } let(:result) { service.execute(merge_request, user) } let(:todo_service) { spy('todo service') } let(:notification_service) { spy('notification service') } @@ -26,7 +26,7 @@ RSpec.describe MergeRequests::RequestReviewService do describe '#execute' do describe 'invalid permissions' do - let(:service) { described_class.new(project, create(:user)) } + let(:service) { described_class.new(project: project, current_user: create(:user)) } it 'returns an error' do expect(result[:status]).to eq :error diff --git a/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb b/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb index 874cf66659a..74f3a1b06fc 100644 --- a/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb +++ b/spec/services/merge_requests/resolved_discussion_notification_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe MergeRequests::ResolvedDiscussionNotificationService do let(:user) { create(:user) } let(:project) { merge_request.project } - subject { described_class.new(project, user) } + subject { described_class.new(project: project, current_user: user) } describe "#execute" do context "when not all discussions are resolved" do diff --git a/spec/services/merge_requests/retarget_chain_service_spec.rb b/spec/services/merge_requests/retarget_chain_service_spec.rb index 3937fbe58c3..87bde4a1400 100644 --- a/spec/services/merge_requests/retarget_chain_service_spec.rb +++ b/spec/services/merge_requests/retarget_chain_service_spec.rb @@ -9,7 +9,7 @@ RSpec.describe MergeRequests::RetargetChainService do let_it_be(:merge_request, reload: true) { create(:merge_request, assignees: [user]) } let_it_be(:project) { merge_request.project } - subject { described_class.new(project, user).execute(merge_request) } + subject { described_class.new(project: project, current_user: user).execute(merge_request) } before do project.add_maintainer(user) diff --git a/spec/services/merge_requests/squash_service_spec.rb b/spec/services/merge_requests/squash_service_spec.rb index acbd0a42fcd..149748cdabc 100644 --- a/spec/services/merge_requests/squash_service_spec.rb +++ b/spec/services/merge_requests/squash_service_spec.rb @@ -5,7 +5,7 @@ require 'spec_helper' RSpec.describe MergeRequests::SquashService do include GitHelpers - let(:service) { described_class.new(project, user, { merge_request: merge_request }) } + let(:service) { described_class.new(project: project, current_user: user, params: { merge_request: merge_request }) } let(:user) { project.owner } let(:project) { create(:project, :repository) } let(:repository) { project.repository.raw } @@ -62,7 +62,7 @@ RSpec.describe MergeRequests::SquashService do end it 'will still perform the squash when a custom squash commit message has been provided' do - service = described_class.new(project, user, { merge_request: merge_request, squash_commit_message: 'A custom commit message' }) + service = described_class.new(project: project, current_user: user, params: { merge_request: merge_request, squash_commit_message: 'A custom commit message' }) expect(merge_request.target_project.repository).to receive(:squash).and_return('sha') @@ -98,7 +98,7 @@ RSpec.describe MergeRequests::SquashService do end context 'if a message was provided' do - let(:service) { described_class.new(project, user, { merge_request: merge_request, squash_commit_message: message }) } + let(:service) { described_class.new(project: project, current_user: user, params: { merge_request: merge_request, squash_commit_message: message }) } let(:message) { 'My custom message' } let(:squash_sha) { service.execute[:squash_sha] } diff --git a/spec/services/merge_requests/update_assignees_service_spec.rb b/spec/services/merge_requests/update_assignees_service_spec.rb index de03aab5418..113bfb0f31a 100644 --- a/spec/services/merge_requests/update_assignees_service_spec.rb +++ b/spec/services/merge_requests/update_assignees_service_spec.rb @@ -26,7 +26,7 @@ RSpec.describe MergeRequests::UpdateAssigneesService do project.add_developer(user3) end - let(:service) { described_class.new(project, user, opts) } + let(:service) { described_class.new(project: project, current_user: user, params: opts) } let(:opts) { { assignee_ids: [user2.id] } } describe 'execute' do @@ -37,7 +37,7 @@ RSpec.describe MergeRequests::UpdateAssigneesService do context 'when the parameters are valid' do it 'updates the MR, and queues the more expensive work for later' do - expect_next(MergeRequests::HandleAssigneesChangeService, project, user) do |service| + expect_next(MergeRequests::HandleAssigneesChangeService, project: project, current_user: user) do |service| expect(service) .to receive(:async_execute) .with(merge_request, [user3], execute_hooks: true) @@ -56,7 +56,7 @@ RSpec.describe MergeRequests::UpdateAssigneesService do end it 'is more efficient than using the full update-service' do - allow_next(MergeRequests::HandleAssigneesChangeService, project, user) do |service| + allow_next(MergeRequests::HandleAssigneesChangeService, project: project, current_user: user) do |service| expect(service) .to receive(:async_execute) .with(merge_request, [user3], execute_hooks: true) @@ -69,7 +69,7 @@ RSpec.describe MergeRequests::UpdateAssigneesService do source_project: merge_request.project, author: merge_request.author) - update_service = ::MergeRequests::UpdateService.new(project, user, opts) + update_service = ::MergeRequests::UpdateService.new(project: project, current_user: user, params: opts) expect { service.execute(merge_request) } .to issue_fewer_queries_than { update_service.execute(other_mr) } diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 2d5365a195d..a85fbd77d70 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -43,7 +43,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do end def update_merge_request(opts) - @merge_request = MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + @merge_request = MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) @merge_request.reload end @@ -64,7 +64,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do } end - let(:service) { described_class.new(project, current_user, opts) } + let(:service) { described_class.new(project: project, current_user: current_user, params: opts) } let(:current_user) { user } before do @@ -99,7 +99,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do expect(Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter) .to receive(:track_description_edit_action).once.with(user: user) - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request2) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request2) end it 'tracks Draft/WIP marking' do @@ -108,7 +108,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:title] = "WIP: #{opts[:title]}" - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request2) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request2) end it 'tracks Draft/WIP un-marking' do @@ -117,7 +117,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:title] = "Non-draft/wip title string" - MergeRequests::UpdateService.new(project, user, opts).execute(draft_merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(draft_merge_request) end context 'when MR is locked' do @@ -128,7 +128,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:discussion_locked] = true - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end end @@ -139,7 +139,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:discussion_locked] = false - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end end end @@ -154,7 +154,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:discussion_locked] = false - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end end @@ -165,7 +165,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:discussion_locked] = true - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end end end @@ -184,7 +184,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do spent_at: Date.parse('2021-02-24') } - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end it 'tracks milestone change' do @@ -193,7 +193,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:milestone] = milestone - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end it 'track labels change' do @@ -202,7 +202,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:label_ids] = [label2.id] - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end context 'reviewers' do @@ -213,7 +213,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:reviewers] = [user2] - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end end @@ -224,7 +224,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts[:reviewers] = merge_request.reviewers - MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) + MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) end end end @@ -439,7 +439,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do let(:milestone) { create(:milestone, project: project) } let(:req_opts) { { source_branch: 'feature', target_branch: 'master' } } - subject { MergeRequests::UpdateService.new(project, user, opts).execute(merge_request) } + subject { MergeRequests::UpdateService.new(project: project, current_user: user, params: opts).execute(merge_request) } context 'when mentionable attributes change' do let(:opts) { { description: "Description with #{user.to_reference}" }.merge(req_opts) } @@ -486,7 +486,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do } end - let(:service) { described_class.new(project, user, opts) } + let(:service) { described_class.new(project: project, current_user: user, params: opts) } context 'without pipeline' do before do @@ -547,7 +547,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do context 'with a non-authorised user' do let(:visitor) { create(:user) } - let(:service) { described_class.new(project, visitor, opts) } + let(:service) { described_class.new(project: project, current_user: visitor, params: opts) } before do merge_request.update_attribute(:merge_error, 'Error') @@ -805,7 +805,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts = { title: 'New title' } perform_enqueued_jobs do - @merge_request = described_class.new(project, user, opts).execute(merge_request) + @merge_request = described_class.new(project: project, current_user: user, params: opts).execute(merge_request) end should_email(subscriber) @@ -818,7 +818,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts = { title: 'Draft: New title' } perform_enqueued_jobs do - @merge_request = described_class.new(project, user, opts).execute(merge_request) + @merge_request = described_class.new(project: project, current_user: user, params: opts).execute(merge_request) end should_not_email(subscriber) @@ -840,7 +840,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts = { label_ids: [label.id] } perform_enqueued_jobs do - @merge_request = described_class.new(project, user, opts).execute(merge_request) + @merge_request = described_class.new(project: project, current_user: user, params: opts).execute(merge_request) end should_email(subscriber) @@ -856,7 +856,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts = { label_ids: [label.id, label2.id] } perform_enqueued_jobs do - @merge_request = described_class.new(project, user, opts).execute(merge_request) + @merge_request = described_class.new(project: project, current_user: user, params: opts).execute(merge_request) end should_not_email(subscriber) @@ -867,7 +867,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do opts = { label_ids: [label2.id] } perform_enqueued_jobs do - @merge_request = described_class.new(project, user, opts).execute(merge_request) + @merge_request = described_class.new(project: project, current_user: user, params: opts).execute(merge_request) end should_not_email(subscriber) @@ -933,7 +933,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do it 'creates a `MergeRequestsClosingIssues` record for each issue' do issue_closing_opts = { description: "Closes #{first_issue.to_reference} and #{second_issue.to_reference}" } - service = described_class.new(project, user, issue_closing_opts) + service = described_class.new(project: project, current_user: user, params: issue_closing_opts) allow(service).to receive(:execute_hooks) service.execute(merge_request) @@ -945,7 +945,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do create(:merge_requests_closing_issues, issue: first_issue, merge_request: merge_request) create(:merge_requests_closing_issues, issue: second_issue, merge_request: merge_request) - service = described_class.new(project, user, description: "not closing any issues") + service = described_class.new(project: project, current_user: user, params: { description: "not closing any issues" }) allow(service).to receive(:execute_hooks) service.execute(merge_request.reload) @@ -1002,7 +1002,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do it 'unassigns assignee when user id is 0' do merge_request.update!(assignee_ids: [user.id]) - expect_next_instance_of(MergeRequests::HandleAssigneesChangeService, project, user) do |service| + expect_next_instance_of(MergeRequests::HandleAssigneesChangeService, project: project, current_user: user) do |service| expect(service) .to receive(:async_execute) .with(merge_request, [user]) @@ -1014,7 +1014,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do end it 'saves assignee when user id is valid' do - expect_next_instance_of(MergeRequests::HandleAssigneesChangeService, project, user) do |service| + expect_next_instance_of(MergeRequests::HandleAssigneesChangeService, project: project, current_user: user) do |service| expect(service) .to receive(:async_execute) .with(merge_request, [user3]) @@ -1174,7 +1174,7 @@ RSpec.describe MergeRequests::UpdateService, :mailer do it_behaves_like 'issuable record that supports quick actions' do let(:existing_merge_request) { create(:merge_request, source_project: project) } - let(:issuable) { described_class.new(project, user, params).execute(existing_merge_request) } + let(:issuable) { described_class.new(project: project, current_user: user, params: params).execute(existing_merge_request) } end end end diff --git a/spec/services/notes/copy_service_spec.rb b/spec/services/notes/copy_service_spec.rb index fd44aa7cf40..d9b6bafd7ff 100644 --- a/spec/services/notes/copy_service_spec.rb +++ b/spec/services/notes/copy_service_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Notes::CopyService do let_it_be(:noteable) { create(:issue) } it 'validates that we cannot copy notes to the same Noteable' do - expect { described_class.new(noteable, noteable) }.to raise_error(ArgumentError) + expect { described_class.new(nil, noteable, noteable) }.to raise_error(ArgumentError) end end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index d28cb118529..31263feb947 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -176,7 +176,7 @@ RSpec.describe Notes::CreateService do end it 'note is associated with a note diff file' do - MergeRequests::MergeToRefService.new(merge_request.project, merge_request.author).execute(merge_request) + MergeRequests::MergeToRefService.new(project: merge_request.project, current_user: merge_request.author).execute(merge_request) note = described_class.new(project_with_repo, user, new_opts).execute diff --git a/spec/services/notes/quick_actions_service_spec.rb b/spec/services/notes/quick_actions_service_spec.rb index bdce049bd3d..9692bb08379 100644 --- a/spec/services/notes/quick_actions_service_spec.rb +++ b/spec/services/notes/quick_actions_service_spec.rb @@ -238,25 +238,25 @@ RSpec.describe Notes::QuickActionsService do end end - describe '.noteable_update_service' do + describe '.noteable_update_service_class' do include_context 'note on noteable' it 'returns Issues::UpdateService for a note on an issue' do note = create(:note_on_issue, project: project) - expect(described_class.noteable_update_service(note)).to eq(Issues::UpdateService) + expect(described_class.noteable_update_service_class(note)).to eq(Issues::UpdateService) end it 'returns MergeRequests::UpdateService for a note on a merge request' do note = create(:note_on_merge_request, project: project) - expect(described_class.noteable_update_service(note)).to eq(MergeRequests::UpdateService) + expect(described_class.noteable_update_service_class(note)).to eq(MergeRequests::UpdateService) end it 'returns Commits::TagService for a note on a commit' do note = create(:note_on_commit, project: project) - expect(described_class.noteable_update_service(note)).to eq(Commits::TagService) + expect(described_class.noteable_update_service_class(note)).to eq(Commits::TagService) end end diff --git a/spec/services/projects/unlink_fork_service_spec.rb b/spec/services/projects/unlink_fork_service_spec.rb index 90def365fca..d939a79b7e9 100644 --- a/spec/services/projects/unlink_fork_service_spec.rb +++ b/spec/services/projects/unlink_fork_service_spec.rb @@ -16,11 +16,11 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin let(:merge_request2) { create(:merge_request, source_project: forked_project, target_project: fork_project(project)) } let(:merge_request_in_fork) { create(:merge_request, source_project: forked_project, target_project: forked_project) } - let(:mr_close_service) { MergeRequests::CloseService.new(forked_project, user) } + let(:mr_close_service) { MergeRequests::CloseService.new(project: forked_project, current_user: user) } before do allow(MergeRequests::CloseService).to receive(:new) - .with(forked_project, user) + .with(project: forked_project, current_user: user) .and_return(mr_close_service) end @@ -79,11 +79,11 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin let!(:merge_request2) { create(:merge_request, source_project: project, target_project: fork_project(project)) } let!(:merge_request_in_fork) { create(:merge_request, source_project: forked_project, target_project: forked_project) } - let(:mr_close_service) { MergeRequests::CloseService.new(project, user) } + let(:mr_close_service) { MergeRequests::CloseService.new(project: project, current_user: user) } before do allow(MergeRequests::CloseService).to receive(:new) - .with(project, user) + .with(project: project, current_user: user) .and_return(mr_close_service) end @@ -142,11 +142,11 @@ RSpec.describe Projects::UnlinkForkService, :use_clean_rails_memory_store_cachin let!(:mr_from_child) { create(:merge_request, source_project: fork_of_fork, target_project: forked_project) } let!(:merge_request_in_fork) { create(:merge_request, source_project: forked_project, target_project: forked_project) } - let(:mr_close_service) { MergeRequests::CloseService.new(forked_project, user) } + let(:mr_close_service) { MergeRequests::CloseService.new(project: forked_project, current_user: user) } before do allow(MergeRequests::CloseService).to receive(:new) - .with(forked_project, user) + .with(project: forked_project, current_user: user) .and_return(mr_close_service) end diff --git a/spec/services/suggestions/apply_service_spec.rb b/spec/services/suggestions/apply_service_spec.rb index f6d7297bf60..9cf794cde7e 100644 --- a/spec/services/suggestions/apply_service_spec.rb +++ b/spec/services/suggestions/apply_service_spec.rb @@ -398,7 +398,7 @@ RSpec.describe Suggestions::ApplyService do suggestion.reload expect(result[:status]).to eq(:success) - refresh = MergeRequests::RefreshService.new(project, user) + refresh = MergeRequests::RefreshService.new(project: project, current_user: user) refresh.execute(merge_request.diff_head_sha, suggestion.commit_id, merge_request.source_branch_ref) diff --git a/spec/support/helpers/cycle_analytics_helpers.rb b/spec/support/helpers/cycle_analytics_helpers.rb index 5915ffe202d..5510284b30d 100644 --- a/spec/support/helpers/cycle_analytics_helpers.rb +++ b/spec/support/helpers/cycle_analytics_helpers.rb @@ -125,17 +125,17 @@ module CycleAnalyticsHelpers target_branch: 'master' } - mr = MergeRequests::CreateService.new(project, user, opts).execute + mr = MergeRequests::CreateService.new(project: project, current_user: user, params: opts).execute NewMergeRequestWorker.new.perform(mr, user) mr end def merge_merge_requests_closing_issue(user, project, issue) merge_requests = Issues::ReferencedMergeRequestsService - .new(project, user) + .new(project: project, current_user: user) .closed_by_merge_requests(issue) - merge_requests.each { |merge_request| MergeRequests::MergeService.new(project, user, sha: merge_request.diff_head_sha).execute(merge_request) } + merge_requests.each { |merge_request| MergeRequests::MergeService.new(project: project, current_user: user, params: { sha: merge_request.diff_head_sha }).execute(merge_request) } end def deploy_master(user, project, environment: 'production') diff --git a/spec/support/services/issuable_update_service_shared_examples.rb b/spec/support/services/issuable_update_service_shared_examples.rb index 8867a1e3a90..4d2843af1c4 100644 --- a/spec/support/services/issuable_update_service_shared_examples.rb +++ b/spec/support/services/issuable_update_service_shared_examples.rb @@ -12,13 +12,13 @@ RSpec.shared_examples 'issuable update service' do context 'to reopened' do it 'executes hooks only once' do - described_class.new(project, user, state_event: 'reopen').execute(closed_issuable) + described_class.new(project: project, current_user: user, params: { state_event: 'reopen' }).execute(closed_issuable) end end context 'to closed' do it 'executes hooks only once' do - described_class.new(project, user, state_event: 'close').execute(open_issuable) + described_class.new(project: project, current_user: user, params: { state_event: 'close' }).execute(open_issuable) end end end diff --git a/spec/support/shared_examples/models/chat_service_shared_examples.rb b/spec/support/shared_examples/models/chat_service_shared_examples.rb index 59e249bb865..4a47aad0957 100644 --- a/spec/support/shared_examples/models/chat_service_shared_examples.rb +++ b/spec/support/shared_examples/models/chat_service_shared_examples.rb @@ -163,7 +163,7 @@ RSpec.shared_examples "chat service" do |service_name| context "with issue events" do let(:opts) { { title: "Awesome issue", description: "please fix" } } let(:sample_data) do - service = Issues::CreateService.new(project, user, opts) + service = Issues::CreateService.new(project: project, current_user: user, params: opts) issue = service.execute service.hook_data(issue, "open") end @@ -182,7 +182,7 @@ RSpec.shared_examples "chat service" do |service_name| end let(:sample_data) do - service = MergeRequests::CreateService.new(project, user, opts) + service = MergeRequests::CreateService.new(project: project, current_user: user, params: opts) merge_request = service.execute service.hook_data(merge_request, "open") end diff --git a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb index 5b603ac08c8..afc902dd184 100644 --- a/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/time_tracking_shared_examples.rb @@ -128,7 +128,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name| if issuable_name == 'merge_request' it 'calls update service with :use_specialized_service param' do - expect(::MergeRequests::UpdateService).to receive(:new).with(project, user, hash_including(use_specialized_service: true)) + expect(::MergeRequests::UpdateService).to receive(:new).with(project: project, current_user: user, params: hash_including(use_specialized_service: true)) post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user), params: { duration: '2h' } end @@ -136,7 +136,7 @@ RSpec.shared_examples 'time tracking endpoints' do |issuable_name| if issuable_name == 'issue' it 'calls update service without :use_specialized_service param' do - expect(::Issues::UpdateService).to receive(:new).with(project, user, hash_not_including(use_specialized_service: true)) + expect(::Issues::UpdateService).to receive(:new).with(project: project, current_user: user, params: hash_not_including(use_specialized_service: true)) post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user), params: { duration: '2h' } end diff --git a/spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb b/spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb index 4aa5d7d890b..7d4fbeea0dc 100644 --- a/spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb +++ b/spec/support/shared_examples/services/boards/issues_move_service_shared_examples.rb @@ -146,7 +146,7 @@ RSpec.shared_examples 'issues move service' do |group| params.merge!(move_after_id: issue1.id, move_before_id: issue2.id) match_params = { move_between_ids: [issue1.id, issue2.id], board_group_id: parent.id } - expect(Issues::UpdateService).to receive(:new).with(issue.project, user, match_params).and_return(double(execute: build(:issue))) + expect(Issues::UpdateService).to receive(:new).with(project: issue.project, current_user: user, params: match_params).and_return(double(execute: build(:issue))) described_class.new(parent, user, params).execute(issue) end diff --git a/spec/support/shared_examples/services/common_system_notes_shared_examples.rb b/spec/support/shared_examples/services/common_system_notes_shared_examples.rb index 7b277d4bede..ce412ef55de 100644 --- a/spec/support/shared_examples/services/common_system_notes_shared_examples.rb +++ b/spec/support/shared_examples/services/common_system_notes_shared_examples.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true RSpec.shared_examples 'system note creation' do |update_params, note_text| - subject { described_class.new(project, user).execute(issuable, old_labels: []) } + subject { described_class.new(project: project, current_user: user).execute(issuable, old_labels: []) } before do issuable.assign_attributes(update_params) @@ -18,7 +18,7 @@ RSpec.shared_examples 'system note creation' do |update_params, note_text| end RSpec.shared_examples 'draft notes creation' do |action| - subject { described_class.new(project, user).execute(issuable, old_labels: []) } + subject { described_class.new(project: project, current_user: user).execute(issuable, old_labels: []) } it 'creates Draft toggle and title change notes' do expect { subject }.to change { Note.count }.from(0).to(2) diff --git a/spec/support/shared_examples/services/issuable_shared_examples.rb b/spec/support/shared_examples/services/issuable_shared_examples.rb index 5b3e0f9e0b9..a50a386afe1 100644 --- a/spec/support/shared_examples/services/issuable_shared_examples.rb +++ b/spec/support/shared_examples/services/issuable_shared_examples.rb @@ -4,14 +4,14 @@ RSpec.shared_examples 'cache counters invalidator' do it 'invalidates counter cache for assignees' do expect_any_instance_of(User).to receive(:invalidate_merge_request_cache_counts) - described_class.new(project, user, {}).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end end RSpec.shared_examples 'updating a single task' do def update_issuable(opts) issuable = try(:issue) || try(:merge_request) - described_class.new(project, user, opts).execute(issuable) + described_class.new(project: project, current_user: user, params: opts).execute(issuable) end before do diff --git a/spec/support/shared_examples/services/merge_request_shared_examples.rb b/spec/support/shared_examples/services/merge_request_shared_examples.rb index 178b6bc47e1..d2595b92cbc 100644 --- a/spec/support/shared_examples/services/merge_request_shared_examples.rb +++ b/spec/support/shared_examples/services/merge_request_shared_examples.rb @@ -70,7 +70,7 @@ RSpec.shared_examples 'merge request reviewers cache counters invalidator' do it 'invalidates counter cache for reviewers' do expect(merge_request.reviewers).to all(receive(:invalidate_merge_request_cache_counts)) - described_class.new(project, user, {}).execute(merge_request) + described_class.new(project: project, current_user: user).execute(merge_request) end end @@ -86,7 +86,7 @@ RSpec.shared_examples_for 'a service that can create a merge request' do context 'when project has been forked', :sidekiq_might_not_need_inline do let(:forked_project) { fork_project(project, user1, repository: true) } - let(:service) { described_class.new(forked_project, user1, changes, push_options) } + let(:service) { described_class.new(project: forked_project, current_user: user1, changes: changes, push_options: push_options) } before do allow(forked_project).to receive(:empty_repo?).and_return(false) diff --git a/spec/support/shared_examples/services/updating_mentions_shared_examples.rb b/spec/support/shared_examples/services/updating_mentions_shared_examples.rb index 84f6c4d136a..13a2aa9ddac 100644 --- a/spec/support/shared_examples/services/updating_mentions_shared_examples.rb +++ b/spec/support/shared_examples/services/updating_mentions_shared_examples.rb @@ -15,7 +15,7 @@ RSpec.shared_examples 'updating mentions' do |service_class| def update_mentionable(opts) perform_enqueued_jobs do - service_class.new(project, user, opts).execute(mentionable) + service_class.new(project: project, current_user: user, params: opts).execute(mentionable) end mentionable.reload diff --git a/spec/workers/ci/merge_requests/add_todo_when_build_fails_worker_spec.rb b/spec/workers/ci/merge_requests/add_todo_when_build_fails_worker_spec.rb index 4690c73d121..e5de0ba0143 100644 --- a/spec/workers/ci/merge_requests/add_todo_when_build_fails_worker_spec.rb +++ b/spec/workers/ci/merge_requests/add_todo_when_build_fails_worker_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Ci::MergeRequests::AddTodoWhenBuildFailsWorker do include_examples 'an idempotent worker' do it 'executes todo service' do service = double - expect(::MergeRequests::AddTodoWhenBuildFailsService).to receive(:new).with(project, nil).and_return(service).twice + expect(::MergeRequests::AddTodoWhenBuildFailsService).to receive(:new).with(project: project).and_return(service).twice expect(service).to receive(:execute).with(job).twice perform_twice diff --git a/spec/workers/merge_requests/create_pipeline_worker_spec.rb b/spec/workers/merge_requests/create_pipeline_worker_spec.rb index 8efce5220be..06d44c45706 100644 --- a/spec/workers/merge_requests/create_pipeline_worker_spec.rb +++ b/spec/workers/merge_requests/create_pipeline_worker_spec.rb @@ -13,7 +13,7 @@ RSpec.describe MergeRequests::CreatePipelineWorker do context 'when the objects exist' do it 'calls the merge request create pipeline service and calls update head pipeline' do aggregate_failures do - expect_next_instance_of(MergeRequests::CreatePipelineService, project, user) do |service| + expect_next_instance_of(MergeRequests::CreatePipelineService, project: project, current_user: user) do |service| expect(service).to receive(:execute).with(merge_request) end diff --git a/spec/workers/process_commit_worker_spec.rb b/spec/workers/process_commit_worker_spec.rb index 7a168bf054e..294a05c652b 100644 --- a/spec/workers/process_commit_worker_spec.rb +++ b/spec/workers/process_commit_worker_spec.rb @@ -94,7 +94,7 @@ RSpec.describe ProcessCommitWorker do project.repository.after_create_branch MergeRequests::MergeService - .new(project, merge_request.author, { sha: merge_request.diff_head_sha }) + .new(project: project, current_user: merge_request.author, params: { sha: merge_request.diff_head_sha }) .execute(merge_request) merge_request.reload.merge_commit diff --git a/spec/workers/rebase_worker_spec.rb b/spec/workers/rebase_worker_spec.rb index 9246b283be5..4bdfd7219f2 100644 --- a/spec/workers/rebase_worker_spec.rb +++ b/spec/workers/rebase_worker_spec.rb @@ -19,7 +19,7 @@ RSpec.describe RebaseWorker, '#perform' do it 'sets the correct project for running hooks' do expect(MergeRequests::RebaseService) - .to receive(:new).with(forked_project, merge_request.author).and_call_original + .to receive(:new).with(project: forked_project, current_user: merge_request.author).and_call_original subject.perform(merge_request.id, merge_request.author.id) end diff --git a/spec/workers/update_merge_requests_worker_spec.rb b/spec/workers/update_merge_requests_worker_spec.rb index fb12086c2f4..bd0dc2f9ef4 100644 --- a/spec/workers/update_merge_requests_worker_spec.rb +++ b/spec/workers/update_merge_requests_worker_spec.rb @@ -20,7 +20,7 @@ RSpec.describe UpdateMergeRequestsWorker do end it 'executes MergeRequests::RefreshService with expected values' do - expect_next_instance_of(MergeRequests::RefreshService, project, user) do |refresh_service| + expect_next_instance_of(MergeRequests::RefreshService, project: project, current_user: user) do |refresh_service| expect(refresh_service).to receive(:execute).with(oldrev, newrev, ref) end diff --git a/yarn.lock b/yarn.lock index 7583e5c5f4b..2a7415037bd 100644 --- a/yarn.lock +++ b/yarn.lock @@ -897,20 +897,20 @@ stylelint-declaration-strict-value "1.7.7" stylelint-scss "3.18.0" -"@gitlab/svgs@1.192.0": - version "1.192.0" - resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.192.0.tgz#d08d86b39d5bb9724b1249f71193b7393f185c2e" - integrity sha512-hGKtkeC5IplTOjLqLs2pwHh7/JIBnXkJKfl0txHhOFUiIP40YKKAZYiGNSrXVv5JTwQYn+zyK8a0EBuS884/xA== +"@gitlab/svgs@1.195.0": + version "1.195.0" + resolved "https://registry.yarnpkg.com/@gitlab/svgs/-/svgs-1.195.0.tgz#addf7a31fe8b49a9b91af7373877ec46e56603b1" + integrity sha512-ZEVfFEONZSUf0SmUY1+BDCQPTXBKPamHp/qfTVPXvuXxkihsSBRGwqdu57fNQq/0/Jk95IY2uxm+I9fzb4uQUQ== "@gitlab/tributejs@1.0.0": version "1.0.0" resolved "https://registry.yarnpkg.com/@gitlab/tributejs/-/tributejs-1.0.0.tgz#672befa222aeffc83e7d799b0500a7a4418e59b8" integrity sha512-nmKw1+hB6MHvlmPz63yPwVs1qQkycHwsKgxpEbzmky16Y6mL4EJMk3w1b8QlOAF/AIAzjCERPhe/R4MJiohbZw== -"@gitlab/ui@29.18.0": - version "29.18.0" - resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-29.18.0.tgz#c6ca04d9ef0b2e96ab0cd1e0570441e0d2accda7" - integrity sha512-CNa7eZn+8/j4/NzDBG5cdiIfyRZEb9J84WkEFTk7ZafZKdP2jtbdzys9eotQcS11kS1rf95rSVpQy0P6vebt1A== +"@gitlab/ui@29.21.0": + version "29.21.0" + resolved "https://registry.yarnpkg.com/@gitlab/ui/-/ui-29.21.0.tgz#f91169d693bc92109deadc412cf6ed76a8ae07ad" + integrity sha512-fAqhQjLXsl6JRM56NewFqtYowzjvne7IsM6F+aD4C/9OF9u81bXfa5do1o5usl030Hz9e8+PTwkXWwLTe0Nz8w== dependencies: "@babel/standalone" "^7.0.0" "@gitlab/vue-toasted" "^1.3.0" |