From ed016f97c82eecd7b46404d64ea96439d54849f8 Mon Sep 17 00:00:00 2001 From: "Ronald Portier (Therp BV)" Date: Fri, 20 Jan 2023 00:29:46 +0100 Subject: [PATCH] [IMP] *_online_ponto: improved data retrieval logic With Ponto we can only retrieve data backwards in time. Therefore we override the normal _pull method to create statements in descending order of statement date. --- .../online_bank_statement_provider_ponto.py | 148 ++++++++++++---- ...t_account_statement_import_online_ponto.py | 163 ++++++++++++++---- .../tests/test_ponto_interface.py | 10 +- .../views/online_bank_statement_provider.xml | 1 + 4 files changed, 251 insertions(+), 71 deletions(-) diff --git a/account_statement_import_online_ponto/models/online_bank_statement_provider_ponto.py b/account_statement_import_online_ponto/models/online_bank_statement_provider_ponto.py index 4b673bc6..fe247214 100644 --- a/account_statement_import_online_ponto/models/online_bank_statement_provider_ponto.py +++ b/account_statement_import_online_ponto/models/online_bank_statement_provider_ponto.py @@ -4,7 +4,8 @@ import json import logging import re -from datetime import datetime +from datetime import datetime, timedelta +from operator import itemgetter import pytz @@ -13,9 +14,10 @@ from odoo import _, api, fields, models _logger = logging.getLogger(__name__) -class OnlineBankStatementProviderPonto(models.Model): +class OnlineBankStatementProvider(models.Model): _inherit = "online.bank.statement.provider" + ponto_last_identifier = fields.Char(readonly=True) ponto_date_field = fields.Selection( [ ("execution_date", "Execution Date"), @@ -36,43 +38,63 @@ class OnlineBankStatementProviderPonto(models.Model): ("ponto", "MyPonto.com"), ] - def _obtain_statement_data(self, date_since, date_until): - """Check wether called for ponto servide, otherwise pass the buck.""" + def _pull(self, date_since, date_until): + """For Ponto the pulling of data will not be grouped by statement. + + Instead we will pull data from the last available backwards. + + For a scheduled pull we will continue until we get to data + already retrieved or there is no more data available. + + For a wizard pull we will discard data after date_until and + stop retrieving when either we get before date_since or there is + no more data available. + """ + ponto_providers = self.filtered(lambda provider: provider.service == "ponto") + super(OnlineBankStatementProvider, self - ponto_providers)._pull( + date_since, date_until + ) + for provider in ponto_providers: + provider._ponto_pull(date_since, date_until) + + def _ponto_pull(self, date_since, date_until): + """Translate information from Ponto to Odoo bank statement lines.""" self.ensure_one() - if self.service != "ponto": # pragma: no cover - return super()._obtain_statement_data( + is_scheduled = self.env.context.get("scheduled") + if is_scheduled: + _logger.debug( + _("Ponto obtain statement data for journal %s from %s to %s"), + self.journal_id.name, date_since, date_until, ) - return self._ponto_obtain_statement_data(date_since, date_until) + else: + _logger.debug( + _("Ponto obtain all new statement data for journal %s"), + self.journal_id.name, + ) + lines = self._ponto_retrieve_data(date_since, date_until) + # For scheduled runs, store latest identifier. + if is_scheduled and lines: + self.ponto_last_identifier = lines[0].get("id") + self._ponto_store_lines(lines) - def _ponto_obtain_statement_data(self, date_since, date_until): - """Translate information from Ponto to Odoo bank statement lines.""" - self.ensure_one() - _logger.debug( - _("Ponto obtain statement data for journal %s from %s to %s"), - self.journal_id.name, - date_since, - date_until, - ) - lines = self._ponto_retrieve_data(date_since) - new_transactions = [] - sequence = 0 - for transaction in lines: - date = self._ponto_get_transaction_datetime(transaction) - if date < date_since or date > date_until: - continue - sequence += 1 - vals_line = self._ponto_get_transaction_vals(transaction, sequence) - new_transactions.append(vals_line) - return new_transactions, {} - - def _ponto_retrieve_data(self, date_since): + def _ponto_retrieve_data(self, date_since, date_until): """Fill buffer with data from Ponto. We will retrieve data from the latest transactions present in Ponto - backwards, until we find data that has an execution date before date_since. + backwards, until we find data that has an execution date before date_since, + or until we get to a transaction that we already have. + + Note: when reading data they are likely to be in descending order of + execution_date (not seen a guarantee for this in Ponto API). When using + value date, they may well be out of order. So we cannot simply stop + when we have foundd a transaction date before the date_since. + + We will not read transactions more then a week before before date_since. """ + date_stop = date_since - timedelta(days=7) + is_scheduled = self.env.context.get("scheduled") lines = [] interface_model = self.env["ponto.interface"] access_data = interface_model._login(self.username, self.password) @@ -80,17 +102,71 @@ class OnlineBankStatementProviderPonto(models.Model): latest_identifier = False transactions = interface_model._get_transactions(access_data, latest_identifier) while transactions: - lines.extend(transactions) + for line in transactions: + identifier = line.get("id") + transaction_datetime = self._ponto_get_transaction_datetime(line) + if (is_scheduled and identifier == self.ponto_last_identifier) or ( + transaction_datetime < date_stop + and (not self.ponto_last_identifier or not is_scheduled) + ): + return lines + if not is_scheduled: + if transaction_datetime < date_since: + return lines + if transaction_datetime > date_until: + continue + line["transaction_datetime"] = transaction_datetime + lines.append(line) latest_identifier = transactions[-1].get("id") - earliest_datetime = self._ponto_get_transaction_datetime(transactions[-1]) - if earliest_datetime < date_since: - break transactions = interface_model._get_transactions( access_data, latest_identifier ) + # We get here if we found no transactions before date_since, + # or not equal to stored last identifier. return lines - def _ponto_get_transaction_vals(self, transaction, sequence): + def _ponto_store_lines(self, lines): + """Store transactions retrieved from Ponto in statements. + + The data retrieved has the most recent first. However we need to create + the bank statements in ascending date order. as the balance_end of + one statement will be the balanxe_start of the next statement. + """ + + def reset_transactions(date_since): + """Reset values for new statement.""" + statement_date_since = self._get_statement_date_since(date_since) + statement_date_until = ( + statement_date_since + self._get_statement_date_step() + ) + statement_lines = [] + return statement_date_since, statement_date_until, statement_lines + + lines = sorted(lines, key=itemgetter("transaction_datetime")) + ( + statement_date_since, + statement_date_until, + statement_lines, + ) = reset_transactions(lines[0]["transaction_datetime"]) + for line in lines: + line.pop("transaction_datetime") + vals_line = self._ponto_get_transaction_vals(line) + if vals_line["date"] >= statement_date_until: + self._create_or_update_statement( + (statement_lines, {}), statement_date_since, statement_date_until + ) + ( + statement_date_since, + statement_date_until, + statement_lines, + ) = reset_transactions(statement_date_until) + statement_lines.append(vals_line) + # Handle lines in last statement. + self._create_or_update_statement( + (statement_lines, {}), statement_date_since, statement_date_until + ) + + def _ponto_get_transaction_vals(self, transaction): """Translate information from Ponto to statement line vals.""" attributes = transaction.get("attributes", {}) ref_list = [ @@ -105,7 +181,7 @@ class OnlineBankStatementProviderPonto(models.Model): ref = " ".join(ref_list) date = self._ponto_get_transaction_datetime(transaction) vals_line = { - "sequence": sequence, + "sequence": 1, # Sequence is not meaningfull for Ponto. "date": date, "ref": re.sub(" +", " ", ref) or "/", "payment_ref": attributes.get("remittanceInformation", ref), diff --git a/account_statement_import_online_ponto/tests/test_account_statement_import_online_ponto.py b/account_statement_import_online_ponto/tests/test_account_statement_import_online_ponto.py index 6273d43c..fdd1411c 100644 --- a/account_statement_import_online_ponto/tests/test_account_statement_import_online_ponto.py +++ b/account_statement_import_online_ponto/tests/test_account_statement_import_online_ponto.py @@ -1,17 +1,47 @@ # Copyright 2020 Florent de Labarre # Copyright 2022 Therp BV . # License AGPL-3.0 or later (https://www.gnu.org/licenses/agpl). - +import logging from datetime import date, datetime from unittest import mock -from odoo import fields +from odoo import _, fields from odoo.tests import Form, common +_logger = logging.getLogger(__name__) + _module_ns = "odoo.addons.account_statement_import_online_ponto" _interface_class = _module_ns + ".models.ponto_interface" + ".PontoInterface" -THREE_TRANSACTIONS = [ + +# Transactions should be ordered by descending executionDate. +FOUR_TRANSACTIONS = [ + # First transaction will be after date_until. + { + "type": "transaction", + "relationships": { + "account": { + "links": {"related": "https://api.myponto.com/accounts/"}, + "data": { + "type": "account", + "id": "fd3d5b1d-fca9-4310-a5c8-76f2a9dc7c75", + }, + } + }, + "id": "1552c32f-e63f-4ce6-a974-f270e6cd53a9", + "attributes": { + "valueDate": "2019-12-04T12:30:00.000Z", + "remittanceInformationType": "unstructured", + "remittanceInformation": "Arresto Momentum", + "executionDate": "2019-12-04T10:25:00.000Z", + "description": "Wire transfer after execution", + "currency": "EUR", + "counterpartReference": "BE10325927501996", + "counterpartName": "Some other customer", + "amount": 8.95, + }, + }, + # Next transaction has valueDate before, executionDate after date_until. { "type": "transaction", "relationships": { @@ -88,6 +118,8 @@ THREE_TRANSACTIONS = [ EMPTY_TRANSACTIONS = [] +transaction_amounts = [5.48, 5.83, 6.08, 8.95] + class TestAccountStatementImportOnlinePonto(common.TransactionCase): post_install = True @@ -123,6 +155,8 @@ class TestAccountStatementImportOnlinePonto(common.TransactionCase): } ) self.provider = self.journal.online_bank_statement_provider_id + # To get all the moves in a month at once + self.provider.statement_creation_mode = "monthly" self.mock_login = lambda: mock.patch( _interface_class + "._login", @@ -141,7 +175,7 @@ class TestAccountStatementImportOnlinePonto(common.TransactionCase): self.mock_get_transactions = lambda: mock.patch( _interface_class + "._get_transactions", side_effect=[ - THREE_TRANSACTIONS, + FOUR_TRANSACTIONS, EMPTY_TRANSACTIONS, ], ) @@ -176,32 +210,101 @@ class TestAccountStatementImportOnlinePonto(common.TransactionCase): self.assertEqual(len(new_statement.line_ids), 1) self.assertEqual(new_statement.balance_start, 100) self.assertEqual(new_statement.balance_end, 105.83) - # Ponto does not give balance info in transactions. - # self.assertEqual(new_statement.balance_end_real, 105.83) - def test_ponto(self): + def test_ponto_execution_date(self): with self.mock_login(), self.mock_set_access_account(), self.mock_get_transactions(): # noqa: B950 - vals = { - "provider_ids": [(4, self.provider.id)], - "date_since": datetime(2019, 11, 3), - "date_until": datetime(2019, 11, 17), - } - wizard = self.AccountStatementPull.with_context( - active_model="account.journal", - active_id=self.journal.id, - ).create(vals) - # To get all the moves at once - self.provider.statement_creation_mode = "monthly" - # For some reason the provider is not set in the create. - wizard.provider_ids = self.provider - wizard.action_pull() - statement = self.AccountBankStatement.search( - [("journal_id", "=", self.journal.id)] + # First base selection on execution date. + self.provider.ponto_date_field = "execution_date" + statement = self._get_statement_from_wizard() + self._check_line_count(statement.line_ids, expected_count=2) + self._check_statement_amounts(statement, transaction_amounts[:2]) + + def test_ponto_value_date(self): + with self.mock_login(), self.mock_set_access_account(), self.mock_get_transactions(): # noqa: B950 + # First base selection on execution date. + self.provider.ponto_date_field = "value_date" + statement = self._get_statement_from_wizard() + self._check_line_count(statement.line_ids, expected_count=3) + self._check_statement_amounts(statement, transaction_amounts[:3]) + + def test_ponto_scheduled(self): + with self.mock_login(), self.mock_set_access_account(), self.mock_get_transactions(): # noqa: B950 + # Scheduled should get all transaction, ignoring date_until. + self.provider.ponto_last_identifier = False + date_since = datetime(2019, 11, 3) + date_until = datetime(2019, 11, 18) + self.provider.with_context(scheduled=True)._pull(date_since, date_until) + statements = self._get_statements_from_journal(expected_count=2) + self._check_line_count(statements[0].line_ids, expected_count=3) + self._check_statement_amounts(statements[0], transaction_amounts[:3]) + self._check_line_count(statements[1].line_ids, expected_count=1) + # Expected balance_end will include amounts of previous statement. + self._check_statement_amounts( + statements[1], transaction_amounts[3:], expected_balance_end=26.34 ) - self.assertEqual(len(statement), 1) - self.assertEqual(len(statement.line_ids), 3) - sorted_amounts = sorted(statement.line_ids.mapped("amount")) - self.assertEqual(sorted_amounts, [5.48, 5.83, 6.08]) - self.assertEqual(statement.balance_end, 17.39) - # Ponto does not give balance info in transactions. - # self.assertEqual(statement.balance_end_real, 17.39) + + def test_ponto_scheduled_from_identifier(self): + with self.mock_login(), self.mock_set_access_account(), self.mock_get_transactions(): # noqa: B950 + # Scheduled should get all transactions after last identifier. + self.provider.ponto_last_identifier = "9ac50483-16dc-4a82-aa60-df56077405cd" + date_since = datetime(2019, 11, 3) + date_until = datetime(2019, 11, 18) + self.provider.with_context(scheduled=True)._pull(date_since, date_until) + # First two transactions for statement 0 should have been ignored. + statements = self._get_statements_from_journal(expected_count=2) + self._check_line_count(statements[0].line_ids, expected_count=1) + self._check_statement_amounts(statements[0], transaction_amounts[2:3]) + self._check_line_count(statements[1].line_ids, expected_count=1) + # Expected balance_end will include amounts of previous statement. + self._check_statement_amounts( + statements[1], transaction_amounts[3:], expected_balance_end=15.03 + ) + + def _get_statement_from_wizard(self): + """Run wizard to pull data and return statement.""" + vals = { + "provider_ids": [(4, self.provider.id)], + "date_since": datetime(2019, 11, 3), + "date_until": datetime(2019, 11, 18), + } + wizard = self.AccountStatementPull.with_context( + active_model="account.journal", + active_id=self.journal.id, + ).create(vals) + # For some reason the provider is not set in the create. + wizard.provider_ids = self.provider + wizard.action_pull() + return self._get_statements_from_journal(expected_count=1) + + def _get_statements_from_journal(self, expected_count=0): + """We only expect statements created by our tests.""" + statements = self.AccountBankStatement.search( + [("journal_id", "=", self.journal.id)], + order="date asc", + ) + self.assertEqual(len(statements), expected_count) + return statements + + def _check_line_count(self, lines, expected_count=0): + """Check wether lines contain expected number of transactions. + + If count differs, show the unique id's of lines that are present. + """ + # If we do not get all lines, show lines we did get: + line_count = len(lines) + if line_count != expected_count: + _logger.info( + _("Statement contains transactions: %s"), + " ".join(lines.mapped("unique_import_id")), + ) + self.assertEqual(line_count, expected_count) + + def _check_statement_amounts( + self, statement, expected_amounts, expected_balance_end=0.0 + ): + """Check wether amount in lines and end_balance as expected.""" + sorted_amounts = sorted([round(line.amount, 2) for line in statement.line_ids]) + self.assertEqual(sorted_amounts, expected_amounts) + if not expected_balance_end: + expected_balance_end = sum(expected_amounts) + self.assertEqual(statement.balance_end, expected_balance_end) diff --git a/account_statement_import_online_ponto/tests/test_ponto_interface.py b/account_statement_import_online_ponto/tests/test_ponto_interface.py index f4588992..4e8d1e54 100644 --- a/account_statement_import_online_ponto/tests/test_ponto_interface.py +++ b/account_statement_import_online_ponto/tests/test_ponto_interface.py @@ -8,7 +8,7 @@ from dateutil.relativedelta import relativedelta from odoo import fields from odoo.tests import common -from .test_account_statement_import_online_ponto import THREE_TRANSACTIONS +from .test_account_statement_import_online_ponto import FOUR_TRANSACTIONS class TestPontoInterface(common.TransactionCase): @@ -72,16 +72,16 @@ class TestPontoInterface(common.TransactionCase): mock_response = MagicMock() mock_response.status_code = 200 # Key "data" will contain a list of transactions. - mock_response.text = json.dumps({"data": THREE_TRANSACTIONS}) + mock_response.text = json.dumps({"data": FOUR_TRANSACTIONS}) requests_get.return_value = mock_response # Start of actual test. access_data = self._get_access_dict() interface_model = self.env["ponto.interface"] transactions = interface_model._get_transactions(access_data, False) - self.assertEqual(len(transactions), 3) - self.assertEqual(transactions[2]["id"], "b21a6c65-1c52-4ba6-8cbc-127d2b2d85ff") + self.assertEqual(len(transactions), 4) + self.assertEqual(transactions[3]["id"], "b21a6c65-1c52-4ba6-8cbc-127d2b2d85ff") self.assertEqual( - transactions[2]["attributes"]["counterpartReference"], "BE10325927501996" + transactions[3]["attributes"]["counterpartReference"], "BE10325927501996" ) def _get_access_dict(self, include_account=True): diff --git a/account_statement_import_online_ponto/views/online_bank_statement_provider.xml b/account_statement_import_online_ponto/views/online_bank_statement_provider.xml index 212cc475..d8464245 100644 --- a/account_statement_import_online_ponto/views/online_bank_statement_provider.xml +++ b/account_statement_import_online_ponto/views/online_bank_statement_provider.xml @@ -17,6 +17,7 @@ +