diff options
Diffstat (limited to 'app/services')
124 files changed, 1300 insertions, 926 deletions
diff --git a/app/services/alert_management/alerts/update_service.rb b/app/services/alert_management/alerts/update_service.rb index 0769adc862e..6bdceb0f27b 100644 --- a/app/services/alert_management/alerts/update_service.rb +++ b/app/services/alert_management/alerts/update_service.rb @@ -162,8 +162,9 @@ module AlertManagement end def filter_duplicate - # Only need to check if changing to an open status - return unless params[:status_event] && AlertManagement::Alert.open_status?(status) + # Only need to check if changing to a not-resolved status + return if params[:status_event].blank? || params[:status_event] == :resolve + return unless alert.resolved? param_errors << unresolved_alert_error if duplicate_alert? end @@ -171,24 +172,23 @@ module AlertManagement def duplicate_alert? return if alert.fingerprint.blank? - open_alerts.any? && open_alerts.exclude?(alert) + unresolved_alert.present? end - def open_alerts - strong_memoize(:open_alerts) do - AlertManagement::Alert.for_fingerprint(project, alert.fingerprint).open + def unresolved_alert + strong_memoize(:unresolved_alert) do + AlertManagement::Alert.find_unresolved_alert(project, alert.fingerprint) end end def unresolved_alert_error _('An %{link_start}alert%{link_end} with the same fingerprint is already open. ' \ 'To change the status of this alert, resolve the linked alert.' - ) % open_alert_url_params + ) % unresolved_alert_url_params end - def open_alert_url_params - open_alert = open_alerts.first - alert_path = Gitlab::Routing.url_helpers.details_project_alert_management_path(project, open_alert) + def unresolved_alert_url_params + alert_path = Gitlab::Routing.url_helpers.details_project_alert_management_path(project, unresolved_alert) { link_start: '<a href="%{url}">'.html_safe % { url: alert_path }, diff --git a/app/services/alert_management/metric_images/upload_service.rb b/app/services/alert_management/metric_images/upload_service.rb index e9db10594df..46e7e3dbedd 100644 --- a/app/services/alert_management/metric_images/upload_service.rb +++ b/app/services/alert_management/metric_images/upload_service.rb @@ -39,7 +39,7 @@ module AlertManagement private def can_upload_metrics? - alert.metric_images_available? && current_user&.can?(:upload_alert_management_metric_image, alert) + current_user&.can?(:upload_alert_management_metric_image, alert) end end end diff --git a/app/services/authorized_project_update/project_access_changed_service.rb b/app/services/authorized_project_update/project_access_changed_service.rb index 62bf4ced1ae..dafec1fef59 100644 --- a/app/services/authorized_project_update/project_access_changed_service.rb +++ b/app/services/authorized_project_update/project_access_changed_service.rb @@ -7,6 +7,8 @@ module AuthorizedProjectUpdate end def execute(blocking: true) + return if @project_ids.empty? + bulk_args = @project_ids.map { |id| [id] } if blocking diff --git a/app/services/authorized_project_update/project_create_service.rb b/app/services/authorized_project_update/project_create_service.rb deleted file mode 100644 index 5809315a066..00000000000 --- a/app/services/authorized_project_update/project_create_service.rb +++ /dev/null @@ -1,34 +0,0 @@ -# frozen_string_literal: true - -module AuthorizedProjectUpdate - class ProjectCreateService < BaseService - BATCH_SIZE = 1000 - - def initialize(project) - @project = project - end - - def execute - group = project.group - - unless group - return ServiceResponse.error(message: 'Project does not have a group') - end - - group.members_from_self_and_ancestors_with_effective_access_level - .each_batch(of: BATCH_SIZE, column: :user_id) do |members| - attributes = members.map do |member| - { user_id: member.user_id, project_id: project.id, access_level: member.access_level } - end - - ProjectAuthorization.insert_all(attributes) unless attributes.empty? - end - - ServiceResponse.success - end - - private - - attr_reader :project - end -end diff --git a/app/services/authorized_project_update/project_group_link_create_service.rb b/app/services/authorized_project_update/project_group_link_create_service.rb deleted file mode 100644 index 10cf4c50569..00000000000 --- a/app/services/authorized_project_update/project_group_link_create_service.rb +++ /dev/null @@ -1,72 +0,0 @@ -# frozen_string_literal: true - -module AuthorizedProjectUpdate - class ProjectGroupLinkCreateService < BaseService - include Gitlab::Utils::StrongMemoize - - BATCH_SIZE = 1000 - - def initialize(project, group, group_access = nil) - @project = project - @group = group - @group_access = group_access - end - - def execute - group.members_from_self_and_ancestors_with_effective_access_level - .each_batch(of: BATCH_SIZE, column: :user_id) do |members| - existing_authorizations = existing_project_authorizations(members) - authorizations_to_create = [] - user_ids_to_delete = [] - - members.each do |member| - new_access_level = access_level(member.access_level) - existing_access_level = existing_authorizations[member.user_id] - - if existing_access_level - # User might already have access to the project unrelated to the - # current project share - next if existing_access_level >= new_access_level - - user_ids_to_delete << member.user_id - end - - authorizations_to_create << { user_id: member.user_id, - project_id: project.id, - access_level: new_access_level } - end - - update_authorizations(user_ids_to_delete, authorizations_to_create) - end - - ServiceResponse.success - end - - private - - attr_reader :project, :group, :group_access - - def access_level(membership_access_level) - return membership_access_level unless group_access - - # access level (role) must not be higher than the max access level (role) set when - # creating the project share - [membership_access_level, group_access].min - end - - def existing_project_authorizations(members) - user_ids = members.map(&:user_id) - - ProjectAuthorization.where(project_id: project.id, user_id: user_ids) # rubocop: disable CodeReuse/ActiveRecord - .select(:user_id, :access_level) - .each_with_object({}) do |authorization, hash| - hash[authorization.user_id] = authorization.access_level - end - end - - def update_authorizations(user_ids_to_delete, authorizations_to_create) - project.remove_project_authorizations(user_ids_to_delete) if user_ids_to_delete.any? - ProjectAuthorization.insert_all_in_batches(authorizations_to_create) if authorizations_to_create.any? - end - end -end diff --git a/app/services/bulk_imports/file_decompression_service.rb b/app/services/bulk_imports/file_decompression_service.rb index b76746b199f..41616fc1c75 100644 --- a/app/services/bulk_imports/file_decompression_service.rb +++ b/app/services/bulk_imports/file_decompression_service.rb @@ -21,7 +21,7 @@ module BulkImports def execute validate_tmpdir validate_filepath - validate_decompressed_file_size if Feature.enabled?(:validate_import_decompressed_archive_size, default_enabled: :yaml) + validate_decompressed_file_size if Feature.enabled?(:validate_import_decompressed_archive_size) validate_symlink(filepath) decompress_file diff --git a/app/services/ci/create_downstream_pipeline_service.rb b/app/services/ci/create_downstream_pipeline_service.rb index 0a0c614bb87..b38b3b93353 100644 --- a/app/services/ci/create_downstream_pipeline_service.rb +++ b/app/services/ci/create_downstream_pipeline_service.rb @@ -6,6 +6,7 @@ module Ci # specifications. class CreateDownstreamPipelineService < ::BaseService include Gitlab::Utils::StrongMemoize + include Ci::DownstreamPipelineHelpers DuplicateDownstreamPipelineError = Class.new(StandardError) @@ -37,6 +38,8 @@ module Ci .execute(pipeline_params.fetch(:source), **pipeline_params[:execute_params]) .payload + log_downstream_pipeline_creation(downstream_pipeline) + downstream_pipeline.tap do |pipeline| update_bridge_status!(@bridge, pipeline) end diff --git a/app/services/ci/expire_pipeline_cache_service.rb b/app/services/ci/expire_pipeline_cache_service.rb index 8622b1a5863..bf2355c447a 100644 --- a/app/services/ci/expire_pipeline_cache_service.rb +++ b/app/services/ci/expire_pipeline_cache_service.rb @@ -86,7 +86,7 @@ module Ci etag_paths << path end - pipeline.self_with_upstreams_and_downstreams.includes(project: [:route, { namespace: :route }]).each do |relative_pipeline| # rubocop: disable CodeReuse/ActiveRecord + pipeline.all_pipelines_in_hierarchy.includes(project: [:route, { namespace: :route }]).each do |relative_pipeline| # rubocop: disable CodeReuse/ActiveRecord etag_paths << project_pipeline_path(relative_pipeline.project, relative_pipeline) etag_paths << graphql_pipeline_path(relative_pipeline) etag_paths << graphql_pipeline_sha_path(relative_pipeline.sha) diff --git a/app/services/ci/generate_kubeconfig_service.rb b/app/services/ci/generate_kubeconfig_service.rb index 18f68c0ff09..894ab8e8505 100644 --- a/app/services/ci/generate_kubeconfig_service.rb +++ b/app/services/ci/generate_kubeconfig_service.rb @@ -2,8 +2,9 @@ module Ci class GenerateKubeconfigService - def initialize(build) - @build = build + def initialize(pipeline, token:) + @pipeline = pipeline + @token = token @template = Gitlab::Kubernetes::Kubeconfig::Template.new end @@ -33,10 +34,10 @@ module Ci private - attr_reader :build, :template + attr_reader :pipeline, :token, :template def agents - build.pipeline.authorized_cluster_agents + pipeline.authorized_cluster_agents end def cluster_name @@ -52,7 +53,7 @@ module Ci end def agent_token(agent) - ['ci', agent.id, build.token].join(delimiter) + ['ci', agent.id, token].join(delimiter) end def delimiter diff --git a/app/services/ci/job_artifacts/create_service.rb b/app/services/ci/job_artifacts/create_service.rb index 7c67a2e175d..635111130d6 100644 --- a/app/services/ci/job_artifacts/create_service.rb +++ b/app/services/ci/job_artifacts/create_service.rb @@ -133,7 +133,7 @@ module Ci job.update_column(:artifacts_expire_at, artifact.expire_at) end - success + success(artifact: artifact) rescue ActiveRecord::RecordNotUnique => error track_exception(error, params) error('another artifact of the same type already exists', :bad_request) diff --git a/app/services/ci/job_artifacts/destroy_batch_service.rb b/app/services/ci/job_artifacts/destroy_batch_service.rb index 90d157373c3..5121a8b0a8b 100644 --- a/app/services/ci/job_artifacts/destroy_batch_service.rb +++ b/app/services/ci/job_artifacts/destroy_batch_service.rb @@ -33,9 +33,11 @@ module Ci destroy_related_records(@job_artifacts) - Ci::DeletedObject.transaction do - Ci::DeletedObject.bulk_import(@job_artifacts, @pick_up_at) - Ci::JobArtifact.id_in(@job_artifacts.map(&:id)).delete_all + destroy_around_hook(@job_artifacts) do + Ci::DeletedObject.transaction do + Ci::DeletedObject.bulk_import(@job_artifacts, @pick_up_at) + Ci::JobArtifact.id_in(@job_artifacts.map(&:id)).delete_all + end end after_batch_destroy_hook(@job_artifacts) @@ -52,6 +54,13 @@ module Ci private # Overriden in EE + # :nocov: + def destroy_around_hook(artifacts) + yield + end + # :nocov: + + # Overriden in EE def destroy_related_records(artifacts); end # Overriden in EE @@ -121,7 +130,7 @@ module Ci end def fix_expire_at? - Feature.enabled?(:ci_detect_wrongly_expired_artifacts, default_enabled: :yaml) + Feature.enabled?(:ci_detect_wrongly_expired_artifacts) end def wrongly_expired?(artifact) diff --git a/app/services/ci/pipeline_creation/start_pipeline_service.rb b/app/services/ci/pipeline_creation/start_pipeline_service.rb index 27c12caaa0a..65a045f32dd 100644 --- a/app/services/ci/pipeline_creation/start_pipeline_service.rb +++ b/app/services/ci/pipeline_creation/start_pipeline_service.rb @@ -10,6 +10,11 @@ module Ci end def execute + ## + # Create a persistent ref for the pipeline. + # The pipeline ref is fetched in the jobs and deleted when the pipeline transitions to a finished state. + pipeline.ensure_persistent_ref + Ci::ProcessPipelineService.new(pipeline).execute end end diff --git a/app/services/ci/pipeline_trigger_service.rb b/app/services/ci/pipeline_trigger_service.rb index 7746382b845..06eb1aee8e6 100644 --- a/app/services/ci/pipeline_trigger_service.rb +++ b/app/services/ci/pipeline_trigger_service.rb @@ -4,6 +4,7 @@ module Ci class PipelineTriggerService < BaseService include Gitlab::Utils::StrongMemoize include Services::ReturnServiceResponses + include Ci::DownstreamPipelineHelpers def execute if trigger_from_token @@ -69,6 +70,7 @@ module Ci pipeline.source_pipeline = source end + log_downstream_pipeline_creation(response.payload) pipeline_service_response(response.payload) end diff --git a/app/services/ci/queue/build_queue_service.rb b/app/services/ci/queue/build_queue_service.rb index 9f476c8a785..fefbdb151ec 100644 --- a/app/services/ci/queue/build_queue_service.rb +++ b/app/services/ci/queue/build_queue_service.rb @@ -80,7 +80,7 @@ module Ci def strategy strong_memoize(:strategy) do - if ::Feature.enabled?(:ci_pending_builds_queue_source, runner, default_enabled: :yaml) + if ::Feature.enabled?(:ci_pending_builds_queue_source, runner) Queue::PendingBuildsStrategy.new(runner) else Queue::BuildsTableStrategy.new(runner) diff --git a/app/services/ci/queue/builds_table_strategy.rb b/app/services/ci/queue/builds_table_strategy.rb index 237dd510d50..c27c10bd18d 100644 --- a/app/services/ci/queue/builds_table_strategy.rb +++ b/app/services/ci/queue/builds_table_strategy.rb @@ -18,7 +18,7 @@ module Ci .joins('LEFT JOIN project_features ON ci_builds.project_id = project_features.project_id') .where('project_features.builds_access_level IS NULL or project_features.builds_access_level > 0') - if Feature.enabled?(:ci_queueing_disaster_recovery_disable_fair_scheduling, runner, type: :ops, default_enabled: :yaml) + if Feature.enabled?(:ci_queueing_disaster_recovery_disable_fair_scheduling, runner, type: :ops) # if disaster recovery is enabled, we fallback to FIFO scheduling relation.order('ci_builds.id ASC') else diff --git a/app/services/ci/queue/pending_builds_strategy.rb b/app/services/ci/queue/pending_builds_strategy.rb index 47158b8ea1d..f2eba0681db 100644 --- a/app/services/ci/queue/pending_builds_strategy.rb +++ b/app/services/ci/queue/pending_builds_strategy.rb @@ -51,7 +51,7 @@ module Ci end def use_denormalized_data_strategy? - ::Feature.enabled?(:ci_queuing_use_denormalized_data_strategy, default_enabled: :yaml) + ::Feature.enabled?(:ci_queuing_use_denormalized_data_strategy) end private @@ -70,7 +70,7 @@ module Ci end def builds_ordered_for_shared_runners(relation) - if Feature.enabled?(:ci_queueing_disaster_recovery_disable_fair_scheduling, runner, type: :ops, default_enabled: :yaml) + if Feature.enabled?(:ci_queueing_disaster_recovery_disable_fair_scheduling, runner, type: :ops) # if disaster recovery is enabled, we fallback to FIFO scheduling relation.order('ci_pending_builds.build_id ASC') else diff --git a/app/services/ci/register_job_service.rb b/app/services/ci/register_job_service.rb index 6c9044b5089..8969b95b81f 100644 --- a/app/services/ci/register_job_service.rb +++ b/app/services/ci/register_job_service.rb @@ -157,15 +157,13 @@ module Ci unless build.pending? @metrics.increment_queue_operation(:build_not_pending) - if Feature.enabled?(:ci_pending_builds_table_resiliency, default_enabled: :yaml) - ## - # If this build can not be picked because we had stale data in - # `ci_pending_builds` table, we need to respond with 409 to retry - # this operation. - # - if ::Ci::UpdateBuildQueueService.new.remove!(build) - return Result.new(nil, nil, false) - end + ## + # If this build can not be picked because we had stale data in + # `ci_pending_builds` table, we need to respond with 409 to retry + # this operation. + # + if ::Ci::UpdateBuildQueueService.new.remove!(build) + return Result.new(nil, nil, false) end return diff --git a/app/services/ci/retry_job_service.rb b/app/services/ci/retry_job_service.rb index af7e7fa16e9..e0ced3d0197 100644 --- a/app/services/ci/retry_job_service.rb +++ b/app/services/ci/retry_job_service.rb @@ -23,11 +23,11 @@ module Ci # Cloning a job requires a strict type check to ensure # the attributes being used for the clone are taken straight # from the model and not overridden by other abstractions. - raise TypeError unless job.instance_of?(Ci::Build) + raise TypeError unless job.instance_of?(Ci::Build) || job.instance_of?(Ci::Bridge) check_access!(job) - new_job = clone_job(job) + new_job = job.clone(current_user: current_user) new_job.run_after_commit do ::Ci::CopyCrossDatabaseAssociationsService.new.execute(job, new_job) @@ -53,9 +53,12 @@ module Ci private + def check_assignable_runners!(job); end + def retry_job(job) clone!(job).tap do |new_job| - check_assignable_runners!(new_job) + check_assignable_runners!(new_job) if new_job.is_a?(Ci::Build) + next if new_job.failed? Gitlab::OptimisticLocking.retry_lock(new_job, name: 'retry_build', &:enqueue) @@ -68,26 +71,6 @@ module Ci raise Gitlab::Access::AccessDeniedError, '403 Forbidden' end end - - def check_assignable_runners!(job); end - - def clone_job(job) - project.builds.new(job_attributes(job)) - end - - def job_attributes(job) - attributes = job.class.clone_accessors.to_h do |attribute| - [attribute, job.public_send(attribute)] # rubocop:disable GitlabSecurity/PublicSend - end - - if job.persisted_environment.present? - attributes[:metadata_attributes] ||= {} - attributes[:metadata_attributes][:expanded_environment_name] = job.expanded_environment_name - end - - attributes[:user] = current_user - attributes - end end end diff --git a/app/services/ci/runners/register_runner_service.rb b/app/services/ci/runners/register_runner_service.rb index 7978d094d9b..196d2de1a65 100644 --- a/app/services/ci/runners/register_runner_service.rb +++ b/app/services/ci/runners/register_runner_service.rb @@ -47,7 +47,7 @@ module Ci end def runner_registrar_valid?(type) - Feature.disabled?(:runner_registration_control, default_enabled: :yaml) || Gitlab::CurrentSettings.valid_runner_registrars.include?(type) + Feature.disabled?(:runner_registration_control) || Gitlab::CurrentSettings.valid_runner_registrars.include?(type) end def token_scope diff --git a/app/services/ci/runners/reset_registration_token_service.rb b/app/services/ci/runners/reset_registration_token_service.rb index bbe49c04644..2a3fb08c5e1 100644 --- a/app/services/ci/runners/reset_registration_token_service.rb +++ b/app/services/ci/runners/reset_registration_token_service.rb @@ -29,3 +29,5 @@ module Ci end end end + +Ci::Runners::ResetRegistrationTokenService.prepend_mod diff --git a/app/services/ci/stuck_builds/drop_running_service.rb b/app/services/ci/stuck_builds/drop_running_service.rb index a79224cc231..dfcf3ca3836 100644 --- a/app/services/ci/stuck_builds/drop_running_service.rb +++ b/app/services/ci/stuck_builds/drop_running_service.rb @@ -16,7 +16,7 @@ module Ci private def running_timed_out_builds - if Feature.enabled?(:ci_new_query_for_running_stuck_jobs, default_enabled: :yaml) + if Feature.enabled?(:ci_new_query_for_running_stuck_jobs) Ci::Build .running .created_at_before(BUILD_RUNNING_OUTDATED_TIMEOUT.ago) diff --git a/app/services/ci/update_build_state_service.rb b/app/services/ci/update_build_state_service.rb index 9df36b86404..a74ddcfaf06 100644 --- a/app/services/ci/update_build_state_service.rb +++ b/app/services/ci/update_build_state_service.rb @@ -217,11 +217,11 @@ module Ci def chunks_migration_enabled? ::Feature.enabled?(:ci_enable_live_trace, build.project) && - ::Feature.enabled?(:ci_accept_trace, build.project, type: :ops, default_enabled: true) + ::Feature.enabled?(:ci_accept_trace, build.project, type: :ops) end def log_invalid_chunks? - ::Feature.enabled?(:ci_trace_log_invalid_chunks, build.project, type: :ops, default_enabled: false) + ::Feature.enabled?(:ci_trace_log_invalid_chunks, build.project, type: :ops) end end end diff --git a/app/services/clusters/kubernetes.rb b/app/services/clusters/kubernetes.rb index ef549b56946..819ac4c8464 100644 --- a/app/services/clusters/kubernetes.rb +++ b/app/services/clusters/kubernetes.rb @@ -14,7 +14,5 @@ module Clusters GITLAB_CROSSPLANE_DATABASE_ROLE_BINDING_NAME = 'gitlab-crossplane-database-rolebinding' KNATIVE_SERVING_NAMESPACE = 'knative-serving' ISTIO_SYSTEM_NAMESPACE = 'istio-system' - GITLAB_CILIUM_ROLE_NAME = 'gitlab-cilium-role' - GITLAB_CILIUM_ROLE_BINDING_NAME = 'gitlab-cilium-rolebinding' end end diff --git a/app/services/clusters/kubernetes/create_or_update_service_account_service.rb b/app/services/clusters/kubernetes/create_or_update_service_account_service.rb index ecad33fc7c0..eabc428d0d2 100644 --- a/app/services/clusters/kubernetes/create_or_update_service_account_service.rb +++ b/app/services/clusters/kubernetes/create_or_update_service_account_service.rb @@ -53,8 +53,6 @@ module Clusters create_or_update_knative_serving_role_binding create_or_update_crossplane_database_role create_or_update_crossplane_database_role_binding - create_or_update_cilium_role - create_or_update_cilium_role_binding end private @@ -99,14 +97,6 @@ module Clusters kubeclient.update_role_binding(crossplane_database_role_binding_resource) end - def create_or_update_cilium_role - kubeclient.update_role(cilium_role_resource) - end - - def create_or_update_cilium_role_binding - kubeclient.update_role_binding(cilium_role_binding_resource) - end - def service_account_resource Gitlab::Kubernetes::ServiceAccount.new( service_account_name, @@ -185,28 +175,6 @@ module Clusters service_account_name: service_account_name ).generate end - - def cilium_role_resource - Gitlab::Kubernetes::Role.new( - name: Clusters::Kubernetes::GITLAB_CILIUM_ROLE_NAME, - namespace: service_account_namespace, - rules: [{ - apiGroups: %w(cilium.io), - resources: %w(ciliumnetworkpolicies), - verbs: %w(get list create update patch) - }] - ).generate - end - - def cilium_role_binding_resource - Gitlab::Kubernetes::RoleBinding.new( - name: Clusters::Kubernetes::GITLAB_CILIUM_ROLE_BINDING_NAME, - role_name: Clusters::Kubernetes::GITLAB_CILIUM_ROLE_NAME, - role_kind: :Role, - namespace: service_account_namespace, - service_account_name: service_account_name - ).generate - end end end end diff --git a/app/services/concerns/alert_management/alert_processing.rb b/app/services/concerns/alert_management/alert_processing.rb index 2b556a4339d..abbfd4d66d4 100644 --- a/app/services/concerns/alert_management/alert_processing.rb +++ b/app/services/concerns/alert_management/alert_processing.rb @@ -104,7 +104,7 @@ module AlertManagement def find_existing_alert return unless incoming_payload.gitlab_fingerprint - AlertManagement::Alert.not_resolved.for_fingerprint(project, incoming_payload.gitlab_fingerprint).first + AlertManagement::Alert.find_unresolved_alert(project, incoming_payload.gitlab_fingerprint) end def build_new_alert diff --git a/app/services/concerns/ci/downstream_pipeline_helpers.rb b/app/services/concerns/ci/downstream_pipeline_helpers.rb new file mode 100644 index 00000000000..39c0adb6e4e --- /dev/null +++ b/app/services/concerns/ci/downstream_pipeline_helpers.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Ci + module DownstreamPipelineHelpers + def log_downstream_pipeline_creation(downstream_pipeline) + return unless downstream_pipeline&.persisted? + + hierarchy_size = downstream_pipeline.all_pipelines_in_hierarchy.count + root_pipeline = downstream_pipeline.upstream_root + + ::Gitlab::AppLogger.info( + message: "downstream pipeline created", + class: self.class.name, + root_pipeline_id: root_pipeline.id, + downstream_pipeline_id: downstream_pipeline.id, + downstream_pipeline_relationship: downstream_pipeline.parent_pipeline? ? :parent_child : :multi_project, + hierarchy_size: hierarchy_size, + root_pipeline_plan: root_pipeline.project.actual_plan_name, + root_pipeline_namespace_path: root_pipeline.project.namespace.full_path, + root_pipeline_project_path: root_pipeline.project.full_path + ) + end + end +end diff --git a/app/services/concerns/group_linkable.rb b/app/services/concerns/group_linkable.rb new file mode 100644 index 00000000000..43d10e01a4a --- /dev/null +++ b/app/services/concerns/group_linkable.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module GroupLinkable + extend ActiveSupport::Concern + + def execute + return error('Not Found', 404) unless valid_to_create? + + build_link + + if link.save + after_successful_save + success(link: link) + else + error(link.errors.full_messages.to_sentence, 409) + end + end + + private + + attr_reader :shared_with_group, :link + + def sharing_allowed? + sharing_outside_hierarchy_allowed? || within_hierarchy? + end + + def sharing_outside_hierarchy_allowed? + !root_ancestor.prevent_sharing_groups_outside_hierarchy + end + + def within_hierarchy? + root_ancestor.self_and_descendants_ids.include?(shared_with_group.id) + end + + def after_successful_save + setup_authorizations + end +end diff --git a/app/services/container_expiration_policies/cleanup_service.rb b/app/services/container_expiration_policies/cleanup_service.rb index 0da5e552c48..34889e58127 100644 --- a/app/services/container_expiration_policies/cleanup_service.rb +++ b/app/services/container_expiration_policies/cleanup_service.rb @@ -35,7 +35,8 @@ module ContainerExpirationPolicies if service_result[:status] == :success repository.update!( expiration_policy_cleanup_status: :cleanup_unscheduled, - expiration_policy_completed_at: Time.zone.now + expiration_policy_completed_at: Time.zone.now, + last_cleanup_deleted_tags_count: service_result[:deleted_size] ) success(:finished, service_result) diff --git a/app/services/container_expiration_policies/update_service.rb b/app/services/container_expiration_policies/update_service.rb index 2f34941d692..32ee47457b7 100644 --- a/app/services/container_expiration_policies/update_service.rb +++ b/app/services/container_expiration_policies/update_service.rb @@ -28,7 +28,7 @@ module ContainerExpirationPolicies end def allowed? - Ability.allowed?(current_user, :destroy_container_image, @container) + Ability.allowed?(current_user, :admin_container_image, @container) end def container_expiration_policy_params diff --git a/app/services/container_expiration_policy_service.rb b/app/services/container_expiration_policy_service.rb deleted file mode 100644 index cf5d702a9ef..00000000000 --- a/app/services/container_expiration_policy_service.rb +++ /dev/null @@ -1,18 +0,0 @@ -# frozen_string_literal: true - -class ContainerExpirationPolicyService < BaseService - InvalidPolicyError = Class.new(StandardError) - - def execute(container_expiration_policy) - container_expiration_policy.schedule_next_run! - - container_expiration_policy.container_repositories.find_each do |container_repository| - CleanupContainerRepositoryWorker.perform_async( - nil, - container_repository.id, - container_expiration_policy.policy_params - .merge(container_expiration_policy: true) - ) - end - end -end diff --git a/app/services/customer_relations/contacts/base_service.rb b/app/services/customer_relations/contacts/base_service.rb index 1797e5021a1..4c6bcabe9e0 100644 --- a/app/services/customer_relations/contacts/base_service.rb +++ b/app/services/customer_relations/contacts/base_service.rb @@ -12,6 +12,23 @@ module CustomerRelations def error(message) ServiceResponse.error(message: Array(message)) end + + def organization_valid? + return true unless params[:organization_id] + + organization = Organization.find(params[:organization_id]) + organization.group_id == group.id + rescue ActiveRecord::RecordNotFound + false + end + + def error_organization_invalid + error('The specified organization was not found or does not belong to this group') + end + + def error_no_permissions + error('You have insufficient permissions to manage contacts for this group') + end end end end diff --git a/app/services/customer_relations/contacts/create_service.rb b/app/services/customer_relations/contacts/create_service.rb index 7ff8b731e0d..2fabc51eebf 100644 --- a/app/services/customer_relations/contacts/create_service.rb +++ b/app/services/customer_relations/contacts/create_service.rb @@ -8,7 +8,6 @@ module CustomerRelations return error_organization_invalid unless organization_valid? contact = Contact.create(params.merge(group_id: group.id)) - return error_creating(contact) unless contact.persisted? ServiceResponse.success(payload: contact) @@ -16,23 +15,6 @@ module CustomerRelations private - def organization_valid? - return true unless params[:organization_id] - - organization = Organization.find(params[:organization_id]) - organization.group_id == group.id - rescue ActiveRecord::RecordNotFound - false - end - - def error_organization_invalid - error('The specified organization was not found or does not belong to this group') - end - - def error_no_permissions - error('You have insufficient permissions to create a contact for this group') - end - def error_creating(contact) error(contact&.errors&.full_messages || 'Failed to create contact') end diff --git a/app/services/customer_relations/contacts/update_service.rb b/app/services/customer_relations/contacts/update_service.rb index 473a80be262..66eb5731bc9 100644 --- a/app/services/customer_relations/contacts/update_service.rb +++ b/app/services/customer_relations/contacts/update_service.rb @@ -5,6 +5,9 @@ module CustomerRelations class UpdateService < BaseService def execute(contact) return error_no_permissions unless allowed? + + handle_active_param + return error_organization_invalid unless organization_valid? return error_updating(contact) unless contact.update(params) ServiceResponse.success(payload: contact) @@ -12,8 +15,11 @@ module CustomerRelations private - def error_no_permissions - error('You have insufficient permissions to update a contact for this group') + def handle_active_param + return if params[:active].nil? + + active = params.delete(:active) + params[:state] = active ? 'active' : 'inactive' end def error_updating(contact) diff --git a/app/services/customer_relations/organizations/update_service.rb b/app/services/customer_relations/organizations/update_service.rb index 9d8f908db14..78fff9cf8f4 100644 --- a/app/services/customer_relations/organizations/update_service.rb +++ b/app/services/customer_relations/organizations/update_service.rb @@ -5,6 +5,8 @@ module CustomerRelations class UpdateService < BaseService def execute(organization) return error_no_permissions unless allowed? + + handle_active_param return error_updating(organization) unless organization.update(params) ServiceResponse.success(payload: organization) @@ -12,6 +14,13 @@ module CustomerRelations private + def handle_active_param + return if params[:active].nil? + + active = params.delete(:active) + params[:state] = active ? 'active' : 'inactive' + end + def error_no_permissions error('You have insufficient permissions to update an organization for this group') end diff --git a/app/services/database/consistency_fix_service.rb b/app/services/database/consistency_fix_service.rb new file mode 100644 index 00000000000..402fa7541b3 --- /dev/null +++ b/app/services/database/consistency_fix_service.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Database + class ConsistencyFixService + def initialize(source_model:, target_model:, sync_event_class:, source_sort_key:, target_sort_key:) + @source_model = source_model + @target_model = target_model + @sync_event_class = sync_event_class + @source_sort_key = source_sort_key + @target_sort_key = target_sort_key + end + + attr_accessor :source_model, :target_model, :sync_event_class, :source_sort_key, :target_sort_key + + def execute(ids:) + ids.each do |id| + if source_object(id) && target_object(id) + create_sync_event_for(id) + elsif target_object(id) + target_object(id).destroy! + end + end + sync_event_class.enqueue_worker + end + + private + + # rubocop: disable CodeReuse/ActiveRecord + def source_object(id) + source_model.find_by(source_sort_key => id) + end + + def target_object(id) + target_model.find_by(target_sort_key => id) + end + # rubocop: enable CodeReuse/ActiveRecord + + def create_sync_event_for(id) + if source_model == Namespace + sync_event_class.create!(namespace_id: id) + elsif source_model == Project + sync_event_class.create!(project_id: id) + else + raise("Unknown Source Model #{source_model.name}") + end + end + end +end diff --git a/app/services/deployments/update_environment_service.rb b/app/services/deployments/update_environment_service.rb index b0ba8ecaa47..b0eb153a7af 100644 --- a/app/services/deployments/update_environment_service.rb +++ b/app/services/deployments/update_environment_service.rb @@ -58,7 +58,7 @@ module Deployments def expanded_environment_url return unless environment_url - if ::Feature.enabled?(:ci_expand_environment_name_and_url, deployment.project, default_enabled: :yaml) + if ::Feature.enabled?(:ci_expand_environment_name_and_url, deployment.project) ExpandVariables.expand(environment_url, -> { variables.sort_and_expand_all }) else ExpandVariables.expand(environment_url, -> { variables }) diff --git a/app/services/environments/stop_service.rb b/app/services/environments/stop_service.rb index 24ae658d3d6..54ad94947ff 100644 --- a/app/services/environments/stop_service.rb +++ b/app/services/environments/stop_service.rb @@ -18,7 +18,9 @@ module Environments environments.each { |environment| execute(environment) } end - def execute_for_merge_request(merge_request) + def execute_for_merge_request_pipeline(merge_request) + return unless merge_request.actual_head_pipeline&.merge_request? + merge_request.environments_in_head_pipeline(deployment_status: :success).each do |environment| execute(environment) end diff --git a/app/services/error_tracking/base_service.rb b/app/services/error_tracking/base_service.rb index 598621f70e1..d2ecd0a6d5a 100644 --- a/app/services/error_tracking/base_service.rb +++ b/app/services/error_tracking/base_service.rb @@ -71,5 +71,15 @@ module ErrorTracking def can_update? can?(current_user, :update_sentry_issue, project) end + + def error_repository + Gitlab::ErrorTracking::ErrorRepository.build(project) + end + + def handle_error_repository_exceptions + yield + rescue Gitlab::ErrorTracking::ErrorRepository::DatabaseError => e + { error: e.message } + end end end diff --git a/app/services/error_tracking/collect_error_service.rb b/app/services/error_tracking/collect_error_service.rb index 6376b743255..8cb3793ba97 100644 --- a/app/services/error_tracking/collect_error_service.rb +++ b/app/services/error_tracking/collect_error_service.rb @@ -5,30 +5,24 @@ module ErrorTracking include Gitlab::Utils::StrongMemoize def execute - # Error is a way to group events based on common data like name or cause - # of exception. We need to keep a sane balance here between taking too little - # and too much data into group logic. - error = project.error_tracking_errors.report_error( - name: exception['type'], # Example: ActionView::MissingTemplate - description: exception['value'], # Example: Missing template posts/show in... - actor: actor, # Example: PostsController#show - platform: event['platform'], # Example: ruby - timestamp: timestamp - ) - - # The payload field contains all the data on error including stacktrace in jsonb. - # Together with occurred_at these are 2 main attributes that we need to save here. - error.events.create!( - environment: event['environment'], + error_repository.report_error( + name: exception['type'], description: exception['value'], - level: event['level'], + actor: actor, + platform: event['platform'], occurred_at: timestamp, + environment: event['environment'], + level: event['level'], payload: event ) end private + def error_repository + Gitlab::ErrorTracking::ErrorRepository.build(project) + end + def event @event ||= format_event(params[:event]) end diff --git a/app/services/error_tracking/issue_details_service.rb b/app/services/error_tracking/issue_details_service.rb index 1614c597a8e..e82ad540e57 100644 --- a/app/services/error_tracking/issue_details_service.rb +++ b/app/services/error_tracking/issue_details_service.rb @@ -49,13 +49,10 @@ module ErrorTracking # Issue https://gitlab.com/gitlab-org/gitlab/-/issues/329596 # if project_error_tracking_setting.integrated_client? - error = project.error_tracking_errors.find(issue_id) - - # We use the same response format as project_error_tracking_setting - # method below for compatibility with existing code. - { - issue: error.to_sentry_detailed_error - } + handle_error_repository_exceptions do + error = error_repository.find_error(issue_id) + { issue: error } + end else project_error_tracking_setting.issue_details(issue_id: issue_id) end diff --git a/app/services/error_tracking/issue_latest_event_service.rb b/app/services/error_tracking/issue_latest_event_service.rb index 1bf86c658fc..0290c8eac86 100644 --- a/app/services/error_tracking/issue_latest_event_service.rb +++ b/app/services/error_tracking/issue_latest_event_service.rb @@ -26,14 +26,13 @@ module ErrorTracking # Issue https://gitlab.com/gitlab-org/gitlab/-/issues/329596 # if project_error_tracking_setting.integrated_client? - error = project.error_tracking_errors.find(issue_id) - event = error.events.last + handle_error_repository_exceptions do + event = error_repository.last_event_for(issue_id) - # We use the same response format as project_error_tracking_setting - # method below for compatibility with existing code. - { - latest_event: event.to_sentry_error_event - } + # We use the same response format as project_error_tracking_setting + # method below for compatibility with existing code. + { latest_event: event } + end else project_error_tracking_setting.issue_latest_event(issue_id: issue_id) end diff --git a/app/services/error_tracking/issue_update_service.rb b/app/services/error_tracking/issue_update_service.rb index 624e5f94dde..ca5e8d656a6 100644 --- a/app/services/error_tracking/issue_update_service.rb +++ b/app/services/error_tracking/issue_update_service.rb @@ -84,14 +84,12 @@ module ErrorTracking # Issue https://gitlab.com/gitlab-org/gitlab/-/issues/329596 # if project_error_tracking_setting.integrated_client? - error = project.error_tracking_errors.find(opts[:issue_id]) - error.status = opts[:params][:status] - error.save! + updated = error_repository.update_error(opts[:issue_id], status: opts[:params][:status]) # We use the same response format as project_error_tracking_setting # method below for compatibility with existing code. { - updated: true + updated: updated } else project_error_tracking_setting.update_issue(**opts) diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb index 1979816b88d..ca7208dba96 100644 --- a/app/services/error_tracking/list_issues_service.rb +++ b/app/services/error_tracking/list_issues_service.rb @@ -73,24 +73,24 @@ module ErrorTracking if project_error_tracking_setting.integrated_client? # We are going to support more options in the future. # For now we implement the bare minimum for rendering the list in UI. - filter_opts = { - status: opts[:issue_status], + list_opts = { + filters: { status: opts[:issue_status] }, sort: opts[:sort], limit: opts[:limit], cursor: opts[:cursor] } - errors = ErrorTracking::ErrorsFinder.new(current_user, project, filter_opts).execute + errors, pagination = error_repository.list_errors(**list_opts) - pagination = {} - pagination[:next] = { cursor: errors.cursor_for_next_page } if errors.has_next_page? - pagination[:previous] = { cursor: errors.cursor_for_previous_page } if errors.has_previous_page? + pagination_hash = {} + pagination_hash[:next] = { cursor: pagination.next } if pagination.next + pagination_hash[:previous] = { cursor: pagination.prev } if pagination.prev # We use the same response format as project_error_tracking_setting # method below for compatibility with existing code. { - issues: errors.map(&:to_sentry_error), - pagination: pagination + issues: errors, + pagination: pagination_hash } else project_error_tracking_setting.list_sentry_issues(**opts) diff --git a/app/services/event_create_service.rb b/app/services/event_create_service.rb index 417680e37cf..5a2c29f8e7a 100644 --- a/app/services/event_create_service.rb +++ b/app/services/event_create_service.rb @@ -185,7 +185,7 @@ class EventCreateService track_event(event_action: :pushed, event_target: Project, author_id: current_user.id) namespace = project.namespace - if Feature.enabled?(:route_hll_to_snowplow, namespace, default_enabled: :yaml) + if Feature.enabled?(:route_hll_to_snowplow, namespace) Gitlab::Tracking.event(self.class.to_s, 'action_active_users_project_repo', namespace: namespace, user: current_user, project: project) end diff --git a/app/services/git/base_hooks_service.rb b/app/services/git/base_hooks_service.rb index 63f3f73905a..269637805ad 100644 --- a/app/services/git/base_hooks_service.rb +++ b/app/services/git/base_hooks_service.rb @@ -172,7 +172,7 @@ module Git else # This service runs in Sidekiq, so this shouldn't ever be # called, but this is included just in case. - Gitlab::ProjectServiceLogger + Gitlab::IntegrationsLogger end end end diff --git a/app/services/groups/group_links/create_service.rb b/app/services/groups/group_links/create_service.rb index 8c3ba0a63f2..56ddf3ec0b4 100644 --- a/app/services/groups/group_links/create_service.rb +++ b/app/services/groups/group_links/create_service.rb @@ -3,50 +3,35 @@ module Groups module GroupLinks class CreateService < Groups::BaseService - def initialize(shared_group, shared_with_group, user, params) - @shared_group = shared_group - super(shared_with_group, user, params) - end - - def execute - unless shared_with_group && shared_group && - can?(current_user, :admin_group_member, shared_group) && - can?(current_user, :read_group, shared_with_group) && - sharing_allowed? - return error('Not Found', 404) - end + include GroupLinkable - link = GroupGroupLink.new( - shared_group: shared_group, - shared_with_group: shared_with_group, - group_access: params[:shared_group_access], - expires_at: params[:expires_at] - ) + def initialize(group, shared_with_group, user, params) + @shared_with_group = shared_with_group - if link.save - shared_with_group.refresh_members_authorized_projects(blocking: false, direct_members_only: true) - success(link: link) - else - error(link.errors.full_messages.to_sentence, 409) - end + super(group, user, params) end private - attr_reader :shared_group + delegate :root_ancestor, to: :group - alias_method :shared_with_group, :group - - def sharing_allowed? - sharing_outside_hierarchy_allowed? || within_hierarchy? + def valid_to_create? + can?(current_user, :admin_group_member, group) && + can?(current_user, :read_group, shared_with_group) && + sharing_allowed? end - def sharing_outside_hierarchy_allowed? - !shared_group.root_ancestor.namespace_settings.prevent_sharing_groups_outside_hierarchy + def build_link + @link = GroupGroupLink.new( + shared_group: group, + shared_with_group: shared_with_group, + group_access: params[:shared_group_access], + expires_at: params[:expires_at] + ) end - def within_hierarchy? - shared_group.root_ancestor.self_and_descendants_ids.include?(shared_with_group.id) + def setup_authorizations + shared_with_group.refresh_members_authorized_projects(blocking: false, direct_members_only: true) end end end diff --git a/app/services/groups/import_export/export_service.rb b/app/services/groups/import_export/export_service.rb index ea26ebec20b..2bfd5a5ebab 100644 --- a/app/services/groups/import_export/export_service.rb +++ b/app/services/groups/import_export/export_service.rb @@ -78,7 +78,7 @@ module Groups end def ndjson? - ::Feature.enabled?(:group_export_ndjson, group&.parent, default_enabled: :yaml) + ::Feature.enabled?(:group_export_ndjson, group&.parent) end def version_saver diff --git a/app/services/groups/import_export/import_service.rb b/app/services/groups/import_export/import_service.rb index c8c2124078d..f026f1698a9 100644 --- a/app/services/groups/import_export/import_service.rb +++ b/app/services/groups/import_export/import_service.rb @@ -72,7 +72,7 @@ module Groups end def ndjson? - ::Feature.enabled?(:group_import_ndjson, group&.parent, default_enabled: true) && + ::Feature.enabled?(:group_import_ndjson, group&.parent) && File.exist?(File.join(shared.export_path, 'tree/groups/_all.ndjson')) end diff --git a/app/services/groups/open_issues_count_service.rb b/app/services/groups/open_issues_count_service.rb index c18d239998b..17cf3d38987 100644 --- a/app/services/groups/open_issues_count_service.rb +++ b/app/services/groups/open_issues_count_service.rb @@ -3,15 +3,11 @@ module Groups # Service class for counting and caching the number of open issues of a group. class OpenIssuesCountService < Groups::CountService - # TOTAL_COUNT_KEY includes confidential and hidden issues (admin) - # TOTAL_COUNT_WITHOUT_HIDDEN_KEY includes confidential issues but not hidden issues (reporter and above) - # PUBLIC_COUNT_WITHOUT_HIDDEN_KEY does not include confidential or hidden issues (guest) - TOTAL_COUNT_KEY = 'group_open_issues_including_hidden_count' - TOTAL_COUNT_WITHOUT_HIDDEN_KEY = 'group_open_issues_without_hidden_count' - PUBLIC_COUNT_WITHOUT_HIDDEN_KEY = 'group_open_public_issues_without_hidden_count' + PUBLIC_COUNT_KEY = 'group_public_open_issues_count' + TOTAL_COUNT_KEY = 'group_total_open_issues_count' def clear_all_cache_keys - [cache_key(TOTAL_COUNT_KEY), cache_key(TOTAL_COUNT_WITHOUT_HIDDEN_KEY), cache_key(PUBLIC_COUNT_WITHOUT_HIDDEN_KEY)].each do |key| + [cache_key(PUBLIC_COUNT_KEY), cache_key(TOTAL_COUNT_KEY)].each do |key| Rails.cache.delete(key) end end @@ -19,19 +15,7 @@ module Groups private def cache_key_name - if include_hidden? - TOTAL_COUNT_KEY - elsif public_only? - PUBLIC_COUNT_WITHOUT_HIDDEN_KEY - else - TOTAL_COUNT_WITHOUT_HIDDEN_KEY - end - end - - def include_hidden? - strong_memoize(:user_is_admin) do - user&.can_admin_all_resources? - end + public_only? ? PUBLIC_COUNT_KEY : TOTAL_COUNT_KEY end def public_only? @@ -51,8 +35,7 @@ module Groups state: 'opened', non_archived: true, include_subgroups: true, - public_only: public_only?, - include_hidden: include_hidden? + public_only: public_only? ).execute end diff --git a/app/services/groups/transfer_service.rb b/app/services/groups/transfer_service.rb index f2e959396bc..a0021ae2ccb 100644 --- a/app/services/groups/transfer_service.rb +++ b/app/services/groups/transfer_service.rb @@ -191,18 +191,9 @@ module Groups end def refresh_project_authorizations - projects_to_update = Set.new + project_ids = Groups::ProjectsRequiringAuthorizationsRefresh::OnTransferFinder.new(@group).execute - # All projects in this hierarchy need to have their project authorizations recalculated - @group.all_projects.each_batch { |prjs| projects_to_update.merge(prjs.ids) } # rubocop: disable CodeReuse/ActiveRecord - - # When a group is transferred, it also affects who gets access to the projects shared to - # the subgroups within its hierarchy, so we also schedule jobs that refresh authorizations for all such shared projects. - ProjectGroupLink.in_group(@group.self_and_descendants.select(:id)).each_batch do |project_group_links| - projects_to_update.merge(project_group_links.pluck(:project_id)) # rubocop: disable CodeReuse/ActiveRecord - end - - AuthorizedProjectUpdate::ProjectAccessChangedService.new(projects_to_update.to_a).execute unless projects_to_update.empty? + AuthorizedProjectUpdate::ProjectAccessChangedService.new(project_ids).execute end def raise_transfer_error(message) diff --git a/app/services/import/bitbucket_server_service.rb b/app/services/import/bitbucket_server_service.rb index cdb23370ddc..d1c22f06464 100644 --- a/app/services/import/bitbucket_server_service.rb +++ b/app/services/import/bitbucket_server_service.rb @@ -21,6 +21,8 @@ module Import if project.persisted? success(project) + elsif project.errors[:import_source_disabled].present? + error(project.errors[:import_source_disabled], :forbidden) else log_and_return_error(project_save_error(project), :unprocessable_entity) end diff --git a/app/services/import/github_service.rb b/app/services/import/github_service.rb index a891dcc11e3..033f6bcb043 100644 --- a/app/services/import/github_service.rb +++ b/app/services/import/github_service.rb @@ -25,6 +25,8 @@ module Import if project.persisted? success(project) + elsif project.errors[:import_source_disabled].present? + error(project.errors[:import_source_disabled], :forbidden) else error(project_save_error(project), :unprocessable_entity) end diff --git a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb index ae9a450660c..ac58711a0ac 100644 --- a/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb +++ b/app/services/import/gitlab_projects/file_acquisition_strategies/remote_file.rb @@ -58,7 +58,7 @@ module Import end def validate_aws_s3? - ::Feature.enabled?(:import_project_from_remote_file_s3, default_enabled: :yaml) + ::Feature.enabled?(:import_project_from_remote_file_s3) end def headers diff --git a/app/services/incident_management/timeline_events/base_service.rb b/app/services/incident_management/timeline_events/base_service.rb new file mode 100644 index 00000000000..cae58465e4a --- /dev/null +++ b/app/services/incident_management/timeline_events/base_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module IncidentManagement + module TimelineEvents + class BaseService + def allowed? + user&.can?(:admin_incident_management_timeline_event, incident) + end + + def success(timeline_event) + ServiceResponse.success(payload: { timeline_event: timeline_event }) + end + + def error(message) + ServiceResponse.error(message: message) + end + + def error_no_permissions + error(_('You have insufficient permissions to manage timeline events for this incident')) + end + + def error_in_save(timeline_event) + error(timeline_event.errors.full_messages.to_sentence) + end + end + end +end diff --git a/app/services/incident_management/timeline_events/create_service.rb b/app/services/incident_management/timeline_events/create_service.rb new file mode 100644 index 00000000000..7d287e1bd82 --- /dev/null +++ b/app/services/incident_management/timeline_events/create_service.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +module IncidentManagement + module TimelineEvents + DEFAULT_ACTION = 'comment' + + class CreateService < TimelineEvents::BaseService + def initialize(incident, user, params) + @project = incident.project + @incident = incident + @user = user + @params = params + end + + def execute + return error_no_permissions unless allowed? + + timeline_event_params = { + project: project, + incident: incident, + author: user, + note: params[:note], + action: params.fetch(:action, DEFAULT_ACTION), + note_html: params[:note_html].presence || params[:note], + occurred_at: params[:occurred_at], + promoted_from_note: params[:promoted_from_note] + } + + timeline_event = IncidentManagement::TimelineEvent.new(timeline_event_params) + + if timeline_event.save + add_system_note(timeline_event) + + success(timeline_event) + else + error_in_save(timeline_event) + end + end + + private + + attr_reader :project, :user, :incident, :params + + def add_system_note(timeline_event) + return unless Feature.enabled?(:incident_timeline, project) + + SystemNoteService.add_timeline_event(timeline_event) + end + end + end +end diff --git a/app/services/incident_management/timeline_events/destroy_service.rb b/app/services/incident_management/timeline_events/destroy_service.rb new file mode 100644 index 00000000000..8bb186c289a --- /dev/null +++ b/app/services/incident_management/timeline_events/destroy_service.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +module IncidentManagement + module TimelineEvents + class DestroyService < TimelineEvents::BaseService + # @param timeline_event [IncidentManagement::TimelineEvent] + # @param user [User] + def initialize(timeline_event, user) + @timeline_event = timeline_event + @user = user + @incident = timeline_event.incident + @project = @incident.project + end + + def execute + return error_no_permissions unless allowed? + + if timeline_event.destroy + add_system_note(incident, user) + + success(timeline_event) + else + error_in_save(timeline_event) + end + end + + private + + attr_reader :project, :timeline_event, :user, :incident + + def add_system_note(incident, user) + return unless Feature.enabled?(:incident_timeline, project) + + SystemNoteService.delete_timeline_event(incident, user) + end + end + end +end diff --git a/app/services/incident_management/timeline_events/update_service.rb b/app/services/incident_management/timeline_events/update_service.rb new file mode 100644 index 00000000000..fe8b4879561 --- /dev/null +++ b/app/services/incident_management/timeline_events/update_service.rb @@ -0,0 +1,61 @@ +# frozen_string_literal: true + +module IncidentManagement + module TimelineEvents + # @param timeline_event [IncidentManagement::TimelineEvent] + # @param user [User] + # @param params [Hash] + # @option params [string] note + # @option params [datetime] occurred_at + class UpdateService < TimelineEvents::BaseService + def initialize(timeline_event, user, params) + @timeline_event = timeline_event + @incident = timeline_event.incident + @user = user + @note = params[:note] + @occurred_at = params[:occurred_at] + end + + def execute + return error_no_permissions unless allowed? + + if timeline_event.update(update_params) + add_system_note(timeline_event) + + success(timeline_event) + else + error_in_save(timeline_event) + end + end + + private + + attr_reader :timeline_event, :incident, :user, :note, :occurred_at + + def update_params + { updated_by_user: user, note: note.presence, occurred_at: occurred_at.presence }.compact + end + + def add_system_note(timeline_event) + return unless Feature.enabled?(:incident_timeline, incident.project) + + changes = was_changed(timeline_event) + return if changes == :none + + SystemNoteService.edit_timeline_event(timeline_event, user, was_changed: changes) + end + + def was_changed(timeline_event) + changes = timeline_event.previous_changes + occurred_at_changed = changes.key?('occurred_at') + note_changed = changes.key?('note') + + return :occurred_at_and_note if occurred_at_changed && note_changed + return :occurred_at if occurred_at_changed + return :note if note_changed + + :none + end + end + end +end diff --git a/app/services/issuable/destroy_service.rb b/app/services/issuable/destroy_service.rb index b75905fb5b0..6aab56f0f68 100644 --- a/app/services/issuable/destroy_service.rb +++ b/app/services/issuable/destroy_service.rb @@ -26,13 +26,15 @@ module Issuable end def delete_todos(actor, issuable) - TodosDestroyer::DestroyedIssuableWorker - .perform_async(issuable.id, issuable.class.name) + issuable.run_after_commit_or_now do + TodosDestroyer::DestroyedIssuableWorker.perform_async(issuable.id, issuable.class.name) + end end def delete_label_links(actor, issuable) - Issuable::LabelLinksDestroyWorker - .perform_async(issuable.id, issuable.class.name) + issuable.run_after_commit_or_now do + Issuable::LabelLinksDestroyWorker.perform_async(issuable.id, issuable.class.name) + end end end end diff --git a/app/services/jira/requests/base.rb b/app/services/jira/requests/base.rb index 3e15d47e8af..8e8511e5180 100644 --- a/app/services/jira/requests/base.rb +++ b/app/services/jira/requests/base.rb @@ -3,8 +3,6 @@ module Jira module Requests class Base - include ProjectServicesLoggable - JIRA_API_VERSION = 2 # Limit the size of the JSON error message we will attempt to parse, as the JSON is external input. JIRA_ERROR_JSON_SIZE_LIMIT = 5_000 @@ -54,17 +52,13 @@ module Jira def request response = client.get(url) build_service_response(response) - rescue *ALL_ERRORS => e - log_error('Error sending message', - client_url: client.options[:site], - error: { - exception_class: e.class.name, - exception_message: e.message, - exception_backtrace: Gitlab::BacktraceCleaner.clean_backtrace(e.backtrace) - } + rescue *ALL_ERRORS => error + jira_integration.log_exception(error, + message: 'Error sending message', + client_url: client.options[:site] ) - ServiceResponse.error(message: error_message(e)) + ServiceResponse.error(message: error_message(error)) end def auth_docs_link_start diff --git a/app/services/jira_connect/sync_service.rb b/app/services/jira_connect/sync_service.rb index bddc7cbe5a0..92255711399 100644 --- a/app/services/jira_connect/sync_service.rb +++ b/app/services/jira_connect/sync_service.rb @@ -39,7 +39,7 @@ module JiraConnect end def logger - Gitlab::ProjectServiceLogger + Gitlab::IntegrationsLogger end end end diff --git a/app/services/jira_import/start_import_service.rb b/app/services/jira_import/start_import_service.rb index c9ffdeb2a16..4d1f2c94ac8 100644 --- a/app/services/jira_import/start_import_service.rb +++ b/app/services/jira_import/start_import_service.rb @@ -73,7 +73,7 @@ module JiraImport jira_imports_for_project = project.jira_imports.by_jira_project_key(jira_project_key).size + 1 title = "jira-import::#{jira_project_key}-#{jira_imports_for_project}" description = "Label for issues that were imported from Jira on #{import_start_time.strftime('%Y-%m-%d %H:%M:%S')}" - color = "#{Label.color_for(title)}" + color = "#{::Gitlab::Color.color_for(title)}" { title: title, description: description, color: color } end diff --git a/app/services/loose_foreign_keys/batch_cleaner_service.rb b/app/services/loose_foreign_keys/batch_cleaner_service.rb index b89de15a568..95eb8b47009 100644 --- a/app/services/loose_foreign_keys/batch_cleaner_service.rb +++ b/app/services/loose_foreign_keys/batch_cleaner_service.rb @@ -54,8 +54,6 @@ module LooseForeignKeys attr_reader :parent_table, :loose_foreign_key_definitions, :deleted_parent_records, :modification_tracker, :deleted_records_counter, :deleted_records_rescheduled_count, :deleted_records_incremented_count def handle_over_limit - return if Feature.disabled?(:lfk_fair_queueing, default_enabled: :yaml) - records_to_reschedule = [] records_to_increment = [] diff --git a/app/services/members/create_service.rb b/app/services/members/create_service.rb index 8f7b63c32c8..8485e7cbafa 100644 --- a/app/services/members/create_service.rb +++ b/app/services/members/create_service.rb @@ -46,9 +46,11 @@ module Members :tasks_to_be_done_members, :member_created_member_task_id def invites_from_params - return params[:user_ids] if params[:user_ids].is_a?(Array) + # String, Nil, Array, Integer + return params[:user_id] if params[:user_id].is_a?(Array) + return [] unless params[:user_id] - params[:user_ids]&.to_s&.split(',')&.uniq&.flatten || [] + params[:user_id].to_s.split(',').uniq end def validate_invite_source! diff --git a/app/services/members/creator_service.rb b/app/services/members/creator_service.rb index 321658ac9c5..81986a2883f 100644 --- a/app/services/members/creator_service.rb +++ b/app/services/members/creator_service.rb @@ -82,7 +82,13 @@ module Members if member.request? approve_request else - member.save + # Calling #save triggers callbacks even if there is no change on object. + # This previously caused an incident due to the hard to predict + # behaviour caused by the large number of callbacks. + # See https://gitlab.com/gitlab-com/gl-infra/production/-/issues/6351 + # and https://gitlab.com/gitlab-org/gitlab/-/merge_requests/80920#note_911569038 + # for details. + member.save if member.changed? end end diff --git a/app/services/members/projects/creator_service.rb b/app/services/members/projects/creator_service.rb index d92fe60c54a..9e9389d3c18 100644 --- a/app/services/members/projects/creator_service.rb +++ b/app/services/members/projects/creator_service.rb @@ -6,16 +6,11 @@ module Members private def can_create_new_member? - # order is important here! - # The `admin_project_member` check has side-effects that causes projects not be created if this area is hit - # during project creation. - # Call that triggers is current_user.can?(:admin_project_member, member.project) - # I tracked back to base_policy.rb admin check and specifically in - # Gitlab::Auth::CurrentUserMode.new(@user).admin_mode? call. - # This calls user.admin? and that specific call causes issues with project creation in - # spec/requests/api/projects_spec.rb specs and others, mostly around project creation. - # https://gitlab.com/gitlab-org/gitlab/-/issues/358931 for investigation - adding_the_creator_as_owner_in_a_personal_project? || current_user.can?(:admin_project_member, member.project) + # This access check(`admin_project_member`) will write to safe request store cache for the user being added. + # This means any operations inside the same request will need to purge that safe request + # store cache if operations are needed to be done inside the same request that checks max member access again on + # that user. + current_user.can?(:admin_project_member, member.project) || adding_the_creator_as_owner_in_a_personal_project? end def can_update_existing_member? diff --git a/app/services/merge_requests/approval_service.rb b/app/services/merge_requests/approval_service.rb index 37c2676e51c..e3f0758699b 100644 --- a/app/services/merge_requests/approval_service.rb +++ b/app/services/merge_requests/approval_service.rb @@ -33,6 +33,8 @@ module MergeRequests def execute_approval_hooks(merge_request, current_user) # Only one approval is required for a merge request to be approved + notification_service.async.approve_mr(merge_request, current_user) + execute_hooks(merge_request, 'approved') end diff --git a/app/services/merge_requests/base_service.rb b/app/services/merge_requests/base_service.rb index d197c13378a..44be254441d 100644 --- a/app/services/merge_requests/base_service.rb +++ b/app/services/merge_requests/base_service.rb @@ -43,6 +43,8 @@ module MergeRequests end def handle_assignees_change(merge_request, old_assignees) + bulk_update_assignees_state(merge_request, merge_request.assignees - old_assignees) + MergeRequests::HandleAssigneesChangeService .new(project: project, current_user: current_user) .async_execute(merge_request, old_assignees) @@ -58,17 +60,16 @@ module MergeRequests new_reviewers = merge_request.reviewers - old_reviewers merge_request_activity_counter.track_users_review_requested(users: new_reviewers) merge_request_activity_counter.track_reviewers_changed_action(user: current_user) + bulk_update_reviewers_state(merge_request, new_reviewers) unless new_reviewers.include?(current_user) remove_attention_requested(merge_request) - - merge_request.merge_request_reviewers_with(new_reviewers).update_all(updated_state_by_user_id: current_user.id) end end def cleanup_environments(merge_request) Environments::StopService.new(merge_request.source_project, current_user) - .execute_for_merge_request(merge_request) + .execute_for_merge_request_pipeline(merge_request) end def cancel_review_app_jobs!(merge_request) @@ -246,7 +247,7 @@ module MergeRequests end def remove_all_attention_requests(merge_request) - return unless merge_request.attention_requested_enabled? + return unless current_user.mr_attention_requests_enabled? users = merge_request.reviewers + merge_request.assignees @@ -254,9 +255,49 @@ module MergeRequests end def remove_attention_requested(merge_request) - return unless merge_request.attention_requested_enabled? + return unless current_user.mr_attention_requests_enabled? + + ::MergeRequests::RemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request, user: current_user).execute + end + + def bulk_update_assignees_state(merge_request, new_assignees) + return unless current_user.mr_attention_requests_enabled? + return if new_assignees.empty? + + assignees_map = merge_request.merge_request_assignees_with(new_assignees).to_h do |assignee| + state = if assignee.user_id == current_user&.id + :unreviewed + else + merge_request.find_reviewer(assignee.assignee)&.state || :attention_requested + end + + [ + assignee, + { state: MergeRequestAssignee.states[state], updated_state_by_user_id: current_user.id } + ] + end + + ::Gitlab::Database::BulkUpdate.execute(%i[state updated_state_by_user_id], assignees_map) + end + + def bulk_update_reviewers_state(merge_request, new_reviewers) + return unless current_user.mr_attention_requests_enabled? + return if new_reviewers.empty? + + reviewers_map = merge_request.merge_request_reviewers_with(new_reviewers).to_h do |reviewer| + state = if reviewer.user_id == current_user&.id + :unreviewed + else + merge_request.find_assignee(reviewer.reviewer)&.state || :attention_requested + end + + [ + reviewer, + { state: MergeRequestReviewer.states[state], updated_state_by_user_id: current_user.id } + ] + end - ::MergeRequests::RemoveAttentionRequestedService.new(project: merge_request.project, current_user: current_user, merge_request: merge_request).execute + ::Gitlab::Database::BulkUpdate.execute(%i[state updated_state_by_user_id], reviewers_map) end end end diff --git a/app/services/merge_requests/build_service.rb b/app/services/merge_requests/build_service.rb index 878e42172b7..ee6f204be45 100644 --- a/app/services/merge_requests/build_service.rb +++ b/app/services/merge_requests/build_service.rb @@ -203,6 +203,12 @@ module MergeRequests target_branch.blank? || target_project.commit(target_branch) end + def set_draft_title_if_needed + return unless compare_commits.empty? || Gitlab::Utils.to_boolean(params[:draft]) + + merge_request.title = wip_title + end + # When your branch name starts with an iid followed by a dash this pattern will be # interpreted as the user wants to close that issue on this project. # @@ -220,7 +226,7 @@ module MergeRequests assign_title_and_description_from_commits merge_request.title ||= title_from_issue if target_project.issues_enabled? || target_project.external_issue_tracker merge_request.title ||= source_branch.titleize.humanize - merge_request.title = wip_title if compare_commits.empty? + set_draft_title_if_needed append_closes_description end diff --git a/app/services/merge_requests/create_service.rb b/app/services/merge_requests/create_service.rb index 9c525ae8489..8e0f72eb380 100644 --- a/app/services/merge_requests/create_service.rb +++ b/app/services/merge_requests/create_service.rb @@ -25,9 +25,7 @@ module MergeRequests # expose issuable create method so it can be called from email # handler CreateMergeRequestHandler - def create(merge_request) - super - end + public :create private diff --git a/app/services/merge_requests/handle_assignees_change_service.rb b/app/services/merge_requests/handle_assignees_change_service.rb index a169a6dc0b6..78c93d10f2a 100644 --- a/app/services/merge_requests/handle_assignees_change_service.rb +++ b/app/services/merge_requests/handle_assignees_change_service.rb @@ -21,8 +21,6 @@ module MergeRequests merge_request_activity_counter.track_users_assigned_to_mr(users: new_assignees) merge_request_activity_counter.track_assignees_changed_action(user: current_user) - merge_request.merge_request_assignees_with(new_assignees).update_all(updated_state_by_user_id: current_user.id) - execute_assignees_hooks(merge_request, old_assignees) if options[:execute_hooks] unless new_assignees.include?(current_user) diff --git a/app/services/merge_requests/mergeability/run_checks_service.rb b/app/services/merge_requests/mergeability/run_checks_service.rb index 03c6d985c23..fd6907c976b 100644 --- a/app/services/merge_requests/mergeability/run_checks_service.rb +++ b/app/services/merge_requests/mergeability/run_checks_service.rb @@ -37,7 +37,7 @@ module MergeRequests attr_reader :merge_request, :params def run_check(check) - return check.execute unless Feature.enabled?(:mergeability_caching, merge_request.project, default_enabled: :yaml) + return check.execute unless Feature.enabled?(:mergeability_caching, merge_request.project) return check.execute unless check.cacheable? cached_result = results.read(merge_check: check) diff --git a/app/services/merge_requests/post_merge_service.rb b/app/services/merge_requests/post_merge_service.rb index e475b57e4a2..980c757bcbc 100644 --- a/app/services/merge_requests/post_merge_service.rb +++ b/app/services/merge_requests/post_merge_service.rb @@ -45,7 +45,19 @@ module MergeRequests closed_issues = merge_request.visible_closing_issues_for(current_user) closed_issues.each do |issue| - Issues::CloseService.new(project: project, current_user: current_user).execute(issue, commit: merge_request) + # We are intentionally only closing Issues asynchronously (excluding ExternalIssues) + # as the worker only supports finding an Issue. We are also only experiencing + # SQL timeouts when closing an Issue. + if Feature.enabled?(:async_mr_close_issue, project) && issue.is_a?(Issue) + MergeRequests::CloseIssueWorker.perform_async( + project.id, + current_user.id, + issue.id, + merge_request.id + ) + else + Issues::CloseService.new(project: project, current_user: current_user).execute(issue, commit: merge_request) + end 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 adbe3ddfdad..076fe8c3b21 100644 --- a/app/services/merge_requests/push_options_handler_service.rb +++ b/app/services/merge_requests/push_options_handler_service.rb @@ -126,6 +126,7 @@ module MergeRequests params = { title: push_options[:title], description: push_options[:description], + draft: push_options[:draft], target_branch: push_options[:target], force_remove_source_branch: push_options[:remove_source_branch], label: push_options[:label], @@ -147,6 +148,10 @@ module MergeRequests params[:milestone] = milestone if milestone end + if params.key?(:description) + params[:description] = params[:description].gsub('\n', "\n") + end + params end diff --git a/app/services/merge_requests/remove_approval_service.rb b/app/services/merge_requests/remove_approval_service.rb index c7bc3532264..d9bb17a7b1b 100644 --- a/app/services/merge_requests/remove_approval_service.rb +++ b/app/services/merge_requests/remove_approval_service.rb @@ -33,6 +33,7 @@ module MergeRequests def trigger_approval_hooks(merge_request) yield + notification_service.async.unapprove_mr(merge_request, current_user) execute_hooks(merge_request, 'unapproved') end diff --git a/app/services/merge_requests/remove_attention_requested_service.rb b/app/services/merge_requests/remove_attention_requested_service.rb index a32a8071471..8a410fda691 100644 --- a/app/services/merge_requests/remove_attention_requested_service.rb +++ b/app/services/merge_requests/remove_attention_requested_service.rb @@ -2,22 +2,26 @@ module MergeRequests class RemoveAttentionRequestedService < MergeRequests::BaseService - attr_accessor :merge_request + attr_accessor :merge_request, :user - def initialize(project:, current_user:, merge_request:) + def initialize(project:, current_user:, merge_request:, user:) super(project: project, current_user: current_user) @merge_request = merge_request + @user = user end def execute return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) if reviewer || assignee + return success if reviewer&.reviewed? || assignee&.reviewed? + update_state(reviewer) update_state(assignee) - current_user.invalidate_attention_requested_count + user.invalidate_attention_requested_count + create_remove_attention_request_note success else @@ -28,15 +32,19 @@ module MergeRequests private def assignee - merge_request.find_assignee(current_user) + @assignee ||= merge_request.find_assignee(user) end def reviewer - merge_request.find_reviewer(current_user) + @reviewer ||= merge_request.find_reviewer(user) end def update_state(reviewer_or_assignee) reviewer_or_assignee&.update(state: :reviewed) end + + def create_remove_attention_request_note + SystemNoteService.remove_attention_request(merge_request, merge_request.project, current_user, user) + end end end diff --git a/app/services/merge_requests/request_attention_service.rb b/app/services/merge_requests/request_attention_service.rb new file mode 100644 index 00000000000..07e9996f87b --- /dev/null +++ b/app/services/merge_requests/request_attention_service.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +module MergeRequests + class RequestAttentionService < MergeRequests::BaseService + attr_accessor :merge_request, :user + + def initialize(project:, current_user:, merge_request:, user:) + super(project: project, current_user: current_user) + + @merge_request = merge_request + @user = user + end + + def execute + return error("Invalid permissions") unless can?(current_user, :update_merge_request, merge_request) + + if reviewer || assignee + return success if reviewer&.attention_requested? || assignee&.attention_requested? + + update_state(reviewer) + update_state(assignee) + + user.invalidate_attention_requested_count + create_attention_request_note + notity_user + + if current_user.id != user.id + remove_attention_requested(merge_request) + end + + success + else + error("User is not a reviewer or assignee of the merge request") + end + end + + private + + def notity_user + notification_service.async.attention_requested_of_merge_request(merge_request, current_user, user) + todo_service.create_attention_requested_todo(merge_request, current_user, user) + end + + def create_attention_request_note + SystemNoteService.request_attention(merge_request, merge_request.project, current_user, user) + end + + def assignee + @assignee ||= merge_request.find_assignee(user) + end + + def reviewer + @reviewer ||= merge_request.find_reviewer(user) + end + + def update_state(reviewer_or_assignee) + reviewer_or_assignee&.update(state: :attention_requested, updated_state_by: current_user) + end + end +end diff --git a/app/services/merge_requests/update_assignees_service.rb b/app/services/merge_requests/update_assignees_service.rb index d52c1bbbcda..5b23f69ac4a 100644 --- a/app/services/merge_requests/update_assignees_service.rb +++ b/app/services/merge_requests/update_assignees_service.rb @@ -20,6 +20,8 @@ module MergeRequests attrs = update_attrs.merge(assignee_ids: new_ids) merge_request.update!(**attrs) + bulk_update_assignees_state(merge_request, merge_request.assignees - old_assignees) + # Defer the more expensive operations (handle_assignee_changes) to the background MergeRequests::HandleAssigneesChangeService .new(project: project, current_user: current_user) diff --git a/app/services/merge_requests/update_service.rb b/app/services/merge_requests/update_service.rb index 391079223ca..6e8afaecbba 100644 --- a/app/services/merge_requests/update_service.rb +++ b/app/services/merge_requests/update_service.rb @@ -11,6 +11,10 @@ module MergeRequests end def execute(merge_request) + if Gitlab::Utils.to_boolean(params[:draft]) + merge_request.title = merge_request.draft_title + end + update_merge_request_with_specialized_service(merge_request) || general_fallback(merge_request) end diff --git a/app/services/namespaces/in_product_marketing_emails_service.rb b/app/services/namespaces/in_product_marketing_emails_service.rb index e42c3498c21..414f253deb8 100644 --- a/app/services/namespaces/in_product_marketing_emails_service.rb +++ b/app/services/namespaces/in_product_marketing_emails_service.rb @@ -61,7 +61,7 @@ module Namespaces def initialize(track, interval) @track = track @interval = interval - @sent_email_records = InProductMarketingEmailRecords.new + @sent_email_records = ::Users::InProductMarketingEmailRecords.new end def execute @@ -86,7 +86,7 @@ module Namespaces users_for_group(group).each do |user| if can_perform_action?(user, group) send_email(user, group) - sent_email_records.add(user, track, series) + sent_email_records.add(user, track: track, series: series) end end diff --git a/app/services/namespaces/package_settings/update_service.rb b/app/services/namespaces/package_settings/update_service.rb index cbadbe5c907..c0af0900450 100644 --- a/app/services/namespaces/package_settings/update_service.rb +++ b/app/services/namespaces/package_settings/update_service.rb @@ -32,7 +32,7 @@ module Namespaces end def allowed? - Ability.allowed?(current_user, :create_package_settings, @container) + Ability.allowed?(current_user, :admin_package, @container) end def package_settings_params diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index d32d1c8ca12..4074b1d1182 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -45,13 +45,13 @@ module Notes def execute_quick_actions(note) return yield(false) unless quick_actions_supported?(note) - content, update_params, message = quick_actions_service.execute(note, quick_action_options) + content, update_params, message, command_names = quick_actions_service.execute(note, quick_action_options) only_commands = content.empty? note.note = content yield(only_commands) - do_commands(note, update_params, message, only_commands) + do_commands(note, update_params, message, command_names, only_commands) end def quick_actions_supported?(note) @@ -84,7 +84,7 @@ module Notes end end - def do_commands(note, update_params, message, only_commands) + def do_commands(note, update_params, message, command_names, only_commands) return if quick_actions_service.commands_executed_count.to_i == 0 if update_params.present? @@ -96,6 +96,7 @@ module Notes # when #save is called if only_commands note.errors.add(:commands_only, message.presence || _('Failed to apply commands.')) + note.errors.add(:command_names, command_names.flatten) # Allow consumers to detect problems applying commands note.errors.add(:commands, _('Failed to apply commands.')) unless message.present? end @@ -112,6 +113,7 @@ module Notes track_note_creation_usage_for_issues(note) if note.for_issue? track_note_creation_usage_for_merge_requests(note) if note.for_merge_request? track_incident_action(user, note.noteable, 'incident_comment') if note.for_issue? + track_note_creation_in_ipynb(note) if Feature.enabled?(:notes_create_service_tracking, project) Gitlab::Tracking.event('Notes::CreateService', 'execute', **tracking_data_for(note)) @@ -134,6 +136,16 @@ module Notes def track_note_creation_usage_for_merge_requests(note) Gitlab::UsageDataCounters::MergeRequestActivityUniqueCounter.track_create_comment_action(note: note) end + + def should_track_ipynb_notes?(note) + Feature.enabled?(:ipynbdiff_notes_tracker) && note.respond_to?(:diff_file) && note.diff_file&.ipynb? + end + + def track_note_creation_in_ipynb(note) + return unless should_track_ipynb_notes?(note) + + Gitlab::UsageDataCounters::IpynbDiffActivityCounter.note_created(note) + end end end diff --git a/app/services/notification_service.rb b/app/services/notification_service.rb index a3f250bb235..32b23d4978f 100644 --- a/app/services/notification_service.rb +++ b/app/services/notification_service.rb @@ -761,6 +761,20 @@ class NotificationService mailer.in_product_marketing_email(user_id, group_id, track, series).deliver_later end + def approve_mr(merge_request, current_user) + approve_mr_email(merge_request, merge_request.target_project, current_user) + end + + def unapprove_mr(merge_request, current_user) + unapprove_mr_email(merge_request, merge_request.target_project, current_user) + end + + def inactive_project_deletion_warning(project, deletion_date) + owners_and_maintainers_without_invites(project).each do |recipient| + mailer.inactive_project_deletion_warning_email(project, recipient.user, deletion_date).deliver_later + end + end + protected def new_resource_email(target, current_user, method) @@ -866,6 +880,22 @@ class NotificationService private + def approve_mr_email(merge_request, project, current_user) + recipients = ::NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: 'approve') + + recipients.each do |recipient| + mailer.approved_merge_request_email(recipient.user.id, merge_request.id, current_user.id).deliver_later + end + end + + def unapprove_mr_email(merge_request, project, current_user) + recipients = ::NotificationRecipients::BuildService.build_recipients(merge_request, current_user, action: 'unapprove') + + recipients.each do |recipient| + mailer.unapproved_merge_request_email(recipient.user.id, merge_request.id, current_user.id).deliver_later + end + end + def pipeline_notification_status(ref_status, pipeline) if Ci::Ref.failing_state?(ref_status) 'failed' diff --git a/app/services/projects/after_import_service.rb b/app/services/projects/after_import_service.rb deleted file mode 100644 index bb0d084d191..00000000000 --- a/app/services/projects/after_import_service.rb +++ /dev/null @@ -1,43 +0,0 @@ -# frozen_string_literal: true - -module Projects - class AfterImportService - RESERVED_REF_PREFIXES = Repository::RESERVED_REFS_NAMES.map { |n| File.join('refs', n, '/') } - - def initialize(project) - @project = project - end - - def execute - service = Repositories::HousekeepingService.new(@project) - - service.execute do - import_failure_service.with_retry(action: 'delete_all_refs') do - repository.delete_all_refs_except(RESERVED_REF_PREFIXES) - end - end - - # Right now we don't actually have a way to know if a project - # import actually changed, so we increment the counter to avoid - # causing GC to run every time. - service.increment! - rescue Repositories::HousekeepingService::LeaseTaken => e - Gitlab::Import::Logger.info( - message: 'Project housekeeping failed', - project_full_path: @project.full_path, - project_id: @project.id, - 'error.message' => e.message - ) - end - - private - - def import_failure_service - Gitlab::ImportExport::ImportFailureService.new(@project) - end - - def repository - @project.repository - end - end -end diff --git a/app/services/projects/android_target_platform_detector_service.rb b/app/services/projects/android_target_platform_detector_service.rb new file mode 100644 index 00000000000..11635ad18d5 --- /dev/null +++ b/app/services/projects/android_target_platform_detector_service.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Projects + # Service class to detect if a project is made to run on the Android platform. + # + # This service searches for an AndroidManifest.xml file which all Android app + # project must have. It returns the symbol :android if the given project is an + # Android app project. + # + # Ref: https://developer.android.com/guide/topics/manifest/manifest-intro + # + # Example usage: + # > AndroidTargetPlatformDetectorService.new(a_project).execute + # => nil + # > AndroidTargetPlatformDetectorService.new(an_android_project).execute + # => :android + class AndroidTargetPlatformDetectorService < BaseService + # <manifest> element is required and must occur once inside AndroidManifest.xml + MANIFEST_FILE_SEARCH_QUERY = '<manifest filename:AndroidManifest.xml' + + def execute + detect + end + + private + + def file_finder + @file_finder ||= ::Gitlab::FileFinder.new(project, project.default_branch) + end + + def detect + return :android if file_finder.find(MANIFEST_FILE_SEARCH_QUERY).present? + end + end +end diff --git a/app/services/projects/blame_service.rb b/app/services/projects/blame_service.rb new file mode 100644 index 00000000000..f7c1240a3ba --- /dev/null +++ b/app/services/projects/blame_service.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true + +# Service class to correctly initialize Gitlab::Blame and Kaminari pagination +# objects +module Projects + class BlameService + PER_PAGE = 1000 + + def initialize(blob, commit, params) + @blob = blob + @commit = commit + @page = extract_page(params) + end + + def blame + Gitlab::Blame.new(blob, commit, range: blame_range) + end + + def pagination + return unless pagination_enabled? + + Kaminari.paginate_array([], total_count: blob_lines_count) + .page(page) + .per(per_page) + .limit(per_page) + end + + private + + attr_reader :blob, :commit, :page + + def blame_range + return unless pagination_enabled? + + first_line = (page - 1) * per_page + 1 + last_line = (first_line + per_page).to_i - 1 + + first_line..last_line + end + + def extract_page(params) + page = params.fetch(:page, 1).to_i + + return 1 if page < 1 || overlimit?(page) + + page + end + + def per_page + PER_PAGE + end + + def overlimit?(page) + page * per_page >= blob_lines_count + per_page + end + + def blob_lines_count + @blob_lines_count ||= blob.data.lines.count + end + + def pagination_enabled? + Feature.enabled?(:blame_page_pagination, commit.project) + end + end +end diff --git a/app/services/projects/branches_by_mode_service.rb b/app/services/projects/branches_by_mode_service.rb index 090671cc79a..0248f997a03 100644 --- a/app/services/projects/branches_by_mode_service.rb +++ b/app/services/projects/branches_by_mode_service.rb @@ -37,7 +37,7 @@ class Projects::BranchesByModeService def use_gitaly_pagination? return false if params[:page].present? || params[:search].present? - Feature.enabled?(:branch_list_keyset_pagination, project, default_enabled: :yaml) + Feature.enabled?(:branch_list_keyset_pagination, project) end def fetch_branches_via_offset_pagination diff --git a/app/services/projects/container_repository/cleanup_tags_service.rb b/app/services/projects/container_repository/cleanup_tags_service.rb index 72f3fddb4c3..0a8e8e72766 100644 --- a/app/services/projects/container_repository/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_service.rb @@ -117,7 +117,6 @@ module Projects @counts[:before_truncate_size] = @tags.size @counts[:after_truncate_size] = @tags.size - return unless throttling_enabled? return if max_list_size == 0 # truncate the list to make sure that after the #filter_keep_n @@ -151,10 +150,6 @@ module Projects !!result end - def throttling_enabled? - Feature.enabled?(:container_registry_expiration_policies_throttling, default_enabled: :yaml) - end - def max_list_size ::Gitlab::CurrentSettings.current_application_settings.container_registry_cleanup_tags_service_max_list_size.to_i end diff --git a/app/services/projects/container_repository/delete_tags_service.rb b/app/services/projects/container_repository/delete_tags_service.rb index b4a57c70111..a3e533c670e 100644 --- a/app/services/projects/container_repository/delete_tags_service.rb +++ b/app/services/projects/container_repository/delete_tags_service.rb @@ -8,13 +8,13 @@ module Projects def execute(container_repository) @container_repository = container_repository - unless params[:container_expiration_policy] + unless container_expiration_policy? return error('access denied') unless can?(current_user, :destroy_container_image, project) end @tag_names = params[:tags] return error('not tags specified') if @tag_names.blank? - return error('repository importing') if @container_repository.migration_importing? + return error('repository importing') if cancel_while_importing? delete_tags end @@ -49,6 +49,20 @@ module Projects log_error(log_data) end end + + def cancel_while_importing? + return true if @container_repository.importing? + + if container_expiration_policy? + return @container_repository.pre_importing? || @container_repository.pre_import_done? + end + + false + end + + def container_expiration_policy? + params[:container_expiration_policy].present? + end end end end diff --git a/app/services/projects/container_repository/gitlab/delete_tags_service.rb b/app/services/projects/container_repository/gitlab/delete_tags_service.rb index f109cb0ca20..81cef554dec 100644 --- a/app/services/projects/container_repository/gitlab/delete_tags_service.rb +++ b/app/services/projects/container_repository/gitlab/delete_tags_service.rb @@ -46,18 +46,11 @@ module Projects end def timeout?(start_time) - return false unless throttling_enabled? return false if service_timeout.in?(DISABLED_TIMEOUTS) (Time.zone.now - start_time) > service_timeout end - def throttling_enabled? - strong_memoize(:feature_flag) do - Feature.enabled?(:container_registry_expiration_policies_throttling, default_enabled: :yaml) - end - end - def service_timeout ::Gitlab::CurrentSettings.current_application_settings.container_registry_delete_tags_service_timeout end diff --git a/app/services/projects/create_service.rb b/app/services/projects/create_service.rb index 3e26c8c35b2..c7f284bec9b 100644 --- a/app/services/projects/create_service.rb +++ b/app/services/projects/create_service.rb @@ -4,6 +4,9 @@ module Projects class CreateService < BaseService include ValidatesClassificationLabel + ImportSourceDisabledError = Class.new(StandardError) + INTERNAL_IMPORT_SOURCES = %w[bare_repository gitlab_custom_project_template gitlab_project_migration].freeze + def initialize(user, params) @current_user = user @params = params.dup @@ -25,6 +28,8 @@ module Projects @project = Project.new(params) + validate_import_source_enabled! + @project.visibility_level = @project.group.visibility_level unless @project.visibility_level_allowed_by_group? # If a project is newly created it should have shared runners settings @@ -77,6 +82,9 @@ module Projects rescue ActiveRecord::RecordInvalid => e message = "Unable to save #{e.inspect}: #{e.record.errors.full_messages.join(", ")}" fail(error: message) + rescue ImportSourceDisabledError => e + @project.errors.add(:import_source_disabled, e.message) if @project + fail(error: e.message) rescue StandardError => e @project.errors.add(:base, e.message) if @project fail(error: e.message) @@ -124,11 +132,7 @@ module Projects end def create_project_settings - if Feature.enabled?(:create_project_settings, default_enabled: :yaml) - @project.project_setting.save if @project.project_setting.changed? - else - @project.create_project_setting unless @project.project_setting - end + @project.project_setting.save if @project.project_setting.changed? end # Add an authorization for the current user authorizations inline @@ -157,6 +161,13 @@ module Projects ) else @project.add_owner(@project.namespace.owner, current_user: current_user) + # During the process of adding a project owner, a check on permissions is made on the user which caches + # the max member access for that user on this project. + # Since that is `0` before the member is created - and we are still inside the request + # cycle when we need to do other operations that might check those permissions (e.g. write a commit) + # we need to purge that cache so that the updated permissions is fetched instead of using the outdated cached value of 0 + # from before member creation + @project.team.purge_member_access_cache_for_user_id(@project.namespace.owner.id) end end @@ -242,6 +253,18 @@ module Projects private + def validate_import_source_enabled! + return unless @params[:import_type] + + import_type = @params[:import_type].to_s + + return if INTERNAL_IMPORT_SOURCES.include?(import_type) + + unless ::Gitlab::CurrentSettings.import_sources&.include?(import_type) + raise ImportSourceDisabledError, "#{import_type} import source is disabled" + end + end + def parent_namespace @parent_namespace ||= Namespace.find_by_id(@params[:namespace_id]) || current_user.namespace end diff --git a/app/services/projects/group_links/create_service.rb b/app/services/projects/group_links/create_service.rb index a0232779c97..72036aaff35 100644 --- a/app/services/projects/group_links/create_service.rb +++ b/app/services/projects/group_links/create_service.rb @@ -3,26 +3,31 @@ module Projects module GroupLinks class CreateService < BaseService - def execute(group) - return error('Not Found', 404) unless group && can?(current_user, :read_namespace, group) + include GroupLinkable - link = project.project_group_links.new( - group: group, - group_access: params[:link_group_access], - expires_at: params[:expires_at] - ) + def initialize(project, shared_with_group, user, params) + @shared_with_group = shared_with_group - if link.save - setup_authorizations(group) - success(link: link) - else - error(link.errors.full_messages.to_sentence, 409) - end + super(project, user, params) end private - def setup_authorizations(group) + delegate :root_ancestor, to: :project + + def valid_to_create? + can?(current_user, :read_namespace, shared_with_group) && sharing_allowed? + end + + def build_link + @link = project.project_group_links.new( + group: shared_with_group, + group_access: params[:link_group_access], + expires_at: params[:expires_at] + ) + end + + def setup_authorizations AuthorizedProjectUpdate::ProjectRecalculateWorker.perform_async(project.id) # AuthorizedProjectsWorker uses an exclusive lease per user but @@ -30,7 +35,7 @@ module Projects # compare the inconsistency rates of both approaches, we still run # AuthorizedProjectsWorker but with some delay and lower urgency as a # safety net. - group.refresh_members_authorized_projects( + shared_with_group.refresh_members_authorized_projects( blocking: false, priority: UserProjectAccessChangedService::LOW_PRIORITY ) diff --git a/app/services/projects/in_product_marketing_campaign_emails_service.rb b/app/services/projects/in_product_marketing_campaign_emails_service.rb new file mode 100644 index 00000000000..249a2d89fc1 --- /dev/null +++ b/app/services/projects/in_product_marketing_campaign_emails_service.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +module Projects + class InProductMarketingCampaignEmailsService + include Gitlab::Experiment::Dsl + + def initialize(project, campaign) + @project = project + @campaign = campaign + @sent_email_records = ::Users::InProductMarketingEmailRecords.new + end + + def execute + send_emails + end + + private + + attr_reader :project, :campaign, :sent_email_records + + def send_emails + project_users.each do |user| + send_email(user) + end + + sent_email_records.save! + end + + # rubocop: disable CodeReuse/ActiveRecord + def project_users + @project_users ||= project.users + .where(email_opted_in: true) + .merge(Users::InProductMarketingEmail.without_campaign(campaign)) + end + # rubocop: enable CodeReuse/ActiveRecord + + def project_users_max_access_levels + ids = project_users.map(&:id) + @project_users_max_access_levels ||= project.team.max_member_access_for_user_ids(ids) + end + + def send_email(user) + return unless user.can?(:receive_notifications) + return unless target_user?(user) + + Notify.build_ios_app_guide_email(user.notification_email_or_default).deliver_later + + sent_email_records.add(user, campaign: campaign) + experiment(:build_ios_app_guide_email, project: project).track(:email_sent) + end + + def target_user?(user) + max_access_level = project_users_max_access_levels[user.id] + max_access_level >= Gitlab::Access::DEVELOPER + end + end +end diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb index 76005a1c96e..c032fbf1508 100644 --- a/app/services/projects/lfs_pointers/lfs_download_service.rb +++ b/app/services/projects/lfs_pointers/lfs_download_service.rb @@ -23,7 +23,7 @@ module Projects def execute return unless project&.lfs_enabled? && lfs_download_object return error("LFS file with oid #{lfs_oid} has invalid attributes") unless lfs_download_object.valid? - return link_existing_lfs_object! if Feature.enabled?(:lfs_link_existing_object, project, default_enabled: :yaml) && lfs_size > LARGE_FILE_SIZE && lfs_object + return link_existing_lfs_object! if Feature.enabled?(:lfs_link_existing_object, project) && lfs_size > LARGE_FILE_SIZE && lfs_object wrap_download_errors do download_lfs_file! diff --git a/app/services/projects/open_issues_count_service.rb b/app/services/projects/open_issues_count_service.rb index 8b7a418edf5..ee4d559e612 100644 --- a/app/services/projects/open_issues_count_service.rb +++ b/app/services/projects/open_issues_count_service.rb @@ -7,12 +7,8 @@ module Projects include Gitlab::Utils::StrongMemoize # Cache keys used to store issues count - # TOTAL_COUNT_KEY includes confidential and hidden issues (admin) - # TOTAL_COUNT_WITHOUT_HIDDEN_KEY includes confidential issues but not hidden issues (reporter and above) - # PUBLIC_COUNT_WITHOUT_HIDDEN_KEY does not include confidential or hidden issues (guest) - TOTAL_COUNT_KEY = 'project_open_issues_including_hidden_count' - TOTAL_COUNT_WITHOUT_HIDDEN_KEY = 'project_open_issues_without_hidden_count' - PUBLIC_COUNT_WITHOUT_HIDDEN_KEY = 'project_open_public_issues_without_hidden_count' + PUBLIC_COUNT_KEY = 'public_open_issues_count' + TOTAL_COUNT_KEY = 'total_open_issues_count' def initialize(project, user = nil) @user = user @@ -20,98 +16,59 @@ module Projects super(project) end - # rubocop: disable CodeReuse/ActiveRecord - def refresh_cache(&block) - if block_given? - super(&block) - else - update_cache_for_key(total_count_cache_key) do - issues_with_hidden - end - - update_cache_for_key(public_count_without_hidden_cache_key) do - issues_without_hidden_without_confidential - end - - update_cache_for_key(total_count_without_hidden_cache_key) do - issues_without_hidden_with_confidential - end - end - end - - private - - def relation_for_count - self.class.query(@project, public_only: public_only?, include_hidden: include_hidden?) - end - def cache_key_name - if include_hidden? - TOTAL_COUNT_KEY - elsif public_only? - PUBLIC_COUNT_WITHOUT_HIDDEN_KEY - else - TOTAL_COUNT_WITHOUT_HIDDEN_KEY - end - end - - def include_hidden? - user_is_admin? + public_only? ? PUBLIC_COUNT_KEY : TOTAL_COUNT_KEY end def public_only? !user_is_at_least_reporter? end - def user_is_admin? - strong_memoize(:user_is_admin) do - @user&.can_admin_all_resources? - end - end - def user_is_at_least_reporter? strong_memoize(:user_is_at_least_reporter) do @user && @project.team.member?(@user, Gitlab::Access::REPORTER) end end - def total_count_without_hidden_cache_key - cache_key(TOTAL_COUNT_WITHOUT_HIDDEN_KEY) + def relation_for_count + self.class.query(@project, public_only: public_only?) end - def public_count_without_hidden_cache_key - cache_key(PUBLIC_COUNT_WITHOUT_HIDDEN_KEY) + def public_count_cache_key + cache_key(PUBLIC_COUNT_KEY) end def total_count_cache_key cache_key(TOTAL_COUNT_KEY) end - def issues_with_hidden - self.class.query(@project, public_only: false, include_hidden: true).count - end + # rubocop: disable CodeReuse/ActiveRecord + def refresh_cache(&block) + count_grouped_by_confidential = self.class.query(@project, public_only: false).group(:confidential).count + public_count = count_grouped_by_confidential[false] || 0 + total_count = public_count + (count_grouped_by_confidential[true] || 0) - def issues_without_hidden_without_confidential - self.class.query(@project, public_only: true, include_hidden: false).count - end + update_cache_for_key(public_count_cache_key) do + public_count + end - def issues_without_hidden_with_confidential - self.class.query(@project, public_only: false, include_hidden: false).count + update_cache_for_key(total_count_cache_key) do + total_count + end end - # We only show total issues count for admins, who are allowed to view hidden issues. - # We also only show issues count including confidential for reporters, who are allowed to view confidential issues. + # We only show issues count including confidential for reporters, who are allowed to view confidential issues. # This will still show a discrepancy on issues number but should be less than before. # Check https://gitlab.com/gitlab-org/gitlab-foss/issues/38418 description. + # rubocop: disable CodeReuse/ActiveRecord + def self.query(projects, public_only: true) + issues_filtered_by_type = Issue.opened.with_issue_type(Issue::TYPES_FOR_LIST) - def self.query(projects, public_only: true, include_hidden: false) - if include_hidden - Issue.opened.with_issue_type(Issue::TYPES_FOR_LIST).where(project: projects) - elsif public_only - Issue.public_only.opened.with_issue_type(Issue::TYPES_FOR_LIST).where(project: projects) + if public_only + issues_filtered_by_type.public_only.where(project: projects) else - Issue.without_hidden.opened.with_issue_type(Issue::TYPES_FOR_LIST).where(project: projects) + issues_filtered_by_type.where(project: projects) end end # rubocop: enable CodeReuse/ActiveRecord diff --git a/app/services/projects/overwrite_project_service.rb b/app/services/projects/overwrite_project_service.rb index eea8f867b45..d3fed43363c 100644 --- a/app/services/projects/overwrite_project_service.rb +++ b/app/services/projects/overwrite_project_service.rb @@ -41,7 +41,7 @@ module Projects private def track_service(start_time, source_project, exception) - return if ::Feature.disabled?(:project_overwrite_service_tracking, source_project, default_enabled: :yaml) + return if ::Feature.disabled?(:project_overwrite_service_tracking, source_project) duration = ::Gitlab::Metrics::System.monotonic_time - start_time diff --git a/app/services/projects/participants_service.rb b/app/services/projects/participants_service.rb index c7a34afffb3..c29770d0c5f 100644 --- a/app/services/projects/participants_service.rb +++ b/app/services/projects/participants_service.rb @@ -45,7 +45,7 @@ module Projects def visible_groups visible_groups = project.invited_groups - unless project_owner? + unless project.team.owner?(current_user) visible_groups = visible_groups.public_or_visible_to_user(current_user) end @@ -60,13 +60,5 @@ module Projects def individual_project_members project.project_members.select(*GroupMember.cached_column_list) end - - def project_owner? - if project.group.present? - project.group.owners.include?(current_user) - else - project.namespace.owner == current_user - end - end end end diff --git a/app/services/projects/prometheus/alerts/alert_params.rb b/app/services/projects/prometheus/alerts/alert_params.rb deleted file mode 100644 index 1c39ed36b12..00000000000 --- a/app/services/projects/prometheus/alerts/alert_params.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -module Projects - module Prometheus - module Alerts - module AlertParams - def alert_params - return params if params[:operator].blank? - - params.merge( - operator: PrometheusAlert.operator_to_enum(params[:operator]) - ) - end - end - end - end -end diff --git a/app/services/projects/prometheus/alerts/create_service.rb b/app/services/projects/prometheus/alerts/create_service.rb deleted file mode 100644 index 0d7d8ab1a62..00000000000 --- a/app/services/projects/prometheus/alerts/create_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module Projects - module Prometheus - module Alerts - class CreateService < BaseProjectService - include AlertParams - - def execute - project.prometheus_alerts.create(alert_params) - end - end - end - end -end diff --git a/app/services/projects/prometheus/alerts/destroy_service.rb b/app/services/projects/prometheus/alerts/destroy_service.rb deleted file mode 100644 index 243b12eb654..00000000000 --- a/app/services/projects/prometheus/alerts/destroy_service.rb +++ /dev/null @@ -1,13 +0,0 @@ -# frozen_string_literal: true - -module Projects - module Prometheus - module Alerts - class DestroyService < BaseProjectService - def execute(alert) - alert.destroy - end - end - end - end -end diff --git a/app/services/projects/prometheus/alerts/update_service.rb b/app/services/projects/prometheus/alerts/update_service.rb deleted file mode 100644 index 1802f35dae9..00000000000 --- a/app/services/projects/prometheus/alerts/update_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module Projects - module Prometheus - module Alerts - class UpdateService < BaseProjectService - include AlertParams - - def execute(alert) - alert.update(alert_params) - end - end - end - end -end diff --git a/app/services/projects/prometheus/metrics/base_service.rb b/app/services/projects/prometheus/metrics/base_service.rb index be1783dde70..15247d45776 100644 --- a/app/services/projects/prometheus/metrics/base_service.rb +++ b/app/services/projects/prometheus/metrics/base_service.rb @@ -8,40 +8,12 @@ module Projects def initialize(metric, params = {}) @metric = metric - @project = metric.project @params = params.dup end protected - attr_reader :metric, :project, :params - - def application - alert.environment.cluster_prometheus_adapter - end - - def schedule_alert_update - return unless alert - return unless alert.environment - - ::Clusters::Applications::ScheduleUpdateService.new( - alert.environment.cluster_prometheus_adapter, project).execute - end - - def alert - strong_memoize(:alert) { find_alert(metric) } - end - - def find_alert(metric) - Projects::Prometheus::AlertsFinder - .new(project: project, metric: metric) - .execute - .first - end - - def has_alert? - alert.present? - end + attr_reader :metric, :params end end end diff --git a/app/services/projects/prometheus/metrics/destroy_service.rb b/app/services/projects/prometheus/metrics/destroy_service.rb index 6a46eb5516c..d85499dc4ae 100644 --- a/app/services/projects/prometheus/metrics/destroy_service.rb +++ b/app/services/projects/prometheus/metrics/destroy_service.rb @@ -5,7 +5,6 @@ module Projects module Metrics class DestroyService < Metrics::BaseService def execute - schedule_alert_update if has_alert? metric.destroy end end diff --git a/app/services/projects/prometheus/metrics/update_service.rb b/app/services/projects/prometheus/metrics/update_service.rb deleted file mode 100644 index 9b51f4ab47d..00000000000 --- a/app/services/projects/prometheus/metrics/update_service.rb +++ /dev/null @@ -1,29 +0,0 @@ -# frozen_string_literal: true - -module Projects - module Prometheus - module Metrics - class UpdateService < Metrics::BaseService - def execute - metric.update!(params) - schedule_alert_update if requires_alert_update? - metric - end - - private - - def requires_alert_update? - has_alert? && (changing_title? || changing_query?) - end - - def changing_title? - metric.previous_changes.include?(:title) - end - - def changing_query? - metric.previous_changes.include?(:query) - end - end - end - end -end diff --git a/app/services/projects/record_target_platforms_service.rb b/app/services/projects/record_target_platforms_service.rb index 224e16f53b3..664e72e9785 100644 --- a/app/services/projects/record_target_platforms_service.rb +++ b/app/services/projects/record_target_platforms_service.rb @@ -4,26 +4,50 @@ module Projects class RecordTargetPlatformsService < BaseService include Gitlab::Utils::StrongMemoize + def initialize(project, detector_service) + @project = project + @detector_service = detector_service + end + def execute record_target_platforms end private + attr_reader :project, :detector_service + def target_platforms strong_memoize(:target_platforms) do - AppleTargetPlatformDetectorService.new(project).execute + Array(detector_service.new(project).execute) end end def record_target_platforms return unless target_platforms.present? - setting = ::ProjectSetting.find_or_initialize_by(project: project) # rubocop:disable CodeReuse/ActiveRecord - setting.target_platforms = target_platforms - setting.save + project_setting.target_platforms = target_platforms + project_setting.save + + send_build_ios_app_guide_email + + project_setting.target_platforms + end + + def project_setting + @project_setting ||= ::ProjectSetting.find_or_initialize_by(project: project) # rubocop:disable CodeReuse/ActiveRecord + end + + def experiment_candidate? + experiment(:build_ios_app_guide_email, project: project).run + end + + def send_build_ios_app_guide_email + return unless target_platforms.include? :ios + return unless experiment_candidate? - setting.target_platforms + campaign = Users::InProductMarketingEmail::BUILD_IOS_APP_GUIDE + Projects::InProductMarketingCampaignEmailsService.new(project, campaign).execute end end end diff --git a/app/services/projects/update_pages_service.rb b/app/services/projects/update_pages_service.rb index 2ec965fe2f4..c6ea364320f 100644 --- a/app/services/projects/update_pages_service.rb +++ b/app/services/projects/update_pages_service.rb @@ -30,6 +30,7 @@ module Projects validate_state! validate_max_size! + validate_public_folder! validate_max_entries! build.artifacts_file.use_file do |artifacts_path| @@ -180,6 +181,10 @@ module Projects end end + def validate_public_folder! + raise InvalidStateError, 'Error: The `public/` folder is missing, or not declared in `.gitlab-ci.yml`.' unless total_size > 0 + end + def entries_count # we're using the full archive and pages daemon needs to read it # so we want the total count from entries, not only "public/" directory diff --git a/app/services/projects/update_remote_mirror_service.rb b/app/services/projects/update_remote_mirror_service.rb index f3ea0967a99..705d23ec704 100644 --- a/app/services/projects/update_remote_mirror_service.rb +++ b/app/services/projects/update_remote_mirror_service.rb @@ -65,7 +65,7 @@ module Projects message += "Error synchronizing LFS files:" message += "\n\n#{lfs_status[:message]}\n\n" - failed = Feature.enabled?(:remote_mirror_fail_on_lfs, project, default_enabled: :yaml) + failed = Feature.enabled?(:remote_mirror_fail_on_lfs, project) end if response.divergent_refs.any? diff --git a/app/services/prometheus/create_default_alerts_service.rb b/app/services/prometheus/create_default_alerts_service.rb deleted file mode 100644 index eb8a9d45658..00000000000 --- a/app/services/prometheus/create_default_alerts_service.rb +++ /dev/null @@ -1,105 +0,0 @@ -# frozen_string_literal: true - -# DEPRECATED: To be removed as part of https://gitlab.com/groups/gitlab-org/-/epics/5877 -module Prometheus - class CreateDefaultAlertsService < BaseService - include Gitlab::Utils::StrongMemoize - - attr_reader :project - - DEFAULT_ALERTS = [ - { - identifier: 'response_metrics_nginx_ingress_16_http_error_rate', - operator: 'gt', - threshold: 0.1 - }, - { - identifier: 'response_metrics_nginx_ingress_http_error_rate', - operator: 'gt', - threshold: 0.1 - }, - { - identifier: 'response_metrics_nginx_http_error_percentage', - operator: 'gt', - threshold: 0.1 - } - ].freeze - - def initialize(project:) - @project = project - end - - def execute - return ServiceResponse.error(message: 'Invalid project') unless project - return ServiceResponse.error(message: 'Invalid environment') unless environment - - create_alerts - schedule_prometheus_update - - ServiceResponse.success - end - - private - - def create_alerts - DEFAULT_ALERTS.each do |alert_hash| - identifier = alert_hash[:identifier] - next if alerts_by_identifier(environment).key?(identifier) - - metric = metrics_by_identifier[identifier] - next unless metric - - create_alert(alert: alert_hash, metric: metric) - end - end - - def schedule_prometheus_update - return unless prometheus_adapter - - ::Clusters::Applications::ScheduleUpdateService.new(prometheus_adapter, project).execute - end - - def prometheus_adapter - environment.cluster_prometheus_adapter - end - - def metrics_by_identifier - strong_memoize(:metrics_by_identifier) do - metric_identifiers = DEFAULT_ALERTS.map { |alert| alert[:identifier] } - - PrometheusMetricsFinder - .new(identifier: metric_identifiers, common: true) - .execute - .index_by(&:identifier) - end - end - - def alerts_by_identifier(environment) - strong_memoize(:alerts_by_identifier) do - Projects::Prometheus::AlertsFinder - .new(project: project, metric: metrics_by_identifier.values, environment: environment) - .execute - .index_by { |alert| alert.prometheus_metric.identifier } - end - end - - def environment - strong_memoize(:environment) do - Environments::EnvironmentsFinder.new(project, nil, name: 'production').execute.first || - project.environments.first - end - end - - def create_alert(alert:, metric:) - PrometheusAlert.create!( - project: project, - prometheus_metric: metric, - environment: environment, - threshold: alert[:threshold], - operator: alert[:operator] - ) - rescue ActiveRecord::RecordNotUnique - # Ignore duplicate creations although it unlikely to happen - end - end -end diff --git a/app/services/quick_actions/interpret_service.rb b/app/services/quick_actions/interpret_service.rb index 47f4b9c6898..4bcb15b2d9c 100644 --- a/app/services/quick_actions/interpret_service.rb +++ b/app/services/quick_actions/interpret_service.rb @@ -44,7 +44,7 @@ module QuickActions content, commands = extractor.extract_commands(content, only: only) extract_updates(commands) - [content, @updates, execution_messages_for(commands)] + [content, @updates, execution_messages_for(commands), command_names(commands)] end # Takes a text and interprets the commands that are extracted from it. @@ -83,8 +83,10 @@ module QuickActions args.map! { _1.gsub(/\\_/, '_') } usernames = (args - ['me']).map { _1.delete_prefix('@') } found = User.by_username(usernames).to_a.select { can?(:read_user, _1) } - found_names = found.map(&:username).to_set - missing = args.reject { |arg| arg == 'me' || found_names.include?(arg.delete_prefix('@')) }.map { "'#{_1}'" } + found_names = found.map(&:username).map(&:downcase).to_set + missing = args.reject do |arg| + arg == 'me' || found_names.include?(arg.downcase.delete_prefix('@')) + end.map { "'#{_1}'" } failed_parse(format(_("Failed to find users for %{missing}"), missing: missing.to_sentence)) if missing.present? @@ -165,6 +167,15 @@ module QuickActions end.compact end + def command_names(commands) + commands.flatten.map do |name| + definition = self.class.definition_by_name(name) + next unless definition + + name + end.compact + end + def extract_updates(commands) commands.each do |name, arg| definition = self.class.definition_by_name(name) diff --git a/app/services/service_ping/build_payload_service.rb b/app/services/service_ping/build_payload_service.rb deleted file mode 100644 index f4ae939fd07..00000000000 --- a/app/services/service_ping/build_payload_service.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -module ServicePing - class BuildPayloadService - def execute - return {} unless allowed_to_report? - - raw_payload - end - - private - - def allowed_to_report? - product_intelligence_enabled? && !User.single_user&.requires_usage_stats_consent? - end - - def product_intelligence_enabled? - ::Gitlab::CurrentSettings.usage_ping_enabled? - end - - def raw_payload - @raw_payload ||= ::Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values) - end - end -end - -ServicePing::BuildPayloadService.prepend_mod_with('ServicePing::BuildPayloadService') diff --git a/app/services/service_ping/devops_report_service.rb b/app/services/service_ping/devops_report_service.rb deleted file mode 100644 index 3b8f5dfdb82..00000000000 --- a/app/services/service_ping/devops_report_service.rb +++ /dev/null @@ -1,26 +0,0 @@ -# frozen_string_literal: true - -module ServicePing - class DevopsReportService - def initialize(data) - @data = data - end - - def execute - # `conv_index` was previously named `dev_ops_score` in - # version-gitlab-com, so we check both for backwards compatibility. - metrics = @data['conv_index'] || @data['dev_ops_score'] - - # Do not attempt to save a report for the first Service Ping - # response for a given GitLab instance, which comes without - # metrics. - return if metrics.keys == ['usage_data_id'] - - report = DevOpsReport::Metric.create( - metrics.slice(*DevOpsReport::Metric::METRICS) - ) - - Gitlab::ErrorTracking.track_and_raise_for_dev_exception(ActiveRecord::RecordInvalid.new(report)) unless report.persisted? - end - end -end diff --git a/app/services/service_ping/permit_data_categories_service.rb b/app/services/service_ping/permit_data_categories_service.rb deleted file mode 100644 index d8fa255a485..00000000000 --- a/app/services/service_ping/permit_data_categories_service.rb +++ /dev/null @@ -1,24 +0,0 @@ -# frozen_string_literal: true - -module ServicePing - class PermitDataCategoriesService - STANDARD_CATEGORY = 'standard' - SUBSCRIPTION_CATEGORY = 'subscription' - OPERATIONAL_CATEGORY = 'operational' - OPTIONAL_CATEGORY = 'optional' - CATEGORIES = [ - STANDARD_CATEGORY, - SUBSCRIPTION_CATEGORY, - OPERATIONAL_CATEGORY, - OPTIONAL_CATEGORY - ].to_set.freeze - - def execute - return [] unless ServicePingSettings.product_intelligence_enabled? - - CATEGORIES - end - end -end - -ServicePing::PermitDataCategoriesService.prepend_mod_with('ServicePing::PermitDataCategoriesService') diff --git a/app/services/service_ping/service_ping_settings.rb b/app/services/service_ping/service_ping_settings.rb deleted file mode 100644 index 6964210b1db..00000000000 --- a/app/services/service_ping/service_ping_settings.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -module ServicePing - module ServicePingSettings - extend self - - def product_intelligence_enabled? - enabled? && !User.single_user&.requires_usage_stats_consent? - end - - def enabled? - ::Gitlab::CurrentSettings.usage_ping_enabled? - end - end -end - -ServicePing::ServicePingSettings.extend_mod_with('ServicePing::ServicePingSettings') diff --git a/app/services/service_ping/submit_service.rb b/app/services/service_ping/submit_service.rb index c8733bc2f11..343fc00a2f0 100644 --- a/app/services/service_ping/submit_service.rb +++ b/app/services/service_ping/submit_service.rb @@ -6,6 +6,7 @@ module ServicePing STAGING_BASE_URL = 'https://gitlab-services-version-gitlab-com-staging.gs-staging.gitlab.org' USAGE_DATA_PATH = 'usage_data' ERROR_PATH = 'usage_ping_errors' + METADATA_PATH = 'usage_ping_metadata' SubmissionError = Class.new(StandardError) @@ -18,26 +19,27 @@ module ServicePing start = Time.current begin - usage_data = BuildPayloadService.new.execute + usage_data = ServicePing::BuildPayload.new.execute response = submit_usage_data_payload(usage_data) rescue StandardError => e return unless Gitlab::CurrentSettings.usage_ping_enabled? error_payload = { time: Time.current, - uuid: Gitlab::UsageData.add_metric('UuidMetric'), - hostname: Gitlab::UsageData.add_metric('HostnameMetric'), - version: Gitlab::UsageData.alt_usage_data { Gitlab::VERSION }, - message: e.message, + uuid: Gitlab::CurrentSettings.uuid, + hostname: Gitlab.config.gitlab.host, + version: Gitlab.version_info.to_s, + message: "#{e.message.presence || e.class} at #{e.backtrace[0]}", elapsed: (Time.current - start).round(1) } - submit_payload({ error: error_payload }, url: error_url) + submit_payload({ error: error_payload }, path: ERROR_PATH) usage_data = Gitlab::Usage::ServicePingReport.for(output: :all_metrics_values) response = submit_usage_data_payload(usage_data) end - version_usage_data_id = response.dig('conv_index', 'usage_data_id') || response.dig('dev_ops_score', 'usage_data_id') + version_usage_data_id = + response.dig('conv_index', 'usage_data_id') || response.dig('dev_ops_score', 'usage_data_id') unless version_usage_data_id.is_a?(Integer) && version_usage_data_id > 0 raise SubmissionError, "Invalid usage_data_id in response: #{version_usage_data_id}" @@ -46,23 +48,32 @@ module ServicePing unless @skip_db_write raw_usage_data = save_raw_usage_data(usage_data) raw_usage_data.update_version_metadata!(usage_data_id: version_usage_data_id) - DevopsReportService.new(response).execute + ServicePing::DevopsReport.new(response).execute end - end - def url - URI.join(base_url, USAGE_DATA_PATH) - end + return unless Feature.enabled?(:measure_service_ping_metric_collection) - def error_url - URI.join(base_url, ERROR_PATH) + submit_payload({ metadata: { metrics: metrics_collection_time(usage_data) } }, path: METADATA_PATH) end private - def submit_payload(payload, url: self.url) + def metrics_collection_time(payload, parents = []) + return [] unless payload.is_a?(Hash) + + payload.flat_map do |key, metric_value| + key_path = parents.dup.append(key) + if metric_value.respond_to?(:duration) + { name: key_path.join('.'), time_elapsed: metric_value.duration } + else + metrics_collection_time(metric_value, key_path) + end + end + end + + def submit_payload(payload, path: USAGE_DATA_PATH) Gitlab::HTTP.post( - url, + URI.join(base_url, path), body: payload.to_json, allow_local_requests: true, headers: { 'Content-type' => 'application/json' } @@ -80,9 +91,13 @@ module ServicePing end def save_raw_usage_data(usage_data) - RawUsageData.safe_find_or_create_by(recorded_at: usage_data[:recorded_at]) do |record| + # safe_find_or_create_by! was originally called here. + # We merely switched to `find_or_create_by!` + # rubocop: disable CodeReuse/ActiveRecord + RawUsageData.find_or_create_by(recorded_at: usage_data[:recorded_at]) do |record| record.payload = usage_data end + # rubocop: enable CodeReuse/ActiveRecord end # See https://gitlab.com/gitlab-org/gitlab/-/issues/233615 for details diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index 9db39a5e174..d7e4b53b5de 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -111,6 +111,21 @@ module SystemNoteService ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).change_time_spent end + # Called when a timelog is removed from a Noteable + # + # noteable - Noteable object + # project - Project owning the noteable + # author - User performing the change + # timelog - The removed timelog + # + # Example Note text: + # "deleted 2h 30m of time spent from 22-03-2022" + # + # Returns the created Note object + def remove_timelog(noteable, project, author, timelog) + ::SystemNotes::TimeTrackingService.new(noteable: noteable, project: project, author: author).remove_timelog(timelog) + end + def close_after_error_tracking_resolve(issue, project, author) ::SystemNotes::IssuablesService.new(noteable: issue, project: project, author: author).close_after_error_tracking_resolve end @@ -351,11 +366,27 @@ module SystemNoteService ::SystemNotes::IssuablesService.new(noteable: issue, project: issue.project, author: author).change_issue_type end + def add_timeline_event(timeline_event) + incidents_service(timeline_event.incident).add_timeline_event(timeline_event) + end + + def edit_timeline_event(timeline_event, author, was_changed:) + incidents_service(timeline_event.incident).edit_timeline_event(timeline_event, author, was_changed: was_changed) + end + + def delete_timeline_event(noteable, author) + incidents_service(noteable).delete_timeline_event(author) + end + private def merge_requests_service(noteable, project, author) ::SystemNotes::MergeRequestsService.new(noteable: noteable, project: project, author: author) end + + def incidents_service(incident) + ::SystemNotes::IncidentsService.new(noteable: incident) + end end SystemNoteService.prepend_mod_with('SystemNoteService') diff --git a/app/services/system_notes/incidents_service.rb b/app/services/system_notes/incidents_service.rb new file mode 100644 index 00000000000..d5da684a2d8 --- /dev/null +++ b/app/services/system_notes/incidents_service.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module SystemNotes + class IncidentsService < ::SystemNotes::BaseService + CHANGED_TEXT = { + occurred_at: 'the event time/date on ', + note: 'the text on ', + occurred_at_and_note: 'the event time/date and text on ' + }.freeze + + def initialize(noteable:) + @noteable = noteable + @project = noteable.project + end + + def add_timeline_event(timeline_event) + author = timeline_event.author + anchor = "timeline_event_#{timeline_event.id}" + path = url_helpers.project_issues_incident_path(project, noteable, anchor: anchor) + body = "added an [incident timeline event](#{path})" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'timeline_event')) + end + + def edit_timeline_event(timeline_event, author, was_changed:) + anchor = "timeline_event_#{timeline_event.id}" + path = url_helpers.project_issues_incident_path(project, noteable, anchor: anchor) + changed_text = CHANGED_TEXT.fetch(was_changed, '') + body = "edited #{changed_text}[incident timeline event](#{path})" + + create_note(NoteSummary.new(noteable, project, author, body, action: 'timeline_event')) + end + + def delete_timeline_event(author) + body = 'deleted an incident timeline event' + + create_note(NoteSummary.new(noteable, project, author, body, action: 'timeline_event')) + end + end +end diff --git a/app/services/system_notes/time_tracking_service.rb b/app/services/system_notes/time_tracking_service.rb index a804a06fe4c..a9b1f6d3d37 100644 --- a/app/services/system_notes/time_tracking_service.rb +++ b/app/services/system_notes/time_tracking_service.rb @@ -76,6 +76,18 @@ module SystemNotes create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) end + def remove_timelog(timelog) + time_spent = timelog.time_spent + spent_at = timelog.spent_at&.to_date + + parsed_time = Gitlab::TimeTrackingFormatter.output(time_spent) + + body = "deleted #{parsed_time} of spent time" + body += " from #{spent_at}" if spent_at + + create_note(NoteSummary.new(noteable, project, author, body, action: 'time_tracking')) + end + private def issue_activity_counter diff --git a/app/services/timelogs/base_service.rb b/app/services/timelogs/base_service.rb new file mode 100644 index 00000000000..be46c26e047 --- /dev/null +++ b/app/services/timelogs/base_service.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Timelogs + class BaseService + include BaseServiceUtility + include Gitlab::Utils::StrongMemoize + + attr_accessor :timelog, :current_user + + def initialize(timelog, user) + @timelog = timelog + @current_user = user + end + end +end diff --git a/app/services/timelogs/delete_service.rb b/app/services/timelogs/delete_service.rb new file mode 100644 index 00000000000..0df888a3706 --- /dev/null +++ b/app/services/timelogs/delete_service.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +module Timelogs + class DeleteService < Timelogs::BaseService + def execute + unless can?(current_user, :admin_timelog, timelog) + return ServiceResponse.error( + message: "Timelog doesn't exist or you don't have permission to delete it", + http_status: 404) + end + + if timelog.destroy + issuable = timelog.issuable + + if issuable + # Add a system note for the timelog removal + SystemNoteService.remove_timelog(issuable, issuable.project, current_user, timelog) + end + + ServiceResponse.success(payload: timelog) + else + ServiceResponse.error(message: 'Failed to remove timelog', http_status: 400) + end + end + end +end diff --git a/app/services/users/destroy_service.rb b/app/services/users/destroy_service.rb index 1ea65049dc2..dfa9316889e 100644 --- a/app/services/users/destroy_service.rb +++ b/app/services/users/destroy_service.rb @@ -63,10 +63,7 @@ module Users # destroying: https://github.com/rails/rails/issues/22510 # This ensures we delete records in batches. user.destroy_dependent_associations_in_batches(exclude: [:snippets]) - - if Feature.enabled?(:nullify_in_batches_on_user_deletion, default_enabled: :yaml) - user.nullify_dependent_associations_in_batches - end + user.nullify_dependent_associations_in_batches # Destroy the namespace after destroying the user since certain methods may depend on the namespace existing user_data = user.destroy diff --git a/app/services/namespaces/in_product_marketing_email_records.rb b/app/services/users/in_product_marketing_email_records.rb index 1237a05ea13..94dbd809496 100644 --- a/app/services/namespaces/in_product_marketing_email_records.rb +++ b/app/services/users/in_product_marketing_email_records.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -module Namespaces +module Users class InProductMarketingEmailRecords attr_reader :records @@ -13,9 +13,10 @@ module Namespaces @records = [] end - def add(user, track, series) + def add(user, campaign: nil, track: nil, series: nil) @records << Users::InProductMarketingEmail.new( user: user, + campaign: campaign, track: track, series: series, created_at: Time.zone.now, diff --git a/app/services/users/validate_otp_service.rb b/app/services/users/validate_manual_otp_service.rb index c8a9f217d22..96a827db13c 100644 --- a/app/services/users/validate_otp_service.rb +++ b/app/services/users/validate_manual_otp_service.rb @@ -1,13 +1,13 @@ # frozen_string_literal: true module Users - class ValidateOtpService < BaseService + class ValidateManualOtpService < BaseService include ::Gitlab::Auth::Otp::Fortinet def initialize(current_user) @current_user = current_user @strategy = if forti_authenticator_enabled?(current_user) - ::Gitlab::Auth::Otp::Strategies::FortiAuthenticator.new(current_user) + ::Gitlab::Auth::Otp::Strategies::FortiAuthenticator::ManualOtp.new(current_user) elsif forti_token_cloud_enabled?(current_user) ::Gitlab::Auth::Otp::Strategies::FortiTokenCloud.new(current_user) else @@ -19,7 +19,7 @@ module Users strategy.validate(otp_code) rescue StandardError => ex Gitlab::ErrorTracking.log_exception(ex) - error(message: ex.message) + error(ex.message) end private diff --git a/app/services/users/validate_push_otp_service.rb b/app/services/users/validate_push_otp_service.rb new file mode 100644 index 00000000000..6a914cda28c --- /dev/null +++ b/app/services/users/validate_push_otp_service.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Users + class ValidatePushOtpService < BaseService + include ::Gitlab::Auth::Otp::Fortinet + + def initialize(current_user) + @current_user = current_user + @strategy = if forti_authenticator_enabled?(current_user) + ::Gitlab::Auth::Otp::Strategies::FortiAuthenticator::PushOtp.new(current_user) + end + end + + def execute + strategy.validate + rescue StandardError => ex + Gitlab::ErrorTracking.log_exception(ex) + error(ex.message) + end + + private + + attr_reader :strategy + end +end diff --git a/app/services/work_items/delete_task_service.rb b/app/services/work_items/delete_task_service.rb new file mode 100644 index 00000000000..3bb23576442 --- /dev/null +++ b/app/services/work_items/delete_task_service.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module WorkItems + class DeleteTaskService + def initialize(work_item:, current_user: nil, task_params: {}, lock_version:) + @work_item = work_item + @current_user = current_user + @task_params = task_params + @lock_version = lock_version + @task = task_params[:task] + @errors = [] + end + + def execute + transaction_result = ::WorkItem.transaction do + replacement_result = TaskListReferenceRemovalService.new( + work_item: @work_item, + task: @task, + line_number_start: @task_params[:line_number_start], + line_number_end: @task_params[:line_number_end], + lock_version: @lock_version, + current_user: @current_user + ).execute + + break ::ServiceResponse.error(message: replacement_result.errors, http_status: 422) if replacement_result.error? + + delete_result = ::WorkItems::DeleteService.new( + project: @task.project, + current_user: @current_user + ).execute(@task) + + if delete_result.error? + @errors += delete_result.errors + raise ActiveRecord::Rollback + end + + delete_result + end + + return transaction_result if transaction_result + + ::ServiceResponse.error(message: @errors, http_status: 422) + end + end +end diff --git a/app/services/work_items/task_list_reference_removal_service.rb b/app/services/work_items/task_list_reference_removal_service.rb new file mode 100644 index 00000000000..e7ec73a96e0 --- /dev/null +++ b/app/services/work_items/task_list_reference_removal_service.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +module WorkItems + class TaskListReferenceRemovalService + STALE_OBJECT_MESSAGE = 'Stale work item. Check lock version' + + def initialize(work_item:, task:, line_number_start:, line_number_end:, lock_version:, current_user:) + @work_item = work_item + @task = task + @line_number_start = line_number_start + @line_number_end = line_number_end + @lock_version = lock_version + @current_user = current_user + end + + def execute + return ::ServiceResponse.error(message: 'line_number_start must be greater than 0') if @line_number_start < 1 + return ::ServiceResponse.error(message: "Work item description can't be blank") if @work_item.description.blank? + + if @line_number_end < @line_number_start + return ::ServiceResponse.error(message: 'line_number_end must be greater or equal to line_number_start') + end + + source_lines = @work_item.description.split("\n") + + line_matches_reference = (@line_number_start..@line_number_end).any? do |line_number| + markdown_line = source_lines[line_number - 1] + + /#{Regexp.escape(@task.to_reference)}(?!\d)/.match?(markdown_line) + end + + unless line_matches_reference + return ::ServiceResponse.error( + message: "Unable to detect a task on lines #{@line_number_start}-#{@line_number_end}" + ) + end + + remove_task_lines!(source_lines) + + ::WorkItems::UpdateService.new( + project: @work_item.project, + current_user: @current_user, + params: { description: source_lines.join("\n"), lock_version: @lock_version } + ).execute(@work_item) + + if @work_item.valid? + ::ServiceResponse.success + else + ::ServiceResponse.error(message: @work_item.errors.full_messages) + end + rescue ActiveRecord::StaleObjectError + ::ServiceResponse.error(message: STALE_OBJECT_MESSAGE) + end + + private + + def remove_task_lines!(source_lines) + source_lines.delete_if.each_with_index do |_line, index| + index >= @line_number_start - 1 && index < @line_number_end + end + end + end +end |