[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.

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),

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 <https://therp.nl>.
 # 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)

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):

account_statement_import_online_ponto/views/online_bank_statement_provider.xml

@@ -17,6 +17,7 @@
                     <field name="username" string="Login" />
                     <field name="password" string="Secret Key" />
                     <field name="ponto_date_field" />
+                    <field name="ponto_last_identifier" />
                 </group>
             </xpath>
         </field>