From 65fc144a4654330ef2abd9c28a14774b8b0a44a5 Mon Sep 17 00:00:00 2001 From: cyrillefr Date: Wed, 29 May 2024 10:32:21 +0200 Subject: [PATCH 1/3] Task to check missing foreing ids in spree_line_items --- ...sing_required_ids_in_spree_line_items.rake | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 lib/tasks/data/check_missing_required_ids_in_spree_line_items.rake diff --git a/lib/tasks/data/check_missing_required_ids_in_spree_line_items.rake b/lib/tasks/data/check_missing_required_ids_in_spree_line_items.rake new file mode 100644 index 0000000000..cb4b788b94 --- /dev/null +++ b/lib/tasks/data/check_missing_required_ids_in_spree_line_items.rake @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +namespace :ofn do + namespace :data do + desc 'Checking missing required ids in Spree::LineItem' + task check_missing_required_missing_ids_in_spree_line_items: :environment do + puts 'Checking for null order_id' + ids = Spree::LineItem.where(order_id: nil).pluck(:id) + + if ids.empty? + puts 'No NULL order_id found in spree_line_items' + else + puts 'NULL order_ids s have been found in spree_line_items:' + print ids + end + + puts 'Checking for null variant_id' + ids = Spree::LineItem.where(variant_id: nil).pluck(:id) + + if ids.empty? + puts 'No NULL variant_id found in spree_line_items' + else + puts 'NULL variant_id s have been found in spree_line_items:' + print ids + end + end + end +end From 4bb996e77fecbff66361d4132f3151e736660c49 Mon Sep 17 00:00:00 2001 From: cyrillefr Date: Wed, 29 May 2024 10:38:18 +0200 Subject: [PATCH 2/3] Fix RedundantPresenceValidationOnBelongs on some files (part VI) - presence: true is redundant since Rails 5.0 BUT applies with new default config of belongs_to_required_by_default to true. Lots of files with belongs_to_required_by_default = false (backward compatibility). So: deleting this setting implies to adding optional: true - added 'NOT NULL' constraints so model constraints match with contraints on DB tables. - corresponding migration files to match AR Models & DB tables - rake tasks to check corrupt data (ie: NULL/nil in id fields) (previous commit) - updated the todo --- .rubocop_todo.yml | 3 +-- app/models/spree/line_item.rb | 5 +---- ...20240529081209_require_order_and_variant_on_line_item.rb | 6 ++++++ db/schema.rb | 6 +++--- 4 files changed, 11 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20240529081209_require_order_and_variant_on_line_item.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index 8a6981e6b8..b99b52a9a8 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -642,11 +642,10 @@ Rails/RedundantActiveRecordAllMethod: - 'app/models/spree/variant.rb' - 'spec/system/admin/product_import_spec.rb' -# Offense count: 4 +# Offense count: 3 # This cop supports unsafe autocorrection (--autocorrect-all). Rails/RedundantPresenceValidationOnBelongsTo: Exclude: - - 'app/models/spree/line_item.rb' - 'app/models/spree/order.rb' # Offense count: 1 diff --git a/app/models/spree/line_item.rb b/app/models/spree/line_item.rb index 74d3acdc7e..eed6741b31 100644 --- a/app/models/spree/line_item.rb +++ b/app/models/spree/line_item.rb @@ -7,8 +7,6 @@ module Spree include VariantUnits::VariantAndLineItemNaming include LineItemStockChanges - self.belongs_to_required_by_default = false - searchable_attributes :price, :quantity, :order_id, :variant_id, :tax_category_id searchable_associations :order, :order_cycle, :variant, :product, :supplier, :tax_category searchable_scopes :with_tax, :without_tax @@ -19,7 +17,7 @@ module Spree belongs_to :variant, -> { with_deleted }, class_name: "Spree::Variant" has_one :product, through: :variant has_one :supplier, through: :product - belongs_to :tax_category, class_name: "Spree::TaxCategory" + belongs_to :tax_category, class_name: "Spree::TaxCategory", optional: true has_many :adjustments, as: :adjustable, dependent: :destroy @@ -28,7 +26,6 @@ module Spree before_validation :copy_tax_category before_validation :copy_dimensions - validates :variant, presence: true validates :quantity, numericality: { only_integer: true, greater_than: -1, diff --git a/db/migrate/20240529081209_require_order_and_variant_on_line_item.rb b/db/migrate/20240529081209_require_order_and_variant_on_line_item.rb new file mode 100644 index 0000000000..8d2920147a --- /dev/null +++ b/db/migrate/20240529081209_require_order_and_variant_on_line_item.rb @@ -0,0 +1,6 @@ +class RequireOrderAndVariantOnLineItem < ActiveRecord::Migration[7.0] + def change + change_column_null :spree_line_items, :order_id, false + change_column_null :spree_line_items, :variant_id, false + end +end diff --git a/db/schema.rb b/db/schema.rb index d279b1931b..70feb4b8e7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_05_17_121235) do +ActiveRecord::Schema[7.0].define(version: 2024_05_29_081209) do # These are extensions that must be enabled in order to support this database enable_extension "pg_stat_statements" enable_extension "plpgsql" @@ -541,8 +541,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_05_17_121235) do end create_table "spree_line_items", id: :serial, force: :cascade do |t| - t.integer "order_id" - t.integer "variant_id" + t.integer "order_id", null: false + t.integer "variant_id", null: false t.integer "quantity", null: false t.decimal "price", precision: 10, scale: 2, null: false t.datetime "created_at", precision: nil, null: false From b15e136980b6a98574d9343d8a5d551f6b4780b6 Mon Sep 17 00:00:00 2001 From: Maikel Linke Date: Fri, 31 May 2024 09:40:52 +1000 Subject: [PATCH 3/3] Remove obsolete rake task --- ...sing_required_ids_in_spree_line_items.rake | 28 ------------------- 1 file changed, 28 deletions(-) delete mode 100644 lib/tasks/data/check_missing_required_ids_in_spree_line_items.rake diff --git a/lib/tasks/data/check_missing_required_ids_in_spree_line_items.rake b/lib/tasks/data/check_missing_required_ids_in_spree_line_items.rake deleted file mode 100644 index cb4b788b94..0000000000 --- a/lib/tasks/data/check_missing_required_ids_in_spree_line_items.rake +++ /dev/null @@ -1,28 +0,0 @@ -# frozen_string_literal: true - -namespace :ofn do - namespace :data do - desc 'Checking missing required ids in Spree::LineItem' - task check_missing_required_missing_ids_in_spree_line_items: :environment do - puts 'Checking for null order_id' - ids = Spree::LineItem.where(order_id: nil).pluck(:id) - - if ids.empty? - puts 'No NULL order_id found in spree_line_items' - else - puts 'NULL order_ids s have been found in spree_line_items:' - print ids - end - - puts 'Checking for null variant_id' - ids = Spree::LineItem.where(variant_id: nil).pluck(:id) - - if ids.empty? - puts 'No NULL variant_id found in spree_line_items' - else - puts 'NULL variant_id s have been found in spree_line_items:' - print ids - end - end - end -end