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.

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

Add hidden seeds from migrations #1962

Merged

Conversation

BravoSimone
Copy link
Contributor

As requested in issue #1502 I'm adding seeds for data previously created only in migrations.
I've added only what I thought would be necessary, see the comment for the complete list

@mamhoff
Copy link
Contributor

mamhoff commented May 23, 2017

When would these files be run?

@BravoSimone
Copy link
Contributor Author

seed.rb requires every file into db/default/ recursively so each time rake db:seed is used it will also load this data

Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

Great work and great report in your comment

A couple of things, you wrote:

# 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"}
])

why not changing this seed as well?

I'm just not sure how to set cart_tax_country_iso field. Migrations bootstraps data with that field nil and seeds with that fields to US, I'd keep both to nil for consistency but maybe it's better to wait for @mamhoff feedback on this.

Also, be sure to change url: "demo.spreecommerce.com" and other references to spree 😄

@@ -0,0 +1 @@
Spree::RefundReason.find_or_create_by(name: "Return processing")
Copy link
Member

Choose a reason for hiding this comment

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

I think this should be added here instead

@mamhoff
Copy link
Contributor

mamhoff commented Jun 3, 2017

The cart_tax_country_iso determines for which country taxes are shown in the cart. As stores that sell to the US do not need to show taxes in the cart, I think it's fine to leave this as nil.

@BravoSimone BravoSimone force-pushed the add-hidden-seeds-from-migrations branch from bdf9503 to 41ad1d8 Compare June 6, 2017 16:48
Copy link
Member

@kennyadsl kennyadsl left a comment

Choose a reason for hiding this comment

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

👍

@kennyadsl kennyadsl merged commit b6fbb37 into solidusio:master Jun 12, 2017
@BravoSimone BravoSimone deleted the add-hidden-seeds-from-migrations branch January 29, 2019 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants