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

Proposed by Olivier Dony (Odoo) on 2013-04-08
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 on 2013-04-18
hbto [Vauxoo] http://www.vauxoo.com (community) Needs Fixing on 2013-04-11
Moisés López - http://www.vauxoo.com (community) Needs Fixing on 2013-04-09
Nhomar - Vauxoo (community) Needs Fixing on 2013-04-09
Stefan Rijnhart (Opener) (community) Disapprove on 2013-04-09
Yannick Vaucher @ Camptocamp (community) code, test Needs Fixing on 2013-04-08
OpenERP Core Team 2013-04-08 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.

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

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)

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> on 2013-04-08

[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

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
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
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

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
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
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
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

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)

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
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

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) on 2013-04-10

[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) on 2013-04-10

[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) on 2013-04-10

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

4929. By Olivier Dony (Odoo) on 2013-04-10

[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) on 2013-04-10

[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.

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) on 2013-04-11

[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) on 2013-04-11

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

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

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) on 2013-04-15

[MERGE] Sync with 7.0

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) on 2013-04-16

[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) on 2013-04-16

[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) on 2013-04-16

[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.

4937. By Olivier Dony (Odoo) on 2013-04-18

[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) on 2013-04-18

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

4939. By Olivier Dony (Odoo) on 2013-04-19

[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) on 2013-04-19

[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) on 2013-04-19

[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) on 2013-04-19

[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) on 2013-04-20

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