Comment 181 for bug 1160365

Revision history for this message
Mario Arias (the-clone-master) wrote : Re: [Bug 1160365] Re: [7.0] incorrect handling of contact/companies for invoicing and related purposes

Thanks Olivier for your time,

I still don't like this "denormalized" record. As you said, information
is all there, but that attribute copying makes me really nervous.... all
the floating point and dates based on UTC nightmares we all are still
suffering come to my mind right away....

Besides, that extra "sanity check" that you are imposing to migrate
modules from 6.1 to 7.0 is by no means "small".

Now, best way to test it is with real life module migrations, so we will
start working on a couple upgrades from 6.1 to 7.0, and will report back
when done...

As to echo others, having companies and contacts in one table is not the
problem, but the lack of complete information about the change and too
optimistic testing, and mainly... not acknowledging at first that there
was a BIG problem with the original release, and instead attacking the
ones raising the flag...

You really are in debt with Raphaël....

Please stop repeating that "we have tested it for the last year" song,
as that makes us wonder what kind of testing you did that missed these
problems in the model. Instead, be humble and accept you screwed up...
same as WE ALL failed to catch this sooner too...

Regards,
-Mario

On 04/15/2013 08:42 AM, Olivier Dony (OpenERP) wrote:
> On 04/15/2013 04:09 PM, Mario Arias wrote:
>> Again, could you please explain how are you handling encapsulation of
>> your "denormalized" contacts proposal ??
> The bulk of the idea is in the description of this bug ("Proposed solution")
> and the actual implementation can be reviewed on the framework merge proposal:
> https://code.launchpad.net/~openerp-dev/openobject-server/7.0-fix-contact-company-handling/+merge/157577
>
>
>> * How are you copying data/attributes from companies to contacts?
> This is part B of the described solution.
>
> Since business contact and companies live in the same database table, they both
> have all accounting attributes at SQL level. In the UI we forbid editing the
> accounting attributes on business contacts and replace the fields with a link
> to edit these attributes on the parent company.
>
> On the database side, in order to avoid keeping empty (or worse: incorrect)
> values in the accounting fields of business contacts, we implement automated
> inheritance of the values (similarly to what fields.related would do). Whenever
> these accounting attributes are modified on the company, the values are
> automatically synchronized to the children. Same thing if the "contact-company"
> link changes.
>
>
>> * How is this being encapsulated/hidden from addons?
> This logic is completely implemented by the res.partner model in the framework.
> Addons can extend the list of fields that are considered "accounting
> atttributes" and therefore automatically synchronized by overriding the
> res.partner._commercial_fields() method.
>
> The merge proposal for addons does this for the accounting attributes of all
> official addons, if you want to get an idea:
> https://code.launchpad.net/~openerp-dev/openobject-addons/7.0-fix-contact-company-handling/+merge/157576
>
>
>> * How is this better than a relation from contact to company?
> It's not better, it comes *in addition* to the contact-company relationship as
> an extra security. Addons that do care about the contact vs company distinction
> can directly access the correct value on the company.
> But if some fail to do it properly they will still get the right value, be it
> payment term, payable account, etc.
>
>
>> * How should we proceed if we need to add any aditional field from company to contact?
> If your module adds an accounting-related field 'special_invoice_data_id' to
> res.partner, just override the method as follows and synchronization will be
> done automatically:
>
> def _commercial_fields(self, cr, uid, context=None):
> return super(res_partner, self)._commercial_fields(cr, uid, context=context)\
> + ['special_invoice_data_id']
>
> If your field is normally editable in the "Accounting" tab of the Partner form
> you have nothing else to do: it will be replaced by a link to the company form
> when viewing a contact of a company. If not, have a look at the examples in the
> addons merge proposal, a simple `attrs` attribute will do the job, and you can
> easily add a link to open the company form if you want.
>