Update product handles won't update existing images

[huoxito]

ps. opening this here because I'm not really sure how we could best address it.

Currently the ProductUpdateHandler will always create new images. So if a product with one image is updated say ten times from Wombat it will have 10 images.

I believe we need to figure some way to avoid creating those new images on every product update. Or at least give some kind of option / configuration to do so. Not sure paperclip supports that out of the box. I'm wondering whether we should add a url column on the database so we could map the images by the url and update / create based on that field.

[JDutil]

IIRC this only occurred if the query string wasn't being stripped from the url, and if the url was the same as an existing one then it wouldn't continue to create a new image. I started stripping out query strings here because I ran into that problem before with them: https://github.com/spree/spree_wombat/commit/ecddad9324c01b5d9e0fdd40607fe6a6149adf22

[jao]

I probably missed something, but it seems to me that the payload doesn't go though the serializer to prevent any images from being uploaded/created when a product is updated via the update_product endpoint. And the proccess_images method seems to only have active record shenanigans going on.

Also, it still happens when I update my products. (but I'm using 2-3-stable).

[huoxito]

hey guys I've put together this spec to exemplify the issue:

it "shouldnt create duplicated images" do
  message = Hub::Samples::Product.request
  message['product']['variants'][0].delete 'images'
  message['product']['images'][0]['url'] = 'https://avatars3.githubusercontent.com/u/56702'

  expect {
    handler = Handler::AddProductHandler.new message.to_json
    handler.process
  }.to change { Image.count }.by(1)

  # will a create a copy of the same image from same given url
  handler = Handler::UpdateProductHandler.new message.to_json
  handler.process

  expect {
    handler = Handler::UpdateProductHandler.new message.to_json
    handler.process
  }.not_to change { Image.count }.by(1)
end

Only way I can think of avoiding this is checking the url in advance and not call create there's an existing asset with the same url. I could be missing some feature on paperclip though. Also I'm not sure we should add a migration on this extension, could be too much overhead for users who don't need this.