[10.0] [MIG] product_multi_image (supersede) #359

FFernandez-PlanetaTIC · 2018-06-13T15:49:49Z

Hi,

Supersedes #201 and improves uninstall hook to move images from multi to single mode.

Requires of OCA/server-tools#1286 to do it.

Thanks!

FFernandez-PlanetaTIC · 2018-06-22T07:09:21Z

Hi,

This PR requires OCA/server-tools#1286 but the latter is pending reviewers.

Those interested can collaborate in the revision.

Thanks!

pedrobaeza · 2018-06-22T11:52:58Z

Now that the other module has been merged, please review this one and fix lint and CI errors

FFernandez-PlanetaTIC

846709c avoids resizing large image to small/medium when saved from single image. This fix is required to pass product_default_image tests.

FFernandez-PlanetaTIC

922f31a fixes error in product_default_image install hook when product_multi_image changes image field to computed and non-store

FFernandez-PlanetaTIC · 2018-07-13T08:50:18Z

@LRovira-PlanetaTIC Please, could you review this? Thanks

LRovira-PlanetaTIC

OK, it works correctly. Now, you can uninstall the module and the main image is saved as a product image.

pedrobaeza · 2018-07-21T21:57:26Z

product_multi_image/models/product_template.py

+    def _set_multi_image_main_medium(self):
+        # on save product module resizes large image to medium
+        # medium image should not overwrite the large
+        pass


I'm not sure about this inheritance break, as you are not calling super here in any case. Can you discriminate any way when to not call super? What is the use case for always inhibiting this method?

There was an issue saving an image as the main of a product, it was resized to small before saving:

This happens because:

product module resizes an image to all sizes before saving it.
https://github.com/OCA/OCB/blob/7a824c376ffe0ef6326c7c03a12e9105f9ce9f51/addons/product/models/product_template.py#L289
https://github.com/OCA/OCB/blob/7a824c376ffe0ef6326c7c03a12e9105f9ce9f51/odoo/tools/image.py#L266

product_multi_image redefines image fields for all sizes as related from main image.

base_multi_image saves main image for all sizes in the same field:
https://github.com/OCA/server-tools/blob/201129d9905f1312405bf4f3e08bf6e082312ac9/base_multi_image/models/owner.py#L19

Consequently the images for all sizes are saved in the same field and the last one, in this case the small one, wins 🐥🏆

As an image is always saved for all sizes and, to not lose quality, I always save it as the largest, I fix the issue not saving it for the rest of sizes.

I can not think of a better solution... 😕

Have studied this also. I think the solution is good because it's an exception for products only, necessary because products have already this built-in resizing on write. As base_multi_image mentions clearly, the _set_multi_image_main_* family of functions is meant only as a compatibility layer. Product IMO could be an exception to this rule - the images are still saved correctly and without loss.

pedrobaeza · 2018-07-21T21:57:51Z

I have squashed a bit the commit history before reviewing.

thomaspaulb · 2018-10-01T09:37:03Z

product_multi_image/tests/test_product_multi_image.py

+        )
+        self.assertFalse(len(images))
+
+    def test_uninstall_hook_teplate(self):


thomaspaulb · 2018-10-01T10:02:30Z

product_multi_image/models/product_template.py

+    def _set_multi_image_main_medium(self):
+        # on save product module resizes large image to medium
+        # medium image should not overwrite the large
+        pass


Have studied this also. I think the solution is good because it's an exception for products only, necessary because products have already this built-in resizing on write. As base_multi_image mentions clearly, the _set_multi_image_main_* family of functions is meant only as a compatibility layer. Product IMO could be an exception to this rule - the images are still saved correctly and without loss.

fuentes73

Please can you rebase and resolve conflicts?

…r icon

* Upgrade product_multi_image to v9 * Add server-tools to oca_dependencies

* Migrate product_multi_image to v10 * Remove old api from `product.product` * Add test coverage for hooks and variant count * Fix oca depends * Add uninstall hook

multi to single mode

when saved from single image

been changed to non-store

FFernandez-PlanetaTIC · 2019-04-11T10:42:57Z

@fuentes010 , done.

Regards! 👋

FFernandez-PlanetaTIC · 2019-07-10T07:00:22Z

Can we merge this?

nikul-serpentcs

Nitpicking

product_multi_image/__manifest__.py

OCA-git-bot · 2019-07-10T07:08:25Z

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

pedrobaeza · 2019-07-10T07:15:22Z

/ocabot merge

OCA-git-bot · 2019-07-10T07:15:34Z

Hey, thanks for contributing! Proceeding to merge this for you.
Rebased to 10.0-ocabot-merge-pr-359-by-pedrobaeza-bump-no, awaiting test results.

OCA-git-bot · 2019-07-10T08:21:00Z

Congratulations, your PR was merged at ab78d90. Thanks a lot for contributing to OCA. ❤️

PS: Don't worry if GitHub says there are unmerged commits: it is due to a rebase before merge. All commits of this PR have been merged into 10.0.

Signed-off-by pedrobaeza

FFernandez-PlanetaTIC mentioned this pull request Jun 13, 2018

[10.0] [IMP] base_multi_image: improve uninstall hook OCA/server-tools#1286

Merged

FFernandez-PlanetaTIC force-pushed the 10.0-MIG-product_multi_image branch 4 times, most recently from ac4f402 to 922f31a Compare July 5, 2018 11:28

FFernandez-PlanetaTIC commented Jul 5, 2018

View reviewed changes

FFernandez-PlanetaTIC mentioned this pull request Jul 13, 2018

[WIP] migrate connector v10 Tecnativa/connector-prestashop#6

Merged

LRovira-PlanetaTIC approved these changes Jul 17, 2018

View reviewed changes

FFernandez-PlanetaTIC mentioned this pull request Jul 20, 2018

[10.0] [MIG] [FIX] connector_prestashop: Fixes in v10.0 migration akretion/connector-prestashop#4

Merged

7 tasks

pedrobaeza force-pushed the 10.0-MIG-product_multi_image branch from 4e16434 to bb4dca2 Compare July 21, 2018 21:54

pedrobaeza reviewed Jul 21, 2018

View reviewed changes

FFernandez-PlanetaTIC mentioned this pull request Sep 4, 2018

[10.0] [MIG] [WIP] connector_prestashop: Migration to v 10.0 OCA/connector-prestashop#58

Merged

thomaspaulb mentioned this pull request Oct 1, 2018

[MIG][10.0] product_multi_image #201

Closed

5 tasks

pedrobaeza mentioned this pull request Oct 1, 2018

Migration to version 11.0 #293

Closed

39 tasks

thomaspaulb approved these changes Oct 1, 2018

View reviewed changes

fuentes73 approved these changes Apr 10, 2019

View reviewed changes

pedrobaeza and others added 9 commits April 11, 2019 11:26

product_multi_image: Module totally redone from old product_images

af98320

[IMP] product_multi_image: Split model in abstract & concrete + bette…

ffaee53

…r icon

Fix bugs found in review time, improve README.

cf13ecc

product_multi_image: Handle images in variants

f09d720

[MIG] product_multi_image: Upgrade to v9

d287a84

* Upgrade product_multi_image to v9 * Add server-tools to oca_dependencies

[MIG] product_multi_image: Upgrade to v10

def5ad1

* Migrate product_multi_image to v10 * Remove old api from `product.product` * Add test coverage for hooks and variant count * Fix oca depends * Add uninstall hook

[fix] prod image update broken w/ no image on template OCA#228

c8e36e1

[IMP] product_multi_image: Improve uninstall hook to move images from

7a46543

multi to single mode

[FIX] product_multi_image: Do not resize large image to small/medium

5f13957

when saved from single image

FFernandez-PlanetaTIC added 2 commits April 11, 2019 11:31

[FIX] product_default_image: Fix on install hook when image field has

4c83915

