Accounting: better selection of bank account on payment transaction

This includes two parts, first is to change the condition for an organisation to be able to use the bank accounts from organisations from parent groups. This is useful for the case where organisations from subgroups does not represent different legal entities but simply some "business units". 

The condition for that have always been: if organisation does not contain bank account, this organisation can use bank accounts from the parent groups.

To be consistent with other places, such a closing accounting periods or setting grouping references, change the condition to: if the organisation does not contain accounting periods, this organisation can use bank accounts from parent organisations.

The behavior should not change much and allow to create bank accounts or cash registers that can be used only in one sub-group organisation.


Second part is about making it visually easier to understand which organisation the bank account belong to by adding some non-selectable options in the select for the organisations. This is only enabled when available bank accounts are not all contained in the same organisation.

![Screenshot_2017-06-19_at_15.24.35](/uploads/dc538645bb79a00f25444547d524635f/Screenshot_2017-06-19_at_15.24.35.png)

/reviewed-on nexedi/erp5!299

bt5/erp5_accounting/SkinTemplateItem/portal_skins/erp5_accounting/AccountModule_getBankAccountItemList.py

 """Returns an item list of the acceptable bank accounts.
 If `organisation` is passed, then we only show bank accounts available for that
 organisation, using the following policy:
- - if organisation contains bank accounts directly, only those bank accounts
-   can be selected
+ - if organisation is independant accounting entity (ie. have accounting periods),
+   only bank accounts from this organisation can be selected
+ - otherwise, bank accounts from this organisation and all organisation directly
+   members of the parent groups can be us
  - if organisation higher in the group hierarchy contains bank accounts, bank
    accounts from parent organisations can be selected
- - it means a higher in the group cannot use bank account from organisations
-   below, maybe we'll want to change this ...
 
 If organisation is not passed, this script will return all bank accounts
 applicable for section_category and section_category_strict_membership.
@@ -20,11 +20,12 @@ if skip_invalidated_bank_accounts:
 if organisation:
   organisation_value = portal.restrictedTraverse(organisation)
 
-  # if organisation contains bank accounts, only take into account those.
-  bank_account_list = organisation_value.searchFolder(**search_kw)
-
-    # else we lookup in parent organisations
-  if not bank_account_list:
+  # if organisation is an independant accounting section and contains bank accounts,
+  # only take into account those.
+  if organisation_value == organisation_value.Organisation_getMappingRelatedOrganisation():
+    bank_account_list = organisation_value.searchFolder(**search_kw)
+  # else we lookup in organisations from parent groups.
+  else:
     group_value = organisation_value.getGroupValue(None)
     if group_value is not None:
       uid_list = []
@@ -47,19 +48,36 @@ else:
 
 
 item_list = [('', '')]
-for bank in bank_account_list:
-  bank = bank.getObject()
-     
+
+
+# If we have bank accounts from more than one organisation, include
+# the organisation as hierarchy to show which organisation the bank
+# account belongs to.
+include_organisation_hierarchy = len(set(
+  ['/'.join(b.path.split('/')[:-1]) for b in bank_account_list])) > 1
+
+previous_organisation = None
+# sort bank accounts in a way that bank accounts from the same
+# organisation are consecutive
+for brain in sorted(bank_account_list, key=lambda b:b.path):
+  bank = brain.getObject()
+  if include_organisation_hierarchy:
+    organisation = bank.getParentValue()
+    if organisation != previous_organisation:
+      previous_organisation = organisation
+      # include non-selectable element to show hierarchy
+      item_list.append((organisation.getTranslatedTitle(), None))
+
   if bank.getReference() and bank.getTitle() \
                   and bank.getReference() != bank.getTitle():
     item_list.append(('%s - %s' % ( bank.getReference(),
-                                    bank.getTitle() or 
+                                    bank.getTitle() or
                                     bank.getSourceFreeText() or
                                     bank.getSourceTitle()),
                                     bank.getRelativeUrl()))
   else:
     item_list.append(( bank.getReference() or
-                       bank.getTitle() or 
+                       bank.getTitle() or
                        bank.getSourceFreeText() or
                        bank.getSourceTitle(),
                        bank.getRelativeUrl() ))

product/ERP5/tests/testAccounting.py

@@ -4332,6 +4332,11 @@ class TestTransactions(AccountingTestCase):
       portal_type='Accounting Period',
     )
     main_section_accounting_period.start()
+    bank_account = self.section.newContent(
+      portal_type='Bank Account',
+      reference='from section'
+    )
+    bank_account.validate()
     self.tic()
 
     source_transaction = self._makeOne(
@@ -4345,6 +4350,20 @@ class TestTransactions(AccountingTestCase):
     self.assertIn(
       ('from main section', parent_bank_account.getRelativeUrl()),
       source_transaction.AccountingTransaction_getSourcePaymentItemList())
+    self.assertIn(
+      ('from section', bank_account.getRelativeUrl()),
+      source_transaction.AccountingTransaction_getSourcePaymentItemList())
+
+    # We include non selectable elements in the drop down to show which to which organisation
+    # the bank account belongs to.
+    self.assertEqual(
+      [('', ''),
+       (self.main_section.getTitle(), None),
+       ('from main section', parent_bank_account.getRelativeUrl()),
+       (self.section.getTitle(), None),
+       ('from section', bank_account.getRelativeUrl()),
+      ],
+      source_transaction.AccountingTransaction_getSourcePaymentItemList())
 
     destination_transaction = self._makeOne(
       portal_type='Payment Transaction',
@@ -4357,6 +4376,88 @@ class TestTransactions(AccountingTestCase):
     self.assertIn(
       ('from main section', parent_bank_account.getRelativeUrl()),
       destination_transaction.AccountingTransaction_getDestinationPaymentItemList())
+    self.assertIn(
+      ('from section', bank_account.getRelativeUrl()),
+      destination_transaction.AccountingTransaction_getDestinationPaymentItemList())
+
+    main_section_transaction = self._makeOne(
+      portal_type='Payment Transaction',
+      source_section_value=self.main_section,
+      destination_section_value=self.organisation_module.client_1,
+      lines=(dict(source_value=self.account_module.goods_purchase,
+                  source_debit=500),
+             dict(source_value=self.account_module.receivable,
+                  source_credit=500)))
+    self.assertIn(
+      ('from main section', parent_bank_account.getRelativeUrl()),
+      main_section_transaction.AccountingTransaction_getSourcePaymentItemList())
+    self.assertNotIn(
+      ('from section', bank_account.getRelativeUrl()),
+      main_section_transaction.AccountingTransaction_getSourcePaymentItemList())
+
+    # We don't have this non selectable element when all bank accounts are from
+    # the same organisation
+    self.assertEqual(
+      [('', ''),
+       ('from main section', parent_bank_account.getRelativeUrl()),
+      ],
+      main_section_transaction.AccountingTransaction_getSourcePaymentItemList())
+
+  def test_AccountingTransaction_getSourcePaymentItemList_parent_section_with_accounting_period(self):
+    # AccountingTransaction_getSourcePaymentItemList and AccountingTransaction_getDestinationPaymentItemList
+    # allows to select bank accounts from parent groups of source section, but not if
+    # the organisation has accounting periods, in this case it acts as an independant section.
+    parent_bank_account = self.main_section.newContent(
+      portal_type='Bank Account',
+      reference='from main section'
+    )
+    parent_bank_account.validate()
+    main_section_accounting_period = self.main_section.newContent(
+      portal_type='Accounting Period',
+    )
+    main_section_accounting_period.start()
+    bank_account = self.section.newContent(
+      portal_type='Bank Account',
+      reference='from section'
+    )
+    bank_account.validate()
+    # open an accounting periods in this section, it will act as an independant section
+    # and will not allow bank accounts from parent sections.
+    section_accounting_period = self.section.newContent(
+      portal_type='Accounting Period',
+    )
+    section_accounting_period.start()
+    self.tic()
+
+    source_transaction = self._makeOne(
+      portal_type='Payment Transaction',
+      source_section_value=self.section,
+      destination_section_value=self.organisation_module.client_1,
+      lines=(dict(source_value=self.account_module.goods_purchase,
+                  source_debit=500),
+             dict(source_value=self.account_module.receivable,
+                  source_credit=500)))
+    self.assertNotIn(
+      ('from main section', parent_bank_account.getRelativeUrl()),
+      source_transaction.AccountingTransaction_getSourcePaymentItemList())
+    self.assertIn(
+      ('from section', bank_account.getRelativeUrl()),
+      source_transaction.AccountingTransaction_getSourcePaymentItemList())
+
+    destination_transaction = self._makeOne(
+      portal_type='Payment Transaction',
+      destination_section_value=self.section,
+      source_section_value=self.organisation_module.client_1,
+      lines=(dict(destination_value=self.account_module.goods_purchase,
+                  destination_debit=500),
+             dict(destination_value=self.account_module.receivable,
+                  destination_credit=500)))
+    self.assertNotIn(
+      ('from main section', parent_bank_account.getRelativeUrl()),
+      destination_transaction.AccountingTransaction_getDestinationPaymentItemList())
+    self.assertIn(
+      ('from section', bank_account.getRelativeUrl()),
+      destination_transaction.AccountingTransaction_getDestinationPaymentItemList())
 
 
 class TestAccountingWithSequences(ERP5TypeTestCase):