accounting: support more cases of grouping internal invoices

The logic to guess groupable lines was considering source of
destination side of an accounting transaction depending on
preferences. This was not good, for two reasons:
 - with internal transactions we want to consider both the source
  and the destination
 - if user preferences are mis-configured, grouping would not work.

switch to a logic where we consider both source and destination
sides to prevent these problems.

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountingTransaction_guessGroupedLines.py

@@ -14,8 +14,7 @@ lines_per_node = {}
 portal = context.getPortalObject()
 
 allow_grouping_with_different_quantity = portal.portal_preferences.getPreference(
-                                         'preferred_grouping_with_different_quantities', 0)
-
+  'preferred_grouping_with_different_quantities', 0)
 
 accounting_transaction_line_value_list = []
 if accounting_transaction_line_uid_list is None:
@@ -25,56 +24,71 @@ if accounting_transaction_line_uid_list is None:
                           accounting_transaction.getUid() != context.getUid():
       continue
     for line in accounting_transaction.getMovementList(
-                            portal.getPortalAccountingMovementTypeList()):
+        portal.getPortalAccountingMovementTypeList()):
       if line.getGroupingReference():
         continue
       accounting_transaction_line_value_list.append(line)
 else:
   if accounting_transaction_line_uid_list:
     accounting_transaction_line_value_list = [
-      brain.getObject() for brain in portal.portal_catalog(uid=accounting_transaction_line_uid_list)]
+      brain.getObject() for brain in portal.portal_catalog(
+        uid=accounting_transaction_line_uid_list)
+    ]
+
 
 for line in accounting_transaction_line_value_list:
-  accounting_transaction = line.getParentValue()
-  if accounting_transaction.AccountingTransaction_isSourceView():
-    section_relative_url = None
-    source_section = line.getSourceSectionValue(portal_type='Organisation')
-    if source_section is not None:
-      source_section = \
-        source_section.Organisation_getMappingRelatedOrganisation()
-      section_relative_url = source_section.getRelativeUrl()
+  # source
+  section_relative_url = None
+  source_section = line.getSourceSectionValue(portal_type='Organisation')
+  if source_section is not None:
+    source_section = \
+      source_section.Organisation_getMappingRelatedOrganisation()
+    section_relative_url = source_section.getRelativeUrl()
+  lines_per_node.setdefault(
+      (
+        line.getSource(portal_type='Account'),
+        section_relative_url,
+        line.getDestinationSection(),
+        line.AccountingTransactionLine_getGroupingExtraParameterList(
+          source=True),
+      ), []).append(
+        dict(
+          total_price=line.getSourceInventoriatedTotalAssetPrice() or 0,
+          date=line.getStartDate(),
+          path=line.getRelativeUrl()))
+
+  # destination
+  section_relative_url = None
+  destination_section = line.getDestinationSectionValue(
+    portal_type='Organisation')
+  if destination_section is not None:
+    destination_section = \
+      destination_section.Organisation_getMappingRelatedOrganisation()
+    section_relative_url = destination_section.getRelativeUrl()
+  lines_per_node.setdefault(
+    (
+      line.getDestination(portal_type='Account'),
+      section_relative_url,
+      line.getSourceSection(),
+      line.AccountingTransactionLine_getGroupingExtraParameterList(
+        source=False),
+    ), []).append(
+      dict(
+        total_price=line.getDestinationInventoriatedTotalAssetPrice() or 0,
+        date=line.getStopDate(),
+        path=line.getRelativeUrl()))
 
-    lines_per_node.setdefault(
-                  (line.getSource(portal_type='Account'),
-                   section_relative_url,
-                   line.getDestinationSection(),
-                   line.AccountingTransactionLine_getGroupingExtraParameterList(source=True),
-                   ), []).append(
-      dict(total_price=line.getSourceInventoriatedTotalAssetPrice() or 0,
-           date=line.getStartDate(),
-           path=line.getRelativeUrl()))
-  else:
-    section_relative_url = None
-    destination_section = line.getDestinationSectionValue(
-                                    portal_type='Organisation')
-    if destination_section is not None:
-      destination_section = \
-        destination_section.Organisation_getMappingRelatedOrganisation()
-      section_relative_url = destination_section.getRelativeUrl()
-    lines_per_node.setdefault(
-              (line.getDestination(portal_type='Account'),
-               section_relative_url,
-               line.getSourceSection(),
-               line.AccountingTransactionLine_getGroupingExtraParameterList(source=False),
-               ), []).append(
-    dict(total_price=line.getDestinationInventoriatedTotalAssetPrice() or 0,
-         date=line.getStopDate(),
-         path=line.getRelativeUrl()))
 
