[19.0][ADD] Module partner_title #2188
Conversation
|
Feel free to propose a better icon than the "right hand" that I have used ! |
alexis-via
force-pushed
the
19-add-partner_title
branch
from
96ebf6c to
c394e0d
Compare
|
I changed to use the "user" icon, it's better 
|
alexis-via
force-pushed
the
19-add-partner_title
branch
2 times, most recently
from
a7994a6 to
d0fe8a5
Compare
| title_id = fields.Many2one( | ||
| "res.partner.title", | ||
| ondelete="restrict", | ||
| compute="_compute_title_id", | ||
| store=True, | ||
| precompute=True, | ||
| readonly=False, | ||
| ) |
There was a problem hiding this comment.
I think you created a compute field just to clear the title when the partner type is set to company.
I’d prefer to avoid adding multiple attributes and instead use a standard Many2one field, clearing the value through an onchange.
There was a problem hiding this comment.
We now try to avoid onchange as much as we can, especially in new code. That's what I'm doing here.
| precompute=True, | ||
| readonly=False, | ||
| ) | ||
| name_with_title = fields.Char(compute="_compute_name_with_title") |
There was a problem hiding this comment.
Do you have any specific purpose for using it somewhere?
Right now, it’s not even displayed in the view.
There was a problem hiding this comment.
it's very useful in reports.
There was a problem hiding this comment.
I added a comment about that to explain.
| <menuitem | ||
| id="res_partner_title_parent_menu" | ||
| parent="base.menu_custom" | ||
| name="Contact Titles" | ||
| sequence="500" | ||
| /> | ||
|
|
||
| <menuitem | ||
| id="res_partner_title_menu" | ||
| action="res_partner_title_action" | ||
| parent="res_partner_title_parent_menu" | ||
| sequence="10" | ||
| /> |
There was a problem hiding this comment.
Shouldn’t we add this menu under the Configuration section of the Contacts app, like in the previous versions, by adding contacts as a dependency?
There was a problem hiding this comment.
yep. See my PR #2189 that provides a glue module with contacts.
| <record id="res_partner_title_form" model="ir.ui.view"> | ||
| <field name="model">res.partner.title</field> | ||
| <field name="arch" type="xml"> | ||
| <form> | ||
| <sheet> | ||
| <widget | ||
| name="web_ribbon" | ||
| title="Archived" | ||
| bg_color="bg-danger" | ||
| invisible="active" | ||
| /> | ||
| <group name="main"> | ||
| <field name="name" /> | ||
| <field name="shortcut" /> | ||
| <field name="active" invisible="1" /> | ||
| </group> | ||
| </sheet> | ||
| </form> | ||
| </field> | ||
| </record> |
There was a problem hiding this comment.
Isn’t it enough to just have an editable tree view?
There was a problem hiding this comment.
I personally prefer modules that provide both a tree view and a form view.
partner_title/models/res_partner.py
Outdated
| @api.constrains("title_id", "is_company") | ||
| def _check_title(self): | ||
| for partner in self: | ||
| if partner.is_company and partner.title_id: | ||
| raise ValidationError( | ||
| self.env._( | ||
| "Partner '%(name)s' is a company, so it cannot have a title.", | ||
| name=partner.display_name, | ||
| ) | ||
| ) |
There was a problem hiding this comment.
I found an issue when creating a new record — if I change the partner type to Person, fill in the title, and save, it causes a problem.
There was a problem hiding this comment.
Arf !!! Looks like a 19.0 framework issue: when we enter the constraint code, neither is_company nor company_type has the proper value.
As this constraint is really not important, I removed it.
partner_title/models/res_partner.py
Outdated
| if partner.is_company: | ||
| partner.title_id = False | ||
|
|
||
| @api.depends("is_company", "title_id") |
There was a problem hiding this comment.
This will do nothing for a non-stored computed field.
| @api.depends("is_company", "title_id") |
There was a problem hiding this comment.
Are you really sure about this? I don't think so.
There was a problem hiding this comment.
Of course I’m sure. Why don’t you check and see?
There was a problem hiding this comment.
Here are some examples in odoo code where they use @api.depends on non-stored fields:
https://github.com/odoo/odoo/blob/19.0/odoo/addons/base/models/res_partner.py#L403
https://github.com/odoo/odoo/blob/19.0/addons/account/models/partner.py#L1009
https://github.com/odoo/odoo/blob/19.0/addons/account/models/partner.py#L998
But, of course, the code of Odoo may be wrong.
I thought that @api.depends on non-stored field was useful to invalidate the cache.
Of course, it is not as important as on stored fields where it is vital...
There was a problem hiding this comment.
I checked the behavior in detail and found that without the @api.depends decorator, the compute method is triggered when changing the name field, but the name value inside the method still shows the old value before the change, resulting in an incorrect name_with_title. It also does not trigger when other fields are changed. (Seems like strange)
With the @api.depends decorator, the name_with_title field updates instantly before saving when title is changed.
Since the field isn’t displayed in the view, having or not having the decorator doesn’t affect the final result — the correct name_with_title appears after saving in both cases. Therefore, we can remove the decorator. However, if you prefer to keep it, the name field should also be included in the @api.depends.
There was a problem hiding this comment.
The removal of @api.depends on _compute_name_with_title() broke the tests ! That's the best proof that @api.depends is useful on non-stored fields.
@AungKoKoLin1997 Please, don't say "Of course I’m sure. Why don’t you check and see?" when you're completely wrong. You made me loose my time.
There was a problem hiding this comment.
I restored @api.depends => tests are green again.
| def _compute_name_with_title(self): | ||
| for partner in self: | ||
| name = partner.name | ||
| if not partner.is_company and partner.title_id: | ||
| title = partner.title_id.shortcut or partner.title_id.name | ||
| name = " ".join([title, name]) | ||
| partner.name_with_title = name |
There was a problem hiding this comment.
| def _compute_name_with_title(self): | |
| for partner in self: | |
| name = partner.name | |
| if not partner.is_company and partner.title_id: | |
| title = partner.title_id.shortcut or partner.title_id.name | |
| name = " ".join([title, name]) | |
| partner.name_with_title = name | |
| def _compute_name_with_title(self): | |
| for partner in self: | |
| partner.name_with_title = partner.name | |
| if not partner.is_company and partner.title_id: | |
| title = partner.title_id.shortcut or partner.title_id.name | |
| partner.name_with_title = " ".join([title, partner.name_with_title]) |
There was a problem hiding this comment.
I prefer my implementation...
alexis-via
force-pushed
the
19-add-partner_title
branch
3 times, most recently
from
69a7f47 to
38f8aa8
Compare
alexis-via
force-pushed
the
19-add-partner_title
branch
from
38f8aa8 to
fe006a1
Compare
| class ResPartner(models.Model): | ||
| _inherit = "res.partner" | ||
|
|
||
| title_id = fields.Many2one( |
There was a problem hiding this comment.
I think the field name should be changed to title following 18.0, or a migration script needs to be added?
There was a problem hiding this comment.
A migration script will be needed anyway (for data), so let's put proper field names.
4 participants
res.partner.title was dropped in 19.0.
This module restores title on partners.
I added "active" and "sequence" on res.partner.title, so no more need for OCA modules partner_title_active (cf 17.0) and partner_title_order (cf 18.0)