Refactor variant images replacing polymorphic association

[softr8]

Spree::Image model has a polymorphic association which it looks like was added to be able to assign images either to products or variants, but right now, they're just associated to variants and the polymorphic association is no longer needed.

If we remove the polymorphic association, we can create another join table between variants and images, then, we could, potentially, associate one image to multiple variants, super helpful when having products sold by color and size, a single image with one specific color can be associated to multiple variants: White - S, White -M, White - L, White - XL

This can be challenging, specially for backward compatibility, but now that Solidus has the concept of gallery, it might be not that complex.

What do you think?

[stem]

I like the idea, but unfortunately, the Spree::Image is a Spree::Asset which might be associated to other type of object than the variants :/

[seand7565]

I really like the idea of either removing or improving the polymorphic relation of Assets.

@stem Spree::Asset only has one Paperclip::Attachment hooked in, and it contains styles and paths relevant pretty much just for variant images. Spree::Asset is polymorphic when it comes to attaching an ActiveRecord object to it, but the actual attachment is only useful as a product image.

IMO we should either figure out how to make the attachment polymorphic as well, or just remove the polymorphism. Plus I think @softr8 has a good point with attaching one image to multiple variants!

[softr8]

I found the original commit so the idea was to be able to re-use the asset in other relationships, like user has one avatar (which extends from asset).

So, we can keep that polymorphic idea but make Spree::Image a real model with its table, and make it to belong to an asset via polymorphic and has and belongs to many variants

🤔

[kennyadsl]

I would love to see a spike on this! Ideally, we can make this change by having both behaviors available under a preference, like Spree::Config.spree_image_inherits_from_asset which will be false on new stores and true on existing ones. At the next major release, we can remove the old behavior and open the gates to associating one image to multiple variants.

[stem]

@seand7565 I guess I was trying to say that even if Spree::Image is almost dedicated to a variant image, a Spree::Asset isn't. For instance, I know that we have a ShopImage < Spree::Asset to associate images to a Shop.
We also have a Report < Spree::Asset which saves a monthly PDF for our shops.
We might not be the only one doing things like that :)

The point to attach 1 image to multiple variant would be a HUGE improvement in my opinion, but it might be a bigger challenge than anticipated to keep backward compatibility.

@kennyadsl another path might be to move the relationship from the assets table to a viewable_assets table. That way, we can have a N-N between a "viewable" and an "asset", whatever they might be.
The fix would only be changing a has_many to a has_many trough:.

[softr8]

I'll give a try and send some draft for review/ideas