-changed_line_list = []
-for (node, section, mirror_section, _), line_info_list in lines_per_node.items():
+changed_line_set = set()
+for (
+    node,
+    section,
+    mirror_section,
+    _,
+), line_info_list in lines_per_node.items():
   if node is None:
     continue
+
   # get the currency rounding for this section, with a fallback that something that would
   # allow grouping in case precision is not defined.
   currency_precision = 5
@@ -87,10 +101,10 @@ for (node, section, mirror_section, _), line_info_list in lines_per_node.items()
     # we should include mirror node in the id_group, but this would reset
     # id generators and generate grouping references that were already used.
     id_group = ('grouping_reference', node, section, mirror_section)
-    previous_default = context.portal_ids.getLastGeneratedId(id_group=id_group, default=0)
-    grouping_reference = portal.portal_ids.generateNewId(id_generator='uid',
-                                                  id_group=id_group,
-                                                  default=previous_default + 1)
+    previous_default = context.portal_ids.getLastGeneratedId(
+      id_group=id_group, default=0)
+    grouping_reference = portal.portal_ids.generateNewId(
+      id_generator='uid', id_group=id_group, default=previous_default + 1)
 
     # convert from int to letters
     string_reference = int2letter(grouping_reference)
@@ -99,11 +113,14 @@ for (node, section, mirror_section, _), line_info_list in lines_per_node.items()
     date = max([line['date'] for line in line_info_list])
 
     for line in line_info_list:
+      if line['path'] in changed_line_set:
+        continue
       line_obj = portal.restrictedTraverse(line['path'])
       assert not line_obj.getGroupingReference(), line
       line_obj.setGroupingReference(string_reference)
       line_obj.setGroupingDate(date)
-      line_obj.reindexObject(activate_kw=dict(tag='accounting_grouping_reference'))
-      changed_line_list.append(line['path'])
+      line_obj.reindexObject(
+        activate_kw=dict(tag='accounting_grouping_reference'))
+      changed_line_set.add(line['path'])
 
-return changed_line_list
+return changed_line_set

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

@@ -6169,6 +6169,153 @@ class TestInternalInvoiceTransaction(AccountingTestCase):
     self.assertEqual(stat_internal_transaction.destination_credit, 1) # line1
     self.assertEqual(stat_internal_transaction.destination_asset_credit, 0.22) # line1
 
