solidusio-contrib / solidus_importer

Solidus importer extension to migrate data from other eCommerce systems
BSD 3-Clause "New" or "Revised" License
15 stars 30 forks source link

Don't add an option value if already added #75

Closed vdanciu closed 3 years ago

vdanciu commented 3 years ago

When using multiple images per variant we have to repeat the Options for that variant to identify the variant on each line of that variant where we specify a new variant image. What happens in this case is that an option is added multiple times (as many times as images for that variant). This PR checks if the option is already included.

It lacks a test for this but am not sure how to write one (described_method creates a new variant each time).

jarednorman commented 3 years ago

Could you be a little more specific about where you're having trouble testing this? I would write an example that adds one of the option values that would be otherwise added by the importer, then call described_method, then assert that you only have the option values you expect on the variant once each.

Also, if you're able we appreciate well written commit messages.

vdanciu commented 3 years ago

Could you be a little more specific about where you're having trouble testing this? I would write an example that adds one of the option values that would be otherwise added by the importer, then call described_method, then assert that you only have the option values you expect on the variant once each.

Also, if you're able we appreciate well written commit messages.

I was trying to write a test in option_values_spec.rb similar to the one that would not accept an empty value. I was trying to call described_method twice and somehow to pass the variant created by the first call to the second one but did not how to do that if possible. I will try your approach.

Wrt commit messages. I'll follow those guidelines.