Update product_option_types Seed File #4680
Conversation
Codecov Report
@@ Coverage Diff @@
## master #4680 +/- ##
=======================================
Coverage 86.24% 86.24%
=======================================
Files 574 574
Lines 14596 14596
=======================================
Hits 12588 12588
Misses 2008 2008 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
| "Solidus T-Shirt", "Solidus Long Sleeve", "Solidus Women's T-Shirt", | ||
| "Solidus Snapback Cap", "Solidus Hoodie Zip", "Ruby Hoodie", | ||
| "Ruby Hoodie Zip", "Ruby Polo" | ||
| ] |
There was a problem hiding this comment.
Do all of these products have size/color variants? If yes 👍, if no what's the benefit of adding the option types when there's only the master variant?
There was a problem hiding this comment.
So I'll try to express again the thought process behind these changes:
- I installed a fresh solidus app locally
- Noticed that Solidus Women's T-Shirt did not have any variants (size or colour)
- Went to admin panel to add the variants manually
- Couldn't find how to add a variant
After further investigation I understood that it was because the option types were not assigned to the product.
I just thought it would be a good idea to skip the step of adding the option type and be able to add the variant from the get go.
There was a problem hiding this comment.
The thing is that you add one or more option types to a product when that product has variants. Having an option type assigned but no variants might be considered an inconsistency because it means we are assuming that the product has some variants, but we didn't create them yet.
On a side note, in terms of consistency, with the clothing products we are creating in the demo, it's acceptable not to have the color option type, but maybe the size option type and its variants (eg. s/m/l) should always be present.
My proposal is to add the size option types and variants to all of them and keep the Solidus T-Shirt with both. WDYT?
There was a problem hiding this comment.
ok, will modify as suggested
There was a problem hiding this comment.
By the way, maybe it's not clear or easy to add a variant to a product without option types. If you have any suggestions on how to improve it, let me know!
There was a problem hiding this comment.
My proposal is to add the size option types and variants to all of them and keep the Solidus T-Shirt with both. WDYT?
If that's what we want to communicate, I think it'd be better to refactor instead of:
- Manually select clothing products and add size & color option types.
- Manually select the rest of the clothing products and add the size option type.
Do:
- Manually select clothing products and add color option type.
- Select all clothing products and add the size option type.
That way, we don't need to visually confirm that we do not forget any clothing product to add the size, plus we're safe if we add new items.
WDYT?
There was a problem hiding this comment.
Implemented you suggestion in 9eda486
There was a problem hiding this comment.
Thanks, @Naokimi! Left two suggestions, let me know what you think 🙂
| "Solidus T-Shirt", "Solidus Long Sleeve", "Solidus Women's T-Shirt", | ||
| "Solidus Snapback Cap", "Solidus Hoodie Zip", "Ruby Hoodie", | ||
| "Ruby Hoodie Zip", "Ruby Polo" | ||
| ] |
There was a problem hiding this comment.
My proposal is to add the size option types and variants to all of them and keep the Solidus T-Shirt with both. WDYT?
If that's what we want to communicate, I think it'd be better to refactor instead of:
- Manually select clothing products and add size & color option types.
- Manually select the rest of the clothing products and add the size option type.
Do:
- Manually select clothing products and add color option type.
- Select all clothing products and add the size option type.
That way, we don't need to visually confirm that we do not forget any clothing product to add the size, plus we're safe if we add new items.
WDYT?
| }, | ||
| { | ||
| product: solidus_snapback_cap, | ||
| option_values: [small], |
There was a problem hiding this comment.
Now that we're adding a bunch of new variants, it'd be suitable to leverage automation to create all the possible combinations. However, you're building on top of what we already had, so feel free to ignore this suggestion if you think it deserves a separate PR.
There was a problem hiding this comment.
would personally prefer to refactor it in a separate PR
It is unexpected that the loading of samples only provides size and colour options to Solidus T-Shirt. This commit creates `Spree::ProductOptionType`s for all clothing options and adds size variants for the clothes which were missing any. It also updates the names of the generated `Spree::OptionType`s to match the ones from demo.solidus.io.
waiting-for-dev
added
the
changelog:solidus_sample
Summary
When tinkering with a freshly created solidus app, I was surprised to see that Solidus Women's T-Shirt was missing an option to add variants.
Further investigation revealed that this due to Spree::ProductOptionType not being created when running seeds and that it impacts all clothing objects. I understand that this might have been done due to lack of images for the other variants, but one would expect to see the option to manually add the variants (and related images) through the admin panel.
With this commit all clothing options are assigned variants and the
New Variantbutton i made visible in the admin panel:It also brings the option types in line with the ones displayed in demo.solidus.io
Checklist
Check out our PR guidelines for more details.
The following are mandatory for all PRs:
The following are not always needed (
cross them outif they are not):I have added automated tests to cover my changes.I have opened a PR to update the guides.I have updated the readme to account for my changes.