Refactor imports for project and cost_center #314
Conversation
* Resource Cost_Center: refactored imports * Refactor imports for Project resource (#311) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Project: added cluster import, fixed minor typo, added new supplier field
WalkthroughThe recent update introduces enhancements to the synchronization process, focusing on refining the handling of various mapping types and exceptions. It also improves the logic for scheduling and deleting tasks related to Fyle integrations, alongside the introduction of new functions to better manage auto synchronization permissions and task construction. This overhaul simplifies the process, making it more efficient and adaptable to specific settings and configurations. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 12
Configuration used: CodeRabbit UI
Files selected for processing (10)
- apps/mappings/constants.py (1 hunks)
- apps/mappings/exceptions.py (2 hunks)
- apps/mappings/helpers.py (1 hunks)
- apps/mappings/queue.py (2 hunks)
- apps/mappings/schedules.py (1 hunks)
- apps/mappings/signals.py (2 hunks)
- apps/mappings/tasks.py (3 hunks)
- apps/workspaces/apis/import_settings/triggers.py (4 hunks)
- apps/xero/actions.py (3 hunks)
- fyle_integrations_imports (1 hunks)
Additional comments: 17
fyle_integrations_imports (1)
- 1-1: The subproject commit reference has been updated. Ensure that the new commit (
a14efdaa0d14604f4b898325de3fb600bfa7b1a7) is stable and compatible with the current project setup.apps/mappings/constants.py (1)
- 47-53: New entries have been added to the
SYNC_METHODSdictionary. Ensure that these new mappings are utilized correctly throughout the project and are compatible with any external services or APIs they interact with.apps/mappings/schedules.py (1)
- 8-48: The
new_schedule_or_delete_fyle_import_tasksfunction has been added to schedule or delete Fyle import tasks based on specific conditions. Ensure that the conditions for scheduling and deleting tasks are comprehensive and cover all necessary scenarios. Also, verify that the deletion of tasks does not inadvertently remove necessary schedules.apps/mappings/signals.py (1)
- 43-53: The functionality related to scheduling tasks has been refactored to use
new_schedule_or_delete_fyle_import_tasks, and the logic for handling different source fields has been consolidated usingALLOWED_SOURCE_FIELDS. Ensure thorough testing is conducted to verify that this refactoring does not introduce regressions or unintended behavior changes in task scheduling and handling of source fields.apps/xero/actions.py (1)
- 55-74: Updates to
refersh_xero_dimensioninclude handling new mappings forPROJECTandCOST_CENTERand updates to task calls within the loop to reflect the new logic. Ensure the correct application of theis_auto_sync_allowedfunction and verify that all scenarios are correctly handled by the loop logic, including edge cases that could lead to incorrect behavior.apps/workspaces/apis/import_settings/triggers.py (1)
- 50-53: The functionality in
triggers.pyhas been updated to usenew_schedule_or_delete_fyle_import_tasksfor workspace settings and mapping settings, replacingschedule_cost_centers_creation. Ensure that the new function is called with the correct parameters and that its behavior aligns with expectations. Also, verify that no specific scenarios handled by the previous function are overlooked.apps/mappings/queue.py (1)
- 105-152: The
construct_tasks_and_chain_import_fields_to_fylefunction has been added to construct tasks based on mapping settings and chain import fields to Fyle. Ensure that the task settings are comprehensive and correctly reflect the project's requirements. Also, verify that the chaining of tasks functions as expected in various scenarios, including edge cases that could lead to incorrect behavior.apps/mappings/tasks.py (10)
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [1-13]
The import statements and global variables are well-organized and clear. However, it's important to ensure that all imported modules and variables are used within the file. Unused imports should be removed to keep the code clean and maintainable.
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [60-76]
The function
resolve_expense_attribute_errorsis concise and serves its purpose well. It's good to see the use of type annotations for clarity. Ensure that the database queries are optimized for performance, especially when dealing with large datasets.
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [125-142]
The function
get_all_categories_from_fylefetches categories from Fyle and constructs a name map. It's a good practice to handle API pagination as done here. Ensure that error handling is robust around the API call to handle potential failures gracefully.
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [200-229]
The decorator
@handle_import_exceptionsis used to handle exceptions for theauto_create_category_mappingsfunction. Ensure that this decorator is thoroughly tested, especially for different types of exceptions that might occur during the mapping process. It's crucial that the application can gracefully handle and log errors.Verification successful
The verification process has identified unit tests covering the functionalities decorated by the
@handle_import_exceptionsdecorator, indicating that the decorator's exception handling capabilities are likely tested indirectly through these tests. This addresses the review comment concerning the thorough testing of the decorator for different types of exceptions that might occur during the mapping process.* 287-292: > 📝 **NOTE** > This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [231-253]Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure the handle_import_exceptions decorator is covered by unit tests. rg --type py "def test_handle_import_exceptions"Length of output: 50
Script:
#!/bin/bash # Find usage of the handle_import_exceptions decorator to identify which functions it decorates. rg --type py "@handle_import_exceptions"Length of output: 512
Script:
#!/bin/bash # Placeholder for the next step, to be filled after identifying the functions decorated by handle_import_exceptions. # This script will search for tests covering those functions. # Example: rg --type py "def test_<function_name>"Length of output: 0
Script:
#!/bin/bash # Search for tests related to the functionalities mentioned in the decorator usage. # Given the specific nature of the tasks, a broader search strategy is employed. rg --type py "test_auto_create_category_mappings|test_async_auto_map_employees|test_auto_create_expense_fields_mappings|test_async_auto_create_custom_field_mappings|test_auto_create_tax_codes_mappings"Length of output: 573
Script:
#!/bin/bash # Additionally, search for tests that might indirectly cover the functionalities by mentioning key terms like "mappings" and "auto create" in the test names. rg --type py "def test_.*(mappings|auto_create).*"Length of output: 539
The function
async_auto_map_employeesdemonstrates good use of external service connectors and settings from the database. Ensure that there's error handling around external service calls and database queries to handle any potential failures or exceptions.
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [330-356]
The function
upload_attributes_to_fyleuploads attributes to Fyle after removing duplicates and creating a custom field payload. It's clear and follows a logical flow. Ensure that theplatform.expense_custom_fields.postcall is wrapped in try-except blocks to handle potential API failures gracefully.
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [358-376]
The function
auto_create_expense_fields_mappingsis concise and leverages other functions to perform its task. It's good to see the use of bulk operations for creating mappings. Ensure that there's sufficient logging and error handling around the bulk creation process.
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [431-454]
The function
auto_create_tax_codes_mappingsis responsible for creating tax code mappings. It follows a clear process and makes good use of existing functions. Ensure that there's robust error handling around external service interactions and database operations.
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [456-468]
The function
auto_create_suppliers_as_merchantsdemonstrates a straightforward approach to creating suppliers as merchants. It's good to see the use of external connectors. Ensure that thepostmethod call to the Fyle connector is wrapped in try-except blocks to handle potential API failures.
- 287-292: > 📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [470-496]
The function
auto_import_and_map_fyle_fieldsorchestrates the auto-import and mapping process. It's structured and makes good use of theChainclass for task management. Ensure that there's sufficient error handling and logging within each task to handle and track failures effectively.
apps/mappings/helpers.py
Outdated
| def is_auto_sync_allowed(workspace_general_settings: WorkspaceGeneralSettings, mapping_setting: MappingSetting = None): | ||
| """ | ||
| Get the auto sync permission | ||
| :return: bool | ||
| """ | ||
| 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 |
There was a problem hiding this comment.
The is_auto_sync_allowed function has been added to determine auto-sync permission based on specific conditions. Consider enhancing the function's documentation to provide more details about its parameters and return value. Also, ensure that this function is called in the appropriate contexts and that its return value is handled correctly.
- :return: bool
+ :param workspace_general_settings: WorkspaceGeneralSettings instance
+ :param mapping_setting: Optional MappingSetting instance
+ :return: bool indicating whether auto-sync is allowedCommittable 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.
| def is_auto_sync_allowed(workspace_general_settings: WorkspaceGeneralSettings, mapping_setting: MappingSetting = None): | |
| """ | |
| Get the auto sync permission | |
| :return: bool | |
| """ | |
| 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 | |
| def is_auto_sync_allowed(workspace_general_settings: WorkspaceGeneralSettings, mapping_setting: MappingSetting = None): | |
| """ | |
| Get the auto sync permission | |
| :param workspace_general_settings: WorkspaceGeneralSettings instance | |
| :param mapping_setting: Optional MappingSetting instance | |
| :return: bool indicating whether auto-sync is allowed | |
| """ | |
| 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 |
apps/mappings/exceptions.py
Outdated
| def handle_import_exceptions_v2(func): | ||
| def new_fn(expense_attribute_instance, *args): | ||
| import_log: ImportLog = args[0] | ||
| workspace_id = import_log.workspace_id | ||
| attribute_type = import_log.attribute_type | ||
| error = { | ||
| 'task': 'Import {0} to Fyle and Auto Create Mappings'.format(attribute_type), | ||
| 'workspace_id': workspace_id, | ||
| 'message': None, | ||
| 'response': None | ||
| } | ||
| try: | ||
| return func(expense_attribute_instance, *args) | ||
| except WrongParamsError as exception: | ||
| error['message'] = exception.message | ||
| error['response'] = exception.response | ||
| error['alert'] = True | ||
| import_log.status = 'FAILED' | ||
|
|
||
| except InvalidTokenError: | ||
| error['message'] = 'Invalid Token for fyle' | ||
| error['alert'] = False | ||
| import_log.status = 'FAILED' | ||
|
|
||
| except InternalServerError: | ||
| error['message'] = 'Internal server error while importing to Fyle' | ||
| error['alert'] = True | ||
| import_log.status = 'FAILED' | ||
|
|
||
| except (XeroWrongParamsError, XeroInvalidTokenError, XeroCredentials.DoesNotExist) as exception: | ||
| error['message'] = 'Invalid Token or Xero credentials does not exist workspace_id - {0}'.format(workspace_id) | ||
| error['alert'] = False | ||
| error['response'] = exception.__dict__ | ||
| import_log.status = 'FAILED' | ||
|
|
||
| except Exception: | ||
| response = traceback.format_exc() | ||
| error['message'] = 'Something went wrong' | ||
| error['response'] = response | ||
| error['alert'] = False | ||
| import_log.status = 'FATAL' | ||
|
|
||
| if error['alert']: | ||
| logger.error(error) | ||
| else: | ||
| logger.info(error) | ||
|
|
||
| import_log.error_log = error | ||
| import_log.save() | ||
|
|
||
| return new_fn |
There was a problem hiding this comment.
The handle_import_exceptions_v2 function has been added to handle various exceptions and log errors accordingly. Consider enhancing the function's documentation to provide more details about its parameters and expected behavior. Also, ensure that all relevant exceptions are caught and that the error logging provides sufficient detail for troubleshooting.
- :param expense_attribute_instance: Instance of the expense attribute being processed
+ :param expense_attribute_instance: Instance of the expense attribute being processed
+ :param args: Additional arguments, including an ImportLog instance
+ :return: The result of the function call if no exceptions are caughtCommittable 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.
| def handle_import_exceptions_v2(func): | |
| def new_fn(expense_attribute_instance, *args): | |
| import_log: ImportLog = args[0] | |
| workspace_id = import_log.workspace_id | |
| attribute_type = import_log.attribute_type | |
| error = { | |
| 'task': 'Import {0} to Fyle and Auto Create Mappings'.format(attribute_type), | |
| 'workspace_id': workspace_id, | |
| 'message': None, | |
| 'response': None | |
| } | |
| try: | |
| return func(expense_attribute_instance, *args) | |
| except WrongParamsError as exception: | |
| error['message'] = exception.message | |
| error['response'] = exception.response | |
| error['alert'] = True | |
| import_log.status = 'FAILED' | |
| except InvalidTokenError: | |
| error['message'] = 'Invalid Token for fyle' | |
| error['alert'] = False | |
| import_log.status = 'FAILED' | |
| except InternalServerError: | |
| error['message'] = 'Internal server error while importing to Fyle' | |
| error['alert'] = True | |
| import_log.status = 'FAILED' | |
| except (XeroWrongParamsError, XeroInvalidTokenError, XeroCredentials.DoesNotExist) as exception: | |
| error['message'] = 'Invalid Token or Xero credentials does not exist workspace_id - {0}'.format(workspace_id) | |
| error['alert'] = False | |
| error['response'] = exception.__dict__ | |
| import_log.status = 'FAILED' | |
| except Exception: | |
| response = traceback.format_exc() | |
| error['message'] = 'Something went wrong' | |
| error['response'] = response | |
| error['alert'] = False | |
| import_log.status = 'FATAL' | |
| if error['alert']: | |
| logger.error(error) | |
| else: | |
| logger.info(error) | |
| import_log.error_log = error | |
| import_log.save() | |
| return new_fn | |
| def handle_import_exceptions_v2(func): | |
| """ | |
| :param expense_attribute_instance: Instance of the expense attribute being processed | |
| :param args: Additional arguments, including an ImportLog instance | |
| :return: The result of the function call if no exceptions are caught | |
| """ | |
| def new_fn(expense_attribute_instance, *args): | |
| import_log: ImportLog = args[0] | |
| workspace_id = import_log.workspace_id | |
| attribute_type = import_log.attribute_type | |
| error = { | |
| 'task': 'Import {0} to Fyle and Auto Create Mappings'.format(attribute_type), | |
| 'workspace_id': workspace_id, | |
| 'message': None, | |
| 'response': None | |
| } | |
| try: | |
| return func(expense_attribute_instance, *args) | |
| except WrongParamsError as exception: | |
| error['message'] = exception.message | |
| error['response'] = exception.response | |
| error['alert'] = True | |
| import_log.status = 'FAILED' | |
| except InvalidTokenError: | |
| error['message'] = 'Invalid Token for fyle' | |
| error['alert'] = False | |
| import_log.status = 'FAILED' | |
| except InternalServerError: | |
| error['message'] = 'Internal server error while importing to Fyle' | |
| error['alert'] = True | |
| import_log.status = 'FAILED' | |
| except (XeroWrongParamsError, XeroInvalidTokenError, XeroCredentials.DoesNotExist) as exception: | |
| error['message'] = 'Invalid Token or Xero credentials does not exist workspace_id - {0}'.format(workspace_id) | |
| error['alert'] = False | |
| error['response'] = exception.__dict__ | |
| import_log.status = 'FAILED' | |
| except Exception: | |
| response = traceback.format_exc() | |
| error['message'] = 'Something went wrong' | |
| error['response'] = response | |
| error['alert'] = False | |
| import_log.status = 'FATAL' | |
| if error['alert']: | |
| logger.error(error) | |
| else: | |
| logger.info(error) | |
| import_log.error_log = error | |
| import_log.save() | |
| return new_fn |
apps/mappings/tasks.py
Outdated
| fyle_projects = ExpenseAttribute.objects.filter( | ||
| workspace_id=workspace_id, attribute_type=FyleAttributeEnum.PROJECT, auto_mapped=True | ||
| ).order_by("value", "id")[offset:limit] | ||
|
|
||
| project_names = list(set(field.value for field in fyle_projects)) | ||
|
|
||
| xero_customers = DestinationAttribute.objects.filter( | ||
| attribute_type=destination_field, | ||
| workspace_id=workspace_id, | ||
| value__in=project_names, | ||
| ).values_list("value", flat=True) | ||
|
|
||
| for fyle_project in fyle_projects: | ||
| if fyle_project.value not in xero_customers: | ||
| fyle_project.active = False | ||
| fyle_project.save() | ||
| expense_attribute_to_be_disabled.append(fyle_project.id) | ||
|
|
||
| return expense_attribute_to_be_disabled | ||
|
|
||
|
|
||
| def post_projects_in_batches( | ||
| platform: PlatformConnector, workspace_id: int, destination_field: str | ||
| ): | ||
| existing_project_names = ExpenseAttribute.objects.filter( | ||
| attribute_type=FyleAttributeEnum.PROJECT, workspace_id=workspace_id | ||
| ).values_list("value", flat=True) | ||
| xero_attributes_count = DestinationAttribute.objects.filter( | ||
| attribute_type=destination_field, workspace_id=workspace_id | ||
| ).count() | ||
| page_size = 200 | ||
|
|
||
| for offset in range(0, xero_attributes_count, page_size): | ||
| limit = offset + page_size | ||
| paginated_xero_attributes = DestinationAttribute.objects.filter( | ||
| attribute_type=destination_field, workspace_id=workspace_id | ||
| ).order_by("value", "id")[offset:limit] | ||
|
|
||
| paginated_xero_attributes = remove_duplicates(paginated_xero_attributes) | ||
|
|
||
| fyle_payload: List[Dict] = create_fyle_projects_payload( | ||
| paginated_xero_attributes, existing_project_names | ||
| ) | ||
|
|
||
| if fyle_payload: | ||
| platform.projects.post_bulk(fyle_payload) | ||
| platform.projects.sync() | ||
|
|
||
| Mapping.bulk_create_mappings( | ||
| paginated_xero_attributes, FyleAttributeEnum.PROJECT, destination_field, workspace_id | ||
| ) | ||
|
|
||
| if destination_field == "CUSTOMER": | ||
| expense_attribute_to_be_disable = disable_renamed_projects( | ||
| workspace_id, destination_field | ||
| ) | ||
| project_ids_to_be_changed = disable_expense_attributes( | ||
| FyleAttributeEnum.PROJECT, "CUSTOMER", workspace_id | ||
| ) | ||
| project_ids_to_be_changed.extend(expense_attribute_to_be_disable) | ||
| if project_ids_to_be_changed: | ||
| expense_attributes = ExpenseAttribute.objects.filter( | ||
| id__in=project_ids_to_be_changed | ||
| ) | ||
| fyle_payload: List[Dict] = create_fyle_projects_payload( | ||
| projects=[], | ||
| existing_project_names=[], | ||
| updated_projects=expense_attributes, | ||
| ) | ||
| platform.projects.post_bulk(fyle_payload) | ||
| platform.projects.sync() | ||
|
|
||
|
|
||
| @handle_import_exceptions(task_name="auto_create_project_mappings") | ||
| def auto_create_project_mappings(workspace_id: int): | ||
| """ | ||
| Create Project Mappings | ||
| :return: mappings | ||
| """ | ||
|
|
||
| fyle_credentials: FyleCredential = FyleCredential.objects.get( | ||
| workspace_id=workspace_id | ||
| ) | ||
|
|
||
| platform = PlatformConnector(fyle_credentials) | ||
|
|
||
| platform.projects.sync() | ||
|
|
||
| mapping_setting = MappingSetting.objects.get( | ||
| source_field=FyleAttributeEnum.PROJECT, workspace_id=workspace_id | ||
| ) | ||
|
|
||
| sync_xero_attributes(mapping_setting.destination_field, workspace_id) | ||
|
|
||
| post_projects_in_batches(platform, workspace_id, mapping_setting.destination_field) | ||
|
|
||
|
|
||
| def create_fyle_expense_custom_fields_payload( | ||
| xero_attributes: List[DestinationAttribute], | ||
| workspace_id: int, |
There was a problem hiding this comment.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [15-17]
The logger configuration is directly set within the file. Consider using a centralized logging configuration if not already in place. This approach allows for easier maintenance and consistency across different modules of the application.
- logger.level = logging.INFO
+ # It's recommended to configure logging levels and handlers in a centralized logging configuration file.📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [19-37]
The DEFAULT_FYLE_CATEGORIES list is hardcoded. If these categories are static and unlikely to change, this approach is acceptable. However, if they might change or need to be customized per workspace or user, consider fetching them from a configuration file or database to enhance flexibility and maintainability.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [39-58]
The function disable_expense_attributes is well-structured and follows good practices. However, ensure that the filter variable name does not overshadow the built-in Python function filter. Using more descriptive variable names can improve readability and avoid potential confusion.
- filter = {"mapping__isnull": False, "mapping__destination_type": destination_field}
+ filter_criteria = {"mapping__isnull": False, "mapping__destination_type": destination_field}📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [78-91]
The function remove_duplicates is a utility function that removes duplicate attributes based on their value. While the function is clear, consider if there's a more efficient way to achieve this, especially if xero_attributes can be large. Using a set or dictionary to track seen values might improve performance.
- attribute_values = []
+ attribute_values = set()
- if attribute.value.lower() not in attribute_values:
+ if attribute.value.lower() not in attribute_values:
- attribute_values.append(attribute.value.lower())
+ attribute_values.add(attribute.value.lower())📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [93-123]
The function create_fyle_categories_payload constructs a payload for category import. It's well-commented and structured. However, consider extracting the payload construction into a separate function or using a more declarative approach to improve readability and maintainability.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [144-198]
The function upload_categories_to_fyle is responsible for uploading categories to Fyle. It's comprehensive and includes logging, which is good for monitoring. However, this function seems to be doing multiple things - fetching categories from Xero, creating a payload, and uploading to Fyle. Consider breaking it down into smaller functions to adhere to the Single Responsibility Principle (SRP) for better maintainability and testing.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [255-269]
The function sync_xero_attributes is a straightforward utility function for syncing attributes from Xero. It's good to see clear separation based on the attribute type. Consider adding logging for each case to provide more visibility into the sync process.
+ logger.info(f"Syncing {xero_attribute_type} from Xero.")📝 NOTE
This review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [271-328]
The function create_fyle_expense_custom_fields_payload is well-documented and handles various scenarios for constructing the payload. However, the nested if-else structure could be simplified for better readability. Consider using a mapping or a separate function to determine the placeholder value.
📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [378-395]
The function async_auto_create_custom_field_mappings iterates over mapping settings to create custom field mappings. It's well-structured and makes good use of existing functions. Consider adding more detailed logging to provide insights into the mapping creation process.
+ logger.info(f"Creating custom field mappings for {mapping_setting.source_field}.")📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [397-429]
The function upload_tax_groups_to_fyle and create_fyle_tax_group_payload work together to upload tax groups to Fyle. These functions are well-documented and structured. However, consider separating the payload creation logic into a different function or module to keep the code DRY and improve maintainability.
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
Hrishabh17
changed the title
* Added integrations_imports submodule, made changes in settings (#310) * Refactor imports for Project resource (#311) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Project: added cluster import, fixed minor typo, added new supplier field * Refactor imports for project and cost_center (#314) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Project: added cluster import, fixed minor typo, added new supplier field * Refactor imports costcenter (#312) * Resource Cost_Center: refactored imports * Refactor imports for Project resource (#311) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Project: added cluster import, fixed minor typo, added new supplier field * Refactor imports custom expense (#313) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Cost_Center: refactored imports * Resource Custom Expense Field: refactored imports * Resource Project: added cluster import, fixed minor typo, added new supplier field * Resource Tax Group: refactored imports (#315) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Cost_Center: refactored imports * Resource Custom Expense Field: refactored imports * Resource Project: added cluster import, fixed minor typo, added new supplier field * Resource Tax Group: refactored imports * Resource Tax Group: changed to is_auto_sync to False by default * Resource Category: refactored imports (#316) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Cost_Center: refactored imports * Resource Custom Expense Field: refactored imports * Resource Tax Group: refactored imports * Resource Category: refactored imports * Resource Category: changed is_auto_sync_enabled to True by default * Refactor imports supplier (#318) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Cost_Center: refactored imports * Resource Custom Expense Field: refactored imports * Resource Tax Group: refactored imports * Resource Category: refactored imports * Added RetryException handler and bumped the sdk versions (#306) * Set tasks limit to 1L (#317) * Set tasks limit to 1L * set timeout * Modified the import schedule creation condition --------- Co-authored-by: ruuushhh <[email protected]> * Refactor old flow (#321) * Added integrations_imports submodule, made changes in settings * Resource Project: Added new flow for the Project Import * Typo Fix * Resource Project: removed old import logic for Project * Resource Cost_Center: refactored imports * Resource Custom Expense Field: refactored imports * Resource Tax Group: refactored imports * Resource Category: refactored imports * Added RetryException handler and bumped the sdk versions (#306) * Set tasks limit to 1L (#317) * Set tasks limit to 1L * set timeout * Modified the import schedule creation condition * Removed dead code, add few test cases, added script for adding new schedule for different workspaces * Added post deploy sql script to dump old schedules and delete --------- Co-authored-by: ruuushhh <[email protected]> * Merged changes from master * Update delete workspace func (#319) * Update delete workspace func * Fix comments * upate workspace last synced fix (#320) * Modified the workflow for submodule * Decreased the coverage to 88% --------- Co-authored-by: ruuushhh <[email protected]>
Summary by CodeRabbit