Skip to content

feat: add sync_inactive_employee for missing employee attributes #922

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.

Sign up for GitHub

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

Open
ruuushhh wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: add sync_inactive_employee for missing employee attributes #922

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
144 changes: 96 additions & 48 deletions apps/sage_intacct/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,74 +3,69 @@
import traceback
from datetime import datetime

from dateutil.relativedelta import relativedelta
from django.conf import settings
from django.db import transaction
from django.utils import timezone
from django.core.cache import cache
from django.db import transaction
from django.db.models.functions import Lower
from dateutil.relativedelta import relativedelta

from django.utils import timezone
from fyle.platform.exceptions import InternalServerError as FyleInternalServerError
from fyle.platform.exceptions import InvalidTokenError as FyleInvalidTokenError
from fyle_accounting_library.system_comments.models import SystemComment
from fyle_accounting_mappings.models import CategoryMapping, DestinationAttribute, EmployeeMapping, ExpenseAttribute, Mapping
from fyle_integrations_platform_connector import PlatformConnector
from intacctsdk.exceptions import BadRequestError as IntacctRESTBadRequestError
from fyle.platform.exceptions import InvalidTokenError as FyleInvalidTokenError
from intacctsdk.exceptions import InvalidTokenError as IntacctRESTInvalidTokenError
from intacctsdk.exceptions import InternalServerError as IntacctRESTInternalServerError
from intacctsdk.exceptions import InvalidTokenError as IntacctRESTInvalidTokenError
from sageintacctsdk.exceptions import InvalidTokenError, NoPrivilegeError, WrongParamsError
from fyle_accounting_mappings.models import (
Mapping,
CategoryMapping,
EmployeeMapping,
ExpenseAttribute,
DestinationAttribute
)

from apps.tasks.models import Error, TaskLog
from apps.mappings.models import GeneralMapping
from fyle_intacct_api.exceptions import BulkError
from apps.exceptions import ValueErrorWithResponse
from apps.fyle.models import Expense, ExpenseGroup
from apps.sage_intacct.utils import SageIntacctConnector
from apps.workspaces.enums import ExportTypeEnum, SystemCommentEntityTypeEnum, SystemCommentIntentEnum, SystemCommentReasonEnum, SystemCommentSourceEnum
from apps.workspaces.system_comments import add_system_comment, create_filtered_system_comments
from apps.sage_intacct.actions import update_last_export_details
from apps.sage_intacct.connector import SageIntacctRestConnector
from apps.sage_intacct.helpers import get_sage_intacct_connection
from apps.sage_intacct.enums import SageIntacctRestConnectionTypeEnum
from fyle_intacct_api.utils import invalidate_sage_intacct_credentials
from fyle_accounting_library.system_comments.models import SystemComment
from fyle_intacct_api.logging_middleware import get_caller_info, get_logger
from apps.workspaces.models import (
Configuration,
FeatureConfig,
FyleCredential,
SageIntacctCredential
)
from apps.fyle.actions import (
update_failed_expenses,
post_accounting_export_summary,
update_complete_expenses,
update_expenses_in_progress,
post_accounting_export_summary
update_failed_expenses,
)
from apps.fyle.models import Expense, ExpenseGroup
from apps.mappings.models import GeneralMapping
from apps.sage_intacct.actions import update_last_export_details
from apps.sage_intacct.connector import SageIntacctRestConnector
from apps.sage_intacct.enums import SageIntacctRestConnectionTypeEnum
from apps.sage_intacct.errors.helpers import (
error_matcher,
get_entity_values,
remove_support_id,
replace_destination_id_with_values
replace_destination_id_with_values,
)
from apps.sage_intacct.helpers import get_sage_intacct_connection
from apps.sage_intacct.models import (
Bill,
APPayment,
BillLineitem,
JournalEntry,
ExpenseReport,
APPaymentLineitem,
JournalEntryLineitem,
Bill,
BillLineitem,
ChargeCardTransaction,
ChargeCardTransactionLineitem,
ExpenseReport,
ExpenseReportLineitem,
JournalEntry,
JournalEntryLineitem,
SageIntacctReimbursement,
ChargeCardTransactionLineitem,
SageIntacctReimbursementLineitem
SageIntacctReimbursementLineitem,
)
from apps.sage_intacct.utils import SageIntacctConnector
from apps.tasks.models import Error, TaskLog
from apps.workspaces.enums import (
ExportTypeEnum,
SystemCommentEntityTypeEnum,
SystemCommentIntentEnum,
SystemCommentReasonEnum,
SystemCommentSourceEnum,
)
from apps.workspaces.models import Configuration, FeatureConfig, FyleCredential, SageIntacctCredential
from apps.workspaces.system_comments import add_system_comment, create_filtered_system_comments
from fyle_intacct_api.exceptions import BulkError
from fyle_intacct_api.logging_middleware import get_caller_info, get_logger
from fyle_intacct_api.utils import invalidate_sage_intacct_credentials

