-
Notifications
You must be signed in to change notification settings - Fork 0
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
Imports refactor master #322
Changes from 13 commits
823f0b7
ef25328
7c0469b
8ac1c16
457537b
44e9af1
ae893af
e168eda
a03449f
4f057c6
c39a459
0b32cd6
55d9050
af10b05
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
[submodule "fyle_integrations_imports"] | ||
path = fyle_integrations_imports | ||
url = [email protected]:fylein/fyle_integrations_imports.git |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,42 +1,14 @@ | ||
from datetime import datetime | ||
|
||
from django_q.models import Schedule | ||
from fyle_accounting_mappings.models import MappingSetting | ||
|
||
from apps.fyle.enums import FyleAttributeEnum | ||
from apps.workspaces.models import WorkspaceGeneralSettings | ||
|
||
|
||
def schedule_or_delete_fyle_import_tasks(configuration: WorkspaceGeneralSettings): | ||
def is_auto_sync_allowed(workspace_general_settings: WorkspaceGeneralSettings, mapping_setting: MappingSetting = None): | ||
""" | ||
:param configuration: WorkspaceGeneralSettings Instance | ||
:return: None | ||
Get the auto sync permission | ||
:return: bool | ||
""" | ||
project_mapping = MappingSetting.objects.filter( | ||
source_field=FyleAttributeEnum.PROJECT, workspace_id=configuration.workspace_id | ||
).first() | ||
if ( | ||
configuration.import_categories | ||
or (project_mapping and project_mapping.import_to_fyle) | ||
or configuration.import_suppliers_as_merchants | ||
): | ||
start_datetime = datetime.now() | ||
Schedule.objects.update_or_create( | ||
func="apps.mappings.tasks.auto_import_and_map_fyle_fields", | ||
cluster='import', | ||
args="{}".format(configuration.workspace_id), | ||
defaults={ | ||
"schedule_type": Schedule.MINUTES, | ||
"minutes": 24 * 60, | ||
"next_run": start_datetime, | ||
}, | ||
) | ||
elif ( | ||
not configuration.import_categories | ||
and not (project_mapping and project_mapping.import_to_fyle) | ||
and not configuration.import_suppliers_as_merchants | ||
): | ||
Schedule.objects.filter( | ||
func="apps.mappings.tasks.auto_import_and_map_fyle_fields", | ||
args="{}".format(configuration.workspace_id), | ||
).delete() | ||
is_auto_sync_status_allowed = False | ||
if (mapping_setting and mapping_setting.destination_field == 'CUSTOMER' and mapping_setting.source_field == 'PROJECT') or workspace_general_settings.import_categories: | ||
is_auto_sync_status_allowed = True | ||
|
||
return is_auto_sync_status_allowed |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,13 @@ | ||
from datetime import datetime, timedelta | ||
from datetime import datetime | ||
|
||
from django_q.models import Schedule | ||
from fyle_accounting_mappings.models import MappingSetting | ||
from apps.fyle.enums import FyleAttributeEnum | ||
from apps.mappings.constants import SYNC_METHODS | ||
from apps.mappings.helpers import is_auto_sync_allowed | ||
from fyle_integrations_imports.dataclasses import TaskSetting | ||
from fyle_integrations_imports.queues import chain_import_fields_to_fyle | ||
from apps.workspaces.models import WorkspaceGeneralSettings, XeroCredentials | ||
|
||
|
||
def schedule_auto_map_employees(employee_mapping_preference: str, workspace_id: str): | ||
|
@@ -26,71 +32,77 @@ | |
schedule.delete() | ||
|
||
|
||
def schedule_cost_centers_creation(import_to_fyle, workspace_id: int): | ||
if import_to_fyle: | ||
schedule, _ = Schedule.objects.update_or_create( | ||
func="apps.mappings.tasks.auto_create_cost_center_mappings", | ||
cluster='import', | ||
args="{}".format(workspace_id), | ||
defaults={ | ||
"schedule_type": Schedule.MINUTES, | ||
"minutes": 24 * 60, | ||
"next_run": datetime.now(), | ||
}, | ||
) | ||
else: | ||
schedule: Schedule = Schedule.objects.filter( | ||
func="apps.mappings.tasks.auto_create_cost_center_mappings", | ||
args="{}".format(workspace_id), | ||
).first() | ||
|
||
if schedule: | ||
schedule.delete() | ||
def construct_tasks_and_chain_import_fields_to_fyle(workspace_id: int): | ||
""" | ||
Construct tasks and chain import fields to fyle | ||
:param workspace_id: Workspace Id | ||
""" | ||
mapping_settings = MappingSetting.objects.filter( | ||
workspace_id=workspace_id, | ||
import_to_fyle=True | ||
) | ||
workspace_general_settings = WorkspaceGeneralSettings.objects.get( | ||
workspace_id=workspace_id | ||
) | ||
credentials = XeroCredentials.objects.get( | ||
workspace_id=workspace_id | ||
) | ||
|
||
# import_vendors_as_merchants is not used in xero, placeholder to avoid KeyError | ||
task_settings: TaskSetting = { | ||
'import_tax': None, | ||
'import_vendors_as_merchants': None, | ||
'import_suppliers_as_merchants': None, | ||
'import_categories': None, | ||
'import_items': None, | ||
'mapping_settings': [], | ||
'credentials': credentials, | ||
'sdk_connection_string': 'apps.xero.utils.XeroConnector', | ||
'custom_properties': None | ||
} | ||
|
||
def schedule_tax_groups_creation(import_tax_codes, workspace_id): | ||
if import_tax_codes: | ||
schedule, _ = Schedule.objects.update_or_create( | ||
func="apps.mappings.tasks.auto_create_tax_codes_mappings", | ||
cluster='import', | ||
args="{}".format(workspace_id), | ||
defaults={ | ||
"schedule_type": Schedule.MINUTES, | ||
"minutes": 24 * 60, | ||
"next_run": datetime.now(), | ||
}, | ||
) | ||
else: | ||
schedule: Schedule = Schedule.objects.filter( | ||
func="apps.mappings.tasks.auto_create_tax_codes_mappings", | ||
args="{}".format(workspace_id), | ||
).first() | ||
# For now adding only for PROJECT | ||
ALLOWED_SOURCE_FIELDS = [ | ||
FyleAttributeEnum.PROJECT, | ||
FyleAttributeEnum.COST_CENTER, | ||
] | ||
|
||
if schedule: | ||
schedule.delete() | ||
if workspace_general_settings.import_tax_codes: | ||
task_settings['import_tax'] = { | ||
'destination_field': 'TAX_CODE', | ||
'destination_sync_methods': [SYNC_METHODS['TAX_CODE']], | ||
'is_auto_sync_enabled': False, | ||
'is_3d_mapping': False | ||
} | ||
|
||
if workspace_general_settings.import_categories: | ||
task_settings['import_categories'] = { | ||
'destination_field': 'ACCOUNT', | ||
'destination_sync_methods': [SYNC_METHODS['ACCOUNT']], | ||
'is_auto_sync_enabled': True, | ||
'is_3d_mapping': False, | ||
'charts_of_accounts': workspace_general_settings.charts_of_accounts | ||
} | ||
|
||
def schedule_fyle_attributes_creation(workspace_id: int): | ||
mapping_settings = MappingSetting.objects.filter( | ||
is_custom=True, import_to_fyle=True, workspace_id=workspace_id | ||
).all() | ||
if workspace_general_settings.import_suppliers_as_merchants: | ||
task_settings['custom_properties'] = { | ||
'func': 'apps.mappings.tasks.auto_create_suppliers_as_merchants', | ||
'args': { | ||
'workspace_id': workspace_id | ||
} | ||
} | ||
|
||
if mapping_settings: | ||
schedule, _ = Schedule.objects.get_or_create( | ||
func="apps.mappings.tasks.async_auto_create_custom_field_mappings", | ||
cluster='import', | ||
args="{0}".format(workspace_id), | ||
defaults={ | ||
"schedule_type": Schedule.MINUTES, | ||
"minutes": 24 * 60, | ||
"next_run": datetime.now() + timedelta(hours=24), | ||
}, | ||
) | ||
else: | ||
schedule: Schedule = Schedule.objects.filter( | ||
func="apps.mappings.tasks.async_auto_create_custom_field_mappings", | ||
args=workspace_id, | ||
).first() | ||
for mapping_setting in mapping_settings: | ||
if mapping_setting.source_field in ALLOWED_SOURCE_FIELDS or mapping_setting.is_custom: | ||
task_settings['mapping_settings'].append( | ||
{ | ||
'source_field': mapping_setting.source_field, | ||
'destination_field': mapping_setting.destination_field, | ||
'is_custom': mapping_setting.is_custom, | ||
'destination_sync_methods': [SYNC_METHODS.get(mapping_setting.destination_field.upper(), 'tracking_categories')], | ||
'is_auto_sync_enabled': is_auto_sync_allowed(workspace_general_settings, mapping_setting) | ||
} | ||
) | ||
|
||
if schedule: | ||
schedule.delete() | ||
chain_import_fields_to_fyle(workspace_id, task_settings) | ||
Comment on lines
+35
to
+108
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The refactoring in
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,50 @@ | ||
from datetime import datetime | ||
from typing import List, Dict | ||
from apps.workspaces.models import WorkspaceGeneralSettings | ||
from django_q.models import Schedule | ||
from fyle_accounting_mappings.models import MappingSetting | ||
|
||
|
||
def new_schedule_or_delete_fyle_import_tasks( | ||
workspace_general_settings_instance: WorkspaceGeneralSettings, | ||
mapping_settings: List[Dict] | ||
): | ||
""" | ||
Schedule or delete fyle import tasks based on the | ||
workspace general settings and mapping settings | ||
:param workspace_general_settings_instance: WorkspaceGeneralSettings instance | ||
:param mapping_settings: List of mapping settings | ||
:return: None | ||
""" | ||
# short-hand notation, it returns True as soon as it encounters import_to_fyle as True | ||
task_to_be_scheduled = any(mapping_setting['import_to_fyle'] for mapping_setting in mapping_settings) | ||
|
||
if ( | ||
task_to_be_scheduled | ||
or workspace_general_settings_instance.import_customers | ||
or workspace_general_settings_instance.import_tax_codes | ||
or workspace_general_settings_instance.import_categories | ||
or workspace_general_settings_instance.import_suppliers_as_merchants | ||
): | ||
Schedule.objects.update_or_create( | ||
func='apps.mappings.queue.construct_tasks_and_chain_import_fields_to_fyle', | ||
args='{}'.format(workspace_general_settings_instance.workspace_id), | ||
defaults={ | ||
'schedule_type':Schedule.MINUTES, | ||
'minutes': 24 * 60, | ||
'next_run': datetime.now(), | ||
'cluster': 'import' | ||
} | ||
) | ||
else: | ||
import_fields_count = MappingSetting.objects.filter( | ||
workspace_id=workspace_general_settings_instance.workspace_id, | ||
import_to_fyle=True | ||
).count() | ||
|
||
# if there are no import fields, delete the schedule | ||
if import_fields_count == 0: | ||
Schedule.objects.filter( | ||
func='apps.mappings.queue.construct_tasks_and_chain_import_fields_to_fyle', | ||
args='{}'.format(workspace_general_settings_instance.workspace_id) | ||
).delete() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The introduction of
handle_import_exceptions_v2
inapps/mappings/exceptions.py
is a valuable addition for handling and logging errors during the import process. A few suggestions for improvement:The function does a good job of handling various exceptions and updating the
ImportLog
status accordingly. Ensure that all relevant exceptions are covered and that the error messages are clear and informative.The use of
error['alert']
to determine whether to log an error as info or error is a good practice. However, consider standardizing the conditions under whicherror['alert']
is set toTrue
orFalse
to ensure consistency across different types of errors.The function updates the
ImportLog
with error details, which is crucial for debugging. Ensure that theImportLog
model has appropriate fields to store all necessary error information and that this information is easily accessible for troubleshooting.Consider adding unit tests specifically for
handle_import_exceptions_v2
to verify that it behaves as expected for different types of exceptions. This can help ensure robustness and ease future maintenance.Verify coverage of all relevant exceptions and clarity of error messages.
Standardize conditions for setting
error['alert']
.Ensure the
ImportLog
model adequately supports error logging.Add unit tests for
handle_import_exceptions_v2
.