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

Expenses Sync APIs #32

Merged
merged 15 commits into from
Nov 30, 2023
112 changes: 103 additions & 9 deletions apps/accounting_exports/models.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
from django.db import models
from django.contrib.postgres.fields import ArrayField
from typing import List

from django.contrib.postgres.aggregates import ArrayAgg
from django.contrib.postgres.fields import ArrayField
from django.db import models
from django.db.models import Count
from fyle_accounting_mappings.models import ExpenseAttribute

from apps.fyle.models import Expense
from apps.workspaces.models import BaseForeignWorkspaceModel, BaseModel, ExportSetting
from ms_business_central_api.models.fields import (
BooleanFalseField,
CustomDateTimeField,
CustomJsonField,
IntegerNullField,
StringNotNullField,
StringNullField,
CustomJsonField,
CustomDateTimeField,
StringOptionsField,
IntegerNullField,
BooleanFalseField,
TextNotNullField
TextNotNullField,
)
from apps.workspaces.models import BaseForeignWorkspaceModel, BaseModel
from apps.fyle.models import Expense

TYPE_CHOICES = (
('INVOICES', 'INVOICES'),
Expand All @@ -31,6 +34,55 @@
)


def _group_expenses(expenses: List[Expense], export_setting: ExportSetting, fund_source: str):
"""
Group expenses based on specified fields
"""

credit_card_expense_grouped_by = export_setting.credit_card_expense_grouped_by
credit_card_expense_date = export_setting.credit_card_expense_date
reimbursable_expense_grouped_by = export_setting.reimbursable_expense_grouped_by
reimbursable_expense_date = export_setting.reimbursable_expense_date

default_fields = ['employee_email', 'fund_source']
report_grouping_fields = ['report_id', 'claim_number']
expense_grouping_fields = ['expense_id', 'expense_number']

# Define a mapping for fund sources and their associated group fields
fund_source_mapping = {
'CCC': {
'group_by': report_grouping_fields if credit_card_expense_grouped_by == 'REPORT' else expense_grouping_fields,
'date_field': credit_card_expense_date.lower() if credit_card_expense_date != 'LAST_SPENT_AT' else None
},
'PERSONAL': {
'group_by': report_grouping_fields if reimbursable_expense_grouped_by == 'REPORT' else expense_grouping_fields,
'date_field': reimbursable_expense_date.lower() if reimbursable_expense_date != 'LAST_SPENT_AT' else None
}
}

# Update expense_group_fields based on the fund_source
fund_source_data = fund_source_mapping.get(fund_source)
group_by_field = fund_source_data.get('group_by')
date_field = fund_source_data.get('date_field')

default_fields.extend(group_by_field)

if date_field:
default_fields.append(date_field)

# Extract expense IDs from the provided expenses
expense_ids = [expense.id for expense in expenses]
# Retrieve expenses from the database
expenses = Expense.objects.filter(id__in=expense_ids).all()

# Create expense groups by grouping expenses based on specified fields
expense_groups = list(expenses.values(*default_fields).annotate(
total=Count('*'), expense_ids=ArrayAgg('id'))
)

return expense_groups


