Implement product options processors #20
Conversation
| 'Option1 Name', | ||
| 'Option2 Name', | ||
| 'Option3 Name' | ||
| ).compact.reject { |t| t == 'Title' } |
There was a problem hiding this comment.
So we could have a title in options?
It would be nice a comment to explain or an explicit method to filter out that case (ex. reject_on_title)
| option_type = option_type(variant.product, i) | ||
|
|
||
| v = Spree::OptionValue.find_or_initialize_by( | ||
| option_type: option_type, |
There was a problem hiding this comment.
I would suggest using a different name for this local variable (option_type) to avoid misunderstandings with the method name.
| end | ||
|
|
||
| def option_type(product, index) | ||
| product.option_types.find_by(position: index + 1) |
There was a problem hiding this comment.
A way to skip the find_by here could be to attach the option types to the context in OptionTypes processor.
There was a problem hiding this comment.
I totally agree! 👍
There was a problem hiding this comment.
I amend previous comment: this solution doesn't fit since each processor works on a row, so we have no reference to the product's option types because they're not replicated on every row, but they're defined in the first row only.
MinasMazar
force-pushed
the
mm/product-options
branch
from
8aa7b12
to
3a65957
Compare
MinasMazar
force-pushed
the
mm/product-options
branch
from
3a65957
to
3951aa1
Compare
| def option_types? | ||
| # NOTE: according to https://help.shopify.com/en/manual/products/import-export | ||
| # when `Option Name1` is equal to 'Default`, means that product has no variants. | ||
| (@data['Option1 Name'] != 'Title') && option_type_names.any? |
There was a problem hiding this comment.
I think there's something not right here, the commit description and the comment refer to "Default", the code uses "Title" and the official docs refer to "Title" and "Default Title" 😅
https://help.shopify.com/en/manual/products/import-export/using-csv#product-csv-file-format
Shopify allows a maximum of three key,value options to define product attributes (I don't know if premium shop permits more options) We map these keys into into Spree::OptionTypes NOTE: according to https://help.shopify.com/en/manual/products/import-export when `Option Name1` is equal to `Title`, means that product has no variants.
Shopify allows a maximum of three key,value options to define product attributes (I don't know if premium shop permits more options) We map these values into into Spree::OptionValue NOTE: according to https://help.shopify.com/en/manual/products/import-export when `Option Value1` is equal to 'Default Title`, means that product has no variants.
MinasMazar
force-pushed
the
mm/product-options
branch
from
3951aa1
to
a622a69
Compare
Ref. #15
Shopify allows a maximum of three key, value options to define product
attributes (I don't know if premium shops permit more options)
We map these keys into into
Spree::OptionType/OptionValue