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

fix disable attribute #222

Merged
merged 12 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 10 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
12 changes: 6 additions & 6 deletions apps/fyle/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,19 +128,19 @@ 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(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'
accounting_export = AccountingExport.objects.filter(
workspace_id=workspace.id,
expenses=expense,
status__in=['EXPORT_READY', 'FAILED', 'FATAL']
).first()

if expense_state and expense_state not in ['COMPLETE', 'IN_PROGRESS']:
if accounting_export:
expense_obj = []
expense_obj.append(data)
expense_objects = FyleExpenses().construct_expense_object(expense_obj, expense.workspace_id)
Expand Down
17 changes: 13 additions & 4 deletions apps/mappings/imports/modules/categories.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,25 +90,34 @@ def create_mappings(self):
)


def disable_categories(workspace_id: int, categories_to_disable: Dict, *args, **kwargs):
def disable_categories(workspace_id: int, categories_to_disable: Dict, is_import_to_fyle_enabled: bool = False, *args, **kwargs):
"""
categories_to_disable object format:
{
'destination_id': {
'value': 'old_category_name',
'updated_value': 'new_category_name',
'code': 'old_code',
'update_code': 'new_code' ---- if the code is updated else same as code
'updated_code': 'new_code' ---- if the code is updated else same as code
}
}
"""
if not is_import_to_fyle_enabled or len(categories_to_disable) == 0:
logger.info("Skipping disabling categories in Fyle | WORKSPACE_ID: %s", workspace_id)
return

fyle_credentials = FyleCredential.objects.get(workspace_id=workspace_id)
platform = PlatformConnector(fyle_credentials=fyle_credentials)

use_code_in_naming = ImportSetting.objects.filter(workspace_id=workspace_id, import_code_fields__contains=['ACCOUNT']).first()

category_values = []
for category_map in categories_to_disable.values():
if not use_code_in_naming and category_map['value'] == category_map['updated_value']:
continue
elif use_code_in_naming and (category_map['value'] == category_map['updated_value'] and category_map['code'] == category_map['updated_code']):
continue

category_name = prepend_code_to_name(prepend_code_in_name=use_code_in_naming, value=category_map['value'], code=category_map['code'])
category_values.append(category_name)