+  def test_grouping_reference_both_sides(self):
+    # Group together lines from two internal invoices:
+    #
+    # | Source Account | Debit | Credit | Grouping | Destination Account | Debit | Credit | Grouping |
+    # |----------------|-------|--------|----------|---------------------|-------|--------|----------|
+    # | receivable     | 10    |        | A        |                     |       |        |          |
+    # | sales          |       | 10     |          | purchase            | 10    |        |          |
+    # |                |       |        |          | payable             |       | 10     | B        |
+    # and
+    # | Source Account | Debit | Credit | Grouping | Destination Account | Debit | Credit | Grouping |
+    # |----------------|-------|--------|----------|---------------------|-------|--------|----------|
+    # | sales          | 10    |        |          | purchase            |       | 10     |          |
+    # | receivable     |       | 10     | A        |                     |       |        |          |
+    # |                |       |        |          | payable             | 10    |        | B        |
+    # This example does not really make sense from usage of internal invoices, because we usually
+    # use the same line for receivable and purchase in such a case, but it reproduces a case that did
+    # not group automatically.
+
+    internal_invoice1 = self.portal.accounting_module.newContent(
+      portal_type='Internal Invoice Transaction',
+      title='internal_invoice1',
+      source_section_value=self.section,
+      destination_section_value=self.main_section,
+      start_date=DateTime(2015, 1, 1),
+      created_by_builder=True,
+    )
+    # start before creating lines, because we don't want our lines to
+    # be initialized with mirror accounts.
+    internal_invoice1.start()
+    internal_invoice1.newContent(
+      id='line_a',
+      source_value=self.portal.account_module.receivable,
+      source_debit=10,
+    )
+    internal_invoice1.newContent(
+      source_value=self.portal.account_module.goods_sales,
+      destination_value=self.portal.account_module.goods_purchase,
+      source_credit=10,
+    )
+    internal_invoice1.newContent(
+      id='line_b',
+      destination_value=self.portal.account_module.payable,
+      destination_credit=10,
+    )
+    internal_invoice1.stop()
+    self.tic()
+
+    internal_invoice2 = self.portal.accounting_module.newContent(
+      portal_type='Internal Invoice Transaction',
+      title='internal_invoice2',
+      source_section_value=self.section,
+      destination_section_value=self.main_section,
+      start_date=DateTime(2015, 1, 1),
+      causality_value=internal_invoice1,
+      created_by_builder=True,
+    )
+    internal_invoice2.start()
+    internal_invoice2.newContent(
+      source_value=self.portal.account_module.goods_sales,
+      destination_value=self.portal.account_module.goods_purchase,
+      source_debit=10,
+    )
+    internal_invoice2.newContent(
+      id='line_a',
+      source_value=self.portal.account_module.receivable,
+      source_credit=10,
+    )
+    internal_invoice2.newContent(
+      id='line_b',
+      destination_value=self.portal.account_module.payable,
+      destination_debit=10,
+    )
+    internal_invoice2.stop()
+    self.tic()
+    self.assertTrue(internal_invoice1.line_a.getGroupingReference())
+    self.assertEqual(
+      internal_invoice1.line_a.getGroupingReference(),
+      internal_invoice2.line_a.getGroupingReference(),
+    )
+    self.assertTrue(internal_invoice1.line_b.getGroupingReference())
+    self.assertEqual(
+      internal_invoice1.line_b.getGroupingReference(),
+      internal_invoice2.line_b.getGroupingReference(),
+    )
+
+  def test_grouping_reference_no_group_when_mirror_accounts_are_different(self):
+    # Does not together lines from two internal invoices:
+    #
+    # | Source Account | Debit | Credit | Grouping | Destination Account | Debit | Credit | Grouping |
+    # |----------------|-------|--------|----------|---------------------|-------|--------|----------|
+    # | receivable     | 10    |        |  no  ->  | payable             |       | 10     |          |
+    # | sales          |       | 10     |          | purchase            | 10    |        |          |
+    # and
+    # | Source Account | Debit | Credit | Grouping | Destination Account | Debit | Credit | Grouping |
+    # |----------------|-------|--------|----------|---------------------|-------|--------|----------|
+    # | sales          | 10    |        |          | payable             |       | 10     |          |
+    # | receivable     |       | 10     |  no  ->  | purchase            | 10    |        |          |
+
+    internal_invoice1 = self.portal.accounting_module.newContent(
+      portal_type='Internal Invoice Transaction',
+      title='internal_invoice1',
+      source_section_value=self.section,
+      destination_section_value=self.main_section,
+      start_date=DateTime(2015, 1, 1),
+      created_by_builder=True,
+    )
+    internal_invoice1.start()
+    internal_invoice1.newContent(
+      id='not_grouped',
+      source_value=self.portal.account_module.receivable,
+      destination_value=self.portal.account_module.payable,
+      source_debit=10,
+    )
+    internal_invoice1.newContent(
+      source_value=self.portal.account_module.goods_sales,
+      destination_value=self.portal.account_module.goods_purchase,
+      source_credit=10,
+    )
+    internal_invoice1.stop()
+    self.tic()
+
+    internal_invoice2 = self.portal.accounting_module.newContent(
+      portal_type='Internal Invoice Transaction',
+      title='internal_invoice2',
+      source_section_value=self.section,
+      destination_section_value=self.main_section,
+      start_date=DateTime(2015, 1, 1),
+      causality_value=internal_invoice1,
+      created_by_builder=True,
+    )
+    internal_invoice2.start()
+    internal_invoice2.newContent(
+      source_value=self.portal.account_module.goods_sales,
+      destination_value=self.portal.account_module.payable,
+      source_debit=10,
+    )
+    internal_invoice2.newContent(
+      id='not_grouped',
+      source_value=self.portal.account_module.receivable,
+      destination_value=self.portal.account_module.goods_purchase,
+      source_debit=10,
+    )
+    internal_invoice2.stop()
+    self.tic()
+    self.assertFalse(internal_invoice1.not_grouped.getGroupingReference())
+    self.assertFalse(internal_invoice1.not_grouped.getGroupingReference())
+
 
 class TestAccountingAlarms(AccountingTestCase):
   def test_check_payable_receivable_account_grouped(self):