Hidden seed data in migrations

[tudorpavel]

I was using rake db:reset to reset my database when I realized some default Solidus seed data is missing. After searching through the code, I noticed that the Solidus migrations contain some hidden seed data such as the default StockLocation.

The problem is that when running rake db:reset it is doing rake db:schema:load, which only recreates database tables based on the db/schema.rb file and the hidden seed data from migrations is never created.

I think the expected behavior should be that migrations only create the schema, while data such as the default StockLocation is extracted into the seeds.

[BravoSimone]

Working on it! 👍

[BravoSimone]

I've made a dump right after the migration, and after the reset.
I'm adding the "after migration dump" marked on what to me should be added and what not.

# NEED TO ADD
Spree::ShippingCategory.create!([
  {name: "Default"}
])

# NEED TO ADD
Spree::StockLocation.create!([
  {name: "default", default: false, address1: nil, address2: nil, city: nil, state_id: nil, state_name: nil, country_id: nil, zipcode: nil, phone: nil, active: true, backorderable_default: true, propagate_all_variants: true, admin_name: nil, position: 0, restock_inventory: true, fulfillable: true, code: nil, check_stock_on_transfer: true}
])

# THIS DIFFERS
Spree::Store.create!([
  {name: "Sample Store", url: "example.com", meta_description: nil, meta_keywords: nil, seo_title: nil, mail_from_address: "store@example.com", default_currency: nil, code: "spree", default: true, cart_tax_country_iso: nil}
])
# IN THIS WAY
# Spree::Store.create!([
#   {name: "Spree Demo Site", url: "demo.spreecommerce.com", meta_description: nil, meta_keywords: nil, seo_title: nil, mail_from_address: "spree@example.com", default_currency: nil, code: "spree", default: true, cart_tax_country_iso: "US"}
# ])

# GENERATED IN SEEDS TOO
Spree::ReturnReason.create!([
  {name: "Accidental order", active: true, mutable: true},
  {name: "Better price available", active: true, mutable: true},
  {name: "Damaged/Defective", active: true, mutable: true},
  {name: "Different from description", active: true, mutable: true},
  {name: "Different from what was ordered", active: true, mutable: true},
  {name: "Missed estimated delivery date", active: true, mutable: true},
  {name: "Missing parts or accessories", active: true, mutable: true},
  {name: "No longer needed/wanted", active: true, mutable: true},
  {name: "Unauthorized purchase", active: true, mutable: true}
])

# NEED TO ADD
Spree::RefundReason.create!([
  {name: "Return processing", active: true, mutable: false, code: nil}
])

# "original" IS ADDED IN MIGRATION
Spree::ReimbursementType.create!([
  {name: "original", active: true, mutable: true, type: "Spree::ReimbursementType::OriginalPayment"},
  {name: "Store Credit", active: true, mutable: true, type: "Spree::ReimbursementType::StoreCredit"}
])

# AFTER RESET "Gift Card" IS ADDED
Spree::StoreCreditCategory.create!([
  {name: "Default"}
])

# NOT NEEDED
Spree::StoreCreditType.create!([
  {name: "Expiring", priority: 1},
  {name: "Non-expiring", priority: 2}
])

# NOT NEEDED
Spree::StoreCreditUpdateReason.create!([
  {name: "Credit Given In Error"}
])

# NOT NEEDED
Spree::PaymentMethod.create!([
  {type: "Spree::PaymentMethod::StoreCredit", name: "Store Credit", description: "Store Credit", active: true, deleted_at: nil, auto_capture: nil, preferences: {}, preference_source: nil, position: 0, available_to_users: false, available_to_admin: false}
])

What do you think about it?

[tvdeyen]

Thanks for the work on this. This is very much appreciated.

I think we should actually put only the bare minimum of what's needed to get a store up and running into the seeds. And most importantly we should copy the seeds over to the seeds file of the host app, in order to be able to change things before seeding. Lots of stores may not using all of these return reasons for instance.

This means we should update the install task after we updated the seeds so it copies the seeds over instead of just running them, but this could be another PR.

I'm fine with having one PR that copies all seeds from the migrations into the seeds now and accept another one that changes the install task.