Expand All @@ -121,9 +130,9 @@ def disable_categories(workspace_id: int, categories_to_disable: Dict, *args, **

# Expense attribute value map is as follows: {old_category_name: destination_id}
expense_attribute_value_map = {}
for k, v in categories_to_disable.items():
for destination_id, v in categories_to_disable.items():
category_name = prepend_code_to_name(prepend_code_in_name=use_code_in_naming, value=v['value'], code=v['code'])
expense_attribute_value_map[category_name] = k
expense_attribute_value_map[category_name] = destination_id

expense_attributes = ExpenseAttribute.objects.filter(**filters)

Expand Down
17 changes: 13 additions & 4 deletions apps/mappings/imports/modules/cost_centers.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,22 @@ def construct_fyle_payload(
return payload


def disable_cost_centers(workspace_id: int, cost_centers_to_disable: Dict, *args, **kwargs):
def disable_cost_centers(workspace_id: int, cost_centers_to_disable: Dict, is_import_to_fyle_enabled: bool = False, *args, **kwargs):
"""
cost_centers_to_disable object format:
{
'destination_id': {
'value': 'old_cost_center_name',
'updated_value': 'new_cost_center_name',
'code': 'old_code',
'update_code': 'new_code' ---- if the code is updated else same as code
'updated_code': 'new_code' ---- if the code is updated else same as code
}
}
"""
if not is_import_to_fyle_enabled or len(cost_centers_to_disable) == 0:
logger.info("Skipping disabling cost centers in Fyle | WORKSPACE_ID: %s", workspace_id)
return

destination_type = MappingSetting.objects.get(workspace_id=workspace_id, source_field='COST_CENTER').destination_field
use_code_in_naming = ImportSetting.objects.filter(workspace_id=workspace_id, import_code_fields__contains=[destination_type]).first()

Expand All @@ -85,6 +89,11 @@ def disable_cost_centers(workspace_id: int, cost_centers_to_disable: Dict, *args

cost_center_values = []
for cost_center_map in cost_centers_to_disable.values():
if not use_code_in_naming and cost_center_map['value'] == cost_center_map['updated_value']:
continue
elif use_code_in_naming and (cost_center_map['value'] == cost_center_map['updated_value'] and cost_center_map['code'] == cost_center_map['updated_code']):
continue

cost_center_name = prepend_code_to_name(prepend_code_in_name=use_code_in_naming, value=cost_center_map['value'], code=cost_center_map['code'])
cost_center_values.append(cost_center_name)

Expand All @@ -96,9 +105,9 @@ def disable_cost_centers(workspace_id: int, cost_centers_to_disable: Dict, *args
}

expense_attribute_value_map = {}
for k, v in cost_centers_to_disable.items():
for destination_id, v in cost_centers_to_disable.items():
cost_center_name = prepend_code_to_name(prepend_code_in_name=use_code_in_naming, value=v['value'], code=v['code'])
expense_attribute_value_map[cost_center_name] = k
expense_attribute_value_map[cost_center_name] = destination_id

expense_attributes = ExpenseAttribute.objects.filter(**filters)

Expand Down
20 changes: 16 additions & 4 deletions apps/mappings/imports/modules/merchants.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,24 +75,35 @@ def import_destination_attribute_to_fyle(self, import_log: ImportLog):
self.sync_expense_attributes(platform)


def disable_merchants(workspace_id: int, merchants_to_disable: Dict, *args, **kwargs):
def disable_merchants(workspace_id: int, merchants_to_disable: Dict, is_import_to_fyle_enabled: bool = False, *args, **kwargs):
"""
merchants_to_disable object format:
{
'destination_id': {
'value': 'old_merchant_name',
'updated_value': 'new_merchant_name',
'code': 'old_code',
'update_code': 'new_code' ---- if the code is updated else same as code
'updated_code': 'new_code' ---- if the code is updated else same as code
}
}
"""
if not is_import_to_fyle_enabled or len(merchants_to_disable) == 0:
logger.info("Skipping disabling merchants in Fyle | WORKSPACE_ID: %s", workspace_id)
return

fyle_credentials = FyleCredential.objects.get(workspace_id=workspace_id)
platform = PlatformConnector(fyle_credentials=fyle_credentials)
use_code_in_naming = ImportSetting.objects.filter(workspace_id = workspace_id, import_code_fields__contains=['VENDOR']).first()

logger.info(f"Inside disable_merchants | WORKSPACE_ID: {workspace_id} | USE_CODE_IN_NAMING: {use_code_in_naming}")
merchant_values = []
for merchant_map in merchants_to_disable.values():
if not use_code_in_naming and merchant_map['value'] == merchant_map['updated_value']:
logger.info(f"Skipping2 disabling merchant {merchant_map['value']} in Fyle | WORKSPACE_ID: {workspace_id}")
continue
elif use_code_in_naming and (merchant_map['value'] == merchant_map['updated_value'] and merchant_map['code'] == merchant_map['updated_code']):
logger.info(f"Skipping3 disabling merchant {merchant_map['value']} in Fyle | WORKSPACE_ID: {workspace_id}")
continue

merchant_name = prepend_code_to_name(prepend_code_in_name=use_code_in_naming, value=merchant_map['value'], code=merchant_map['code'])
merchant_values.append(merchant_name)

Expand All @@ -102,9 +113,10 @@ def disable_merchants(workspace_id: int, merchants_to_disable: Dict, *args, **kw
'value__in': merchant_values,
'active': True
}
logger.info("Filters: %s", filters)

bulk_payload = ExpenseAttribute.objects.filter(**filters).values_list('value', flat=True)

logger.info("Bulk Payload: %s", bulk_payload)
if bulk_payload:
logger.info(f"Disabling Merchants in Fyle | WORKSPACE_ID: {workspace_id} | COUNT: {len(bulk_payload)}")
platform.merchants.post(bulk_payload, delete_merchants=True)
Expand Down
20 changes: 15 additions & 5 deletions apps/mappings/imports/modules/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def construct_fyle_payload(
return payload


def disable_projects(workspace_id: int, projects_to_disable: Dict, *args, **kwargs):
def disable_projects(workspace_id: int, projects_to_disable: Dict, is_import_to_fyle_enabled: bool = False, *args, **kwargs):
"""
Disable projects in Fyle when the projects are updated in Sage 300.
This is a callback function that is triggered from accounting_mappings.
Expand All @@ -92,6 +92,10 @@ def disable_projects(workspace_id: int, projects_to_disable: Dict, *args, **kwar
}

"""
if not is_import_to_fyle_enabled or len(projects_to_disable) == 0:
logger.info("Skipping disabling projects in Fyle | WORKSPACE_ID: %s", workspace_id)
return

fyle_credentials = FyleCredential.objects.get(workspace_id=workspace_id)
platform = PlatformConnector(fyle_credentials=fyle_credentials)
platform.projects.sync()
Expand All @@ -103,6 +107,11 @@ def disable_projects(workspace_id: int, projects_to_disable: Dict, *args, **kwar

project_values = []
for projects_map in projects_to_disable.values():
if not use_code_in_naming and projects_map['value'] == projects_map['updated_value']:
continue
elif use_code_in_naming and (projects_map['value'] == projects_map['updated_value'] and projects_map['code'] == projects_map['updated_code']):
continue

project_name = prepend_code_to_name(prepend_code_in_name=use_code_in_naming, value=projects_map['value'], code=projects_map['code'])
project_values.append(project_name)

Expand All @@ -115,9 +124,9 @@ def disable_projects(workspace_id: int, projects_to_disable: Dict, *args, **kwar

# Expense attribute value map is as follows: {old_project_name: destination_id}
expense_attribute_value_map = {}
for k, v in projects_to_disable.items():
for destination_id, v in projects_to_disable.items():
project_name = prepend_code_to_name(prepend_code_in_name=use_code_in_naming, value=v['value'], code=v['code'])
expense_attribute_value_map[project_name] = k
expense_attribute_value_map[project_name] = destination_id

expense_attributes = ExpenseAttribute.objects.filter(**filters)

Expand All @@ -142,8 +151,9 @@ def disable_projects(workspace_id: int, projects_to_disable: Dict, *args, **kwar
if bulk_payload:
logger.info(f"Disabling Projects in Fyle | WORKSPACE_ID: {workspace_id} | COUNT: {len(bulk_payload)}")
platform.projects.post_bulk(bulk_payload)
update_and_disable_cost_code(workspace_id, projects_to_disable, platform, use_code_in_naming)
platform.projects.sync()
else:
logger.info(f"No Projects to Disable in Fyle | WORKSPACE_ID: {workspace_id}")
update_and_disable_cost_code(workspace_id, projects_to_disable, platform, use_code_in_naming)
platform.projects.sync()

return bulk_payload
4 changes: 2 additions & 2 deletions apps/sage300/dependent_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def create_dependent_custom_field_in_fyle(workspace_id: int, fyle_attribute_type


@handle_import_exceptions
def post_dependent_cost_code(import_log: ImportLog, dependent_field_setting: DependentFieldSetting, platform: PlatformConnector, filters: Dict, is_enabled: bool = True) -> List[str]:
def post_dependent_cost_code(import_log: ImportLog, dependent_field_setting: DependentFieldSetting, platform: PlatformConnector, filters: Dict, is_enabled: bool = True) -> tuple[List[str], bool]:
import_settings = ImportSetting.objects.filter(workspace_id=import_log.workspace.id).first()
use_job_code_in_naming = False
use_cost_code_in_naming = False
Expand Down Expand Up @@ -281,7 +281,7 @@ def update_and_disable_cost_code(workspace_id: int, cost_codes_to_disable: Dict,
}
cost_code_import_log = ImportLog.create('COST_CODE', workspace_id)
# This call will disable the cost codes in Fyle that has old project name
posted_cost_codes = post_dependent_cost_code(cost_code_import_log, dependent_field_setting, platform, filters, is_enabled=False)
posted_cost_codes, _ = post_dependent_cost_code(cost_code_import_log, dependent_field_setting, platform, filters, is_enabled=False)

logger.info(f"Disabled Cost Codes in Fyle | WORKSPACE_ID: {workspace_id} | COUNT: {len(posted_cost_codes)}")

Expand Down
46 changes: 40 additions & 6 deletions apps/sage300/utils.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
from django.utils.module_loading import import_string
from fyle_accounting_mappings.models import DestinationAttribute, MappingSetting
from apps.workspaces.models import Sage300Credential
from apps.workspaces.models import Sage300Credential, ImportSetting
from sage_desktop_sdk.sage_desktop_sdk import SageDesktopSDK
from apps.sage300.models import CostCategory
from apps.mappings.models import Version
Expand Down Expand Up @@ -131,7 +131,7 @@ def _remove_credit_card_vendors(self):
logger.info(f'Deleting {vendor_count} credit card vendors from workspace_id {self.workspace_id}')
credit_card_vendor.delete()

def _sync_data(self, data_gen, attribute_type, display_name, workspace_id, field_names, is_generator: bool = True, vendor_type_mapping = None):
def _sync_data(self, data_gen, attribute_type, display_name, workspace_id, field_names, is_generator: bool = True, vendor_type_mapping = None, is_import_to_fyle_enabled: bool = False):
"""
Synchronize data from Sage Desktop SDK to your application
:param data: Data to synchronize
Expand Down Expand Up @@ -159,7 +159,8 @@ def _sync_data(self, data_gen, attribute_type, display_name, workspace_id, field
attribute_type,
workspace_id,
True,
attribute_disable_callback_path=ATTRIBUTE_CALLBACK_MAP[source_type]
attribute_disable_callback_path=ATTRIBUTE_CALLBACK_MAP[source_type],
is_import_to_fyle_enabled=is_import_to_fyle_enabled
)
else:
DestinationAttribute.bulk_create_or_update_destination_attributes(
Expand All @@ -185,7 +186,10 @@ def sync_accounts(self):
"""
version = Version.objects.get(workspace_id=self.workspace_id).account
accounts = self.connection.accounts.get_all(version=version)
self._sync_data(accounts, 'ACCOUNT', 'accounts', self.workspace_id, ['code', 'version'])

is_import_to_fyle_enabled = self.is_imported_enabled('ACCOUNT', self.workspace_id)

self._sync_data(accounts, 'ACCOUNT', 'accounts', self.workspace_id, ['code', 'version'], is_import_to_fyle_enabled=is_import_to_fyle_enabled)
return []

def sync_vendors(self):
Expand All @@ -201,14 +205,16 @@ def sync_vendors(self):
vendor_types = None
vendor_type_mapping = None

is_import_to_fyle_enabled = self.is_imported_enabled('VENDOR', self.workspace_id)
logger.info(f'is_import_to_fyle_enabled: {is_import_to_fyle_enabled}')
if not DestinationAttribute.objects.filter(workspace_id=self.workspace_id, attribute_type='VENDOR_TYPE').exists():
vendor_types = self.connection.vendors.get_vendor_types()
self._sync_data(vendor_types, 'VENDOR_TYPE', 'vendor_type', self.workspace_id, ['version'])

vendor_types = DestinationAttribute.objects.filter(workspace_id=self.workspace_id, attribute_type='VENDOR_TYPE').values('destination_id', 'value').distinct()
vendor_type_mapping = {vendor_type['destination_id']: vendor_type['value'] for vendor_type in vendor_types}

self._sync_data(vendors, 'VENDOR', 'vendor', self.workspace_id, field_names, vendor_type_mapping=vendor_type_mapping)
self._sync_data(vendors, 'VENDOR', 'vendor', self.workspace_id, field_names, vendor_type_mapping=vendor_type_mapping, is_import_to_fyle_enabled=is_import_to_fyle_enabled)
return []

def sync_jobs(self):
Expand All @@ -220,7 +226,10 @@ def sync_jobs(self):
field_names = [
'code', 'status', 'version', 'account_prefix_id', 'created_on_utc'
]
self._sync_data(jobs, 'JOB', 'job', self.workspace_id, field_names)

is_import_to_fyle_enabled = self.is_imported_enabled('JOB', self.workspace_id)

self._sync_data(jobs, 'JOB', 'job', self.workspace_id, field_names, is_import_to_fyle_enabled=is_import_to_fyle_enabled)
return []

def sync_standard_cost_codes(self):
Expand Down Expand Up @@ -317,3 +326,28 @@ def get_source_type(self, attribute_type, workspace_id):
source_type = 'CATEGORY'

return source_type

def is_imported_enabled(self, attribute_type, workspace_id):
"""
Check if import is enabled for the attribute type
:param attribute_type: Type of the attribute
:return: Whether import is enabled
"""
is_import_to_fyle_enabled = False

import_settings = ImportSetting.objects.filter(workspace_id=self.workspace_id).first()
if not import_settings:
return is_import_to_fyle_enabled

if attribute_type == 'ACCOUNT' and import_settings.import_categories:
is_import_to_fyle_enabled = True

elif attribute_type == 'VENDOR' and import_settings.import_vendors_as_merchants:
is_import_to_fyle_enabled = True

elif attribute_type == 'JOB':
mapping_setting = MappingSetting.objects.filter(workspace_id=workspace_id, destination_field='JOB').first()
if mapping_setting and mapping_setting.import_to_fyle:
is_import_to_fyle_enabled = True

return is_import_to_fyle_enabled
Comment on lines +330 to +353
Copy link

Choose a reason for hiding this comment

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

Review of is_imported_enabled method

The method effectively encapsulates the logic for checking import settings based on the attribute type, enhancing code readability and maintainability.

However, the static analysis tool suggests a potential improvement:

  • Combine similar if branches to simplify the conditional logic.

Refactor the method to combine similar conditions, improving the clarity and reducing redundancy:

- if attribute_type == 'ACCOUNT' and import_settings.import_categories:
-     is_import_to_fyle_enabled = True
- elif attribute_type == 'VENDOR' and import_settings.import_vendors_as_merchants:
-     is_import_to_fyle_enabled = True
+ if (attribute_type == 'ACCOUNT' and import_settings.import_categories) or \
+    (attribute_type == 'VENDOR' and import_settings.import_vendors_as_merchants):
+    is_import_to_fyle_enabled = 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 is_imported_enabled(self, attribute_type, workspace_id):
"""
Check if import is enabled for the attribute type
:param attribute_type: Type of the attribute
:return: Whether import is enabled
"""
is_import_to_fyle_enabled = False
import_settings = ImportSetting.objects.filter(workspace_id=self.workspace_id).first()
if not import_settings:
return is_import_to_fyle_enabled
if attribute_type == 'ACCOUNT' and import_settings.import_categories:
is_import_to_fyle_enabled = True
elif attribute_type == 'VENDOR' and import_settings.import_vendors_as_merchants:
is_import_to_fyle_enabled = True
elif attribute_type == 'JOB':
mapping_setting = MappingSetting.objects.filter(workspace_id=workspace_id, destination_field='JOB').first()
if mapping_setting and mapping_setting.import_to_fyle:
is_import_to_fyle_enabled = True
return is_import_to_fyle_enabled
def is_imported_enabled(self, attribute_type, workspace_id):
"""
Check if import is enabled for the attribute type
:param attribute_type: Type of the attribute
:return: Whether import is enabled
"""
is_import_to_fyle_enabled = False
import_settings = ImportSetting.objects.filter(workspace_id=self.workspace_id).first()
if not import_settings:
return is_import_to_fyle_enabled
if (attribute_type == 'ACCOUNT' and import_settings.import_categories) or \
(attribute_type == 'VENDOR' and import_settings.import_vendors_as_merchants):
is_import_to_fyle_enabled = True
elif attribute_type == 'JOB':
mapping_setting = MappingSetting.objects.filter(workspace_id=workspace_id, destination_field='JOB').first()
if mapping_setting and mapping_setting.import_to_fyle:
is_import_to_fyle_enabled = True
return is_import_to_fyle_enabled
Tools
Ruff

342-346: Combine if branches using logical or operator

Combine if branches

(SIM114)

4 changes: 2 additions & 2 deletions requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ fyle==0.37.0

# Reusable Fyle Packages
fyle-rest-auth==1.7.2
fyle-accounting-mappings==1.34.2
fyle-integrations-platform-connector==1.39.0
fyle-accounting-mappings==1.34.4
fyle-integrations-platform-connector==1.39.1


# Postgres Dependincies
Expand Down
12 changes: 11 additions & 1 deletion tests/test_fyle/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
)
from apps.fyle.models import Expense
from apps.workspaces.models import Workspace
from apps.accounting_exports.models import AccountingExport


def test_update_non_exported_expenses(db, create_temp_workspace, mocker, api_client):
Expand All @@ -29,6 +30,14 @@ def test_update_non_exported_expenses(db, create_temp_workspace, mocker, api_cli
expense_created.accounting_export_summary = {}
expense_created.save()

accounting_export, _ = AccountingExport.objects.update_or_create(
workspace_id=1,
type='PURCHASE_INVOICE',
status='EXPORT_READY'
)
accounting_export.expenses.add(expense_created)
accounting_export.save()

workspace = Workspace.objects.filter(id=1).first()
workspace.org_id = org_id
workspace.save()
Expand All @@ -40,7 +49,8 @@ def test_update_non_exported_expenses(db, create_temp_workspace, mocker, api_cli
expense = Expense.objects.get(expense_id='txhJLOSKs1iN', org_id=org_id)
assert expense.category == 'ABN Withholding'

expense.accounting_export_summary = {"synced": True, "state": "COMPLETE"}
accounting_export.status = 'COMPLETE'
accounting_export.save()
expense.category = 'Old Category'
expense.save()

Expand Down
Loading
Loading