Some more precisions regarding Joël's questions... On 04/11/2013 12:18 PM, Joël Grand-Guillaume @ camptocamp wrote: > A) When making a SO, choosing Camptocamp as the Customer, it set Joel as > Invoice address and Guewen as shipping one. That means I can invoice Joel > from here and not from the accounting menu... That seems strange IMO, what > do you think ? If SO can make an invoice for "Joel (Camptocamp)", why I > can't invoice Joel as well from the accounting menu... => this is a bug I > think and can be fixed allowing to invoice Joel in Invoice (mean allow to > invoice "invoicing" partner"), you agree ? Yes, that was inconsistent in the proposed solution and in comment #50. The domain change was reverted on invoices. > B) When making a SO to Camptocamp with "invoice from delivery", confirm it, > confirm deliver and invoice it => it invoice Camptocamp not "Joel" selected > on the offer as the invoicing contact... That is not good. We need to be > consistent... I expect to invoice Joel in that case right ? => This is a bug > and can be fixed, you agree ? Yes that was clearly a bug as well. It existed in previous versions (the invoice address was not copied but instead reset), but was only apparent if using an invoice address that was not the default one selected by the onchange. This is fixed in the sale_stock module. > C) In the case invoice are related to "Joel (Camptocamp)", the sales > turnover for Camptocamp is not correct (from Reporting -> Invoice analysis > menu, not from grouping invoices,...). This is handled by the extra "account_report_company" module, to be merged in account in trunk, and which will allow grouping and analyzing invoices per "Commercial Entity". We only need this in Invoices because the contact-level granularity is explicitly overridden on the Journal Entries, and because Invoices are a key reporting area in any business. For all other cases we think the contact-level granularity will be correct and expected. As Fabien mentioned, there can be cases where it will be useful to allow search criterions on "Partner" to also match the contacts (child_of), but these should only be for usability purposes. Performance-wise, the "child_of" operator used for a Partner criterions only depends on the depth of the partner hierarchy, not the number of business documents. For very deep hierarchies it can even be made O(1) by enabling _parent_store on res.partner, if ever needed. > D) We still cannot invoice a specific address of a specific company... We > have (and others too) some complex structure to invoice, like the states, > big university and even with those corrections, we do not solve this major > issue. You have: > - Group Company XY > - Company A > - Department A, Contact 1 > - Department A, Contact 2 > - Company B > - Department B, Contact 3 > => This is mandatory that you can invoice Company A, for submission to > "Department A, Contact 1" or "Department A, Contact 2". We do need two > fields on the invoice... Because even if properties are correctly set (the > only bug you saw and correct here), we can still make invoice addressed to > Contact 1 or 2 depending on the project (cause they are they people to > validate the invoice internally), but still, the invoice belong to Company > A, will be paid by Company A and the turnover belong to Company A not > invoicing Contact 1 or 2 !!! Using group by must show this as it was in > previous version. Invoicing "Department A, Contact 1" or "Department A, Contact 2" should work fine with the updated proposal, and the Invoice analysis and group by will behave as expected on "Company A" or "Company B". > E) Now migration (done by your service). We get strong trouble because > before migration, those WERE good, and aren't any more: > - Turnover by company is right because based on partner_id (through analysis > or whatever group by) > - Invoices to various department/contact for the same partner/company based > on contact_id can be done > - External link to other systems that are based on the 6.1 data model... > => How will you keep this information correct / backward compatible in the > migrated instance without having 2 fields on the invoice (or other objects) > I think it is just impossible. How will you handle that ? As points C and D are covered the result of the migration should be immediately correct. As for external links to other systems it will be trivial to fix with the "account_report_company" module, even though a proper migration should still be planned, just like for everything else. > F) Another trouble for migration (still done by you): > > Before migration: Invoice open to Company A, Contact M. Martin => Invoice > printed and sent to M. Martin in Switzerland while Company A is in Sweden. > > After Migration (and you've done it): Invoice open to Company A, no more > contact => Print out the invoice, and send it to... Company A in Sweden > !!??!? > > => Again, if you do not store 2 fields, you cannot guarantee backward > compatibility here right ? Or what solution do you have ? That looks like an error in the migration, the Invoice after migration should have been issued for "M. Martin in Switzerland", contact of "Company A in Sweden" - which still only needs one field. The second field is only needed when you want Invoice grouping and reporting for "Company A". > G) All modules in standard addons or community or whatever will have to be > rebuild them to use what you suggest with your "commercial_entity_id" so why > not using the Raphaël's way that keep backward compatibility, while still > keeping you DB schema and views simpler as you liked to have ? => Any > concrete reasons or facts on that ? I mean in both cases there is work to be > done, just the solution you suggest need more... Or I may be missing > something? On the contrary, very few changes are required in community modules, just like very few were needed in official addons. The contact level of granularity is correct and it would be an error to change all community modules to use the "commercial entity" instead. Even for modules that handle accounting/invoicing data, our proposed solution takes care of delegating the relevant fields from contact to company. So unless a module deals with very low-level accounting data and partially bypasses the generation of Journal Entries of the core accounting, no change should be needed. Porting a module from OpenERP 6.1 to OpenERP 7.0 still requires a real review/migration, and fortunately the community is better organized now to perform those reviews efficiently and with shared resources. (thanks to you!) This point is critical and must be very clear, especially for contributors who will take part in porting/reviewing modules for v7.0! As for Raphael Valyi's proposed module, it is a clever piece of OpenERP magic but it looks like the wrong option for 2 main reasons: 1. It goes in the opposite direction model-wise: in 7.0 the level of granularity for partners is "contact" by default everywhere, and "company" is the exception for Journal entries. Yes this is a big change, and some of the consequences were missed during the design and test phases, but we still think this is the way to go. The solution he proposes goes backwards and only shows contacts on the surface, while keeping a "company" level of granularity everywhere. All reporting/group-by/propagation would still be based on the "company" level, the "contact" info would be lost and become the exception rather than the rule, so that will also break any new feature that expects contact-level granularity to be the default. That's exactly the opposite of what 7.0 wanted to achieve. 2. The way that solution is implemented is dangerous, and it might even be worse than whatever issues it hopes to cure. It creates hidden fields and hidden behaviors everywhere in the system, affecting all modules in a way that developers can easily miss during their developments, and adding possibly hard-to-detect side-effects. Moreover, it does not have any escape plan: if you install it there is no easy way to go back, short of deciding to add an extra field and an extra on_change method on *every* model of *every* module you will ever use (be it community or official). And we explicitly do *not* want that to happen everywhere, as explained. So if you use it you'll either have to stay with some amount of ORM magic/hidden behavior forever, or to migrate/opt-out all your models/tables one by one to drop the extra fields, eventually ending up with the same result we have now. So yes, there might be a few remaining cases we missed (like those you spotted, thanks!), but there shouldn't be many because this is the exception, not the rule. And it's definitely better to fix these few cases than trying to go backwards for fear of the changes, ending up with a mixed model that is not consistent and no clear way of improving it. > H) You said : > """ > Note also that "partner_id, address_id" is not exactly the same than > "partner_commercial_id, partner_id". The semantic really changed, it's > not just a switch in field names. > """ > About what you're speaking here ? V8.0 ? No such a fields > (invoice_commercial_ids) are present today on the partner, nor the foreign > key on the invoice... Fabien spoke of v7, and the FK is added by "account_report_company" for reporting purposes. The "invoice_commercial_ids" would be the virtual reverse o2m relationship, but it does not necessarily have to be materialized, as the FK key is only present for reporting purposes. Similarly, the "partner.invoice_ids" fields did not really need to be materialized, it is not used anywhere in official addons as far as I can see. > * Why the hell is it so difficult to make an exception in the release > policy when you break something so badly (mainly F, G & H). The question of making an exception to the release policy is not very important, we can patch DB schema using new modules if we really need it (like what "account_company_report" does), without breaking compatibility with existing 7.0 modules or production deployments. It's a question of doing the right thing for the product, and we think this is the best course both for 7.0 and for the future. We would have done the same thing if those issues had been detected before the 7.0 release (without needing an extra module of course), and we're very sorry that they were not. Thanks a lot for taking the time to thoroughly test and analyze the proposed solution in an open-minded way! We *do* want to be sure we cover all cases as they should be, based on the 7.0 model, while preserving the correct level of granularity everywhere now and in the future.