[FIX] account_statement_import_online_gocardless: IBAN comparison

It is possible that, when making the request to the requisitions endpoint,
the IBAN bank account comes with a lower alphanumeric string.
When comparing with the sanitized bank account (stored with upper) fails.

self.journal_id.bank_account_id.sanitized_acc_number == account_data["iban"]

to

self.journal_id.bank_account_id.sanitized_acc_number == account_data["iban"].upper()

Refactor method _gocardless_finish_requisition to be able to mock the requests made inside and create a unit test.
Refactor requests methods.

account_statement_import_online_gocardless/models/online_bank_statement_provider.py

@@ -13,7 +13,7 @@ from odoo import _, api, fields, models
 from odoo.exceptions import UserError
 from odoo.tools import DEFAULT_SERVER_DATE_FORMAT as DF
 
-GOCARDLESS_ENDPOINT = "https://bankaccountdata.gocardless.com/api/v2"
+GOCARDLESS_API = "https://bankaccountdata.gocardless.com/api/v2"
 REQUESTS_TIMEOUT = 60
 
 
@@ -46,6 +46,31 @@ class OnlineBankStatementProvider(models.Model):
             ("gocardless", "GoCardless"),
         ]
 
+    def _gocardless_get_headers(self, basic=False):
+        """Generic method for providing the needed request headers."""
+        self.ensure_one()
+        headers = {
+            "accept": "application/json",
+            "Content-Type": "application/json",
+        }
+        if not basic:
+            headers["Authorization"] = f"Bearer {self._gocardless_get_token()}"
+        return headers
+
+    def _gocardless_request(self, endpoint, request_type="get", params=None, data=None):
+        content = {}
+        url = url_join(GOCARDLESS_API, endpoint)
+        response = getattr(requests, request_type)(
+            url,
+            data=data,
+            params=params,
+            headers=self._gocardless_get_headers(),
+            timeout=REQUESTS_TIMEOUT,
+        )
+        if response.status_code in [200, 201]:
+            content = json.loads(response.text)
+        return response, content
+
     def _gocardless_get_token(self):
         """Resolve and return the corresponding GoCardless token for doing the requests.
         If there's still no token, it's requested. If it exists, but it's expired and
@@ -59,20 +84,16 @@ class OnlineBankStatementProvider(models.Model):
                 self.gocardless_refresh_token
                 and now > self.gocardless_refresh_expiration
             ):
-                url = f"{GOCARDLESS_ENDPOINT}/token/refresh/"
+                endpoint = "token/refresh"
             else:
-                url = f"{GOCARDLESS_ENDPOINT}/token/new/"
-            response = requests.post(
-                url,
+                endpoint = "token/new"
+            _response, data = self._gocardless_request(
+                endpoint,
+                request_type="post",
                 data=json.dumps(
                     {"secret_id": self.username, "secret_key": self.password}
                 ),
-                headers=self._gocardless_get_headers(basic=True),
-                timeout=REQUESTS_TIMEOUT,
             )
-            data = {}
-            if response.status_code == 200:
-                data = json.loads(response.text)
             expiration_date = now + relativedelta(seconds=data.get("access_expires", 0))
             vals = {
                 "gocardless_token": data.get("access", False),
@@ -86,17 +107,6 @@ class OnlineBankStatementProvider(models.Model):
             self.sudo().write(vals)
         return self.gocardless_token
 
-    def _gocardless_get_headers(self, basic=False):
-        """Generic method for providing the needed request headers."""
-        self.ensure_one()
-        headers = {
-            "accept": "application/json",
-            "Content-Type": "application/json",
-        }
-        if not basic:
-            headers["Authorization"] = f"Bearer {self._gocardless_get_token()}"
-        return headers
-
     def action_select_gocardless_bank(self):
         if not self.journal_id.bank_account_id:
             raise UserError(
@@ -137,15 +147,12 @@ class OnlineBankStatementProvider(models.Model):
         country = (
             self.journal_id.bank_account_id.company_id or self.journal_id.company_id
         ).country_id
-        response = requests.get(
-            f"{GOCARDLESS_ENDPOINT}/institutions/",
-            params={"country": country.code},
-            headers=self._gocardless_get_headers(),
-            timeout=REQUESTS_TIMEOUT,
+        response, data = self._gocardless_request(
+            "institutions", params={"country": country.code}
         )
         if response.status_code == 400:
             raise UserError(_("Incorrect country code or country not supported."))
-        institutions = json.loads(response.text)
+        institutions = data
         # Prepare data for being showed in the JS widget
         ctx = self.env.context.copy()
         ctx.update(
@@ -172,8 +179,9 @@ class OnlineBankStatementProvider(models.Model):
         self.gocardless_requisition_ref = str(uuid4())
         base_url = self.env["ir.config_parameter"].sudo().get_param("web.base.url")
         redirect_url = url_join(base_url, "gocardless/response")
-        response = requests.post(
-            f"{GOCARDLESS_ENDPOINT}/requisitions/",
+        _response, data = self._gocardless_request(
+            "requisitions",
+            request_type="post",
             data=json.dumps(
                 {
                     "redirect": redirect_url,
@@ -181,15 +189,27 @@ class OnlineBankStatementProvider(models.Model):
                     "reference": self.gocardless_requisition_ref,
                 }
             ),
-            headers=self._gocardless_get_headers(),
-            timeout=REQUESTS_TIMEOUT,
         )
-        if response.status_code == 201:
-            requisition_data = json.loads(response.text)
+        if data:
+            requisition_data = data
             self.gocardless_requisition_id = requisition_data["id"]
             # JS code expects here to return a plain link or nothing
             return requisition_data["link"]
 
+    def _gocardless_request_requisition(self):
+        _response, data = self._gocardless_request(
+            f"requisitions/{self.gocardless_requisition_id}"
+        )
+        return data
+
+    def _gocardless_request_account(self, account_id):
+        _response, data = self._gocardless_request(f"accounts/{account_id}")
+        return data
+
+    def _gocardless_request_agreement(self, agreement_id):
+        _response, data = self._gocardless_request(f"agreements/enduser/{agreement_id}")
+        return data
+
     def _gocardless_finish_requisition(self, dry=False):
         """Once the requisiton to the bank institution has been made, and this is called
         from the controller assigned to the redirect URL, we check that the IBAN account
@@ -203,39 +223,25 @@ class OnlineBankStatementProvider(models.Model):
           process, so no fail message is logged in chatter in this case.
         """
         self.ensure_one()
-        requisition_response = requests.get(
-            f"{GOCARDLESS_ENDPOINT}/requisitions/{self.gocardless_requisition_id}/",
-            headers=self._gocardless_get_headers(),
-            timeout=REQUESTS_TIMEOUT,
-        )
-        requisition_data = json.loads(requisition_response.text)
+        requisition_data = self._gocardless_request_requisition()
         accounts = requisition_data.get("accounts", [])
         found_account = False
         accounts_iban = []
         for account_id in accounts:
-            account_response = requests.get(
-                f"{GOCARDLESS_ENDPOINT}/accounts/{account_id}/",
-                headers=self._gocardless_get_headers(),
-                timeout=REQUESTS_TIMEOUT,
-            )
-            if account_response.status_code == 200:
-                account_data = json.loads(account_response.text)
+            account_data = self._gocardless_request_account(account_id)
+            if account_data:
                 accounts_iban.append(account_data["iban"])
                 if (
                     self.journal_id.bank_account_id.sanitized_acc_number
-                    == account_data["iban"]
+                    == account_data["iban"].upper()
                 ):
                     found_account = True
                     self.gocardless_account_id = account_data["id"]
                     break
         if found_account:
-            agreement_response = requests.get(
-                f"{GOCARDLESS_ENDPOINT}/agreements/enduser/"
-                f"{requisition_data['agreement']}/",
-                headers=self._gocardless_get_headers(),
-                timeout=REQUESTS_TIMEOUT,
+            agreement_data = self._gocardless_request_agreement(
+                requisition_data["agreement"]
             )
-            agreement_data = json.loads(agreement_response.text)
             self.gocardless_requisition_expiration = datetime.strptime(
                 agreement_data["accepted"], "%Y-%m-%dT%H:%M:%S.%fZ"
             ) + relativedelta(days=agreement_data["access_valid_for_days"])
@@ -279,19 +285,14 @@ class OnlineBankStatementProvider(models.Model):
         now = fields.Datetime.now()
         if now > date_since and now < date_until:
             date_until = now
-        transaction_response = requests.get(
-            f"{GOCARDLESS_ENDPOINT}/accounts/"
-            f"{self.gocardless_account_id}/transactions/",
+        _response, data = self._gocardless_request(
+            f"accounts/{self.gocardless_account_id}/transactions",
             params={
                 "date_from": date_since.strftime(DF),
                 "date_to": date_until.strftime(DF),
             },
-            headers=self._gocardless_get_headers(),
-            timeout=REQUESTS_TIMEOUT,
         )
-        if transaction_response.status_code == 200:
-            return json.loads(transaction_response.text)
-        return {}
+        return data
 
     def _gocardless_obtain_statement_data(self, date_since, date_until):
         """Called from the cron or the manual pull wizard to obtain transactions for

