From 3e234ee504b11e8543a826d036240f94c51e7fac Mon Sep 17 00:00:00 2001 From: Viswas Haridas <37623357+JustARatherRidiculouslyLongUsername@users.noreply.github.com> Date: Fri, 14 Feb 2025 13:24:51 +0530 Subject: [PATCH 1/2] feat: PATCH `/integrations` on export errors (#435) * feat: PATCH `/integrations` on export errors * feat: patch `/integrations` on token expiry (#436) * feat: patch `/integrations` on token expiry * fix: move `patch_integration_settings` to `apps/workspaces/actions.py` Avoids a circular import between actions.py and tasks.py * test: unit test `/integrations` patch calls (#437) * test: unit test `/integrations` patch calls * fix: token expiry logic (#440) * fix: token expiry logic * fix: rename invalidate_token to invalidate_xero_credentials * test: unit test updates for comment fixes (#441) --- apps/exceptions.py | 3 ++ apps/fyle/helpers.py | 21 +++++++++++ apps/mappings/actions.py | 2 ++ apps/mappings/exceptions.py | 5 +++ apps/workspaces/actions.py | 5 +++ apps/workspaces/helpers.py | 44 +++++++++++++++++++++++ apps/xero/exceptions.py | 6 ++-- apps/xero/queue.py | 3 ++ apps/xero/tasks.py | 4 +++ tests/test_exceptions.py | 20 +++++++++++ tests/test_workspaces/test_actions.py | 52 +++++++++++++++++++++++++++ tests/test_workspaces/test_views.py | 8 +++++ tests/test_xero/test_exceptions.py | 31 ++++++++++++++++ 13 files changed, 202 insertions(+), 2 deletions(-) create mode 100644 apps/workspaces/helpers.py create mode 100644 tests/test_exceptions.py create mode 100644 tests/test_workspaces/test_actions.py create mode 100644 tests/test_xero/test_exceptions.py diff --git a/apps/exceptions.py b/apps/exceptions.py index b3b28b96..f4a33114 100644 --- a/apps/exceptions.py +++ b/apps/exceptions.py @@ -14,6 +14,7 @@ from apps.fyle.models import ExpenseGroup from apps.mappings.models import GeneralMapping, TenantMapping +from apps.workspaces.helpers import invalidate_xero_credentials from apps.workspaces.models import FyleCredential, Workspace, WorkspaceGeneralSettings, WorkspaceSchedule, XeroCredentials logger = logging.getLogger(__name__) @@ -49,6 +50,7 @@ def new_fn(*args, **kwargs): kwargs["workspace_id"], {"error": exception.response}, ) + invalidate_xero_credentials(kwargs["workspace_id"]) return Response( data={"message": "Xero token expired workspace_id"}, status=status.HTTP_400_BAD_REQUEST, @@ -72,6 +74,7 @@ def new_fn(*args, **kwargs): InvalidClientError, ) as exception: logger.info(exception) + invalidate_xero_credentials(kwargs["workspace_id"]) return Response( data={"message": "Xero connection expired"}, status=status.HTTP_400_BAD_REQUEST, diff --git a/apps/fyle/helpers.py b/apps/fyle/helpers.py index 8e7b9ca6..ce8604af 100644 --- a/apps/fyle/helpers.py +++ b/apps/fyle/helpers.py @@ -38,6 +38,27 @@ def post_request(url, body, refresh_token=None): raise Exception(response.text) +def patch_request(url, body, refresh_token=None): + """ + Create a HTTP patch request. + """ + access_token = None + api_headers = { + 'content-type': 'application/json', + } + if refresh_token: + access_token = get_access_token(refresh_token) + + api_headers['Authorization'] = 'Bearer {0}'.format(access_token) + + response = requests.patch(url, headers=api_headers, data=json.dumps(body)) + + if response.status_code in [200, 201]: + return json.loads(response.text) + else: + raise Exception(response.text) + + def get_source_account_type(fund_source: List[str]) -> List[str]: """ Get source account type diff --git a/apps/mappings/actions.py b/apps/mappings/actions.py index 28ab23e3..a7988c97 100644 --- a/apps/mappings/actions.py +++ b/apps/mappings/actions.py @@ -2,6 +2,7 @@ from xerosdk.exceptions import UnsuccessfulAuthentication +from apps.exceptions import invalidate_xero_credentials from apps.mappings.models import TenantMapping from apps.mappings.utils import MappingUtils from apps.workspaces.models import Workspace, XeroCredentials @@ -47,6 +48,7 @@ def create_tenant_mapping(workspace_id, tenant_mapping_payload): except UnsuccessfulAuthentication: logger.info('Xero refresh token is invalid for workspace_id - %s', workspace_id) + invalidate_xero_credentials(workspace_id) except Exception: logger.info('Error while fetching company information') diff --git a/apps/mappings/exceptions.py b/apps/mappings/exceptions.py index 680ba76f..795a31e8 100644 --- a/apps/mappings/exceptions.py +++ b/apps/mappings/exceptions.py @@ -16,6 +16,7 @@ WrongParamsError as XeroWrongParamsError ) +from apps.workspaces.helpers import invalidate_xero_credentials from apps.workspaces.models import XeroCredentials from fyle_integrations_imports.models import ImportLog @@ -58,6 +59,7 @@ def new_fn(workspace_id, *args): except (XeroWrongParamsError, XeroInvalidTokenError) as exception: error["message"] = "Xero token expired" error["response"] = exception.__dict__ + invalidate_xero_credentials(workspace_id) except PlatformError as exception: error["message"] = "Platform error while importing to Fyle" @@ -65,6 +67,7 @@ def new_fn(workspace_id, *args): except (UnsuccessfulAuthentication, InvalidGrant): error["message"] = "Xero refresh token is invalid" + invalidate_xero_credentials(workspace_id) except Exception: response = traceback.format_exc() @@ -117,12 +120,14 @@ def new_fn(expense_attribute_instance, *args): error['alert'] = False error['response'] = exception.__dict__ import_log.status = 'FAILED' + invalidate_xero_credentials(workspace_id) except UnsuccessfulAuthentication as exception: error["message"] = "Invalid xero tenant ID or xero-tenant-id header missing in workspace_id - {0}".format(workspace_id) error["alert"] = False error['response'] = exception.__dict__ import_log.status = 'FAILED' + invalidate_xero_credentials(workspace_id) except Exception: response = traceback.format_exc() diff --git a/apps/workspaces/actions.py b/apps/workspaces/actions.py index c70a19b4..98920052 100644 --- a/apps/workspaces/actions.py +++ b/apps/workspaces/actions.py @@ -3,6 +3,8 @@ from django.contrib.auth import get_user_model from django.core.cache import cache +from apps.exceptions import invalidate_xero_credentials +from apps.workspaces.helpers import patch_integration_settings from django_q.tasks import async_task from fyle_accounting_mappings.models import ExpenseAttribute from fyle_rest_auth.helpers import get_fyle_admin @@ -94,6 +96,8 @@ def connect_xero(authorization_code, redirect_uri, workspace_id): xero_credentials.is_expired = False xero_credentials.save() + patch_integration_settings(workspace_id, is_token_expired=False) + if tenant_mapping and not tenant_mapping.connection_id: xero_connector = XeroConnector(xero_credentials, workspace_id=workspace_id) connections = xero_connector.connection.connections.get_all() @@ -120,6 +124,7 @@ def connect_xero(authorization_code, redirect_uri, workspace_id): xero_exc.WrongParamsError, xero_exc.UnsuccessfulAuthentication, ) as exception: + invalidate_xero_credentials(workspace_id) logger.info(exception.response) if workspace.onboarding_state == 'CONNECTION': diff --git a/apps/workspaces/helpers.py b/apps/workspaces/helpers.py new file mode 100644 index 00000000..5aabadc2 --- /dev/null +++ b/apps/workspaces/helpers.py @@ -0,0 +1,44 @@ +import logging +from django.conf import settings +from apps.fyle.helpers import patch_request +from apps.workspaces.models import FyleCredential, XeroCredentials + +logger = logging.getLogger(__name__) +logger.level = logging.INFO + + +def patch_integration_settings(workspace_id: int, errors: int = None, is_token_expired = None): + """ + Patch integration settings + """ + + refresh_token = FyleCredential.objects.get(workspace_id=workspace_id).refresh_token + url = '{}/integrations/'.format(settings.INTEGRATIONS_SETTINGS_API) + payload = { + 'tpa_name': 'Fyle Xero Integration', + } + + if errors is not None: + payload['errors_count'] = errors + + if is_token_expired is not None: + payload['is_token_expired'] = is_token_expired + + try: + patch_request(url, payload, refresh_token) + except Exception as error: + logger.error(error, exc_info=True) + + +def invalidate_xero_credentials(workspace_id): + try: + xero_credentials = XeroCredentials.get_active_xero_credentials(workspace_id) + if xero_credentials: + if not xero_credentials.is_expired: + patch_integration_settings(workspace_id, is_token_expired=True) + xero_credentials.refresh_token = None + xero_credentials.is_expired = True + xero_credentials.save() + except XeroCredentials.DoesNotExist as error: + logger.error(f'Xero credentials not found for {workspace_id = }:', ) + logger.error(error, exc_info=True) diff --git a/apps/xero/exceptions.py b/apps/xero/exceptions.py index aaddfc3b..c8b88210 100644 --- a/apps/xero/exceptions.py +++ b/apps/xero/exceptions.py @@ -17,6 +17,7 @@ from apps.tasks.enums import ErrorTypeEnum, TaskLogStatusEnum, TaskLogTypeEnum from apps.tasks.models import Error, TaskLog from apps.workspaces.models import FyleCredential, LastExportDetail, XeroCredentials +from apps.workspaces.helpers import invalidate_xero_credentials, patch_integration_settings from fyle_xero_api.exceptions import BulkError logger = logging.getLogger(__name__) @@ -44,6 +45,8 @@ def update_last_export_details(workspace_id): last_export_detail.total_expense_groups_count = failed_exports + successful_exports last_export_detail.save() + patch_integration_settings(workspace_id, errors=failed_exports) + return last_export_detail @@ -189,12 +192,11 @@ def new_fn(*args): ) except (NoPrivilegeError, UnsuccessfulAuthentication) as exception: + invalidate_xero_credentials(workspace_id) xero_credentials = XeroCredentials.objects.filter( workspace_id=workspace_id ).first() - xero_credentials.refresh_token = None xero_credentials.country = None - xero_credentials.is_expired = True xero_credentials.save() logger.info(exception.message) task_log.status = TaskLogStatusEnum.FAILED diff --git a/apps/xero/queue.py b/apps/xero/queue.py index 5dbc0d59..55d4671f 100644 --- a/apps/xero/queue.py +++ b/apps/xero/queue.py @@ -3,6 +3,7 @@ import logging from django.db.models import Q +from apps.workspaces.helpers import invalidate_xero_credentials from django_q.models import Schedule from django_q.tasks import Chain from xerosdk.exceptions import InvalidGrant, UnsuccessfulAuthentication @@ -181,6 +182,7 @@ def schedule_bills_creation(workspace_id: int, expense_group_ids: List[str], is_ xero_connection = XeroConnector(xero_credentials, workspace_id) __create_chain_and_run(fyle_credentials, xero_connection, in_progress_expenses, workspace_id, chain_tasks, fund_source) except (UnsuccessfulAuthentication, InvalidGrant, XeroCredentials.DoesNotExist): + invalidate_xero_credentials(workspace_id) xero_connection = None for task in chain_tasks: task_log = TaskLog.objects.get(id=task['task_log_id']) @@ -258,6 +260,7 @@ def schedule_bank_transaction_creation( xero_connection = XeroConnector(xero_credentials, workspace_id) __create_chain_and_run(fyle_credentials, xero_connection, in_progress_expenses, workspace_id, chain_tasks, fund_source) except (UnsuccessfulAuthentication, InvalidGrant, XeroCredentials.DoesNotExist): + invalidate_xero_credentials(workspace_id) xero_connection = None for task in chain_tasks: task_log = TaskLog.objects.get(id=task['task_log_id']) diff --git a/apps/xero/tasks.py b/apps/xero/tasks.py index d59b10f9..8d2ce813 100644 --- a/apps/xero/tasks.py +++ b/apps/xero/tasks.py @@ -19,6 +19,7 @@ from apps.mappings.models import GeneralMapping, TenantMapping from apps.tasks.enums import ErrorTypeEnum, TaskLogStatusEnum, TaskLogTypeEnum from apps.tasks.models import Error, TaskLog +from apps.workspaces.helpers import invalidate_xero_credentials from apps.workspaces.models import FyleCredential, Workspace, WorkspaceGeneralSettings, XeroCredentials from apps.xero.exceptions import handle_xero_exceptions from apps.xero.models import BankTransaction, BankTransactionLineItem, Bill, BillLineItem, Payment @@ -830,6 +831,7 @@ def check_xero_object_status(workspace_id): "Xero refresh token expired for workspace_id %s", workspace_id, ) + invalidate_xero_credentials(workspace_id) def process_reimbursements(workspace_id): @@ -927,6 +929,7 @@ def create_missing_currency(workspace_id: int): "Xero refresh token expired for workspace_id %s", workspace_id, ) + invalidate_xero_credentials(workspace_id) except Exception as exception: logger.exception("Error creating currency in Xero", exception) @@ -957,6 +960,7 @@ def update_xero_short_code(workspace_id: int): "Xero refresh token expired for workspace_id %s", workspace_id, ) + invalidate_xero_credentials(workspace_id) except Exception as exception: logger.exception("Error updating Xero short code", exception) diff --git a/tests/test_exceptions.py b/tests/test_exceptions.py new file mode 100644 index 00000000..900d2602 --- /dev/null +++ b/tests/test_exceptions.py @@ -0,0 +1,20 @@ +from unittest.mock import MagicMock +from xerosdk.exceptions import UnsuccessfulAuthentication +from apps.exceptions import handle_view_exceptions + + +def test_handle_view_exceptions(mocker): + workspace_id = 123 + + mocked_invalidate_call = MagicMock() + mocker.patch('apps.exceptions.invalidate_xero_credentials', side_effect=mocked_invalidate_call) + + @handle_view_exceptions() + def func(*args, **kwargs): + raise UnsuccessfulAuthentication('Invalid Token') + + func(workspace_id=workspace_id) + + args, _ = mocked_invalidate_call.call_args + + assert args[0] == workspace_id diff --git a/tests/test_workspaces/test_actions.py b/tests/test_workspaces/test_actions.py new file mode 100644 index 00000000..78aefcd1 --- /dev/null +++ b/tests/test_workspaces/test_actions.py @@ -0,0 +1,52 @@ + +import json +from unittest.mock import MagicMock +import pytest + +from apps.workspaces.actions import patch_integration_settings + + +@pytest.mark.django_db(databases=['default']) +def test_patch_integration_settings(mocker): + + workspace_id = 1 + mocked_patch = MagicMock() + mocker.patch('apps.fyle.helpers.requests.patch', side_effect=mocked_patch) + + # Test patch request with only errors + errors = 7 + patch_integration_settings(workspace_id, errors) + + expected_payload = {'errors_count': errors, 'tpa_name': 'Fyle Xero Integration'} + + _, kwargs = mocked_patch.call_args + actual_payload = json.loads(kwargs['data']) + + assert actual_payload == expected_payload + + # Test patch request with only is_token_expired + is_token_expired = True + patch_integration_settings(workspace_id, is_token_expired=is_token_expired) + + expected_payload = {'is_token_expired': is_token_expired, 'tpa_name': 'Fyle Xero Integration'} + + _, kwargs = mocked_patch.call_args + actual_payload = json.loads(kwargs['data']) + + assert actual_payload == expected_payload + + # Test patch request with errors_count and is_token_expired + is_token_expired = True + errors = 241 + patch_integration_settings(workspace_id, errors=errors, is_token_expired=is_token_expired) + + expected_payload = { + 'errors_count': errors, + 'is_token_expired': is_token_expired, + 'tpa_name': 'Fyle Xero Integration' + } + + _, kwargs = mocked_patch.call_args + actual_payload = json.loads(kwargs['data']) + + assert actual_payload == expected_payload diff --git a/tests/test_workspaces/test_views.py b/tests/test_workspaces/test_views.py index d07c0dd2..2ae67e59 100644 --- a/tests/test_workspaces/test_views.py +++ b/tests/test_workspaces/test_views.py @@ -115,6 +115,9 @@ def test_connect_xero_view_post(mocker, api_client, test_connection): return_value=xero_data["get_all_organisations"], ) + mocked_patch = mock.MagicMock() + mocker.patch('apps.workspaces.actions.patch_integration_settings', side_effect=mocked_patch) + code = "asdfghj" url = "/api/workspaces/{}/connect_xero/authorization_code/".format(workspace_id) @@ -125,6 +128,11 @@ def test_connect_xero_view_post(mocker, api_client, test_connection): response = api_client.post(url, data={"code": code}) assert response.status_code == 200 + args, kwargs = mocked_patch.call_args + + assert args[0] == workspace_id + assert kwargs['is_token_expired'] == False + xero_credentials = XeroCredentials.objects.get(workspace_id=workspace_id) xero_credentials.delete() diff --git a/tests/test_xero/test_exceptions.py b/tests/test_xero/test_exceptions.py new file mode 100644 index 00000000..3deff261 --- /dev/null +++ b/tests/test_xero/test_exceptions.py @@ -0,0 +1,31 @@ +from unittest.mock import MagicMock +from xerosdk.exceptions import NoPrivilegeError +from apps.xero.exceptions import handle_xero_exceptions + + +def test_handle_view_exceptions(mocker, db): + workspace_id = 1 + task_log_id = 1 + + mocked_invalidate_call = MagicMock() + mocker.patch('apps.xero.exceptions.invalidate_xero_credentials', side_effect=mocked_invalidate_call) + + @handle_xero_exceptions(payment=False) + def func(expense_group_id: int, task_log_id: int, xero_connection): + raise NoPrivilegeError('Invalid Token') + + func(workspace_id, task_log_id, MagicMock()) + + args, _ = mocked_invalidate_call.call_args + + assert args[0] == workspace_id + + @handle_xero_exceptions(payment=True) + def func2(bill, workspace_id: int, task_log): + raise NoPrivilegeError('Invalid Token') + + func2(MagicMock(), workspace_id, MagicMock()) + + args, _ = mocked_invalidate_call.call_args + + assert args[0] == workspace_id From 6a1f55e4c581d92ca5f057482ab2204f61700e03 Mon Sep 17 00:00:00 2001 From: Viswas Haridas <37623357+JustARatherRidiculouslyLongUsername@users.noreply.github.com> Date: Fri, 14 Feb 2025 15:22:05 +0530 Subject: [PATCH 2/2] fix: POST to `/integrations` on disconnect and reconnect (#434) --- apps/workspaces/views.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/apps/workspaces/views.py b/apps/workspaces/views.py index 19da105e..4ba1c025 100644 --- a/apps/workspaces/views.py +++ b/apps/workspaces/views.py @@ -1,6 +1,7 @@ import logging from django.contrib.auth import get_user_model +from apps.workspaces.tasks import post_to_integration_settings from django_q.tasks import async_task from fyle_rest_auth.utils import AuthUtils from rest_framework import generics @@ -118,6 +119,8 @@ def post(self, request, **kwargs): workspace_id=kwargs["workspace_id"], ) + post_to_integration_settings(kwargs["workspace_id"], True) + return Response( data=XeroCredentialSerializer(xero_credentials).data, status=status.HTTP_200_OK, @@ -152,6 +155,7 @@ def post(self, request, **kwargs): """ # TODO: cleanup later - merge with ConnectXeroView revoke_connections(workspace_id=kwargs["workspace_id"]) + post_to_integration_settings(kwargs["workspace_id"], False) return Response( data={"message": "Revoked Xero connection"}, status=status.HTTP_200_OK