been changed to non-store

[FIX] product_multi_image: Fix image removal from variants

b068943

FFernandez-PlanetaTIC force-pushed the 10.0-MIG-product_multi_image branch from bb4dca2 to b068943 Compare April 11, 2019 09:33

nikul-serpentcs reviewed Jul 10, 2019

View reviewed changes

product_multi_image/__manifest__.py Outdated Show resolved Hide resolved

OCA-git-bot added approved ready to merge labels Jul 10, 2019

Fix module website to OCA

a09e770

OCA-git-bot added a commit that referenced this pull request Jul 10, 2019

Merge PR #359 into 10.0

ab78d90

Signed-off-by pedrobaeza

OCA-git-bot added the merged 🎉 label Jul 10, 2019

OCA-git-bot closed this Jul 10, 2019

FFernandez-PlanetaTIC deleted the 10.0-MIG-product_multi_image branch July 10, 2019 09:35

FFernandez-PlanetaTIC mentioned this pull request Jul 10, 2019

11.0 mig product multi image #358

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[10.0] [MIG] product_multi_image (supersede) #359

[10.0] [MIG] product_multi_image (supersede) #359

FFernandez-PlanetaTIC commented Jun 13, 2018

FFernandez-PlanetaTIC commented Jun 22, 2018

pedrobaeza commented Jun 22, 2018

FFernandez-PlanetaTIC left a comment •

edited

Loading

FFernandez-PlanetaTIC left a comment •

edited

Loading

FFernandez-PlanetaTIC commented Jul 13, 2018

LRovira-PlanetaTIC left a comment

pedrobaeza Jul 21, 2018

FFernandez-PlanetaTIC Jul 23, 2018

thomaspaulb Oct 1, 2018

pedrobaeza commented Jul 21, 2018

thomaspaulb Oct 1, 2018

thomaspaulb Oct 1, 2018

fuentes73 left a comment

FFernandez-PlanetaTIC commented Apr 11, 2019

FFernandez-PlanetaTIC commented Jul 10, 2019

nikul-serpentcs left a comment

OCA-git-bot commented Jul 10, 2019

pedrobaeza commented Jul 10, 2019

OCA-git-bot commented Jul 10, 2019

OCA-git-bot commented Jul 10, 2019

[10.0] [MIG] product_multi_image (supersede) #359

[10.0] [MIG] product_multi_image (supersede) #359

Conversation

FFernandez-PlanetaTIC commented Jun 13, 2018

FFernandez-PlanetaTIC commented Jun 22, 2018

pedrobaeza commented Jun 22, 2018

FFernandez-PlanetaTIC left a comment • edited Loading

Choose a reason for hiding this comment

FFernandez-PlanetaTIC left a comment • edited Loading

Choose a reason for hiding this comment

FFernandez-PlanetaTIC commented Jul 13, 2018

LRovira-PlanetaTIC left a comment

Choose a reason for hiding this comment

pedrobaeza Jul 21, 2018

Choose a reason for hiding this comment

FFernandez-PlanetaTIC Jul 23, 2018

Choose a reason for hiding this comment

thomaspaulb Oct 1, 2018

Choose a reason for hiding this comment

pedrobaeza commented Jul 21, 2018

thomaspaulb Oct 1, 2018

Choose a reason for hiding this comment

thomaspaulb Oct 1, 2018

Choose a reason for hiding this comment

fuentes73 left a comment

Choose a reason for hiding this comment

FFernandez-PlanetaTIC commented Apr 11, 2019

FFernandez-PlanetaTIC commented Jul 10, 2019

nikul-serpentcs left a comment

Choose a reason for hiding this comment

OCA-git-bot commented Jul 10, 2019

pedrobaeza commented Jul 10, 2019

OCA-git-bot commented Jul 10, 2019

OCA-git-bot commented Jul 10, 2019

FFernandez-PlanetaTIC left a comment •

edited

Loading

FFernandez-PlanetaTIC left a comment •

edited

Loading