From 17c724b159ee021bb0686b57506f8b6f652eee2e Mon Sep 17 00:00:00 2001 From: Vitali Tatarintev <vtatarintev@gitlab.com> Date: Tue, 2 May 2023 13:36:16 +0200 Subject: [PATCH] Fix Layout/ArgumentAlignment offenses in models Fix Layout/ArgumentAlignment offenses in models --- .rubocop_todo/layout/argument_alignment.yml | 10 ---- .../loose_foreign_keys/deleted_record.rb | 34 ++++++------- app/models/merge_request.rb | 50 +++++++++++-------- app/models/merge_request_diff.rb | 10 ++-- app/models/merge_requests_closing_issues.rb | 9 ++-- app/models/ml/candidate_metadata.rb | 6 +-- app/models/ml/experiment_metadata.rb | 6 +-- app/models/namespace.rb | 16 +++--- .../namespaces/traversal/linear_scopes.rb | 8 +-- app/models/note.rb | 6 ++- app/models/note_diff_file.rb | 10 ++-- 11 files changed, 85 insertions(+), 80 deletions(-) diff --git a/.rubocop_todo/layout/argument_alignment.yml b/.rubocop_todo/layout/argument_alignment.yml index b73074fe8ec7..a710a8ea8711 100644 --- a/.rubocop_todo/layout/argument_alignment.yml +++ b/.rubocop_todo/layout/argument_alignment.yml @@ -529,16 +529,6 @@ Layout/ArgumentAlignment: - 'app/models/integrations/jira.rb' - 'app/models/jira_connect_installation.rb' - 'app/models/lfs_object.rb' - - 'app/models/loose_foreign_keys/deleted_record.rb' - - 'app/models/merge_request.rb' - - 'app/models/merge_request_diff.rb' - - 'app/models/merge_requests_closing_issues.rb' - - 'app/models/ml/candidate_metadata.rb' - - 'app/models/ml/experiment_metadata.rb' - - 'app/models/namespace.rb' - - 'app/models/namespaces/traversal/linear_scopes.rb' - - 'app/models/note.rb' - - 'app/models/note_diff_file.rb' - 'app/models/packages/cleanup/policy.rb' - 'app/models/packages/conan/metadatum.rb' - 'app/models/packages/debian/file_entry.rb' diff --git a/app/models/loose_foreign_keys/deleted_record.rb b/app/models/loose_foreign_keys/deleted_record.rb index f28e8f81b407..7f64606e97b9 100644 --- a/app/models/loose_foreign_keys/deleted_record.rb +++ b/app/models/loose_foreign_keys/deleted_record.rb @@ -9,23 +9,23 @@ class LooseForeignKeys::DeletedRecord < Gitlab::Database::SharedModel self.ignored_columns = %i[partition] partitioned_by :partition, strategy: :sliding_list, - next_partition_if: -> (active_partition) do - oldest_record_in_partition = LooseForeignKeys::DeletedRecord - .select(:id, :created_at) - .for_partition(active_partition.value) - .order(:id) - .limit(1) - .take - - oldest_record_in_partition.present? && - oldest_record_in_partition.created_at < PARTITION_DURATION.ago - end, - detach_partition_if: -> (partition) do - !LooseForeignKeys::DeletedRecord - .for_partition(partition.value) - .status_pending - .exists? - end + next_partition_if: -> (active_partition) do + oldest_record_in_partition = LooseForeignKeys::DeletedRecord + .select(:id, :created_at) + .for_partition(active_partition.value) + .order(:id) + .limit(1) + .take + + oldest_record_in_partition.present? && + oldest_record_in_partition.created_at < PARTITION_DURATION.ago + end, + detach_partition_if: -> (partition) do + !LooseForeignKeys::DeletedRecord + .for_partition(partition.value) + .status_pending + .exists? + end scope :for_table, -> (table) { where(fully_qualified_table_name: table) } scope :for_partition, -> (partition) { where(partition: partition) } diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index a67c6a47f59d..b9c3019bc2d7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -41,13 +41,13 @@ class MergeRequest < ApplicationRecord belongs_to :merge_user, class_name: "User" has_internal_id :iid, scope: :target_project, track_if: -> { !importing? }, - init: ->(mr, scope) do - if mr - mr.target_project&.merge_requests&.maximum(:iid) - elsif scope[:project] - where(target_project: scope[:project]).maximum(:iid) - end - end + init: ->(mr, scope) do + if mr + mr.target_project&.merge_requests&.maximum(:iid) + elsif scope[:project] + where(target_project: scope[:project]).maximum(:iid) + end + end has_many :merge_request_diffs, -> { regular }, inverse_of: :merge_request @@ -350,11 +350,12 @@ def public_merge_status end scope :references_project, -> { references(:target_project) } scope :with_api_entity_associations, -> { - preload_routables - .preload(:assignees, :author, :unresolved_notes, :labels, :milestone, - :timelogs, :latest_merge_request_diff, :reviewers, - target_project: :project_feature, - metrics: [:latest_closed_by, :merged_by]) + preload_routables.preload( + :assignees, :author, :unresolved_notes, :labels, :milestone, + :timelogs, :latest_merge_request_diff, :reviewers, + target_project: :project_feature, + metrics: [:latest_closed_by, :merged_by] + ) } scope :with_csv_entity_associations, -> { preload(:assignees, :approved_by_users, :author, :milestone, metrics: [:merged_by]) } @@ -397,8 +398,10 @@ def public_merge_status scope :preload_target_project, -> { preload(:target_project) } scope :preload_target_project_with_namespace, -> { preload(target_project: [:namespace]) } scope :preload_routables, -> do - preload(target_project: [:route, { namespace: :route }], - source_project: [:route, { namespace: :route }]) + preload( + target_project: [:route, { namespace: :route }], + source_project: [:route, { namespace: :route }] + ) end scope :preload_author, -> { preload(:author) } scope :preload_approved_by_users, -> { preload(:approved_by_users) } @@ -1019,8 +1022,7 @@ def validate_fork return true if target_project == source_project return true unless source_project_missing? - errors.add :validate_fork, - 'Source project is not a fork of the target project' + errors.add :validate_fork, 'Source project is not a fork of the target project' end def validate_reviewer_size_length @@ -1187,8 +1189,10 @@ def draft_title alias_method :wip_title, :draft_title def mergeable?(skip_ci_check: false, skip_discussions_check: false) - return false unless mergeable_state?(skip_ci_check: skip_ci_check, - skip_discussions_check: skip_discussions_check) + return false unless mergeable_state?( + skip_ci_check: skip_ci_check, + skip_discussions_check: skip_discussions_check + ) check_mergeability @@ -1209,10 +1213,12 @@ def mergeability_checks end def mergeable_state?(skip_ci_check: false, skip_discussions_check: false) - additional_checks = execute_merge_checks(params: { - skip_ci_check: skip_ci_check, - skip_discussions_check: skip_discussions_check - }) + additional_checks = execute_merge_checks( + params: { + skip_ci_check: skip_ci_check, + skip_discussions_check: skip_discussions_check + } + ) additional_checks.success? end diff --git a/app/models/merge_request_diff.rb b/app/models/merge_request_diff.rb index 1395b8ff162e..0e699d7a81d6 100644 --- a/app/models/merge_request_diff.rb +++ b/app/models/merge_request_diff.rb @@ -622,10 +622,12 @@ def convert_external_diffs_to_database end def diffs_in_batch_collection(batch_page, batch_size, diff_options:) - Gitlab::Diff::FileCollection::MergeRequestDiffBatch.new(self, - batch_page, - batch_size, - diff_options: diff_options) + Gitlab::Diff::FileCollection::MergeRequestDiffBatch.new( + self, + batch_page, + batch_size, + diff_options: diff_options + ) end def encode_in_base64?(diff_text) diff --git a/app/models/merge_requests_closing_issues.rb b/app/models/merge_requests_closing_issues.rb index 5c53cfd8c276..54cb6b7888b2 100644 --- a/app/models/merge_requests_closing_issues.rb +++ b/app/models/merge_requests_closing_issues.rb @@ -17,10 +17,11 @@ class MergeRequestsClosingIssues < ApplicationRecord scope :accessible_by, ->(user) do joins(:merge_request) .joins('INNER JOIN project_features ON merge_requests.target_project_id = project_features.project_id') - .where('project_features.merge_requests_access_level >= :access OR EXISTS(:authorizations)', - access: ProjectFeature::ENABLED, - authorizations: user.authorizations_for_projects(min_access_level: Gitlab::Access::REPORTER, related_project_column: "merge_requests.target_project_id") - ) + .where( + 'project_features.merge_requests_access_level >= :access OR EXISTS(:authorizations)', + access: ProjectFeature::ENABLED, + authorizations: user.authorizations_for_projects(min_access_level: Gitlab::Access::REPORTER, related_project_column: "merge_requests.target_project_id") + ) end class << self diff --git a/app/models/ml/candidate_metadata.rb b/app/models/ml/candidate_metadata.rb index 06b893c211ff..1191051b1a3d 100644 --- a/app/models/ml/candidate_metadata.rb +++ b/app/models/ml/candidate_metadata.rb @@ -4,9 +4,9 @@ module Ml class CandidateMetadata < ApplicationRecord validates :candidate, presence: true validates :name, - length: { maximum: 250 }, - presence: true, - uniqueness: { scope: :candidate, message: ->(candidate, _) { "'#{candidate.name}' already taken" } } + length: { maximum: 250 }, + presence: true, + uniqueness: { scope: :candidate, message: ->(candidate, _) { "'#{candidate.name}' already taken" } } validates :value, length: { maximum: 5000 }, presence: true belongs_to :candidate, class_name: 'Ml::Candidate' diff --git a/app/models/ml/experiment_metadata.rb b/app/models/ml/experiment_metadata.rb index 93496807e1ac..37cb27142681 100644 --- a/app/models/ml/experiment_metadata.rb +++ b/app/models/ml/experiment_metadata.rb @@ -4,9 +4,9 @@ module Ml class ExperimentMetadata < ApplicationRecord validates :experiment, presence: true validates :name, - length: { maximum: 250 }, - presence: true, - uniqueness: { scope: :experiment, message: ->(exp, _) { "'#{exp.name}' already taken" } } + length: { maximum: 250 }, + presence: true, + uniqueness: { scope: :experiment, message: ->(exp, _) { "'#{exp.name}' already taken" } } validates :value, length: { maximum: 5000 }, presence: true belongs_to :experiment, class_name: 'Ml::Experiment' diff --git a/app/models/namespace.rb b/app/models/namespace.rb index 86b5d7ea05f7..146ce2aa5b6f 100644 --- a/app/models/namespace.rb +++ b/app/models/namespace.rb @@ -124,18 +124,18 @@ class Namespace < ApplicationRecord delegate :name, to: :owner, allow_nil: true, prefix: true delegate :avatar_url, to: :owner, allow_nil: true delegate :prevent_sharing_groups_outside_hierarchy, :prevent_sharing_groups_outside_hierarchy=, - to: :namespace_settings, allow_nil: true + to: :namespace_settings, allow_nil: true delegate :show_diff_preview_in_email, :show_diff_preview_in_email?, :show_diff_preview_in_email=, - to: :namespace_settings + to: :namespace_settings delegate :runner_registration_enabled, :runner_registration_enabled?, :runner_registration_enabled=, - to: :namespace_settings + to: :namespace_settings delegate :allow_runner_registration_token, - :allow_runner_registration_token=, - to: :namespace_settings + :allow_runner_registration_token=, + to: :namespace_settings delegate :maven_package_requests_forwarding, - :pypi_package_requests_forwarding, - :npm_package_requests_forwarding, - to: :package_settings + :pypi_package_requests_forwarding, + :npm_package_requests_forwarding, + to: :package_settings before_save :update_new_emails_created_column, if: -> { emails_disabled_changed? } before_create :sync_share_with_group_lock_with_parent diff --git a/app/models/namespaces/traversal/linear_scopes.rb b/app/models/namespaces/traversal/linear_scopes.rb index 843de9bce339..792964a6c7f2 100644 --- a/app/models/namespaces/traversal/linear_scopes.rb +++ b/app/models/namespaces/traversal/linear_scopes.rb @@ -27,9 +27,11 @@ def roots def self_and_ancestors(include_self: true, upto: nil, hierarchy_order: nil) return super unless use_traversal_ids_for_ancestor_scopes? - self_and_ancestors_from_inner_join(include_self: include_self, - upto: upto, hierarchy_order: - hierarchy_order) + self_and_ancestors_from_inner_join( + include_self: include_self, + upto: upto, hierarchy_order: + hierarchy_order + ) end def self_and_ancestor_ids(include_self: true) diff --git a/app/models/note.rb b/app/models/note.rb index 597ba767a119..ac2b54629ae7 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -171,8 +171,10 @@ class Note < ApplicationRecord scope :with_associations, -> do # FYI noteable cannot be loaded for LegacyDiffNote for commits - includes(:author, :noteable, :updated_by, - project: [:project_members, :namespace, { group: [:group_members] }]) + includes( + :author, :noteable, :updated_by, + project: [:project_members, :namespace, { group: [:group_members] }] + ) end scope :with_metadata, -> { includes(:system_note_metadata) } diff --git a/app/models/note_diff_file.rb b/app/models/note_diff_file.rb index 4238de0a2f84..e4936de7b402 100644 --- a/app/models/note_diff_file.rb +++ b/app/models/note_diff_file.rb @@ -19,9 +19,11 @@ class NoteDiffFile < ApplicationRecord def raw_diff_file raw_diff = Gitlab::Git::Diff.new(to_hash) - Gitlab::Diff::File.new(raw_diff, - repository: project.repository, - diff_refs: original_position.diff_refs, - unique_identifier: id) + Gitlab::Diff::File.new( + raw_diff, + repository: project.repository, + diff_refs: original_position.diff_refs, + unique_identifier: id + ) end end -- GitLab