From d1868d4ebf9893a8be359389aaca1d27858fcbe8 Mon Sep 17 00:00:00 2001 From: Viswas Haridas <37623357+JustARatherRidiculouslyLongUsername@users.noreply.github.com> Date: Wed, 21 Aug 2024 11:55:04 +0530 Subject: [PATCH] fix: improve response times of sync/refresh dimensions requests (#661) * fix(apps/qbo): delegate sync dimensions tasks to workers * fix(apps/fyle): delegate sync dimensions tasks to workers * fix: respond with a 400 for invalid workspaces/creds * refactor: linting * test: add unit tests for functions that have been made async --- apps/fyle/views.py | 18 +++++++++--- apps/quickbooks_online/views.py | 18 ++++++++++-- tests/test_fyle/test_actions.py | 27 ++++++++++++++++++ tests/test_quickbooks_online/test_actions.py | 30 +++++++++++++++++++- 4 files changed, 85 insertions(+), 8 deletions(-) diff --git a/apps/fyle/views.py b/apps/fyle/views.py index b30e9b9d..eff524e5 100644 --- a/apps/fyle/views.py +++ b/apps/fyle/views.py @@ -1,7 +1,10 @@ import logging + from apps.fyle.helpers import ExpenseGroupSearchFilter, ExpenseSearchFilter +from apps.workspaces.models import FyleCredential, Workspace from django_filters.rest_framework import DjangoFilterBackend +from django_q.tasks import async_task from rest_framework import generics from rest_framework.response import Response from rest_framework.views import status @@ -11,8 +14,6 @@ get_custom_fields, get_expense_fields, get_expense_group_ids, - refresh_fyle_dimension, - sync_fyle_dimensions, ) from apps.fyle.models import Expense, ExpenseFilter, ExpenseGroup, ExpenseGroupSettings from apps.fyle.queue import async_import_and_export_expenses @@ -120,7 +121,12 @@ def post(self, request, *args, **kwargs): """ Sync Data From Fyle """ - sync_fyle_dimensions(workspace_id=kwargs['workspace_id']) + + # Check for a valid workspace and fyle creds and respond with 400 if not found + Workspace.objects.get(id=kwargs['workspace_id']) + FyleCredential.objects.get(workspace_id=kwargs['workspace_id']) + + async_task('apps.fyle.actions.sync_fyle_dimensions', kwargs['workspace_id']) return Response(status=status.HTTP_200_OK) @@ -136,7 +142,11 @@ def post(self, request, *args, **kwargs): Sync data from Fyle """ - refresh_fyle_dimension(workspace_id=kwargs['workspace_id']) + # Check for a valid workspace and fyle creds and respond with 400 if not found + Workspace.objects.get(id=kwargs['workspace_id']) + FyleCredential.objects.get(workspace_id=kwargs['workspace_id']) + + async_task('apps.fyle.actions.refresh_fyle_dimension', kwargs['workspace_id']) return Response(status=status.HTTP_200_OK) diff --git a/apps/quickbooks_online/views.py b/apps/quickbooks_online/views.py index 833aa40a..18f3900d 100644 --- a/apps/quickbooks_online/views.py +++ b/apps/quickbooks_online/views.py @@ -1,7 +1,9 @@ import logging from django.db.models import Q +from apps.workspaces.models import QBOCredential, Workspace from django_filters.rest_framework import DjangoFilterBackend +from django_q.tasks import async_task from fyle_accounting_mappings.models import DestinationAttribute from fyle_accounting_mappings.serializers import DestinationAttributeSerializer from rest_framework import generics @@ -9,7 +11,7 @@ from rest_framework.views import status from apps.exceptions import handle_view_exceptions -from apps.quickbooks_online.actions import get_preferences, refresh_quickbooks_dimensions, sync_quickbooks_dimensions +from apps.quickbooks_online.actions import get_preferences from fyle_qbo_api.utils import LookupFieldMixin from .serializers import QuickbooksFieldSerializer @@ -73,7 +75,12 @@ class SyncQuickbooksDimensionView(generics.ListCreateAPIView): @handle_view_exceptions() def post(self, request, *args, **kwargs): - sync_quickbooks_dimensions(kwargs['workspace_id']) + + # Check for a valid workspace and qbo creds and respond with 400 if not found + Workspace.objects.get(id=kwargs['workspace_id']) + QBOCredential.get_active_qbo_credentials(kwargs['workspace_id']) + + async_task('apps.quickbooks_online.actions.sync_quickbooks_dimensions', kwargs['workspace_id']) return Response(status=status.HTTP_200_OK) @@ -88,7 +95,12 @@ def post(self, request, *args, **kwargs): """ Sync data from quickbooks """ - refresh_quickbooks_dimensions(kwargs['workspace_id']) + + # Check for a valid workspace and qbo creds and respond with 400 if not found + Workspace.objects.get(id=kwargs['workspace_id']) + QBOCredential.get_active_qbo_credentials(kwargs['workspace_id']) + + async_task('apps.quickbooks_online.actions.refresh_quickbooks_dimensions', kwargs['workspace_id']) return Response(status=status.HTTP_200_OK) diff --git a/tests/test_fyle/test_actions.py b/tests/test_fyle/test_actions.py index ee6e5384..240b1df5 100644 --- a/tests/test_fyle/test_actions.py +++ b/tests/test_fyle/test_actions.py @@ -8,6 +8,8 @@ from apps.fyle.models import Expense, ExpenseGroup from apps.fyle.actions import ( + refresh_fyle_dimension, + sync_fyle_dimensions, update_expenses_in_progress, mark_expenses_as_skipped, mark_accounting_export_summary_as_synced, @@ -164,3 +166,28 @@ def test_handle_post_accounting_export_summary_exception(db): settings.QBO_INTEGRATION_APP_URL ) assert expense.accounting_export_summary['id'] == expense_id + + +def test_sync_fyle_dimensions(db): + workspace = Workspace.objects.get(id=3) + + with mock.patch('fyle_integrations_platform_connector.fyle_integrations_platform_connector.PlatformConnector.import_fyle_dimensions') as mock_call: + mock_call.return_value = None + + sync_fyle_dimensions(3) + assert workspace.source_synced_at is not None + + # If dimensions were synced ≤ 1 day ago, they shouldn't be synced again + old_source_synced_at = workspace.source_synced_at + sync_fyle_dimensions(3) + assert workspace.source_synced_at == old_source_synced_at + + +def test_refresh_fyle_dimension(db): + workspace = Workspace.objects.get(id=3) + + with mock.patch('fyle_integrations_platform_connector.fyle_integrations_platform_connector.PlatformConnector.import_fyle_dimensions') as mock_call: + mock_call.return_value = None + + refresh_fyle_dimension(3) + assert workspace.source_synced_at is not None diff --git a/tests/test_quickbooks_online/test_actions.py b/tests/test_quickbooks_online/test_actions.py index 9b2be568..a0a57189 100644 --- a/tests/test_quickbooks_online/test_actions.py +++ b/tests/test_quickbooks_online/test_actions.py @@ -1,5 +1,7 @@ +from unittest import mock from apps.fyle.models import ExpenseGroup -from apps.quickbooks_online.actions import generate_export_url_and_update_expense +from apps.quickbooks_online.actions import generate_export_url_and_update_expense, refresh_quickbooks_dimensions, sync_quickbooks_dimensions +from apps.workspaces.models import Workspace def test_generate_export_url_and_update_expense(db): @@ -14,3 +16,29 @@ def test_generate_export_url_and_update_expense(db): assert expense.accounting_export_summary['error_type'] == None assert expense.accounting_export_summary['id'] == expense.expense_id assert expense.accounting_export_summary['state'] == 'COMPLETE' + + +def test_refresh_quickbooks_dimensions(db): + workspace_id = 1 + workspace = Workspace.objects.get(id=3) + + with mock.patch('apps.quickbooks_online.utils.QBOConnector.sync_dimensions') as mock_call: + mock_call.return_value = None + + refresh_quickbooks_dimensions(workspace_id) + assert workspace.destination_synced_at is not None + + +def test_sync_quickbooks_dimensions(db): + workspace = Workspace.objects.get(id=3) + + with mock.patch('apps.quickbooks_online.utils.QBOConnector.sync_dimensions') as mock_call: + mock_call.return_value = None + + sync_quickbooks_dimensions(3) + assert workspace.destination_synced_at is not None + + # If dimensions were synced ≤ 1 day ago, they shouldn't be synced again + old_destination_synced_at = workspace.destination_synced_at + sync_quickbooks_dimensions(3) + assert workspace.destination_synced_at == old_destination_synced_at