Skip to content
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][ADD][partner_multi_image] possibility to have multiple images for a partner #287

Merged
merged 7 commits into from
Jun 1, 2017

Conversation

sergio-teruel
Copy link
Contributor

No description provided.

@rafaelbn rafaelbn added this to the 9.0 milestone Jul 1, 2016
Multiple Images in Partners
===========================

This module implements the possibility to have multiple images for a product

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably Partner?

@elicoidal
Copy link

Thanks for your contribution ! looks promising :)

#. Add a new image.
#. Refresh the page.
#. The first image in the collection is the main image for the product.
#. Go to *Sales > Products > Product Variants* and choose a product variant.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wrong path

@yajo
Copy link
Member

yajo commented Jul 21, 2016

Hi, I just added a test that should fail until OCA/server-tools#485 is merged in 42a87ee

image = fields.Binary(
related='image_main',
store=False,
multi=False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird that you don't get affected by odoo/odoo#10799. These fields are found in core Odoo, or they are new? If found in core, and they have multi there, you'll probably have to declare them with v7 style to avoid failures.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fields are in core but they haven't a multi parameter here. So @yajo can you retest and approve this pr?

Copy link
Member

@yajo yajo May 31, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In such case just 🔥 the multis

@pedrobaeza
Copy link
Member

@sergio-teruel, can you finish this one?

@rafaelbn rafaelbn requested a review from yajo May 31, 2017 09:28
@yajo
Copy link
Member

yajo commented May 31, 2017

I already had done a review; besides, bots are ❌

@cubells cubells force-pushed the 9.0-PR-partner_multi_image branch from de94316 to a349693 Compare May 31, 2017 11:09
@rafaelbn rafaelbn changed the title [WIP][9.0] partner_multi_image: Develop [9.0][ADD][partner_multi_image] possibility to have multiple images for a partner May 31, 2017
@rafaelbn rafaelbn requested a review from pedrobaeza May 31, 2017 11:13
@rafaelbn
Copy link
Member

@pedrobaeza @yajo changes made by @cubells please review thanks!

# © 2016 Sodexis
# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

from openerp.addons.base_multi_image.hooks import pre_init_hook_for_submodules
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are missing the uninstall hook.

Copy link
Sponsor Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@yajo done!

# License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).

from . import res_partner

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💩

image_medium = fields.Binary(
related='image_main_medium',
store=False,
multi=False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

image_small = fields.Binary(
related='image_main_small',
store=False,
multi=False
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

@cubells cubells force-pushed the 9.0-PR-partner_multi_image branch 2 times, most recently from ae42d84 to ec61aa5 Compare June 1, 2017 08:32


def uninstall_hook(cr, registry):
uninstall_hook_for_submodules(cr, registry, "res.partner")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you skip tests here (# pragma: no-cover) you will get to 100% coverage for the patch!

@cubells cubells force-pushed the 9.0-PR-partner_multi_image branch from 32c0dd3 to 795db52 Compare June 1, 2017 14:48
@yajo yajo self-assigned this Jun 1, 2017
@yajo
Copy link
Member

yajo commented Jun 1, 2017

Perfect! 😋 Need more reviews!

"author": "Tecnativa, "
"Odoo Community Association (OCA)",
"license": "AGPL-3",
"website": "http://www.tecnativa.com",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https

"license": "AGPL-3",
"website": "http://www.tecnativa.com",
"category": "Customer Relationship Management",
"pre_init_hook": "pre_init_hook",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uninstall_hook is not included here

<?xml version="1.0" encoding="UTF-8"?>
<!--
2009 Sharoon Thomas Open Labs
2014 Serv. Tecnol. Avanzados Pedro M. Baeza
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tecnativa here

@@ -0,0 +1,30 @@
# -*- coding: utf-8 -*-
# © 2009 Sharoon Thomas Open Labs Business Solutions
# © 2014 Serv. Tecnol. Avanzados Pedro M. Baeza
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tecnativa

@@ -0,0 +1,9 @@
# -*- coding: utf-8 -*-
# © 2009 Sharoon Thomas Open Labs Business Solutions
# © 2014 Serv. Tecnol. Avanzados Pedro M. Baeza
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tecnativa

@@ -0,0 +1,17 @@
# -*- coding: utf-8 -*-
# © 2009 Sharoon Thomas Open Labs Business Solutions
# © 2014 Serv. Tecnol. Avanzados Pedro M. Baeza
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tecnativa

@cubells cubells force-pushed the 9.0-PR-partner_multi_image branch from d8a4aff to 8249c0c Compare June 1, 2017 18:04
@pedrobaeza pedrobaeza merged commit f587c13 into OCA:9.0 Jun 1, 2017
@pedrobaeza pedrobaeza deleted the 9.0-PR-partner_multi_image branch June 1, 2017 19:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants