Merge lp:~openerp-dev/openobject-server/7.0-fix-contact-company-handling into lp:openobject-server/7.0

Proposed by Olivier Dony (Odoo)
Status: Merged
Merged at revision: 4946
Proposed branch: lp:~openerp-dev/openobject-server/7.0-fix-contact-company-handling
Merge into: lp:openobject-server/7.0
Diff against target: 727 lines (+465/-95)
4 files modified
openerp/addons/base/res/res_partner.py (+209/-76)
openerp/addons/base/res/res_partner_view.xml (+45/-18)
openerp/addons/base/tests/test_base.py (+210/-0)
openerp/addons/base/tests/test_expression.py (+1/-1)
To merge this branch: bzr merge lp:~openerp-dev/openobject-server/7.0-fix-contact-company-handling
Reviewer Review Type Date Requested Status
Joël Grand-Guillaume @ camptocamp (community) test, no code review Needs Fixing
hbto [Vauxoo] http://www.vauxoo.com (community) Needs Fixing
Moisés López - http://www.vauxoo.com (community) Needs Fixing
Nhomar - Vauxoo (community) Needs Fixing
Stefan Rijnhart (Opener) (community) Disapprove
Yannick Vaucher @ Camptocamp (community) code, test Needs Fixing
OpenERP Core Team Pending
Review via email: mp+157577@code.launchpad.net

Description of the change

Fix autosync of commercial fields and address fields + correct views accordingly + basic tests

* Commercial fields (bug 1160365)

Fix autosync of accounting/invoicing-related fields on contacts, just as if they were actually modeled as fields.related to the parent commercial entity.
This starts with the addition of the new functional field `commercial_id`, to locate the commercial entity for any partner.
The commercial entity is defined as the first ancestor (starting at the partner itself) that is either marked `is_company` or has no parent. [This is Part A of the solution on bug 1160365].

Then, whenever a partner is created or modified:
- if it is a commercial entity, the commercial fields are synced downstream to all its descendants, stopping at company boundaries.
- if is is not a commercial entity, the commercial fields are synced from its parent commercial entity.
The list of fields to sync is determined by calling the new res.partner method `_commercial_fields()` which can be easily extended by any module that adds commercial fields on res.partner.
A utility method `open_commercial_entity()` was added to res.partner to make it easy to include a button for editing the parent commercial entity, to be displayed instead of now-hidden commercial fields. [This is part B of the solution on bug 1160365]

The res.partner.address_get() method (used to find child partners of certain types, e.g. "invoice") was udpated to take the new "commercial entity" notion into account: it will now look for matches in children but also parents and siblings that are part of the same "commercial entity". The default partner `type` is now "contact" to reflect the new model ; "default" is inappropriate because it is a wildcard and would stop the type lookup early. [This composes parts C and D of the solution on bug 1160365]

Note: This fix comes with a matching addons fix to implement module-specific extensions of part B, as well as part E of the solution: lp:~openerp-dev/openobject-addons/7.0-fix-contact-company-handling

* Address fields (bug 1160425)

Corrected autosync of address fields is also included in the same branch, because those two mechanisms are closely related and share some parts of the implementation.

The `use_parent_address` field now defaults to False (except in the mini-kanban view of contacts on a partner form), and the autosync of address fields has been modified to only work downstream and never update a parent company if a child contact is modified. Instead, the address fields are now displayed readonly on contacts that use the parent address, with a button to edit the parent address (new method open_parent(), similar to open_commercial_entity() but opening the direct parent).

To make the initial creation of a contact + company pair, a special case was added: when a new contact is created for a company that has no other contact and no address, the address of the contact is assumed to be that of the company so it is moved to the company, then the `use_parent_address` flag is enabled on the contact, and the `is_company` flag on the company. This covers a use case where contact and company are created on-the-fly when creating a new document.

Many logical flaws in the autosync of address fields have been corrected and are now covered by unit tests.

* Misc related fixes

- checking `is_company` does not reset the parent_id field anymore, to allow for multi-level structures. The field is still hidden automatically because having an empty "Company" field on the form view of a company is quite suprising), but this UI behavior is easily customized;
- the `email`, `phone`, `fax`, `mobile`, `lang`, etc. that were sometimes synced when changing parent company are now properly left alone;
- the `use_parent_address` field is now always visible next to the parent_id field when a parent is set

# UPDATE 2013-04-11:

- res.partner.address_get() now defaults to the partner being looked up rather than company when no match is found at all and no "default" exists. This avoids losing the contact info on invoices when a new contact+company pair is created.

- fix incorrect autosync of `type` field along with other address fields - a different `type` makes sense even when address is inherited because they can have different name/email/phones/etc.

- res.partner.name_search: important change in the way name_search() returns companies and contacts: make sure companies always come before contacts when both match, to make selection more predictable.

- the embedded contact creation mini-form was updated to include the address fields and the `type` field

Note: a series of important changes were done on the addons branch as well, see updated description on https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-fix-contact-company-handling/+merge/157576

# UPDATE 2013-04-20:

- Important change: res.partner.name_get now return "Company, Contact" rather than "Contact (Company)" to make it clearer that the company is selected as well.

- Another Import change: improved group_by issues on all models by adding a new stored function field `display_name` on res.partner, set as the default _order for res.partner. So search results for same company will always be grouped next to each other, and will match the display name (name_get) (see the addons MP, this is account_report_company module for 7.0)

- Added warning message when changing the Company of a Contact that already has a company, so it is clear that it should only be done if the Company was incorrect - in other cases a new contact must be created under the new company.

- Fixed search domains using "child_of" to also include deactived records, so that using this operator in the search view of business documents returns the expected results.

- renamed fields pointing to commercial entity to `commercial_partner_id`, so the fact that it is a FK to res.partner is clearer, and the same name can be used everywhere without confusion

- fixed propagation of "is_supplier" flag when creation a parent company for a contact created on the fly on a Purchase Order, and when adding contacts to an existing Supplier Company

To post a comment you must log in.
Revision history for this message
Guewen Baconnier @ Camptocamp (gbaconnier-c2c) wrote :

Hello,

Thanks for your proposal and your communication around it.

From what I see, it seems to address well most of the issues.

Did you took in account the Raphaël Valyi's concerns?
Raphaël, do you still see theses concerns? Can you give one proper example step-by-step based on this merge proposal please?

We saw a thing which seems to be a remaining bug:
The domain of the partner of an invoice restricts to use only commercial entities (companies), which is fine.
However, when you create a sale order, it does not prevent to use a contact as 'Invoice Address' so the invoice will be created with the contact instead of the company.

On a side note, do you plan to fix lp:1151947 regarding the addresses of customers (b2c) on the bounce while you are at it? :-)

[0] https://lists.launchpad.net/openerp-community/msg02519.html
[1] https://bugs.launchpad.net/openobject-server/+bug/1151947

Revision history for this message
Yannick Vaucher @ Camptocamp (yvaucher-c2c) wrote :

It looks great on adressing on invoicing issue and adresses synchronizing. Letting the parent_id will make things easier to have company affiliates

However there is still in my opinion one issue.

What about multi-level company + use_company_address ?

If you create a company and give it a parent company, the field use_company_address will be visible and used but only for one level.

So if you create the following hierarchy
Company A :
 is_company True
Company B :
 is_company True
 parent_id Company A;
 use_company_address True
Customer 1 :
  parent_id Company B;
  use_company_address True

If you do that, you will be able to edit only address on Company A, this is a good thing, however, it will only change Company A and Company B addresses. You cannot edit Customer 1 address

I see 2 options

A) forbidding partners with is_company='True' to have a use_company_address='True' plus hidding use_company_address field
I'm ok with this option as I do not see the case of contact having a contact as parent_id

B) the sync of address fields must be propagated downward recursively
This is already addressed for commercial field(s), so it could also be done for address fields

Otherwise, about the commercial fields, it seems it doesn't work properly

If you create a company with a vat number, and then you create a customer you link to this company, you will get a Constraint error "The vat of the partner must be unique !"

review: Needs Fixing (code, test)
Revision history for this message
Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote :

Hi,

First thanks for that work Olivier! My comments are here : https://bugs.launchpad.net/openobject-addons/+bug/1160365/comments/34

Regards,

Joël

4925. By Quentin (OpenERP) <email address hidden>

[IMP] base, res_partner and contacts management: improve usability and user experience.
1) whennever a contact is created from a parent record: allow to set a dedicated address
2) move the use_parent_address field near the address

Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

Hello Oliver.

Good Job, I know ypou are working so hard on it, and i trust we will have solved all issues.

Some comment regarding the extensiveness for localization.

In line :
19 +ADDRESS_FIELDS = ('street', 'street2', 'zip', 'city', 'state_id', 'country_id', 'type')

This variable will be able to change in what way?
It is a tupple, and it can not be used in multi-localization enviroments, I think a List is a best approach, in this case we can make in our localizations a simple ADDRESS_FIELDS.append("myfield").

A real use case is in our Mexican Localization where we need more field to comply with our electronic invoice.

Example here:

http://bazaar.launchpad.net/~openerp-mexico-maintainer/openerp-mexico-localization/6.1/view/head:/l10n_mx_partner_address/partner_address.py#L33

WE use them with access rules only in our multi-localization enviroments, with this variable you use, it will be a lot more complicated i think.

I will put comment by comment to avoid big mails with very big complicated/missing important points.

review: Needs Information
Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

Hello Again.

In the field,

+ 'commercial_id': fields.function(_commercial_id_compute, type='many2one', relation='res.partner', string='Commercial Entity')

shouldn't It be store = True?,

Several reports with SQL will depend of it, and we have a lot, all our fiscal reports are so complicated in Venezuela and Mexico, and we use a lot of SQL to compute them, IOH, it allow audit better the data in terms of go directly SQL access, and control external tools.

See here a good example:

http://bazaar.launchpad.net/~openerp-venezuela/openerp-venezuela-localization/trunk/view/head:/l10n_ve_fiscal_reports_V2/l10n_ve_fiscal_reports.py#L65

I think this important field not recorded is dangerous and a bad design, (this is one of thing a critizice a lot from other tools have business logic without trace in data base, remember this is a "Legal" element.

Thanks.

review: Needs Information
Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

Hello.

From menu Customer and Supplier, it is suposed you will be loading a Company (because the semantic says it is a Customer or a Supplier) is_company should be checked by default.

Regards.

review: Needs Information
Revision history for this message
Stefan Rijnhart (Opener) (stefan-opener) wrote :

We favour the approach of having a link between business documents and both company partner (partner_id) and contact partner (contact_id), and not having to copy the financial data from company partners to its contacts. See the comments by Raphaël Valyi and others on the bug discussion thread.

review: Disapprove
Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

Hello.

With this approach i can not solve the issue to invoice to different fiscal addresses for the same partner.

See the video:

http://youtu.be/mwWauRDrVzY

Thanks.

review: Needs Fixing
Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

About Sales.

Tray to enable the use different address stuff.

The wizard to invoice after deliver (what is incorrectly open when you deliver automagically but it is other history) create an invoice with partner id and not for Invoice Partner Id.

See the video.
http://youtu.be/eaeBGWtktuk

Regards.

review: Needs Fixing
Revision history for this message
Nhomar - Vauxoo (nhomar) wrote :

Regarding this topic:

>>>>>
* Misc related fixes

- checking `is_company` does not reset the parent_id field anymore, to allow for multi-level structures. The field is still hidden automatically because having an empty "Company" field on the form view of a company is quite suprising), but this UI behavior is easily customized;
>>>>>

IMHO this is correct, but we have an useability problem here. See the vide with explanation, but basically, onw time you set the parent_id <> False it shouldn't be invisible, it should be readonly I think.

I other case.

Understanding deeply your approach.

I think it is incomplete just that.

We have some options.

Res Partner is his own Own Invoice contact [As QuickBook Does]
Childs are contacts. [Simple Contact as we had in V6.1, and for all the extra features we have CRM may be.]

Childs are contact with address type (invoice) and is_company = True. [Subsidiaries Multi address feature we had in V6.1. with a better approach, we can not hide the parent_id if it is setted, because the concept of subsidiary will be this, and the _commercial_id_compute should consider that the parent give the commercial information (But the partner Id will be this one)]

Childs are Company itself. [Parent Child relate we had in V6.1]

I think if we improve this concepts we cover a bigger expectrum as V6.1 did.

IOH, I continue thinking we need the contact_id in documents mandatory, and just hide them for useability issue, the migration should not be difficult, if somebody is using as it is now, when install the module, you can copy the partner_id in the init method, it is easy, may be an extra (but I think it should be in the core of sales , purchase and so on.....

With this, one, we can mantain an onchange with this feature in all places and have recorded the data.

review: Needs Fixing
Revision history for this message
Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote :

Forbid to set it as "Need fixing", refer to this post for details:

https://bugs.launchpad.net/openobject-addons/+bug/1160365/comments/34

review: Needs Fixing (test, no code review)
Revision history for this message
Moisés López - http://www.vauxoo.com (moylop260) wrote :

Hello Oliver,

We have the same next bug:
https://bugs.launchpad.net/openobject-server/+bug/1122363

We created a new merge proposal with just this feature.

it should have any function that allows customize the fields to show by the function _display_address() even when this fields have been inherited.

review: Needs Fixing
Revision history for this message
hbto [Vauxoo] http://www.vauxoo.com (humbertoarocha) wrote :
Download full text (3.5 KiB)

Hello Community Guys

For my first review of this merge proposal I just have to say that this should address
recursion, there is risk of recursion if you try to update the res.partner
record from other media different from the UI (Web Browser), if you try
updating a res.partner record which have children and you try to set
one of its children as a parent_id the res.partner model won't complain about that.

To prove it just:

import oerplib
oerp = oerplib.OERP('localhost', protocol='xmlrpc', port=8069)
user = oerp.login('admin', 'admin', 'db_name')
rp_obj = oerp.get('res.partner')
rp_obj.search([('name','ilike','vauxoo')])
== > [27]

rp_obj.search([('parent_id','=',27)])
==> [63]

rp_brw = rp_obj.browse(27)
rp_brw.parent_id
==> False

rp_brw.parent_id= 63
oerp.write_record(rp_brw)
==> True (This is Wrong, it should not do that)

As you mention on the Technical Memento we should add something like this

And that should avoid recursion.

If you even try to create a larger hierarchical structure and then from
the UI you try to search from the backbone names, let's say the
partner records forming the loop, you will have an amazing result,
which is not True either,

=== modified file 'openerp/addons/base/res/res_partner.py'
--- openerp/addons/base/res/res_partner.py 2013-04-08 01:37:42 +0000
+++ openerp/addons/base/res/res_partner.py 2013-04-11 02:34:15 +0000
@@ -208,11 +208,12 @@
         return result

     _order = "name"
+ _parent_store = True
     _columns = {
         'name': fields.char('Name', size=128, required=True, select=True),
         'date': fields.date('Date', select=1),
         'title': fields.many2one('res.partner.title', 'Title'),
- 'parent_id': fields.many2one('res.partner', 'Related Company'),
+ 'parent_id': fields.many2one('res.partner', 'Related Company', ondelete='restrict'),
         'child_ids': fields.one2many('res.partner', 'parent_id', 'Contacts'),
         'ref': fields.char('Reference', size=64, select=1),
         'lang': fields.selection(_lang_get, 'Language',
@@ -278,7 +279,9 @@
         'color': fields.integer('Color Index'),
         'user_ids': fields.one2many('res.users', 'partner_id', 'Users'),
         'contact_address': fields.function(_address_display, type='char', string='Complete Address'),
-
+ # Just to avoid recursion ...

Read more...

review: Needs Fixing
Revision history for this message
hbto [Vauxoo] http://www.vauxoo.com (humbertoarocha) wrote :

Res.partner demo data lacks of TIN / VAT / RIF / RUC / FIN / A.K.A. Fiscal/Tax/Statuary Id Number, so that is triggering The check for uniqueness in TIN.

Could pliz check how did we managed at Openerp-Venezuela-Localization (OVL) to deal with this.

Uniqueness
http://bazaar.launchpad.net/~openerp-venezuela/openerp-venezuela-localization/trunk/view/head:/l10n_ve_fiscal_requirements/model/partner.py#L134

Mandatoriness
http://bazaar.launchpad.net/~openerp-venezuela/openerp-venezuela-localization/trunk/view/head:/l10n_ve_fiscal_requirements/model/partner.py#L170

BTW, if the merge proposal get a go ahead, we ought to refactory this methods, in order to comply
with the new contact-company-management approach, witch by now with this module is broken.

Regards.

4926. By Olivier Dony (Odoo)

[FIX] res.partner.address_get: default to partner being looked up rather than company when no match is found at all (and no "default" exists)

Using the commercial entity is not a very good default choice
in many cases. If a new contact is created on-the-fly for a
new document (e.g. sales order), his/her company may be an
empty shell and/or a large corporation that should not be
directly used for e.g. billing/invoicing purposed.
Once a contact/company is set to "invoicing" or "delivery"
we use it, but in other cases we stick to the provided
partner/contact as a saner default.
This should not change anything for cases where advanced
contact management is used and proper address types are
set.

4927. By Olivier Dony (Odoo)

[FIX] res.partner address sync: `type` field should not be synced wih other address fields

It is valid to use the parent address but still set a different
type of address - the name, email, phone, etc. could be different.

4928. By Olivier Dony (Odoo)

[FIX] res.partner.name_search: make sure companies always come before contacts when both match

4929. By Olivier Dony (Odoo)

[FIX] res.partner form view: remove now useless oe_inline class so `company` field can take 100% width of parent div

4930. By Olivier Dony (Odoo)

[FIX] res.partner form: switch default `use_parent_address` to False in kanban contact too, replace by defaults for address fields

We should never turn the `use_parent_address` option on
unless the user explicitly wants to use it. It makes
further editing more convoluted (need to go to the
company address) and is really an advanced feature.

It was only set by via context defaults when adding
contacts to a company ; we can replace that with a
set of defaults that will copy the company address
fields to the contact. Users can still switch to
automatic synchronization if they really want it,
or simply further edit the address of the contact.

Also fixed the layout of the mini-form view, now
using a simpler vertical layout with address at
bottom, since the main info of a contact needs
to be entered first.

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

On 2013-04-09, Nhomar wrote:
> Some comment regarding the extensiveness for localization.
>
> In line :
> 19 +ADDRESS_FIELDS = ('street', 'street2', 'zip', 'city', 'state_id',
> 'country_id', 'type')
>
> This variable will be able to change in what way?
> It is a tupple, and it can not be used in multi-localization enviroments, I
> think a List is a best approach, in this case we can make in our localizations
> a simple ADDRESS_FIELDS.append("myfield").

Good point! The idea for localizations is to instead inherit the _address_fields() method that was added in this merge proposal on the res.partner model. The ADDRESS_FIELDS is meant to be an implementation detail of res.partner, and I'd rather not change its type. I've just fixed _address_fields() so it returns a proper list, to make inheriting easier, and updated _display_address() to use it.
This is very similar to what Julio's merge proposal does (as mentioned by Moisés, thanks!), so I updated the corresponding bug report as well.

Thanks for spotting this!

4931. By Olivier Dony (Odoo)

[FIX] res.partner: make _address_fields method conform to its documented return type (detected by Nhomar Hernandez)

This will make overriddes more straightforward.

4932. By Olivier Dony (Odoo)

[FIX] res.partner: make the computed display_address use all address fields, including extra/custom ones

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

On 2013-04-09, Nhomar wrote:
> In the field,
>
> + 'commercial_id': fields.function(_commercial_id_compute,
> type='many2one', relation='res.partner', string='Commercial Entity')
>
> shouldn't It be store = True?,

Yes it will be stored in the next major version of OpenERP, but we don't think it is strictly necessary to store it in 7.0, and this avoids breaking compatibility for existing 7.0 databases.

> Several reports with SQL will depend of it, and we have a lot, all our fiscal
> reports are so complicated in Venezuela and Mexico, and we use a lot of SQL to
> compute them, IOH, it allow audit better the data in terms of go directly SQL
> access, and control external tools.

The "account_report_company" module that is added by the corresponding addons MP[1] will help to solve many cases where you need the extra foreign key in the database structure. After installing it you will find the "commercial entity" physical foreign key on all account.move.line and all account.invoice records.
Do you think that will cover all your use cases?

Technically it is even possible to compute the same "commercial entity" in pure SQL using PostgreSQL's CTE queries[2] (emulating temporary tables via WITH RECURSIVE), but there should be no need if you are mainly concerned about accounting data, the info is already there.

We also implemented the delegation of accounting data from contact to company in a way that is similar to "related fields", to make sure that the information is consistent in the database as well. If your localization needs to add extra accounting fields on res.partner, you simply have to inherit _commercial_fields() (similarly to _address_fields()) and these new fields will be correctly synchronized on contacts as well. This should help ensure data consistency and make direct SQL reporting or control possible.

> See here a good example:
>
> http://bazaar.launchpad.net/~openerp-venezuela/openerp-venezuela-localization/
> trunk/view/head:/l10n_ve_fiscal_reports_V2/l10n_ve_fiscal_reports.py#L65

Thanks for the pointer to a concrete example! You have two options for migrating this kind of report as far as I can see:

 a. As the report is not aggregated by "Company" or "Commercial Entity", you can simply keep it unchanged, knowing that the results will show partners as "John Doe (Coca-Cola Company)" instead of simply "Coca-Cola Company". Depending on the use case this may be a good improvement or a problem.
 b. Or if you need the report to always show partners as "Coca-Cola Company", you simply have to modify the join condition to rp."id" = ai."partner_commercial_id", after installing "account_report_company".

What do you think?

[1] https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-fix-contact-company-handling/+merge/157576
[2] http://www.postgresql.org/docs/9.2/static/queries-with.html

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

On 2013-04-11, hbto wrote:
> For my first review of this merge proposal I just have to say that this should
> address
> recursion, there is risk of recursion if you try to update the res.partner
> record from other media different from the UI (Web Browser), if you try
> updating a res.partner record which have children and you try to set
> one of its children as a parent_id the res.partner model won't complain about
> that.

Well spotted, thanks! I've added the appropriate _constraint in commit 4933.

You proposed solution of using parent_store to prevent this would have worked as well but it is normally meant to address a different issue. We can do it in trunk to improve performance of the child_of operator, but for 7.0 we should simply do a proper recursion_check, to avoid breaking compatibility with existing 7.0 databases when it's not strictly necessary.

4933. By Olivier Dony (Odoo)

[MERGE] Sync with 7.0

Revision history for this message
Olivier Dony (Odoo) (odo-openerp) wrote :

On 2013-04-11, hbto wrote:
> Res.partner demo data lacks of TIN / VAT / RIF / RUC / FIN / A.K.A.
> Fiscal/Tax/Statuary Id Number, so that is triggering The check for uniqueness
> in TIN.

You're right, but the check for TIN uniqueness is only done in l10n_ro in standard addons, as far as I can see.
It could make sense to move it to base_vat in trunk, but for 7.0 I'd prefer to fix it in l10n_ro only.

The semantics of the uniqueness of TIN numbers needs to change according to the new 7.0 partner model, where contacts have a series of pseudo-related fields towards their company. The unique check must therefore only apply to "commercial entity" partner, not to simple contacts. If all commercial entity partners have unique TIN numbers, their contacts will too, thanks to the auto-sync of these pseudo-related fields.

> Could pliz check how did we managed at Openerp-Venezuela-Localization (OVL) to
> deal with this.

I've had a look at the way you managed the uniqueness in the OVL, and I think we can use a simpler and pure SQL approach to solve this, thanks to PostgreSQL's partial indexes[1]. The idea is to use a UNIQUE INDEX on the TIN field, but make it apply only on records that are "commercial entities". This is very simple to express: a commercial entity either has the flag "is_company" set to True or has no parent:

  CREATE UNIQUE INDEX res_partner_vat_uniq_for_companies ON res_partner (vat) WHERE is_company OR parent_id IS NULL;

I've updated the addons merge proposal to include this patch for l10n_ro[2], and I think it could do the job for your localization as well.

[1] http://www.postgresql.org/docs/current/static/indexes-partial.html
[2] https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-fix-contact-company-handling/+merge/157576

4934. By Olivier Dony (Odoo)

[FIX] res.partner: propagate `is_supplier` property when creating parent or child partner for a supplier

This is not necessary for the `is_customer` flag, as it is
always True by default (customers are created more frequently).
Passing the field value via an invisible field in the mini
contact form is necessary because context `default_*` values are
automatically discarded by the ORM when creating o2m
or m2m records (as they are supposed to apply to a different model).

4935. By Olivier Dony (Odoo)

[FIX] res.partner: name_get: return "Company, Contact" rather than "Contact (Company)" to make it clearer that the company is selected as well

4936. By Olivier Dony (Odoo)

[FIX] res.partner: avoid hiding `parent_id` and `child_ids` fields if they are set, irregardless of the rest of the `is_company` flag

This prevents hiding real data and also allows creating more
complex/flexible structures by setting the values of these
fields before or after setting is_company, to reach the
desired result.

Revision history for this message
Joël Grand-Guillaume @ camptocamp (jgrandguillaume-c2c) wrote :
review: Needs Fixing (test, no code review)
4937. By Olivier Dony (Odoo)

[IMP] res.partner: rename commercial_id to commercial_partner_id + make function field inheritable

The name `commercial_partner_id` better reflects its
purpose and the fact that it is a FK to a partner.

An extra indirection through a lambda function was
also added to the definition of the function field
to make it possible to override it in other modules
(otherwise the function is passed by copy directly
and cannot be inherited later)

4938. By Olivier Dony (Odoo)

[FIX] res.partner: add onchange warning when changing an existing contact's company

4939. By Olivier Dony (Odoo)

[FIX] res.partner: search using 'child_of' should include inactive children

This is necessary for 2 reasons:
- when searching on Business documents the search domain will be
  [('partner_id', 'child_of', 'ACME')] in order to match all
  descendants, and it must match inactive children as well
- in other cases like for resolving IDs to update via store
  triggers, it is necessary that 'child_of' returns inactive
  children too.

The implementation is tricky because the ORM automatically
transform 'child_of' domains into recursive searches with
[('parent_id', 'in', ids)], which is the same query that the
reverse one2many 'child_ids' will also use to find contacts.
The overridden search() therefore matches this domain pattern
only when there is one criterion (to avoid side-effects in
other cases) and a dummy extra 'domain' was added to the
definition of the 'child_ids' o2m so it won't match.
The net result is that child_ids will not return inactive
children while child_of will return all descendants when
it is the only criterion. This is the expected behavior
whenever child_of is used on res.partner, because
it's safer to always show business documents.
The only side-effects will be for custom/manual search
calls with a single criterion of the form ('parent_id','in', x)
and those can be fixed by adding an extra domain
component ('active','=',True), just like child_ids does.

4940. By Olivier Dony (Odoo)

[REVERT] res.partner: undo change of ordering in name_search result, to be addressed via a stored display_name field (temporarily added by account_report_company module in 7.0)

4941. By Olivier Dony (Odoo)

[FIX] res.partner.name_get: partners marked as "is_company" should appear as standalone

This is more consistent with the way we expect reporting
to work, and will also ensure that these companies
appear right above their contacts in search order
(which will match name_get)

4942. By Olivier Dony (Odoo)

[FIX] test_expression: assertion was too sensitive to exact parameters

Now that res.partner.child_ids has an extra domain
attribute the exact number of parameters of queries
using `child_ids` in the WHERE clause is different.

4943. By Olivier Dony (Odoo)

[FIX] res.partner: fix invisible attrs on Contacts tab, empty o2m field results in `[]`, not `False`

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'openerp/addons/base/res/res_partner.py'
--- openerp/addons/base/res/res_partner.py 2013-04-15 17:17:32 +0000
+++ openerp/addons/base/res/res_partner.py 2013-04-20 00:50:34 +0000
@@ -30,6 +30,7 @@
30from openerp import pooler, tools30from openerp import pooler, tools
31from openerp.osv import osv, fields31from openerp.osv import osv, fields
32from openerp.tools.translate import _32from openerp.tools.translate import _
33from openerp.tools.yaml_import import is_comment
3334
34class format_address(object):35class format_address(object):
35 def fields_view_get_address(self, cr, uid, arch, context={}):36 def fields_view_get_address(self, cr, uid, arch, context={}):
@@ -159,8 +160,9 @@
159 res = lang_pool.read(cr, uid, ids, ['code', 'name'], context)160 res = lang_pool.read(cr, uid, ids, ['code', 'name'], context)
160 return [(r['code'], r['name']) for r in res]161 return [(r['code'], r['name']) for r in res]
161162
162POSTAL_ADDRESS_FIELDS = ('street', 'street2', 'zip', 'city', 'state_id', 'country_id')163# fields copy if 'use_parent_address' is checked
163ADDRESS_FIELDS = POSTAL_ADDRESS_FIELDS + ('email', 'phone', 'fax', 'mobile', 'website', 'ref', 'lang')164ADDRESS_FIELDS = ('street', 'street2', 'zip', 'city', 'state_id', 'country_id')
165POSTAL_ADDRESS_FIELDS = ADDRESS_FIELDS # deprecated, to remove after 7.0
164166
165class res_partner(osv.osv, format_address):167class res_partner(osv.osv, format_address):
166 _description = 'Partner'168 _description = 'Partner'
@@ -193,13 +195,28 @@
193 result[obj.id] = obj.image != False195 result[obj.id] = obj.image != False
194 return result196 return result
195197
198 def _commercial_partner_compute(self, cr, uid, ids, name, args, context=None):
199 """ Returns the partner that is considered the commercial
200 entity of this partner. The commercial entity holds the master data
201 for all commercial fields (see :py:meth:`~_commercial_fields`) """
202 result = dict.fromkeys(ids, False)
203 for partner in self.browse(cr, uid, ids, context=context):
204 current_partner = partner
205 while not current_partner.is_company and current_partner.parent_id:
206 current_partner = current_partner.parent_id
207 result[partner.id] = current_partner.id
208 return result
209
210 # indirection to avoid passing a copy of the overridable method when declaring the function field
211 _commercial_partner_id = lambda self, *args, **kwargs: self._commercial_partner_compute(*args, **kwargs)
212
196 _order = "name"213 _order = "name"
197 _columns = {214 _columns = {
198 'name': fields.char('Name', size=128, required=True, select=True),215 'name': fields.char('Name', size=128, required=True, select=True),
199 'date': fields.date('Date', select=1),216 'date': fields.date('Date', select=1),
200 'title': fields.many2one('res.partner.title', 'Title'),217 'title': fields.many2one('res.partner.title', 'Title'),
201 'parent_id': fields.many2one('res.partner', 'Related Company'),218 'parent_id': fields.many2one('res.partner', 'Related Company'),
202 'child_ids': fields.one2many('res.partner', 'parent_id', 'Contacts'),219 'child_ids': fields.one2many('res.partner', 'parent_id', 'Contacts', domain=[('active','=',True)]), # force "active_test" domain to bypass _search() override
203 'ref': fields.char('Reference', size=64, select=1),220 'ref': fields.char('Reference', size=64, select=1),
204 'lang': fields.selection(_lang_get, 'Language',221 'lang': fields.selection(_lang_get, 'Language',
205 help="If the selected language is loaded in the system, all documents related to this contact will be printed in this language. If not, it will be English."),222 help="If the selected language is loaded in the system, all documents related to this contact will be printed in this language. If not, it will be English."),
@@ -264,6 +281,9 @@
264 'color': fields.integer('Color Index'),281 'color': fields.integer('Color Index'),
265 'user_ids': fields.one2many('res.users', 'partner_id', 'Users'),282 'user_ids': fields.one2many('res.users', 'partner_id', 'Users'),
266 'contact_address': fields.function(_address_display, type='char', string='Complete Address'),283 'contact_address': fields.function(_address_display, type='char', string='Complete Address'),
284
285 # technical field used for managing commercial fields
286 'commercial_partner_id': fields.function(_commercial_partner_id, type='many2one', relation='res.partner', string='Commercial Entity')
267 }287 }
268288
269 def _default_category(self, cr, uid, context=None):289 def _default_category(self, cr, uid, context=None):
@@ -302,8 +322,8 @@
302 'company_id': lambda self, cr, uid, ctx: self.pool.get('res.company')._company_default_get(cr, uid, 'res.partner', context=ctx),322 'company_id': lambda self, cr, uid, ctx: self.pool.get('res.company')._company_default_get(cr, uid, 'res.partner', context=ctx),
303 'color': 0,323 'color': 0,
304 'is_company': False,324 'is_company': False,
305 'type': 'default',325 'type': 'contact', # type 'default' is wildcard and thus inappropriate
306 'use_parent_address': True,326 'use_parent_address': False,
307 'image': False,327 'image': False,
308 }328 }
309329
@@ -322,7 +342,6 @@
322 value = {}342 value = {}
323 value['title'] = False343 value['title'] = False
324 if is_company:344 if is_company:
325 value['parent_id'] = False
326 domain = {'title': [('domain', '=', 'partner')]}345 domain = {'title': [('domain', '=', 'partner')]}
327 else:346 else:
328 domain = {'title': [('domain', '=', 'contact')]}347 domain = {'title': [('domain', '=', 'contact')]}
@@ -332,11 +351,22 @@
332 def value_or_id(val):351 def value_or_id(val):
333 """ return val or val.id if val is a browse record """352 """ return val or val.id if val is a browse record """
334 return val if isinstance(val, (bool, int, long, float, basestring)) else val.id353 return val if isinstance(val, (bool, int, long, float, basestring)) else val.id
335354 result = {}
336 if use_parent_address and parent_id:355 if parent_id:
356 if ids:
357 partner = self.browse(cr, uid, ids[0], context=context)
358 if partner.parent_id and partner.parent_id.id != parent_id:
359 result['warning'] = {'title': _('Warning'),
360 'message': _('Changing the company of a contact should only be done if it '
361 'was never correctly set. If an existing contact starts working for a new '
362 'company then a new contact should be created under that new '
363 'company. You can use the "Discard" button to abandon this change.')}
337 parent = self.browse(cr, uid, parent_id, context=context)364 parent = self.browse(cr, uid, parent_id, context=context)
338 return {'value': dict((key, value_or_id(parent[key])) for key in ADDRESS_FIELDS)}365 address_fields = self._address_fields(cr, uid, context=context)
339 return {}366 result['value'] = dict((key, value_or_id(parent[key])) for key in address_fields)
367 else:
368 result['value'] = {'use_parent_address': False}
369 return result
340370
341 def onchange_state(self, cr, uid, ids, state_id, context=None):371 def onchange_state(self, cr, uid, ids, state_id, context=None):
342 if state_id:372 if state_id:
@@ -362,56 +392,134 @@
362392
363# _constraints = [(_check_ean_key, 'Error: Invalid ean code', ['ean13'])]393# _constraints = [(_check_ean_key, 'Error: Invalid ean code', ['ean13'])]
364394
365 def write(self, cr, uid, ids, vals, context=None):395 def _update_fields_values(self, cr, uid, partner, fields, context=None):
366 # Update parent and siblings or children records396 """ Returns dict of write() values for synchronizing ``fields`` """
367 if isinstance(ids, (int, long)):397 values = {}
368 ids = [ids]398 for field in fields:
369 if vals.get('is_company')==False:399 column = self._all_columns[field].column
370 vals.update({'child_ids' : [(5,)]})400 if column._type == 'one2many':
371 for partner in self.browse(cr, uid, ids, context=context):401 raise AssertionError('One2Many fields cannot be synchronized as part of `commercial_fields` or `address fields`')
372 update_ids = []402 if column._type == 'many2one':
373 if partner.is_company:403 values[field] = partner[field].id if partner[field] else False
374 domain_children = [('parent_id', '=', partner.id), ('use_parent_address', '=', True)]404 elif column._type == 'many2many':
375 update_ids = self.search(cr, uid, domain_children, context=context)405 values[field] = [(6,0,[r.id for r in partner[field] or []])]
376 elif partner.parent_id:
377 if vals.get('use_parent_address')==True:
378 domain_siblings = [('parent_id', '=', partner.parent_id.id), ('use_parent_address', '=', True)]
379 update_ids = [partner.parent_id.id] + self.search(cr, uid, domain_siblings, context=context)
380 if 'use_parent_address' not in vals and partner.use_parent_address:
381 domain_siblings = [('parent_id', '=', partner.parent_id.id), ('use_parent_address', '=', True)]
382 update_ids = [partner.parent_id.id] + self.search(cr, uid, domain_siblings, context=context)
383 self.update_address(cr, uid, update_ids, vals, context)
384 return super(res_partner,self).write(cr, uid, ids, vals, context=context)
385
386 def create(self, cr, uid, vals, context=None):
387 if context is None:
388 context = {}
389 # Update parent and siblings records
390 if vals.get('parent_id'):
391 if 'use_parent_address' in vals:
392 use_parent_address = vals['use_parent_address']
393 else:406 else:
394 use_parent_address = self.default_get(cr, uid, ['use_parent_address'], context=context)['use_parent_address']407 values[field] = partner[field]
395408 return values
396 if use_parent_address:409
397 domain_siblings = [('parent_id', '=', vals['parent_id']), ('use_parent_address', '=', True)]410 def _address_fields(self, cr, uid, context=None):
398 update_ids = [vals['parent_id']] + self.search(cr, uid, domain_siblings, context=context)411 """ Returns the list of address fields that are synced from the parent
399 self.update_address(cr, uid, update_ids, vals, context)412 when the `use_parent_address` flag is set. """
400413 return list(ADDRESS_FIELDS)
401 # add missing address keys
402 onchange_values = self.onchange_address(cr, uid, [], use_parent_address,
403 vals['parent_id'], context=context).get('value') or {}
404 vals.update(dict((key, value)
405 for key, value in onchange_values.iteritems()
406 if key in ADDRESS_FIELDS and key not in vals))
407
408 return super(res_partner, self).create(cr, uid, vals, context=context)
409414
410 def update_address(self, cr, uid, ids, vals, context=None):415 def update_address(self, cr, uid, ids, vals, context=None):
411 addr_vals = dict((key, vals[key]) for key in POSTAL_ADDRESS_FIELDS if key in vals)416 address_fields = self._address_fields(cr, uid, context=context)
417 addr_vals = dict((key, vals[key]) for key in address_fields if key in vals)
412 if addr_vals:418 if addr_vals:
413 return super(res_partner, self).write(cr, uid, ids, addr_vals, context)419 return super(res_partner, self).write(cr, uid, ids, addr_vals, context)
414420
421 def _commercial_fields(self, cr, uid, context=None):
422 """ Returns the list of fields that are managed by the commercial entity
423 to which a partner belongs. These fields are meant to be hidden on
424 partners that aren't `commercial entities` themselves, and will be
425 delegated to the parent `commercial entity`. The list is meant to be
426 extended by inheriting classes. """
427 return ['vat']
428
429 def _commercial_sync_from_company(self, cr, uid, partner, context=None):
430 """ Handle sync of commercial fields when a new parent commercial entity is set,
431 as if they were related fields """
432 if partner.commercial_partner_id != partner:
433 commercial_fields = self._commercial_fields(cr, uid, context=context)
434 sync_vals = self._update_fields_values(cr, uid, partner.commercial_partner_id,
435 commercial_fields, context=context)
436 return self.write(cr, uid, partner.id, sync_vals, context=context)
437
438 def _commercial_sync_to_children(self, cr, uid, partner, context=None):
439 """ Handle sync of commercial fields to descendants """
440 commercial_fields = self._commercial_fields(cr, uid, context=context)
441 sync_vals = self._update_fields_values(cr, uid, partner.commercial_partner_id,
442 commercial_fields, context=context)
443 sync_children = [c for c in partner.child_ids if not c.is_company]
444 for child in sync_children:
445 self._commercial_sync_to_children(cr, uid, child, context=context)
446 return self.write(cr, uid, [c.id for c in sync_children], sync_vals, context=context)
447
448 def _fields_sync(self, cr, uid, partner, update_values, context=None):
449 """ Sync commercial fields and address fields from company and to children after create/update,
450 just as if those were all modeled as fields.related to the parent """
451 # 1. From UPSTREAM: sync from parent
452 if update_values.get('parent_id') or update_values.get('use_company_address'):
453 # 1a. Commercial fields: sync if parent changed
454 if update_values.get('parent_id'):
455 self._commercial_sync_from_company(cr, uid, partner, context=context)
456 # 1b. Address fields: sync if parent or use_parent changed *and* both are now set
457 if partner.parent_id and partner.use_parent_address:
458 onchange_vals = self.onchange_address(cr, uid, [partner.id],
459 use_parent_address=partner.use_parent_address,
460 parent_id=partner.parent_id.id,
461 context=context).get('value', {})
462 self.update_address(cr, uid, partner.id, onchange_vals, context=context)
463
464 # 2. To DOWNSTREAM: sync children
465 if partner.child_ids:
466 # 2a. Commercial Fields: sync if commercial entity
467 if partner.commercial_partner_id == partner:
468 self._commercial_sync_to_children(cr, uid, partner, context=context)
469 # 2b. Address fields: sync if address changed
470 address_fields = self._address_fields(cr, uid, context=context)
471 if any(field in update_values for field in address_fields):
472 domain_children = [('parent_id', '=', partner.id), ('use_parent_address', '=', True)]
473 update_ids = self.search(cr, uid, domain_children, context=context)
474 self.update_address(cr, uid, update_ids, update_values, context=context)
475
476 def _handle_first_contact_creation(self, cr, uid, partner, context=None):
477 """ On creation of first contact for a company (or root) that has no address, assume contact address
478 was meant to be company address """
479 parent = partner.parent_id
480 address_fields = self._address_fields(cr, uid, context=context)
481 if parent and (parent.is_company or not parent.parent_id) and len(parent.child_ids) == 1 and \
482 any(partner[f] for f in address_fields) and not any(parent[f] for f in address_fields):
483 addr_vals = self._update_fields_values(cr, uid, partner, address_fields, context=context)
484 parent.update_address(addr_vals)
485 if not parent.is_company:
486 parent.write({'is_company': True})
487
488 def write(self, cr, uid, ids, vals, context=None):
489 if isinstance(ids, (int, long)):
490 ids = [ids]
491 result = super(res_partner,self).write(cr, uid, ids, vals, context=context)
492 for partner in self.browse(cr, uid, ids, context=context):
493 self._fields_sync(cr, uid, partner, vals, context)
494 return result
495
496 def create(self, cr, uid, vals, context=None):
497 new_id = super(res_partner, self).create(cr, uid, vals, context=context)
498 partner = self.browse(cr, uid, new_id, context=context)
499 self._fields_sync(cr, uid, partner, vals, context)
500 self._handle_first_contact_creation(cr, uid, partner, context)
501 return new_id
502
503 def open_commercial_entity(self, cr, uid, ids, context=None):
504 """ Utility method used to add an "Open Company" button in partner views """
505 partner = self.browse(cr, uid, ids[0], context=context)
506 return {'type': 'ir.actions.act_window',
507 'res_model': 'res.partner',
508 'view_mode': 'form',
509 'res_id': partner.commercial_partner_id.id,
510 'target': 'new',
511 'flags': {'form': {'action_buttons': True}}}
512
513 def open_parent(self, cr, uid, ids, context=None):
514 """ Utility method used to add an "Open Parent" button in partner views """
515 partner = self.browse(cr, uid, ids[0], context=context)
516 return {'type': 'ir.actions.act_window',
517 'res_model': 'res.partner',
518 'view_mode': 'form',
519 'res_id': partner.parent_id.id,
520 'target': 'new',
521 'flags': {'form': {'action_buttons': True}}}
522
415 def name_get(self, cr, uid, ids, context=None):523 def name_get(self, cr, uid, ids, context=None):
416 if context is None:524 if context is None:
417 context = {}525 context = {}
@@ -420,8 +528,8 @@
420 res = []528 res = []
421 for record in self.browse(cr, uid, ids, context=context):529 for record in self.browse(cr, uid, ids, context=context):
422 name = record.name530 name = record.name
423 if record.parent_id:531 if record.parent_id and not record.is_company:
424 name = "%s (%s)" % (name, record.parent_id.name)532 name = "%s, %s" % (record.parent_id.name, name)
425 if context.get('show_address'):533 if context.get('show_address'):
426 name = name + "\n" + self._display_address(cr, uid, record, without_company=True, context=context)534 name = name + "\n" + self._display_address(cr, uid, record, without_company=True, context=context)
427 name = name.replace('\n\n','\n')535 name = name.replace('\n\n','\n')
@@ -460,6 +568,15 @@
460 rec_id = self.create(cr, uid, {self._rec_name: name or email, 'email': email or False}, context=context)568 rec_id = self.create(cr, uid, {self._rec_name: name or email, 'email': email or False}, context=context)
461 return self.name_get(cr, uid, [rec_id], context)[0]569 return self.name_get(cr, uid, [rec_id], context)[0]
462570
571 def _search(self, cr, user, args, offset=0, limit=None, order=None, context=None, count=False, access_rights_uid=None):
572 """ Override search() to always show inactive children when searching via ``child_of`` operator. The ORM will
573 always call search() with a simple domain of the form [('parent_id', 'in', [ids])]. """
574 # a special ``domain`` is set on the ``child_ids`` o2m to bypass this logic, as it uses similar domain expressions
575 if len(args) == 1 and len(args[0]) == 3 and args[0][:2] == ('parent_id','in'):
576 context = dict(context or {}, active_test=False)
577 return super(res_partner, self)._search(cr, user, args, offset=offset, limit=limit, order=order, context=context,
578 count=count, access_rights_uid=access_rights_uid)
579
463 def name_search(self, cr, uid, name, args=None, operator='ilike', context=None, limit=100):580 def name_search(self, cr, uid, name, args=None, operator='ilike', context=None, limit=100):
464 if not args:581 if not args:
465 args = []582 args = []
@@ -520,25 +637,42 @@
520 ids = ids[16:]637 ids = ids[16:]
521 return True638 return True
522639
523 def address_get(self, cr, uid, ids, adr_pref=None):640 def address_get(self, cr, uid, ids, adr_pref=None, context=None):
524 if adr_pref is None:641 """ Find contacts/addresses of the right type(s) by doing a depth-first-search
525 adr_pref = ['default']642 through descendants within company boundaries (stop at entities flagged ``is_company``)
643 then continuing the search at the ancestors that are within the same company boundaries.
644 Defaults to partners of type ``'default'`` when the exact type is not found, or to the
645 provided partner itself if no type ``'default'`` is found either. """
646 adr_pref = set(adr_pref or [])
647 if 'default' not in adr_pref:
648 adr_pref.add('default')
526 result = {}649 result = {}
527 # retrieve addresses from the partner itself and its children650 visited = set()
528 res = []651 for partner in self.browse(cr, uid, filter(None, ids), context=context):
529 # need to fix the ids ,It get False value in list like ids[False]652 current_partner = partner
530 if ids and ids[0]!=False:653 while current_partner:
531 for p in self.browse(cr, uid, ids):654 to_scan = [current_partner]
532 res.append((p.type, p.id))655 # Scan descendants, DFS
533 res.extend((c.type, c.id) for c in p.child_ids)656 while to_scan:
534 address_dict = dict(reversed(res))657 record = to_scan.pop(0)
535 # get the id of the (first) default address if there is one,658 visited.add(record)
536 # otherwise get the id of the first address in the list659 if record.type in adr_pref and not result.get(record.type):
537 default_address = False660 result[record.type] = record.id
538 if res:661 if len(result) == len(adr_pref):
539 default_address = address_dict.get('default', res[0][1])662 return result
540 for adr in adr_pref:663 to_scan = [c for c in record.child_ids
541 result[adr] = address_dict.get(adr, default_address)664 if c not in visited
665 if not c.is_company] + to_scan
666
667 # Continue scanning at ancestor if current_partner is not a commercial entity
668 if current_partner.is_company or not current_partner.parent_id:
669 break
670 current_partner = current_partner.parent_id
671
672 # default to type 'default' or the partner itself
673 default = result.get('default', partner.id)
674 for adr_type in adr_pref:
675 result[adr_type] = result.get(adr_type) or default
542 return result676 return result
543677
544 def view_header_get(self, cr, uid, view_id, view_type, context):678 def view_header_get(self, cr, uid, view_id, view_type, context):
@@ -580,8 +714,7 @@
580 'country_name': address.country_id and address.country_id.name or '',714 'country_name': address.country_id and address.country_id.name or '',
581 'company_name': address.parent_id and address.parent_id.name or '',715 'company_name': address.parent_id and address.parent_id.name or '',
582 }716 }
583 address_field = ['title', 'street', 'street2', 'zip', 'city']717 for field in self._address_fields(cr, uid, context=context):
584 for field in address_field :
585 args[field] = getattr(address, field) or ''718 args[field] = getattr(address, field) or ''
586 if without_company:719 if without_company:
587 args['company_name'] = ''720 args['company_name'] = ''
588721
=== modified file 'openerp/addons/base/res/res_partner_view.xml'
--- openerp/addons/base/res/res_partner_view.xml 2013-04-09 12:29:54 +0000
+++ openerp/addons/base/res/res_partner_view.xml 2013-04-20 00:50:34 +0000
@@ -138,8 +138,8 @@
138 </h1>138 </h1>
139 <field name="parent_id"139 <field name="parent_id"
140 placeholder="Company"140 placeholder="Company"
141 domain="[('is_company', '=', True)]" context="{'default_is_company': True}"141 domain="[('is_company', '=', True)]" context="{'default_is_company': True, 'default_supplier': supplier}"
142 attrs="{'invisible': [('is_company','=', True)]}"142 attrs="{'invisible': [('is_company','=', True),('parent_id', '=', False)]}"
143 on_change="onchange_address(use_parent_address, parent_id)"/>143 on_change="onchange_address(use_parent_address, parent_id)"/>
144 <field name="category_id" widget="many2many_tags" placeholder="Tags..."/>144 <field name="category_id" widget="many2many_tags" placeholder="Tags..."/>
145 </div>145 </div>
@@ -151,21 +151,24 @@
151 <div attrs="{'invisible': [('parent_id','=', False)]}" name="div_type">151 <div attrs="{'invisible': [('parent_id','=', False)]}" name="div_type">
152 <field class="oe_inline"152 <field class="oe_inline"
153 name="type"/>153 name="type"/>
154 <label for="use_parent_address" class="oe_edit_only"/>
155 <field name="use_parent_address" class="oe_edit_only oe_inline"
156 on_change="onchange_address(use_parent_address, parent_id)"/>
157 </div>154 </div>
158155
159 <label for="street" string="Address"/>156 <label for="street" string="Address"/>
160 <div>157 <div>
161 <field name="street" placeholder="Street..."/>158 <field name="use_parent_address" class="oe_edit_only oe_inline"
162 <field name="street2"/>159 on_change="onchange_address(use_parent_address, parent_id)"
160 attrs="{'invisible': [('parent_id','=', False)]}"/>
161 <label for="use_parent_address" class="oe_edit_only" attrs="{'invisible': [('parent_id','=', False)]}"/>
162 <button name="open_parent" type="object" string="(edit company address)" class="oe_link oe_edit_only"
163 attrs="{'invisible': ['|',('parent_id','=', False),('use_parent_address','=',False)]}"/>
164 <field name="street" placeholder="Street..." attrs="{'readonly': [('use_parent_address','=',True)]}"/>
165 <field name="street2" attrs="{'readonly': [('use_parent_address','=',True)]}"/>
163 <div class="address_format">166 <div class="address_format">
164 <field name="city" placeholder="City" style="width: 40%%"/>167 <field name="city" placeholder="City" style="width: 40%%" attrs="{'readonly': [('use_parent_address','=',True)]}"/>
165 <field name="state_id" class="oe_no_button" placeholder="State" style="width: 37%%" options='{"no_open": True}' on_change="onchange_state(state_id)"/>168 <field name="state_id" class="oe_no_button" placeholder="State" style="width: 37%%" options='{"no_open": True}' on_change="onchange_state(state_id)" attrs="{'readonly': [('use_parent_address','=',True)]}"/>
166 <field name="zip" placeholder="ZIP" style="width: 20%%"/>169 <field name="zip" placeholder="ZIP" style="width: 20%%" attrs="{'readonly': [('use_parent_address','=',True)]}"/>
167 </div>170 </div>
168 <field name="country_id" placeholder="Country" class="oe_no_button" options='{"no_open": True}'/>171 <field name="country_id" placeholder="Country" class="oe_no_button" options='{"no_open": True}' attrs="{'readonly': [('use_parent_address','=',True)]}"/>
169 </div>172 </div>
170 <field name="website" widget="url" placeholder="e.g. www.openerp.com"/>173 <field name="website" widget="url" placeholder="e.g. www.openerp.com"/>
171 </group>174 </group>
@@ -182,8 +185,8 @@
182 </group>185 </group>
183186
184 <notebook colspan="4">187 <notebook colspan="4">
185 <page string="Contacts" attrs="{'invisible': [('is_company','=',False)]}" autofocus="autofocus">188 <page string="Contacts" attrs="{'invisible': [('is_company','=',False), ('child_ids', '=', [])]}" autofocus="autofocus">
186 <field name="child_ids" context="{'default_parent_id': active_id}" mode="kanban">189 <field name="child_ids" mode="kanban" context="{'default_parent_id': active_id, 'default_street': street, 'default_street2': street2, 'default_city': city, 'default_state_id': state_id, 'default_zip': zip, 'default_country_id': country_id, 'default_supplier': supplier}">
187 <kanban>190 <kanban>
188 <field name="color"/>191 <field name="color"/>
189 <field name="name"/>192 <field name="name"/>
@@ -249,17 +252,41 @@
249 </templates>252 </templates>
250 </kanban>253 </kanban>
251 <form string="Contact" version="7.0">254 <form string="Contact" version="7.0">
252 <field name="image" widget='image' class="oe_avatar oe_left" options='{"preview_image": "image_medium"}'/>255 <sheet>
253 <div class="oe_title">256 <field name="image" widget='image' class="oe_avatar oe_left" options='{"preview_image": "image_medium"}'/>
257 <div class="oe_title">
258 <label for="name" class="oe_edit_only"/>
259 <h1><field name="name" style="width: 70%%"/></h1>
260 <field name="category_id" widget="many2many_tags" placeholder="Tags..." style="width: 70%%"/>
261 </div>
254 <group>262 <group>
255 <field name="name"/>
256 <field name="category_id" widget="many2many_tags" placeholder="Tags..."/>
257 <field name="function" placeholder="e.g. Sales Director"/>263 <field name="function" placeholder="e.g. Sales Director"/>
258 <field name="email"/>264 <field name="email"/>
259 <field name="phone"/>265 <field name="phone"/>
260 <field name="mobile"/>266 <field name="mobile"/>
261 </group>267 </group>
262 </div>268 <div>
269 <field name="use_parent_address"/><label for="use_parent_address"/>
270 </div>
271 <group>
272 <label for="type"/>
273 <div name="div_type">
274 <field class="oe_inline" name="type"/>
275 </div>
276 <label for="street" string="Address" attrs="{'invisible': [('use_parent_address','=', True)]}"/>
277 <div attrs="{'invisible': [('use_parent_address','=', True)]}" name="div_address">
278 <field name="street" placeholder="Street..."/>
279 <field name="street2"/>
280 <div class="address_format">
281 <field name="city" placeholder="City" style="width: 40%%"/>
282 <field name="state_id" class="oe_no_button" placeholder="State" style="width: 37%%" options='{"no_open": True}' on_change="onchange_state(state_id)"/>
283 <field name="zip" placeholder="ZIP" style="width: 20%%"/>
284 </div>
285 <field name="country_id" placeholder="Country" class="oe_no_button" options='{"no_open": True}'/>
286 </div>
287 </group>
288 <field name="supplier" invisible="True"/>
289 </sheet>
263 </form>290 </form>
264 </field>291 </field>
265 </page>292 </page>
266293
=== modified file 'openerp/addons/base/tests/test_base.py'
--- openerp/addons/base/tests/test_base.py 2013-04-15 17:17:32 +0000
+++ openerp/addons/base/tests/test_base.py 2013-04-20 00:50:34 +0000
@@ -40,6 +40,216 @@
40 self.assertTrue(new_id2 > new_id, 'find_or_create failed - should have created new one again')40 self.assertTrue(new_id2 > new_id, 'find_or_create failed - should have created new one again')
4141
4242
43 def test_20_res_partner_address_sync(self):
44 cr, uid = self.cr, self.uid
45 ghoststep = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid,
46 {'name': 'GhostStep',
47 'is_company': True,
48 'street': 'Main Street, 10',
49 'phone': '123456789',
50 'email': 'info@ghoststep.com',
51 'vat': 'BE0477472701',
52 'type': 'default'}))
53 p1 = self.res_partner.browse(cr, uid, self.res_partner.name_create(cr, uid, 'Denis Bladesmith <denis.bladesmith@ghoststep.com>')[0])
54 self.assertEqual(p1.type, 'contact', 'Default type must be "contact"')
55 p1phone = '123456789#34'
56 p1.write({'phone': p1phone,
57 'parent_id': ghoststep.id,
58 'use_parent_address': True})
59 p1.refresh()
60 self.assertEqual(p1.street, ghoststep.street, 'Address fields must be synced')
61 self.assertEqual(p1.phone, p1phone, 'Phone should be preserved after address sync')
62 self.assertEqual(p1.type, 'contact', 'Type should be preserved after address sync')
63 self.assertEqual(p1.email, 'denis.bladesmith@ghoststep.com', 'Email should be preserved after sync')
64 ghoststreet = 'South Street, 25'
65 ghoststep.write({'street': ghoststreet})
66 p1.refresh()
67 self.assertEqual(p1.street, ghoststreet, 'Address fields must be synced automatically')
68 self.assertEqual(p1.phone, p1phone, 'Phone should not be synced')
69 self.assertEqual(p1.email, 'denis.bladesmith@ghoststep.com', 'Email should be preserved after sync')
70
71 p1street = 'My Street, 11'
72 p1.write({'street': p1street})
73 ghoststep.refresh()
74 self.assertEqual(ghoststep.street, ghoststreet, 'Touching contact should never alter parent')
75
76
77 def test_30_res_partner_first_contact_sync(self):
78 """ Test initial creation of company/contact pair where contact address gets copied to
79 company """
80 cr, uid = self.cr, self.uid
81 ironshield = self.res_partner.browse(cr, uid, self.res_partner.name_create(cr, uid, 'IronShield')[0])
82 self.assertFalse(ironshield.is_company, 'Partners are not companies by default')
83 self.assertFalse(ironshield.use_parent_address, 'use_parent_address defaults to False')
84 self.assertEqual(ironshield.type, 'contact', 'Default type must be "contact"')
85 ironshield.write({'type': 'default'}) # force default type to double-check sync
86 p1 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid,
87 {'name': 'Isen Hardearth',
88 'street': 'Strongarm Avenue, 12',
89 'parent_id': ironshield.id}))
90 self.assertEquals(p1.type, 'contact', 'Default type must be "contact", not the copied parent type')
91 ironshield.refresh()
92 self.assertEqual(ironshield.street, p1.street, 'Address fields should be copied to company')
93 self.assertTrue(ironshield.is_company, 'Company flag should be turned on after first contact creation')
94
95 def test_40_res_partner_address_getc(self):
96 """ Test address_get address resolution mechanism: it should first go down through descendants,
97 stopping when encountering another is_copmany entity, then go up, stopping again at the first
98 is_company entity or the root ancestor and if nothing matches, it should use the provided partner
99 itself """
100 cr, uid = self.cr, self.uid
101 elmtree = self.res_partner.browse(cr, uid, self.res_partner.name_create(cr, uid, 'Elmtree')[0])
102 branch1 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid, {'name': 'Branch 1',
103 'parent_id': elmtree.id,
104 'is_company': True}))
105 leaf10 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid, {'name': 'Leaf 10',
106 'parent_id': branch1.id,
107 'type': 'invoice'}))
108 branch11 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid, {'name': 'Branch 11',
109 'parent_id': branch1.id,
110 'type': 'other'}))
111 leaf111 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid, {'name': 'Leaf 111',
112 'parent_id': branch11.id,
113 'type': 'delivery'}))
114 branch11.write({'is_company': False}) # force is_company after creating 1rst child
115 branch2 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid, {'name': 'Branch 2',
116 'parent_id': elmtree.id,
117 'is_company': True}))
118 leaf21 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid, {'name': 'Leaf 21',
119 'parent_id': branch2.id,
120 'type': 'delivery'}))
121 leaf22 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid, {'name': 'Leaf 22',
122 'parent_id': branch2.id}))
123 leaf23 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid, {'name': 'Leaf 23',
124 'parent_id': branch2.id,
125 'type': 'default'}))
126 # go up, stop at branch1
127 self.assertEqual(self.res_partner.address_get(cr, uid, [leaf111.id], ['delivery', 'invoice', 'contact', 'other', 'default']),
128 {'delivery': leaf111.id,
129 'invoice': leaf10.id,
130 'contact': branch1.id,
131 'other': branch11.id,
132 'default': leaf111.id}, 'Invalid address resolution')
133 self.assertEqual(self.res_partner.address_get(cr, uid, [branch11.id], ['delivery', 'invoice', 'contact', 'other', 'default']),
134 {'delivery': leaf111.id,
135 'invoice': leaf10.id,
136 'contact': branch1.id,
137 'other': branch11.id,
138 'default': branch11.id}, 'Invalid address resolution')
139
140 # go down, stop at at all child companies
141 self.assertEqual(self.res_partner.address_get(cr, uid, [elmtree.id], ['delivery', 'invoice', 'contact', 'other', 'default']),
142 {'delivery': elmtree.id,
143 'invoice': elmtree.id,
144 'contact': elmtree.id,
145 'other': elmtree.id,
146 'default': elmtree.id}, 'Invalid address resolution')
147
148 # go down through children
149 self.assertEqual(self.res_partner.address_get(cr, uid, [branch1.id], ['delivery', 'invoice', 'contact', 'other', 'default']),
150 {'delivery': leaf111.id,
151 'invoice': leaf10.id,
152 'contact': branch1.id,
153 'other': branch11.id,
154 'default': branch1.id}, 'Invalid address resolution')
155 self.assertEqual(self.res_partner.address_get(cr, uid, [branch2.id], ['delivery', 'invoice', 'contact', 'other', 'default']),
156 {'delivery': leaf21.id,
157 'invoice': leaf23.id,
158 'contact': branch2.id,
159 'other': leaf23.id,
160 'default': leaf23.id}, 'Invalid address resolution')
161
162 # go up then down through siblings
163 self.assertEqual(self.res_partner.address_get(cr, uid, [leaf21.id], ['delivery', 'invoice', 'contact', 'other', 'default']),
164 {'delivery': leaf21.id,
165 'invoice': leaf23.id,
166 'contact': branch2.id,
167 'other': leaf23.id,
168 'default': leaf23.id
169 }, 'Invalid address resolution, should scan commercial entity ancestor and its descendants')
170 self.assertEqual(self.res_partner.address_get(cr, uid, [leaf22.id], ['delivery', 'invoice', 'contact', 'other', 'default']),
171 {'delivery': leaf21.id,
172 'invoice': leaf23.id,
173 'contact': leaf22.id,
174 'other': leaf23.id,
175 'default': leaf23.id}, 'Invalid address resolution, should scan commercial entity ancestor and its descendants')
176 self.assertEqual(self.res_partner.address_get(cr, uid, [leaf23.id], ['delivery', 'invoice', 'contact', 'other', 'default']),
177 {'delivery': leaf21.id,
178 'invoice': leaf23.id,
179 'contact': branch2.id,
180 'other': leaf23.id,
181 'default': leaf23.id}, 'Invalid address resolution, `default` should only override if no partner with specific type exists')
182
183 # empty adr_pref means only 'default'
184 self.assertEqual(self.res_partner.address_get(cr, uid, [elmtree.id], []),
185 {'default': elmtree.id}, 'Invalid address resolution, no default means commercial entity ancestor')
186 self.assertEqual(self.res_partner.address_get(cr, uid, [leaf111.id], []),
187 {'default': leaf111.id}, 'Invalid address resolution, no default means contact itself')
188 branch11.write({'type': 'default'})
189 self.assertEqual(self.res_partner.address_get(cr, uid, [leaf111.id], []),
190 {'default': branch11.id}, 'Invalid address resolution, branch11 should now be default')
191
192
193 def test_50_res_partner_commercial_sync(self):
194 cr, uid = self.cr, self.uid
195 p0 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid,
196 {'name': 'Sigurd Sunknife',
197 'email': 'ssunknife@gmail.com'}))
198 sunhelm = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid,
199 {'name': 'Sunhelm',
200 'is_company': True,
201 'street': 'Rainbow Street, 13',
202 'phone': '1122334455',
203 'email': 'info@sunhelm.com',
204 'vat': 'BE0477472701',
205 'child_ids': [(4, p0.id),
206 (0, 0, {'name': 'Alrik Greenthorn',
207 'email': 'agr@sunhelm.com'})],
208 }))
209 p1 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid,
210 {'name': 'Otto Blackwood',
211 'email': 'otto.blackwood@sunhelm.com',
212 'parent_id': sunhelm.id}))
213 p11 = self.res_partner.browse(cr, uid, self.res_partner.create(cr, uid,
214 {'name': 'Gini Graywool',
215 'email': 'ggr@sunhelm.com',
216 'parent_id': p1.id}))
217 p2 = self.res_partner.browse(cr, uid, self.res_partner.search(cr, uid,
218 [('email', '=', 'agr@sunhelm.com')])[0])
219
220 for p in (p0, p1, p11, p2):
221 p.refresh()
222 self.assertEquals(p.commercial_partner_id, sunhelm, 'Incorrect commercial entity resolution')
223 self.assertEquals(p.vat, sunhelm.vat, 'Commercial fields must be automatically synced')
224 sunhelmvat = 'BE0123456789'
225 sunhelm.write({'vat': sunhelmvat})
226 for p in (p0, p1, p11, p2):
227 p.refresh()
228 self.assertEquals(p.vat, sunhelmvat, 'Commercial fields must be automatically and recursively synced')
229
230 p1vat = 'BE0987654321'
231 p1.write({'vat': p1vat})
232 for p in (sunhelm, p0, p11, p2):
233 p.refresh()
234 self.assertEquals(p.vat, sunhelmvat, 'Sync to children should only work downstream and on commercial entities')
235
236 # promote p1 to commercial entity
237 vals = p1.onchange_type(is_company=True)['value']
238 p1.write(dict(vals, parent_id=sunhelm.id,
239 is_company=True,
240 name='Sunhelm Subsidiary'))
241 p1.refresh()
242 self.assertEquals(p1.vat, p1vat, 'Setting is_company should stop auto-sync of commercial fields')
243 self.assertEquals(p1.commercial_partner_id, p1, 'Incorrect commercial entity resolution after setting is_company')
244
245 # writing on parent should not touch child commercial entities
246 sunhelmvat2 = 'BE0112233445'
247 sunhelm.write({'vat': sunhelmvat2})
248 p1.refresh()
249 self.assertEquals(p1.vat, p1vat, 'Setting is_company should stop auto-sync of commercial fields')
250 p0.refresh()
251 self.assertEquals(p0.vat, sunhelmvat2, 'Commercial fields must be automatically synced')
252
43class test_partner_recursion(common.TransactionCase):253class test_partner_recursion(common.TransactionCase):
44254
45 def setUp(self):255 def setUp(self):
46256
=== modified file 'openerp/addons/base/tests/test_expression.py'
--- openerp/addons/base/tests/test_expression.py 2013-03-21 17:37:37 +0000
+++ openerp/addons/base/tests/test_expression.py 2013-04-20 00:50:34 +0000
@@ -263,7 +263,7 @@
263 "_auto_join on: ('child_ids.bank_ids.id', 'in', [..]) query incorrect join condition")263 "_auto_join on: ('child_ids.bank_ids.id', 'in', [..]) query incorrect join condition")
264 self.assertIn('"res_partner__child_ids"."id"="res_partner__child_ids__bank_ids"."partner_id"', sql_query[1],264 self.assertIn('"res_partner__child_ids"."id"="res_partner__child_ids__bank_ids"."partner_id"', sql_query[1],
265 "_auto_join on: ('child_ids.bank_ids.id', 'in', [..]) query incorrect join condition")265 "_auto_join on: ('child_ids.bank_ids.id', 'in', [..]) query incorrect join condition")
266 self.assertEqual(set([b_aa, b_ba]), set(sql_query[2]),266 self.assertEqual(set([b_aa, b_ba]), set(sql_query[2][-2:]),
267 "_auto_join on: ('child_ids.bank_ids.id', 'in', [..]) query incorrect parameter")267 "_auto_join on: ('child_ids.bank_ids.id', 'in', [..]) query incorrect parameter")
268268
269 # --------------------------------------------------269 # --------------------------------------------------