class AccountingExport(BaseForeignWorkspaceModel):
"""
Table to store accounting exports
Expand All @@ -50,6 +102,48 @@ class AccountingExport(BaseForeignWorkspaceModel):
class Meta:
db_table = 'accounting_exports'

@staticmethod
def create_accounting_export(expense_objects: List[Expense], fund_source: str, workspace_id):
"""
Group expenses by report_id and fund_source, format date fields, and create AccountingExport objects.
"""
# Retrieve the ExportSetting for the workspace
export_setting = ExportSetting.objects.get(workspace_id=workspace_id)

# Group expenses based on specified fields and fund_source
accounting_exports = _group_expenses(expense_objects, export_setting, fund_source)

fund_source_map = {
'PERSONAL': 'reimbursable',
'CCC': 'credit_card'
}

for accounting_export in accounting_exports:
# Determine the date field based on fund_source
date_field = getattr(export_setting, f"{fund_source_map.get(fund_source)}_expense_date", None)

# Calculate and assign 'last_spent_at' based on the chosen date field
if date_field == 'last_spent_at':
latest_expense = Expense.objects.filter(id__in=accounting_export['expense_ids']).order_by('-spent_at').first()
accounting_export['last_spent_at'] = latest_expense.spent_at if latest_expense else None

# Store expense IDs and remove unnecessary keys
expense_ids = accounting_export['expense_ids']
accounting_export[date_field] = accounting_export[date_field].strftime('%Y-%m-%dT%H:%M:%S')
accounting_export.pop('total')
accounting_export.pop('expense_ids')

# Create an AccountingExport object for the expense group
accounting_export_instance = AccountingExport.objects.create(
workspace_id=workspace_id,
fund_source=accounting_export['fund_source'],
description=accounting_export,
status='EXPORT_READY'
)

# Add related expenses to the AccountingExport object
accounting_export_instance.expenses.add(*expense_ids)


class AccountingExportSummary(BaseModel):
Comment on lines +105 to 147
Copy link

Choose a reason for hiding this comment

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

In the create_accounting_export function, there is a potential issue with the date formatting logic. If the date_field is not 'last_spent_at', the code attempts to format the date without checking if the date is present or not. This could lead to an AttributeError if accounting_export[date_field] is None.

- accounting_export[date_field] = accounting_export[date_field].strftime('%Y-%m-%dT%H:%M:%S')
+ if accounting_export.get(date_field):
+     accounting_export[date_field] = accounting_export[date_field].strftime('%Y-%m-%dT%H:%M:%S')

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.

Suggested change
@staticmethod
def create_accounting_export(expense_objects: List[Expense], fund_source: str, workspace_id):
"""
Group expenses by report_id and fund_source, format date fields, and create AccountingExport objects.
"""
# Retrieve the ExportSetting for the workspace
export_setting = ExportSetting.objects.get(workspace_id=workspace_id)
# Group expenses based on specified fields and fund_source
accounting_exports = _group_expenses(expense_objects, export_setting, fund_source)
fund_source_map = {
'PERSONAL': 'reimbursable',
'CCC': 'credit_card'
}
for accounting_export in accounting_exports:
# Determine the date field based on fund_source
date_field = getattr(export_setting, f"{fund_source_map.get(fund_source)}_expense_date", None)
# Calculate and assign 'last_spent_at' based on the chosen date field
if date_field == 'last_spent_at':
latest_expense = Expense.objects.filter(id__in=accounting_export['expense_ids']).order_by('-spent_at').first()
accounting_export['last_spent_at'] = latest_expense.spent_at if latest_expense else None
# Store expense IDs and remove unnecessary keys
expense_ids = accounting_export['expense_ids']
accounting_export[date_field] = accounting_export[date_field].strftime('%Y-%m-%dT%H:%M:%S')
accounting_export.pop('total')
accounting_export.pop('expense_ids')
# Create an AccountingExport object for the expense group
accounting_export_instance = AccountingExport.objects.create(
workspace_id=workspace_id,
fund_source=accounting_export['fund_source'],
description=accounting_export,
status='EXPORT_READY'
)
# Add related expenses to the AccountingExport object
accounting_export_instance.expenses.add(*expense_ids)
@staticmethod
def create_accounting_export(expense_objects: List[Expense], fund_source: str, workspace_id):
"""
Group expenses by report_id and fund_source, format date fields, and create AccountingExport objects.
"""
# Retrieve the ExportSetting for the workspace
export_setting = ExportSetting.objects.get(workspace_id=workspace_id)
# Group expenses based on specified fields and fund_source
accounting_exports = _group_expenses(expense_objects, export_setting, fund_source)
fund_source_map = {
'PERSONAL': 'reimbursable',
'CCC': 'credit_card'
}
for accounting_export in accounting_exports:
# Determine the date field based on fund_source
date_field = getattr(export_setting, f"{fund_source_map.get(fund_source)}_expense_date", None)
# Calculate and assign 'last_spent_at' based on the chosen date field
if date_field == 'last_spent_at':
latest_expense = Expense.objects.filter(id__in=accounting_export['expense_ids']).order_by('-spent_at').first()
accounting_export['last_spent_at'] = latest_expense.spent_at if latest_expense else None
# Store expense IDs and remove unnecessary keys
expense_ids = accounting_export['expense_ids']
if accounting_export.get(date_field):
accounting_export[date_field] = accounting_export[date_field].strftime('%Y-%m-%dT%H:%M:%S')
accounting_export.pop('total')
accounting_export.pop('expense_ids')
# Create an AccountingExport object for the expense group
accounting_export_instance = AccountingExport.objects.create(
workspace_id=workspace_id,
fund_source=accounting_export['fund_source'],
description=accounting_export,
status='EXPORT_READY'
)
# Add related expenses to the AccountingExport object
accounting_export_instance.expenses.add(*expense_ids)

"""
Expand Down
28 changes: 17 additions & 11 deletions apps/business_central/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def __init__(self, credentials_object: BusinessCentralCredentials, workspace_id:
refresh_token = credentials_object.refresh_token

self.connection = Dynamics(
enviroment=environment,
environment=environment,
client_id=client_id,
client_secret=client_secret,
refresh_token=refresh_token,
Expand Down Expand Up @@ -59,18 +59,20 @@ def _sync_data(self, data, attribute_type, display_name, workspace_id, field_nam
"""

destination_attributes = []

for item in data:
detail = {field: getattr(item, field) for field in field_names}
detail = {field: item[field] for field in field_names}
if (attribute_type == 'EMPLOYEE' and item['status'] == 'Active') or attribute_type == 'LOCATION' or item['blocked'] != True:
active = True
else:
active = False
Comment on lines +64 to +67
Copy link

Choose a reason for hiding this comment

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

The logical condition in the _sync_data method may not work as intended due to the lack of parentheses. The condition should ensure that item['blocked'] != True is evaluated in the context of the attribute_type being 'EMPLOYEE'.

- if (attribute_type == 'EMPLOYEE' and item['status'] == 'Active') or attribute_type == 'LOCATION' or item['blocked'] != True:
+ if (attribute_type == 'EMPLOYEE' and item['status'] == 'Active') or (attribute_type == 'LOCATION' and item['blocked'] != 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.

Suggested change
if (attribute_type == 'EMPLOYEE' and item['status'] == 'Active') or attribute_type == 'LOCATION' or item['blocked'] != True:
active = True
else:
active = False
if (attribute_type == 'EMPLOYEE' and item['status'] == 'Active') or (attribute_type == 'LOCATION' and item['blocked'] != True):
active = True
else:
active = False

destination_attributes.append(self._create_destination_attribute(
attribute_type,
display_name,
item.name,
item.id,
item.is_active,
item['displayName'],
item['id'],
active,
detail
))

DestinationAttribute.bulk_create_or_update_destination_attributes(
destination_attributes, attribute_type, workspace_id, True)

Expand All @@ -89,9 +91,10 @@ def sync_accounts(self):
"""
workspace = Workspace.objects.get(id=self.workspace_id)
self.connection.company_id = workspace.business_central_company_id
field_names = ['category', 'subCategory', 'accountType', 'directPosting', 'lastModifiedDateTime']

accounts = self.connection.accounts.get_all()
self._sync_data(accounts, 'ACCOUNT', 'accounts', self.workspace_id)
self._sync_data(accounts, 'ACCOUNT', 'accounts', self.workspace_id, field_names)
return []

def sync_vendors(self):
Expand All @@ -100,9 +103,10 @@ def sync_vendors(self):
"""
workspace = Workspace.objects.get(id=self.workspace_id)
self.connection.company_id = workspace.business_central_company_id
field_names = ['email', 'currencyId', 'currencyCode', 'lastModifiedDateTime']

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

def sync_employees(self):
Expand All @@ -111,9 +115,10 @@ def sync_employees(self):
"""
workspace = Workspace.objects.get(id=self.workspace_id)
self.connection.company_id = workspace.business_central_company_id
field_names = ['email', 'email', 'personalEmail', 'lastModifiedDateTime']

employees = self.connection.employees.get_all()
self._sync_data(employees, 'EMPLOYEE', 'employee', self.workspace_id)
self._sync_data(employees, 'EMPLOYEE', 'employee', self.workspace_id, field_names)
return []

def sync_locations(self):
Expand All @@ -122,7 +127,8 @@ def sync_locations(self):
"""
workspace = Workspace.objects.get(id=self.workspace_id)
self.connection.company_id = workspace.business_central_company_id
field_names = ['code', 'city', 'country']

locations = self.connection.locations.get_all()
self._sync_data(locations, 'LOCATION', 'location', self.workspace_id)
self._sync_data(locations, 'LOCATION', 'location', self.workspace_id, field_names)
return []
37 changes: 37 additions & 0 deletions apps/fyle/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
import logging
import traceback
from functools import wraps

from fyle.platform.exceptions import NoPrivilegeError

from apps.workspaces.models import FyleCredential

logger = logging.getLogger(__name__)
logger.level = logging.INFO


def handle_exceptions(func):
@wraps(func)
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except FyleCredential.DoesNotExist:
logger.info('Fyle credentials not found %s', args[0]) # args[1] is workspace_id
args[1].detail = {'message': 'Fyle credentials do not exist in workspace'}
args[1].status = 'FAILED'
args[1].save()

except NoPrivilegeError:
logger.info('Invalid Fyle Credentials / Admin is disabled')
args[1].detail = {'message': 'Invalid Fyle Credentials / Admin is disabled'}
args[1].status = 'FAILED'
args[1].save()

except Exception:
error = traceback.format_exc()
args[1].detail = {'error': error}
args[1].status = 'FATAL'
args[1].save()
logger.exception('Something unexpected happened workspace_id: %s %s', args[0], args[1].detail)

return wrapper
Comment on lines +13 to +37
Copy link

Choose a reason for hiding this comment

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

The handle_exceptions decorator assumes a specific order and type of arguments, which could lead to issues if the decorated function does not follow this pattern. Consider making the decorator more flexible to handle different argument orders or types, and ensure that exceptions are re-raised after logging and updating the status to notify the calling code of the exception.

+            raise

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.

Suggested change
def handle_exceptions(func):
@wraps(func)
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except FyleCredential.DoesNotExist:
logger.info('Fyle credentials not found %s', args[0]) # args[1] is workspace_id
args[1].detail = {'message': 'Fyle credentials do not exist in workspace'}
args[1].status = 'FAILED'
args[1].save()
except NoPrivilegeError:
logger.info('Invalid Fyle Credentials / Admin is disabled')
args[1].detail = {'message': 'Invalid Fyle Credentials / Admin is disabled'}
args[1].status = 'FAILED'
args[1].save()
except Exception:
error = traceback.format_exc()
args[1].detail = {'error': error}
args[1].status = 'FATAL'
args[1].save()
logger.exception('Something unexpected happened workspace_id: %s %s', args[0], args[1].detail)
return wrapper
def handle_exceptions(func):
@wraps(func)
def wrapper(*args, **kwargs):
try:
return func(*args, **kwargs)
except FyleCredential.DoesNotExist:
logger.info('Fyle credentials not found %s', args[0]) # args[1] is workspace_id
args[1].detail = {'message': 'Fyle credentials do not exist in workspace'}
args[1].status = 'FAILED'
args[1].save()
raise
except NoPrivilegeError:
logger.info('Invalid Fyle Credentials / Admin is disabled')
args[1].detail = {'message': 'Invalid Fyle Credentials / Admin is disabled'}
args[1].status = 'FAILED'
args[1].save()
raise
except Exception:
error = traceback.format_exc()
args[1].detail = {'error': error}
args[1].status = 'FATAL'
args[1].save()
logger.exception('Something unexpected happened workspace_id: %s %s', args[0], args[1].detail)
raise
return wrapper

43 changes: 43 additions & 0 deletions apps/fyle/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,40 @@ def post_request(url, body, refresh_token=None):
raise Exception(response.text)
Copy link

Choose a reason for hiding this comment

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

Consider raising a more specific exception or logging the error to provide better context for debugging purposes.



def get_request(url, params, refresh_token):
"""
Create a HTTP get request.
"""
access_token = get_access_token(refresh_token)
api_headers = {
'content-type': 'application/json',
'Authorization': 'Bearer {0}'.format(access_token)
}
api_params = {}

for k in params:
# ignore all unused params
if not params[k] is None:
p = params[k]

# convert boolean to lowercase string
if isinstance(p, bool):
p = str(p).lower()

api_params[k] = p

response = requests.get(
url,
headers=api_headers,
params=api_params
)

if response.status_code == 200:
return json.loads(response.text)
Comment on lines +64 to +65
Copy link

Choose a reason for hiding this comment

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

Verify that the response content type is 'application/json' before attempting to parse the response text as JSON to avoid potential errors.

else:
raise Exception(response.text)


def get_access_token(refresh_token: str) -> str:
"""
Get access token from fyle
Expand Down Expand Up @@ -102,3 +136,12 @@ def get_exportable_accounting_exports_ids(workspace_id: int):
).values_list('id', flat=True)

return accounting_export_ids


def get_fyle_orgs(refresh_token: str, cluster_domain: str):
"""
Get fyle orgs of a user
"""
api_url = '{0}/api/orgs/'.format(cluster_domain)

return get_request(api_url, {}, refresh_token)
57 changes: 57 additions & 0 deletions apps/fyle/queue.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
"""
All the tasks which are queued into django-q
* User Triggered Async Tasks
* Schedule Triggered Async Tasks
"""
from django_q.tasks import async_task

from apps.accounting_exports.models import AccountingExport
from apps.fyle.tasks import import_expenses


def queue_import_reimbursable_expenses(workspace_id: int, synchronous: bool = False):
"""
Queue Import of Reimbursable Expenses from Fyle
:param workspace_id: Workspace id
:return: None
"""
accounting_export, _ = AccountingExport.objects.update_or_create(
workspace_id=workspace_id,
type='FETCHING_REIMBURSABLE_EXPENSES',
defaults={
'status': 'IN_PROGRESS'
}
)

if not synchronous:
async_task(
'apps.fyle.tasks.import_expenses',
workspace_id, accounting_export, 'PERSONAL_CASH_ACCOUNT', 'PERSONAL'
)
return

import_expenses(workspace_id, accounting_export, 'PERSONAL_CASH_ACCOUNT', 'PERSONAL')
Copy link

Choose a reason for hiding this comment

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

The import_expenses function is called with hardcoded values 'PERSONAL_CASH_ACCOUNT' and 'PERSONAL'. Consider making these values configurable or ensuring they are appropriate for all use cases.

Comment on lines +12 to +33
Copy link

Choose a reason for hiding this comment

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

The hardcoded values 'PERSONAL_CASH_ACCOUNT' and 'PERSONAL' are still present in the queue_import_reimbursable_expenses function. Consider making these values configurable or ensuring they are appropriate for all use cases.


There is no error handling in the queue_import_reimbursable_expenses function. Consider using the new handle_exceptions decorator to manage exceptions and ensure the AccountingExport status is accurately reflected.



def queue_import_credit_card_expenses(workspace_id: int, synchronous: bool = False):
"""
Queue Import of Credit Card Expenses from Fyle
:param workspace_id: Workspace id
:return: None
"""
accounting_export, _ = AccountingExport.objects.update_or_create(
workspace_id=workspace_id,
type='FETCHING_CREDIT_CARD_EXPENSES',
defaults={
'status': 'IN_PROGRESS'
}
)

if not synchronous:
async_task(
'apps.fyle.tasks.import_expenses',
workspace_id, accounting_export, 'PERSONAL_CORPORATE_CREDIT_CARD_ACCOUNT', 'CCC'
)
return

import_expenses(workspace_id, accounting_export, 'PERSONAL_CORPORATE_CREDIT_CARD_ACCOUNT', 'CCC')
Comment on lines +36 to +57
Copy link

Choose a reason for hiding this comment

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

The hardcoded values 'PERSONAL_CORPORATE_CREDIT_CARD_ACCOUNT' and 'CCC' are still present in the queue_import_credit_card_expenses function. Consider making these values configurable or ensuring they are appropriate for all use cases.


There is no error handling in the queue_import_credit_card_expenses function. Consider using the new handle_exceptions decorator to manage exceptions and ensure the AccountingExport status is accurately reflected.

Loading
Loading