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: Single credit line for Journal Entry #655

Merged
merged 4 commits into from
Oct 16, 2024
Merged

feat: Single credit line for Journal Entry #655

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f6cddc4
feat: Single credit line for Journal Entry
ashwin1111 Oct 11, 2024
885678a
merge master
ashwin1111 Oct 11, 2024
1e38d5f
test: add tests for single credit line <> JE (#656)
ashwin1111 Oct 16, 2024
b220cf2
dummy
ashwin1111 Oct 16, 2024
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
2 changes: 1 addition & 1 deletion apps/fyle/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ def get_task_log_and_fund_source(workspace_id: int):

configuration = Configuration.objects.get(workspace_id=workspace_id)
fund_source = []

if configuration.reimbursable_expenses_object:
fund_source.append('PERSONAL')
if configuration.corporate_credit_card_expenses_object:
Expand Down
96 changes: 91 additions & 5 deletions apps/netsuite/connector.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2085,8 +2085,90 @@ def construct_journal_entry_lineitems(self, journal_entry_lineitems: List[Journa

return lines

@staticmethod
def __construct_single_itemized_credit_line(journal_entry_lineitems: List[JournalEntryLineItem]):
"""
Create journal entry line items for single credit line
:return: constructed line items
"""
lines = []
distinct_line_ids = {}

for line in journal_entry_lineitems:
account_ref = line.debit_account_id
entity_id = line.entity_id
line_id = '{account_ref}::::{entity_id}'.format(account_ref=account_ref, entity_id=entity_id)

if line_id in distinct_line_ids:
distinct_line_ids[line_id] += line.amount
Comment on lines +2100 to +2103
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue

Use Tuples Instead of String Concatenation for Dictionary Keys

Using string concatenation with separators for dictionary keys can lead to potential issues if the account_ref or entity_id contain the separator string '::::'. Consider using tuples as dictionary keys to avoid this problem.

Apply this diff to fix the issue:

-                line_id = '{account_ref}::::{entity_id}'.format(account_ref=account_ref, entity_id=entity_id)

-                if line_id in distinct_line_ids:
-                    distinct_line_ids[line_id] += line.amount
-                else:
-                    distinct_line_ids[line_id] = line.amount
+                key = (account_ref, entity_id)
+
+                if key in distinct_line_ids:
+                    distinct_line_ids[key] += line.amount
+                else:
+                    distinct_line_ids[key] = line.amount

Committable suggestion was skipped due to low confidence.

else:
distinct_line_ids[line_id] = line.amount

for line_id, amount in distinct_line_ids.items():
account_ref, entity_id = line_id.split('::::')
lineitem = {
'account': {
'name': None,
Comment on lines +2107 to +2111
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Unpack Tuple Keys Without String Splitting

Since tuples are used as keys, updating the unpacking logic improves clarity and efficiency by avoiding string manipulation.

Apply this diff to refactor the code:

-            for line_id, amount in distinct_line_ids.items():
-                account_ref, entity_id = line_id.split('::::')
+            for (account_ref, entity_id), amount in distinct_line_ids.items():
📝 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 line_id, amount in distinct_line_ids.items():
account_ref, entity_id = line_id.split('::::')
lineitem = {
'account': {
'name': None,
for (account_ref, entity_id), amount in distinct_line_ids.items():
lineitem = {
'account': {
'name': None,

'internalId': account_ref,
'externalId': None,
'type': 'account'
},
'department': {
'name': None,
'internalId': None,
'externalId': None,
'type': 'department'
},
'location': {
'name': None,
'internalId': None,
'externalId': None,
'type': 'location'
},
'class': {
'name': None,
'internalId': None,
'externalId': None,
'type': 'classification'
},
'entity': {
'name': None,
'internalId': entity_id,
'externalId': None,
'type': 'vendor'
},
'credit': amount,
'creditTax': None,
'customFieldList': [],
'debit': None,
'debitTax': None,
'eliminate': None,
'endDate': None,
'grossAmt': None,
'line': None,
'lineTaxCode': None,
'lineTaxRate': None,
'memo': 'Total Amount',
'residual': None,
'revenueRecognitionRule': None,
'schedule': None,
'scheduleNum': None,
'startDate': None,
'tax1Acct': None,
'taxAccount': None,
'taxBasis': None,
'tax1Amt': None,
'taxCode': None,
'taxRate1': None,
'totalAmount': None,
}

lines.append(lineitem)

return lines

def __construct_journal_entry(self, journal_entry: JournalEntry,
journal_entry_lineitems: List[JournalEntryLineItem]) -> Dict:
journal_entry_lineitems: List[JournalEntryLineItem], configuration: Configuration) -> Dict:
ruuushhh marked this conversation as resolved.
Show resolved Hide resolved
"""
Create a journal entry report
:return: constructed journal entry
Expand All @@ -2096,7 +2178,11 @@ def __construct_journal_entry(self, journal_entry: JournalEntry,
cluster_domain = fyle_credentials.cluster_domain
org_id = Workspace.objects.get(id=journal_entry.expense_group.workspace_id).fyle_org_id

credit_line = self.construct_journal_entry_lineitems(journal_entry_lineitems, credit='Credit', org_id=org_id)
if configuration.je_single_credit_line:
credit_line = self.__construct_single_itemized_credit_line(journal_entry_lineitems)
else:
credit_line = self.construct_journal_entry_lineitems(journal_entry_lineitems, credit='Credit', org_id=org_id)

debit_line = self.construct_journal_entry_lineitems(
journal_entry_lineitems,
debit='Debit', attachment_links={},
Expand Down Expand Up @@ -2166,13 +2252,13 @@ def __construct_journal_entry(self, journal_entry: JournalEntry,
return journal_entry_payload

def post_journal_entry(self, journal_entry: JournalEntry,
journal_entry_lineitems: List[JournalEntryLineItem]):
journal_entry_lineitems: List[JournalEntryLineItem], configuration: Configuration):
"""
Post journal entries to NetSuite
"""
configuration = Configuration.objects.get(workspace_id=self.workspace_id)
try:
journal_entry_payload = self.__construct_journal_entry(journal_entry, journal_entry_lineitems)
journal_entry_payload = self.__construct_journal_entry(journal_entry, journal_entry_lineitems, configuration)

logger.info("| Payload for Journal Entry creation | Content: {{WORKSPACE_ID: {} EXPENSE_GROUP_ID: {} JOURNAL_ENTRY_PAYLOAD: {}}}".format(self.workspace_id, journal_entry.expense_group.id, journal_entry_payload))

Expand All @@ -2186,7 +2272,7 @@ def post_journal_entry(self, journal_entry: JournalEntry,

if configuration.change_accounting_period and detail['message'] == message:
first_day_of_month = datetime.today().date().replace(day=1)
journal_entry_payload = self.__construct_journal_entry(journal_entry, journal_entry_lineitems)
journal_entry_payload = self.__construct_journal_entry(journal_entry, journal_entry_lineitems, configuration)
journal_entry_payload['tranDate'] = first_day_of_month
created_journal_entry = self.connection.journal_entries.post(journal_entry_payload)

Expand Down
2 changes: 0 additions & 2 deletions apps/netsuite/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1134,8 +1134,6 @@ def create_journal_entry_lineitems(expense_group: ExpenseGroup, configuration: C
configuration = Configuration.objects.get(workspace_id=expense_group.workspace_id)
employee_field_mapping = configuration.employee_field_mapping

debit_account_id = None

journal_entry_lineitem_objects = []

for lineitem in expenses:
Expand Down
2 changes: 1 addition & 1 deletion apps/netsuite/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -698,7 +698,7 @@ def create_journal_entry(expense_group, task_log_id, last_export):
)

created_journal_entry = netsuite_connection.post_journal_entry(
journal_entry_object, journal_entry_lineitems_objects
journal_entry_object, journal_entry_lineitems_objects, configuration
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Inconsistent Use of configuration Parameter in create_journal_entry Calls

The configuration parameter has been added to the post_journal_entry method; however, not all calls to create_journal_entry are passing this new parameter. This inconsistency may lead to potential runtime errors or unexpected behaviors.

  • Affected Files:
    • apps/netsuite/tasks.py: Multiple calls to create_journal_entry need to include the configuration parameter.
    • tests/test_netsuite/test_tasks.py: Tests invoking create_journal_entry should be updated to pass the configuration parameter.
🔗 Analysis chain

Verify the impact of the new configuration parameter

The post_journal_entry method now includes a configuration parameter. This change allows for more flexible journal entry creation based on configuration settings.

To ensure this change doesn't break existing functionality and is properly implemented throughout the codebase, please run the following checks:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Check if the configuration parameter is properly passed in all calls to create_journal_entry
rg "create_journal_entry\(" --type py

# Verify if the post_journal_entry method in the NetSuiteConnector class accepts the new configuration parameter
rg "def post_journal_entry" --type py

# Check for any tests that need to be updated due to this change
rg "test.*create_journal_entry" --type py

Length of output: 1868

)
worker_logger.info('Created Journal Entry with Expense Group %s successfully', expense_group.id)

Expand Down
6 changes: 4 additions & 2 deletions apps/workspaces/apis/advanced_settings/serializers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ class Meta:
'sync_netsuite_to_fyle_payments',
'auto_create_destination_entity',
'auto_create_merchants',
'memo_structure'
'memo_structure',
'je_single_credit_line'
]


Expand Down Expand Up @@ -153,7 +154,8 @@ def update(self, instance, validated):
'auto_create_destination_entity': configurations.get('auto_create_destination_entity'),
'auto_create_merchants': configurations.get('auto_create_merchants'),
'change_accounting_period': configurations.get('change_accounting_period'),
'memo_structure': configurations.get('memo_structure')
'memo_structure': configurations.get('memo_structure'),
'je_single_credit_line': configurations.get('je_single_credit_line')
}
)

Expand Down
18 changes: 18 additions & 0 deletions apps/workspaces/migrations/0039_configuration_je_single_credit_line.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 3.2.14 on 2024-10-09 12:10

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
('workspaces', '0038_configuration_allow_intercompany_vendors'),
]

operations = [
migrations.AddField(
model_name='configuration',
name='je_single_credit_line',
field=models.BooleanField(default=False, help_text='Journal Entry Single Credit Line'),
),
]
Comment on lines +12 to +18
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider adding an explicit reverse migration.

While Django automatically handles reverse migrations for field additions, it's a good practice to explicitly define them for clarity and to ensure proper rollback functionality.

Consider updating the operations list to include a reverse migration:

operations = [
    migrations.AddField(
        model_name='configuration',
        name='je_single_credit_line',
        field=models.BooleanField(default=False, help_text='Enable single credit line for journal entries instead of multiple lines'),
    ),
]

# Add this method to the Migration class
def reverse_migration(apps, schema_editor):
    Configuration = apps.get_model('workspaces', 'Configuration')
    Configuration.objects.all().update(je_single_credit_line=False)

operations.append(migrations.RunPython(migrations.noop, reverse_migration))

This ensures that if the migration needs to be reversed, the field will be properly removed and any data cleaned up.

1 change: 1 addition & 0 deletions apps/workspaces/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ class Configuration(models.Model):
updated_at = models.DateTimeField(auto_now=True, help_text='Updated at')
name_in_journal_entry = models.CharField(max_length=100, help_text='Name in jounral entry for ccc expense only', default='MERCHANT',choices=NAME_IN_JOURNAL_ENTRY)
allow_intercompany_vendors = models.BooleanField(default=False, help_text='Allow intercompany vendors')
je_single_credit_line = models.BooleanField(default=False, help_text='Journal Entry Single Credit Line')
Copy link

Choose a reason for hiding this comment

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

💡 Codebase verification

Update all affected files to handle the new je_single_credit_line field appropriately.

The addition of the je_single_credit_line field in the Configuration model impacts multiple areas of the application. Please ensure that the following files are reviewed and updated to handle this new configuration option:

  • tests/test_workspaces/fixtures.py
  • tests/test_workspaces/test_apis/test_export_settings/test_views.py
  • tests/test_netsuite/test_views.py
  • tests/test_netsuite/test_tasks.py
  • tests/test_netsuite/conftest.py
  • scripts/python/parse-error.py
  • tests/conftest.py
  • apps/workspaces/models.py
  • apps/netsuite/models.py
  • apps/netsuite/connector.py
  • apps/netsuite/queue.py
  • apps/netsuite/tasks.py
  • apps/netsuite/migrations/0002_expensereports_journalentries.py
  • apps/tasks/models.py
  • apps/workspaces/actions.py
  • apps/workspaces/serializers.py

Ensure that each of these files properly references and handles the new je_single_credit_line field to maintain consistency and functionality across the application.

🔗 Analysis chain

Verify related code for consistency with the new field.

While the addition of the je_single_credit_line field is straightforward, it's important to ensure that other parts of the application are updated to handle this new configuration option.

Please run the following script to identify areas of the codebase that might need updates:

Review the output and update any relevant code to handle the new je_single_credit_line configuration option.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Find potential areas that might need updates due to the new je_single_credit_line field.

# Search for uses of the Configuration model
echo "Files referencing the Configuration model:"
rg "from .+models import .+Configuration" --type python

# Search for journal entry creation or processing
echo "\nFiles potentially involved in journal entry processing:"
rg "journal.+entr(y|ies)" --type python

# Search for existing uses of configuration fields related to journal entries
echo "\nExisting uses of configuration fields for journal entries:"
rg "configuration\..+journal" --type python

Length of output: 589

Script:

#!/bin/bash
# Description: Find potential areas that might need updates due to the new je_single_credit_line field.

# Search for uses of the Configuration model
echo "Files referencing the Configuration model:"
rg "from .+models import .+Configuration" --type py

# Search for journal entry creation or processing
echo "\nFiles potentially involved in journal entry processing:"
rg "journal.+entr(y|ies)" --type py

# Search for existing uses of configuration fields related to journal entries
echo "\nExisting uses of configuration fields for journal entries:"
rg "configuration\..+journal" --type py

Length of output: 18413


class Meta:
db_table = 'configurations'
Expand Down
14 changes: 8 additions & 6 deletions tests/sql_fixtures/reset_db_fixtures/reset_db.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,8 @@ CREATE TABLE public.configurations (
is_simplify_report_closure_enabled boolean NOT NULL,
import_items boolean NOT NULL,
name_in_journal_entry character varying(100) NOT NULL,
allow_intercompany_vendors boolean NOT NULL
allow_intercompany_vendors boolean NOT NULL,
je_single_credit_line boolean NOT NULL
);


Expand Down Expand Up @@ -2691,10 +2692,10 @@ COPY public.category_mappings (id, created_at, updated_at, destination_account_i
-- Data for Name: configurations; Type: TABLE DATA; Schema: public; Owner: postgres
--

COPY public.configurations (id, reimbursable_expenses_object, corporate_credit_card_expenses_object, created_at, updated_at, workspace_id, sync_fyle_to_netsuite_payments, sync_netsuite_to_fyle_payments, import_projects, auto_map_employees, import_categories, auto_create_destination_entity, auto_create_merchants, employee_field_mapping, import_tax_items, change_accounting_period, memo_structure, map_fyle_cards_netsuite_account, skip_cards_mapping, import_vendors_as_merchants, import_netsuite_employees, is_simplify_report_closure_enabled, import_items, name_in_journal_entry, allow_intercompany_vendors) FROM stdin;
1 EXPENSE REPORT BILL 2021-11-15 08:56:07.193743+00 2021-11-15 08:56:07.193795+00 1 f f f \N f f f EMPLOYEE f f {employee_email,category,spent_on,report_number,purpose} t f f f f f MERCHANT f
2 JOURNAL ENTRY CREDIT CARD CHARGE 2021-11-16 04:18:15.836271+00 2021-11-16 04:20:09.969589+00 2 f f f \N f f f EMPLOYEE t f {employee_email,category,spent_on,report_number,purpose} t f f f f f MERCHANT f
3 JOURNAL ENTRY CREDIT CARD CHARGE 2021-12-03 11:04:00.194287+00 2021-12-03 11:04:00.1943+00 49 f f f \N f f f EMPLOYEE f f {employee_email,category,spent_on,report_number,purpose} t f f f f f MERCHANT f
COPY public.configurations (id, reimbursable_expenses_object, corporate_credit_card_expenses_object, created_at, updated_at, workspace_id, sync_fyle_to_netsuite_payments, sync_netsuite_to_fyle_payments, import_projects, auto_map_employees, import_categories, auto_create_destination_entity, auto_create_merchants, employee_field_mapping, import_tax_items, change_accounting_period, memo_structure, map_fyle_cards_netsuite_account, skip_cards_mapping, import_vendors_as_merchants, import_netsuite_employees, is_simplify_report_closure_enabled, import_items, name_in_journal_entry, allow_intercompany_vendors, je_single_credit_line) FROM stdin;
1 EXPENSE REPORT BILL 2021-11-15 08:56:07.193743+00 2021-11-15 08:56:07.193795+00 1 f f f \N f f f EMPLOYEE f f {employee_email,category,spent_on,report_number,purpose} t f f f f f MERCHANT f f
2 JOURNAL ENTRY CREDIT CARD CHARGE 2021-11-16 04:18:15.836271+00 2021-11-16 04:20:09.969589+00 2 f f f \N f f f EMPLOYEE t f {employee_email,category,spent_on,report_number,purpose} t f f f f f MERCHANT f f
3 JOURNAL ENTRY CREDIT CARD CHARGE 2021-12-03 11:04:00.194287+00 2021-12-03 11:04:00.1943+00 49 f f f \N f f f EMPLOYEE f f {employee_email,category,spent_on,report_number,purpose} t f f f f f MERCHANT f f
\.


Expand Down Expand Up @@ -7984,6 +7985,7 @@ COPY public.django_migrations (id, app, name, applied) FROM stdin;
197 netsuite 0026_auto_20240902_1650 2024-09-02 16:50:55.770274+00
198 netsuite 0027_auto_20240924_0820 2024-09-24 08:24:35.223017+00
199 fyle_accounting_mappings 0026_destinationattribute_code 2024-10-01 08:54:06.770864+00
200 workspaces 0039_configuration_je_single_credit_line 2024-10-11 13:43:49.169823+00
\.


Expand Down Expand Up @@ -11895,7 +11897,7 @@ SELECT pg_catalog.setval('public.django_content_type_id_seq', 47, true);
-- Name: django_migrations_id_seq; Type: SEQUENCE SET; Schema: public; Owner: postgres
--

SELECT pg_catalog.setval('public.django_migrations_id_seq', 199, true);
SELECT pg_catalog.setval('public.django_migrations_id_seq', 200, true);


--
Expand Down
58 changes: 57 additions & 1 deletion tests/test_netsuite/fixtures.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1053,8 +1053,64 @@
'tranDate': '2021-12-08T10:33:44',
'tranId': None,
'externalId': 'journal 47 - [email protected]',
}
},
],
'journal_entry_clubbed_lines': [{
'account': {
'name': None,
'internalId': '228',
'externalId': None,
'type': 'account',
},
'department': {
'name': None,
'internalId': None,
'externalId': None,
'type': 'department',
},
'location': {
'name': None,
'internalId': None,
'externalId': None,
'type': 'location',
},
'class': {
'name': None,
'internalId': None,
'externalId': None,
'type': 'classification',
},
'entity': {
'name': None,
'internalId': '10491',
'externalId': None,
'type': 'vendor',
},
'credit': 120.0,
'creditTax': None,
'customFieldList': [],
'debit': None,
'debitTax': None,
'eliminate': None,
'endDate': None,
'grossAmt': None,
'line': None,
'lineTaxCode': None,
'lineTaxRate': None,
'memo': 'Total Amount',
'residual': None,
'revenueRecognitionRule': None,
'schedule': None,
'scheduleNum': None,
'startDate': None,
'tax1Acct': None,
'taxAccount': None,
'taxBasis': None,
'tax1Amt': None,
'taxCode': None,
'taxRate1': None,
'totalAmount': None,
}],
'credit_card_charge': [
{
'account': {'internalId': '228'},
Expand Down
Loading
Loading