From 44e862e45f265e3632c41c4d35ad11b1865cd868 Mon Sep 17 00:00:00 2001 From: Viswas Haridas <37623357+JustARatherRidiculouslyLongUsername@users.noreply.github.com> Date: Tue, 20 Aug 2024 17:51:18 +0530 Subject: [PATCH] fix: improve response times of sync/refresh dimensions requests (#633) * fix(apps/netsuite): delegate sync_dimension tasks to workers * fix(apps/fyle): delegate sync_dimension tasks to workers * test: update unit tests for helpers and views (cherry picked from commit 7d610447275f99b7224cff6e8a64463b641b43ca) --- apps/fyle/helpers.py | 22 ++-- apps/fyle/views.py | 16 +-- apps/mappings/queue.py | 32 +---- apps/netsuite/helpers.py | 181 ++++++++++++++++++++++++++-- apps/netsuite/views.py | 144 +--------------------- tests/test_fyle/test_helpers.py | 14 +-- tests/test_fyle/test_views.py | 4 +- tests/test_netsuite/test_helpers.py | 12 +- tests/test_netsuite/test_views.py | 4 +- 9 files changed, 216 insertions(+), 213 deletions(-) diff --git a/apps/fyle/helpers.py b/apps/fyle/helpers.py index 412866c3..9d99cc65 100644 --- a/apps/fyle/helpers.py +++ b/apps/fyle/helpers.py @@ -277,25 +277,26 @@ def update_use_employee_attributes_flag(workspace_id: int) -> None: general_mapping.save() -def check_interval_and_sync_dimension(workspace: Workspace, fyle_credentials: FyleCredential) -> bool: +def check_interval_and_sync_dimension(workspace_id: int): """ Check sync interval and sync dimension - :param workspace: Workspace Instance - :param refresh_token: Refresh token of an org - - return: True/False based on sync + :param workspace_id: Workspace ID """ + + workspace = Workspace.objects.get(pk=workspace_id) + if workspace.source_synced_at: time_interval = datetime.now(timezone.utc) - workspace.source_synced_at if workspace.source_synced_at is None or time_interval.days > 0: - sync_dimensions(fyle_credentials) - return True + sync_dimensions(workspace_id) - return False -def sync_dimensions(fyle_credentials: FyleCredential, is_export: bool = False) -> None: +def sync_dimensions(workspace_id, is_export: bool = False) -> None: + workspace = Workspace.objects.get(id=workspace_id) + fyle_credentials = FyleCredential.objects.get(workspace_id=workspace.id) + platform = PlatformConnector(fyle_credentials) platform.import_fyle_dimensions(is_export=is_export) if is_export: @@ -317,6 +318,9 @@ def sync_dimensions(fyle_credentials: FyleCredential, is_export: bool = False) - if projects_count != projects_expense_attribute_count: platform.projects.sync() + workspace.source_synced_at = datetime.now() + workspace.save(update_fields=['source_synced_at']) + def construct_expense_filter_query(expense_filters: List[ExpenseFilter]): final_filter = None for expense_filter in expense_filters: diff --git a/apps/fyle/views.py b/apps/fyle/views.py index 1243a8cf..e242f64a 100644 --- a/apps/fyle/views.py +++ b/apps/fyle/views.py @@ -17,7 +17,7 @@ from fyle_netsuite_api.utils import LookupFieldMixin from .tasks import schedule_expense_group_creation, get_task_log_and_fund_source, create_expense_groups -from .helpers import ExpenseGroupSearchFilter, ExpenseSearchFilter, check_interval_and_sync_dimension, sync_dimensions +from .helpers import ExpenseGroupSearchFilter, ExpenseSearchFilter, check_interval_and_sync_dimension from .models import Expense, ExpenseGroup, ExpenseGroupSettings, ExpenseFilter from .serializers import ExpenseGroupSerializer, ExpenseSerializer, ExpenseFieldSerializer, \ ExpenseGroupSettingsSerializer, ExpenseFilterSerializer, ExpenseGroupExpenseSerializer @@ -27,6 +27,7 @@ from apps.exceptions import handle_view_exceptions from django_filters.rest_framework import DjangoFilterBackend +from django_q.tasks import async_task logger = logging.getLogger(__name__) logger.level = logging.INFO @@ -264,12 +265,9 @@ def post(self, request, *args, **kwargs): """ try: workspace = Workspace.objects.get(pk=kwargs['workspace_id']) - fyle_credentials = FyleCredential.objects.get(workspace_id=workspace.id) + FyleCredential.objects.get(workspace_id=workspace.id) - synced = check_interval_and_sync_dimension(workspace, fyle_credentials) - if synced: - workspace.source_synced_at = datetime.now() - workspace.save(update_fields=['source_synced_at']) + async_task('apps.fyle.helpers.check_interval_and_sync_dimension', kwargs['workspace_id']) return Response( status=status.HTTP_200_OK @@ -300,11 +298,9 @@ def post(self, request, *args, **kwargs): """ try: workspace = Workspace.objects.get(id=kwargs['workspace_id']) - fyle_credentials = FyleCredential.objects.get(workspace_id=workspace.id) - sync_dimensions(fyle_credentials) + FyleCredential.objects.get(workspace_id=workspace.id) - workspace.source_synced_at = datetime.now() - workspace.save(update_fields=['source_synced_at']) + async_task('apps.fyle.helpers.sync_dimensions', kwargs['workspace_id']) return Response( status=status.HTTP_200_OK diff --git a/apps/mappings/queue.py b/apps/mappings/queue.py index b785d4fa..bb492cfd 100644 --- a/apps/mappings/queue.py +++ b/apps/mappings/queue.py @@ -5,37 +5,7 @@ from apps.mappings.helpers import is_auto_sync_allowed from apps.mappings.constants import SYNC_METHODS from apps.mappings.models import GeneralMapping -from apps.netsuite.helpers import sync_override_tax_items - - -def get_import_categories_settings(configurations: Configuration): - """ - Get import categories settings - :return: is_3d_mapping_enabled, destination_field, destination_sync_methods - """ - destination_sync_methods = [] - destination_field = None - is_3d_mapping_enabled = False - - if configurations.import_items: - destination_sync_methods.append(SYNC_METHODS['ITEM']) - - if (configurations.reimbursable_expenses_object and configurations.reimbursable_expenses_object == 'EXPENSE REPORT') or configurations.corporate_credit_card_expenses_object == 'EXPENSE REPORT': - destination_sync_methods.append(SYNC_METHODS['EXPENSE_CATEGORY']) - destination_field = 'EXPENSE_CATEGORY' - - if configurations.reimbursable_expenses_object != 'EXPENSE REPORT' and ( - configurations.reimbursable_expenses_object in ('BILL', 'JOURNAL ENTRY') - or configurations.corporate_credit_card_expenses_object in ('BILL', 'JOURNAL ENTRY', 'CREDIT CARD CHARGE')): - destination_sync_methods.append(SYNC_METHODS['ACCOUNT']) - destination_field = 'ACCOUNT' - - if configurations.reimbursable_expenses_object == 'EXPENSE REPORT' and \ - configurations.corporate_credit_card_expenses_object in ('BILL', 'CREDIT CARD CHARGE', 'JOURNAL ENTRY'): - is_3d_mapping_enabled = True - - return is_3d_mapping_enabled, destination_field, destination_sync_methods - +from apps.netsuite.helpers import get_import_categories_settings, sync_override_tax_items def construct_tasks_and_chain_import_fields_to_fyle(workspace_id: int): """ diff --git a/apps/netsuite/helpers.py b/apps/netsuite/helpers.py index c192bb32..fafe58ac 100644 --- a/apps/netsuite/helpers.py +++ b/apps/netsuite/helpers.py @@ -3,8 +3,13 @@ from django.utils.module_loading import import_string +from apps.mappings.constants import SYNC_METHODS +from apps.mappings.helpers import is_auto_sync_allowed +from apps.mappings.models import GeneralMapping from apps.workspaces.models import Configuration, Workspace, NetSuiteCredentials from apps.netsuite.connector import NetSuiteConnector +from fyle_accounting_mappings.models import MappingSetting +from django_q.tasks import Chain from .tasks import schedule_vendor_payment_creation, schedule_netsuite_objects_status_sync, \ schedule_reimbursements_sync @@ -45,22 +50,21 @@ def schedule_payment_sync(configuration: Configuration): workspace_id=configuration.workspace_id ) -def check_interval_and_sync_dimension(workspace: Workspace, netsuite_credentials: NetSuiteCredentials) -> bool: +def check_interval_and_sync_dimension(workspace_id): """ Check sync interval and sync dimension - :param workspace: Workspace Instance - :param netsuite_credentials: NetSuiteCredentials Instance - - return: True/False based on sync + :param workspace_id: Workspace ID """ + workspace = Workspace.objects.get(pk=workspace_id) + netsuite_credentials = NetSuiteCredentials.objects.get(workspace_id=workspace.id) + if workspace.destination_synced_at: time_interval = datetime.now(timezone.utc) - workspace.source_synced_at if workspace.destination_synced_at is None or time_interval.days > 0: sync_dimensions(netsuite_credentials, workspace.id) - return True - - return False + workspace.destination_synced_at = datetime.now() + workspace.save(update_fields=['destination_synced_at']) def sync_dimensions(ns_credentials: NetSuiteCredentials, workspace_id: int, dimensions: list = []) -> None: netsuite_connection = import_string('apps.netsuite.connector.NetSuiteConnector')(ns_credentials, workspace_id) @@ -77,3 +81,164 @@ def sync_dimensions(ns_credentials: NetSuiteCredentials, workspace_id: int, dime except Exception as exception: logger.info(exception) + +def get_import_categories_settings(configurations: Configuration): + """ + Get import categories settings + :return: is_3d_mapping_enabled, destination_field, destination_sync_methods + """ + destination_sync_methods = [] + destination_field = None + is_3d_mapping_enabled = False + + if configurations.import_items: + destination_sync_methods.append(SYNC_METHODS['ITEM']) + + if (configurations.reimbursable_expenses_object and configurations.reimbursable_expenses_object == 'EXPENSE REPORT') or configurations.corporate_credit_card_expenses_object == 'EXPENSE REPORT': + destination_sync_methods.append(SYNC_METHODS['EXPENSE_CATEGORY']) + destination_field = 'EXPENSE_CATEGORY' + + if configurations.reimbursable_expenses_object != 'EXPENSE REPORT' and ( + configurations.reimbursable_expenses_object in ('BILL', 'JOURNAL ENTRY') + or configurations.corporate_credit_card_expenses_object in ('BILL', 'JOURNAL ENTRY', 'CREDIT CARD CHARGE')): + destination_sync_methods.append(SYNC_METHODS['ACCOUNT']) + destination_field = 'ACCOUNT' + + if configurations.reimbursable_expenses_object == 'EXPENSE REPORT' and \ + configurations.corporate_credit_card_expenses_object in ('BILL', 'CREDIT CARD CHARGE', 'JOURNAL ENTRY'): + is_3d_mapping_enabled = True + + return is_3d_mapping_enabled, destination_field, destination_sync_methods + +def handle_refresh_dimensions(workspace_id, dimensions_to_sync): + + workspace = Workspace.objects.get(pk=workspace_id) + netsuite_credentials = NetSuiteCredentials.objects.get(workspace_id=workspace.id) + + mapping_settings = MappingSetting.objects.filter(workspace_id=workspace.id, import_to_fyle=True) + configurations = Configuration.objects.filter(workspace_id=workspace.id).first() + general_mappings = GeneralMapping.objects.filter(workspace_id=workspace.id).first() + workspace_id = workspace.id + + chain = Chain() + + ALLOWED_SOURCE_FIELDS = [ + "PROJECT", + "COST_CENTER", + ] + + for mapping_setting in mapping_settings: + if mapping_setting.source_field in ALLOWED_SOURCE_FIELDS or mapping_setting.is_custom: + # run new_schedule_or_delete_fyle_import_tasks + destination_sync_methods = [SYNC_METHODS.get(mapping_setting.destination_field.upper(), 'custom_segments')] + + if mapping_setting.destination_field == 'PROJECT': + destination_sync_methods.append(SYNC_METHODS['CUSTOMER']) + + chain.append( + 'fyle_integrations_imports.tasks.trigger_import_via_schedule', + workspace_id, + mapping_setting.destination_field, + mapping_setting.source_field, + 'apps.netsuite.connector.NetSuiteConnector', + netsuite_credentials, + destination_sync_methods, + is_auto_sync_allowed(configuration=configurations, mapping_setting=mapping_setting), + False, + None, + mapping_setting.is_custom, + q_options={ + 'cluster': 'import' + } + ) + + if configurations: + if configurations.import_vendors_as_merchants: + chain.append( + 'fyle_integrations_imports.tasks.trigger_import_via_schedule', + workspace_id, + 'VENDOR', + 'MERCHANT', + 'apps.netsuite.connector.NetSuiteConnector', + netsuite_credentials, + [SYNC_METHODS['VENDOR']], + False, + False, + None, + False, + q_options={ + 'cluster': 'import' + } + ) + + if configurations.import_categories: + # get import categories settings + is_3d_mapping_enabled, destination_field, destination_sync_methods = get_import_categories_settings(configurations) + chain.append( + 'fyle_integrations_imports.tasks.trigger_import_via_schedule', + workspace_id, + destination_field, + 'CATEGORY', + 'apps.netsuite.connector.NetSuiteConnector', + netsuite_credentials, + destination_sync_methods, + True, + is_3d_mapping_enabled, + None, + False, + False, + q_options={ + 'cluster': 'import' + } + ) + + if configurations.import_tax_items and general_mappings.override_tax_details: + chain.append( + 'apps.netsuite.helpers.sync_override_tax_items', + netsuite_credentials, + workspace_id, + q_options={'cluster': 'import'} + ) + chain.append( + 'fyle_integrations_imports.tasks.trigger_import_via_schedule', + workspace_id, + 'TAX_ITEM', + 'TAX_GROUP', + 'apps.netsuite.connector.NetSuiteConnector', + netsuite_credentials, + [], + False, + False, + None, + False, + q_options={ + 'cluster': 'import' + } + ) + elif configurations.import_tax_items: + chain.append( + 'fyle_integrations_imports.tasks.trigger_import_via_schedule', + workspace_id, + 'TAX_ITEM', + 'TAX_GROUP', + 'apps.netsuite.connector.NetSuiteConnector', + netsuite_credentials, + [SYNC_METHODS['TAX_ITEM']], + False, + False, + None, + False, + q_options={ + 'cluster': 'import' + } + ) + + if chain.length() > 0: + chain.run() + + sync_dimensions(netsuite_credentials, workspace.id, dimensions_to_sync) + + # Update destination_synced_at to current time only when full refresh happens + if not dimensions_to_sync: + workspace.destination_synced_at = datetime.now() + workspace.save(update_fields=['destination_synced_at']) diff --git a/apps/netsuite/views.py b/apps/netsuite/views.py index 80974fba..701e6291 100644 --- a/apps/netsuite/views.py +++ b/apps/netsuite/views.py @@ -14,17 +14,13 @@ from apps.workspaces.models import NetSuiteCredentials, Workspace, Configuration -from django_q.tasks import Chain +from django_q.tasks import async_task from .serializers import NetSuiteFieldSerializer, CustomSegmentSerializer from .tasks import create_vendor_payment, check_netsuite_object_status, process_reimbursements from .models import CustomSegment from .helpers import check_interval_and_sync_dimension, sync_dimensions from apps.workspaces.actions import export_to_netsuite -from apps.mappings.constants import SYNC_METHODS -from apps.mappings.helpers import is_auto_sync_allowed -from apps.mappings.queue import get_import_categories_settings -from apps.mappings.models import GeneralMapping logger = logging.getLogger(__name__) @@ -151,13 +147,10 @@ def post(self, request, *args, **kwargs): """ try: workspace = Workspace.objects.get(pk=kwargs['workspace_id']) - netsuite_credentials = NetSuiteCredentials.objects.get(workspace_id=workspace.id) + NetSuiteCredentials.objects.get(workspace_id=workspace.id) - synced = check_interval_and_sync_dimension(workspace, netsuite_credentials) + async_task('apps.netsuite.helpers.check_interval_and_sync_dimension', kwargs['workspace_id']) - if synced: - workspace.destination_synced_at = datetime.now() - workspace.save(update_fields=['destination_synced_at']) return Response( status=status.HTTP_200_OK @@ -189,136 +182,9 @@ def post(self, request, *args, **kwargs): try: dimensions_to_sync = request.data.get('dimensions_to_sync', []) workspace = Workspace.objects.get(pk=kwargs['workspace_id']) + NetSuiteCredentials.objects.get(workspace_id=workspace.id) - netsuite_credentials = NetSuiteCredentials.objects.get(workspace_id=workspace.id) - - mapping_settings = MappingSetting.objects.filter(workspace_id=workspace.id, import_to_fyle=True) - configurations = Configuration.objects.filter(workspace_id=workspace.id).first() - general_mappings = GeneralMapping.objects.filter(workspace_id=workspace.id).first() - workspace_id = workspace.id - - chain = Chain() - - ALLOWED_SOURCE_FIELDS = [ - "PROJECT", - "COST_CENTER", - ] - - for mapping_setting in mapping_settings: - if mapping_setting.source_field in ALLOWED_SOURCE_FIELDS or mapping_setting.is_custom: - # run new_schedule_or_delete_fyle_import_tasks - destination_sync_methods = [SYNC_METHODS.get(mapping_setting.destination_field.upper(), 'custom_segments')] - - if mapping_setting.destination_field == 'PROJECT': - destination_sync_methods.append(SYNC_METHODS['CUSTOMER']) - - chain.append( - 'fyle_integrations_imports.tasks.trigger_import_via_schedule', - workspace_id, - mapping_setting.destination_field, - mapping_setting.source_field, - 'apps.netsuite.connector.NetSuiteConnector', - netsuite_credentials, - destination_sync_methods, - is_auto_sync_allowed(configuration=configurations, mapping_setting=mapping_setting), - False, - None, - mapping_setting.is_custom, - q_options={ - 'cluster': 'import' - } - ) - - if configurations: - if configurations.import_vendors_as_merchants: - chain.append( - 'fyle_integrations_imports.tasks.trigger_import_via_schedule', - workspace_id, - 'VENDOR', - 'MERCHANT', - 'apps.netsuite.connector.NetSuiteConnector', - netsuite_credentials, - [SYNC_METHODS['VENDOR']], - False, - False, - None, - False, - q_options={ - 'cluster': 'import' - } - ) - - if configurations.import_categories: - # get import categories settings - is_3d_mapping_enabled, destination_field, destination_sync_methods = get_import_categories_settings(configurations) - chain.append( - 'fyle_integrations_imports.tasks.trigger_import_via_schedule', - workspace_id, - destination_field, - 'CATEGORY', - 'apps.netsuite.connector.NetSuiteConnector', - netsuite_credentials, - destination_sync_methods, - True, - is_3d_mapping_enabled, - None, - False, - False, - q_options={ - 'cluster': 'import' - } - ) - - if configurations.import_tax_items and general_mappings.override_tax_details: - chain.append( - 'apps.netsuite.helpers.sync_override_tax_items', - netsuite_credentials, - workspace_id, - q_options={'cluster': 'import'} - ) - chain.append( - 'fyle_integrations_imports.tasks.trigger_import_via_schedule', - workspace_id, - 'TAX_ITEM', - 'TAX_GROUP', - 'apps.netsuite.connector.NetSuiteConnector', - netsuite_credentials, - [], - False, - False, - None, - False, - q_options={ - 'cluster': 'import' - } - ) - elif configurations.import_tax_items: - chain.append( - 'fyle_integrations_imports.tasks.trigger_import_via_schedule', - workspace_id, - 'TAX_ITEM', - 'TAX_GROUP', - 'apps.netsuite.connector.NetSuiteConnector', - netsuite_credentials, - [SYNC_METHODS['TAX_ITEM']], - False, - False, - None, - False, - q_options={ - 'cluster': 'import' - } - ) - - if chain.length() > 0: - chain.run() - - sync_dimensions(netsuite_credentials, workspace.id, dimensions_to_sync) - - # Update destination_synced_at to current time only when full refresh happens - if not dimensions_to_sync: - workspace.destination_synced_at = datetime.now() - workspace.save(update_fields=['destination_synced_at']) + async_task('apps.netsuite.helpers.handle_refresh_dimensions', kwargs['workspace_id'], dimensions_to_sync) return Response( status=status.HTTP_200_OK diff --git a/tests/test_fyle/test_helpers.py b/tests/test_fyle/test_helpers.py index de3d0632..b1b19026 100644 --- a/tests/test_fyle/test_helpers.py +++ b/tests/test_fyle/test_helpers.py @@ -93,14 +93,14 @@ def test_check_interval_and_sync_dimension(access_token, mocker, db): return_value=data['get_all_tax_groups'] ) workspace = Workspace.objects.get(id=1) - fyle_credentials = FyleCredential.objects.get(workspace_id=1) - response = check_interval_and_sync_dimension(workspace, fyle_credentials) - - assert response == True + + check_interval_and_sync_dimension(workspace_id=1) + assert workspace.source_synced_at is not None - workspace.source_synced_at = datetime.now(timezone.utc) - response = check_interval_and_sync_dimension(workspace, settings.FYLE_REFRESH_TOKEN) - assert response == False + # If interval between syncs is less than 1 day, source_synced_at should not change + old_source_synced_at = workspace.source_synced_at = datetime.now(timezone.utc) + check_interval_and_sync_dimension(workspace_id=1) + assert old_source_synced_at == workspace.source_synced_at def test_post_request(mocker): diff --git a/tests/test_fyle/test_views.py b/tests/test_fyle/test_views.py index 77da9ec8..b96d1c33 100644 --- a/tests/test_fyle/test_views.py +++ b/tests/test_fyle/test_views.py @@ -230,7 +230,7 @@ def test_fyle_refresh_dimension(api_client, access_token, mocker): response = api_client.post(url) assert response.status_code == 200 - with mock.patch('apps.fyle.views.sync_dimensions') as mock_call: + with mock.patch('apps.fyle.views.Workspace.objects.get') as mock_call: mock_call.side_effect = Exception() response = api_client.post(url) @@ -293,7 +293,7 @@ def test_fyle_sync_dimension(api_client, access_token, mocker): response = api_client.post(url) assert response.status_code == 200 - with mock.patch('apps.fyle.views.check_interval_and_sync_dimension') as mock_call: + with mock.patch('apps.fyle.views.Workspace.objects.get') as mock_call: mock_call.side_effect = Exception() response = api_client.post(url) diff --git a/tests/test_netsuite/test_helpers.py b/tests/test_netsuite/test_helpers.py index b27066e7..c02f834c 100644 --- a/tests/test_netsuite/test_helpers.py +++ b/tests/test_netsuite/test_helpers.py @@ -9,14 +9,16 @@ def test_check_interval_and_sync_dimension(db): - netsuite_credentials = NetSuiteCredentials.objects.get(workspace_id=2) workspace = Workspace.objects.get(id=2) - synced = check_interval_and_sync_dimension(workspace=workspace, netsuite_credentials=netsuite_credentials) - assert synced == True + check_interval_and_sync_dimension(2) + assert workspace.destination_synced_at is not None + old_destination_synced_at = workspace.destination_synced_at + + # If interval between syncs is less than 1 day, destination_synced_at should not change workspace.source_synced_at = datetime.now(timezone.utc) - synced = check_interval_and_sync_dimension(workspace=workspace, netsuite_credentials=netsuite_credentials) - assert synced == False + check_interval_and_sync_dimension(2) + assert old_destination_synced_at == workspace.destination_synced_at def test_sync_dimensions(mocker, db): diff --git a/tests/test_netsuite/test_views.py b/tests/test_netsuite/test_views.py index b29f1f6d..10a749dd 100644 --- a/tests/test_netsuite/test_views.py +++ b/tests/test_netsuite/test_views.py @@ -173,7 +173,7 @@ def test_sync_netsuite_dimensions(api_client, access_token, add_netsuite_credent response = api_client.post(url) assert response.status_code == 200 - with mock.patch('apps.netsuite.views.check_interval_and_sync_dimension') as mock_call: + with mock.patch('apps.netsuite.views.Workspace.objects.get') as mock_call: mock_call.side_effect = Exception() response = api_client.post(url) @@ -227,7 +227,7 @@ def test_refresh_netsuite_dimensions(api_client, access_token, add_netsuite_cred response = api_client.post(url) assert response.status_code == 200 - with mock.patch('apps.netsuite.views.sync_dimensions') as mock_call: + with mock.patch('apps.netsuite.views.Workspace.objects.get') as mock_call: mock_call.side_effect = Exception() response = api_client.post(url)