[15.0][IMP] crm_multicompany_reporting_currency #640
Conversation
imlopes
force-pushed
the
15.0-improve-crm_multicompany_reporting_currency
branch
from
6c92cfe to
e79732d
Compare
imlopes
force-pushed
the
15.0-improve-crm_multicompany_reporting_currency
branch
from
e79732d to
1a9b7d1
Compare
| init: function (parent, options, columnState) { | ||
| this._super.apply(this, arguments); | ||
| var companyCurrency = | ||
| columnState.progressBarValues.company_currency_field; | ||
| if (companyCurrency && columnState.data.length) { | ||
| this.currency = | ||
| session.currencies[ | ||
| columnState.data[0].data[companyCurrency].res_id | ||
| ]; | ||
| } | ||
| }, |
There was a problem hiding this comment.
I suppose companyCurrency field existence is to incapsulate the usage only on CRM? Maybe there is more elegant way to check if current active model has such an attribute?
| # Override read_group to calculate the sum of the | ||
| # expected revenue in current company currency | ||
| current_company = self.env.company | ||
| result = super().read_group( |
There was a problem hiding this comment.
If we use read_group to calculate the sum, why do we use search() and sum() then? Can't we try with read_group aggregation?
There was a problem hiding this comment.
I see an inconsistency in the currency usage, as the field defined on crm.lead records is used in the JS code, but the read_group uses the environmental currency. Can you please uniform the behavior?
Also, I see there's already another PR opened by you for fixing crm_salesperson_planner, you should rebase this PR upon that instead of adding another commit to fix the same issue twice.
| @@ -37,6 +38,15 @@ def _get_multicompany_reporting_currency_id(self): | |||
| readonly=True, | |||
| ) | |||
|
|
|||
| # technical field to be used in sum_field options on kanban | |||
| current_company_currency = fields.Many2one( | |||
There was a problem hiding this comment.
Many2one fields should conventionally end up with _id
| current_company_currency = fields.Many2one( | |
| current_company_currency_id = fields.Many2one( |
| "res.currency", compute="_compute_current_company_currency", readonly=True | ||
| ) | ||
|
|
||
| def _compute_current_company_currency(self): |
There was a problem hiding this comment.
I would add @api.depends_context("company")
| if "__domain" in line: | ||
| opportunities = self.search(line["__domain"]) |
There was a problem hiding this comment.
If a line doesn't define the __domain keyword, then it will use the opportunities of the previous line, which is wrong. Please fix this.
imlopes
force-pushed
the
15.0-improve-crm_multicompany_reporting_currency
branch
from
6c7e75c to
1a9b7d1
Compare
|
Closing this PR as finally I think this module do almost the same things (with few differences) https://github.com/OCA/crm/tree/14.0/crm_lead_currency, which is available on 14.0, 16.0 and 17.0 |
If we use more than one currency on
crm.lead, odoo will sum theexpected_revenueignoring the currency :USD 50,00 + EUR 50,00 = USD 100,00 (odoo takes the first record on kanban view)
With this update, we will use the
current_company_currency:Company currency = BRL
(1 USD = 1.5 BRL and 1 EUR = 2 BRL)
USD 50,00 = 75,00 BRL
EUR 50,00 = 100,00 BRL
We display on kanban progress bar
BRL 175,00The second commit is the same than here: #642