Commit 9f3fa956 authored by Jérome Perrin's avatar Jérome Perrin

Merge !495 Fix bank reconciliation with internal transaction

In internal transactions, we can have a payment line where source is using a bank account and destination is using another bank account.

Bank Reconciliation did not support these lines properly, because it simply assumed that *"if there's a bank reconciliation on this line, it means the line is reconciled"*, but this was incorrect, because source and destination needs to be able to reconcile this line independently.

The fixes is to change the rule from *"if there's a bank reconciliation on this line, it means the line is reconciled"* to *"if there's a bank reconciliation with the same bank account as this line's source payment, it means the line is reconciled for source"* and *"if there's a bank reconciliation with the same bank account as this line's destination payment, it means the line is reconciled for destination"*.

The related  key used to select the already reconciled lines or lines to reconcile becomes a bit more complex, it joins with category table to ensure the bank reconciliation's bank account  (which is `source_payment` on the bank reconciliation) is the same as the `section_uid` of the line in stock table that we are considering. This also has a side effect that this related key only work with queries on stock table, but it was never intended to be used elsewhere.

Another problem is that queries made by bank reconciliation module become a bit more slower. For sites where this is a problem, the recommended approach is to replace the related keys by columns on stock table. In cfec8d2b (which is part of this MR) we add indexation methods. In 5afb002090cd965bd17ffcf5c4c54ba73dfcbfc4 (which is not part of this MR and not planned for inclusion in "default" ERP5) we show an example of how to enable these methods to actually add the columns in stock table.

