From 85b0385b55a91741c48e2fb09ea3b97bc927c7d5 Mon Sep 17 00:00:00 2001 From: Alexey Pelykh Date: Mon, 6 Apr 2020 16:58:11 +0200 Subject: [PATCH] [FIX] account_bank_statement_import_online_paypal: workaround for PayPal issue --- .../online_bank_statement_provider_paypal.py | 66 +++++++---- .../readme/ROADMAP.rst | 4 + ...unt_bank_statement_import_online_paypal.py | 110 +++++++++++++++++- 3 files changed, 157 insertions(+), 23 deletions(-) diff --git a/account_bank_statement_import_online_paypal/models/online_bank_statement_provider_paypal.py b/account_bank_statement_import_online_paypal/models/online_bank_statement_provider_paypal.py index fed83e0..b0e27b9 100644 --- a/account_bank_statement_import_online_paypal/models/online_bank_statement_provider_paypal.py +++ b/account_bank_statement_import_online_paypal/models/online_bank_statement_provider_paypal.py @@ -166,6 +166,7 @@ EVENT_DESCRIPTIONS = { 'T9800': _('Display only transaction'), 'T9900': _('Other'), } +NO_DATA_FOR_DATE_AVAIL_MSG = 'Data for the given start date is not available.' class OnlineBankStatementProviderPayPal(models.Model): @@ -416,7 +417,18 @@ class OnlineBankStatementProviderPayPal(models.Model): interval_end.isoformat() + 'Z', page, )) - data = self._paypal_retrieve(url, token) + + # NOTE: Workaround for INVALID_REQUEST (see ROADMAP.rst) + invalid_data_workaround = self.env.context.get( + 'test_account_bank_statement_import_online_paypal_monday', + interval_start.weekday() == 0 and ( + datetime.utcnow() - interval_start + ).total_seconds() < 28800 + ) + + data = self.with_context( + invalid_data_workaround=invalid_data_workaround, + )._paypal_retrieve(url, token) interval_transactions = map( lambda transaction: self._paypal_preparse_transaction( transaction @@ -465,15 +477,22 @@ class OnlineBankStatementProviderPayPal(models.Model): return Decimal(transaction_amount['value']) @api.model - def _paypal_validate(self, content): - content = json.loads(content) - if 'error' in content and content['error']: - raise UserError( - content['error_description'] - if 'error_description' in content - else 'Unknown error' - ) - return content + def _paypal_decode_error(self, content): + generic_error = content.get('name') + if generic_error: + return UserError('%s: %s' % ( + generic_error, + content.get('message') or _('Unknown error'), + )) + + identity_error = content.get('error') + if identity_error: + UserError('%s: %s' % ( + generic_error, + content.get('error_description') or _('Unknown error'), + )) + + return None @api.model def _paypal_retrieve(self, url, auth, data=None): @@ -481,18 +500,21 @@ class OnlineBankStatementProviderPayPal(models.Model): with self._paypal_urlopen(url, auth, data) as response: content = response.read().decode('utf-8') except HTTPError as e: - content = self._paypal_validate( - e.read().decode('utf-8') - ) - if 'name' in content and content['name']: - raise UserError('%s: %s' % ( - content['name'], - content['error_description'] - if 'error_description' in content - else 'Unknown error', - )) - raise e - return self._paypal_validate(content) + content = json.loads(e.read().decode('utf-8')) + + # NOTE: Workaround for INVALID_REQUEST (see ROADMAP.rst) + if self.env.context.get('invalid_data_workaround') \ + and content.get('name') == 'INVALID_REQUEST' \ + and content.get('message') == NO_DATA_FOR_DATE_AVAIL_MSG: + return { + 'transaction_details': [], + 'page': 1, + 'total_items': 0, + 'total_pages': 0, + } + + raise self._paypal_decode_error(content) or e + return json.loads(content) @api.model def _paypal_urlopen(self, url, auth, data=None): diff --git a/account_bank_statement_import_online_paypal/readme/ROADMAP.rst b/account_bank_statement_import_online_paypal/readme/ROADMAP.rst index d811420..f696842 100644 --- a/account_bank_statement_import_online_paypal/readme/ROADMAP.rst +++ b/account_bank_statement_import_online_paypal/readme/ROADMAP.rst @@ -5,3 +5,7 @@ * `PayPal Transaction Info `_ defines extra fields like ``tip_amount``, ``shipping_amount``, etc. that could be useful to be decomposed from a single transaction. +* There's a known issue with PayPal API that on every Monday for couple of + hours after UTC midnight it returns ``INVALID_REQUEST`` incorrectly: their + servers have not inflated the data yet. PayPal tech support confirmed this + behaviour in case #06650320 (private). diff --git a/account_bank_statement_import_online_paypal/tests/test_account_bank_statement_import_online_paypal.py b/account_bank_statement_import_online_paypal/tests/test_account_bank_statement_import_online_paypal.py index fc7e814..c600184 100644 --- a/account_bank_statement_import_online_paypal/tests/test_account_bank_statement_import_online_paypal.py +++ b/account_bank_statement_import_online_paypal/tests/test_account_bank_statement_import_online_paypal.py @@ -7,9 +7,11 @@ from dateutil.relativedelta import relativedelta from decimal import Decimal import json from unittest import mock +from urllib.error import HTTPError -from odoo.tests import common from odoo import fields +from odoo.exceptions import UserError +from odoo.tests import common _module_ns = 'odoo.addons.account_bank_statement_import_online_paypal' _provider_class = ( @@ -19,6 +21,31 @@ _provider_class = ( ) +class FakeHTTPError(HTTPError): + def __init__(self, content): + self.content = content + + def read(self): + return self.content.encode('utf-8') + + +class UrlopenRetValMock: + def __init__(self, content, throw=False): + self.content = content + self.throw = throw + + def __enter__(self): + return self + + def __exit__(self, type, value, tb): + pass + + def read(self): + if self.throw: + raise FakeHTTPError(self.content) + return self.content.encode('utf-8') + + class TestAccountBankAccountStatementImportOnlinePayPal( common.TransactionCase ): @@ -156,6 +183,87 @@ class TestAccountBankAccountStatementImportOnlinePayPal( with self.assertRaises(Exception): provider._paypal_get_token() + def test_no_data_on_monday(self): + journal = self.AccountJournal.create({ + 'name': 'Bank', + 'type': 'bank', + 'code': 'BANK', + 'currency_id': self.currency_eur.id, + 'bank_statements_source': 'online', + 'online_bank_statement_provider': 'paypal', + }) + + provider = journal.online_bank_statement_provider_id + mocked_response = UrlopenRetValMock("""{ + "debug_id": "eec890ebd5798", + "details": "xxxxxx", + "links": "xxxxxx", + "message": "Data for the given start date is not available.", + "name": "INVALID_REQUEST" +}""", throw=True) + with mock.patch( + _provider_class + '._paypal_urlopen', + return_value=mocked_response, + ), self.mock_token(): + data = provider.with_context( + test_account_bank_statement_import_online_paypal_monday=True, + )._obtain_statement_data( + self.now - relativedelta(hours=1), + self.now, + ) + + self.assertIsNone(data) + + def test_error_handling_1(self): + journal = self.AccountJournal.create({ + 'name': 'Bank', + 'type': 'bank', + 'code': 'BANK', + 'currency_id': self.currency_eur.id, + 'bank_statements_source': 'online', + 'online_bank_statement_provider': 'paypal', + }) + + provider = journal.online_bank_statement_provider_id + mocked_response = UrlopenRetValMock("""{ + "message": "MSG", + "name": "ERROR" +}""", throw=True) + with mock.patch( + _provider_class + '._paypal_urlopen', + return_value=mocked_response, + ), self.mock_token(): + with self.assertRaises(UserError): + provider._obtain_statement_data( + self.now - relativedelta(years=5), + self.now, + ) + + def test_error_handling_2(self): + journal = self.AccountJournal.create({ + 'name': 'Bank', + 'type': 'bank', + 'code': 'BANK', + 'currency_id': self.currency_eur.id, + 'bank_statements_source': 'online', + 'online_bank_statement_provider': 'paypal', + }) + + provider = journal.online_bank_statement_provider_id + mocked_response = UrlopenRetValMock("""{ + "error_description": "DESC", + "error": "ERROR" +}""", throw=True) + with mock.patch( + _provider_class + '._paypal_urlopen', + return_value=mocked_response, + ), self.mock_token(): + with self.assertRaises(UserError): + provider._obtain_statement_data( + self.now - relativedelta(years=5), + self.now, + ) + def test_empty_pull(self): journal = self.AccountJournal.create({ 'name': 'Bank',