feat: Single credit line for Journal Entry #655
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThe changes encompass multiple files, primarily focusing on enhancing expense and reimbursement handling. Key modifications include updates to method signatures, the introduction of new methods for grouping expenses, and improvements in error handling and logic across various classes. Additionally, a new boolean field Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
ashwin1111
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (14)
apps/workspaces/migrations/0039_configuration_je_single_credit_line.py (1)
13-17: Consider enhancing the help text for clarity.The new
je_single_credit_linefield is correctly added with appropriate default value. However, the help text "Journal Entry Single Credit Line" could be more descriptive to provide better context.Consider updating the help text to something more informative, such as:
help_text='Enable single credit line for journal entries instead of multiple lines'This would provide clearer guidance on the purpose and effect of this configuration option.
apps/workspaces/apis/advanced_settings/serializers.py (3)
34-35: LGTM! Consider updating documentation.The addition of the
je_single_credit_linefield to theConfigurationSerializerlooks good. This new field likely introduces a configuration option for handling journal entries with a single credit line.Consider updating the relevant documentation to explain the purpose and usage of this new configuration option.
Line range hint
134-145: UpdateAdvancedSettingsSerializer.update()to handle the new fieldThe
updatemethod inAdvancedSettingsSerializerdoesn't handle the newly addedje_single_credit_linefield. This could lead to the new configuration option being ignored when updating settings.To fix this, update the
Configuration.objects.update_or_create()call in theupdatemethod to include the new field:configuration_instance, _ = Configuration.objects.update_or_create( workspace=instance, defaults={ 'sync_fyle_to_netsuite_payments': configurations.get('sync_fyle_to_netsuite_payments'), 'sync_netsuite_to_fyle_payments': configurations.get('sync_netsuite_to_fyle_payments'), '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') } )This change ensures that the new
je_single_credit_lineconfiguration is properly saved when updating the settings.
Line range hint
1-190: Summary of changes and recommendations
The addition of the
je_single_credit_linefield to theConfigurationSerializeris a good improvement, potentially allowing for more flexible journal entry configurations.However, the
updatemethod inAdvancedSettingsSerializerneeds to be updated to handle this new field, as pointed out in the previous comment.After making the suggested changes, consider the following next steps:
- Update any relevant documentation to explain the purpose and usage of the new
je_single_credit_lineconfiguration.- Ensure that any frontend components or API consumers are updated to handle this new field.
- Add appropriate unit tests to cover the new configuration option and its handling in the serializer.
These changes appear to be part of a larger feature implementation. Make sure to coordinate with the frontend team (if applicable) to ensure a smooth integration of this new configuration option throughout the application.
apps/workspaces/models.py (1)
183-183: LGTM! Consider enhancing the help text.The addition of the
je_single_credit_linefield is consistent with the existing code style and aligns with the PR objective. Good job on following the naming conventions and using appropriate field type and default value.To improve clarity, consider expanding the help text to provide more context about the feature's purpose and implications. For example:
je_single_credit_line = models.BooleanField( default=False, help_text='When enabled, creates a single credit line for journal entries instead of multiple lines.' )This change would make it easier for other developers to understand the feature's impact without needing to refer to external documentation.
apps/fyle/tasks.py (6)
Line range hint
18-18: Consolidate duplicate imports from.helpersmoduleAt line 18,
construct_expense_filter_queryis imported again along with additional functions. Since it's already imported at line 17, consider consolidating these into a single import statement to avoid redundancy.Apply this diff to consolidate the imports:
-from .helpers import construct_expense_filter_query -from .helpers import construct_expense_filter_query, get_filter_credit_expenses, get_source_account_type, get_fund_source, handle_import_exception +from .helpers import ( + construct_expense_filter_query, + get_filter_credit_expenses, + get_source_account_type, + get_fund_source, + handle_import_exception +)
Line range hint
42-46: Handle potentialConfiguration.DoesNotExistexceptionIn the
get_task_log_and_fund_sourcefunction, fetchingConfigurationwithout exception handling may raise aConfiguration.DoesNotExistexception if the configuration is missing for the given workspace. Consider adding exception handling to manage this scenario gracefully.Apply this diff to handle the exception:
def get_task_log_and_fund_source(workspace_id: int): task_log, _ = TaskLog.objects.update_or_create( workspace_id=workspace_id, type='FETCHING_EXPENSES', defaults={ 'status': 'IN_PROGRESS' } ) - configuration = Configuration.objects.get(workspace_id=workspace_id) + try: + configuration = Configuration.objects.get(workspace_id=workspace_id) + except Configuration.DoesNotExist: + logger.error('Configuration does not exist for workspace_id: %s', workspace_id) + task_log.status = 'FAILED' + task_log.detail = {'message': 'Configuration not found'} + task_log.save() + return task_log, [], None fund_source = [] if configuration and configuration.reimbursable_expenses_object: fund_source.append('PERSONAL') if configuration and configuration.corporate_credit_card_expenses_object: fund_source.append('CCC') return task_log, fund_source, configuration
Line range hint
154-154: EnsureConfigurationexists before usageIn the
group_expenses_and_savefunction, fetchingConfigurationwithout handling possible exceptions may lead to errors if the configuration is absent. Similar to earlier functions, consider implementing exception handling forConfiguration.DoesNotExistto prevent runtime exceptions.Apply this diff to handle the exception:
def group_expenses_and_save(expenses: List[Dict], task_log: TaskLog, workspace: Workspace): expense_objects = Expense.create_expense_objects(expenses, workspace.id) expense_filters = ExpenseFilter.objects.filter(workspace_id=workspace.id).order_by('rank') - configuration : Configuration = Configuration.objects.get(workspace_id=workspace.id) + try: + configuration: Configuration = Configuration.objects.get(workspace_id=workspace.id) + except Configuration.DoesNotExist: + logger.error('Configuration not found for workspace_id: %s', workspace.id) + task_log.status = 'FAILED' + task_log.detail = {'message': 'Configuration not found'} + task_log.save() + return filtered_expenses = expense_objects if expense_filters: expenses_object_ids = [expense_object.id for expense_object in expense_objects]
Line range hint
171-172: Missingasync_post_accounting_export_summarycall after marking expenses as skippedIn the
create_expense_groupsfunction, after marking expenses as skipped when expense filters are applied, theasync_post_accounting_export_summaryfunction is not called. This may lead to the export summary not being posted for the workspace, causing inconsistencies.Apply this diff to ensure the export summary is posted:
if expense_filters: expenses_object_ids = [expense_object.id for expense_object in expense_objects] final_query = construct_expense_filter_query(expense_filters) Expense.objects.filter(final_query, id__in=expenses_object_ids, expensegroup__isnull=True, org_id=workspace.fyle_org_id).update(is_skipped=True) filtered_expenses = Expense.objects.filter(is_skipped=False, id__in=expenses_object_ids, expensegroup__isnull=True, org_id=workspace.fyle_org_id) + async_post_accounting_export_summary(workspace.fyle_org_id, workspace.id) else: filtered_expenses = expense_objects
Line range hint
207-207: Correct list wrapping when filteringexpense_groupsIn the
import_and_export_expensesfunction, wrappingexpense_idswith brackets creates a nested list, which may lead to incorrect filtering. Remove the extra brackets to fix the issue.Apply this diff to correct the filtering:
- expense_groups = ExpenseGroup.objects.filter(expenses__id__in=[expense_ids], workspace_id=workspace.id).distinct('id').values('id') + expense_groups = ExpenseGroup.objects.filter(expenses__id__in=expense_ids, workspace_id=workspace.id).distinct('id').values('id')
Line range hint
154-154: Adjust type hint syntax for variable annotationAt line 154, the type hint for
configurationhas an extra space before the colon. The correct syntax doesn't include a space before the colon in variable annotations.Apply this diff to correct the syntax:
- configuration : Configuration = Configuration.objects.get(workspace_id=workspace.id) + configuration: Configuration = Configuration.objects.get(workspace_id=workspace.id)tests/test_netsuite/test_connector.py (2)
67-67: Avoid hard-coding workspace IDsLine 67 hard-codes
workspace_id=1, which can reduce the flexibility and reusability of the test. Consider using a variable or fixture to obtain theworkspace_iddynamically.You might adjust the code as follows:
-configuration = Configuration.objects.get(workspace_id=1) +configuration = Configuration.objects.get(workspace_id=netsuite_credentials.workspace_id)
639-640: Reuse 'configuration' object to avoid redundant queriesLines 639 and 640 retrieve the
configurationobject, and immediately after,workspace_general_settingis fetched using the same query parameters. To optimize performance, reuse theconfigurationobject instead of making an additional database call.You can modify the code as follows:
-configuration = Configuration.objects.get(workspace_id=workspace_id) -workspace_general_setting = Configuration.objects.get(workspace_id=workspace_id) -workspace_general_setting.change_accounting_period = True -workspace_general_setting.save() +configuration.change_accounting_period = True +configuration.save()apps/netsuite/connector.py (1)
2088-2092: Enhance the docstring for better clarityThe docstring in
construct_single_itemized_credit_lineis brief. Providing more detailed documentation about the method's purpose, parameters, and return value can improve readability and maintainability.Consider updating the docstring as follows:
def construct_single_itemized_credit_line(self, journal_entry_lineitems: List[JournalEntryLineItem]): """ - Create journal entry line items for single credit line + Constructs journal entry line items with a single aggregated credit line. + Args: + journal_entry_lineitems (List[JournalEntryLineItem]): A list of journal entry line items. + Returns: + List[Dict]: A list of dictionaries representing the journal entry lines with aggregated credit amounts. """
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- apps/fyle/models.py (1 hunks)
- apps/fyle/tasks.py (1 hunks)
- apps/netsuite/connector.py (4 hunks)
- apps/netsuite/models.py (0 hunks)
- apps/netsuite/tasks.py (1 hunks)
- apps/workspaces/apis/advanced_settings/serializers.py (1 hunks)
- apps/workspaces/migrations/0039_configuration_je_single_credit_line.py (1 hunks)
- apps/workspaces/models.py (1 hunks)
- tests/test_netsuite/test_connector.py (2 hunks)
💤 Files with no reviewable changes (1)
- apps/netsuite/models.py
🧰 Additional context used
🔇 Additional comments (6)
apps/workspaces/migrations/0039_configuration_je_single_credit_line.py (1)
1-18: LGTM: Migration file structure is correct.The migration file follows the standard Django migration file structure, including the necessary imports and the Migration class. This adheres to Django best practices for database schema changes.
apps/netsuite/tasks.py (1)
701-701: LGTM! Verify function calls across the codebase.The addition of the
configurationparameter tonetsuite_connection.post_journal_entryenhances the flexibility of journal entry creation. This change looks good and aligns with the function's purpose.To ensure this change doesn't break existing functionality, please run the following script to check for any calls to
create_journal_entrythat might need updating:tests/test_netsuite/test_connector.py (2)
647-647: Verify all 'post_journal_entry' method calls include the 'configuration' parameterThe
post_journal_entrymethod now expects aconfigurationparameter. Ensure that all invocations of this method in the codebase have been updated to pass this parameter.Run the following script to check for any calls missing the
configurationparameter:#!/bin/bash # Description: Find calls to post_journal_entry without the 'configuration' argument. # Test: Search for function calls with two arguments. # Expect: No matches should be found. ast-grep --lang python --pattern '$_.post_journal_entry($_, $_)'
70-70: Ensure method calls are updated with the new 'configuration' parameterThe method
_NetSuiteConnector__construct_journal_entrynow requires aconfigurationparameter. Verify that all calls to this method throughout the codebase have been updated to include this parameter.Run the following script to confirm that there are no calls missing the
configurationparameter:✅ Verification successful
All calls to
__construct_journal_entryinclude theconfigurationparameterNo instances of
__construct_journal_entrywith only two arguments were found in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find calls to __construct_journal_entry lacking the 'configuration' argument. # Test: Search for function calls with two arguments. # Expect: No matches should be found. ast-grep --lang python --pattern '$_.__construct_journal_entry($_, $_)'Length of output: 219
Script:
#!/bin/bash # Description: Find calls to __construct_journal_entry lacking the 'configuration' argument. # Corrected ast-grep pattern to match function calls with two arguments. ast-grep --lang python --pattern '$_.__construct_journal_entry($_, $_)'Length of output: 73
Script:
#!/bin/bash # Description: Find calls to __construct_journal_entry with exactly two arguments, indicating missing 'configuration'. # Refined ast-grep pattern to accurately match function calls with two arguments. ast-grep --lang python --pattern '$_.__construct_journal_entry($_, $_)'Length of output: 73
Script:
#!/bin/bash # Description: Find calls to __construct_journal_entry with exactly two arguments, indicating missing 'configuration'. # Using ripgrep to search for __construct_journal_entry calls with two arguments. rg '__construct_journal_entry\s*\(\s*[^,]+,\s*[^)]+\s*\)' --context 2Length of output: 1748
apps/netsuite/connector.py (2)
Line range hint
2170-2184: Check for missing 'org_id' when calling 'construct_single_itemized_credit_line'In the conditional block within
__construct_journal_entry, whenconfiguration.je_single_credit_lineisTrue, you're callingconstruct_single_itemized_credit_linewithout passingorg_id. If this method requiresorg_id, it might lead to errors.Please ensure that
construct_single_itemized_credit_linefunctions correctly withoutorg_idor modify the call if needed.
2254-2256:⚠️ Potential issueAvoid reassigning the 'configuration' parameter inside the method
In
post_journal_entry, you acceptconfigurationas a parameter but then reassign it within the method. This overrides the passed-in value and may lead to confusion or unintended behavior.Consider removing the reassignment if it's unnecessary:
def post_journal_entry(self, journal_entry: JournalEntry, journal_entry_lineitems: List[JournalEntryLineItem], configuration: Configuration): - configuration = Configuration.objects.get(workspace_id=self.workspace_id)If you need to ensure that
configurationis always fetched based onself.workspace_id, you might omit it from the parameters:def post_journal_entry(self, journal_entry: JournalEntry, - journal_entry_lineitems: List[JournalEntryLineItem], configuration: Configuration): + journal_entry_lineitems: List[JournalEntryLineItem]): configuration = Configuration.objects.get(workspace_id=self.workspace_id)Likely invalid or redundant comment.
| operations = [ | ||
| migrations.AddField( | ||
| model_name='configuration', | ||
| name='je_single_credit_line', | ||
| field=models.BooleanField(default=False, help_text='Journal Entry Single Credit Line'), | ||
| ), | ||
| ] |
There was a problem hiding this comment.
🛠️ 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.
| @@ -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') | |||
There was a problem hiding this comment.
💡 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.pytests/test_workspaces/test_apis/test_export_settings/test_views.pytests/test_netsuite/test_views.pytests/test_netsuite/test_tasks.pytests/test_netsuite/conftest.pyscripts/python/parse-error.pytests/conftest.pyapps/workspaces/models.pyapps/netsuite/models.pyapps/netsuite/connector.pyapps/netsuite/queue.pyapps/netsuite/tasks.pyapps/netsuite/migrations/0002_expensereports_journalentries.pyapps/tasks/models.pyapps/workspaces/actions.pyapps/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
apps/fyle/models.py
Outdated
| @@ -184,6 +184,7 @@ def create_expense_objects(expenses: List[Dict], workspace_id, skip_update: bool | |||
| if expense_data_to_append: | |||
| defaults.update(expense_data_to_append) | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Usage of update_or_create in a loop may affect performance.
The use of Expense.objects.update_or_create() inside a loop can lead to multiple database hits, which may affect performance when processing a large number of expenses. Consider optimizing this by batching operations.
You might refactor the code to use bulk operations. For instance, collect the expenses to be created and use bulk_create for new expenses, and handle updates separately with bulk_update. This can significantly reduce the number of database queries.
apps/netsuite/connector.py
Outdated
| def construct_single_itemized_credit_line(self, 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 | ||
| 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, | ||
| '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 Credit 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 | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider using tuples as dictionary keys instead of string concatenation
In the construct_single_itemized_credit_line method, you're concatenating account_ref and entity_id into a string to use as a dictionary key. This approach can lead to issues if account_ref or entity_id contain the separator ::::. A more robust solution is to use tuples as dictionary keys.
Here's how you can modify the code:
- line_id = '{account_ref}::::{entity_id}'.format(account_ref=account_ref, entity_id=entity_id)
...
- if line_id in distinct_line_ids:
+ key = (account_ref, entity_id)
+ if key in distinct_line_ids:
distinct_line_ids[key] += line.amount
...
- 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.
| def construct_single_itemized_credit_line(self, 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 | |
| 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, | |
| '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 Credit 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_single_itemized_credit_line(self, 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 | |
| key = (account_ref, entity_id) | |
| if key in distinct_line_ids: | |
| distinct_line_ids[key] += line.amount | |
| else: | |
| distinct_line_ids[key] = line.amount | |
| 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 Credit 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 |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (18)
apps/workspaces/migrations/0039_configuration_je_single_credit_line.py (1)
1-18: Migration file implements the new 'je_single_credit_line' feature correctly.This migration file successfully adds the 'je_single_credit_line' boolean field to the Configuration model, which aligns with the PR objective of implementing a single credit line for journal entries. The migration is properly structured and follows Django's best practices.
Considerations for the development team:
- Ensure that any existing configurations are handled appropriately, given the new field's default value of False.
- Update any relevant views, forms, or serializers to include this new field where necessary.
- Consider adding appropriate documentation or updating user guides to explain the purpose and impact of this new configuration option.
apps/workspaces/apis/advanced_settings/serializers.py (2)
Line range hint
170-224: Update theupdatemethod to handle the newje_single_credit_linefield.The
updatemethod inAdvancedSettingsSerializerneeds to be modified to include the newje_single_credit_linefield. Currently, this field is not being handled in theConfiguration.objects.update_or_createcall, which means it won't be updated when changes are made through this serializer.Add the new field to the
defaultsdictionary in theupdatemethod:configuration_instance, _ = Configuration.objects.update_or_create( workspace=instance, defaults={ 'sync_fyle_to_netsuite_payments': configurations.get('sync_fyle_to_netsuite_payments'), 'sync_netsuite_to_fyle_payments': configurations.get('sync_netsuite_to_fyle_payments'), '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') } )
Line range hint
1-224: Summary of changes and recommendations
- The new
je_single_credit_linefield has been correctly added to theConfigurationSerializer.- The
AdvancedSettingsSerializerneeds to be updated to handle this new field in itsupdatemethod.- Consider adding appropriate validation for the new field if necessary.
- Update any related views or other components that might be affected by this new field.
- Ensure that appropriate unit tests are added or updated to cover the new field and its handling in the serializers.
To maintain consistency and prevent future oversights, consider implementing a more dynamic approach to handle configuration fields. This could involve using a common set of fields that is shared between the
ConfigurationSerializerand theupdatemethod inAdvancedSettingsSerializer, which would automatically propagate any new fields added to the serializer.apps/workspaces/models.py (2)
182-182: LGTM! Consider adding a migration for the field change.The update to the
name_in_journal_entryfield looks good. Adding a default value and explicit choices improves the field's definition and aligns with the PR's objective.Don't forget to create and run a migration to apply this field change to the database schema.
182-183: Summary: Changes look good, consider broader implementation impact.The modifications to the
Configurationmodel, including the updatedname_in_journal_entryfield and the newje_single_credit_linefield, are well-implemented and align with the PR objective of introducing a single credit line for journal entries.To fully implement this feature:
- Create a database migration for these model changes.
- Update relevant serializers, forms, and views to incorporate the new field.
- Modify the journal entry creation logic to use the
je_single_credit_lineconfiguration.- Add or update unit tests to cover the new functionality.
- Consider updating the user interface to allow toggling of this new option.
- Update documentation to reflect this new feature and its implications.
Would you like assistance with any of these follow-up tasks?
apps/netsuite/tasks.py (2)
701-701: Consider the wider implications of adding theconfigurationparameterWhile the change to
post_journal_entryis straightforward, it may have broader implications:
- Ensure that the
NetSuiteConnectorclass has been updated to handle this new parameter in itspost_journal_entrymethod.- Check if there are any other calls to
create_journal_entryin the codebase that need to be updated to pass theconfigurationparameter.- Update any relevant tests for
create_journal_entryand related functionality to include the new parameter.To fully implement this change, consider the following steps:
- Update the
NetSuiteConnectorclass to use theconfigurationparameter in itspost_journal_entrymethod.- Modify any other functions that call
create_journal_entryto pass theconfigurationparameter.- Update unit tests and integration tests related to journal entry creation.
- Update the documentation to reflect this change in the function signature and usage.
701-701: Summary of changes and next stepsThe addition of the
configurationparameter to thepost_journal_entrycall in thecreate_journal_entryfunction is a small but potentially impactful change. It appears to be part of a larger effort to make the journal entry creation process more configurable.To ensure a smooth implementation of this change:
- Verify that all necessary updates have been made to the
NetSuiteConnectorclass and any other relevant parts of the codebase.- Update all tests related to journal entry creation.
- Review and update the documentation to reflect this change.
- Consider adding a brief comment explaining the purpose of the
configurationparameter and how it affects the journal entry creation process.Consider adding a brief comment above the
post_journal_entrycall to explain the purpose and impact of theconfigurationparameter:# Pass configuration to allow for dynamic journal entry creation based on workspace settings created_journal_entry = netsuite_connection.post_journal_entry( journal_entry_object, journal_entry_lineitems_objects, configuration )apps/fyle/tasks.py (8)
Line range hint
22-23: Remove duplicate import ofconstruct_expense_filter_query.The function
construct_expense_filter_queryis imported twice from.helpers. Please remove the duplicate import to avoid redundancy.Apply this diff to fix the issue:
-from .helpers import construct_expense_filter_query from .helpers import construct_expense_filter_query, get_filter_credit_expenses, get_source_account_type, get_fund_source, handle_import_exception
Line range hint
35-35: Add return type annotation toget_task_log_and_fund_source.For consistency and clarity, add a return type annotation to the function
get_task_log_and_fund_source.Apply this diff to update the function signature:
def get_task_log_and_fund_source(workspace_id: int): + -> Tuple[TaskLog, List[str], Configuration]:Ensure
Tupleis imported fromtyping. If not already imported, modify the import statement:-from typing import List, Dict +from typing import List, Dict, Tuple
Line range hint
67-69: Remove unuseduserparameter from docstring.The docstring mentions
:param user: User email, but the function does not accept auserparameter. Please update the docstring for accuracy.Apply this diff to correct the docstring:
def schedule_expense_group_creation(workspace_id: int): """ Schedule Expense group creation :param workspace_id: Workspace id - :param user: User email :return: None """
Line range hint
81-130: Consider refactoring to eliminate code duplication betweencreate_expense_groupsandgroup_expenses_and_save.Both functions perform similar operations such as creating expense objects, filtering expenses, and creating expense groups. Refactoring common logic into shared helper functions would improve maintainability and reduce code redundancy.
Line range hint
101-115: Correct the typo in exception messages: "occured" should be "occurred".There is a typo in the exception messages and log statements. The word "occured" should be corrected to "occurred" for proper spelling.
Apply this diff to fix the typos:
# Exception handling for RetryException -logger.info('Fyle Retry Exception occured in workspace_id: %s', workspace_id) +logger.info('Fyle Retry Exception occurred in workspace_id: %s', workspace_id) task_log.detail = { - 'message': 'Fyle Retry Exception occured' + 'message': 'Fyle Retry Exception occurred' } # Exception handling for InternalServerError -logger.info('Fyle Internal Server Error occured in workspace_id: %s', workspace_id) +logger.info('Fyle Internal Server Error occurred in workspace_id: %s', workspace_id) task_log.detail = { - 'message': 'Fyle Internal Server Error occured' + 'message': 'Fyle Internal Server Error occurred' }
Line range hint
129-129: Remove space around colon in type annotation.According to PEP 8 style guidelines, there should be no spaces around the colon in type annotations.
Apply this diff to fix the formatting:
-configuration : Configuration = Configuration.objects.get(workspace_id=workspace.id) +configuration: Configuration = Configuration.objects.get(workspace_id=workspace.id)
Line range hint
182-189: Ensuretask_logis defined before exception handlers use it.If an exception occurs before
task_logis defined, referencingtask_login the exception handlers will raise an error. Consider definingtask_logbefore thetryblock to prevent this issue.Apply this diff to initialize
task_logbefore entering thetryblock:+task_log = None try: with transaction.atomic(): task_log, _ = TaskLog.objects.update_or_create( workspace_id=workspace.id, type='FETCHING_EXPENSES', defaults={'status': 'IN_PROGRESS'} )Update the exception handlers to check if
task_logis notNonebefore using it:except Configuration.DoesNotExist: logger.info('Configuration not found %s', workspace.id) + if task_log: task_log.detail = {'message': 'Configuration not found'} task_log.status = 'FAILED' task_log.save() + else: + logger.error('TaskLog not defined when handling Configuration.DoesNotExist exception')
Line range hint
197-198: Fix incorrect usage of list when filteringexpense_groups.The
expense_idsvariable is already a list of IDs. Wrapping it in another list leads to incorrect query. Remove the extra brackets to fix the query.Apply this diff to correct the filter:
expense_ids = Expense.objects.filter(report_id=report_id, org_id=org_id).values_list('id', flat=True) -expense_groups = ExpenseGroup.objects.filter(expenses__id__in=[expense_ids], workspace_id=workspace.id).distinct('id').values('id') +expense_groups = ExpenseGroup.objects.filter(expenses__id__in=expense_ids, workspace_id=workspace.id).distinct('id').values('id')apps/fyle/models.py (2)
Line range hint
249-263: Avoid modifyinggroup_fieldswhile iterating over itIn the
_group_expensesfunction, modifying thegroup_fieldslist while iterating over it can lead to unexpected behavior or skipped elements. Specifically, removing items from a list during iteration is not safe.Consider iterating over a copy of the list or collecting the valid fields separately to avoid this issue.
Apply this diff to fix the problem:
def _group_expenses(expenses, group_fields, workspace_id): expense_ids = list(map(lambda expense: expense.id, expenses)) expenses = Expense.objects.filter(id__in=expense_ids).all() custom_fields = {} + valid_group_fields = group_fields.copy() for field in group_fields: if field.lower() not in ALLOWED_FIELDS: - group_fields.pop(group_fields.index(field)) + valid_group_fields.remove(field) field_obj = ExpenseAttribute.objects.filter( workspace_id=workspace_id, attribute_type=field.upper() ).first() if field_obj: custom_fields[field_obj.attribute_type.lower()] = KeyTextTransform( field_obj.display_name, 'custom_properties' ) - expense_groups = list( - expenses.values(*group_fields, **custom_fields).annotate(total=Count('*'), expense_ids=ArrayAgg('id')) - ) + expense_groups = list( + expenses.values(*valid_group_fields, **custom_fields).annotate(total=Count('*'), expense_ids=ArrayAgg('id')) + ) return expense_groups
Line range hint
413-414: Remove duplicate assignment toemployee_nameThe assignment to
employee_nameis duplicated consecutively:employee_name = Expense.objects.filter( id__in=expense_group['expense_ids'] ).first().employee_name employee_name = Expense.objects.filter( id__in=expense_group['expense_ids'] ).first().employee_nameThis redundancy is unnecessary and may cause confusion. Remove the duplicate assignment to clean up the code.
Apply this diff to fix the redundancy:
employee_name = Expense.objects.filter( id__in=expense_group['expense_ids'] ).first().employee_name -employee_name = Expense.objects.filter( - id__in=expense_group['expense_ids'] -).first().employee_nameapps/netsuite/connector.py (1)
2150-2150: Provide More Descriptive Memo FieldThe memo
'Total Credit Amount'may be too generic. Consider including contextual information, such as account names or transaction details, to make the memo more informative.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- apps/fyle/models.py (1 hunks)
- apps/fyle/tasks.py (1 hunks)
- apps/netsuite/connector.py (4 hunks)
- apps/netsuite/models.py (0 hunks)
- apps/netsuite/tasks.py (1 hunks)
- apps/workspaces/apis/advanced_settings/serializers.py (1 hunks)
- apps/workspaces/migrations/0039_configuration_je_single_credit_line.py (1 hunks)
- apps/workspaces/models.py (1 hunks)
- tests/test_netsuite/test_connector.py (2 hunks)
💤 Files with no reviewable changes (1)
- apps/netsuite/models.py
🧰 Additional context used
🔇 Additional comments (13)
apps/workspaces/migrations/0039_configuration_je_single_credit_line.py (2)
6-10: Migration class and dependencies look good.The migration class is correctly defined, and the dependency on the previous migration '0038_configuration_allow_intercompany_vendors' is properly specified. This ensures that migrations are applied in the correct order.
13-17: New field 'je_single_credit_line' added correctly.The AddField operation is properly implemented:
- The field name 'je_single_credit_line' aligns with the migration filename.
- It's added to the 'configuration' model as intended.
- The field type (BooleanField) is appropriate for a flag.
- A default value of False is set, which is a good practice for backward compatibility.
- The help text provides a brief description of the field's purpose.
To ensure consistency, let's verify if this new field is properly reflected in the model definition:
✅ Verification successful
Verified: The 'je_single_credit_line' field is properly defined in the Configuration model.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the new field in the Configuration model # Test: Search for the field definition in the model file rg --type python 'je_single_credit_line' apps/workspaces/models.pyLength of output: 97
Script:
#!/bin/bash # Description: Retry verifying the new field in the Configuration model with correct file type # Corrected Test: Search for the field definition in the model file using the correct file type rg --type py 'je_single_credit_line' apps/workspaces/models.pyLength of output: 171
apps/workspaces/apis/advanced_settings/serializers.py (1)
34-35: LGTM! Verify update method consistency.The addition of the
je_single_credit_linefield to theConfigurationSerializeris correct. However, ensure that theupdatemethod in theAdvancedSettingsSerializeris updated to handle this new field.To verify the update method, run the following script:
apps/workspaces/models.py (1)
183-183: LGTM! New field aligns with PR objective.The addition of the
je_single_credit_linefield is well-implemented and directly supports the PR's goal of introducing a single credit line for journal entries.To ensure this change is properly integrated:
- Create and run a migration for this new field.
- Update any relevant forms or serializers to include this new field.
- Modify the business logic in views or services to utilize this new configuration option.
Would you like me to assist with generating the migration file or updating related components?
apps/fyle/tasks.py (1)
Line range hint
107-115: Verify consistency oftask_log.statusvalues in exception handling.In the exception handler for
RetryException,task_log.statusis set to'FATAL', whereas forInternalServerError, it is set to'FAILED'. Please verify if this is intentional and aligns with the expected error handling strategy.tests/test_netsuite/test_connector.py (4)
67-67: Configuration Retrieval Added totest_construct_journal_entryThe test case now includes retrieving the
configurationobject, which is necessary due to the updated method signature of__construct_journal_entrythat requires a configuration parameter.
70-70: Updated Method Call with Configuration ParameterThe call to
__construct_journal_entryhas been correctly updated to include theconfigurationparameter, aligning with the updated method signature.
639-640: Configuration Retrieval Added totest_post_journal_entry_exceptionThe
configurationobject is retrieved in this test case to support the updatedpost_journal_entrymethod, which now requires a configuration parameter.
647-647: Updatedpost_journal_entryCall with Configuration ParameterThe call to
post_journal_entryhas been correctly updated to include theconfigurationparameter, ensuring the test case aligns with the modified method signature.apps/netsuite/connector.py (4)
2115-2132: Populate Department, Location, and Class IDs if ApplicableThe
'internalId'fields for'department','location', and'class'are currently set toNone. If these dimensions are relevant to your journal entries, consider assigning appropriate IDs to these fields to ensure accurate reporting.Would you like to confirm whether these fields should have valid IDs? Updating them may enhance the accuracy of your financial data.
2170-2170: Ensure Consistent Parameter Usage in Method SignaturesThe
__construct_journal_entrymethod signature has been updated to include theconfigurationparameter. Verify that all calls to this method have been updated accordingly to prevent potential errors.
2260-2260: Pass Configuration Parameter CorrectlyEnsure that the
configurationparameter is passed correctly to the__construct_journal_entrymethod to maintain consistent behavior.
2254-2255:⚠️ Potential issueRemove Redundant Configuration Retrieval
The line
configuration = Configuration.objects.get(workspace_id=self.workspace_id)is unnecessary becauseconfigurationis already being passed as a parameter to thepost_journal_entrymethod.Apply this diff to remove the redundant code:
- configuration = Configuration.objects.get(workspace_id=self.workspace_id)Likely invalid or redundant comment.
| @@ -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 | |||
There was a problem hiding this comment.
💡 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 tocreate_journal_entryneed to include theconfigurationparameter.tests/test_netsuite/test_tasks.py: Tests invokingcreate_journal_entryshould be updated to pass theconfigurationparameter.
🔗 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
apps/netsuite/connector.py
Outdated
| 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) |
There was a problem hiding this comment.
Control Flow Based on Configuration Should Handle Edge Cases
When using configuration.je_single_credit_line, ensure that both the True and False branches are thoroughly tested, and consider what happens if the configuration is missing or None.
Consider adding a check to handle cases where configuration might not have the je_single_credit_line attribute.
| for line_id, amount in distinct_line_ids.items(): | ||
| account_ref, entity_id = line_id.split('::::') | ||
| lineitem = { | ||
| 'account': { | ||
| 'name': None, |
There was a problem hiding this comment.
🛠️ 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.
| 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, |
| 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 |
There was a problem hiding this comment.
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.amountCommittable suggestion was skipped due to low confidence.
apps/fyle/models.py
Outdated
| @@ -184,6 +184,7 @@ def create_expense_objects(expenses: List[Dict], workspace_id, skip_update: bool | |||
| if expense_data_to_append: | |||
| defaults.update(expense_data_to_append) | |||
|
|
|||
|
|
|||
apps/netsuite/connector.py
Outdated
| 'line': None, | ||
| 'lineTaxCode': None, | ||
| 'lineTaxRate': None, | ||
| 'memo': 'Total Credit Amount', |
There was a problem hiding this comment.
One question should we also add the unique identifier for this
For example we have 2 credit lines
123-ACB
456-XYZ
If both will have same memo then there is no referral to memo
We should add something like Total Credit Amount for ACC/Entity
There was a problem hiding this comment.
account and entity info is present in same line, hence didn't add - will check with Hemanth once
* test: add tests for single credit line <> JE * test: add tests * API fix * fix memo
ashwin1111
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
ashwin1111
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
|
Description
Please add PR description here, add screenshots if needed
Clickup
https://app.clickup.com/t/86cwrqeuy
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests