From 850dfd776abdb07048881bc26b843275050b064d Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Mon, 1 Feb 2021 21:27:32 -0800 Subject: [PATCH 1/9] migrate variants with nil unit_value --- .../20210202052337_migrate_variant_unit_values.rb | 13 +++++++++++++ 1 file changed, 13 insertions(+) create mode 100644 db/migrate/20210202052337_migrate_variant_unit_values.rb diff --git a/db/migrate/20210202052337_migrate_variant_unit_values.rb b/db/migrate/20210202052337_migrate_variant_unit_values.rb new file mode 100644 index 00000000000..2a797bd04a7 --- /dev/null +++ b/db/migrate/20210202052337_migrate_variant_unit_values.rb @@ -0,0 +1,13 @@ +class MigrateVariantUnitValues < ActiveRecord::Migration + def up + Spree::Variant.all.select { |v| + v.unit_value.nil? && v.product&.variant_unit == "items" + }.each do |variant| + variant.unit_value = 1 + variant.save + end + end + + def down + end +end From 109a3da1048c4604ed7a773fa4e69169dcd0be6b Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Tue, 2 Feb 2021 09:32:47 -0800 Subject: [PATCH 2/9] use more efficient query; enforce not null at db level --- db/migrate/20210202052337_migrate_variant_unit_values.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/db/migrate/20210202052337_migrate_variant_unit_values.rb b/db/migrate/20210202052337_migrate_variant_unit_values.rb index 2a797bd04a7..862710f4bf4 100644 --- a/db/migrate/20210202052337_migrate_variant_unit_values.rb +++ b/db/migrate/20210202052337_migrate_variant_unit_values.rb @@ -1,11 +1,13 @@ class MigrateVariantUnitValues < ActiveRecord::Migration def up - Spree::Variant.all.select { |v| - v.unit_value.nil? && v.product&.variant_unit == "items" - }.each do |variant| + Spree::Variant.includes(:product).where( + spree_products: { variant_unit: "items" }, + spree_variants: { unit_value: nil } + ).each do |variant| variant.unit_value = 1 variant.save end + change_column_null :spree_variants, :unit_value, false, 1 end def down From 142af0055c8cdc46d0239ebedac2b0dd08cff08a Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Tue, 2 Feb 2021 09:40:17 -0800 Subject: [PATCH 3/9] use #find_each instead of #each --- db/migrate/20210202052337_migrate_variant_unit_values.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/db/migrate/20210202052337_migrate_variant_unit_values.rb b/db/migrate/20210202052337_migrate_variant_unit_values.rb index 862710f4bf4..cfd906c3cb2 100644 --- a/db/migrate/20210202052337_migrate_variant_unit_values.rb +++ b/db/migrate/20210202052337_migrate_variant_unit_values.rb @@ -3,7 +3,7 @@ def up Spree::Variant.includes(:product).where( spree_products: { variant_unit: "items" }, spree_variants: { unit_value: nil } - ).each do |variant| + ).find_each do |variant| variant.unit_value = 1 variant.save end From 50074aae9d04f67d2ab40196fc7d74edd2519a5b Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Wed, 3 Feb 2021 08:45:42 -0800 Subject: [PATCH 4/9] update db schema --- db/schema.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/db/schema.rb b/db/schema.rb index f3c76493eb8..82647c958b9 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20210127174120) do +ActiveRecord::Schema.define(version: 20210202052337) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1071,7 +1071,7 @@ t.decimal "cost_price", precision: 8, scale: 2 t.integer "position" t.string "cost_currency", limit: 255 - t.float "unit_value" + t.float "unit_value", null: false t.string "unit_description", limit: 255, default: "" t.string "display_name", limit: 255 t.string "display_as", limit: 255 From 91245ae6ab282cdb49359fa38c8c1d8657c3c850 Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 4 Feb 2021 08:05:48 -0800 Subject: [PATCH 5/9] also set weight to default to 0.0 --- db/migrate/20210202052337_migrate_variant_unit_values.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/db/migrate/20210202052337_migrate_variant_unit_values.rb b/db/migrate/20210202052337_migrate_variant_unit_values.rb index cfd906c3cb2..3c63a919bb6 100644 --- a/db/migrate/20210202052337_migrate_variant_unit_values.rb +++ b/db/migrate/20210202052337_migrate_variant_unit_values.rb @@ -8,6 +8,7 @@ def up variant.save end change_column_null :spree_variants, :unit_value, false, 1 + change_column_null :spree_variants, :weight, false, 0.0 end def down From d642984261c1a85f68c80fc698666ef28c6f534d Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 4 Feb 2021 11:24:35 -0800 Subject: [PATCH 6/9] add db constraint; check for existing nil/nan in migration --- .../20210202052337_migrate_variant_unit_values.rb | 15 ++++++++++++++- db/schema.rb | 2 +- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/db/migrate/20210202052337_migrate_variant_unit_values.rb b/db/migrate/20210202052337_migrate_variant_unit_values.rb index 3c63a919bb6..3dd5da48055 100644 --- a/db/migrate/20210202052337_migrate_variant_unit_values.rb +++ b/db/migrate/20210202052337_migrate_variant_unit_values.rb @@ -2,15 +2,28 @@ class MigrateVariantUnitValues < ActiveRecord::Migration def up Spree::Variant.includes(:product).where( spree_products: { variant_unit: "items" }, - spree_variants: { unit_value: nil } + spree_variants: { unit_value: [nil, Float::NAN] } ).find_each do |variant| variant.unit_value = 1 variant.save end + Spree::Variant.includes(:product).where( + spree_products: { variant_unit: "items" }, + spree_variants: { weight: [nil, Float::NAN] } + ).find_each do |variant| + variant.weight = 0 + variant.save + end change_column_null :spree_variants, :unit_value, false, 1 change_column_null :spree_variants, :weight, false, 0.0 + execute "ALTER TABLE spree_variants ADD CONSTRAINT check_unit_value_for_nan CHECK (unit_value <> 'NaN')" + execute "ALTER TABLE spree_variants ADD CONSTRAINT check_weight_for_nan CHECK (weight <> 'NaN')" end def down + change_column_null :spree_variants, :unit_value, true + change_column_null :spree_variants, :weight, true + execute "ALTER TABLE spree_variants DROP CONSTRAINT check_unit_value_for_nan" + execute "ALTER TABLE spree_variants DROP CONSTRAINT check_weight_for_nan" end end diff --git a/db/schema.rb b/db/schema.rb index 82647c958b9..9c4b067bd49 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1061,7 +1061,7 @@ create_table "spree_variants", force: :cascade do |t| t.string "sku", limit: 255, default: "", null: false - t.decimal "weight", precision: 8, scale: 2 + t.decimal "weight", precision: 8, scale: 2, null: false t.decimal "height", precision: 8, scale: 2 t.decimal "width", precision: 8, scale: 2 t.decimal "depth", precision: 8, scale: 2 From 212186e059a0cb7863b60a7871fcc286b577c78c Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 4 Feb 2021 11:45:36 -0800 Subject: [PATCH 7/9] check all variants, not just where we use 'items' --- .../20210202052337_migrate_variant_unit_values.rb | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/db/migrate/20210202052337_migrate_variant_unit_values.rb b/db/migrate/20210202052337_migrate_variant_unit_values.rb index 3dd5da48055..769a431ac48 100644 --- a/db/migrate/20210202052337_migrate_variant_unit_values.rb +++ b/db/migrate/20210202052337_migrate_variant_unit_values.rb @@ -1,16 +1,10 @@ class MigrateVariantUnitValues < ActiveRecord::Migration def up - Spree::Variant.includes(:product).where( - spree_products: { variant_unit: "items" }, - spree_variants: { unit_value: [nil, Float::NAN] } - ).find_each do |variant| + Spree::Variant.where(unit_value: [nil, Float::NAN]).find_each do |variant| variant.unit_value = 1 variant.save end - Spree::Variant.includes(:product).where( - spree_products: { variant_unit: "items" }, - spree_variants: { weight: [nil, Float::NAN] } - ).find_each do |variant| + Spree::Variant.where(weight: [nil, Float::NAN]).find_each do |variant| variant.weight = 0 variant.save end From 9c135ee0f772ba2f1e4456fa0f9d95d553ca2edc Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 4 Feb 2021 13:17:43 -0800 Subject: [PATCH 8/9] add default to unit_value and weight --- db/migrate/20210202052337_migrate_variant_unit_values.rb | 4 ++++ db/schema.rb | 4 ++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/db/migrate/20210202052337_migrate_variant_unit_values.rb b/db/migrate/20210202052337_migrate_variant_unit_values.rb index 769a431ac48..9c6cf46a2eb 100644 --- a/db/migrate/20210202052337_migrate_variant_unit_values.rb +++ b/db/migrate/20210202052337_migrate_variant_unit_values.rb @@ -10,6 +10,8 @@ def up end change_column_null :spree_variants, :unit_value, false, 1 change_column_null :spree_variants, :weight, false, 0.0 + change_column_default :spree_variants, :unit_value, 1 + change_column_default :spree_variants, :weight, 0.0 execute "ALTER TABLE spree_variants ADD CONSTRAINT check_unit_value_for_nan CHECK (unit_value <> 'NaN')" execute "ALTER TABLE spree_variants ADD CONSTRAINT check_weight_for_nan CHECK (weight <> 'NaN')" end @@ -17,6 +19,8 @@ def up def down change_column_null :spree_variants, :unit_value, true change_column_null :spree_variants, :weight, true + change_column_default :spree_variants, :unit_value, nil + change_column_default :spree_variants, :weight, nil execute "ALTER TABLE spree_variants DROP CONSTRAINT check_unit_value_for_nan" execute "ALTER TABLE spree_variants DROP CONSTRAINT check_weight_for_nan" end diff --git a/db/schema.rb b/db/schema.rb index 9c4b067bd49..05c3effff65 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -1061,7 +1061,7 @@ create_table "spree_variants", force: :cascade do |t| t.string "sku", limit: 255, default: "", null: false - t.decimal "weight", precision: 8, scale: 2, null: false + t.decimal "weight", precision: 8, scale: 2, default: 0.0, null: false t.decimal "height", precision: 8, scale: 2 t.decimal "width", precision: 8, scale: 2 t.decimal "depth", precision: 8, scale: 2 @@ -1071,7 +1071,7 @@ t.decimal "cost_price", precision: 8, scale: 2 t.integer "position" t.string "cost_currency", limit: 255 - t.float "unit_value", null: false + t.float "unit_value", default: 1.0, null: false t.string "unit_description", limit: 255, default: "" t.string "display_name", limit: 255 t.string "display_as", limit: 255 From f746dec537c5281d351ea9ec97f86c662c97735b Mon Sep 17 00:00:00 2001 From: Andy Brett Date: Thu, 4 Feb 2021 13:52:45 -0800 Subject: [PATCH 9/9] update spec --- spec/models/spree/variant_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/models/spree/variant_spec.rb b/spec/models/spree/variant_spec.rb index 5ef2d0637fb..cfd0d41d8bd 100644 --- a/spec/models/spree/variant_spec.rb +++ b/spec/models/spree/variant_spec.rb @@ -637,7 +637,7 @@ module Spree describe "setting the variant's weight from the unit value" do it "sets the variant's weight when unit is weight" do p = create(:simple_product, variant_unit: 'volume') - v = create(:variant, product: p, weight: nil) + v = create(:variant, product: p, weight: 0) p.update! variant_unit: 'weight', variant_unit_scale: 1 v.update! unit_value: 10, unit_description: 'foo'