Make option value to variant association unique

[jarednorman]

Description

I can't think of a reason why someone would want to associate the same option value with a given variant multiple times. This makes the index on the variant/option_value_id columns of the spree_option_values_variants join table a unique index so we can't have a variant that has the same option value multiple times.

Checklist:

• [x] I have followed Pull Request guidelines
• [x] I have added a detailed description into each commit message
• [ ] I have updated Guides and README accordingly to this change (if needed)
• [x] I have added tests to cover this change (if needed)
• [ ] I have attached screenshots to this PR for visual changes (if needed)

[jarednorman]

I don't think any of the errors have anything to do with my change. 🤔

[jarednorman]

Had to scroll back more. I used 6.2 as the migration version, but currently we have to write migrations for the lowest supported ActiveRecord version.

[mamhoff]

This is not to hold this back: We associate the same option value with multiple variants. We have a third field, an integer, that differentiates the variants from one another. We are a special case though as we sell a lot of stuff in differently sized bags.

[jarednorman]

@mamhoff I don't think this will impact you then? You can still have multiple variants per option value, just not the the same option value on one variant multiple times.

[mamhoff]

Ah true! I mistakenly thought we need to have different option value combos for each variant. :+1:

[kennyadsl]

Hey @jarednorman, can you please rebase this one? Thanks! 🙏

[waiting-for-dev]

Hey @jarednorman, can you please rebase this one? Thanks! pray

I did it myself and fixed the conflict

[BenMorganIO]

This would be a good chance to mark the variant_id and the option_value_id as NOT NULL and add the two foreign key constraints. I would assume ON DELETE CASCADE ON UPDATE CASCADE would be appropriate here.

[kennyadsl]

This would be a good chance to mark the variant_id and the option_value_id as NOT NULL and add the two foreign key constraints. I would assume ON DELETE CASCADE ON UPDATE CASCADE would be appropriate here.

We decided to skip this kind of changes for this major release. There are already a bunch of database changes and we don't want to be a nightmare to update.

@BenMorganIO thanks a lot for the suggestion, I think we should design a process to make these changes safer. An idea is a script that runs at boot and checks the database (especially important in production) and gives some hints if you need to fix inconsistencies. BTW, we can discuss in Slack or in a separate issue/PR if you want.