accounting: fix validation of accounting transactions with too precise amounts

Round the amounts line by line when validating transactions balances and
showing the totals on an accounting transaction.

Even though float field configurations do not allow users to input transactions
where amounts uses more digits that what makes sense for the selected currency,
we had cases where some transactions generated by custom scripts were "too
precise" and passed the validation because unlike in all reports where we
display sum of rounded values, the check was only rounding the sum, which cause
some transactions with 1.00 on one side and 0.333333 0.333333 and 0.333333 on
the other side to be valid.

The scripts used to display sums on an accounting transaction had the same
problem, they were also showing the rounded sum and not the sum of rounded
values.

bt5/erp5_accounting/DocumentTemplateItem/portal_components/document.erp5.AccountingTransactionBalanceConstraint.py

@@ -27,6 +27,8 @@
 #
 ##############################################################################
 
+import collections
+
 from Products.ERP5Type.mixin.constraint import ConstraintMixin
 from Products.ERP5Type import PropertySheet
 
@@ -49,40 +51,38 @@ class AccountingTransactionBalanceConstraint(ConstraintMixin):
     Check the object's consistency
     """
     error_list = []
-    source_sum = {}
-    destination_sum = {}
+    source_sum = collections.defaultdict(list)
+    destination_sum = collections.defaultdict(list)
     for line in obj.getMovementList(
           portal_type=obj.getPortalAccountingMovementTypeList()):
       if line.getSourceValue(portal_type='Account') is not None:
         section = line.getSourceSectionValue()
-        source_sum[section] = source_sum.get(section, 0) + \
-            (line.getSourceInventoriatedTotalAssetPrice() or 0)
+        source_sum[section].append(line.getSourceInventoriatedTotalAssetPrice() or 0)
       if line.getDestinationValue(portal_type='Account') is not None:
         section = line.getDestinationSectionValue()
-        destination_sum[section] = destination_sum.get(section, 0) + \
-          (line.getDestinationInventoriatedTotalAssetPrice() or 0)
+        destination_sum[section].append(line.getDestinationInventoriatedTotalAssetPrice() or 0)
 
-    for section, total in source_sum.items():
+    for section, amount_list in source_sum.items():
       precision = 2
-      if section is not None and\
+      if amount_list and section is not None and\
           section.getPortalType() == 'Organisation':
         section_currency = section.getPriceCurrencyValue()
         if section_currency is not None:
           precision = section_currency.getQuantityPrecision()
-        if round(total, precision) != 0:
+        if round(sum((round(amount, precision) for amount in amount_list)), precision) != 0:
           error_list.append(self._generateError(obj, self._getMessage(
                 'message_transaction_not_balanced_for_source'),
                 mapping=dict(section_title=section.getTranslatedTitle())))
           break
 
-    for section, total in destination_sum.items():
+    for section, amount_list in destination_sum.items():
       precision = 2
-      if section is not None and\
+      if amount_list and section is not None and\
           section.getPortalType() == 'Organisation':
         section_currency = section.getPriceCurrencyValue()
         if section_currency is not None:
           precision = section_currency.getQuantityPrecision()
-        if round(total, precision) != 0:
+        if round(sum((round(amount, precision) for amount in amount_list)), precision) != 0:
           error_list.append(self._generateError(obj, self._getMessage(
                 'message_transaction_not_balanced_for_destination'),
                 mapping=dict(section_title=section.getTranslatedTitle())))

bt5/erp5_accounting/DocumentTemplateItem/portal_components/document.erp5.AccountingTransactionBalanceConstraint.xml

@@ -6,6 +6,12 @@
     </pickle>
     <pickle>
       <dictionary>
+        <item>
+            <key> <string>_recorded_property_dict</string> </key>
+            <value>
+              <persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
+            </value>
+        </item>
         <item>
             <key> <string>default_reference</string> </key>
             <value> <string>AccountingTransactionBalanceConstraint</string> </value>
@@ -14,6 +20,12 @@
             <key> <string>default_source_reference</string> </key>
             <value> <string>Products.ERP5.Document.AccountingTransactionBalanceConstraint</string> </value>
         </item>
+        <item>
+            <key> <string>description</string> </key>
+            <value>
+              <none/>
+            </value>
+        </item>
         <item>
             <key> <string>id</string> </key>
             <value> <string>document.erp5.AccountingTransactionBalanceConstraint</string> </value>
@@ -47,13 +59,28 @@
         <item>
             <key> <string>workflow_history</string> </key>
             <value>
-              <persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
+              <persistent> <string encoding="base64">AAAAAAAAAAM=</string> </persistent>
             </value>
         </item>
       </dictionary>
     </pickle>
   </record>
   <record id="2" aka="AAAAAAAAAAI=">
+    <pickle>
+      <global name="PersistentMapping" module="Persistence.mapping"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>data</string> </key>
+            <value>
+              <dictionary/>
+            </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+  <record id="3" aka="AAAAAAAAAAM=">
     <pickle>
       <global name="PersistentMapping" module="Persistence.mapping"/>
     </pickle>
@@ -66,7 +93,7 @@
                 <item>
                     <key> <string>component_validation_workflow</string> </key>
                     <value>
-                      <persistent> <string encoding="base64">AAAAAAAAAAM=</string> </persistent>
+                      <persistent> <string encoding="base64">AAAAAAAAAAQ=</string> </persistent>
                     </value>
                 </item>
               </dictionary>
@@ -75,7 +102,7 @@
       </dictionary>
     </pickle>
   </record>
-  <record id="3" aka="AAAAAAAAAAM=">
+  <record id="4" aka="AAAAAAAAAAQ=">
     <pickle>
       <global name="WorkflowHistoryList" module="Products.ERP5Type.Workflow"/>
     </pickle>

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransactionLine_statDestinationAssetCredit.py

 total = 0
+precision = 2
+destination_section = context.getDestinationSectionValue(portal_type='Organisation')
+if destination_section is not None:
+  precision = context.getQuantityPrecisionFromResource(destination_section.getPriceCurrency())
+
 for line in context.objectValues(
-        portal_type = context.getPortalAccountingMovementTypeList()) :
-  total += line.getDestinationInventoriatedTotalAssetCredit()
-  
+        portal_type=context.getPortalAccountingMovementTypeList()):
+  total += round(line.getDestinationInventoriatedTotalAssetCredit(), precision)
+
 return total

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransactionLine_statDestinationAssetDebit.py

 total = 0
+precision = 2
+destination_section = context.getDestinationSectionValue(portal_type='Organisation')
+if destination_section is not None:
+  precision = context.getQuantityPrecisionFromResource(destination_section.getPriceCurrency())
+
 for line in context.objectValues(
-          portal_type = context.getPortalAccountingMovementTypeList()) :
-  total += line.getDestinationInventoriatedTotalAssetDebit()
-  
+        portal_type=context.getPortalAccountingMovementTypeList()):
+  total += round(line.getDestinationInventoriatedTotalAssetDebit(), precision)
+
 return total

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransactionLine_statSourceAssetCredit.py

 total = 0
+precision = 2
+source_section = context.getSourceSectionValue(portal_type='Organisation')
+if source_section is not None:
+  precision = context.getQuantityPrecisionFromResource(source_section.getPriceCurrency())
+
 for line in context.objectValues(
-        portal_type = context.getPortalAccountingMovementTypeList()) :
-  total += line.getSourceInventoriatedTotalAssetCredit()
-  
+      portal_type=context.getPortalAccountingMovementTypeList()):
+  total += round(line.getSourceInventoriatedTotalAssetCredit(), precision)
+
 return total

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransactionLine_statSourceAssetDebit.py

 total = 0
+precision = 2
+source_section = context.getSourceSectionValue(portal_type='Organisation')
+if source_section is not None:
+  precision = context.getQuantityPrecisionFromResource(source_section.getPriceCurrency())
+
 for line in context.objectValues(
-      portal_type = context.getPortalAccountingMovementTypeList()) :
-  total += line.getSourceInventoriatedTotalAssetDebit()
-  
+      portal_type = context.getPortalAccountingMovementTypeList()):
+  total += round(line.getSourceInventoriatedTotalAssetDebit(), precision)
+
 return total

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransactionLine_statSourceCredit.py

 total = 0
+precision = context.getQuantityPrecisionFromResource(context.getResource())
 for line in context.objectValues(
-            portal_type = context.getPortalAccountingMovementTypeList()) :
-  total += line.getSourceCredit()
-  
+    portal_type=context.getPortalAccountingMovementTypeList()):
+  total += round(line.getSourceCredit(), precision)
+
 return total

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransactionLine_statSourceDebit.py

 total = 0
+precision = context.getQuantityPrecisionFromResource(context.getResource())
 for line in context.objectValues(
-        portal_type = context.getPortalAccountingMovementTypeList()) :
-  total += line.getSourceDebit()
-  
+    portal_type=context.getPortalAccountingMovementTypeList()):
+  total += round(line.getSourceDebit(), precision)
+
 return total

bt5/erp5_accounting/TestTemplateItem/portal_components/test.erp5.testAccounting.py

@@ -901,6 +901,62 @@ class TestTransactionValidation(AccountingTestCase):
     # but if there are no accounts defined it's not a problem
     self.portal.portal_workflow.doActionFor(accounting_transaction, 'stop_action')
 
+  def test_NonBalancedSourceAccountingTransactionRounding(self):
+    accounting_transaction = self._makeOne(
+               portal_type='Accounting Transaction',
+               start_date=DateTime('2007/01/02'),
+               destination_section_value=self.organisation_module.client_1,
+               destination_value=self.organisation_module.client_1,
+               resource='currency_module/euro',
+               lines=(dict(source_value=self.account_module.payable,
+                           source_debit=1),
+                      dict(source_value=self.account_module.receivable,
+                           source_credit=1/3.0),
+                      dict(source_value=self.account_module.receivable,
+                           source_credit=1/3.0),
+                      dict(source_value=self.account_module.receivable,
+                           source_credit=1/3.0),))
+    self.assertRaisesRegexp(
+        ValidationFailed,
+        'Transaction is not balanced for',
+        self.portal.portal_workflow.doActionFor,
+        accounting_transaction,
+        'stop_action',
+    )
+    self.assertEqual(accounting_transaction.AccountingTransactionLine_statSourceDebit(), 1)
+    self.assertEqual(accounting_transaction.AccountingTransactionLine_statSourceAssetDebit(), 1)
+    self.assertEqual(accounting_transaction.AccountingTransactionLine_statSourceCredit(), 0.99)
+    self.assertEqual(accounting_transaction.AccountingTransactionLine_statSourceAssetCredit(), 0.99)
+
+  def test_NonBalancedDestinationAccountingTransactionRounding(self):
+    accounting_transaction = self._makeOne(
+               portal_type='Accounting Transaction',
+               start_date=DateTime('2007/01/02'),
+               destination_section_value=self.section,
+               destination_value=self.section,
+               source_section_value=self.organisation_module.client_1,
+               source_value=self.organisation_module.client_1,
+               resource='currency_module/euro',
+               lines=(dict(destination_value=self.account_module.payable,
+                           destination_debit=1),
+                      dict(destination_value=self.account_module.receivable,
+                           destination_credit=1/3.0),
+                      dict(destination_value=self.account_module.receivable,
+                           destination_credit=1/3.0),
+                      dict(destination_value=self.account_module.receivable,
+                           destination_credit=1/3.0),))
+    self.assertRaisesRegexp(
+        ValidationFailed,
+        'Transaction is not balanced for',
+        self.portal.portal_workflow.doActionFor,
+        accounting_transaction,
+        'stop_action',
+    )
+    self.assertEqual(accounting_transaction.AccountingTransactionLine_statSourceCredit(), 1)
+    self.assertEqual(accounting_transaction.AccountingTransactionLine_statDestinationAssetDebit(), 1)
+    self.assertEqual(accounting_transaction.AccountingTransactionLine_statSourceDebit(), 0.99)
+    self.assertEqual(accounting_transaction.AccountingTransactionLine_statDestinationAssetCredit(), 0.99)
+
   def test_AccountingTransactionValidationRefusedWithCategoriesAsSections(self):
     # Validating a transaction with categories as sections is refused.
     # See http://wiki.erp5.org/Discussion/AccountingProblems

bt5/erp5_accounting/TestTemplateItem/portal_components/test.erp5.testAccounting.xml

@@ -6,6 +6,12 @@
     </pickle>
     <pickle>
       <dictionary>
+        <item>
+            <key> <string>_recorded_property_dict</string> </key>
+            <value>
+              <persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
+            </value>
+        </item>
         <item>
             <key> <string>default_reference</string> </key>
             <value> <string>testAccounting</string> </value>
@@ -53,13 +59,28 @@
         <item>
             <key> <string>workflow_history</string> </key>
             <value>
-              <persistent> <string encoding="base64">AAAAAAAAAAI=</string> </persistent>
+              <persistent> <string encoding="base64">AAAAAAAAAAM=</string> </persistent>
             </value>
         </item>
       </dictionary>
     </pickle>
   </record>
   <record id="2" aka="AAAAAAAAAAI=">
+    <pickle>
+      <global name="PersistentMapping" module="Persistence.mapping"/>
+    </pickle>
+    <pickle>
+      <dictionary>
+        <item>
+            <key> <string>data</string> </key>
+            <value>
+              <dictionary/>
+            </value>
+        </item>
+      </dictionary>
+    </pickle>
+  </record>
+  <record id="3" aka="AAAAAAAAAAM=">
     <pickle>
       <global name="PersistentMapping" module="Persistence.mapping"/>
     </pickle>
@@ -72,7 +93,7 @@
                 <item>
                     <key> <string>component_validation_workflow</string> </key>
                     <value>
-                      <persistent> <string encoding="base64">AAAAAAAAAAM=</string> </persistent>
+                      <persistent> <string encoding="base64">AAAAAAAAAAQ=</string> </persistent>
                     </value>
                 </item>
               </dictionary>
@@ -81,7 +102,7 @@
       </dictionary>
     </pickle>
   </record>
-  <record id="3" aka="AAAAAAAAAAM=">
+  <record id="4" aka="AAAAAAAAAAQ=">
     <pickle>
       <global name="WorkflowHistoryList" module="Products.ERP5Type.Workflow"/>
     </pickle>

bt5/erp5_accounting/bt/change_log

+2020-09-23
+* Round the amounts line by line when validating transactions balances and showing the totals on an accounting transaction.
+
 2015-06-12 arnaud.fontaine
 * Rounding debit/credit only makes sense if there is SIT Line.