account_statement_import_online_gocardless/tests/test_account_statement_import_online_gocardless.py

@@ -21,6 +21,14 @@ class TestAccountBankAccountStatementImportOnlineGocardless(common.TransactionCa
         cls.now = fields.Datetime.now()
         cls.currency_eur = cls.env.ref("base.EUR")
         cls.currency_eur.write({"active": True})
+        bank_account = cls.env["res.partner.bank"].create(
+            {
+                "acc_number": "NL77ABNA0574908765",
+                "partner_id": cls.env.ref("base.main_partner").id,
+                "company_id": cls.env.ref("base.main_company").id,
+                "bank_id": cls.env.ref("base.res_bank_1").id,
+            }
+        )
         cls.journal = cls.env["account.journal"].create(
             {
                 "name": "GoCardless Bank Test",
@@ -29,6 +37,7 @@ class TestAccountBankAccountStatementImportOnlineGocardless(common.TransactionCa
                 "currency_id": cls.currency_eur.id,
                 "bank_statements_source": "online",
                 "online_bank_statement_provider": "gocardless",
+                "bank_account_id": bank_account.id,
             }
         )
         cls.provider = cls.journal.online_bank_statement_provider_id
@@ -77,6 +86,31 @@ class TestAccountBankAccountStatementImportOnlineGocardless(common.TransactionCa
             _provider_class + "._gocardless_request_transactions",
             return_value=cls.return_value,
         )
+        cls.request_requisition_value = {
+            "accounts": ["ACCOUNT-ID-1"],
+            "agreement": "TEST-AGREEMENT-ID",
+        }
+        cls.mock_requisition = lambda cls: mock.patch(
+            _provider_class + "._gocardless_request_requisition",
+            return_value=cls.request_requisition_value,
+        )
+        cls.request_account_value = {
+            "id": "ACCOUNT-ID-1",
+            "iban": "nl77abna0574908765",
+        }
+        cls.mock_account = lambda cls: mock.patch(
+            _provider_class + "._gocardless_request_account",
+            return_value=cls.request_account_value,
+        )
+        cls.request_agreement_value = {
+            "id": "TEST-AGREEMENT-ID",
+            "accepted": cls.now.strftime("%Y-%m-%dT%H:%M:%S.%fZ"),
+            "access_valid_for_days": 30,
+        }
+        cls.mock_agreement = lambda cls: mock.patch(
+            _provider_class + "._gocardless_request_agreement",
+            return_value=cls.request_agreement_value,
+        )
 
     def test_mocked_gocardless(self):
         vals = {
@@ -100,3 +134,8 @@ class TestAccountBankAccountStatementImportOnlineGocardless(common.TransactionCa
         lines = statements.line_ids.sorted(lambda x: x.date)
         self.assertEqual(len(lines), 2)
         self.assertEqual(lines.mapped("amount"), [45.0, -15.0])
+
+    def test_provider_gocardless_finish_requisition(self):
+        with self.mock_requisition(), self.mock_account(), self.mock_agreement():
+            res = self.provider._gocardless_finish_requisition(dry=True)
+            self.assertTrue(res, "Bank account not found!")