[MIG][10.0] product_multi_image #201
Conversation
|
You can rebase now |
lasley
force-pushed
the
release/10.0/SMD-216-product_multi_image
branch
2 times, most recently
from
dd16b90
to
ed5884d
Compare
|
Rebased. I don't think it made any difference though? I'm still fixing the variant removal issue noted in #199 |
|
This is solved in OCA/server-tools#629 & should be ready for review after merge |
oca_dependencies.txt
Outdated
| @@ -0,0 +1 @@ | |||
| server-tools https://github.com/laslabs/server-tools release/10.0/base_multi_image | |||
lasley
force-pushed
the
release/10.0/SMD-216-product_multi_image
branch
from
bc8fef9
to
3af98db
Compare
|
Restarted build now that base fix is merged. Hopefully we go 🍏 ! |
lasley
force-pushed
the
release/10.0/SMD-216-product_multi_image
branch
from
3af98db
to
bb971d2
Compare
|
Oh crap this is still pending OCA/server-tools#677 - updating issue description |
* Upgrade product_multi_image to v9 * Add server-tools to oca_dependencies
lasley
force-pushed
the
release/10.0/SMD-216-product_multi_image
branch
2 times, most recently
from
1ab7984
to
871f110
Compare
|
Alright rebased now the OCA/server-tools#677 is merged. We should hopefully be 🍏 here now. @pedrobaeza @yajo - you guys mind if I squash your commits into changesets by author? |
|
No problem on my side. Still ❌ though. |
|
Yes, please squash, and check Travis status. |
lasley
force-pushed
the
release/10.0/SMD-216-product_multi_image
branch
from
b2be042
to
5e229c2
Compare
|
Ok so it seems the registry does not operate the same way that we expect it to in the hooks. I totally remember now why I went down the path of making an environment in my server-tools PR. It's wanting a I'm submitting a new PR for the base & linked this PR against it. We will use this one to determine that my fix actually worked. Don't merge this PR until I switch back 😉 Edit - the MetaModel linked to Travis, which overwrote when I rebuilt - sorry! |
lasley
force-pushed
the
release/10.0/SMD-216-product_multi_image
branch
from
5e229c2
to
a57258a
Compare
|
OK so now it's 💚, but... are we allowed to merge it like this? |
|
What are you referring to? |
oca_dependencies.txt
Outdated
| @@ -0,0 +1 @@ | |||
| server-tools https://github.com/laslabs/server-tools bugfix/10.0/base_multi_image-hook-revisited | |||
There was a problem hiding this comment.
@pedrobaeza I'm talking about this.
There was a problem hiding this comment.
Ah, OK. No, then we should wait for the merge on the server-tools repo.
There was a problem hiding this comment.
Yeah please don't merge yet. I'm just using this as proof that my patch in server-tools actually works this time. I didn't do it last time & now we're in this situation. Sorry, I should have been more clear in my comment.
lasley
force-pushed
the
release/10.0/SMD-216-product_multi_image
branch
from
a57258a
to
871f110
Compare
|
Alright at long last, these tests should pass! Well, except for the fact that they won't pass due to OCA/maintainer-quality-tools#427 😆 |
* 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
lasley
force-pushed
the
release/10.0/SMD-216-product_multi_image
branch
from
871f110
to
57958f5
Compare
|
Hrm no idea how tests are actually passing with the missing headers, but I'll take it! |
|
Without futher ado, I present a 🍏 build - using the production server-tools! Ready for review 🎸 Btw nobody rebuild Travis or it'll go 🔴 on unrelated issue in odoo/odoo. Delicate tip-toe, this passing CI is. |
|
Added |
|
@tedsalmon please review |
|
Checked on local environment and works nice! |
|
@lasley have you checked if in uninstallation product images are restored? |
|
@pedrobaeza @lasley Just I try to uninstall and it fail File "/opt/odoo/sources/odoo/odoo/modules/loading.py", line 389, in load_modules |
|
hehe, that why I was asking. It seems anyway something easy to fix. |
|
Hahahah this is the PR that just keeps on giving- I'll fix it up, thanks for the catch guys! |
|
@pedrobaeza @angelmoya - I added the import to the uninstall hook into the init file, which cleared up the uninstall issue in local. |
| def uninstall_hook(cr, registry): | ||
| """Remove multi images for models that no longer use them.""" | ||
| uninstall_hook_for_submodules(cr, registry, "product.template") | ||
| uninstall_hook_for_submodules(cr, registry, "product.product") |
There was a problem hiding this comment.
You still need to restore main image for products/templates here, before calling the base_multi_image hook, or you will lose them on uninstallation.
There was a problem hiding this comment.
Ah yes good call. Any idea how we would go about that with the DB modifications still in place? The fields in the product will still be overridden as related, so any changes made to those will simply update the underlying image record instead of the binary field.
There was a problem hiding this comment.
My best bet would be to create the column in SQL, and copy data to this column also in SQL. ORM wouldn't interfere in that, and when the field definition with the compute dissapears, Odoo will look again to this column for getting the image.
|
@pedrobaeza Since this was superseded by #359, can you close this? |
|
Yeah, closing. |
…irm website sale order (OCA#201) * [FIX] product_pack: Do not reset packs components when confirm sale order. If a sale order is updated from the website whrn confirm the order we do not expand the pack, we usit as it is. * [ADD] product_pack: Allow modify pack from backend/website Now we can choose if the product can be modified in the only int he backend or if can be modified also in the frontend by the customers. * [FIX] lint
This is WIP & served to rule out old API wire crossing as the 9.0 upgrade issue in #199. We're getting the same effect here, so that's not the case.
I'll fix this PR up once the issue is figured out in the 9.0 version.
Depends: