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.

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

feat: support split expense grouping #681

Merged
merged 1 commit into from
Nov 25, 2024
Merged

feat: support split expense grouping #681

Show file tree
Hide file tree
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
24 changes: 24 additions & 0 deletions apps/fyle/migrations/0035_support_split_expense_grouping.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Generated by Django 3.2.14 on 2024-11-18 15:00

import apps.fyle.models
from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('fyle', '0034_expense_is_posted_at_null'),
]

operations = [
migrations.AddField(
model_name='expense',
name='bank_transaction_id',
field=models.CharField(blank=True, help_text='Bank Transaction ID', max_length=255, null=True),
),
migrations.AddField(
model_name='expensegroupsettings',
name='split_expense_grouping',
field=models.CharField(choices=[('SINGLE_LINE_ITEM', 'SINGLE_LINE_ITEM'), ('MULTIPLE_LINE_ITEM', 'MULTIPLE_LINE_ITEM')], default=apps.fyle.models.get_default_split_expense_grouping, help_text='specify line items for split expenses grouping', max_length=100),
),
]
62 changes: 57 additions & 5 deletions apps/fyle/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@
ALLOWED_FIELDS = [
'employee_email', 'report_id', 'claim_number', 'settlement_id',
'fund_source', 'vendor', 'category', 'project', 'cost_center',
'verified_at', 'approved_at', 'spent_at', 'expense_id', 'posted_at'
'verified_at', 'approved_at', 'spent_at', 'expense_id', 'posted_at',
'bank_transaction_id'
]


Expand Down Expand Up @@ -72,6 +73,8 @@
('not_in', 'not_in')
)

SPLIT_EXPENSE_GROUPING = (('SINGLE_LINE_ITEM', 'SINGLE_LINE_ITEM'), ('MULTIPLE_LINE_ITEM', 'MULTIPLE_LINE_ITEM'))

class Expense(models.Model):
"""
Expense
Expand Down Expand Up @@ -103,6 +106,7 @@ class Expense(models.Model):
report_id = models.CharField(max_length=255, help_text='Report ID')
report_title = models.TextField(null=True, blank=True, help_text='Report title')
corporate_card_id = models.CharField(max_length=255, null=True, blank=True, help_text='Corporate Card ID')
bank_transaction_id = models.CharField(max_length=255, null=True, blank=True, help_text='Bank Transaction ID')
file_ids = ArrayField(base_field=models.CharField(max_length=255), null=True, help_text='File IDs')
spent_at = models.DateTimeField(null=True, help_text='Expense spent at')
approved_at = models.DateTimeField(null=True, help_text='Expense approved at')
Expand Down Expand Up @@ -173,6 +177,7 @@ def create_expense_objects(expenses: List[Dict], workspace_id, skip_update: bool
'purpose': expense['purpose'],
'report_id': expense['report_id'],
'corporate_card_id': expense['corporate_card_id'],
'bank_transaction_id': expense['bank_transaction_id'],
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Handle potential KeyError when accessing expense['bank_transaction_id']

Accessing expense['bank_transaction_id'] without checking may raise a KeyError if 'bank_transaction_id' is missing in the expense dictionary. Consider using expense.get('bank_transaction_id') to prevent possible exceptions.

Apply this diff to fix the issue:

-'bank_transaction_id': expense['bank_transaction_id'],
+'bank_transaction_id': expense.get('bank_transaction_id'),
📝 Committable suggestion

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

Suggested change
'bank_transaction_id': expense['bank_transaction_id'],
'bank_transaction_id': expense.get('bank_transaction_id'),

'file_ids': expense['file_ids'],
'spent_at': expense['spent_at'],
'posted_at': expense['posted_at'],
Expand Down Expand Up @@ -207,6 +212,9 @@ def get_default_expense_state():
def get_default_ccc_expense_state():
return 'PAID'

def get_default_split_expense_grouping():
return 'MULTIPLE_LINE_ITEM'


class ExpenseGroupSettings(models.Model):
"""
Expand All @@ -232,6 +240,11 @@ class ExpenseGroupSettings(models.Model):
reimbursable_export_date_type = models.CharField(max_length=100, default='current_date', help_text='Export Date')
ccc_export_date_type = models.CharField(max_length=100, default='current_date', help_text='CCC Export Date')
import_card_credits = models.BooleanField(help_text='Import Card Credits', default=False)
split_expense_grouping = models.CharField(
max_length=100,
default=get_default_split_expense_grouping,
choices=SPLIT_EXPENSE_GROUPING, help_text='specify line items for split expenses grouping'
)
workspace = models.OneToOneField(
Workspace, on_delete=models.PROTECT, help_text='To which workspace this expense group setting belongs to',
related_name = 'expense_group_settings'
Expand Down Expand Up @@ -311,7 +324,8 @@ def update_expense_group_settings(expense_group_settings: Dict, workspace_id: in
'expense_state': expense_group_settings['expense_state'],
'ccc_expense_state': expense_group_settings['ccc_expense_state'],
'reimbursable_export_date_type': expense_group_settings['reimbursable_export_date_type'],
'ccc_export_date_type': expense_group_settings['ccc_export_date_type']
'ccc_export_date_type': expense_group_settings['ccc_export_date_type'],
'split_expense_grouping': expense_group_settings['split_expense_grouping']
}
)

