-
-
Notifications
You must be signed in to change notification settings - Fork 661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[9.0][WIP] Migrated product multi image #161
Conversation
5b73c3f
to
20bbaef
Compare
Known issues / Roadmap | ||
====================== | ||
|
||
* Provide proper migration scripts from module product_images from 7.0. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from 8.0?
Can you check travis? |
@yajo: I would like your help on the hook i.e. pre_init_hook(https://github.com/OCA/server-tools/blob/9.0/base_multi_image/hooks.py). For version 9.0, it seems the attachment=True parameter for fields.Binary() makes the field unavailable in the current table and is stored in ir_attachments as a new record. |
And what is the question or the problem? |
When we try to call the hook available in base_multi_image with pre_init_hook_for_submodules(cr, "product.template", "image"), it results in ProgrammingError: column "image" does not exist. So, I would like some suggestions on how this scenario could be handled. cc: @yajo |
Interesting, I did not notice the new attachment feature. It's possible that now base module needs some refactoring to use that storage by default too. |
Yeah, I also think that one is something to be taken into account in base_multi_image, not here. |
@dreispt Why close this PR? This PR uses the fix available in OCA/server-tools#468. Also, you might have confused this module(OCA/product-attribute/product_multi_image) with OCA/server-tools/base_multi_image. |
@atchuthan It was accidental, I don't what happened. Sorry! Reopened. |
okay @dreispt |
078713c
to
7694256
Compare
This module received a major update in v8 in #151, could you merge with your migration to get that please? |
Functional test in runbot fails:
If I attach the image from the images tab, then no exception occurs, but no image is attached neither. |
@yajo: already rebased commits from PR#151 to this PR On Tuesday 05 July 2016 08:18 PM, Yajo wrote:
Thanks & Regards, |
"name": "Multiple Images in Products", | ||
"version": "9.0.1.0.0", | ||
"author": "Serv. Tecnol. Avanzados - Pedro M. Baeza, " | ||
"Antiun Ingeniería, Tecnativa, Sodexis, " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@yajo may I know who I missed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah forget it, I saw 1 per line in v8 and did not realize you mixed some in the same line. My fault.
I cannot install the |
Please bear in mind that you probably have a bug that lets dangling images in the table when you uninstall this module. Please refer to https://github.com/OCA/server-tools/pull/485/files#diff-9ebf6925e40a87a77d2472a9134d8a2aR62 and implement that uninstall hook. I know, that's still in PR, but I'd appreciate help to get it fixed ASAP. Thanks. |
Hi @SodexisTeam - Are you going to continue with this PR? |
@lasley we were busy with some customer task, so I was unable to work on this. It seems @yajo raised an issue on odoo/odoo#13076 and it relates to the issue we are facing now. and we have to add the uninstall hook as suggested by @yajo. We would appreciate if you could submit your changes as PR to this branch. |
We are making it, so please wait for our PR. @yajo, @carlosdauden is the migration already available? |
Nothing yet AFAIK |
Thanks for the update guys. This just came up on some of our project scope, so I have some time that can be pushed this way. Sounds like we're on the right track, but let me know if my team can assist in anything. I'll go back to lurker status now 😉 |
@SodexisTeam, the problems you mentioned were in base_multi_image, so basically this module should work right now. Please check. |
image_small = fields.Binary( | ||
related='image_main_small', | ||
store=False, | ||
multi=False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm afraid these multi
probably don't have the desired effect.
Can you confirm please? Review odoo/odoo#10799 (comment)
3201a3f
to
401d507
Compare
Please check Travis. |
Hi, @SodexisTeam / @atchuthan, any news regarding this PR? |
I am superseding with #199, please close |
[MIG] account_invoice_supplier_ref_unique: Migration to 9.0
No description provided.