Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: PATCH /integrations on export errors and token expiry #442

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions apps/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down
21 changes: 21 additions & 0 deletions apps/fyle/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions apps/mappings/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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')
Expand Down
5 changes: 5 additions & 0 deletions apps/mappings/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -58,13 +59,15 @@ 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"
error["response"] = exception.response

except (UnsuccessfulAuthentication, InvalidGrant):
error["message"] = "Xero refresh token is invalid"
invalidate_xero_credentials(workspace_id)

except Exception:
response = traceback.format_exc()
Expand Down Expand Up @@ -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()
Expand Down
5 changes: 5 additions & 0 deletions apps/workspaces/actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand 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':
Expand Down
44 changes: 44 additions & 0 deletions apps/workspaces/helpers.py
Original file line number Diff line number Diff line change
@@ -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)
Comment on lines +33 to +44
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix error message formatting and improve error handling.

The f-string in the error message has an invalid syntax, and the error handling could be more specific.

Apply this diff to fix the issues:

 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(f'Xero credentials not found for workspace_id: {workspace_id}')
         logger.error(error, exc_info=True)
+    except Exception as error:
+        logger.error(f'Unexpected error while invalidating Xero credentials for workspace_id: {workspace_id}')
+        logger.error(error, exc_info=True)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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)
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: {workspace_id}')
logger.error(error, exc_info=True)
except Exception as error:
logger.error(f'Unexpected error while invalidating Xero credentials for workspace_id: {workspace_id}')
logger.error(error, exc_info=True)

4 changes: 4 additions & 0 deletions apps/workspaces/views.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions apps/xero/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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


Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions apps/xero/queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'])
Expand Down Expand Up @@ -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'])
Expand Down
4 changes: 4 additions & 0 deletions apps/xero/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
20 changes: 20 additions & 0 deletions tests/test_exceptions.py
Original file line number Diff line number Diff line change
@@ -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
52 changes: 52 additions & 0 deletions tests/test_workspaces/test_actions.py
Original file line number Diff line number Diff line change
@@ -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
8 changes: 8 additions & 0 deletions tests/test_workspaces/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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()

Expand Down
Loading
Loading