/reviewed-on nexedi/erp5!495
parents a1d47205 f29209e9
......@@ -6,4 +6,15 @@ AND <dtml-var table_0>.base_category_uid = <dtml-var "portal_categories.aggregat
<dtml-var table_2>.uid = <dtml-var table_1>.uid
</dtml-if>
<dtml-var RELATED_QUERY_SEPARATOR>
<dtml-var table_0>.uid = <dtml-var query_table>.uid
\ No newline at end of file
<dtml-var table_0>.uid = <dtml-var query_table>.uid
-- A line can be reconcilied for source_payment or destination_payment
-- so we also add a condition that the related bank reconciliation is
-- for "this side". Another approach, more efficient, is to customize `stock`
-- table to add `aggregate_bank_reconciliation_date` and `aggregate_bank_reconciliation_uid`
-- columns. See https://lab.nexedi.com/nexedi/erp5/merge_requests/495
AND stock.payment_uid in (
select category.category_uid
from category
where <dtml-var table_1>.uid = category.uid
and category.base_category_uid = <dtml-var "portal_categories.source_payment.getUid()"> )
"""Set the aggregate relation to a payment request on this line.
"""Adds the bank reconciliation to the already aggregated bank reconciliation on this line.
This script has a proxy role to be able to modify delivered lines
"""
......@@ -6,7 +6,10 @@ portal = context.getPortalObject()
assert context.getPortalType() in portal.getPortalAccountingMovementTypeList()
context.setAggregateValue(payment_request_value, portal_type='Bank Reconciliation')
context.setAggregateSet(
context.getAggregateList(portal_type='Bank Reconciliation') + [bank_reconciliation_relative_url],
portal_type='Bank Reconciliation')
# for traceability
portal.portal_workflow.doActionFor(
context.getParentValue(),
......
......@@ -50,7 +50,7 @@
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>payment_request_value, message</string> </value>
<value> <string>bank_reconciliation_relative_url, message</string> </value>
</item>
<item>
<key> <string>_proxy_roles</string> </key>
......@@ -62,7 +62,7 @@
</item>
<item>
<key> <string>id</string> </key>
<value> <string>AccountingTransactionLine_setBankReconciliation</string> </value>
<value> <string>AccountingTransactionLine_addBankReconciliation</string> </value>
</item>
</dictionary>
</pickle>
......
"""Removes the bank reconciliation from the already aggregated bank reconciliation on this line.
This script has a proxy role to be able to modify delivered lines
"""
portal = context.getPortalObject()
assert context.getPortalType() in portal.getPortalAccountingMovementTypeList()
bank_reconciliation_list = context.getAggregateList(portal_type='Bank Reconciliation')
bank_reconciliation_list.remove(bank_reconciliation_relative_url)
context.setAggregateList(
bank_reconciliation_list,
portal_type='Bank Reconciliation')
# for traceability
portal.portal_workflow.doActionFor(
context.getParentValue(),
"edit_action",
comment=message)
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string>bank_reconciliation_relative_url, message</string> </value>
</item>
<item>
<key> <string>_proxy_roles</string> </key>
<value>
<tuple>
<string>Manager</string>
</tuple>
</value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>AccountingTransactionLine_removeBankReconciliation</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
......@@ -5,21 +5,37 @@ portal = context.getPortalObject()
portal.portal_selections.updateSelectionCheckedUidList(list_selection_name, listbox_uid, uids)
selection_uid_list = portal.portal_selections.getSelectionCheckedUidsFor(list_selection_name)
reconciled_bank_account = context.getSourcePayment()
if mode == 'reconcile':
for line in portal.portal_catalog(uid=selection_uid_list or -1):
line = line.getObject()
if line.getAggregate(portal_type='Bank Reconciliation'):
return context.Base_redirect(dialog_id,
abort_transaction=True,
keep_items={'portal_status_message': translateString("Line Already Reconciled"),
'reset': 1,
'cancel_url': cancel_url,
'mode': mode,
'field_your_mode': mode})
line.AccountingTransactionLine_setBankReconciliation(context,
message=translateString("Reconciling Bank Line"))
# Sanity check: line should not already be reconciled.
# But what can happen is that this line is an internal transaction line that was
# reconciled for payment on one side but not yet on the other side (ex. reconciled
# for the bank account used as source_payment, not not bank account used at
# destination_payment). So we can accept if the line is already reconciled with a
# bank reconciliation, if that bank reconciliation is using another bank account
# that the one on this bank reconciliation.
# To prevent unauthorized errors, we only consider bank reconciliation users can access.
for existing_bank_reconciliation in line.getAggregateValueList(
portal_type='Bank Reconciliation',
checked_permission='Access contents information'):
if existing_bank_reconciliation.getSourcePayment() == reconciled_bank_account:
return context.Base_redirect(
dialog_id,
abort_transaction=True,
keep_items={
'portal_status_message': translateString("Line Already Reconciled"),
'reset': 1,
'cancel_url': cancel_url,
'mode': mode,
'field_your_mode': mode})
line.AccountingTransactionLine_addBankReconciliation(
context.getRelativeUrl(),
message=translateString("Reconciling Bank Line"))
return context.Base_redirect(dialog_id, keep_items={
'portal_status_message': translateString("Line Reconciled"),
'portal_status_message': translateString("Lines Reconciled"),
'reset': 1,
'cancel_url': cancel_url,
'field_your_mode': mode,
......@@ -29,8 +45,9 @@ if mode == 'reconcile':
assert mode == 'unreconcile'
for line in portal.portal_catalog(uid=selection_uid_list or -1):
line = line.getObject()
line.AccountingTransactionLine_setBankReconciliation(None,
message=translateString("Reconciling Bank Line"))
line.AccountingTransactionLine_removeBankReconciliation(
context.getRelativeUrl(),
message=translateString("Reconciling Bank Line"))
return context.Base_redirect(dialog_id, keep_items={
'portal_status_message': translateString("Lines Unreconciled"),
......
from Products.ERP5Type.Message import translateString
context.getPortalObject().setPlacelessDefaultReindexParameters(activate_kw=dict(tag=tag))
bank_reconciliation_relative_url = context.getRelativeUrl()
# XXX maybe we want to distribute this more.
# At the moment we cannot use searchAndActivate from Inventory API query
for line in context.BankReconciliation_getAccountingTransactionLineList():
line.AccountingTransactionLine_setBankReconciliation(context,
line.AccountingTransactionLine_addBankReconciliation(
bank_reconciliation_relative_url,
message=translateString("Select Non Reconciled Transactions"))
for bank_reconciliation in context.getAggregateValueList(portal_type='Bank Reconciliation'):
if bank_reconciliation.getSourcePayment() == context.getDestinationPayment():
return bank_reconciliation.getStopDate()
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>Movement_getDestinationAggregateBankReconciliationDate</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
for bank_reconciliation in context.getAggregateValueList(portal_type='Bank Reconciliation'):
if bank_reconciliation.getSourcePayment() == context.getDestinationPayment():
return bank_reconciliation.getUid()
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>Movement_getDestinationAggregateBankReconciliationUid</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
for bank_reconciliation in context.getAggregateValueList(portal_type='Bank Reconciliation'):
if bank_reconciliation.getSourcePayment() == context.getSourcePayment():
return bank_reconciliation.getStopDate()
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>Movement_getSourceAggregateBankReconciliationDate</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
for bank_reconciliation in context.getAggregateValueList(portal_type='Bank Reconciliation'):
if bank_reconciliation.getSourcePayment() == context.getSourcePayment():
return bank_reconciliation.getUid()
<?xml version="1.0"?>
<ZopeData>
<record id="1" aka="AAAAAAAAAAE=">
<pickle>
<global name="PythonScript" module="Products.PythonScripts.PythonScript"/>
</pickle>
<pickle>
<dictionary>
<item>
<key> <string>Script_magic</string> </key>
<value> <int>3</int> </value>
</item>
<item>
<key> <string>_bind_names</string> </key>
<value>
<object>
<klass>
<global name="NameAssignments" module="Shared.DC.Scripts.Bindings"/>
</klass>
<tuple/>
<state>
<dictionary>
<item>
<key> <string>_asgns</string> </key>
<value>
<dictionary>
<item>
<key> <string>name_container</string> </key>
<value> <string>container</string> </value>
</item>
<item>
<key> <string>name_context</string> </key>
<value> <string>context</string> </value>
</item>
<item>
<key> <string>name_m_self</string> </key>
<value> <string>script</string> </value>
</item>
<item>
<key> <string>name_subpath</string> </key>
<value> <string>traverse_subpath</string> </value>
</item>
</dictionary>
</value>
</item>
</dictionary>
</state>
</object>
</value>
</item>
<item>
<key> <string>_params</string> </key>
<value> <string></string> </value>
</item>
<item>
<key> <string>id</string> </key>
<value> <string>Movement_getSourceAggregateBankReconciliationUid</string> </value>
</item>
</dictionary>
</pickle>
</record>
</ZopeData>
from Products.ERP5Type.Message import translateString
portal = sci['object'].getPortalObject()
bank_reconciliation = sci['object']
# We activate after BankReconciliation_selectNonReconciledTransactionList, in
# case the user cancels just after using this action.
# case the user cancels just after using this action.
portal.portal_catalog.activate(
activity='SQLQueue', after_tag='BankReconciliation_selectNonReconciledTransactionList'
).searchAndActivate(
portal_type=portal.getPortalAccountingMovementTypeList(),
default_aggregate_uid=sci['object'].getUid(),
method_args=(None, translateString('Cancelling bank reconciliation')),
method_id='AccountingTransactionLine_setBankReconciliation'
default_aggregate_uid=bank_reconciliation.getUid(),
method_args=(bank_reconciliation.getRelativeUrl(), translateString('Cancelling bank reconciliation')),
method_id='AccountingTransactionLine_removeBankReconciliation'
)
......@@ -556,7 +556,154 @@ class TestBankReconciliation(AccountingTestCase, ERP5ReportTestCase):
self.tic()
self.assertEqual(100, bank_reconciliation.BankReconciliation_getReconciledAccountBalance())
def test_BankReconciliation_internal_transaction(self):
# Allow internal invoice in accounting module
module_allowed_type_list = self.portal.portal_types[
'Accounting Transaction Module'].getTypeAllowedContentTypeList()
self.portal.portal_types[
'Accounting Transaction Module'].setTypeAllowedContentTypeList(
module_allowed_type_list + ['Internal Invoice Transaction',])
account_module = self.account_module
main_section_bank_account = self.main_section.newContent(
portal_type='Bank Account',
price_currency_value=self.portal.currency_module.euro)
main_section_bank_account.validate()
internal_transaction = self.portal.accounting_module.newContent(
portal_type='Internal Invoice Transaction',
source_section_value=self.section,
source_payment_value=self.bank_account,
destination_section_value=self.main_section,
destination_payment_value=main_section_bank_account,
resource_value=self.portal.currency_module.euro,
start_date=DateTime(2014, 1, 1))
internal_transaction.newContent(
portal_type='Internal Invoice Transaction Line',
source_value=account_module.bank,
destination_value=account_module.bank,
source_debit=100,
id='bank')
internal_transaction.newContent(
portal_type='Internal Invoice Transaction Line',
destination_value=account_module.payable,
source_value=account_module.receivable,
source_credit=100)
internal_transaction.start()
internal_transaction.stop()
# this internal_transaction.bank line exists in both reconciliation for
# `section` and for `main_section` and can be reconciled independently by
# each section
bank_reconciliation_for_main_section = self.portal.bank_reconciliation_module.newContent(
portal_type='Bank Reconciliation',
title='Bank Reconcilisation for Main Section',
source_section_value=self.main_section,
source_payment_value=main_section_bank_account,
stop_date=DateTime(2014, 1, 31))
self.tic()
self.assertEqual(-100, bank_reconciliation_for_main_section.BankReconciliation_getAccountBalance())
self.assertEqual(
[internal_transaction.bank, ],
[x.getObject() for x in
bank_reconciliation_for_main_section.BankReconciliation_getAccountingTransactionLineList()])
bank_reconciliation_for_section = self.portal.bank_reconciliation_module.newContent(
portal_type='Bank Reconciliation',
title='Bank Reconcilisation for Section',
source_section_value=self.section,
source_payment_value=self.bank_account,
stop_date=DateTime(2014, 1, 31))
self.tic()
self.assertEqual(100, bank_reconciliation_for_section.BankReconciliation_getAccountBalance())
self.assertEqual(
[internal_transaction.bank, ],
[x.getObject() for x in
bank_reconciliation_for_section.BankReconciliation_getAccountingTransactionLineList()])
# if `section` reconciles, the line is not reconciled for `main_section`
list_selection_name = bank_reconciliation_for_section\
.BankReconciliation_viewBankReconciliationFastInputDialog.listbox.get_value(
'selection_name')
bank_reconciliation_for_section.BankReconciliation_reconcileTransactionList(
list_selection_name=list_selection_name,
uids=(internal_transaction.bank.getUid(), ),
mode='reconcile')
self.tic()
# reconciled for `section`
self.assertEqual(100, bank_reconciliation_for_section.BankReconciliation_getReconciledAccountBalance())
self.assertEqual(
[],
[x.getObject() for x in
bank_reconciliation_for_section.BankReconciliation_getAccountingTransactionLineList()])
# Not reconciled for `main_section`
self.assertEqual(0, bank_reconciliation_for_main_section.BankReconciliation_getReconciledAccountBalance())
self.assertEqual(
[internal_transaction.bank, ],
[x.getObject() for x in
bank_reconciliation_for_main_section.BankReconciliation_getAccountingTransactionLineList()])
self.assertItemsEqual(
[bank_reconciliation_for_section],
internal_transaction.bank.getAggregateValueList())
# if `main_section` also reconcile, line will be reconciled for both
list_selection_name = bank_reconciliation_for_main_section\
.BankReconciliation_viewBankReconciliationFastInputDialog.listbox.get_value(
'selection_name')
bank_reconciliation_for_main_section.BankReconciliation_reconcileTransactionList(
list_selection_name=list_selection_name,
uids=(internal_transaction.bank.getUid(), ),
mode='reconcile')
self.tic()
# Reconciled for `main_section`
self.assertEqual(-100, bank_reconciliation_for_main_section.BankReconciliation_getReconciledAccountBalance())
self.assertEqual(
[],
[x.getObject() for x in
bank_reconciliation_for_main_section.BankReconciliation_getAccountingTransactionLineList()])
# Still reconciled for `section`
self.assertEqual(100, bank_reconciliation_for_section.BankReconciliation_getReconciledAccountBalance())
self.assertEqual(
[],
[x.getObject() for x in
bank_reconciliation_for_section.BankReconciliation_getAccountingTransactionLineList()])
self.assertItemsEqual(
[bank_reconciliation_for_section, bank_reconciliation_for_main_section],
internal_transaction.bank.getAggregateValueList())
# if `section` un-reconcile, line will be not reconciled for `section`, but still reconciled for `main_section`
list_selection_name = bank_reconciliation_for_section\
.BankReconciliation_viewBankReconciliationFastInputDialog.listbox.get_value(
'selection_name')
bank_reconciliation_for_section.BankReconciliation_reconcileTransactionList(
list_selection_name=list_selection_name,
uids=(internal_transaction.bank.getUid(), ),
mode='unreconcile')
self.tic()
# no longer reconciled for `section`
self.assertEqual(0, bank_reconciliation_for_section.BankReconciliation_getReconciledAccountBalance())
self.assertEqual(
[internal_transaction.bank,],
[x.getObject() for x in
bank_reconciliation_for_section.BankReconciliation_getAccountingTransactionLineList()])
# Still reconciled for `main_section`
self.assertEqual(-100, bank_reconciliation_for_main_section.BankReconciliation_getReconciledAccountBalance())
self.assertEqual(
[],
[x.getObject() for x in
bank_reconciliation_for_main_section.BankReconciliation_getAccountingTransactionLineList()])
self.assertItemsEqual(
[bank_reconciliation_for_main_section],
internal_transaction.bank.getAggregateValueList())
def test_suite():
suite = unittest.TestSuite()
suite.addTest(unittest.makeSuite(TestBankReconciliation))
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment