financial_risk modules are not multicompany compatible

[gdgellatly]

credit-control/account_financial_risk/models/res_partner.py

Line 194 in eeb2551

if self.property_account_receivable_id.id == reg['account_id'][0]:

We probably need to capture each receivable account for each company here.

[pedrobaeza]

Yes, multi-company is not alllowed right now, but not easy either as computed field can't be multi-valued depending on the company.

[gdgellatly]

@pedrobaeza for my use case thats a feature not a bug, as it gives a picture of groupwide risk.

In other words, if a company is overdue with one subsidiary, it prevents them just to start ordering from another. In other words I'm not concerned with the numbers being for the company, I'm concerned they are in the right boxes.

If I make a PR that does nothing more than just consider the receivable accounts for all companies what do you think?

[pedrobaeza]

I'm afraid it will mean more than that, as you will need to sudo other searches (at least for global coherence) and it's too much interference for you specific feature. I would say you to inherit the module and use existing hooks or propose new ones if needed.

[gdgellatly]

@pedrobaeza well at the very least the module should take the company_id of the partner into account rather than just a random company don't you agree. As it stands it is useless for multicompany, even if all the partners are assigned a company_id.

[pedrobaeza]

Yes, but you have to take into account as well if company_id = False.

[gdgellatly]

I just reviewed the code, honestly I don't see the issue where any search will break. We are literally talking about a single in clause for a bunch of ints, all the relevant searches are already sudoed. What am I missing?

[pedrobaeza]

I suppose you are only focusing in your specific use, not counting sales part for example. Any way, I let @carlosdauden to give his opinion, as he's the author of this.

[gdgellatly]

I just looked at sale_financial_risk, I want to use that as well, in fact it is the primary use.

That is a bug and a bad one anyway that it doesn't use sudo. The mere act of confirming an order on Company B, can substantially reduce risk in company A.

[pedrobaeza]

As said, module are not multi-company, but my general thought was that it requires a total check and see possible caveats, not only changing that line.

[gdgellatly]

Nah, its nothing IMO, I'll just fork it and run it for a few months and see what happens. Useless to me as it is anyway and I can't see any issues with it.

[pedrobaeza]

OK, if you say so, I haven't dug enough. If you prefer to run it on your side and then tell us back, OK.

[gdgellatly]

@pedrobaeza Well 6 months or whatever later, actually it works great for multicompany if you share a group wide limit. The main issue is at any sort of volume especially in anglosaxon with realtime. Lots and lots of failed transactions due to recomputes on a very broad range of irrelevant transactions but I'm yet to work out how to stop those triggers, but made some performance improvements to at least partially mitigate.

[carlosdauden]

Have you tried this after the improvements made in v11, v12 and v13?
#65
#66

Can you close this issue?

[gdgellatly]

@carlosdauden Due to disagreement about whether this was possible we have long since forked and changed this module. To be honest, it also has some performance issues as well which we needed to fix so we are quite far from this original now. Plus for whatever reason, these modules don't consider locked sale orders even if not invoiced which is an absolute must for us. But maybe looking at it , not storing the fields might fix the performance.

We are doing a v13 migration of this module at moment so we will try and integrate changes and let you know.

[carlosdauden]

The modules with this improvements are already migrated to version 13:
https://github.com/OCA/credit-control/tree/13.0

and there are other improvements in process:
#78