Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix disable attribute #222

Merged
merged 12 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions apps/mappings/imports/modules/categories.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ def create_mappings(self):
)


def disable_categories(workspace_id: int, categories_to_disable: Dict, *args, **kwargs):
def disable_categories(workspace_id: int, categories_to_disable: Dict, is_import_to_fyle_enabled: bool = False, *args, **kwargs):
"""
categories_to_disable object format:
{
Expand All @@ -102,13 +102,22 @@ def disable_categories(workspace_id: int, categories_to_disable: Dict, *args, **
}
}
"""
if not is_import_to_fyle_enabled or len(categories_to_disable) == 0:
logger.info("Skipping disabling categories in Fyle | WORKSPACE_ID: %s", workspace_id)
return

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

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

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

Choose a reason for hiding this comment

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

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']):
+            continue
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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 if branches using logical or operator

Combine if branches

(SIM114)


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

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

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

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

Expand Down
15 changes: 12 additions & 3 deletions apps/mappings/imports/modules/cost_centers.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def construct_fyle_payload(
return payload


def disable_cost_centers(workspace_id: int, cost_centers_to_disable: Dict, *args, **kwargs):
def disable_cost_centers(workspace_id: int, cost_centers_to_disable: Dict, is_import_to_fyle_enabled: bool = False, *args, **kwargs):
"""
cost_centers_to_disable object format:
{
Expand All @@ -77,6 +77,10 @@ def disable_cost_centers(workspace_id: int, cost_centers_to_disable: Dict, *args
}
}
"""
if not is_import_to_fyle_enabled or len(cost_centers_to_disable) == 0:
logger.info("Skipping disabling cost centers in Fyle | WORKSPACE_ID: %s", workspace_id)
return

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

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

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

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

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

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

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

Expand Down
11 changes: 10 additions & 1 deletion apps/mappings/imports/modules/merchants.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ def import_destination_attribute_to_fyle(self, import_log: ImportLog):
self.sync_expense_attributes(platform)


def disable_merchants(workspace_id: int, merchants_to_disable: Dict, *args, **kwargs):
def disable_merchants(workspace_id: int, merchants_to_disable: Dict, is_import_to_fyle_enabled: bool = False, *args, **kwargs):
"""
merchants_to_disable object format:
{
Expand All @@ -87,12 +87,21 @@ def disable_merchants(workspace_id: int, merchants_to_disable: Dict, *args, **kw
}
}
"""
if not is_import_to_fyle_enabled or len(merchants_to_disable) == 0:
logger.info("Skipping disabling merchants in Fyle | WORKSPACE_ID: %s", workspace_id)
return

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

merchant_values = []
for merchant_map in merchants_to_disable.values():
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

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

Expand Down
20 changes: 15 additions & 5 deletions apps/mappings/imports/modules/projects.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def construct_fyle_payload(
return payload


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

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

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

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

Choose a reason for hiding this comment

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

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']):
+            continue
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
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 if branches using logical or operator

Combine if branches

(SIM114)


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

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

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

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

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

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

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

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

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

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

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

if not DestinationAttribute.objects.filter(workspace_id=self.workspace_id, attribute_type='VENDOR_TYPE').exists():
vendor_types = self.connection.vendors.get_vendor_types()
self._sync_data(vendor_types, 'VENDOR_TYPE', 'vendor_type', self.workspace_id, ['version'])

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

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

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

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

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

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

return source_type

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

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

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

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

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

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

Choose a reason for hiding this comment

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

Review of is_imported_enabled method

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

However, the static analysis tool suggests a potential improvement:

  • Combine similar if branches to simplify the conditional logic.

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

- if attribute_type == 'ACCOUNT' and import_settings.import_categories:
-     is_import_to_fyle_enabled = True
- elif attribute_type == 'VENDOR' and import_settings.import_vendors_as_merchants:
-     is_import_to_fyle_enabled = True
+ if (attribute_type == 'ACCOUNT' and import_settings.import_categories) or \
+    (attribute_type == 'VENDOR' and import_settings.import_vendors_as_merchants):
+    is_import_to_fyle_enabled = True
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

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

342-346: Combine if branches using logical or operator

Combine if branches

(SIM114)

2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ fyle==0.37.0

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


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ def test_disable_categories(
mock_platform = mocker.patch('apps.mappings.imports.modules.categories.PlatformConnector')
bulk_post_call = mocker.patch.object(mock_platform.return_value.categories, 'post_bulk')

disable_categories(workspace_id, categories_to_disable)
disable_categories(workspace_id, categories_to_disable, is_import_to_fyle_enabled=True)

assert bulk_post_call.call_count == 1

Expand All @@ -190,7 +190,7 @@ def test_disable_categories(
}
}

disable_categories(workspace_id, categories_to_disable)
disable_categories(workspace_id, categories_to_disable, is_import_to_fyle_enabled=True)
assert bulk_post_call.call_count == 1

# Test disable projects with code in naming
Expand Down Expand Up @@ -223,5 +223,5 @@ def test_disable_categories(
'id': 'source_id_123'
}]

bulk_payload = disable_categories(workspace_id, categories_to_disable)
bulk_payload = disable_categories(workspace_id, categories_to_disable, is_import_to_fyle_enabled=True)
assert bulk_payload == payload
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ def test_disable_cost_centers(
mock_platform = mocker.patch('apps.mappings.imports.modules.cost_centers.PlatformConnector')
bulk_post_call = mocker.patch.object(mock_platform.return_value.cost_centers, 'post_bulk')

disable_cost_centers(workspace_id, cost_centers_to_disable)
disable_cost_centers(workspace_id, cost_centers_to_disable, is_import_to_fyle_enabled=True)

assert bulk_post_call.call_count == 1

Expand All @@ -131,7 +131,7 @@ def test_disable_cost_centers(
}
}

disable_cost_centers(workspace_id, cost_centers_to_disable)
disable_cost_centers(workspace_id, cost_centers_to_disable, is_import_to_fyle_enabled=True)
assert bulk_post_call.call_count == 1

# Test disable projects with code in naming
Expand Down Expand Up @@ -167,5 +167,5 @@ def test_disable_cost_centers(
}
]

