-
-
Notifications
You must be signed in to change notification settings - Fork 689
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
12.0 mig product tags #434
Conversation
9dba4f8
to
ff8a46f
Compare
ff8a46f
to
c6b4e71
Compare
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.
Mostly suggestions
tag_ids = fields.Many2many( | ||
comodel_name='product.template.tag', string="Product Tags", | ||
relation='product_template_product_tag_rel', | ||
column1='product_tmpl_id', column2='tag_id') |
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.
Is there a reason for giving the relation
, column1
and column2
arguments? As I understand they will be automatically set.
_description = 'Product Tag' | ||
|
||
name = fields.Char(string="Name", required=True, translate=True) | ||
color = fields.Integer(string="Color Index") |
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 needs to be a constraint here since the colours that can be shown have specific numbers. Or maybe it could be a selection field?
product_tmpl_ids = fields.Many2many( | ||
comodel_name='product.template', string="Products", | ||
relation='product_template_product_tag_rel', | ||
column1='tag_id', column2='product_tmpl_id') |
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.
Same here, do you need the relation
, column1
, and column2
attributes to be set explicitly?
string="# of Products", compute='_compute_products_count', store=True) | ||
company_id = fields.Many2one( | ||
comodel_name='res.company', string="Company", | ||
default=lambda self: self._default_company()) |
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.
Maybe this could be default= lambda self: self.env.user.company_id
so that you could avoid defining a function.
@@ -0,0 +1,21 @@ | |||
from odoo.tests import common | |||
|
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.
Maybe here (and in some init files, you maybe want your copyright plus the licence comment (actually I think on the init files only the licence)
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.
And maybe some tests trying to create tags with users from different companies, and tag colour testing
<field name="arch" type="xml"> | ||
|
||
<field name="categ_id" position="after"> | ||
<field name="tag_ids"/> |
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.
What is this for?
<field name="company_id"/> | ||
<group expand="0"> | ||
<filter name="group_by_company" string="Company" | ||
domain="[]" context="{'group_by': 'company_id'}" |
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 think the domain can be omitted.
<field name="arch" type="xml"> | ||
<tree> | ||
<field name="name"/> | ||
<field name="company_id" widget="selection" groups="base.group_multi_company"/> |
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.
Do we need a widget selection
on a non editable list view?
<field name="name">product.template.tag.form (in product_template_tags)</field> | ||
<field name="model">product.template.tag</field> | ||
<field name="arch" type="xml"> | ||
<form> |
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.
As I understand it, we want the user to be able to set tags/colours for products per company. If that is true, then there should be the colour
field here, and the user should not be able to add their own tags/colours from the product form. Also, the rights should be different (group_user
should be able to only read, and possibly only some stock managers be able to create/write/unlink)
<field name="view_type">form</field> | ||
</record> | ||
|
||
<record model="ir.ui.menu" id="product_template_tag_config_menu"> |
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.
Maybe this would make more sense under Stock -> Products -> Configuration? But of course then you would need stock. Would it be a bad idea to set it as a dependency? Probably. Might as well keep the menu here, still it seems a little bit out of place.
Please close this PR because it is already migrated in #448 |
Closing the PR since it has already been migrated elsewhere. |
Migration of
product_template_tags
from 10.0 to 12.0