From 8b404c16c3afdb7175959d88dc6c282c35ea5bd4 Mon Sep 17 00:00:00 2001 From: Hrishabh Tiwari <74908943+Hrishabh17@users.noreply.github.com> Date: Tue, 25 Jun 2024 13:17:16 +0530 Subject: [PATCH] Add support for editing expenses (#633) * Add support for editing expenses * Remove loggers * change action, method name, add more test case * Add check for url workspace_id and payload org_id * Add loggers for payload --- apps/fyle/helpers.py | 13 +- apps/fyle/queue.py | 14 +- apps/fyle/tasks.py | 26 ++ apps/fyle/views.py | 2 +- requirements.txt | 2 +- tests/test_fyle/fixtures.py | 267 +++++++++++++++++++++ tests/test_fyle/test_queue.py | 2 +- tests/test_fyle/test_tasks.py | 58 ++++- tests/test_quickbooks_online/fixtures.py | 2 +- tests/test_quickbooks_online/test_utils.py | 25 +- 10 files changed, 398 insertions(+), 13 deletions(-) diff --git a/apps/fyle/helpers.py b/apps/fyle/helpers.py index ed63f663..7852f78e 100644 --- a/apps/fyle/helpers.py +++ b/apps/fyle/helpers.py @@ -8,10 +8,11 @@ import django_filters from django.conf import settings from django.db.models import Q +from rest_framework.exceptions import ValidationError from apps.fyle.models import ExpenseFilter, ExpenseGroup, ExpenseGroupSettings, Expense from apps.tasks.models import TaskLog -from apps.workspaces.models import WorkspaceGeneralSettings +from apps.workspaces.models import Workspace, WorkspaceGeneralSettings logger = logging.getLogger(__name__) @@ -260,6 +261,16 @@ def get_batched_expenses(batched_payload: List[dict], workspace_id: int) -> List return Expense.objects.filter(expense_id__in=expense_ids, workspace_id=workspace_id) +def assert_valid_request(workspace_id:int, fyle_org_id:str): + """ + Assert if the request is valid by checking + the url_workspace_id and fyle_org_id workspace + """ + workspace = Workspace.objects.get(fyle_org_id=fyle_org_id) + if workspace.id != workspace_id: + raise ValidationError('Workspace mismatch') + + class AdvanceSearchFilter(django_filters.FilterSet): def filter_queryset(self, queryset): or_filtered_queryset = queryset.none() diff --git a/apps/fyle/queue.py b/apps/fyle/queue.py index 68cec648..866fbfae 100644 --- a/apps/fyle/queue.py +++ b/apps/fyle/queue.py @@ -1,4 +1,9 @@ +import logging from django_q.tasks import async_task +from apps.fyle.helpers import assert_valid_request + +logger = logging.getLogger(__name__) +logger.level = logging.INFO def async_post_accounting_export_summary(org_id: str, workspace_id: int) -> None: @@ -12,7 +17,7 @@ def async_post_accounting_export_summary(org_id: str, workspace_id: int) -> None async_task('apps.fyle.tasks.post_accounting_export_summary', org_id, workspace_id) -def async_import_and_export_expenses(body: dict) -> None: +def async_import_and_export_expenses(body: dict, workspace_id: int) -> None: """ Async'ly import and export expenses :param body: body @@ -21,4 +26,11 @@ def async_import_and_export_expenses(body: dict) -> None: if body.get('action') == 'ACCOUNTING_EXPORT_INITIATED' and body.get('data'): report_id = body['data']['id'] org_id = body['data']['org_id'] + assert_valid_request(workspace_id=workspace_id, fyle_org_id=org_id) async_task('apps.fyle.tasks.import_and_export_expenses', report_id, org_id) + + elif body.get('action') == 'UPDATED_AFTER_APPROVAL' and body.get('data') and body.get('resource') == 'EXPENSE': + org_id = body['data']['org_id'] + assert_valid_request(workspace_id=workspace_id, fyle_org_id=org_id) + logger.info("| Updating non-exported expenses through webhook | Content: {{WORKSPACE_ID: {} Payload: {}}}".format(workspace_id, body.get('data'))) + async_task('apps.fyle.tasks.update_non_exported_expenses', body['data']) diff --git a/apps/fyle/tasks.py b/apps/fyle/tasks.py index b0f9f9b3..8d77a81b 100644 --- a/apps/fyle/tasks.py +++ b/apps/fyle/tasks.py @@ -6,6 +6,7 @@ from fyle.platform.exceptions import InternalServerError, InvalidTokenError, RetryException from fyle_accounting_mappings.models import ExpenseAttribute from fyle_integrations_platform_connector import PlatformConnector +from fyle_integrations_platform_connector.apis.expenses import Expenses as FyleExpenses from apps.fyle.actions import create_generator_and_post_in_batches, mark_expenses_as_skipped from apps.fyle.helpers import ( @@ -289,3 +290,28 @@ def import_and_export_expenses(report_id: str, org_id: str) -> None: except Exception: handle_import_exception(task_log) + + +def update_non_exported_expenses(data: Dict) -> None: + """ + To update expenses not in COMPLETE, IN_PROGRESS state + """ + expense_state = None + org_id = data['org_id'] + expense_id = data['id'] + workspace = Workspace.objects.get(fyle_org_id = org_id) + expense = Expense.objects.filter(workspace_id=workspace.id, expense_id=expense_id).first() + + if expense: + if 'state' in expense.accounting_export_summary: + expense_state = expense.accounting_export_summary['state'] + else: + expense_state = 'NOT_EXPORTED' + + if expense_state and expense_state not in ['COMPLETE', 'IN_PROGRESS']: + expense_obj = [] + expense_obj.append(data) + expense_objects = FyleExpenses().construct_expense_object(expense_obj, expense.workspace_id) + Expense.create_expense_objects( + expense_objects, expense.workspace_id + ) diff --git a/apps/fyle/views.py b/apps/fyle/views.py index b3111673..b30e9b9d 100644 --- a/apps/fyle/views.py +++ b/apps/fyle/views.py @@ -105,7 +105,7 @@ class ExportView(generics.CreateAPIView): @handle_view_exceptions() def post(self, request, *args, **kwargs): - async_import_and_export_expenses(request.data) + async_import_and_export_expenses(request.data, int(kwargs['workspace_id'])) return Response(data={}, status=status.HTTP_200_OK) diff --git a/requirements.txt b/requirements.txt index 6d8d347a..3f8898d5 100644 --- a/requirements.txt +++ b/requirements.txt @@ -22,7 +22,7 @@ enum34==1.1.10 future==0.18.2 fyle==0.37.0 fyle-accounting-mappings==1.32.1 -fyle-integrations-platform-connector==1.38.0 +fyle-integrations-platform-connector==1.38.1 fyle-rest-auth==1.7.2 flake8==4.0.1 gevent==23.9.1 diff --git a/tests/test_fyle/fixtures.py b/tests/test_fyle/fixtures.py index e5dbaf14..f775b900 100644 --- a/tests/test_fyle/fixtures.py +++ b/tests/test_fyle/fixtures.py @@ -1,4 +1,271 @@ data = { + "raw_expense": { + 'accounting_export_summary': { + 'error_type': 'ACCOUNTING_INTEGRATION_ERROR', + 'state': 'ERROR', + 'tpa_id': 'tpayfjPPHTDgv', + 'url': 'https://staging1.fyle.tech/app/settings/#/integrations/native_apps?integrationIframeTarget=integrations/intacct/main/dashboard' + }, + 'activity_details': None, + 'added_to_report_at': None, + 'admin_amount': None, + 'advance_wallet_id': None, + 'amount': 12, + 'approvals': [ + { + 'approver_user': { + 'email': 'admin1@fyleforimporrttest.in', + 'full_name': 'Theresa Brown', + 'id': 'usVN2WTtPqE7' + }, + 'approver_user_id': 'usVN2WTtPqE7', + 'state': 'APPROVAL_DONE' + } + ], + 'approver_comments': [], + 'category': { + 'code': '223', + 'display_name': 'ABN Withholding', + 'id': 317995, + 'name': 'ABN Withholding', + 'sub_category': None, + 'system_category': None + }, + 'category_id': 317995, + 'claim_amount': 12, + 'code': None, + 'commute_deduction': None, + 'commute_details': None, + 'commute_details_id': None, + 'cost_center': { + 'code': '96441', + 'id': 23166, + 'name': 'Administration' + }, + 'cost_center_id': 23166, + 'created_at': '2024-05-10T07:52:10.551260+00:00', + 'creator_user_id': 'usVN2WTtPqE7', + 'currency': 'USD', + 'custom_fields': [ + { + 'is_enabled': True, + 'name': 'Custom Expense Field', + 'type': 'TEXT', + 'value': None + }, + { + 'is_enabled': True, + 'name': 'Locationcustom', + 'type': 'SELECT', + 'value': None + }, + { + 'is_enabled': True, + 'name': 'Deptcustom', + 'type': 'SELECT', + 'value': None + } + ], + 'custom_fields_flattened': { + 'custom_expense_field': None, + 'deptcustom': None, + 'locationcustom': None + }, + 'distance': None, + 'distance_unit': None, + 'employee': { + 'business_unit': None, + 'code': None, + 'custom_fields': [], + 'department': None, + 'department_id': None, + 'flattened_custom_field': {}, + 'has_accepted_invite': True, + 'id': 'ouhC0BNdc33I', + 'is_enabled': True, + 'joined_at': None, + 'level': None, + 'location': None, + 'mobile': None, + 'org_id': 'orAW3T2QmroT', + 'org_name': 'Fyle For import_test', + 'title': None, + 'user': { + 'email': 'admin1@fyleforimporrttest.in', + 'full_name': 'Theresa Brown', + 'id': 'usVN2WTtPqE7' + }, + 'user_id': 'usVN2WTtPqE7' + }, + 'employee_id': 'ouhC0BNdc33I', + 'ended_at': None, + 'expense_rule_data': None, + 'expense_rule_id': None, + 'extracted_data': None, + 'file_ids': [], + 'files': [], + 'foreign_amount': None, + 'foreign_currency': None, + 'hotel_is_breakfast_provided': False, + 'id': 'txhJLOSKs1iN', + 'invoice_number': None, + 'is_billable': None, + 'is_corporate_card_transaction_auto_matched': False, + 'is_exported': None, + 'is_manually_flagged': None, + 'is_physical_bill_submitted': None, + 'is_policy_flagged': None, + 'is_receipt_mandatory': None, + 'is_reimbursable': True, + 'is_split': False, + 'is_verified': True, + 'is_weekend_spend': False, + 'last_exported_at': None, + 'last_settled_at': '2024-05-10T07:55:07.373278+00:00', + 'last_verified_at': '2024-05-10T07:55:02.329280+00:00', + 'locations': [], + 'matched_corporate_card_transaction_ids': [], + 'matched_corporate_card_transactions': [], + 'merchant': None, + 'mileage_calculated_amount': None, + 'mileage_calculated_distance': None, + 'mileage_is_round_trip': None, + 'mileage_rate': None, + 'mileage_rate_id': None, + 'missing_mandatory_fields': { + 'amount': False, + 'currency': False, + 'expense_field_ids': [], + 'receipt': False + }, + 'org_id': 'orAW3T2QmroT', + 'per_diem_num_days': None, + 'per_diem_rate': None, + 'per_diem_rate_id': None, + 'physical_bill_submitted_at': None, + 'policy_amount': None, + 'policy_checks': { + 'are_approvers_added': False, + 'is_amount_limit_applied': False, + 'is_flagged_ever': False, + 'violations': None + }, + 'project': { + 'code': 'B3DNLG7TVM', + 'display_name': 'Project 6', + 'id': 330241, + 'name': 'Project 6', + 'sub_project': None + }, + 'project_id': 330241, + 'purpose': None, + 'report': { + 'amount': 12, + 'approvals': [ + { + 'approver_user': { + 'email': 'admin1@fyleforimporrttest.in', + 'full_name': 'Theresa Brown', + 'id': 'usVN2WTtPqE7' + }, + 'approver_user_id': 'usVN2WTtPqE7', + 'state': 'APPROVAL_DONE' + } + ], + 'id': 'rpN41rGGnxNI', + 'last_approved_at': '2024-05-10T07:53:25.774+00:00', + 'last_paid_at': None, + 'last_submitted_at': '2024-05-10T07:53:09.457+00:00', + 'last_verified_at': '2024-05-10T07:55:02.32928+00:00', + 'reimbursement_id': 'reimYNNUkKQiWp', + 'reimbursement_seq_num': 'P/2024/05/T/P/2024/05/R/30', + 'seq_num': 'C/2024/05/R/45', + 'settlement_id': 'setUkp31alIp7', + 'state': 'PAYMENT_PROCESSING', + 'title': '#5: May 2024' + }, + 'report_id': 'rpN41rGGnxNI', + 'report_last_approved_at': '2024-05-10T07:53:25.774000+00:00', + 'report_last_paid_at': None, + 'report_settlement_id': 'setUkp31alIp7', + 'seq_num': 'E/2024/05/T/442', + 'source': 'WEBAPP', + 'source_account': { + 'id': 'accUMhoA4foa5', + 'type': 'PERSONAL_CASH_ACCOUNT' + }, + 'source_account_id': 'accUMhoA4foa5', + 'spent_at': '2024-05-10T00:00:00+00:00', + 'split_group_amount': None, + 'split_group_id': 'txhJLOSKs1iN', + 'started_at': None, + 'state': 'PAYMENT_PROCESSING', + 'state_display_name': 'Processing', + 'tax_amount': None, + 'tax_group': None, + 'tax_group_id': None, + 'travel_classes': [], + 'updated_at': '2024-06-10T11:41:40.779611+00:00', + 'user': { + 'email': 'admin1@fyleforimporrttest.in', + 'full_name': 'Theresa Brown', + 'id': 'usVN2WTtPqE7' + }, + 'user_id': 'usVN2WTtPqE7', + 'verifications': [ + { + 'verifier_user': { + 'email': 'owner@fyleforimporrttest.in', + 'full_name': 'Fyle For import_test', + 'id': 'usbzW0rVpuWC' + }, + 'verifier_user_id': 'usbzW0rVpuWC' + } + ], + 'verifier_comments': [] + }, + "default_raw_expense": { + 'employee_email': 'admin1@fyleforimporrttest.in', + 'employee_name': 'Theresa Brown', + 'category': 'Old Category', + 'sub_category': None, + 'project': 'Project 6', + 'org_id': 'orAW3T2QmroT', + 'expense_number': 'E/2024/05/T/442', + 'claim_number': 'C/2024/05/R/45', + 'amount': 12.0, + 'currency': 'USD', + 'foreign_amount': None, + 'foreign_currency': None, + 'reimbursable': True, + 'state': 'PAYMENT_PROCESSING', + 'vendor': None, + 'cost_center': 'Administration', + 'corporate_card_id': None, + 'purpose': None, + 'report_id': 'rpN41rGGnxNI', + 'billable': False, + 'file_ids': [], + 'spent_at': '2024-05-10 17:00:00', + 'approved_at': '2024-05-10 07:53:25', + 'posted_at': None, + 'is_skipped': False, + 'expense_created_at': '2024-05-10 07:52:10', + 'expense_updated_at': '2024-05-13 05:53:25', + 'fund_source': 'PERSONAL', + 'verified_at': '2024-05-10 07:53:25', + 'custom_properties': { + 'Deptcustom': None, + 'Locationcustom': None, + 'Custom Expense Field': None + }, + 'report_title': '#5: May 2024', + 'payment_number': 'P/2024/05/T/P/2024/05/R/30', + 'tax_amount': None, + 'tax_group_id': None, + 'previous_export_state': None, + 'accounting_export_summary': [] + }, "expenses_spent_at":[ { 'id': '1234', diff --git a/tests/test_fyle/test_queue.py b/tests/test_fyle/test_queue.py index 6ff8a617..35e31126 100644 --- a/tests/test_fyle/test_queue.py +++ b/tests/test_fyle/test_queue.py @@ -38,4 +38,4 @@ def test_async_import_and_export_expenses(db): } } - async_import_and_export_expenses(body) + async_import_and_export_expenses(body, 3) diff --git a/tests/test_fyle/test_tasks.py b/tests/test_fyle/test_tasks.py index ed51effa..1d34d1e0 100644 --- a/tests/test_fyle/test_tasks.py +++ b/tests/test_fyle/test_tasks.py @@ -5,13 +5,16 @@ from django.urls import reverse import pytest +from rest_framework.exceptions import ValidationError +from rest_framework import status from apps.fyle.models import Expense, ExpenseGroup, ExpenseGroupSettings from apps.fyle.tasks import ( create_expense_groups, post_accounting_export_summary, import_and_export_expenses, - sync_dimensions + sync_dimensions, + update_non_exported_expenses ) from apps.fyle.actions import mark_expenses_as_skipped from apps.tasks.models import TaskLog @@ -148,3 +151,56 @@ def test_sync_dimension(db, mocker): mock_platform_instance.import_fyle_dimensions.assert_called_once_with(is_export=True) mock_platform_instance.categories.sync.assert_called_once() mock_platform_instance.projects.sync.assert_called_once() + + +def test_update_non_exported_expenses(db, create_temp_workspace, mocker, api_client, test_connection): + expense = data['raw_expense'] + default_raw_expense = data['default_raw_expense'] + org_id = expense['org_id'] + payload = { + "resource": "EXPENSE", + "action": 'UPDATED_AFTER_APPROVAL', + "data": expense, + "reason": 'expense update testing', + } + + expense_created, _ = Expense.objects.update_or_create( + org_id=org_id, + expense_id='txhJLOSKs1iN', + workspace_id=1, + defaults=default_raw_expense + ) + expense_created.accounting_export_summary = {} + expense_created.save() + + workspace = Workspace.objects.filter(id=1).first() + workspace.fyle_org_id = org_id + workspace.save() + + assert expense_created.category == 'Old Category' + + update_non_exported_expenses(payload['data']) + + expense = Expense.objects.get(expense_id='txhJLOSKs1iN', org_id=org_id) + assert expense.category == 'ABN Withholding' + + expense.accounting_export_summary = {"synced": True, "state": "COMPLETE"} + expense.category = 'Old Category' + expense.save() + + update_non_exported_expenses(payload['data']) + expense = Expense.objects.get(expense_id='txhJLOSKs1iN', org_id=org_id) + assert expense.category == 'Old Category' + + try: + update_non_exported_expenses(payload['data']) + except ValidationError as e: + assert e.detail[0] == 'Workspace mismatch' + + url = reverse('exports', kwargs={'workspace_id': 1}) + response = api_client.post(url, data=payload, format='json') + assert response.status_code == status.HTTP_200_OK + + url = reverse('exports', kwargs={'workspace_id': 2}) + response = api_client.post(url, data=payload, format='json') + assert response.status_code == status.HTTP_400_BAD_REQUEST diff --git a/tests/test_quickbooks_online/fixtures.py b/tests/test_quickbooks_online/fixtures.py index 37b67391..8313f533 100644 --- a/tests/test_quickbooks_online/fixtures.py +++ b/tests/test_quickbooks_online/fixtures.py @@ -157,7 +157,7 @@ 'DetailType': 'ItemBasedExpenseLineDetail', 'Amount': 1.0, 'ItemBasedExpenseLineDetail': {'CustomerRef': {'value': None}, 'ClassRef': {'value': None}, 'TaxCodeRef': {'value': None}, 'BillableStatus': 'NotBillable', 'ItemRef': {'value': '3'}, 'Qty': 1}, - }, + } ], }, "qbo_expense_payload": { diff --git a/tests/test_quickbooks_online/test_utils.py b/tests/test_quickbooks_online/test_utils.py index 9845fd57..467d34aa 100644 --- a/tests/test_quickbooks_online/test_utils.py +++ b/tests/test_quickbooks_online/test_utils.py @@ -16,6 +16,13 @@ logger = logging.getLogger(__name__) +def sort_lines(expense): + """Helper function to sort the Line items by DetailType.""" + if 'Line' in expense: + expense['Line'] = sorted(expense['Line'], key=lambda x: x['DetailType']) + return expense + + @pytest.mark.django_db def test_sync_employees(mocker, db): mocker.patch('qbosdk.apis.Employees.get_all_generator', return_value=[data['employee_response']]) @@ -152,10 +159,13 @@ def test_construct_bill_item_and_account_based(create_bill_item_and_account_base # for item-based and account-based line-items bill, bill_lineitems = create_bill_item_and_account_based bill_object = qbo_connection._QBOConnector__construct_bill(bill=bill, bill_lineitems=bill_lineitems) - bill_object['Line'][0]['DetailType'] == 'ItemBasedExpenseLineDetail' - bill_object['Line'][1]['DetailType'] == 'AccountBasedExpenseLineDetail' + assert bill_object['Line'][0]['DetailType'] == 'ItemBasedExpenseLineDetail' + assert bill_object['Line'][1]['DetailType'] == 'AccountBasedExpenseLineDetail' - assert dict_compare_keys(bill_object, data['bill_payload_item_and_account_based_payload']) == [], 'construct bill_payload entry api return diffs in keys' + sorted_bill_object = sort_lines(bill_object) + sorted_expected_payload = sort_lines(data['bill_payload_item_and_account_based_payload']) + + assert dict_compare_keys(sorted_bill_object, sorted_expected_payload) == [], 'construct bill_payload entry api return diffs in keys' def test_construct_credit_card_purchase(create_credit_card_purchase, db): @@ -244,10 +254,13 @@ def test_construct_qbo_expense_item_and_account_based(create_qbo_expense_item_an qbo_expense, qbo_expense_lineitems = create_qbo_expense_item_and_account_based qbo_expense_object = qbo_connection._QBOConnector__construct_qbo_expense(qbo_expense=qbo_expense, qbo_expense_lineitems=qbo_expense_lineitems) - qbo_expense_object['Line'][0]['DetailType'] == 'ItemBasedExpenseLineDetail' - qbo_expense_object['Line'][1]['DetailType'] == 'AccountBasedExpenseLineDetail' + assert qbo_expense_object['Line'][0]['DetailType'] == 'ItemBasedExpenseLineDetail' + assert qbo_expense_object['Line'][1]['DetailType'] == 'AccountBasedExpenseLineDetail' + + qbo_expense_object_sorted = sort_lines(qbo_expense_object) + expected_payload_sorted = sort_lines(data['qbo_expense_item_and_account_based_payload']) - assert dict_compare_keys(qbo_expense_object, data['qbo_expense_item_and_account_based_payload']) == [], 'construct expense api return diffs in keys' + assert dict_compare_keys(qbo_expense_object_sorted, expected_payload_sorted) == [], 'construct expense api return diffs in keys' def test_construct_cheque(create_cheque, db):