OCA / product-attribute

Odoo Product Attribute
https://odoo-community.org/project/product-maintainers-27
GNU Affero General Public License v3.0
185 stars 698 forks source link

`product_multi_image` broken if no image is on the template #228

Closed simahawk closed 7 years ago

simahawk commented 7 years ago

Context:

Issue: If you add an image to a variant:

  File "/opt/odoo/external-src/product-attribute/product_multi_image/models/product_product.py", line 44, in _inverse_main_image_medium
    product._inverse_main_image(product.image_main_medium)
  File "/opt/odoo/src/openerp/api.py", line 248, in wrapper
    return new_api(self, *args, **kwargs)
  File "/opt/odoo/external-src/product-attribute/product_multi_image/models/product_product.py", line 29, in _inverse_main_image
    product.image_ids[0].write({
  File "/opt/odoo/src/openerp/models.py", line 5756, in __getitem__
    return self._browse(self.env, (self._ids[key],))
IndexError: tuple index out of range

We miss a safe check here https://github.com/OCA/product-attribute/blob/9.0/product_multi_image/models/product_product.py#L28

pedrobaeza commented 7 years ago

Well, it's supposed that if you have an image, automatically gets the first image also on multi-images, so this is the part we have to check. Why don't you have this first image? Maybe some piece of code that makes it?

simahawk commented 7 years ago

hmm, I don't get this :)

It's pretty easy to replicate:

  1. create a product template w/ variants
  2. go to one of the variants
  3. add an image
  4. save => kabooom! :)

So, are we expecting to not be able to add images to variants IF the template has no image? If yes, we should make variant field readonly, otherwise we need to fix that method to write on image_ids only if there's something there or to add a new image if none.

pedrobaeza commented 7 years ago

Uhm, let me check then. The initial idea is:

In this logic, that was made for v8, I don't know if something has changed in the port to v9.

simahawk commented 7 years ago

That makes sense, the problem is that it's broken because it tries to write on product.image_ids[0] only because image is valued. Adding a new image is not taken into account at all.

simahawk commented 7 years ago

@pedrobaeza this is what I mean ;) https://github.com/OCA/product-attribute/pull/229/files

lasley commented 7 years ago

@simahawk - Can this be closed now?

simahawk commented 7 years ago

yup!