Refactor job sync #193
Refactor job sync #193
Conversation
…check for current_state
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dep-import-log #193 +/- ##
=================================================
Coverage ? 94.13%
=================================================
Files ? 101
Lines ? 5645
Branches ? 0
=================================================
Hits ? 5314
Misses ? 331
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Outside diff range and nitpick comments (13)
apps/mappings/imports/queues.py (1)
21-27: Ensure proper management of task dependencies and error handling.The chaining of tasks now includes
ImportLogentries, which is good for tracking. However, consider handling potential failures in the chain to ensure robustness.apps/sage300/models.py (1)
Line range hint
75-88: Optimization Suggestion: Use set comprehension for efficiency.Consider using set comprehensions for initializing
jobs_to_be_updatedwhich can be more efficient and concise.- jobs_to_be_updated = set() + jobs_to_be_updated = {category.job_id for category in list_of_categories if job_name_mapping.get(category.job_id) and cost_code_name_mapping.get(category.cost_code_id)}tests/test_sage300/test_helpers.py (2)
126-126: Clarify Test ExpectationsIt's important to clarify why the
sync_call.call_countis expected to be 2. Adding a comment explaining this would improve the readability and maintainability of the test.
138-138: Clarify Test ExpectationsSimilar to a previous point, explain why the
sync_call.call_countis expected to be 4 in this scenario.apps/sage300/helpers.py (1)
Line range hint
138-160: Use Modern String FormattingConsider using f-strings for better readability and performance in string operations.
- 'Sage 300 Project - {0}, Id - {1}'.format( - expense_attribute.value, - code - ) + f'Sage 300 Project - {expense_attribute.value}, Id - {code}'apps/sage300/dependent_fields.py (2)
Line range hint
26-26: Refactor to use dictionary'sgetmethod.To simplify the code and enhance readability, replace the conditional check with the
getmethod of the dictionary.- placeholder = existing_attribute['placeholder'] if 'placeholder' in existing_attribute else None + placeholder = existing_attribute.get('placeholder', None)
Line range hint
35-35: Optimize string formatting.Convert the string formatting to use f-strings for better performance and readability.
- new_placeholder = 'Select {0}'.format(fyle_attribute) + new_placeholder = f'Select {fyle_attribute}'apps/sage300/utils.py (1)
Line range hint
80-83: Refactor to use dictionary'sgetmethod.Simplify the dictionary access by using the
getmethod, which is more concise and avoids the need for conditional checks.- if vendor_type_mapping: - if item.type_id in vendor_type_mapping: - detail['type'] = vendor_type_mapping[item.type_id] - else: - detail['type'] = None + detail['type'] = vendor_type_mapping.get(item.type_id, None)tests/test_sage300/test_utils.py (1)
Line range hint
457-488: Ensure consistent and accurate testing.Review the test setup to ensure it accurately simulates the conditions under which the synchronization functions operate. Consider adding more detailed assertions to verify the behavior of the functions more comprehensively.
Would you like detailed examples of additional test cases or assertions that could be added to improve the test coverage?
tests/test_sage300/test_exports/test_base_model.py (2)
106-106: Ensure consistent parameter passing forjob_idThe
job_idparameter is hardcoded in thetest_get_cost_code_idfunction. This might limit the test's flexibility and reusability.Consider parameterizing this value or fetching it dynamically to enhance test robustness and maintainability.
130-130: Parameterizeproject_idandcost_code_idfor better test flexibilitySimilar to the previous comment, hardcoding
project_idandcost_code_idintest_get_cost_category_idcould restrict the test's adaptability to different data sets.It would be beneficial to parameterize these values to allow for more dynamic testing scenarios.
tests/conftest.py (2)
Line range hint
135-135: Improve string formatting with f-stringsUsing f-strings can make the code cleaner and more readable.
- name='Fyle For Testing {}'.format(workspace_id), + name=f'Fyle For Testing {workspace_id}', - org_id='riseabovehate{}'.format(workspace_id), + org_id=f'riseabovehate{workspace_id}',Also applies to: 136-136
Line range hint
104-104: Potential security risk: Generic API Key exposedThe presence of a generic API key in the code could pose a security risk as it might expose access to various services.
Consider using environment variables or secure vault solutions to manage sensitive information like API keys.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (20)
- apps/fyle/serializers.py (2 hunks)
- apps/mappings/exceptions.py (2 hunks)
- apps/mappings/imports/modules/base.py (3 hunks)
- apps/mappings/imports/modules/projects.py (2 hunks)
- apps/mappings/imports/queues.py (3 hunks)
- apps/mappings/imports/tasks.py (2 hunks)
- apps/mappings/models.py (1 hunks)
- apps/mappings/tasks.py (2 hunks)
- apps/sage300/dependent_fields.py (4 hunks)
- apps/sage300/helpers.py (5 hunks)
- apps/sage300/models.py (4 hunks)
- apps/sage300/utils.py (3 hunks)
- requirements.txt (1 hunks)
- tests/conftest.py (1 hunks)
- tests/test_fyle/test_views.py (1 hunks)
- tests/test_mappings/test_imports/test_modules/test_projects.py (1 hunks)
- tests/test_sage300/test_dependent_fields.py (5 hunks)
- tests/test_sage300/test_exports/test_base_model.py (2 hunks)
- tests/test_sage300/test_helpers.py (4 hunks)
- tests/test_sage300/test_utils.py (5 hunks)
Files skipped from review due to trivial changes (2)
- apps/mappings/imports/tasks.py
- requirements.txt
Additional context used
Ruff
apps/mappings/exceptions.py
29-29: Use implicit references for positional format fields (UP030)
Remove explicit positional indices
29-29: Use f-string instead of
formatcall (UP032)Convert to f-string
43-43: Use implicit references for positional format fields (UP030)
Remove explicit positional indices
43-43: Use f-string instead of
formatcall (UP032)Convert to f-string
apps/mappings/imports/modules/projects.py
53-56: Use implicit references for positional format fields (UP030)
Remove explicit positional indices
53-56: Use f-string instead of
formatcall (UP032)Convert to f-string
apps/fyle/serializers.py
59-59: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)
63-63: Within an
exceptclause, raise exceptions withraise ... from errorraise ... from Noneto distinguish them from errors in exception handling (B904)tests/test_fyle/test_views.py
17-17: Use f-string instead of
formatcall (UP032)Convert to f-string
52-52: Use f-string instead of
formatcall (UP032)Convert to f-string
82-82: Use f-string instead of
formatcall (UP032)Convert to f-string
99-99: Use f-string instead of
formatcall (UP032)Convert to f-string
112-112: Use f-string instead of
formatcall (UP032)Convert to f-string
130-130: Use f-string instead of
formatcall (UP032)Convert to f-string
apps/sage300/models.py
103-103: Use
key in dictinstead ofkey in dict.keys()(SIM118)Remove
.keys()tests/test_sage300/test_helpers.py
33-33: Avoid equality comparisons to
True; useif result:for truth checks (E712)Replace with
result
43-43: Avoid equality comparisons to
False; useif not result:for false checks (E712)Replace with
not result
139-139: Pointless comparison. Did you mean to assign a value? Otherwise, prepend
assertor remove it. (B015)tests/test_sage300/test_dependent_fields.py
79-79: Avoid equality comparisons to
False; useif not is_errored:for false checks (E712)Replace with
not is_erroredapps/sage300/helpers.py
62-62: Use f-string instead of
formatcall (UP032)Convert to f-string
105-108: Use implicit references for positional format fields (UP030)
Remove explicit positional indices
105-108: Use f-string instead of
formatcall (UP032)Convert to f-string
apps/sage300/dependent_fields.py
26-26: Use
existing_attribute.get('placeholder', None)instead of anifblock (SIM401)Replace with
existing_attribute.get('placeholder', None)
35-35: Use implicit references for positional format fields (UP030)
Remove explicit positional indices
35-35: Use f-string instead of
formatcall (UP032)Convert to f-string
125-125: Do not use mutable data structures for argument defaults (B006)
Replace with
None; initialize within functionapps/sage300/utils.py
80-83: Use
detail['type'] = vendor_type_mapping.get(item.type_id, None)instead of anifblock (SIM401)Replace with
detail['type'] = vendor_type_mapping.get(item.type_id, None)apps/mappings/imports/modules/base.py
65-65: Do not use mutable data structures for argument defaults (B006)
Replace with
None; initialize within function
132-132: Use f-string instead of
formatcall (UP032)Convert to f-string
268-268: Remove unnecessary
True if ... else False(SIM210)Remove unnecessary
True if ... else False
307-307: Use f-string instead of
formatcall (UP032)Convert to f-string
tests/conftest.py
135-135: Use f-string instead of
formatcall (UP032)Convert to f-string
136-136: Use f-string instead of
formatcall (UP032)Convert to f-string
Gitleaks
tests/conftest.py
104-104: Detected a Generic API Key, potentially exposing access to various services and sensitive operations. (generic-api-key)
Additional comments not posted (7)
tests/test_mappings/test_imports/test_modules/test_projects.py (1)
6-6: Approved: Comprehensive test function setup.The test function
test_construct_fyle_payloadis well-structured and appears to cover multiple scenarios effectively, including new project creation and updates. Ensure that the data fixtures used (data['create_fyle_project_payload_create_disable_case2']) align with expected outcomes for robust testing.apps/mappings/models.py (1)
38-51: Approved: Efficient use ofupdate_or_createinImportLog.create.The
createmethod in theImportLogclass efficiently ensures that an import log entry exists before updating, which is crucial for tracking import operations. Consider adding detailed docstrings to explain the parameters and expected behavior for better maintainability.apps/mappings/imports/queues.py (1)
63-63: Confirm the addition of the dependent fields import task.The addition of
import_dependent_fields_to_fyletask at the end of the chain seems appropriate. Please ensure that this task is necessary and correctly placed in the workflow.Verification successful
The search results show that
import_dependent_fields_to_fyleis defined inapps/sage300/dependent_fields.pyand used in tests and the chain inapps/mappings/imports/queues.py.To ensure the correct placement and necessity of this task in the workflow, we should review the context of the chain in
apps/mappings/imports/queues.py.
The context provided shows that the
import_dependent_fields_to_fyletask is appended to the chain only if bothproject_mappinganddependent_field_settingsare present. This conditional placement indicates that the task is specifically required when these conditions are met, ensuring that dependent fields are imported correctly when necessary.
apps/mappings/imports/queues.py:
- The task
import_dependent_fields_to_fyleis appended to the chain under the condition that bothproject_mappinganddependent_field_settingsare present.This confirms that the task is correctly placed in the workflow and is necessary under specific conditions.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the placement and necessity of the `import_dependent_fields_to_fyle` task in the chain. # Test: Search for the task usage in other parts of the codebase. Expect: Consistent usage and placement. rg --type python $'import_dependent_fields_to_fyle'Length of output: 81
Script:
#!/bin/bash # Description: Verify the placement and necessity of the `import_dependent_fields_to_fyle` task in the chain. # Test: Search for the task usage in other parts of the codebase. Expect: Consistent usage and placement. rg --type py 'import_dependent_fields_to_fyle'Length of output: 527
Script:
#!/bin/bash # Description: Review the context of the chain in `apps/mappings/imports/queues.py` to ensure the correct placement of the task. # Test: Display the relevant section of `apps/mappings/imports/queues.py` to understand the context of the chain. rg -A 10 -B 10 'chain.append' apps/mappings/imports/queues.pyLength of output: 2430
apps/sage300/models.py (1)
124-126: Ensure Proper Timezone HandlingMake sure that the timezone used here aligns with the application's standard timezone settings. Using
timezone.utcis generally safe, but it's important to maintain consistency across the application.tests/test_sage300/test_helpers.py (1)
113-114: Good Practice: Properly Initializing Model InstancesProper initialization of model instances with all required fields, including
source_idandactive, enhances data integrity and clarity in tests.tests/test_sage300/test_dependent_fields.py (2)
181-183: Consistent Use of ImportLog for TrackingThe consistent use of
ImportLogfor tracking import operations across different test functions is a good practice.
112-124: Ensure Accurate Status ChecksEnsure to use direct boolean checks for better readability and consistency.
- assert cost_category_import_log.status == 'FATAL' + assert not cost_category_import_log.status == 'COMPLETE'Likely invalid or redundant comment.
| job_ids_in_cost_category = CostCategory.objects.filter( | ||
| workspace_id = self.workspace_id, | ||
| job_id__in = [attribute.destination_id for attribute in paginated_destination_attributes] | ||
| ).values_list('job_id', flat=True).distinct() |
There was a problem hiding this comment.
Consider using a set comprehension for performance.
The current list comprehension inside the filter might be less efficient for larger datasets. Using a set comprehension can improve performance since membership tests in sets are faster.
- job_id__in=[attribute.destination_id for attribute in paginated_destination_attributes]
+ job_id__in={attribute.destination_id for attribute in paginated_destination_attributes}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.
| job_ids_in_cost_category = CostCategory.objects.filter( | |
| workspace_id = self.workspace_id, | |
| job_id__in = [attribute.destination_id for attribute in paginated_destination_attributes] | |
| ).values_list('job_id', flat=True).distinct() | |
| job_ids_in_cost_category = CostCategory.objects.filter( | |
| workspace_id = self.workspace_id, | |
| job_id__in = {attribute.destination_id for attribute in paginated_destination_attributes} | |
| ).values_list('job_id', flat=True).distinct() |
| 'description': 'Sage 300 Project - {0}, Id - {1}'.format( | ||
| attribute.value, | ||
| attribute.destination_id | ||
| ), | ||
| 'is_enabled': True if attribute.active is None else attribute.active | ||
| } |
There was a problem hiding this comment.
Optimize string formatting using f-strings.
Using f-strings can make the code cleaner and potentially faster.
- 'description': 'Sage 300 Project - {0}, Id - {1}'.format(attribute.value, attribute.destination_id)
+ 'description': f'Sage 300 Project - {attribute.value}, Id - {attribute.destination_id}'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.
| 'description': 'Sage 300 Project - {0}, Id - {1}'.format( | |
| attribute.value, | |
| attribute.destination_id | |
| ), | |
| 'is_enabled': True if attribute.active is None else attribute.active | |
| } | |
| 'description': f'Sage 300 Project - {attribute.value}, Id - {attribute.destination_id}', | |
| 'is_enabled': True if attribute.active is None else attribute.active |
Tools
Ruff
53-56: Use implicit references for positional format fields (UP030)
Remove explicit positional indices
53-56: Use f-string instead of
formatcall (UP032)Convert to f-string
| cost_code_import_log = ImportLog.create('COST_CODE', workspace_id) | ||
|
|
||
| result = post_dependent_cost_code( | ||
| result, is_errored = post_dependent_cost_code( | ||
| cost_code_import_log, | ||
| dependent_field_setting=dependent_field_settings, | ||
| platform=platform.return_value, | ||
| filters=filters | ||
| ) | ||
|
|
||
| assert result == ['Direct Mail Campaign', 'Platform APIs'] | ||
| assert cost_code_import_log.status == 'COMPLETE' | ||
| assert is_errored == False |
There was a problem hiding this comment.
Ensure Accurate Status Checks
It's good to see comprehensive status checks. However, consider using direct boolean checks instead of comparing against False.
- assert is_errored == False
+ assert not is_erroredCommittable 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.
| cost_code_import_log = ImportLog.create('COST_CODE', workspace_id) | |
| result = post_dependent_cost_code( | |
| result, is_errored = post_dependent_cost_code( | |
| cost_code_import_log, | |
| dependent_field_setting=dependent_field_settings, | |
| platform=platform.return_value, | |
| filters=filters | |
| ) | |
| assert result == ['Direct Mail Campaign', 'Platform APIs'] | |
| assert cost_code_import_log.status == 'COMPLETE' | |
| assert is_errored == False | |
| cost_code_import_log = ImportLog.create('COST_CODE', workspace_id) | |
| result, is_errored = post_dependent_cost_code( | |
| cost_code_import_log, | |
| dependent_field_setting=dependent_field_settings, | |
| platform=platform.return_value, | |
| filters=filters | |
| ) | |
| assert result == ['Direct Mail Campaign', 'Platform APIs'] | |
| assert cost_code_import_log.status == 'COMPLETE' | |
| assert not is_errored |
Tools
Ruff
79-79: Avoid equality comparisons to
False; useif not is_errored:for false checks (E712)Replace with
not is_errored
| import_log.status = 'PARTIALLY_FAILED' if is_errored else 'COMPLETE' | ||
| import_log.error_log = [] | ||
| import_log.processed_batches_count = processed_batches | ||
| import_log.save() |
There was a problem hiding this comment.
Ensure consistent status updates in the import log.
Consider setting the import log status to 'PARTIALLY_FAILED' before the loop if any preconditions fail, to ensure the log accurately reflects the status throughout the function execution.
+ if <some_precondition>:
+ import_log.status = 'PARTIALLY_FAILED'
+ import_log.save()Committable suggestion was skipped due to low confidence.
| import_log.status = 'PARTIALLY_FAILED' if is_errored else 'COMPLETE' | ||
| import_log.error_log = [] | ||
| import_log.processed_batches_count = processed_batches | ||
| import_log.save() |
There was a problem hiding this comment.
Optimize error handling and import log updates.
To improve error handling, consider adding more detailed error logs and ensure that the import log is updated consistently.
+ if is_errored:
+ import_log.error_log.append({'message': 'Error details here'})
+ import_log.save()Committable suggestion was skipped due to low confidence.
| 'COST_CODE': lambda:sage300_connection.sync_cost_codes(import_log), | ||
| 'COST_CATEGORY': lambda:sage300_connection.sync_cost_categories(import_log), |
There was a problem hiding this comment.
Tip
Codebase Verification
The functions sync_cost_codes and sync_cost_categories in apps/sage300/utils.py include the import_log parameter, but it is not being utilized within their implementations. This confirms the concern that the import_log parameter, while added to the function signatures, is not actively used in the function bodies.
sync_cost_codesfunction: Defined with_import_log = None, but_import_logis not used inside the function.sync_cost_categoriesfunction: Defined withimport_log = None, butimport_logis not used inside the function.
To address this, ensure that the import_log parameter is utilized within these functions to track and log import operations appropriately.
Locations needing attention:
apps/sage300/utils.py: sync_cost_codesapps/sage300/utils.py: sync_cost_categories
Analysis chain
Approved: Proper use of import_log in lambda functions.
The addition of import_log to the lambda functions for syncing cost codes and categories is a significant improvement for error tracking and operational logging. Please ensure that the sync_cost_codes and sync_cost_categories functions are implemented to handle this parameter correctly.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the correct usage of `import_log` in the sync functions.
# Test: Search for the function implementations. Expect: Proper handling of `import_log`.
ast-grep --lang python --pattern $'def sync_cost_codes($_, $_, $_) { $$$ }' apps/sage300/utils.py
ast-grep --lang python --pattern $'def sync_cost_categories($_, $_, $_) { $$$ }' apps/sage300/utils.py
Length of output: 1131
Script:
#!/bin/bash
# Description: Verify the correct usage of `import_log` in the sync functions.
# Corrected Test: Search for the function implementations. Expect: Proper handling of `import_log`.
ast-grep --lang python --pattern 'def sync_cost_codes(import_log, $$$):'
ast-grep --lang python --pattern 'def sync_cost_categories(import_log, $$$):'
Length of output: 154
Script:
#!/bin/bash
# Description: Verify the correct usage of `import_log` in the sync functions across the entire codebase.
# Search for the function definitions of sync_cost_codes and sync_cost_categories.
rg 'def sync_cost_codes'
rg 'def sync_cost_categories'
Length of output: 208
Script:
#!/bin/bash
# Description: Extract the full implementations of the sync functions to verify the correct usage of `import_log`.
# Extract the implementation of sync_cost_codes.
ast-grep --lang python --pattern 'def sync_cost_codes(self, _import_log = None): { $$$ }' apps/sage300/utils.py
# Extract the implementation of sync_cost_categories.
ast-grep --lang python --pattern 'def sync_cost_categories(self, import_log = None): { $$$ }' apps/sage300/utils.py
Length of output: 231
Script:
#!/bin/bash
# Description: Extract the full content of apps/sage300/utils.py for manual inspection.
# Extract the full content of the file.
rg '' apps/sage300/utils.py
Length of output: 12752
| @@ -17,8 +17,12 @@ | |||
|
|
|||
|
|
|||
| def handle_import_exceptions(func): | |||
| def new_fn(expense_attribute_instance, *args): | |||
| import_log: ImportLog = args[0] | |||
| def new_fn(expense_attribute_instance, *args, **kwargs): | |||
There was a problem hiding this comment.
Refactor: Use f-strings for better readability.
The handle_import_exceptions function is well-structured for handling various exceptions. However, using f-strings can enhance readability and maintainability. Here's a suggested refactor:
- 'Import {0} to Fyle and Auto Create Mappings'.format(attribute_type)
+ f'Import {attribute_type} to Fyle and Auto Create Mappings'
- 'Invalid Token or Sage 300 credentials does not exist workspace_id - {0}'.format(workspace_id)
+ f'Invalid Token or Sage 300 credentials does not exist workspace_id - {workspace_id}'Also applies to: 29-29, 43-43
| @@ -39,6 +39,7 @@ def create(self, validated_data): | |||
| platform = PlatformConnector(fyle_credentials) | |||
|
|
|||
| if refresh: | |||
| chain_import_fields_to_fyle(workspace_id=workspace_id) | |||
There was a problem hiding this comment.
Use proper exception chaining to clarify exception sources.
When re-raising exceptions, it's recommended to use from to provide more context and avoid masking other exceptions.
- raise APIException("Internal Server Error", code='server_error')
+ raise APIException("Internal Server Error", code='server_error') from exceptionCommittable suggestion was skipped due to low confidence.
| @@ -299,6 +304,8 @@ def post_to_fyle_and_sync(self, fyle_payload: List[object], resource_class, is_l | |||
| :param is_last_batch: bool | |||
| :param import_log: ImportLog object | |||
| """ | |||
| logger.info("| Importing {} to Fyle | Content: {{WORKSPACE_ID: {} Fyle Payload count: {} is_last_batch: {}}}".format(self.destination_field, self.workspace_id, len(fyle_payload), is_last_batch)) | |||
There was a problem hiding this comment.
Optimize logging with f-strings.
Convert the logging statement to use f-strings for better performance and readability.
- logger.info("| Importing {} to Fyle | Content: {{WORKSPACE_ID: {} Fyle Payload count: {} is_last_batch: {}}}".format(self.destination_field, self.workspace_id, len(fyle_payload), is_last_batch))
+ logger.info(f"| Importing {self.destination_field} to Fyle | Content: {{WORKSPACE_ID: {self.workspace_id} Fyle Payload count: {len(fyle_payload)} is_last_batch: {is_last_batch}}}")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.
| logger.info("| Importing {} to Fyle | Content: {{WORKSPACE_ID: {} Fyle Payload count: {} is_last_batch: {}}}".format(self.destination_field, self.workspace_id, len(fyle_payload), is_last_batch)) | |
| logger.info(f"| Importing {self.destination_field} to Fyle | Content: {{WORKSPACE_ID: {self.workspace_id} Fyle Payload count: {len(fyle_payload)} is_last_batch: {is_last_batch}}}") |
Tools
Ruff
307-307: Use f-string instead of
formatcall (UP032)Convert to f-string
| @@ -10,6 +10,7 @@ | |||
|
|
|||
| def test_import_fyle_attributes(mocker, api_client, test_connection, create_temp_workspace, add_fyle_credentials): | |||
| mocker.patch('fyle_integrations_platform_connector.fyle_integrations_platform_connector.PlatformConnector.import_fyle_dimensions', return_value=[]) | |||
| mocker.patch('apps.fyle.serializers.chain_import_fields_to_fyle', return_value=None) | |||
There was a problem hiding this comment.
Enhance test coverage for chain_import_fields_to_fyle.
Consider adding more detailed assertions to verify the behavior of chain_import_fields_to_fyle beyond just checking the response status.
* Delete purchase_invoice, line_items on failed exports from hh2, type-check for current_state * Refactor deps schedule to run post Job import * Fyle Card <> Vendor Mapping setup * Added script to add mapping_settings * Fix post release script * Projects and Deps fields disable v1 * Remove dep setting trigger, add logger * Modified script, added additional test case * lint fix * Remove mock object * Add details while logging * modify post-release script * bump accounting-mapping version * modify the variable_name, add conditional update * Add example objects * Added loggers * Added test cases * Dependent Field optimizations * fix failing test * Added Import Log for Deps * Fix post-release script * fix failing test cases * Set cost category import to fail on cost code fail * Modify handle_import_exception for both class and func * Modify test cases * fix ordering of saving import_log * Move the import_log creation to ImportLog method * Add logger in import * Refactor the job sync v1 * Fix few sync issues * Remove extra loggers * fix lint * Refactor Job sync v2 * Add batch count * Fixed the import_log related comments * modify import logs save * Remove logger
Summary by CodeRabbit
New Features
Bug Fixes
Dependencies
fyle-accounting-mappingsto version 1.33.1.fyle-integrations-platform-connectorto version 1.37.4.Tests