Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate variants with nil unit_value #6792

Merged
merged 10 commits into from
Feb 5, 2021
+31 −3
Merged

Migrate variants with nil unit_value #6792

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
850dfd7
migrate variants with nil unit_value
andrewpbrett Feb 2, 2021
109a3da
use more efficient query; enforce not null at db level
andrewpbrett Feb 2, 2021
142af00
use #find_each instead of #each
andrewpbrett Feb 2, 2021
50074aa
update db schema
andrewpbrett Feb 3, 2021
91245ae
also set weight to default to 0.0
andrewpbrett Feb 4, 2021
d642984
add db constraint; check for existing nil/nan in migration
andrewpbrett Feb 4, 2021
212186e
check all variants, not just where we use 'items'
andrewpbrett Feb 4, 2021
9c135ee
add default to unit_value and weight
andrewpbrett Feb 4, 2021
1e863c6
Merge branch 'master' into migrate-variant-unit-values
andrewpbrett Feb 4, 2021
f746dec
update spec
andrewpbrett Feb 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions db/migrate/20210202052337_migrate_variant_unit_values.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
class MigrateVariantUnitValues < ActiveRecord::Migration
def up
Spree::Variant.where(unit_value: [nil, Float::NAN]).find_each do |variant|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol I didn't know NAN existed

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know it exists in Rails, but I'm surprised Postgres allows saving it as a value on a decimal field in the DB...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you gotta love the 🐘 ❤️

variant.unit_value = 1
variant.save
andrewpbrett marked this conversation as resolved.
Show resolved Hide resolved
end
Spree::Variant.where(weight: [nil, Float::NAN]).find_each do |variant|
variant.weight = 0
variant.save
end
change_column_null :spree_variants, :unit_value, false, 1
andrewpbrett marked this conversation as resolved.
Show resolved Hide resolved
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')"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😍 finally we leverage PostgreSQL features

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
6 changes: 3 additions & 3 deletions db/schema.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down