Expand Down Expand Up @@ -362,6 +376,7 @@ def create_expense_groups_by_report_id_fund_source(expense_objects: List[Expense
"""
expense_group_settings = ExpenseGroupSettings.objects.get(workspace_id=workspace_id)

# Group Reimbursable Expenses
reimbursable_expense_group_fields = expense_group_settings.reimbursable_expense_group_fields

reimbursable_expenses = list(filter(lambda expense: expense.fund_source == 'PERSONAL', expense_objects))
Expand Down Expand Up @@ -392,6 +407,8 @@ def create_expense_groups_by_report_id_fund_source(expense_objects: List[Expense
reimbursable_expenses = list(filter(lambda expense: expense.amount > 0, reimbursable_expenses))

expense_groups = _group_expenses(reimbursable_expenses, reimbursable_expense_group_fields, workspace_id)

# Group CCC Expenses
corporate_credit_card_expense_group_field = expense_group_settings.corporate_credit_card_expense_group_fields

corporate_credit_card_expenses = list(filter(lambda expense: expense.fund_source == 'CCC', expense_objects))
Expand All @@ -401,10 +418,45 @@ def create_expense_groups_by_report_id_fund_source(expense_objects: List[Expense
filter(lambda expense: expense.amount > 0, corporate_credit_card_expenses)
)

corporate_credit_card_expense_groups = _group_expenses(
corporate_credit_card_expenses, corporate_credit_card_expense_group_field, workspace_id)
if corporate_credit_card_expenses:
# Group split Credit Card Charges by `bank_transaction_id`
if (
configuration.corporate_credit_card_expenses_object == 'CREDIT CARD CHARGE' and
expense_group_settings.split_expense_grouping == 'MULTIPLE_LINE_ITEM'
):
ccc_expenses_without_bank_transaction_id = list(
filter(lambda expense: not expense.bank_transaction_id, corporate_credit_card_expenses)
)

ccc_expenses_with_bank_transaction_id = list(
filter(lambda expense: expense.bank_transaction_id, corporate_credit_card_expenses)
)

if ccc_expenses_without_bank_transaction_id:
groups_without_bank_transaction_id = _group_expenses(
ccc_expenses_without_bank_transaction_id, corporate_credit_card_expense_group_field, workspace_id
)

expense_groups.extend(groups_without_bank_transaction_id)

if ccc_expenses_with_bank_transaction_id:
split_expense_group_fields = [
field for field in corporate_credit_card_expense_group_field
if field not in ('expense_id', 'expense_number')
]
split_expense_group_fields.append('bank_transaction_id')

groups_with_bank_transaction_id = _group_expenses(
ccc_expenses_with_bank_transaction_id, split_expense_group_fields, workspace_id
)
expense_groups.extend(groups_with_bank_transaction_id)

else:
corporate_credit_card_expense_groups = _group_expenses(
corporate_credit_card_expenses, corporate_credit_card_expense_group_field, workspace_id)

expense_groups.extend(corporate_credit_card_expense_groups)

expense_groups.extend(corporate_credit_card_expense_groups)
for expense_group in expense_groups:
if expense_group_settings.reimbursable_export_date_type == 'last_spent_at':
expense_group['last_spent_at'] = Expense.objects.filter(
Expand Down
76 changes: 39 additions & 37 deletions apps/netsuite/connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1581,51 +1581,50 @@ def get_bill(self, internal_id):
return bill

def construct_credit_card_charge_lineitems(
self, credit_card_charge_lineitem: CreditCardChargeLineItem, general_mapping: GeneralMapping,
self, credit_card_charge_lineitems: List[CreditCardChargeLineItem], general_mapping: GeneralMapping,
attachment_links: Dict, cluster_domain: str, org_id: str) -> List[Dict]:
"""
Create credit_card_charge line items
:return: constructed line items
"""
line = credit_card_charge_lineitem

lines = []

expense = Expense.objects.get(pk=line.expense_id)

netsuite_custom_segments = self.prepare_custom_segments(line.netsuite_custom_segments, attachment_links, expense, org_id)

base_line = {
'account': {'internalId': line.account_id},
'amount': line.amount,
'memo': line.memo,
'grossAmt': line.amount,
'department': {'internalId': line.department_id},
'class': {'internalId': line.class_id},
'location': {'internalId': line.location_id},
'customer': {'internalId': line.customer_id},
'customFieldList': netsuite_custom_segments,
'isBillable': line.billable,
'taxAmount': None,
'taxCode': {
'externalId': None,
'internalId': None,
'name': None,
'type': 'taxGroup'
},
}
for line in credit_card_charge_lineitems:
expense = Expense.objects.get(pk=line.expense_id)
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Add Error Handling When Retrieving Expense Object

The code retrieves the Expense object without handling the possibility that it may not exist, which could raise a DoesNotExist exception and cause a crash. Consider adding error handling to manage this scenario gracefully.

Apply this diff to handle the exception:

+from django.core.exceptions import ObjectDoesNotExist
...
-    expense = Expense.objects.get(pk=line.expense_id)
+    try:
+        expense = Expense.objects.get(pk=line.expense_id)
+    except Expense.DoesNotExist:
+        # Handle the missing expense, e.g., log an error or skip this line item
+        continue

Committable suggestion skipped: line range outside the PR's diff.


# Handle cases where no tax is applied first
if line.tax_item_id is None or line.tax_amount is None:
lines.append(base_line)
else:
lines += self.handle_taxed_line_items(base_line, line, expense.workspace_id, 'CREDIT_CARD_CHARGE', general_mapping)
netsuite_custom_segments = self.prepare_custom_segments(line.netsuite_custom_segments, attachment_links, expense, org_id)

base_line = {
'account': {'internalId': line.account_id},
'amount': line.amount,
'memo': line.memo,
'grossAmt': line.amount,
'department': {'internalId': line.department_id},
'class': {'internalId': line.class_id},
'location': {'internalId': line.location_id},
'customer': {'internalId': line.customer_id},
'customFieldList': netsuite_custom_segments,
'isBillable': line.billable,
'taxAmount': None,
'taxCode': {
'externalId': None,
'internalId': None,
'name': None,
'type': 'taxGroup'
},
}

# Handle cases where no tax is applied first
if line.tax_item_id is None or line.tax_amount is None:
lines.append(base_line)
else:
lines += self.handle_taxed_line_items(base_line, line, expense.workspace_id, 'CREDIT_CARD_CHARGE', general_mapping)

return lines

def __construct_credit_card_charge(
self, credit_card_charge: CreditCardCharge,
credit_card_charge_lineitem: CreditCardChargeLineItem, general_mapping: GeneralMapping, attachment_links: Dict) -> Dict:
credit_card_charge_lineitems: List[CreditCardChargeLineItem], general_mapping: GeneralMapping, attachment_links: Dict) -> Dict:
"""
Create a credit_card_charge
:return: constructed credit_card_charge
Expand Down Expand Up @@ -1664,15 +1663,15 @@ def __construct_credit_card_charge(
'memo': credit_card_charge.memo,
'tranid': credit_card_charge.reference_number,
'expenses': self.construct_credit_card_charge_lineitems(
credit_card_charge_lineitem, general_mapping, attachment_links, cluster_domain, org_id
credit_card_charge_lineitems, general_mapping, attachment_links, cluster_domain, org_id
),
'externalId': credit_card_charge.external_id
}

return credit_card_charge_payload

def post_credit_card_charge(self, credit_card_charge: CreditCardCharge,
credit_card_charge_lineitem: CreditCardChargeLineItem, general_mapping: GeneralMapping, attachment_links: Dict,
credit_card_charge_lineitems: List[CreditCardChargeLineItem], general_mapping: GeneralMapping, attachment_links: Dict,
refund: bool):
"""
Post vendor credit_card_charges to NetSuite
Expand All @@ -1694,12 +1693,15 @@ def post_credit_card_charge(self, credit_card_charge: CreditCardCharge,
f"script=customscript_cc_charge_fyle&deploy=customdeploy_cc_charge_fyle"

if refund:
credit_card_charge_lineitem.amount = abs(credit_card_charge_lineitem.amount)
for credit_card_charge_lineitem in credit_card_charge_lineitems:
credit_card_charge_lineitem.amount = abs(credit_card_charge_lineitem.amount)
credit_card_charge_lineitem.save()

Comment on lines +1696 to +1699
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid Modifying and Saving Line Items Within Refund Logic

Modifying and saving credit_card_charge_lineitem.amount directly within the refund logic may lead to unintended side effects by altering the database state. It's better to adjust the amount in the payload without persisting changes to the database.

Apply this diff:

-for credit_card_charge_lineitem in credit_card_charge_lineitems:    
-    credit_card_charge_lineitem.amount = abs(credit_card_charge_lineitem.amount)
-    credit_card_charge_lineitem.save()
+for line_item in credit_card_charge_lineitems:
+    line_item.amount = abs(line_item.amount)
+    # Avoid saving changes to the database to prevent side effects

Alternatively, create a copy of the line items list and modify the copy to ensure the original data remains unchanged.

📝 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
for credit_card_charge_lineitem in credit_card_charge_lineitems:
credit_card_charge_lineitem.amount = abs(credit_card_charge_lineitem.amount)
credit_card_charge_lineitem.save()
for line_item in credit_card_charge_lineitems:
line_item.amount = abs(line_item.amount)
# Avoid saving changes to the database to prevent side effects

url = f"https://{account.lower()}.restlets.api.netsuite.com/app/site/hosting/restlet.nl?" \
f"script=customscript_cc_refund_fyle&deploy=customdeploy_cc_refund_fyle"
f"script=customscript_cc_refund_fyle&deploy=customdeploy_cc_refund_fyle"

credit_card_charges_payload = self.__construct_credit_card_charge(
credit_card_charge, credit_card_charge_lineitem, general_mapping, attachment_links)
credit_card_charge, credit_card_charge_lineitems, general_mapping, attachment_links)

logger.info("| Payload for Credit Card Charge creation | Content: {{WORKSPACE_ID: {} EXPENSE_GROUP_ID: {} CREDIT_CARD_CHARGE_PAYLOAD: {}}}".format(self.workspace_id, credit_card_charge.expense_group.id, credit_card_charges_payload))

Expand Down
Loading
Loading