logger = logging.getLogger(__name__)
logger.level = logging.INFO
Expand Down Expand Up @@ -152,11 +147,13 @@ def create_or_update_employee_mapping(
raise EmployeeMapping.DoesNotExist

except EmployeeMapping.DoesNotExist:
source_employee = ExpenseAttribute.objects.get(
workspace_id=expense_group.workspace_id,
attribute_type='EMPLOYEE',
value=expense_group.description.get('employee_email')
)
source_employee = get_employee_expense_attribute(expense_group.description.get('employee_email'), expense_group.workspace_id)

if not source_employee:
source_employee = sync_inactive_employee(expense_group)

if not source_employee:
return

try:
if auto_map_employees_preference == 'EMAIL':
Expand Down Expand Up @@ -532,6 +529,54 @@ def handle_sage_intacct_rest_errors(exception: Exception, expense_group: Expense
post_accounting_export_summary(workspace_id=expense_group.workspace_id, expense_ids=[expense.id for expense in expense_group.expenses.all()], fund_source=expense_group.fund_source, is_failed=True)


def get_employee_expense_attribute(value: str, workspace_id: int) -> ExpenseAttribute:
"""
Get employee expense attribute
:param value: value
:param workspace_id: workspace id
"""
return ExpenseAttribute.objects.filter(
attribute_type='EMPLOYEE',
value=value,
workspace_id=workspace_id
).first()


def sync_inactive_employee(expense_group: ExpenseGroup) -> ExpenseAttribute:
try:
fyle_credentials = FyleCredential.objects.get(workspace_id=expense_group.workspace_id)
platform = PlatformConnector(fyle_credentials=fyle_credentials)

fyle_employee = platform.employees.get_employee_by_email(expense_group.description.get('employee_email'))
if len(fyle_employee):
fyle_employee = fyle_employee[0]
attribute = {
'attribute_type': 'EMPLOYEE',
'display_name': 'Employee',
'value': fyle_employee['user']['email'],
'source_id': fyle_employee['id'],
'active': True if fyle_employee['is_enabled'] and fyle_employee['has_accepted_invite'] else False,
'detail': {
'user_id': fyle_employee['user_id'],
'employee_code': fyle_employee['code'],
'full_name': fyle_employee['user']['full_name'],
'location': fyle_employee['location'],
'department': fyle_employee['department']['name'] if fyle_employee['department'] else None,
'department_id': fyle_employee['department_id'],
'department_code': fyle_employee['department']['code'] if fyle_employee['department'] else None
}
}
ExpenseAttribute.bulk_create_or_update_expense_attributes([attribute], 'EMPLOYEE', expense_group.workspace_id, True)
return get_employee_expense_attribute(expense_group.description.get('employee_email'), expense_group.workspace_id)
except (FyleInvalidTokenError, FyleInternalServerError) as e:
logger.info('Invalid Fyle refresh token or internal server error for workspace %s: %s', expense_group.workspace_id, str(e))
return None

except Exception as e:
logger.error('Error syncing inactive employee for workspace_id %s: %s', expense_group.workspace_id, str(e))
return None
Comment on lines +545 to +577
Copy link
Contributor

@coderabbitai coderabbitai bot Feb 11, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

Missing docstring causes CI failure; also a potential case-sensitivity bug on re-fetch.

Two issues:

  1. Pipeline failure: flake8: D103 Missing docstring in public function on line 545. Add a docstring.

  2. Bug: Line 569 stores the attribute with value=fyle_employee['user']['email'], but line 570 re-fetches using expense_group.description.get('employee_email'). If the Fyle API returns a differently-cased email (e.g. User@example.com vs user@example.com), the exact value= filter in get_employee_expense_attribute won't find the just-created record, and the function returns None despite a successful sync.

Proposed fix 
 def sync_inactive_employee(expense_group: ExpenseGroup) -> ExpenseAttribute:
+    """
+    Sync inactive employee from Fyle
+    :param expense_group: Expense Group
+    :return: ExpenseAttribute or None
+    """
     try:
         fyle_credentials = FyleCredential.objects.get(workspace_id=expense_group.workspace_id)
         platform = PlatformConnector(fyle_credentials=fyle_credentials)
 
         fyle_employee = platform.employees.get_employee_by_email(expense_group.description.get('employee_email'))
         if len(fyle_employee):
             fyle_employee = fyle_employee[0]
             attribute = {
                 'attribute_type': 'EMPLOYEE',
                 'display_name': 'Employee',
                 'value': fyle_employee['user']['email'],
                 'source_id': fyle_employee['id'],
-                'active': True if fyle_employee['is_enabled'] and fyle_employee['has_accepted_invite'] else False,
+                'active': fyle_employee['is_enabled'] and fyle_employee['has_accepted_invite'],
                 'detail': {
                     'user_id': fyle_employee['user_id'],
                     'employee_code': fyle_employee['code'],
                     'full_name': fyle_employee['user']['full_name'],
                     'location': fyle_employee['location'],
                     'department': fyle_employee['department']['name'] if fyle_employee['department'] else None,
                     'department_id': fyle_employee['department_id'],
                     'department_code': fyle_employee['department']['code'] if fyle_employee['department'] else None
                 }
             }
             ExpenseAttribute.bulk_create_or_update_expense_attributes([attribute], 'EMPLOYEE', expense_group.workspace_id, True)
-            return get_employee_expense_attribute(expense_group.description.get('employee_email'), expense_group.workspace_id)
+            return get_employee_expense_attribute(fyle_employee['user']['email'], expense_group.workspace_id)
     except (FyleInvalidTokenError, FyleInternalServerError) as e:
         logger.info('Invalid Fyle refresh token or internal server error for workspace %s: %s', expense_group.workspace_id, str(e))
         return None
 
     except Exception as e:
-        logger.error('Error syncing inactive employee for workspace_id %s: %s', expense_group.workspace_id, str(e))
+        logger.exception('Error syncing inactive employee for workspace_id %s: %s', expense_group.workspace_id, str(e))
         return None
📝 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 sync_inactive_employee(expense_group: ExpenseGroup) -> ExpenseAttribute:
try:
fyle_credentials = FyleCredential.objects.get(workspace_id=expense_group.workspace_id)
platform = PlatformConnector(fyle_credentials=fyle_credentials)
fyle_employee = platform.employees.get_employee_by_email(expense_group.description.get('employee_email'))
if len(fyle_employee):
fyle_employee = fyle_employee[0]
attribute = {
'attribute_type': 'EMPLOYEE',
'display_name': 'Employee',
'value': fyle_employee['user']['email'],
'source_id': fyle_employee['id'],
'active': True if fyle_employee['is_enabled'] and fyle_employee['has_accepted_invite'] else False,
'detail': {
'user_id': fyle_employee['user_id'],
'employee_code': fyle_employee['code'],
'full_name': fyle_employee['user']['full_name'],
'location': fyle_employee['location'],
'department': fyle_employee['department']['name'] if fyle_employee['department'] else None,
'department_id': fyle_employee['department_id'],
'department_code': fyle_employee['department']['code'] if fyle_employee['department'] else None
}
}
ExpenseAttribute.bulk_create_or_update_expense_attributes([attribute], 'EMPLOYEE', expense_group.workspace_id, True)
return get_employee_expense_attribute(expense_group.description.get('employee_email'), expense_group.workspace_id)
except (FyleInvalidTokenError, FyleInternalServerError) as e:
logger.info('Invalid Fyle refresh token or internal server error for workspace %s: %s', expense_group.workspace_id, str(e))
return None
except Exception as e:
logger.error('Error syncing inactive employee for workspace_id %s: %s', expense_group.workspace_id, str(e))
return None
def sync_inactive_employee(expense_group: ExpenseGroup) -> ExpenseAttribute:
"""
Sync inactive employee from Fyle
:param expense_group: Expense Group
:return: ExpenseAttribute or None
"""
try:
fyle_credentials = FyleCredential.objects.get(workspace_id=expense_group.workspace_id)
platform = PlatformConnector(fyle_credentials=fyle_credentials)
fyle_employee = platform.employees.get_employee_by_email(expense_group.description.get('employee_email'))
if len(fyle_employee):
fyle_employee = fyle_employee[0]
attribute = {
'attribute_type': 'EMPLOYEE',
'display_name': 'Employee',
'value': fyle_employee['user']['email'],
'source_id': fyle_employee['id'],
'active': fyle_employee['is_enabled'] and fyle_employee['has_accepted_invite'],
'detail': {
'user_id': fyle_employee['user_id'],
'employee_code': fyle_employee['code'],
'full_name': fyle_employee['user']['full_name'],
'location': fyle_employee['location'],
'department': fyle_employee['department']['name'] if fyle_employee['department'] else None,
'department_id': fyle_employee['department_id'],
'department_code': fyle_employee['department']['code'] if fyle_employee['department'] else None
}
}
ExpenseAttribute.bulk_create_or_update_expense_attributes([attribute], 'EMPLOYEE', expense_group.workspace_id, True)
return get_employee_expense_attribute(fyle_employee['user']['email'], expense_group.workspace_id)
except (FyleInvalidTokenError, FyleInternalServerError) as e:
logger.info('Invalid Fyle refresh token or internal server error for workspace %s: %s', expense_group.workspace_id, str(e))
return None
except Exception as e:
logger.exception('Error syncing inactive employee for workspace_id %s: %s', expense_group.workspace_id, str(e))
return None
🧰 Tools
🪛 GitHub Actions: Continuous Integration

[error] 545-545: flake8: D103 Missing docstring in public function

🪛 Ruff (0.15.0)

[warning] 575-575: Do not catch blind exception: Exception

(BLE001)

[warning] 576-576: Use logging.exception instead of logging.error

Replace with exception

(TRY400)

🤖 Prompt for AI Agents 
In `@apps/sage_intacct/tasks.py` around lines 545 - 577, Add a docstring to the
public function sync_inactive_employee explaining purpose, args and return
value; also make email case-consistent when creating and re-fetching the
attribute to avoid lookup failures (e.g., normalize to lower-case).
Specifically, in sync_inactive_employee ensure the attribute 'value' is set
using a normalized email (reference fyle_employee['user']['email'] and set to
.lower()) and then call get_employee_expense_attribute with the same normalized
value derived from expense_group.description.get('employee_email').lower(); keep
using ExpenseAttribute.bulk_create_or_update_expense_attributes and
get_employee_expense_attribute as the post-create lookup points.



def get_employee_mapping(employee_email: str, workspace_id: int, configuration: Configuration) -> EmployeeMapping:
"""
Get employee mapping
Expand Down Expand Up @@ -565,6 +610,9 @@ def __validate_employee_mapping(expense_group: ExpenseGroup, configuration: Conf
attribute_type='EMPLOYEE'
).first()

if not employee_attribute:
employee_attribute = sync_inactive_employee(expense_group)

try:
if expense_group.fund_source == 'PERSONAL':
entity = get_employee_mapping(employee_email, workspace_id, configuration)
Expand Down
Loading