[IMP] {mrp,stock}_account: cross analytics support

A recent feature in accounting (odoo#139225) introduces cross analytics,
which allows submitting multiple accounts per line (one account per
plan). This is not done, as would be assumed, by changing the
`account_id` field from a Many2One to a Many2Many, but by adding fields
to `account.analytic.line` every time a new root analytic plan is
created.

This approach has certain advantages, but a particular disadvantage is
that it becomes harder to update existing lines. For MRP, updating
rather unlink-recreating lines is important for employee time tracking,
where the analytic line will be updated every time the time worked is
updated, and the employee's wage can change between updates. In this
edge case, the accumulated cost must remain calculated at the old wage,
and newer costs must accumulate at the new, actual wage.

Concurrent closing line edge case
================================
An analytic distribution assigns divides an expense into smaller
expenses for analytic accounting. Typically, every fractional expense is
credited to a different analytic account, and they total to 100% of the
original expense. With multiple analytic plans, dividing that expense
can happen different ways. For example, one analytic plan may track
expenses per department, while another keeps track of expenses per
region. This means that an analytic distribution may contain accounts
belonging to different plans, with every plan dividing the original
expense in different ways across its own accounts. For cross accounting,
fractional expenses can be assigned simultaneously (i.e. on one line) to
multiple accounts (up to one per analytic plan). In our example, this
would mean that this fraction was spent by a specific department in a
specific region.

For an analytic distribution to be accurate, we must ensure that for
every plan, the sum of all accounts come out to the total percentage
(usually 100%) that was distributed in the first place. Because
multiplying large numbers with small percentages can introduce rounding
errors, we need to correct for any discrepancies. With one account per
line, this is easy; when a line closes out one of the plans in an
analytic distribution, we assign the exact outstanding amount for that
plan to that line, ensuring that the sum across all accounts adds up to
the exact total percentage of the expense that was divided in the first
place. Since every plan closes out on a different line, we can apply
this correction every time and the amounts are always correct across the
entire analytic distribution.

The problem with cross analytics, and thus multiple accounts per line,
is that a line can now sometimes close out two different plans, with
slightly different outstanding amounts. This means we can no longer
guarantee that every plan in an analytic distribution sums out to the
original expense. Luckily, only certain analytic distributions are
vulnerable to this edge case: they must distribute across multiple
plans, and must have at least one line where not all participating plans
are credited. There exists no trivial way to fix this. The current
implementation follows the approach of the accounting module, which
takes no special precautions against this edge case.

Part-of: odoo#146114

Task-3621105

Part-of: odoo/odoo#146114

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):

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):

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,

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)

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

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,