bulk_payload = disable_cost_centers(workspace_id, cost_centers_to_disable)
bulk_payload = disable_cost_centers(workspace_id, cost_centers_to_disable, is_import_to_fyle_enabled=True)
assert bulk_payload == payload
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ def test_disable_merchants(
mock_platform = mocker.patch('apps.mappings.imports.modules.merchants.PlatformConnector')
bulk_post_call = mocker.patch.object(mock_platform.return_value.merchants, 'post')

disable_merchants(workspace_id, merchants_to_disable)
disable_merchants(workspace_id, merchants_to_disable, is_import_to_fyle_enabled=True)

assert bulk_post_call.call_count == 1

Expand All @@ -168,7 +168,7 @@ def test_disable_merchants(
}
}

disable_merchants(workspace_id, merchants_to_disable)
disable_merchants(workspace_id, merchants_to_disable, is_import_to_fyle_enabled=True)
assert bulk_post_call.call_count == 1

# Test disable projects with code in naming
Expand Down Expand Up @@ -196,5 +196,5 @@ def test_disable_merchants(

payload = ['old_merchant_code: old_merchant']

bulk_payload = disable_merchants(workspace_id, merchants_to_disable)
bulk_payload = disable_merchants(workspace_id, merchants_to_disable, is_import_to_fyle_enabled=True)
assert bulk_payload[0] == payload[0]
10 changes: 5 additions & 5 deletions tests/test_sage300/test_helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ def test_disable_projects(

disable_cost_code_call = mocker.patch('apps.sage300.dependent_fields.update_and_disable_cost_code')

disable_projects(workspace_id, projects_to_disable)
disable_projects(workspace_id, projects_to_disable, is_import_to_fyle_enabled=True)

assert bulk_post_call.call_count == 1
assert sync_call.call_count == 2
Expand All @@ -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)
Copy link

Choose a reason for hiding this comment

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

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_count being 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 == 1

Also applies to: 144-145

assert bulk_post_call.call_count == 1
assert sync_call.call_count == 4
disable_cost_code_call.call_count == 2
assert sync_call.call_count == 3
disable_cost_code_call.call_count == 1

# Test disable projects with code in naming
import_settings = ImportSetting.objects.get(workspace_id=workspace_id)
Expand Down Expand Up @@ -178,7 +178,7 @@ def test_disable_projects(
'id': 'source_id_123'
}]

assert disable_projects(workspace_id, projects_to_disable) == payload
assert disable_projects(workspace_id, projects_to_disable, is_import_to_fyle_enabled=True) == payload


def test_update_and_disable_cost_code(
Expand Down
Loading