fix disable attribute #222
Conversation
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Warning Rate limit exceeded@Hrishabh17 has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 15 minutes and 6 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe changes involve a simplification of the Changes
Possibly related PRs
Poem
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Outside diff range, codebase verification and nitpick comments (3)
apps/mappings/imports/modules/merchants.py (2)
Line range hint
78-104: Approve the addition ofis_import_to_fyle_enabledand early exit logic.The addition of the
is_import_to_fyle_enabledparameter and the early exit logic based on this parameter and the length ofmerchants_to_disableare good practices that enhance the function's efficiency and control.Refactor suggestion for conditional checks.
The static analysis tool suggests combining the
ifbranches using the logicaloroperator to simplify the code.Apply this diff to refactor the conditional checks:
- if not use_code_in_naming and merchant_map['value'] == merchant_map['updated_value']: - continue - elif use_code_in_naming and (merchant_map['value'] == merchant_map['updated_value'] and merchant_map['code'] == merchant_map['update_code']): - continue + if (not use_code_in_naming and merchant_map['value'] == merchant_map['updated_value']) or (use_code_in_naming and merchant_map['value'] == merchant_map['updated_value'] and merchant_map['code'] == merchant_map['update_code']): + continueThis change simplifies the logic by combining the conditions into a single line, making it easier to read and maintain.
Tools
Ruff
100-103: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
78-78: Verify the impact of the removed parameter inconstruct_fyle_payload.The removal of
is_auto_sync_status_allowedfromconstruct_fyle_payloadaffects multiple test cases and potentially other logic in the codebase. Ensure that all tests and dependent logic are updated to accommodate this change.
- Check the following test files for necessary updates:
tests/test_mappings/test_imports/test_modules/test_cost_centers.pytests/test_mappings/test_imports/test_modules/test_merchants.pytests/test_mappings/test_imports/test_modules/test_categories.pytests/test_mappings/test_imports/test_modules/test_projects.pyAnalysis chain
Verify the impact of the removed parameter in
construct_fyle_payload.The removal of
is_auto_sync_status_allowedfromconstruct_fyle_payloadmight affect other parts of the codebase where this method is used.Run the following script to verify the usage of
construct_fyle_payload:Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of `construct_fyle_payload` and check if the removed parameter affects other parts of the codebase. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'construct_fyle_payload'Length of output: 77
Script:
#!/bin/bash # Description: Verify the usage of `construct_fyle_payload` and check if the removed parameter affects other parts of the codebase. # Test: Search for the method usage. Expect: Only occurrences of the new signature. rg --type py -A 5 $'construct_fyle_payload'Length of output: 16056
apps/sage300/utils.py (1)
Line range hint
134-163: Review of_sync_datamethod changesThe addition of the
is_import_to_fyle_enabledparameter is well-integrated into the method. This allows for conditional processing of data synchronization based on the import settings, aligning with the PR objectives.However, consider improving the readability of the conditional checks by using a more concise approach for checking if
source_typeis inATTRIBUTE_CALLBACK_MAP.Consider using the
getmethod with a default value to simplify the conditional logic:- if source_type in ATTRIBUTE_CALLBACK_MAP.keys(): + if ATTRIBUTE_CALLBACK_MAP.get(source_type):
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (10)
- apps/mappings/imports/modules/categories.py (3 hunks)
- apps/mappings/imports/modules/cost_centers.py (4 hunks)
- apps/mappings/imports/modules/merchants.py (2 hunks)
- apps/mappings/imports/modules/projects.py (5 hunks)
- apps/sage300/utils.py (7 hunks)
- requirements.txt (1 hunks)
- tests/test_mappings/test_imports/test_modules/test_categories.py (3 hunks)
- tests/test_mappings/test_imports/test_modules/test_cost_centers.py (3 hunks)
- tests/test_mappings/test_imports/test_modules/test_merchants.py (3 hunks)
- tests/test_sage300/test_helpers.py (3 hunks)
Files skipped from review due to trivial changes (1)
- requirements.txt
Additional context used
Ruff
apps/mappings/imports/modules/merchants.py
100-103: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
apps/mappings/imports/modules/cost_centers.py
92-95: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
apps/mappings/imports/modules/categories.py
116-119: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
apps/mappings/imports/modules/projects.py
110-113: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
tests/test_sage300/test_helpers.py
145-145: Pointless comparison. Did you mean to assign a value? Otherwise, prepend
assertor remove it.(B015)
apps/sage300/utils.py
342-346: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
Additional comments not posted (12)
apps/mappings/imports/modules/cost_centers.py (1)
Line range hint
68-96: Refactor conditional logic and verify new parameter integration.The addition of the
is_import_to_fyle_enabledparameter and the new conditional logic in thedisable_cost_centersfunction are significant changes. Here are some observations and suggestions:
Parameter Integration:
- The new parameter
is_import_to_fyle_enabledis used to conditionally skip the disabling of cost centers. This is a sensible addition for enhancing control based on external configurations. Ensure that all calls to this function across the codebase are updated to pass this new parameter where necessary.- **** Verify the integration of the new parameter across the codebase:
Conditional Logic Refinement:
- The logic to skip processing based on
use_code_in_namingand the values ofvalue,updated_value,code, andupdate_codeis crucial for avoiding unnecessary operations. However, the static analysis tool suggests combining theifbranches for clarity and efficiency.- **** Refactor the conditional checks to use a single
ifstatement with logical operators, which will simplify the code and potentially improve readability:-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['update_code']): - continue +if (not use_code_in_naming and cost_center_map['value'] == cost_center_map['updated_value']) or \ + (use_code_in_naming and cost_center_map['value'] == cost_center_map['updated_value'] and cost_center_map['code'] == cost_center_map['update_code']): + continueLogging and Error Handling:
- The function logs various states, which is good for debugging and monitoring. However, consider adding more detailed error handling or recovery mechanisms if the disabling process encounters issues beyond just logging.
Overall, these changes enhance the function's flexibility and efficiency but require careful integration and testing to ensure they work as intended in the broader system.
Also applies to: 108-110
apps/mappings/imports/modules/categories.py (2)
93-93: Addition of new optional parameteris_import_to_fyle_enabled.The addition of the
is_import_to_fyle_enabledparameter allows for conditional processing based on import settings. This is a positive change as it adds flexibility to the function without breaking existing functionality due to its default value ofFalse.
105-107: Efficient early return logic.The early return based on
is_import_to_fyle_enabledand the emptiness ofcategories_to_disableenhances efficiency by preventing unnecessary processing. This is a good use of short-circuiting in conditional checks.tests/test_mappings/test_imports/test_modules/test_cost_centers.py (1)
121-121: Verify integration and suggest additional test cases for the new parameter.The new parameter
is_import_to_fyle_enabledis correctly integrated into thedisable_cost_centersfunction calls. However, the test only covers scenarios where this parameter is set toTrue. Consider adding additional test cases to cover scenarios whereis_import_to_fyle_enabledisFalseor not provided, to ensure comprehensive test coverage.Would you like me to help draft these additional test cases?
Also applies to: 134-134, 170-170
apps/mappings/imports/modules/projects.py (4)
80-80: Parameter Addition:is_import_to_fyle_enabledThe addition of the
is_import_to_fyle_enabledparameter is well implemented with a default value ofFalse. This allows existing calls to the function to remain functional without modification, adhering to backward compatibility.
95-97: Early Exit OptimizationThe early exit when
is_import_to_fyle_enabledisFalseorprojects_to_disableis empty is a good practice. It prevents unnecessary processing and API calls, enhancing performance and reducing costs.
127-129: Code Clarity ImprovementThe loop that iterates over
projects_to_disableand maps project names to their destination IDs is clear and improves readability by using meaningful variable names.
154-155: Post-Processing Sync CallThe call to
platform.projects.sync()after updating and disabling cost codes ensures that the state is consistent across the platform. This is a good practice in ensuring data integrity.tests/test_mappings/test_imports/test_modules/test_merchants.py (1)
158-158: Approve the addition of the new parameter in test cases.The addition of
is_import_to_fyle_enabled=Truein thedisable_merchantsfunction calls within the test cases is consistent with the PR objectives and enhances the function's flexibility. The test assertions are appropriately checking the function's behavior with the new parameter.Consider verifying the integration of the new parameter across other test cases or functions if not covered. This ensures that the new parameter's addition is consistently handled throughout the application.
Also applies to: 171-171, 199-199
tests/test_mappings/test_imports/test_modules/test_categories.py (1)
180-180: Verify the correct usage and impact of the new parameteris_import_to_fyle_enabled.The addition of the
is_import_to_fyle_enabledparameter to thedisable_categoriesfunction calls needs to be verified for correctness and consistency. Ensure that the new parameter is being used as intended within the function and that it does not alter the expected behavior of the function in unintended ways.Run the following script to verify the function usage:
Also applies to: 193-193, 226-226
Verification successful
The new parameter
is_import_to_fyle_enabledis correctly used in test cases.The
disable_categoriesfunction is invoked with theis_import_to_fyle_enabledparameter set toTruein the test filetests/test_mappings/test_imports/test_modules/test_categories.py. This aligns with the function definition, which includes this parameter with a default value ofFalse. The test cases are consistent with the intended usage of the parameter, and no unintended behavior changes are evident.
tests/test_mappings/test_imports/test_modules/test_categories.py: Lines 180, 193, 226Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `disable_categories` match the new signature and check for unintended behavior changes. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type python -A 5 $'disable_categories'Length of output: 73
Script:
#!/bin/bash # Description: Verify all function calls to `disable_categories` match the new signature and check for unintended behavior changes. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg --type py -A 5 $'disable_categories'Length of output: 3747
tests/test_sage300/test_helpers.py (2)
127-127: Review the function call with the new parameter.The addition of the
is_import_to_fyle_enabled=Trueparameter to thedisable_projectsfunction call aligns with the PR objectives. Ensure that all dependent code and tests are updated to handle this new parameter.
181-181: Verify the return value assertion.The assertion checks if the
disable_projectsfunction returns the expected payload. This is crucial for ensuring that the function behaves as expected with the new parameter.Consider adding more comprehensive tests to cover various scenarios where
is_import_to_fyle_enabledmight affect the output differently.
| 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['update_code']): | ||
| continue |
There was a problem hiding this comment.
Refinement of loop conditions.
The loop conditions have been refined to ensure that categories are only processed if their values have changed. This prevents unnecessary updates and enhances performance. However, the static analysis tool suggests combining these conditions using a logical or operator to simplify the code.
Consider refactoring the conditions to use a single if statement with a logical or, which can make the code cleaner and potentially easier to maintain:
- 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['update_code']):
- continue
+ if (not use_code_in_naming and category_map['value'] == category_map['updated_value']) or (use_code_in_naming and category_map['value'] == category_map['updated_value'] and category_map['code'] == category_map['update_code']):
+ continueCommittable 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.
| 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['update_code']): | |
| continue | |
| if (not use_code_in_naming and category_map['value'] == category_map['updated_value']) or (use_code_in_naming and category_map['value'] == category_map['updated_value'] and category_map['code'] == category_map['update_code']): | |
| continue |
Tools
Ruff
116-119: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
| 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['update_code']): | ||
| continue |
There was a problem hiding this comment.
Refactor Suggestion: Combine Conditional Checks
The separate conditional checks for use_code_in_naming can be combined to simplify the code and reduce complexity.
Consider refactoring the conditionals to use a single line with a logical OR operator, as suggested by the static analysis tool:
- 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['update_code']):
- continue
+ if (not use_code_in_naming and projects_map['value'] == projects_map['updated_value']) or (use_code_in_naming and projects_map['value'] == projects_map['updated_value'] and projects_map['code'] == projects_map['update_code']):
+ continueCommittable 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.
| 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['update_code']): | |
| continue | |
| if (not use_code_in_naming and projects_map['value'] == projects_map['updated_value']) or (use_code_in_naming and projects_map['value'] == projects_map['updated_value'] and projects_map['code'] == projects_map['update_code']): | |
| continue |
Tools
Ruff
110-113: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
| @@ -139,10 +139,10 @@ def test_disable_projects( | |||
| } | |||
| } | |||
|
|
|||
| disable_projects(workspace_id, projects_to_disable) | |||
| disable_projects(workspace_id, projects_to_disable, is_import_to_fyle_enabled=True) | |||
There was a problem hiding this comment.
Check the assertions for correctness.
The assertions following the disable_projects function calls have been updated. Ensure that these changes correctly reflect the expected behavior based on the new parameter's influence.
- Line 144: The assertion for
sync_call.call_countbeing 3 instead of 4 suggests a change in the internal logic when the new parameter is utilized. - Line 145: The static analysis tool flagged a potential issue with the comparison. It appears to be a typo or an incomplete statement. This should be an assertion.
Consider correcting the assertion on line 145:
- disable_cost_code_call.call_count == 1
+ assert disable_cost_code_call.call_count == 1Also applies to: 144-145
| 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 |
There was a problem hiding this comment.
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
ifbranches 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 = TrueCommittable 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.
| 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
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
apps/mappings/imports/modules/projects.py (1)
Line range hint
80-158: Functiondisable_projectsReview:The addition of the new parameter
is_import_to_fyle_enabledenhances the control flow by allowing the function to skip disabling projects when not necessary. This change is well-integrated into the existing logic and improves the function's efficiency and adaptability.Regarding the conditional checks at lines 110-113, the static analysis tool suggests combining them using a logical
oroperator to simplify the code. This suggestion is valid and would reduce complexity.Consider refactoring the conditionals to use a single line with a logical OR operator, as suggested by the static analysis tool:
- 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['update_code']): - continue + if (not use_code_in_naming and projects_map['value'] == projects_map['updated_value']) or (use_code_in_naming and projects_map['value'] == projects_map['updated_value'] and projects_map['code'] == projects_map['update_code']): + continueTools
Ruff
110-113: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/mappings/imports/modules/projects.py (5 hunks)
Additional context used
Ruff
apps/mappings/imports/modules/projects.py
110-113: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
Additional comments not posted (1)
apps/mappings/imports/modules/projects.py (1)
Line range hint
11-78: ClassProjectReview:The class
Projectis well-structured and handles project-related data synchronization effectively. The methods are clear and use descriptive variable names.
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
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
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
|
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
apps/sage300/utils.py (1)
330-353: Approve the addition ofis_imported_enabledmethod.The
is_imported_enabledmethod 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
ifbranches 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 = TrueTools
Ruff
342-346: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/sage300/utils.py (7 hunks)
Additional context used
Ruff
apps/sage300/utils.py
342-346: Combine
ifbranches using logicaloroperatorCombine
ifbranches(SIM114)
Additional comments not posted (1)
apps/sage300/utils.py (1)
Line range hint
134-177: LGTM!The addition of the
is_import_to_fyle_enabledparameter enhances the flexibility and adaptability of the_sync_datafunction. By integrating import settings into the synchronization logic, the function can now dynamically adapt its behavior based on the provided flag. This change has a positive impact on the overall synchronization process.
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
* fix disable attribute * typo fix * update accounting mappings * fix return type * update edit expenses condition * fix test cases * fix typo * bump platform connector version * loggers for test * add logger * Revert "loggers for test" This reverts commit 7641ef2. * Revert "add logger" This reverts commit 4ccd251.
Summary by CodeRabbit
New Features
is_import_to_fyle_enabledacross multiple functions, enhancing control over disabling categories, cost centers, merchants, and projects based on import settings.Bug Fixes
Enhancements
update_non_exported_expensesfunction for better performance and clarity.requirements.txtfor improved functionality and compatibility.