diff --git a/addons/mrp_account/models/mrp_bom.py b/addons/mrp_account/models/mrp_bom.py index 7f23d2d1774..21f64285b25 100644 --- a/addons/mrp_account/models/mrp_bom.py +++ b/addons/mrp_account/models/mrp_bom.py @@ -27,7 +27,10 @@ class MrpBom(models.Model): @api.depends('analytic_distribution') def _compute_analytic_account_ids(self): for record in self: - record.analytic_account_ids = list(map(int, record.analytic_distribution.keys())) if record.analytic_distribution else [] + if not record.analytic_distribution: + record.analytic_account_ids = [] + else: + record.analytic_account_ids = list({int(ad_id) for ids in record.analytic_distribution for ad_id in ids.split(",")}) @api.onchange('product_id') def _onchange_analytic_distribution(self): diff --git a/addons/mrp_account/models/mrp_production.py b/addons/mrp_account/models/mrp_production.py index aac8515ff8e..5e38d049098 100644 --- a/addons/mrp_account/models/mrp_production.py +++ b/addons/mrp_account/models/mrp_production.py @@ -33,8 +33,10 @@ class MrpProduction(models.Model): @api.depends('analytic_distribution') def _compute_analytic_account_ids(self): for record in self: - record.analytic_account_ids = list( - map(int, record.analytic_distribution.keys())) if record.analytic_distribution else [] + if not record.analytic_distribution: + record.analytic_account_ids = [] + else: + record.analytic_account_ids = list({int(ad_id) for ids in record.analytic_distribution for ad_id in ids.split(",")}) @api.constrains('analytic_distribution') def _check_analytic(self): diff --git a/addons/mrp_account/models/mrp_workorder.py b/addons/mrp_account/models/mrp_workorder.py index d313dfc3aa5..79b998679ca 100644 --- a/addons/mrp_account/models/mrp_workorder.py +++ b/addons/mrp_account/models/mrp_workorder.py @@ -24,12 +24,12 @@ class MrpWorkorder(models.Model): (self.mo_analytic_account_line_ids | self.wc_analytic_account_line_ids).unlink() return super().action_cancel() - def _prepare_analytic_line_values(self, account, amount, unit_amount): + def _prepare_analytic_line_values(self, account_field_values, amount, unit_amount): self.ensure_one() return { 'name': _("[WC] %s", self.display_name), 'amount': amount, - 'auto_account_id': account, + **account_field_values, 'unit_amount': unit_amount, 'product_id': self.product_id.id, 'product_uom_id': self.env.ref('uom.product_uom_hour').id, diff --git a/addons/mrp_account/tests/test_analytic_account.py b/addons/mrp_account/tests/test_analytic_account.py index 7bfd9bbfa04..3ab6e3559ce 100644 --- a/addons/mrp_account/tests/test_analytic_account.py +++ b/addons/mrp_account/tests/test_analytic_account.py @@ -298,3 +298,97 @@ class TestAnalyticAccount(TestMrpAnalyticAccount): mo.move_raw_ids[0].quantity = 0 self.assertEqual(analytic_account.debit, 0) self.assertFalse(analytic_account.line_ids) + + def test_cross_analytics(self): + """ Test analytic distributions (AD) with cross analytics on an MO.""" + + ap1 = self.env['account.analytic.plan'].create({ + 'name': 'Plan 1', + }) + ap2 = self.env['account.analytic.plan'].create({ + 'name': 'Plan 2', + }) + ac1A = self.env['account.analytic.account'].create({ + 'name': 'test_cross_analytics plan 1 account A', + 'plan_id': ap1.id, + }) + ac1B = self.env['account.analytic.account'].create({ + 'name': 'test_cross_analytics plan 1 account B', + 'plan_id': ap1.id, + }) + ac1C = self.env['account.analytic.account'].create({ + 'name': 'test_cross_analytics plan 1 account C', + 'plan_id': ap1.id, + }) + ac2A = self.env['account.analytic.account'].create({ + 'name': 'test_cross_analytics plan 2 account A', + 'plan_id': ap2.id, + }) + ac2B = self.env['account.analytic.account'].create({ + 'name': 'test_cross_analytics plan 2 account B', + 'plan_id': ap2.id, + }) + ac2C = self.env['account.analytic.account'].create({ + 'name': 'test_cross_analytics plan 2 account C', + 'plan_id': ap2.id, + }) + + analytic_distribution = { + f"{ac1A.id},{ac2A.id}": 33.33, + f"{ac1B.id},{ac2B.id}": 33.33, + f"{ac1C.id},{ac2C.id}": 33.33, + } + + # create a mo + mo_form = Form(self.env['mrp.production']) + mo_form.product_id = self.product + mo_form.bom_id = self.bom + mo_form.product_qty = 10.0 + mo_form.analytic_distribution = analytic_distribution + mo = mo_form.save() + mo.action_confirm() + self.assertEqual(mo.state, 'confirmed') + self.assertEqual(len(mo.move_raw_ids.analytic_account_line_ids), 0) + # increase qty_producing to 5.0 + mo_form = Form(mo) + mo_form.qty_producing = 5.0 + mo_form.save() + self.assertEqual(mo.state, 'progress') + aals = mo.move_raw_ids.analytic_account_line_ids + self.assertEqual(len(aals), 3) + self.assertEqual(sum(aals.mapped('amount')), -49.99) + + # increase qty_producing to 10.0 + mo_form = Form(mo) + mo_form.qty_producing = 10.0 + mo_form.save() + mo.workorder_ids.button_finish() + aals = mo.move_raw_ids.analytic_account_line_ids + + self.assertEqual(mo.state, 'to_close') + self.assertEqual(len(aals), 3) + self.assertEqual(sum(aals.mapped('amount')), -99.99) + + # mark as done + mo.button_mark_done() + aals = mo.move_raw_ids.analytic_account_line_ids + self.assertEqual(mo.state, 'done') + self.assertEqual(len(aals), 3) + self.assertEqual(sum(aals.mapped('amount')), -99.99) + + # assert the right accounts are on the right analytic lines + ap1_column = ap1._column_name() + ap2_column = ap2._column_name() + line_with_A_accounts = aals.filtered_domain([(ap1_column, '=', ac1A.id)]) + line_with_B_accounts = aals.filtered_domain([(ap1_column, '=', ac1B.id)]) + line_with_C_accounts = aals.filtered_domain([(ap1_column, '=', ac1C.id)]) + self.assertEqual(line_with_A_accounts[ap2_column], ac2A) + self.assertEqual(line_with_B_accounts[ap2_column], ac2B) + self.assertEqual(line_with_C_accounts[ap2_column], ac2C) + + # ensure the right amounts are on the right lines + # the distribution calculation corrects for rounding error accumulation to ensure + # the total sum is correct, so individual lines can have minor deviations + self.assertAlmostEqual(line_with_A_accounts.amount, -100/3, delta=0.02) + self.assertAlmostEqual(line_with_B_accounts.amount, -100/3, delta=0.02) + self.assertAlmostEqual(line_with_B_accounts.amount, -100/3, delta=0.02) diff --git a/addons/stock_account/models/analytic_account.py b/addons/stock_account/models/analytic_account.py index 66fe5d62978..85438fd116d 100644 --- a/addons/stock_account/models/analytic_account.py +++ b/addons/stock_account/models/analytic_account.py @@ -2,22 +2,33 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. from odoo import models -from odoo.tools import float_compare, float_is_zero +from odoo.tools import float_compare, float_is_zero, float_round class AccountAnalyticPlan(models.Model): _inherit = 'account.analytic.plan' - def _calculate_distribution_amount(self, amount, percentage, distribution_on_each_plan): - existing_amount = distribution_on_each_plan.get(self, 0) - distribution_plan = existing_amount + percentage + def _calculate_distribution_amount(self, amount, percentage, total_percentage, distribution_on_each_plan): + """ + Ensures that the total amount distributed across all lines always adds up to exactly `amount` per + plan. We try to correct for compounding rounding errors by assigning the exact outstanding amount when + we detect that a line will close out a plan's total percentage. However, since multiple plans can be + assigned to a line, with different prior distributions, there is the possible edge case that one line + closes out two (or more) tallies with different compounding errors. This means there is no one correct + amount that we can assign to a line that will correctly close out both all plans. This is described in + more detail in the commit message, under "concurrent closing line edge case". + """ decimal_precision = self.env['decimal.precision'].precision_get('Percentage Analytic') - if float_compare(distribution_plan, 100, precision_digits=decimal_precision) == 0: - distributed_amount = amount * (100 - existing_amount) / 100.0 + distributed_percentage, distributed_amount = distribution_on_each_plan.get(self, (0, 0)) + allocated_percentage = distributed_percentage + percentage + if float_compare(allocated_percentage, total_percentage, precision_digits=decimal_precision) == 0: + calculated_amount = (amount * total_percentage / 100) - distributed_amount else: - distributed_amount = amount * percentage / 100.0 - distribution_on_each_plan[self] = distribution_plan - return distributed_amount + calculated_amount = amount * percentage / 100 + distributed_amount += float_round(calculated_amount, precision_digits=decimal_precision) + distribution_on_each_plan[self] = (allocated_percentage, distributed_amount) + return calculated_amount + class AccountAnalyticAccount(models.Model): _inherit = 'account.analytic.account' @@ -41,37 +52,60 @@ class AccountAnalyticAccount(models.Model): :returns: a list of dicts containing the values for new analytic lines that need to be created :rtype: dict """ + if not distribution: + lines.unlink() + return [] + + # Does this: {'15': 40, '14,16': 60} -> { account(15): 40, account(14,16): 60 } + distribution = { + self.env['account.analytic.account'].browse(map(int, ids.split(','))) : percentage + for ids, percentage in distribution.items() + } + + plans = self.env['account.analytic.plan'] + plans = sum(plans._get_all_plans(), plans) + line_columns = [p._column_name() for p in plans] + + lines_to_link = [] distribution_on_each_plan = {} - processed_distributions = [] + total_percentages = {} + + for accounts, percentage in distribution.items(): + for plan in accounts.root_plan_id: + total_percentages[plan] = total_percentages.get(plan, 0) + percentage + for existing_aal in lines: - if distribution and existing_aal.account_id.id in [int(i) for i in distribution]: + # TODO: recommend something better for this line in review, please + accounts = sum(map(existing_aal.mapped, line_columns), self.env['account.analytic.account']) + if accounts in distribution: # Update the existing AAL for this account - percentage = distribution[str(existing_aal.account_id.id)] - new_amount = existing_aal.account_id.root_plan_id._calculate_distribution_amount(amount, percentage, distribution_on_each_plan) - new_unit_amount = amount + percentage = distribution[accounts] + new_amount = 0 + new_unit_amount = unit_amount + for account in accounts: + plan = account.root_plan_id + new_amount = plan._calculate_distribution_amount(amount, percentage, total_percentages[plan], distribution_on_each_plan) if additive: new_amount += existing_aal.amount new_unit_amount += existing_aal.unit_amount - currency = existing_aal.account_id.currency_id or obj.company_id.currency_id + currency = accounts[0].currency_id or obj.company_id.currency_id if float_is_zero(new_amount, precision_rounding=currency.rounding): existing_aal.unlink() else: existing_aal.amount = new_amount existing_aal.unit_amount = new_unit_amount - processed_distributions.append(existing_aal.account_id.id) + # Prevent this distribution from being applied again + del distribution[accounts] else: # Delete the existing AAL if it is no longer present in the new distribution existing_aal.unlink() - lines_to_link = [] - if not distribution: - return [] - for account_id, percentage in distribution.items(): - # Only create a new AAL if an existing one was not already modified before - account_id = int(account_id) - if account_id not in processed_distributions: - account = self.browse(account_id) - new_amount = account.root_plan_id._calculate_distribution_amount(amount, percentage, distribution_on_each_plan) - currency = account.currency_id or obj.company_id.currency_id - if not float_is_zero(new_amount, precision_rounding=currency.rounding): - lines_to_link.append(obj._prepare_analytic_line_values(account.id, new_amount, unit_amount)) + # Create new lines from remaining distributions + for accounts, percentage in distribution.items(): + account_field_values = {} + for account in accounts: + new_amount = account.root_plan_id._calculate_distribution_amount(amount, percentage, total_percentages[plan], distribution_on_each_plan) + account_field_values[account.plan_id._column_name()] = account.id + currency = account.currency_id or obj.company_id.currency_id + if not float_is_zero(new_amount, precision_rounding=currency.rounding): + lines_to_link.append(obj._prepare_analytic_line_values(account_field_values, new_amount, unit_amount)) return lines_to_link diff --git a/addons/stock_account/models/stock_move.py b/addons/stock_account/models/stock_move.py index 2ea4ad378df..c906ecbf54e 100644 --- a/addons/stock_account/models/stock_move.py +++ b/addons/stock_account/models/stock_move.py @@ -446,12 +446,12 @@ class StockMove(models.Model): def _ignore_automatic_valuation(self): return False - def _prepare_analytic_line_values(self, account, amount, unit_amount): + def _prepare_analytic_line_values(self, account_field_values, amount, unit_amount): self.ensure_one() return { 'name': self.name, 'amount': amount, - 'auto_account_id': account, + **account_field_values, 'unit_amount': unit_amount, 'product_id': self.product_id.id, 'product_uom_id': self.product_id.uom_id.id,