From 06e217c52748e81f3cba24ddeebf8e9ccdfa5edb Mon Sep 17 00:00:00 2001 From: Neal Chambers Date: Thu, 17 Aug 2023 16:11:24 +0900 Subject: [PATCH] Safely autocorrect Rails/WhereNot Inspecting 1483 files ........................................................................................................................C..................................................................................................................C...........CC.C..........................................C......C..........C.........................C......................CC..........C........................................................................................................................C.......................................................................................................C........................................................C...........................................................................................................................................C......................................C......................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................... Offenses: app/controllers/spree/admin/products_controller.rb:183:11: C: [Corrected] Rails/WhereNot: Use where.not(spree_variants: { import_date: nil }) instead of manually constructing negated SQL in where. where('spree_variants.import_date IS NOT NULL'). ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/concerns/permalink_generator.rb:37:26: C: [Corrected] Rails/WhereNot: Use where.not(id: id) instead of manually constructing negated SQL in where. scope_with_deleted.where('id != ?', id) ^^^^^^^^^^^^^^^^^^^^ app/models/concerns/permalink_generator.rb:37:40: C: [Corrected] Style/HashSyntax: Omit the hash value. scope_with_deleted.where.not(id: id) ^^ app/models/enterprise.rb:152:7: C: [Corrected] Rails/WhereNot: Use where.not(enterprises: { id: ready_enterprises }) instead of manually constructing negated SQL in where. where("enterprises.id NOT IN (?)", ready_enterprises) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/enterprise.rb:158:31: C: [Corrected] Rails/WhereNot: Use where.not(sells: 'none') instead of manually constructing negated SQL in where. scope :is_distributor, -> { where('sells != ?', 'none') } ^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/enterprise.rb:479:17: C: [Corrected] Rails/WhereNot: Use where.not(id: id) instead of manually constructing negated SQL in where. dups = dups.where('id != ?', id) unless new_record? ^^^^^^^^^^^^^^^^^^^^ app/models/enterprise.rb:534:43: C: [Corrected] Rails/WhereNot: Use where.not(enterprises: { id: self }) instead of manually constructing negated SQL in where. enterprises = owner.owned_enterprises.where('enterprises.id != ?', self) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/enterprise.rb:583:7: C: [Corrected] Rails/WhereNot: Use where.not(enterprises: { id: id }) instead of manually constructing negated SQL in where. where('enterprises.id != ?', id). ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/enterprise_fee.rb:40:24: C: [Corrected] Rails/WhereNot: Use where.not(spree_calculators: { type: PER_ORDER_CALCULATORS }) instead of manually constructing negated SQL in where. joins(:calculator).where('spree_calculators.type NOT IN (?)', PER_ORDER_CALCULATORS) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/enterprise_relationship.rb:78:19: C: [Corrected] Rails/WhereNot: Use where.not(name: perms) instead of manually constructing negated SQL in where. permissions.where('name NOT IN (?)', perms).destroy_all ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/product_import/inventory_reset_strategy.rb:27:16: C: [Corrected] Rails/WhereNot: Use where.not(id: excluded_items_ids) instead of manually constructing negated SQL in where. relation.where('id NOT IN (?)', excluded_items_ids) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/proxy_order.rb:19:25: C: [Corrected] Rails/WhereNot: Use where.not(proxy_orders: { canceled_at: nil }) instead of manually constructing negated SQL in where. scope :canceled, -> { where('proxy_orders.canceled_at IS NOT NULL') } ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/spree/credit_card.rb:26:39: C: [Corrected] Rails/WhereNot: Use where.not(gateway_customer_profile_id: nil) instead of manually constructing negated SQL in where. scope :with_payment_profile, -> { where('gateway_customer_profile_id IS NOT NULL') } ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/spree/product.rb:166:9: C: [Corrected] Rails/WhereNot: Use where.not(order_cycles: { id: nil }) instead of manually constructing negated SQL in where. where('order_cycles.id IS NOT NULL') ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/spree/variant.rb:94:30: C: [Corrected] Rails/WhereNot: Use where.not(deleted_at: nil) instead of manually constructing negated SQL in where. scope :deleted, lambda { where('deleted_at IS NOT NULL') } ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/spree/variant.rb:165:43: C: [Corrected] Rails/WhereNot: Use where.not(spree_prices: { amount: nil }) instead of manually constructing negated SQL in where. where('spree_prices.amount IS NOT NULL'). ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/models/spree/zone.rb:141:19: C: [Corrected] Rails/WhereNot: Use where.not(id: id) instead of manually constructing negated SQL in where. Spree::Zone.where('id != ?', id).update_all(default_tax: false) if default_tax ^^^^^^^^^^^^^^^^^^^^ app/models/spree/zone.rb:141:33: C: [Corrected] Style/HashSyntax: Omit the hash value. Spree::Zone.where.not(id: id).update_all(default_tax: false) if default_tax ^^ app/models/variant_override.rb:32:7: C: [Corrected] Rails/WhereNot: Use where.not(variant_overrides: { import_date: nil }) instead of manually constructing negated SQL in where. where('variant_overrides.import_date IS NOT NULL'). ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ app/services/cap_quantity.rb:45:22: C: [Corrected] Rails/WhereNot: Use where.not(variant_id: available_variants_for.select(&:id)) instead of manually constructing negated SQL in where. order.line_items.where('variant_id NOT IN (?)', available_variants_for.select(&:id)) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb:32:18: C: [Corrected] Rails/WhereNot: Use where.not(spree_variants: { id: excluded_items_ids }) instead of manually constructing negated SQL in where. relation.where('spree_variants.id NOT IN (?)', excluded_items_ids) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb:78:18: C: [Corrected] Rails/WhereNot: Use where.not(order_cycle_id: order_cycle_ids) instead of manually constructing negated SQL in where. orphaned.where('order_cycle_id NOT IN (?)', order_cycle_ids) ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lib/reporting/reports/users_and_enterprises/base.rb:27:14: C: [Corrected] Rails/WhereNot: Use where.not(enterprises: { id: nil }) instead of manually constructing negated SQL in where. .where("enterprises.id IS NOT NULL") ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lib/reporting/reports/users_and_enterprises/base.rb:39:14: C: [Corrected] Rails/WhereNot: Use where.not(enterprise_id: nil) instead of manually constructing negated SQL in where. .where("enterprise_id IS NOT NULL") ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lib/reporting/reports/users_and_enterprises/base.rb:40:14: C: [Corrected] Rails/WhereNot: Use where.not(user_id: nil) instead of manually constructing negated SQL in where. .where("user_id IS NOT NULL") ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ lib/tasks/data/anonymize_data.rake:50:16: C: [Corrected] Rails/WhereNot: Use where.not(user_id: nil) instead of manually constructing negated SQL in where. Customer.where("user_id IS NOT NULL") ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ 1483 files inspected, 26 offenses detected, 26 offenses corrected --- .rubocop_todo.yml | 22 ------------------- .../spree/admin/products_controller.rb | 2 +- app/models/concerns/permalink_generator.rb | 2 +- app/models/enterprise.rb | 10 ++++----- app/models/enterprise_fee.rb | 2 +- app/models/enterprise_relationship.rb | 2 +- .../inventory_reset_strategy.rb | 2 +- app/models/proxy_order.rb | 2 +- app/models/spree/credit_card.rb | 2 +- app/models/spree/product.rb | 2 +- app/models/spree/variant.rb | 4 ++-- app/models/spree/zone.rb | 2 +- app/models/variant_override.rb | 2 +- app/services/cap_quantity.rb | 2 +- .../product_import/products_reset_strategy.rb | 2 +- .../subscriptions/proxy_order_syncer.rb | 2 +- lib/tasks/data/anonymize_data.rake | 2 +- 17 files changed, 21 insertions(+), 43 deletions(-) diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 2a88db9bfe..3db3e088d2 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -741,28 +741,6 @@ Rails/WhereExists: - 'lib/tasks/sample_data/order_cycle_factory.rb' - 'lib/tasks/sample_data/taxon_factory.rb' -# Offense count: 24 -# This cop supports safe autocorrection (--autocorrect). -Rails/WhereNot: - Exclude: - - 'app/controllers/spree/admin/products_controller.rb' - - 'app/models/concerns/permalink_generator.rb' - - 'app/models/enterprise.rb' - - 'app/models/enterprise_fee.rb' - - 'app/models/enterprise_relationship.rb' - - 'app/models/product_import/inventory_reset_strategy.rb' - - 'app/models/proxy_order.rb' - - 'app/models/spree/credit_card.rb' - - 'app/models/spree/product.rb' - - 'app/models/spree/variant.rb' - - 'app/models/spree/zone.rb' - - 'app/models/variant_override.rb' - - 'app/services/cap_quantity.rb' - - 'engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb' - - 'engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb' - - 'lib/reporting/reports/users_and_enterprises/base.rb' - - 'lib/tasks/data/anonymize_data.rake' - # Offense count: 1 Security/Open: Exclude: diff --git a/app/controllers/spree/admin/products_controller.rb b/app/controllers/spree/admin/products_controller.rb index 1e55d2a099..5373d040dd 100644 --- a/app/controllers/spree/admin/products_controller.rb +++ b/app/controllers/spree/admin/products_controller.rb @@ -180,7 +180,7 @@ module Spree select('DISTINCT spree_variants.import_date'). joins(:product). where('spree_products.supplier_id IN (?)', editable_enterprises.collect(&:id)). - where('spree_variants.import_date IS NOT NULL'). + where.not(spree_variants: { import_date: nil }). where(spree_variants: { deleted_at: nil }). order('spree_variants.import_date DESC') end diff --git a/app/models/concerns/permalink_generator.rb b/app/models/concerns/permalink_generator.rb index 240d4c7bb2..db51f91c8c 100644 --- a/app/models/concerns/permalink_generator.rb +++ b/app/models/concerns/permalink_generator.rb @@ -34,7 +34,7 @@ module PermalinkGenerator if id.nil? scope_with_deleted else - scope_with_deleted.where('id != ?', id) + scope_with_deleted.where.not(id:) end end diff --git a/app/models/enterprise.rb b/app/models/enterprise.rb index bced274bcf..f1efb62eb9 100644 --- a/app/models/enterprise.rb +++ b/app/models/enterprise.rb @@ -149,13 +149,13 @@ class Enterprise < ApplicationRecord select('DISTINCT enterprises.id') if ready_enterprises.any? - where("enterprises.id NOT IN (?)", ready_enterprises) + where.not(enterprises: { id: ready_enterprises }) else where(nil) end } scope :is_primary_producer, -> { where("enterprises.is_primary_producer IS TRUE") } - scope :is_distributor, -> { where('sells != ?', 'none') } + scope :is_distributor, -> { where.not(sells: 'none') } scope :is_hub, -> { where(sells: 'any') } scope :supplying_variant_in, lambda { |variants| joins(supplied_products: :variants). @@ -476,7 +476,7 @@ class Enterprise < ApplicationRecord def name_is_unique dups = Enterprise.where(name: name) - dups = dups.where('id != ?', id) unless new_record? + dups = dups.where.not(id: id) unless new_record? errors.add :name, I18n.t(:enterprise_name_error, email: dups.first.owner.email) if dups.any? end @@ -531,7 +531,7 @@ class Enterprise < ApplicationRecord # - it grants permissions to all pre-existing hubs # - all producers grant permission to it - enterprises = owner.owned_enterprises.where('enterprises.id != ?', self) + enterprises = owner.owned_enterprises.where.not(enterprises: { id: self }) # We grant permissions to all pre-existing hubs hub_permissions = [:add_to_order_cycle] @@ -580,7 +580,7 @@ class Enterprise < ApplicationRecord # We avoid an infinite loop and don't need to touch the whole distributor tree. def touch_distributors Enterprise.distributing_products(supplied_products.select(:id)). - where('enterprises.id != ?', id). + where.not(enterprises: { id: id }). update_all(updated_at: Time.zone.now) end end diff --git a/app/models/enterprise_fee.rb b/app/models/enterprise_fee.rb index fdbdb82a23..9c22754919 100644 --- a/app/models/enterprise_fee.rb +++ b/app/models/enterprise_fee.rb @@ -37,7 +37,7 @@ class EnterpriseFee < ApplicationRecord } scope :per_item, lambda { - joins(:calculator).where('spree_calculators.type NOT IN (?)', PER_ORDER_CALCULATORS) + joins(:calculator).where.not(spree_calculators: { type: PER_ORDER_CALCULATORS }) } scope :per_order, lambda { joins(:calculator).where('spree_calculators.type IN (?)', PER_ORDER_CALCULATORS) diff --git a/app/models/enterprise_relationship.rb b/app/models/enterprise_relationship.rb index 9093f00509..2efd7bee3e 100644 --- a/app/models/enterprise_relationship.rb +++ b/app/models/enterprise_relationship.rb @@ -75,7 +75,7 @@ class EnterpriseRelationship < ApplicationRecord if perms.nil? permissions.destroy_all else - permissions.where('name NOT IN (?)', perms).destroy_all + permissions.where.not(name: perms).destroy_all perms.map { |name| permissions.find_or_initialize_by name: name } end end diff --git a/app/models/product_import/inventory_reset_strategy.rb b/app/models/product_import/inventory_reset_strategy.rb index 826f0d032c..76ad7705aa 100644 --- a/app/models/product_import/inventory_reset_strategy.rb +++ b/app/models/product_import/inventory_reset_strategy.rb @@ -24,7 +24,7 @@ module ProductImport relation = VariantOverride.where(hub_id: enterprise_ids) return relation if excluded_items_ids.blank? - relation.where('id NOT IN (?)', excluded_items_ids) + relation.where.not(id: excluded_items_ids) end end end diff --git a/app/models/proxy_order.rb b/app/models/proxy_order.rb index 5908db5ade..449478b44e 100644 --- a/app/models/proxy_order.rb +++ b/app/models/proxy_order.rb @@ -16,7 +16,7 @@ class ProxyOrder < ApplicationRecord scope :active, -> { joins(:order_cycle).merge(OrderCycle.active) } scope :closed, -> { joins(:order_cycle).merge(OrderCycle.closed) } scope :not_closed, -> { joins(:order_cycle).merge(OrderCycle.not_closed) } - scope :canceled, -> { where('proxy_orders.canceled_at IS NOT NULL') } + scope :canceled, -> { where.not(proxy_orders: { canceled_at: nil }) } scope :not_canceled, -> { where('proxy_orders.canceled_at IS NULL') } scope :placed_and_open, -> { joins(:order).not_closed diff --git a/app/models/spree/credit_card.rb b/app/models/spree/credit_card.rb index 1d386bdac8..fa3f9ecc9e 100644 --- a/app/models/spree/credit_card.rb +++ b/app/models/spree/credit_card.rb @@ -23,7 +23,7 @@ module Spree after_create :ensure_single_default_card after_save :ensure_single_default_card, if: :default_card_needs_updating? - scope :with_payment_profile, -> { where('gateway_customer_profile_id IS NOT NULL') } + scope :with_payment_profile, -> { where.not(gateway_customer_profile_id: nil) } # needed for some of the ActiveMerchant gateways (eg. SagePay) alias_attribute :brand, :cc_type diff --git a/app/models/spree/product.rb b/app/models/spree/product.rb index 0378df7322..d9277c2c09 100755 --- a/app/models/spree/product.rb +++ b/app/models/spree/product.rb @@ -163,7 +163,7 @@ module Spree with_order_cycles_inner. merge(OrderCycle.active). merge(Exchange.outgoing). - where('order_cycles.id IS NOT NULL') + where.not(order_cycles: { id: nil }) } scope :by_producer, -> { joins(:supplier).order('enterprises.name') } diff --git a/app/models/spree/variant.rb b/app/models/spree/variant.rb index a130769de2..d8c52c1cbd 100644 --- a/app/models/spree/variant.rb +++ b/app/models/spree/variant.rb @@ -91,7 +91,7 @@ module Spree after_save :save_default_price # default variant scope only lists non-deleted variants - scope :deleted, lambda { where('deleted_at IS NOT NULL') } + scope :deleted, lambda { where.not(deleted_at: nil) } scope :with_order_cycles_inner, -> { joins(exchanges: :order_cycle) } @@ -162,7 +162,7 @@ module Spree where(deleted_at: nil). where('spree_prices.currency' => currency || Spree::Config[:currency]). - where('spree_prices.amount IS NOT NULL'). + where.not(spree_prices: { amount: nil }). select("spree_variants.id")) end diff --git a/app/models/spree/zone.rb b/app/models/spree/zone.rb index 4c413c163f..b310c2927f 100644 --- a/app/models/spree/zone.rb +++ b/app/models/spree/zone.rb @@ -138,7 +138,7 @@ module Spree end def remove_previous_default - Spree::Zone.where('id != ?', id).update_all(default_tax: false) if default_tax + Spree::Zone.where.not(id:).update_all(default_tax: false) if default_tax end end end diff --git a/app/models/variant_override.rb b/app/models/variant_override.rb index 40f9a16a1a..af0dbd92fb 100644 --- a/app/models/variant_override.rb +++ b/app/models/variant_override.rb @@ -29,7 +29,7 @@ class VariantOverride < ApplicationRecord scope :distinct_import_dates, lambda { select('DISTINCT variant_overrides.import_date'). - where('variant_overrides.import_date IS NOT NULL'). + where.not(variant_overrides: { import_date: nil }). order('import_date DESC') } diff --git a/app/services/cap_quantity.rb b/app/services/cap_quantity.rb index ca81bf098b..c1bfefaf5a 100644 --- a/app/services/cap_quantity.rb +++ b/app/services/cap_quantity.rb @@ -42,7 +42,7 @@ class CapQuantity end def unavailable_stock_lines_for - order.line_items.where('variant_id NOT IN (?)', available_variants_for.select(&:id)) + order.line_items.where.not(variant_id: available_variants_for.select(&:id)) end def available_variants_for diff --git a/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb b/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb index c2df5a76cc..ac7dac8de4 100644 --- a/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb +++ b/engines/catalog/app/services/catalog/product_import/products_reset_strategy.rb @@ -29,7 +29,7 @@ module Catalog return relation if excluded_items_ids.blank? - relation.where('spree_variants.id NOT IN (?)', excluded_items_ids) + relation.where.not(spree_variants: { id: excluded_items_ids }) end def reset_variants_on_hand_and_on_demand(variants) diff --git a/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb b/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb index 0ac31e25e6..bdb5737090 100644 --- a/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb +++ b/engines/order_management/app/services/order_management/subscriptions/proxy_order_syncer.rb @@ -75,7 +75,7 @@ module OrderManagement order_cycle_ids = in_range_order_cycles.pluck(:id) return orphaned unless order_cycle_ids.any? - orphaned.where('order_cycle_id NOT IN (?)', order_cycle_ids) + orphaned.where.not(order_cycle_id: order_cycle_ids) end def insert_values diff --git a/lib/tasks/data/anonymize_data.rake b/lib/tasks/data/anonymize_data.rake index 21ecc090ce..5a19e56be0 100644 --- a/lib/tasks/data/anonymize_data.rake +++ b/lib/tasks/data/anonymize_data.rake @@ -47,7 +47,7 @@ namespace :ofn do Customer.where("user_id IS NULL") .update_all("email = concat(id, '_ofn_customer@example.com'), name = concat('Customer Number ', id, ' (without connected User)')") - Customer.where("user_id IS NOT NULL") + Customer.where.not(user_id: nil) .update_all("email = concat(user_id, '_ofn_user@example.com'), name = concat('Customer Number ', id, ' - User ', user_id)")