Revert "[16.0][FIX] account_statement_import_online_gocardless: IBAN comparison"

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_API = "https://bankaccountdata.gocardless.com/api/v2"
+GOCARDLESS_ENDPOINT = "https://bankaccountdata.gocardless.com/api/v2"
 REQUESTS_TIMEOUT = 60
 
 
@@ -46,31 +46,6 @@ 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
@@ -84,16 +59,20 @@ class OnlineBankStatementProvider(models.Model):
                 self.gocardless_refresh_token
                 and now > self.gocardless_refresh_expiration
             ):
-                endpoint = "token/refresh"
+                url = f"{GOCARDLESS_ENDPOINT}/token/refresh/"
             else:
-                endpoint = "token/new"
+                url = f"{GOCARDLESS_ENDPOINT}/token/new/"
-            _response, data = self._gocardless_request(
+            response = requests.post(
-                endpoint,
+                url,
-                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),
@@ -107,6 +86,17 @@ 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(
@@ -147,12 +137,15 @@ class OnlineBankStatementProvider(models.Model):
         country = (
             self.journal_id.bank_account_id.company_id or self.journal_id.company_id
         ).country_id
-        response, data = self._gocardless_request(
+        response = requests.get(
-            "institutions", params={"country": country.code}
+            f"{GOCARDLESS_ENDPOINT}/institutions/",
+            params={"country": country.code},
+            headers=self._gocardless_get_headers(),
+            timeout=REQUESTS_TIMEOUT,
         )
         if response.status_code == 400:
             raise UserError(_("Incorrect country code or country not supported."))
-        institutions = data
+        institutions = json.loads(response.text)
         # Prepare data for being showed in the JS widget
         ctx = self.env.context.copy()
         ctx.update(
@@ -179,9 +172,8 @@ 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, data = self._gocardless_request(
+        response = requests.post(
-            "requisitions",
+            f"{GOCARDLESS_ENDPOINT}/requisitions/",
-            request_type="post",
             data=json.dumps(
                 {
                     "redirect": redirect_url,
@@ -189,27 +181,15 @@ class OnlineBankStatementProvider(models.Model):
                     "reference": self.gocardless_requisition_ref,
                 }
             ),
+            headers=self._gocardless_get_headers(),
+            timeout=REQUESTS_TIMEOUT,
         )
-        if data:
+        if response.status_code == 201:
-            requisition_data = data
+            requisition_data = json.loads(response.text)
             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
@@ -223,25 +203,39 @@ class OnlineBankStatementProvider(models.Model):
           process, so no fail message is logged in chatter in this case.
         """
         self.ensure_one()
-        requisition_data = self._gocardless_request_requisition()
+        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)
         accounts = requisition_data.get("accounts", [])
         found_account = False
         accounts_iban = []
         for account_id in accounts:
-            account_data = self._gocardless_request_account(account_id)
+            account_response = requests.get(
-            if account_data:
+                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)
                 accounts_iban.append(account_data["iban"])
                 if (
                     self.journal_id.bank_account_id.sanitized_acc_number
-                    == account_data["iban"].upper()
+                    == account_data["iban"]
                 ):
                     found_account = True
                     self.gocardless_account_id = account_data["id"]
                     break
         if found_account:
-            agreement_data = self._gocardless_request_agreement(
+            agreement_response = requests.get(
-                requisition_data["agreement"]
+                f"{GOCARDLESS_ENDPOINT}/agreements/enduser/"
+                f"{requisition_data['agreement']}/",
+                headers=self._gocardless_get_headers(),
+                timeout=REQUESTS_TIMEOUT,
             )
+            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"])
@@ -285,14 +279,19 @@ class OnlineBankStatementProvider(models.Model):
         now = fields.Datetime.now()
         if now > date_since and now < date_until:
             date_until = now
-        _response, data = self._gocardless_request(
+        transaction_response = requests.get(
-            f"accounts/{self.gocardless_account_id}/transactions",
+            f"{GOCARDLESS_ENDPOINT}/accounts/"
+            f"{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,
         )
-        return data
+        if transaction_response.status_code == 200:
+            return json.loads(transaction_response.text)
+        return {}
 
     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,14 +21,6 @@ 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",
@@ -37,7 +29,6 @@ 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
@@ -86,31 +77,6 @@ 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 = {
@@ -134,8 +100,3 @@ 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!")