From aad30aa0ec4b84e06e3f7da3e624c81b7a2d22b5 Mon Sep 17 00:00:00 2001
From: Dylan Griffith <dyl.griffith@gmail.com>
Date: Fri, 31 Jan 2025 07:52:30 +1100
Subject: [PATCH] Add Cop/ActiveRecordDependent for ee/models
This is related to https://gitlab.com/gitlab-org/gitlab/-/issues/516113
where we discovered that many cops are not being applied to files in the
`ee/` directory.
This MR simply fixes that for `Cop/ActiveRecordDependent` so that we
don't continue adding violations and adds a `rubocop:disable` for all
the existing violations.
---
.rubocop.yml | 4 ++++
.../work_items/unified_associations/award_emoji.rb | 4 +++-
.../work_items/unified_associations/events.rb | 4 ++++
.../unified_associations/subscriptions.rb | 2 ++
.../models/dependencies/dependency_list_export.rb | 2 ++
ee/app/models/ee/epic.rb | 4 ++++
ee/app/models/ee/group.rb | 4 +++-
ee/app/models/ee/issue.rb | 2 ++
ee/app/models/ee/merge_request.rb | 6 ++++--
ee/app/models/ee/namespace.rb | 2 ++
ee/app/models/ee/project.rb | 10 +++++-----
ee/app/models/ee/user.rb | 14 +++++++++++---
ee/app/models/geo_node.rb | 2 ++
.../models/gitlab_subscriptions/add_on_purchase.rb | 2 +-
ee/app/models/issuables/custom_field.rb | 2 ++
.../models/requirements_management/requirement.rb | 2 ++
ee/app/models/search/zoekt/enabled_namespace.rb | 4 ++--
ee/app/models/vulnerabilities/export.rb | 2 +-
18 files changed, 56 insertions(+), 16 deletions(-)
diff --git a/.rubocop.yml b/.rubocop.yml
index 465c65ec86b30..43065204c8116 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -1251,3 +1251,7 @@ Tailwind/StringInterpolation:
- '{,ee/,jh/}app/helpers/**/*.rb'
- '{,ee/,jh/}app/components/**/*.{haml,rb}'
- '{,ee/,jh/}app/views/**/*.haml'
+
+Cop/ActiveRecordDependent:
+ Include:
+ - ee/app/models/**/*.rb
diff --git a/ee/app/models/concerns/work_items/unified_associations/award_emoji.rb b/ee/app/models/concerns/work_items/unified_associations/award_emoji.rb
index 31b4db06a4166..12130f955a7af 100644
--- a/ee/app/models/concerns/work_items/unified_associations/award_emoji.rb
+++ b/ee/app/models/concerns/work_items/unified_associations/award_emoji.rb
@@ -6,11 +6,13 @@ module AwardEmoji
extend ActiveSupport::Concern
included do
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :own_award_emoji, as: :awardable, inverse_of: :awardable, class_name: 'AwardEmoji', dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_many :award_emoji, -> {
includes(:user).order(:id)
- }, as: :awardable, inverse_of: :awardable, dependent: :destroy do
+ }, as: :awardable, inverse_of: :awardable, dependent: :destroy do # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
def find(*args)
return super unless proxy_association.owner.unified_associations?
return super if block_given?
diff --git a/ee/app/models/concerns/work_items/unified_associations/events.rb b/ee/app/models/concerns/work_items/unified_associations/events.rb
index 0ab2223970a9d..03511ab3f5bec 100644
--- a/ee/app/models/concerns/work_items/unified_associations/events.rb
+++ b/ee/app/models/concerns/work_items/unified_associations/events.rb
@@ -7,9 +7,13 @@ module Events
included do
has_many :own_events, ->(record) { where(target_type: [record.class.name, record.class.base_class.name].uniq) },
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
foreign_key: :target_id, inverse_of: :target, class_name: 'Event', dependent: :delete_all
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_many :events, ->(record) { where(target_type: [record.class.name, record.class.base_class.name].uniq) },
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
foreign_key: :target_id, inverse_of: :target, class_name: 'Event', dependent: :delete_all do
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
def load_target
return super unless proxy_association.owner.unified_associations?
diff --git a/ee/app/models/concerns/work_items/unified_associations/subscriptions.rb b/ee/app/models/concerns/work_items/unified_associations/subscriptions.rb
index ec7711a90666f..293f252cd8fb4 100644
--- a/ee/app/models/concerns/work_items/unified_associations/subscriptions.rb
+++ b/ee/app/models/concerns/work_items/unified_associations/subscriptions.rb
@@ -6,7 +6,9 @@ module Subscriptions
extend ActiveSupport::Concern
included do
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :own_subscriptions, dependent: :destroy, class_name: 'Subscription', as: :subscribable
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_many :subscriptions, as: :subscribable do
def load_target
return super unless proxy_association.owner.unified_associations?
diff --git a/ee/app/models/dependencies/dependency_list_export.rb b/ee/app/models/dependencies/dependency_list_export.rb
index 0fbc0244bc60a..980d2aacc66cb 100644
--- a/ee/app/models/dependencies/dependency_list_export.rb
+++ b/ee/app/models/dependencies/dependency_list_export.rb
@@ -15,7 +15,9 @@ class DependencyListExport < Gitlab::Database::SecApplicationRecord
belongs_to :pipeline, class_name: 'Ci::Pipeline'
belongs_to :author, class_name: 'User', foreign_key: :user_id, inverse_of: :dependency_list_exports
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :export_parts, class_name: 'Dependencies::DependencyListExport::Part', dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
validates :status, presence: true
validates :file, presence: true, if: :finished?
diff --git a/ee/app/models/ee/epic.rb b/ee/app/models/ee/epic.rb
index 2a9cde8c47d6d..9f8b7289325cf 100644
--- a/ee/app/models/ee/epic.rb
+++ b/ee/app/models/ee/epic.rb
@@ -69,14 +69,18 @@ module Epic
belongs_to :due_date_sourcing_epic, class_name: 'Epic'
belongs_to :parent, class_name: "Epic"
has_many :children, class_name: "Epic", foreign_key: :parent_id
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
belongs_to :work_item, foreign_key: 'issue_id', inverse_of: :synced_epic, dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
belongs_to :sync_object, foreign_key: 'issue_id', class_name: 'WorkItem', inverse_of: :sync_object
has_internal_id :iid, scope: :group
has_many :epic_issues
has_many :issues, through: :epic_issues
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :user_mentions, class_name: "EpicUserMention", dependent: :delete_all
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_many :boards_epic_user_preferences, class_name: 'Boards::EpicUserPreference', inverse_of: :epic
has_many :epic_board_positions, class_name: 'Boards::EpicBoardPosition', foreign_key: :epic_id, inverse_of: :epic
diff --git a/ee/app/models/ee/group.rb b/ee/app/models/ee/group.rb
index bbb29cf19b020..089e9a7e35c1e 100644
--- a/ee/app/models/ee/group.rb
+++ b/ee/app/models/ee/group.rb
@@ -44,7 +44,9 @@ module Group
accepts_nested_attributes_for :analytics_dashboards_pointer, allow_destroy: true
accepts_nested_attributes_for :value_stream_dashboard_aggregation, update_only: true
has_one :analytics_dashboards_configuration_project, through: :analytics_dashboards_pointer, source: :target_project
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_one :index_status, class_name: 'Elastic::GroupIndexStatus', foreign_key: :namespace_id, dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_one :google_cloud_platform_workload_identity_federation_integration, class_name: 'Integrations::GoogleCloudPlatform::WorkloadIdentityFederation'
has_many :external_audit_event_destinations, class_name: "AuditEvents::ExternalAuditEventDestination", foreign_key: 'namespace_id'
has_many :external_audit_event_streaming_destinations, class_name: "AuditEvents::Group::ExternalStreamingDestination", foreign_key: 'group_id'
@@ -55,7 +57,7 @@ module Group
foreign_key: 'namespace_id',
inverse_of: :group
- has_many :ldap_group_links, foreign_key: 'group_id', dependent: :destroy
+ has_many :ldap_group_links, foreign_key: 'group_id', dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :saml_group_links, foreign_key: 'group_id'
has_many :hooks, class_name: 'GroupHook'
diff --git a/ee/app/models/ee/issue.rb b/ee/app/models/ee/issue.rb
index 66b7315b894fd..1305b5663832e 100644
--- a/ee/app/models/ee/issue.rb
+++ b/ee/app/models/ee/issue.rb
@@ -141,7 +141,9 @@ module Issue
has_many :observability_traces, class_name: 'Observability::TracesIssuesConnection', inverse_of: :issue
has_one :sync_object, class_name: 'Epic', foreign_key: 'issue_id', inverse_of: :sync_object
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_one :synced_epic, class_name: 'Epic', foreign_key: 'issue_id', inverse_of: :work_item, dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
validates :weight, allow_nil: true, numericality: { greater_than_or_equal_to: 0 }
validate :validate_confidential_epic
diff --git a/ee/app/models/ee/merge_request.rb b/ee/app/models/ee/merge_request.rb
index 114423aa987fe..5ea442f034770 100644
--- a/ee/app/models/ee/merge_request.rb
+++ b/ee/app/models/ee/merge_request.rb
@@ -27,9 +27,9 @@ module MergeRequest
belongs_to :iteration, foreign_key: 'sprint_id', inverse_of: :merge_requests
- has_many :approvers, as: :target, dependent: :delete_all
+ has_many :approvers, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :approver_users, through: :approvers, source: :user
- has_many :approver_groups, as: :target, dependent: :delete_all
+ has_many :approver_groups, as: :target, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :status_check_responses, class_name: 'MergeRequests::StatusCheckResponse', inverse_of: :merge_request
has_many :approval_rules, class_name: 'ApprovalMergeRequestRule', inverse_of: :merge_request do
def applicable_to_branch(branch)
@@ -52,7 +52,9 @@ def set_applicable_when_copying_rules(applicable_ids)
inverse_of: :merge_request
has_many :approval_merge_request_rule_sources, through: :approval_rules
has_many :approval_project_rules, through: :approval_merge_request_rule_sources
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_one :merge_train_car, class_name: 'MergeTrains::Car', inverse_of: :merge_request, dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_many :blocks_as_blocker,
class_name: 'MergeRequestBlock',
diff --git a/ee/app/models/ee/namespace.rb b/ee/app/models/ee/namespace.rb
index f24bc107ee647..9db1f706478f2 100644
--- a/ee/app/models/ee/namespace.rb
+++ b/ee/app/models/ee/namespace.rb
@@ -46,7 +46,9 @@ module Namespace
has_many :ci_minutes_additional_packs, class_name: "Ci::Minutes::AdditionalPack"
has_many :compliance_management_frameworks, class_name: "ComplianceManagement::Framework"
has_many :member_roles
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :subscription_add_on_purchases, class_name: 'GitlabSubscriptions::AddOnPurchase', dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
accepts_nested_attributes_for :gitlab_subscription, update_only: true
accepts_nested_attributes_for :namespace_limit
diff --git a/ee/app/models/ee/project.rb b/ee/app/models/ee/project.rb
index b2ad2dfe79864..2be1c216caf21 100644
--- a/ee/app/models/ee/project.rb
+++ b/ee/app/models/ee/project.rb
@@ -83,9 +83,9 @@ def preload_protected_branches
has_many :zoekt_repositories, class_name: '::Search::Zoekt::Repository', inverse_of: :project
has_one :secrets_manager, class_name: '::SecretsManagement::ProjectSecretsManager'
- has_many :approvers, as: :target, dependent: :destroy
+ has_many :approvers, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :approver_users, through: :approvers, source: :user
- has_many :approver_groups, as: :target, dependent: :destroy
+ has_many :approver_groups, as: :target, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :approval_rules, class_name: 'ApprovalProjectRule', extend: FilterByBranch
# NOTE: This was added to avoid N+1 queries when we load list of MergeRequests
has_many :regular_or_any_approver_approval_rules, -> { regular_or_any_approver.order(rule_type: :desc, id: :asc) }, class_name: 'ApprovalProjectRule', extend: FilterByBranch
@@ -99,14 +99,14 @@ def preload_protected_branches
# the rationale behind vulnerabilities and vulnerability_findings can be found here:
# https://gitlab.com/gitlab-org/gitlab/issues/10252#terminology
- has_many :vulnerabilities, dependent: :destroy
+ has_many :vulnerabilities, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :vulnerability_reads, class_name: 'Vulnerabilities::Read'
has_many :vulnerability_feedback, class_name: 'Vulnerabilities::Feedback'
has_many :vulnerability_historical_statistics, class_name: 'Vulnerabilities::HistoricalStatistic'
has_many :vulnerability_findings,
class_name: 'Vulnerabilities::Finding',
inverse_of: :project,
- dependent: :destroy do
+ dependent: :destroy do # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
def lock_for_confirmation!(id)
where(vulnerability_id: nil).lock.find(id)
end
@@ -135,7 +135,7 @@ def lock_for_confirmation!(id)
class_name: 'Security::ScanResultPolicyRead',
foreign_key: 'project_id',
inverse_of: :project,
- dependent: :delete_all
+ dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :scan_result_policy_violations,
class_name: 'Security::ScanResultPolicyViolation',
foreign_key: 'project_id',
diff --git a/ee/app/models/ee/user.rb b/ee/app/models/ee/user.rb
index 4e45621063a01..1b1a6bb709b9e 100644
--- a/ee/app/models/ee/user.rb
+++ b/ee/app/models/ee/user.rb
@@ -68,7 +68,7 @@ module User
has_many :epics, foreign_key: :author_id
has_many :test_reports, foreign_key: :author_id, inverse_of: :author, class_name: 'RequirementsManagement::TestReport'
has_many :assigned_epics, foreign_key: :assignee_id, class_name: "Epic"
- has_many :path_locks, dependent: :destroy
+ has_many :path_locks, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :vulnerability_feedback, foreign_key: :author_id, class_name: 'Vulnerabilities::Feedback'
has_many :vulnerability_state_transitions, foreign_key: :author_id, class_name: 'Vulnerabilities::StateTransition', inverse_of: :author
has_many :vulnerability_severity_overrides, foreign_key: :author_id, class_name: 'Vulnerabilities::SeverityOverride', inverse_of: :author
@@ -77,8 +77,8 @@ module User
has_many :epic_board_recent_visits, class_name: 'Boards::EpicBoardRecentVisit', inverse_of: :user
has_many :project_deletion_schedules, class_name: '::Projects::DeletionSchedule', inverse_of: :deleting_user
- has_many :approvals, dependent: :destroy
- has_many :approvers, dependent: :destroy
+ has_many :approvals, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
+ has_many :approvers, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :minimal_access_group_members, -> { where(access_level: [::Gitlab::Access::MINIMAL_ACCESS]) }, class_name: 'GroupMember'
has_many :minimal_access_groups, through: :minimal_access_group_members, source: :group
@@ -93,9 +93,15 @@ module User
has_many :group_saml_providers, through: :group_saml_identities, source: :saml_provider
# Protected Branch Access
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :protected_branch_merge_access_levels, dependent: :destroy, class_name: "::ProtectedBranch::MergeAccessLevel"
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :protected_branch_push_access_levels, dependent: :destroy, class_name: "::ProtectedBranch::PushAccessLevel"
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :protected_branch_unprotect_access_levels, dependent: :destroy, class_name: "::ProtectedBranch::UnprotectAccessLevel"
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_many :deployment_approvals, class_name: 'Deployments::Approval'
@@ -123,7 +129,9 @@ module User
has_many :dependency_list_exports, class_name: 'Dependencies::DependencyListExport', inverse_of: :author
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :assigned_add_ons, class_name: 'GitlabSubscriptions::UserAddOnAssignment', inverse_of: :user, dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_many :created_remote_development_namespace_cluster_agent_mappings,
class_name: 'RemoteDevelopment::RemoteDevelopmentNamespaceClusterAgentMapping',
diff --git a/ee/app/models/geo_node.rb b/ee/app/models/geo_node.rb
index efb91853d68d6..a3868f21736da 100644
--- a/ee/app/models/geo_node.rb
+++ b/ee/app/models/geo_node.rb
@@ -10,7 +10,9 @@ class GeoNode < ApplicationRecord
# Array of repository storages to synchronize for selective sync by shards
serialize :selective_sync_shards, Array
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
belongs_to :oauth_application, class_name: 'Doorkeeper::Application', dependent: :destroy, autosave: true
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
has_many :geo_node_namespace_links
has_many :namespaces, through: :geo_node_namespace_links
diff --git a/ee/app/models/gitlab_subscriptions/add_on_purchase.rb b/ee/app/models/gitlab_subscriptions/add_on_purchase.rb
index 62803571cb843..bb91923a05ab5 100644
--- a/ee/app/models/gitlab_subscriptions/add_on_purchase.rb
+++ b/ee/app/models/gitlab_subscriptions/add_on_purchase.rb
@@ -13,7 +13,7 @@ class AddOnPurchase < ApplicationRecord
belongs_to :namespace, optional: true
belongs_to :organization, class_name: 'Organizations::Organization'
has_many :assigned_users, class_name: 'GitlabSubscriptions::UserAddOnAssignment', inverse_of: :add_on_purchase,
- dependent: :destroy
+ dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :users, through: :assigned_users
validates :add_on, :expires_on, presence: true
diff --git a/ee/app/models/issuables/custom_field.rb b/ee/app/models/issuables/custom_field.rb
index 9a3d1b15af54d..54f273301c078 100644
--- a/ee/app/models/issuables/custom_field.rb
+++ b/ee/app/models/issuables/custom_field.rb
@@ -14,7 +14,9 @@ class CustomField < ApplicationRecord
belongs_to :namespace
belongs_to :created_by, class_name: 'User', optional: true
belongs_to :updated_by, class_name: 'User', optional: true
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :select_options, -> { order(:position, :id) }, dependent: :delete_all, autosave: true,
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
class_name: 'Issuables::CustomFieldSelectOption', inverse_of: :custom_field
has_many :work_item_type_custom_fields, class_name: 'WorkItems::TypeCustomField'
has_many :work_item_types, -> { order(:name) },
diff --git a/ee/app/models/requirements_management/requirement.rb b/ee/app/models/requirements_management/requirement.rb
index f8d6379a31698..daaf9460e3f62 100644
--- a/ee/app/models/requirements_management/requirement.rb
+++ b/ee/app/models/requirements_management/requirement.rb
@@ -24,7 +24,9 @@ class Requirement < ApplicationRecord
# but to sync the other way around, we require a temporary `dependent: :destroy`
# See https://gitlab.com/gitlab-org/gitlab/-/issues/323779 for details.
# This will be removed in https://gitlab.com/gitlab-org/gitlab/-/issues/329432
+ # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
belongs_to :requirement_issue, class_name: 'Issue', foreign_key: :issue_id, dependent: :destroy
+ # rubocop:enable Cop/ActiveRecordDependent -- legacy usage
validates :project, presence: true
validates :requirement_issue, presence: true, on: [:create, :update]
diff --git a/ee/app/models/search/zoekt/enabled_namespace.rb b/ee/app/models/search/zoekt/enabled_namespace.rb
index c600ef9f8f3e8..22ca80ab724db 100644
--- a/ee/app/models/search/zoekt/enabled_namespace.rb
+++ b/ee/app/models/search/zoekt/enabled_namespace.rb
@@ -12,10 +12,10 @@ class EnabledNamespace < ApplicationRecord
has_many :indices, class_name: '::Search::Zoekt::Index',
foreign_key: :zoekt_enabled_namespace_id, inverse_of: :zoekt_enabled_namespace,
- dependent: :nullify
+ dependent: :nullify # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
has_many :nodes, through: :indices
- has_many :replicas, dependent: :destroy,
+ has_many :replicas, dependent: :destroy, # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
foreign_key: :zoekt_enabled_namespace_id, inverse_of: :zoekt_enabled_namespace
validate :only_root_namespaces_can_be_indexed
diff --git a/ee/app/models/vulnerabilities/export.rb b/ee/app/models/vulnerabilities/export.rb
index ad831daac64ec..bf716258b7691 100644
--- a/ee/app/models/vulnerabilities/export.rb
+++ b/ee/app/models/vulnerabilities/export.rb
@@ -19,7 +19,7 @@ class Export < Gitlab::Database::SecApplicationRecord
belongs_to :organization, class_name: 'Organizations::Organization'
has_many :export_parts, class_name: 'Vulnerabilities::Export::Part', foreign_key: 'vulnerability_export_id',
- dependent: :destroy, inverse_of: :vulnerability_export
+ dependent: :destroy, inverse_of: :vulnerability_export # rubocop:disable Cop/ActiveRecordDependent -- legacy usage
mount_file_store_uploader AttachmentUploader
--
GitLab