From 4b7f5d5e80082b1efd45782279ac907b1d6f9545 Mon Sep 17 00:00:00 2001 From: ruuushhh Date: Wed, 23 Apr 2025 12:04:12 +0530 Subject: [PATCH 1/3] fix: Export Settings validation --- .../apis/export_settings/serializers.py | 52 ++++++++++++++++++- 1 file changed, 51 insertions(+), 1 deletion(-) diff --git a/apps/workspaces/apis/export_settings/serializers.py b/apps/workspaces/apis/export_settings/serializers.py index a6f7477dc..bc537eacb 100644 --- a/apps/workspaces/apis/export_settings/serializers.py +++ b/apps/workspaces/apis/export_settings/serializers.py @@ -179,7 +179,7 @@ def update(self, instance, validated): return instance - def validate(self, data): + def validate(self, data): # noqa: C901 if not data.get('workspace_general_settings'): raise serializers.ValidationError('Workspace general settings are required') @@ -188,4 +188,54 @@ def validate(self, data): if not data.get('general_mappings'): raise serializers.ValidationError('General mappings are required') + + general_settings = data.get('workspace_general_settings') + general_mappings = data.get('general_mappings') + + # Validate based on reimbursable expenses object + if general_settings.get('reimbursable_expenses_object') == 'BILL': + if not (general_mappings.get('accounts_payable_id') or general_mappings.get('accounts_payable_name')): + raise serializers.ValidationError('Accounts Payable is required for BILL type reimbursable expenses') + + elif general_settings.get('reimbursable_expenses_object') == 'CHECK': + if not (general_mappings.get('bank_account_id') or general_mappings.get('bank_account_name')): + raise serializers.ValidationError('Bank Account is required for CHECK type reimbursable expenses') + + elif general_settings.get('reimbursable_expenses_object') == 'EXPENSE': + if not (general_mappings.get('qbo_expense_account_id') or general_mappings.get('qbo_expense_account_name')): + raise serializers.ValidationError('Expense Payment Account is required for EXPENSE type reimbursable expenses') + + elif general_settings.get('reimbursable_expenses_object') == 'JOURNAL ENTRY': + if general_settings.get('employee_field_mapping') == 'VENDOR': + if not (general_mappings.get('accounts_payable_id') or general_mappings.get('accounts_payable_name')): + raise serializers.ValidationError('Accounts Payable is required for JOURNAL ENTRY with VENDOR mapping') + elif general_settings.get('employee_field_mapping') == 'EMPLOYEE': + if not (general_mappings.get('bank_account_id') or general_mappings.get('bank_account_name')): + raise serializers.ValidationError('Bank Account is required for JOURNAL ENTRY with EMPLOYEE mapping') + + # Validate based on corporate credit card expenses object + if general_settings.get('corporate_credit_card_expenses_object') == 'BILL': + if not (general_mappings.get('accounts_payable_id') or general_mappings.get('accounts_payable_name')): + raise serializers.ValidationError('Accounts Payable is required for BILL type corporate credit card expenses') + if not (general_mappings.get('default_ccc_vendor_id') or general_mappings.get('default_ccc_vendor_name')): + raise serializers.ValidationError('Default Credit Card Vendor is required for BILL type corporate credit card expenses') + + elif general_settings.get('corporate_credit_card_expenses_object') == 'CREDIT CARD PURCHASE': + if not (general_mappings.get('default_ccc_account_id') or general_mappings.get('default_ccc_account_name')): + raise serializers.ValidationError('Default Credit Card Account is required for CREDIT CARD PURCHASE type expenses') + + elif general_settings.get('corporate_credit_card_expenses_object') == 'DEBIT CARD EXPENSE': + if not (general_mappings.get('default_debit_card_account_id') or general_mappings.get('default_debit_card_account_name')): + raise serializers.ValidationError('Default Debit Card Account is required for DEBIT CARD EXPENSE type expenses') + + elif general_settings.get('corporate_credit_card_expenses_object') == 'JOURNAL ENTRY': + if general_settings.get('name_in_journal_entry') == 'MERCHANT': + if not (general_mappings.get('default_ccc_account_id') or general_mappings.get('default_ccc_account_name')): + raise serializers.ValidationError('Default Credit Card Account is required for JOURNAL ENTRY with MERCHANT name') + + # Validate tax code settings + if general_settings.get('import_tax_codes'): + if not (general_mappings.get('default_tax_code_id') or general_mappings.get('default_tax_code_name')): + raise serializers.ValidationError('Default Tax Code is required when tax codes are imported') + return data From 79edab4fcd9c4147b6243a2fc215e8ac710cc72d Mon Sep 17 00:00:00 2001 From: ruuushhh Date: Wed, 23 Apr 2025 12:53:49 +0530 Subject: [PATCH 2/3] fix tests --- .../apis/export_settings/serializers.py | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/apps/workspaces/apis/export_settings/serializers.py b/apps/workspaces/apis/export_settings/serializers.py index bc537eacb..b83acb6e0 100644 --- a/apps/workspaces/apis/export_settings/serializers.py +++ b/apps/workspaces/apis/export_settings/serializers.py @@ -194,48 +194,48 @@ def validate(self, data): # noqa: C901 # Validate based on reimbursable expenses object if general_settings.get('reimbursable_expenses_object') == 'BILL': - if not (general_mappings.get('accounts_payable_id') or general_mappings.get('accounts_payable_name')): + if not (general_mappings.get('accounts_payable', {}).get('id') or general_mappings.get('accounts_payable', {}).get('name')): raise serializers.ValidationError('Accounts Payable is required for BILL type reimbursable expenses') elif general_settings.get('reimbursable_expenses_object') == 'CHECK': - if not (general_mappings.get('bank_account_id') or general_mappings.get('bank_account_name')): + if not (general_mappings.get('bank_account', {}).get('id') or general_mappings.get('bank_account', {}).get('name')): raise serializers.ValidationError('Bank Account is required for CHECK type reimbursable expenses') elif general_settings.get('reimbursable_expenses_object') == 'EXPENSE': - if not (general_mappings.get('qbo_expense_account_id') or general_mappings.get('qbo_expense_account_name')): + if not (general_mappings.get('qbo_expense_account', {}).get('id') or general_mappings.get('qbo_expense_account', {}).get('name')): raise serializers.ValidationError('Expense Payment Account is required for EXPENSE type reimbursable expenses') elif general_settings.get('reimbursable_expenses_object') == 'JOURNAL ENTRY': if general_settings.get('employee_field_mapping') == 'VENDOR': - if not (general_mappings.get('accounts_payable_id') or general_mappings.get('accounts_payable_name')): + if not (general_mappings.get('accounts_payable', {}).get('id') or general_mappings.get('accounts_payable', {}).get('name')): raise serializers.ValidationError('Accounts Payable is required for JOURNAL ENTRY with VENDOR mapping') elif general_settings.get('employee_field_mapping') == 'EMPLOYEE': - if not (general_mappings.get('bank_account_id') or general_mappings.get('bank_account_name')): + if not (general_mappings.get('bank_account', {}).get('id') or general_mappings.get('bank_account', {}).get('name')): raise serializers.ValidationError('Bank Account is required for JOURNAL ENTRY with EMPLOYEE mapping') # Validate based on corporate credit card expenses object if general_settings.get('corporate_credit_card_expenses_object') == 'BILL': - if not (general_mappings.get('accounts_payable_id') or general_mappings.get('accounts_payable_name')): + if not (general_mappings.get('accounts_payable', {}).get('id') or general_mappings.get('accounts_payable', {}).get('name')): raise serializers.ValidationError('Accounts Payable is required for BILL type corporate credit card expenses') - if not (general_mappings.get('default_ccc_vendor_id') or general_mappings.get('default_ccc_vendor_name')): + if not (general_mappings.get('default_ccc_vendor', {}).get('id') or general_mappings.get('default_ccc_vendor', {}).get('name')): raise serializers.ValidationError('Default Credit Card Vendor is required for BILL type corporate credit card expenses') elif general_settings.get('corporate_credit_card_expenses_object') == 'CREDIT CARD PURCHASE': - if not (general_mappings.get('default_ccc_account_id') or general_mappings.get('default_ccc_account_name')): + if not (general_mappings.get('default_ccc_account', {}).get('id') or general_mappings.get('default_ccc_account', {}).get('name')): raise serializers.ValidationError('Default Credit Card Account is required for CREDIT CARD PURCHASE type expenses') elif general_settings.get('corporate_credit_card_expenses_object') == 'DEBIT CARD EXPENSE': - if not (general_mappings.get('default_debit_card_account_id') or general_mappings.get('default_debit_card_account_name')): + if not (general_mappings.get('default_debit_card_account', {}).get('id') or general_mappings.get('default_debit_card_account', {}).get('name')): raise serializers.ValidationError('Default Debit Card Account is required for DEBIT CARD EXPENSE type expenses') elif general_settings.get('corporate_credit_card_expenses_object') == 'JOURNAL ENTRY': if general_settings.get('name_in_journal_entry') == 'MERCHANT': - if not (general_mappings.get('default_ccc_account_id') or general_mappings.get('default_ccc_account_name')): + if not (general_mappings.get('default_ccc_account', {}).get('id') or general_mappings.get('default_ccc_account', {}).get('name')): raise serializers.ValidationError('Default Credit Card Account is required for JOURNAL ENTRY with MERCHANT name') # Validate tax code settings if general_settings.get('import_tax_codes'): - if not (general_mappings.get('default_tax_code_id') or general_mappings.get('default_tax_code_name')): + if not (general_mappings.get('default_tax_code', {}).get('id') or general_mappings.get('default_tax_code', {}).get('name')): raise serializers.ValidationError('Default Tax Code is required when tax codes are imported') return data From e93be31bc489b10318381972e10e51b600b5f665 Mon Sep 17 00:00:00 2001 From: ruuushhh Date: Wed, 23 Apr 2025 14:47:53 +0530 Subject: [PATCH 3/3] add tests --- .../apis/export_settings/serializers.py | 7 +- .../test_apis/test_clone_settings/fixtures.py | 6 +- .../test_export_settings/fixtures.py | 4 +- .../test_export_settings/test_views.py | 96 ++++++++++++++++++- 4 files changed, 100 insertions(+), 13 deletions(-) diff --git a/apps/workspaces/apis/export_settings/serializers.py b/apps/workspaces/apis/export_settings/serializers.py index b83acb6e0..ebcbca31e 100644 --- a/apps/workspaces/apis/export_settings/serializers.py +++ b/apps/workspaces/apis/export_settings/serializers.py @@ -25,7 +25,7 @@ def to_internal_value(self, data): class WorkspaceGeneralSettingsSerializer(serializers.ModelSerializer): class Meta: model = WorkspaceGeneralSettings - fields = ['reimbursable_expenses_object', 'corporate_credit_card_expenses_object', 'name_in_journal_entry'] + fields = ['reimbursable_expenses_object', 'corporate_credit_card_expenses_object', 'name_in_journal_entry', 'employee_field_mapping'] class ExpenseGroupSettingsSerializer(serializers.ModelSerializer): @@ -233,9 +233,4 @@ def validate(self, data): # noqa: C901 if not (general_mappings.get('default_ccc_account', {}).get('id') or general_mappings.get('default_ccc_account', {}).get('name')): raise serializers.ValidationError('Default Credit Card Account is required for JOURNAL ENTRY with MERCHANT name') - # Validate tax code settings - if general_settings.get('import_tax_codes'): - if not (general_mappings.get('default_tax_code', {}).get('id') or general_mappings.get('default_tax_code', {}).get('name')): - raise serializers.ValidationError('Default Tax Code is required when tax codes are imported') - return data diff --git a/tests/test_workspaces/test_apis/test_clone_settings/fixtures.py b/tests/test_workspaces/test_apis/test_clone_settings/fixtures.py index 57e541d27..a459f2a8b 100644 --- a/tests/test_workspaces/test_apis/test_clone_settings/fixtures.py +++ b/tests/test_workspaces/test_apis/test_clone_settings/fixtures.py @@ -5,7 +5,8 @@ "workspace_general_settings": { "reimbursable_expenses_object": "EXPENSE", "corporate_credit_card_expenses_object": None, - "name_in_journal_entry": "MERCHANT" + "name_in_journal_entry": "MERCHANT", + "employee_field_mapping": "EMPLOYEE" }, "expense_group_settings": { "reimbursable_expense_group_fields": [ @@ -142,7 +143,8 @@ "workspace_general_settings": { "reimbursable_expenses_object": "EXPENSE", "corporate_credit_card_expenses_object": None, - "name_in_journal_entry": "MERCHANT" + "name_in_journal_entry": "MERCHANT", + "employee_field_mapping": "EMPLOYEE" }, "expense_group_settings": { "reimbursable_expense_group_fields": [ diff --git a/tests/test_workspaces/test_apis/test_export_settings/fixtures.py b/tests/test_workspaces/test_apis/test_export_settings/fixtures.py index 49351cb02..17aebbc13 100644 --- a/tests/test_workspaces/test_apis/test_export_settings/fixtures.py +++ b/tests/test_workspaces/test_apis/test_export_settings/fixtures.py @@ -9,7 +9,7 @@ 'ccc_export_date_type': '', 'split_expense_grouping': 'SINGLE_LINE_ITEM' }, - 'workspace_general_settings': {'reimbursable_expenses_object': 'EXPENSE', 'corporate_credit_card_expenses_object': 'BILL','name_in_journal_entry': 'MERCHANT'}, + 'workspace_general_settings': {'reimbursable_expenses_object': 'EXPENSE', 'corporate_credit_card_expenses_object': 'BILL','name_in_journal_entry': 'MERCHANT', 'employee_field_mapping': 'VENDOR'}, 'general_mappings': { 'bank_account': {'id': '', 'name': ''}, 'default_ccc_account': {'id': '', 'name': ''}, @@ -40,7 +40,7 @@ }, }, 'response': { - 'workspace_general_settings': {'reimbursable_expenses_object': 'EXPENSE', 'corporate_credit_card_expenses_object': 'BILL', 'name_in_journal_entry': 'MERCHANT'}, + 'workspace_general_settings': {'reimbursable_expenses_object': 'EXPENSE', 'corporate_credit_card_expenses_object': 'BILL', 'name_in_journal_entry': 'MERCHANT', 'employee_field_mapping': 'VENDOR'}, 'expense_group_settings': { 'reimbursable_expense_group_fields': ['fund_source', 'claim_number', 'employee_email', 'report_id'], 'corporate_credit_card_expense_group_fields': ['fund_source', 'claim_number', 'employee_email', 'report_id'], diff --git a/tests/test_workspaces/test_apis/test_export_settings/test_views.py b/tests/test_workspaces/test_apis/test_export_settings/test_views.py index 0d63de6d1..cbe3bbc62 100644 --- a/tests/test_workspaces/test_apis/test_export_settings/test_views.py +++ b/tests/test_workspaces/test_apis/test_export_settings/test_views.py @@ -1,3 +1,4 @@ +import copy import json from apps.workspaces.models import Workspace, WorkspaceGeneralSettings @@ -6,7 +7,6 @@ def test_export_settings(api_client, test_connection): - workspace = Workspace.objects.get(id=3) workspace.onboarding_state = 'EXPORT_SETTINGS' workspace.save() @@ -25,16 +25,106 @@ def test_export_settings(api_client, test_connection): response = json.loads(response.content) assert dict_compare_keys(response, data['response']) == [], 'workspaces api returns a diff in the keys' - invalid_workspace_general_settings = data['export_settings'] + invalid_workspace_general_settings = copy.deepcopy(data['export_settings']) invalid_workspace_general_settings['workspace_general_settings'] = {} response = api_client.put(url, data=invalid_workspace_general_settings, format='json') assert response.status_code == 400 - invalid_expense_group_settings = data['export_settings'] + invalid_expense_group_settings = copy.deepcopy(data['export_settings']) invalid_expense_group_settings['expense_group_settings'] = {} invalid_expense_group_settings['workspace_general_settings'] = {'reimbursable_expenses_object': 'EXPENSE', 'corporate_credit_card_expenses_object': 'BILL'} response = api_client.put(url, data=invalid_expense_group_settings, format='json') assert response.status_code == 400 + + +def test_export_settings_validation(api_client, test_connection): + url = '/api/v2/workspaces/3/export_settings/' + api_client.credentials(HTTP_AUTHORIZATION='Bearer {}'.format(test_connection.access_token)) + + # Test BILL type reimbursable expenses validation + bill_settings = copy.deepcopy(data['export_settings']) + bill_settings['workspace_general_settings']['reimbursable_expenses_object'] = 'BILL' + bill_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = 'EXPENSE' # Set to different value to avoid conflict + bill_settings['general_mappings']['accounts_payable'] = {'id': '', 'name': ''} + response = api_client.put(url, data=bill_settings, format='json') + assert response.status_code == 400 + assert 'Accounts Payable is required for BILL type reimbursable expenses' in str(response.content) + + # Test CHECK type reimbursable expenses validation + check_settings = copy.deepcopy(data['export_settings']) + check_settings['workspace_general_settings']['reimbursable_expenses_object'] = 'CHECK' + check_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = 'EXPENSE' # Set to different value to avoid conflict + check_settings['general_mappings']['bank_account'] = {'id': '', 'name': ''} + response = api_client.put(url, data=check_settings, format='json') + assert response.status_code == 400 + assert 'Bank Account is required for CHECK type reimbursable expenses' in str(response.content) + + # Test EXPENSE type reimbursable expenses validation + expense_settings = copy.deepcopy(data['export_settings']) + expense_settings['workspace_general_settings']['reimbursable_expenses_object'] = 'EXPENSE' + expense_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = 'BILL' # Set to different value to avoid conflict + expense_settings['general_mappings']['qbo_expense_account'] = {'id': '', 'name': ''} + response = api_client.put(url, data=expense_settings, format='json') + assert response.status_code == 400 + assert 'Expense Payment Account is required for EXPENSE type reimbursable expenses' in str(response.content) + + # Test JOURNAL ENTRY type with VENDOR mapping validation + je_vendor_settings = copy.deepcopy(data['export_settings']) + je_vendor_settings['workspace_general_settings']['reimbursable_expenses_object'] = 'JOURNAL ENTRY' + je_vendor_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = None + je_vendor_settings['workspace_general_settings']['employee_field_mapping'] = 'VENDOR' + je_vendor_settings['general_mappings']['accounts_payable'] = {'id': '', 'name': ''} + response = api_client.put(url, data=je_vendor_settings, format='json') + assert response.status_code == 400 + assert 'Accounts Payable is required for JOURNAL ENTRY with VENDOR mapping' in str(response.content) + + # Test JOURNAL ENTRY type with EMPLOYEE mapping validation + je_employee_settings = copy.deepcopy(data['export_settings']) + je_employee_settings['workspace_general_settings']['employee_field_mapping'] = 'EMPLOYEE' + je_employee_settings['workspace_general_settings']['reimbursable_expenses_object'] = 'JOURNAL ENTRY' + je_employee_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = None + je_employee_settings['general_mappings']['bank_account'] = {'id': '', 'name': ''} + response = api_client.put(url, data=je_employee_settings, format='json') + assert response.status_code == 400 + assert 'Bank Account is required for JOURNAL ENTRY with EMPLOYEE mapping' in str(response.content) + + # Test BILL type corporate credit card expenses validation + ccc_bill_settings = copy.deepcopy(data['export_settings']) + ccc_bill_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = 'BILL' + ccc_bill_settings['workspace_general_settings']['reimbursable_expenses_object'] = None # Set to different value to avoid conflict + ccc_bill_settings['general_mappings']['accounts_payable'] = {'id': '', 'name': ''} + ccc_bill_settings['general_mappings']['default_ccc_vendor'] = {'id': '', 'name': ''} + response = api_client.put(url, data=ccc_bill_settings, format='json') + assert response.status_code == 400 + assert 'Accounts Payable is required for BILL type corporate credit card expenses' in str(response.content) + + # Test CREDIT CARD PURCHASE type validation + ccp_settings = copy.deepcopy(data['export_settings']) + ccp_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = 'CREDIT CARD PURCHASE' + ccp_settings['workspace_general_settings']['reimbursable_expenses_object'] = None + ccp_settings['general_mappings']['default_ccc_account'] = {'id': '', 'name': ''} + response = api_client.put(url, data=ccp_settings, format='json') + assert response.status_code == 400 + assert 'Default Credit Card Account is required for CREDIT CARD PURCHASE type expenses' in str(response.content) + + # Test DEBIT CARD EXPENSE type validation + debit_settings = copy.deepcopy(data['export_settings']) + debit_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = 'DEBIT CARD EXPENSE' + debit_settings['workspace_general_settings']['reimbursable_expenses_object'] = None + debit_settings['general_mappings']['default_debit_card_account'] = {'id': '', 'name': ''} + response = api_client.put(url, data=debit_settings, format='json') + assert response.status_code == 400 + assert 'Default Debit Card Account is required for DEBIT CARD EXPENSE type expenses' in str(response.content) + + # Test JOURNAL ENTRY with MERCHANT name validation + je_merchant_settings = copy.deepcopy(data['export_settings']) + je_merchant_settings['workspace_general_settings']['corporate_credit_card_expenses_object'] = 'JOURNAL ENTRY' + je_merchant_settings['workspace_general_settings']['name_in_journal_entry'] = 'MERCHANT' + je_merchant_settings['workspace_general_settings']['reimbursable_expenses_object'] = None + je_merchant_settings['general_mappings']['default_ccc_account'] = {'id': '', 'name': ''} + response = api_client.put(url, data=je_merchant_settings, format='json') + assert response.status_code == 400 + assert 'Default Credit Card Account is required for JOURNAL ENTRY with MERCHANT name' in str(response.content)