Journal Entry Export #52
Conversation
WalkthroughThe updates primarily involve enhancements to the accounting export functionality, with a focus on refining the data models and export processes for accounting data. Changes include adjustments to model choices, error handling, and data export tasks, as well as the introduction of new models for managing journal entries and purchase invoices. Additionally, there's a new emphasis on syncing and updating settings related to Business Central, and the handling of employee and expense data has been improved. Changes
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
ruuushhh
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
ruuushhh
had a problem deploying
to
CI Environment
GitHub Actions
Failure
ruuushhh
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
ruuushhh
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
ruuushhh
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
ruuushhh
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.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files selected for processing (28)
- apps/accounting_exports/migrations/0001_initial.py (1 hunks)
- apps/accounting_exports/models.py (2 hunks)
- apps/business_central/actions.py (1 hunks)
- apps/business_central/exceptions.py (1 hunks)
- apps/business_central/exports/accounting_export.py (1 hunks)
- apps/business_central/exports/base_model.py (1 hunks)
- apps/business_central/exports/helpers.py (1 hunks)
- apps/business_central/exports/journal_entry/models.py (1 hunks)
- apps/business_central/exports/journal_entry/queues.py (1 hunks)
- apps/business_central/exports/journal_entry/tasks.py (1 hunks)
- apps/business_central/exports/purchase_invoice/models.py (1 hunks)
- apps/business_central/migrations/0001_initial.py (1 hunks)
- apps/business_central/models.py (1 hunks)
- apps/business_central/serializers.py (1 hunks)
- apps/business_central/utils.py (6 hunks)
- apps/fyle/migrations/0001_initial.py (1 hunks)
- apps/fyle/models.py (2 hunks)
- apps/fyle/tasks.py (2 hunks)
- apps/mappings/migrations/0001_initial.py (1 hunks)
- apps/workspaces/migrations/0002_alter_workspace_ms_business_central_accounts_last_synced_at_and_more.py (1 hunks)
- apps/workspaces/models.py (4 hunks)
- apps/workspaces/serializers.py (3 hunks)
- apps/workspaces/tasks.py (1 hunks)
- apps/workspaces/urls.py (1 hunks)
- apps/workspaces/views.py (2 hunks)
- ms_business_central_api/exceptions.py (1 hunks)
- ms_business_central_api/models/fields.py (2 hunks)
- requirements.txt (1 hunks)
Files skipped from review due to trivial changes (2)
- apps/business_central/models.py
- apps/fyle/models.py
Additional comments: 63
ms_business_central_api/exceptions.py (1)
- 7-22: The
BulkErrorclass is well-defined and documented. It correctly inherits from the baseExceptionclass and overrides the__str__method to return the message representation. This should work well for representing bulk operation errors in the application.requirements.txt (2)
31-31: The version of
ms-dynamics-business-central-sdkhas been updated to1.3.3. Ensure that this version is compatible with the application and that all necessary testing has been performed to confirm that the update does not introduce any breaking changes.35-35: The version of
fyle-accounting-mappingshas been updated to1.29.0. Similar to the previous comment, verify that this version is compatible with the application and that all necessary testing has been performed to confirm that the update does not introduce any breaking changes.apps/business_central/actions.py (1)
- 6-21: The
update_accounting_export_summaryfunction correctly updates the summary counts for failed and successful accounting exports. It's good to see that the function is concise and uses Django's ORM effectively to perform the necessary queries and updates.apps/workspaces/urls.py (2)
9-9: The
TriggerExportsViewhas been added to the list of imported views. This is necessary for the new URL path that has been introduced.17-17: A new URL path
<int:workspace_id>/exports/trigger/has been added, which is set to use theTriggerExportsView. This change aligns with the PR objectives to enhance the application's functionality for triggering exports. Ensure that the corresponding view handles the requests as expected and that appropriate permissions are enforced.apps/business_central/exports/journal_entry/queues.py (1)
- 10-45: The
check_accounting_export_and_start_importfunction is designed to enqueue accounting exports for processing. It filters out exports that are already in progress, complete, or queued, and only includes those that are related to journal entries and have not been exported yet. The function then creates a chain of tasks to be executed. This is a good use of Django Q's chaining feature to manage task execution.apps/mappings/migrations/0001_initial.py (1)
- 17-36: The initial migration for the
mappingsapp is well-structured and includes all necessary fields for theImportLogmodel. The unique constraint on the combination ofworkspaceandattribute_typeis a good practice to avoid duplicate entries.apps/business_central/exports/accounting_export.py (1)
- 14-79: The
AccountingDataExporterclass provides a clear structure for subclasses to implement thepostmethod for exporting data to external systems. The use of Django's ORM and transactions is appropriate, and the error handling within thecreate_business_central_objectmethod is comprehensive. Ensure that subclasses properly implement thepostmethod and that the error handling is consistent with the application's standards.apps/business_central/exceptions.py (1)
- 16-80: The error handling functions and decorator in
exceptions.pyare well-implemented. They log the errors, update theAccountingExportobject's status and details, and callupdate_accounting_export_summaryto reflect the changes in the summary. This is a robust approach to handling exceptions that may occur during the export process.apps/fyle/tasks.py (1)
- 78-89: The
import_expensesfunction has been updated to include a transaction block, which is a good practice to ensure atomicity of operations. The changes also include the use of theconstruct_expense_filter_queryhelper function and the reordering of operations within the function. Ensure that the transaction block is tested thoroughly to confirm that it behaves as expected in all scenarios, including error conditions.apps/workspaces/views.py (2)
18-18: The import of
export_to_business_centralis necessary for the newTriggerExportsViewto function correctly. This change is consistent with the PR objectives to enhance the application's export functionality.144-154: The new
TriggerExportsViewclass with apostmethod has been introduced to trigger exports creation. This view calls theexport_to_business_centralfunction, which should encapsulate the logic for initiating the export process. Ensure that this view is properly secured and that only authorized users can trigger exports.apps/business_central/migrations/0001_initial.py (1)
- 19-58: The initial migration for the
business_centralapp correctly sets up theJournalEntryandJournalEntryLineItemsmodels with appropriate fields and database table names. The relationships between models are established using foreign keys, and the fields are well-documented with help texts.apps/business_central/serializers.py (1)
- 81-81: The addition of "COMPANY" to the list of fields in the
format_business_central_fieldsfunction is consistent with the PR objectives to enhance model structures and data handling. This change will affect the control flow and logic related to this function, so ensure that the inclusion of the "COMPANY" attribute type is tested and works as intended in the application.apps/business_central/exports/journal_entry/tasks.py (3)
13-17: The class
ExportJournalEntryis well-documented and extendsAccountingDataExporteras expected.24-28: The method
trigger_exportis concise and delegates the import process to another function. Ensure thatcheck_accounting_export_and_start_importis handling exceptions properly.64-87: The
postmethod is responsible for exporting the journal entry to Business Central. It uses thebulk_post_journal_lineitemsmethod from theBusinessCentralConnector. Ensure that thebulk_post_journal_lineitemsmethod has proper error handling and logging.apps/business_central/exports/purchase_invoice/models.py (4)
11-24: The
PurchaseInvoicemodel is well-defined with appropriate fields and help texts. The use of custom fields likeCustomDateTimeFieldandFloatNullFieldis noted.28-52: The class method
create_or_update_objectis used to create or update aPurchaseInvoiceobject. It uses theupdate_or_createmethod which is a good practice to avoid duplicates. Ensure that theupdate_or_createmethod is used correctly and handles exceptions as expected.55-69: The
PurchaseInvoiceLineitemsmodel is also well-defined with appropriate fields and help texts. It maintains a foreign key relationship with thePurchaseInvoicemodel.71-103: The class method
create_or_update_objectforPurchaseInvoiceLineitemsis used to create or update line items. It uses theupdate_or_createmethod which is a good practice to avoid duplicates. Ensure that theupdate_or_createmethod is used correctly and handles exceptions as expected.apps/business_central/exports/helpers.py (4)
17-30: The function
get_filtered_mappingretrieves a single mapping based on the provided criteria. It uses a dynamic filter which is a good practice for code reusability.33-78: The function
validate_accounting_exportchecks for missing mappings and raises aBulkErrorif any are found. It also createsErrorobjects for each missing mapping. Ensure that theBulkErrorexception is handled properly where this function is called.81-86: The function
resolve_errors_for_exported_accounting_exportmarks errors as resolved for a given accounting export. It's a straightforward update operation on theErrormodel.89-128: The function
load_attachmentsis responsible for loading attachments from Fyle and posting them to Business Central. Ensure that thepost_attachmentsmethod ofBusinessCentralConnectorhas proper error handling and logging.apps/business_central/exports/base_model.py (4)
11-17: The
BaseExportModelclass provides common fields and methods for export models. It's abstract and not meant to be instantiated directly.22-55: The method
get_expense_commentconstructs a comment for an expense. It uses theexpense_memo_structurefromAdvancedSettingto build the comment. Ensure that theexpense_memo_structureis configured correctly and that the method handles any potential exceptions.57-73: The method
get_total_amountcalculates the total amount of expenses associated with anAccountingExport. It uses Django'saggregatefunction which is a good practice for such calculations.75-98: The method
get_invoice_dateretrieves the invoice date from anAccountingExport. It checks for various keys in thedescriptionfield. Ensure that thedescriptionfield is structured correctly and contains the expected keys.apps/business_central/exports/journal_entry/models.py (4)
17-28: The
JournalEntrymodel is well-defined with appropriate fields and help texts. The use of custom fields likeCustomDateTimeFieldandFloatNullFieldis noted.33-62: The class method
create_or_update_objectis used to create or update aJournalEntryobject. It uses theupdate_or_createmethod which is a good practice to avoid duplicates. Ensure that theupdate_or_createmethod is used correctly and handles exceptions as expected.65-77: The
JournalEntryLineItemsmodel is also well-defined with appropriate fields and help texts. It maintains a foreign key relationship with theJournalEntrymodel.82-121: The class method
create_or_update_objectforJournalEntryLineItemsis used to create or update line items. It uses theupdate_or_createmethod which is a good practice to avoid duplicates. Ensure that theupdate_or_createmethod is used correctly and handles exceptions as expected.apps/accounting_exports/migrations/0001_initial.py (3)
20-41: The
AccountingExportmodel migration is created with appropriate fields and options. Ensure that the migration is applied without issues and that the database schema matches the model definitions.42-59: The
Errormodel migration is created with appropriate fields and options. Ensure that the migration is applied without issues and that the database schema matches the model definitions.60-77: The
AccountingExportSummarymodel migration is created with appropriate fields and options. Ensure that the migration is applied without issues and that the database schema matches the model definitions.apps/workspaces/serializers.py (3)
10-11: The imports of
AccountingExportSummaryandBusinessCentralConnectorare new additions to this file. Ensure that these classes are used appropriately within the serializers and that their methods are called with the correct parameters.67-67: The creation of an
AccountingExportSummaryinstance upon workspace creation is a new addition. This ensures that an export summary is available for each workspace, which is likely a part of the new feature set for accounting exports.113-122: The conditional block starting at line 113 checks if the export settings require a default journal entry folder and if it's not already set. If these conditions are met, it uses the
BusinessCentralConnectorto create one. This is a new feature that integrates with Business Central for journal entries. Ensure that thecreate_default_journal_entry_foldermethod handles exceptions and errors appropriately, as it interacts with an external system.apps/business_central/utils.py (5)
1-2: The import of the
base64andtypingmodules suggests new functionality related to encoding and type annotations. This is likely used in the new methods for handling attachments and bulk operations.75-75: The change in the
_sync_datamethod to useitem['number']if available, otherwiseitem['id'], for thedestination_idis a logical update that provides more specific identification for synced items when available.97-97: The replacement of
Workspace.objects.get(id=self.workspace_id)withself.connection.set_company_id(workspace.business_central_company_id)in various sync methods is a refactoring that centralizes the setting of the company ID for the Business Central connection. This is a cleaner approach and reduces code duplication.Also applies to: 109-109, 121-121, 133-133
140-150: The addition of the
create_default_journal_entry_foldermethod is part of the new functionality for handling journal entries. It's important to ensure that this method includes error handling for the external API call to Business Central.152-162: The
bulk_post_journal_lineitemsmethod is a new addition that allows for bulk posting of journal line items to Business Central. This method should be reviewed to ensure that it correctly formats the payload and handles any potential errors from the external service.apps/workspaces/tasks.py (3)
15-75: The
run_import_exporttask has been modified to include a new export mode parameter and to update theAccountingExportSummarywith the last exported time and mode. This is part of the new functionality to track export operations. Ensure that theexport_modeparameter is always passed correctly where this task is triggered.78-114: The
schedule_synctask has been updated to handle scheduling synchronization tasks. It now accepts additional parameters for email notifications. Ensure that the email lists are being used correctly and that the schedule is being set or deleted as per theschedule_enabledflag.117-168: The
export_to_business_centraltask is a new addition that seems to handle the export of expenses to Business Central. It's important to ensure that this task is scheduled and executed correctly, and that it handles any exceptions that may occur during the export process.apps/accounting_exports/models.py (2)
23-24: The
TYPE_CHOICESin theAccountingExportmodel have been updated to replace 'INVOICES' with 'PURCHASE_INVOICES' and 'DIRECT_COST' with 'JOURNAL_ENTRY'. This change reflects the new types of accounting data that the application will handle. Ensure that all references to these choices throughout the application are updated accordingly.126-128: The
create_accounting_exportfunction has been updated to change 'last_spent_at' to 'LAST_SPENT_AT'. This seems to be a case normalization change. Ensure that all parts of the application that rely on this field are updated to use the new case.apps/fyle/migrations/0001_initial.py (1)
- 10-87: The migration file creates new models
ExpenseFilterandExpense. Ensure that these models and their fields match the intended design and that the migration is consistent with the current state of the database schema.apps/workspaces/models.py (4)
133-136: The addition of
EXPORT_MODE_CHOICESwith options 'MANUAL' and 'AUTO' seems appropriate for specifying the mode of export. This is a straightforward enumeration that should not introduce issues.200-200: The addition of the
default_journal_entry_folder_idfield to theExportSettingmodel is consistent with the PR objectives to enhance export functionalities. Ensure that any existing data is migrated appropriately to accommodate this new field.Verification successful
The output from the executed script provides the contents of a migration file that includes the addition of the
default_journal_entry_folder_idfield to theExportSettingmodel. This confirms that a migration has been created to accommodate the new field in the database schema, which aligns with the PR objectives to enhance export functionalities.Based on the information provided, the review comment regarding the addition of the
default_journal_entry_folder_idfield and the need for a migration to accommodate this new field is verified as accurate.* 227-227: Renaming `schedule_start_datetime` to `start_datetime` in the `AdvancedSetting` model is a simplification that could improve readability. However, it's important to ensure that this change is reflected everywhere in the codebase, including any background tasks or services that rely on this field.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that a migration file exists for adding the default_journal_entry_folder_id field fd '0003_add_default_journal_entry_folder_id.py' --exec cat {}Length of output: 64
Script:
#!/bin/bash # Search for migration files related to the ExportSetting model that might include the new field fd '.*exportsetting.*\.py' apps/workspaces/migrations/ --exec cat {}Length of output: 72
Script:
#!/bin/bash # Search for any migration files in the apps/workspaces/migrations directory fd '.*\.py' apps/workspaces/migrations/ --exec cat {}Length of output: 15253
- 238-251: The addition of the
LastExportDetailmodel aligns with the PR objectives to store details of the last export. This model includes fields for timestamps, mode, and counts of exports, which are relevant for tracking export operations. Ensure that the fields are being populated correctly during the export process.apps/workspaces/migrations/0002_alter_workspace_ms_business_central_accounts_last_synced_at_and_more.py (8)
17-21: The migration operation to alter the
ms_business_central_accounts_last_synced_atfield in theWorkspacemodel to allownullvalues is consistent with the need to handle cases where accounts may not have been synced yet.22-25: The migration operation to alter the
onboarding_statefield in theWorkspacemodel to include a default function and allownullvalues is a good practice for ensuring data integrity and handling new workspaces that have not yet set an onboarding state.27-42: The creation of the
LastExportDetailmodel in the migration file is consistent with the changes in themodels.pyfile. It's important to ensure that this migration is applied without issues and that the table is created successfully in the database.44-57: The creation of the
ImportSettingmodel in the migration file is consistent with the changes in themodels.pyfile. Ensure that the default values for the boolean fields are set correctly and that they reflect the intended functionality.58-71: The creation of the
FyleCredentialmodel in the migration file is consistent with the changes in themodels.pyfile. Ensure that the refresh token and cluster domain fields are handled securely and that sensitive information is not logged or exposed.73-102: The creation of the
ExportSettingmodel in the migration file is consistent with the changes in themodels.pyfile. Ensure that the new fields and choices are consistent with the application's requirements and that any necessary data migrations are handled.Verification successful
The migration file
0002_alter_workspace_ms_business_central_accounts_last_synced_at_and_more.pyincludes the creation of theExportSettingmodel with the new fields as described in the review comment. There are no explicit data migration operations within this migration file, which suggests that either there is no existing data to migrate or that data migrations are handled elsewhere. The review comment is partially verified; the creation of theExportSettingmodel is consistent with the changes in themodels.pyfile, but without further context, it's not possible to fully verify the handling of data migrations.* 104-117: The creation of the `BusinessCentralCredentials` model in the migration file is consistent with the changes in the `models.py` file. Ensure that the handling of the refresh token and the `is_expired` flag is implemented correctly in the application logic.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new fields in ExportSetting are used correctly and that data migrations are handled. rg 'ExportSetting' --type pythonLength of output: 63
Script:
#!/bin/bash # Correcting the file type to 'py' and searching for 'ExportSetting' usage in Python files. rg 'ExportSetting' --type pyLength of output: 3658
Script:
#!/bin/bash # Search for data migration operations related to the 'ExportSetting' model. rg --type py 'ExportSetting' | rg 'migrations'Length of output: 182
Script:
#!/bin/bash # View the contents of the migration file that mentions 'ExportSetting'. cat apps/workspaces/migrations/0002_alter_workspace_ms_business_central_accounts_last_synced_at_and_more.pyLength of output: 12444
Verification successful
The search results indicate that the
refresh_tokenandis_expiredfields of theBusinessCentralCredentialsmodel are indeed being used throughout the application. Therefresh_tokenis being set and updated in various places, such as in thehelpers.pyfile within theworkspacesapp. Theis_expiredflag is also being managed, specifically in thehelpers.pyfile where it is reset toFalsewhen the refresh token is updated.The presence of these fields in the migration file, model definitions, and their usage in application logic suggests that the handling of the refresh token and the
is_expiredflag has been implemented as part of the application's functionality.Based on the information gathered, it appears that the handling of the
BusinessCentralCredentialsmodel, particularly the refresh token and theis_expiredflag, is implemented correctly in the application logic.* 119-136: The creation of the `AdvancedSetting` model in the migration file is consistent with the changes in the `models.py` file. Ensure that the schedule-related fields are used correctly and that the migration does not disrupt any existing scheduling functionality.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that BusinessCentralCredentials is used correctly in the application logic. rg 'BusinessCentralCredentials' --type pythonLength of output: 76
Script:
#!/bin/bash # Correcting the file type and re-running the search for 'BusinessCentralCredentials' rg 'BusinessCentralCredentials' --type pyLength of output: 4767
Script:
#!/bin/bash # Search for methods that handle the refresh token and the is_expired flag in BusinessCentralCredentials rg 'refresh_token|is_expired' --type pyLength of output: 5082
Verification successful
The search results indicate that the
schedule_is_enabled,start_datetime, andinterval_hoursfields of theAdvancedSettingmodel are being referenced and potentially modified within thetasks.pyfile. This suggests that the schedule-related fields are actively used in the application's task scheduling logic.Based on the output, there are no indications of issues with the usage of these fields. The migration should not disrupt existing scheduling functionality as long as it correctly reflects these fields and their intended use within the application logic, which seems to be the case here.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that AdvancedSetting's schedule-related fields are used correctly. rg 'AdvancedSetting' --type python -C 5 | rg 'schedule'Length of output: 84
Script:
#!/bin/bash # Search for references to the schedule-related fields within the AdvancedSetting model across Python files. rg 'AdvancedSetting' -C 5 | rg 'schedule_is_enabled|start_datetime|schedule_id|interval_hours'Length of output: 408
| """ | ||
| Todo: Add last export details | ||
| """ |
There was a problem hiding this comment.
There is a TODO comment within the check_accounting_export_and_start_import function regarding adding last export details. This should be addressed before the final release to ensure that the application correctly tracks the last export details.
| def __construct_journal_entry(self, body: JournalEntry, lineitems: JournalEntryLineItems) -> Dict: | ||
| ''' | ||
| Construct the payload for the direct invoice. | ||
| :param expense_report: ExpenseReport object extracted from database | ||
| :param expense_report_lineitems: ExpenseReportLineitem objects extracted from database | ||
| :return: constructed expense_report | ||
| ''' | ||
| batch_journal_entry_payload = [] | ||
|
|
||
| journal_entry_payload = { | ||
| 'accountNumber': body.accounts_payable_account_id, | ||
| 'postingDate': body.invoice_date, | ||
| 'documentNumber': body.document_number, | ||
| 'amount': body.amount, | ||
| 'comment': body.comment, | ||
| 'description': body.description | ||
| } | ||
|
|
||
| batch_journal_entry_payload.append(journal_entry_payload) | ||
|
|
||
| for lineitem in lineitems: | ||
| journal_entry_lineitem_payload = { | ||
| 'accountNumber': lineitem.accounts_payable_account_id, | ||
| 'postingDate': lineitem.invoice_date, | ||
| 'documentNumber': lineitem.document_number, | ||
| 'amount': lineitem.amount, | ||
| 'comment': lineitem.comment, | ||
| 'description': lineitem.description if lineitem.description else '' | ||
| } | ||
|
|
||
| batch_journal_entry_payload.append(journal_entry_lineitem_payload) | ||
|
|
||
| return batch_journal_entry_payload |
There was a problem hiding this comment.
The method __construct_journal_entry constructs the payload for journal entries. It's private and seems to be used within the class. However, the method name __construct_journal_entry suggests it's a constructor, which it is not. Consider renaming it to construct_journal_entry_payload for clarity.
| def get_vendor_id(accounting_export: AccountingExport) -> str: | ||
| return "" | ||
|
|
||
| def get_employee_id(accounting_export: AccountingExport) -> str: | ||
| return "" |
There was a problem hiding this comment.
The methods get_vendor_id and get_employee_id are placeholders returning empty strings. If these methods are meant to be implemented, add a TODO comment or implement them. If they are not needed, consider removing them to avoid confusion.
ruuushhh
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (1)
- tests/test_workspaces/test_view.py (4 hunks)
Additional comments: 1
tests/test_workspaces/test_view.py (1)
- 75-92: > Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [78-110]
The
test_export_settingsfunction has been updated to include additional parameters and amocker.patchcall. It's important to ensure that themocker.patchis correctly targeting the intended method and that the mock return value aligns with the expected data structure from the actual API call. Additionally, the hardcodedworkspace_idof 1 is used, which may not be reliable for all test environments. Consider using a dynamic retrieval ofworkspace_idfrom thecreate_temp_workspacefixture if it's designed to provide a valid workspace for testing.Verification successful
The
mocker.patchcall in thetest_export_settingsfunction is correctly targeting the intended method, as confirmed by the search results showing a single match in the file under review.The hardcoded
workspace_idof 1 is used extensively throughout the test suite, suggesting that the tests are designed with this assumption. While this is a common pattern in the test suite, it's important to ensure that the test environment is set up to handle this correctly to avoid brittle tests.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the mocker.patch is correctly targeting the intended method rg --type py "dynamics.apis.Journals.post" # Verify that the hardcoded workspace_id of 1 is not used elsewhere in a way that could cause conflicts or unreliable tests rg --type py "workspace_id=1"Length of output: 14228
tests/test_workspaces/test_view.py
Outdated
| assert export_settings.credit_card_expense_state == 'PAID' | ||
| assert export_settings.credit_card_expense_grouped_by == 'EXPENSE' | ||
| assert export_settings.credit_card_expense_date == 'CREATED_AT' | ||
| assert export_settings.default_reimbursable_account_name == 'reimbursable account' | ||
| assert export_settings.default_reimbursable_account_id == '123' | ||
| assert export_settings.default_ccc_credit_card_account_name == 'CCC credit card account' | ||
| assert export_settings.default_ccc_credit_card_account_id == '123' | ||
| assert export_settings.default_reimbursable_account_name == 'reimbursable account' | ||
| assert export_settings.default_reimbursable_account_id == '123' | ||
| assert export_settings.default_reimbursable_credit_card_account_name == 'reimbursable credit card account' | ||
| assert export_settings.default_reimbursable_credit_card_account_id == '342' | ||
| assert export_settings.default_vendor_name == 'Nilesh' |
There was a problem hiding this comment.
Note: This review was outside the patches, so it was mapped to the patch with the greatest overlap. Original lines [119-133]
The assertions for export_settings are well-placed to ensure that the settings are correctly updated. However, the hardcoded workspace_id of 1 is used again for retrieving the export_settings object. This could lead to flaky tests if the workspace with ID 1 does not exist or is not the intended workspace for the test. It would be better to use the workspace_id from the create_temp_workspace fixture or from the response of a workspace creation within the test.
- export_settings = ExportSetting.objects.filter(workspace_id=1).first()
+ # Assuming create_temp_workspace returns a workspace object with an id
+ workspace = create_temp_workspace()
+ export_settings = ExportSetting.objects.filter(workspace_id=workspace.id).first()
ruuushhh
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (3)
- apps/business_central/exports/journal_entry/tasks.py (1 hunks)
- apps/business_central/utils.py (6 hunks)
- ms_business_central_api/models/fields.py (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- apps/business_central/exports/journal_entry/tasks.py
- apps/business_central/utils.py
- ms_business_central_api/models/fields.py
|
ruuushhh
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 6
Configuration used: CodeRabbit UI
Files selected for processing (7)
- apps/business_central/exports/base_model.py (1 hunks)
- apps/business_central/exports/helpers.py (1 hunks)
- apps/business_central/exports/journal_entry/models.py (1 hunks)
- apps/business_central/exports/journal_entry/tasks.py (1 hunks)
- apps/business_central/utils.py (6 hunks)
- apps/workspaces/models.py (6 hunks)
- apps/workspaces/serializers.py (3 hunks)
Additional comments: 23
apps/business_central/exports/helpers.py (2)
17-30: The function
get_filtered_mappingis designed to retrieve a mapping based on various filters. It correctly checks for the presence ofsource_idbefore deciding which filter to apply. The use of**filtersto unpack the dictionary into filter arguments is a Pythonic way to handle dynamic query parameters.81-86: The function
resolve_errors_for_exported_accounting_exportmarks all unresolved errors related to a specificaccounting_exportas resolved. This is a straightforward operation that uses Django's ORM capabilities effectively.apps/business_central/exports/journal_entry/tasks.py (4)
13-24: The
ExportJournalEntryclass is a subclass ofAccountingDataExporterand is responsible for exporting journal entries to Business Central. The constructor initializes thebody_modelandlineitem_modelwhich are used in the export process. Thetrigger_exportmethod is a wrapper around thecheck_accounting_export_and_start_importfunction, which is not defined in this context but is assumed to be correct.30-69: The
__construct_journal_entrymethod constructs the payload for journal entries. It creates a batch payload for both the journal entry and its line items. The method uses the providedJournalEntryandList[JournalEntryLineItems]objects to build the payload. The method is private, which is indicated by the double underscore prefix, and is only meant to be used within the class. The logic appears to be correct, and it properly handles the case where a line item's description might beNone.71-95: The
postmethod is responsible for posting the journal entry to Business Central. It constructs the payload, establishes a connection to Business Central, and posts the journal entry. It then iterates over the responses to upload attachments. The method assumes that thebulk_post_journal_lineitemsmethod of theBusinessCentralConnectorreturns a response with a specific structure. It is important to ensure that this assumption is correct and that theBusinessCentralConnectoris handling any exceptions that may occur during the posting process.98-108: The
create_journal_entryfunction is a helper function that creates and exports a journal entry. It uses theExportJournalEntryclass to perform the export. The function is decorated with@handle_business_central_exceptions(), which is assumed to handle any exceptions that may occur during the export process. The function is straightforward and delegates the creation and export process to theExportJournalEntryclass.apps/business_central/exports/base_model.py (4)
11-21: The
BaseExportModelclass is an abstract Django model that provides common fields for export-related models. It includescreated_at,updated_at, and a foreign key to theWorkspacemodel. The class is marked as abstract, which is correct as it should not be instantiated on its own.57-73: The
get_total_amountmethod calculates the total amount of expenses associated with anAccountingExport. It uses Django's ORMaggregatefunction to sum the amounts, which is an efficient way to perform this calculation. The method correctly handles the case where there may be no expenses by returning0.0if the total amount isNone.75-98: The
get_invoice_datemethod extracts the invoice date from theAccountingExportdescription. It checks for various keys and returns the corresponding value. The method falls back to the current date and time if none of the keys are present. This method assumes that the description is a dictionary with specific keys, which should be validated elsewhere in the code to ensure correctness.100-104: The
get_vendor_idandget_journal_entry_account_id_typemethods are hardcoded to return specific values. This is likely a placeholder or a default value. It's important to ensure that these values are correct and that there is a mechanism to configure them if necessary.apps/business_central/exports/journal_entry/models.py (3)
17-30: The
JournalEntrymodel extendsBaseExportModeland defines fields specific to a journal entry. The fields are well-defined with appropriate help texts. Theaccounting_exportfield is a one-to-one relationship with theAccountingExportmodel, which is correct for the intended use case.35-68: The
create_or_update_objectclass method in theJournalEntrymodel is responsible for creating or updating a journal entry object. It calculates the total amount by summing the amounts of all expenses and negates it, which is a specific business logic. The method usesupdate_or_createto ensure that an existing object is updated if it matches the given criteria. The method assumes that thedescriptionfield ofaccounting_exportcontains certain keys likeclaim_numberorexpense_number, which should be validated elsewhere in the code.71-133: The
JournalEntryLineItemsmodel also extendsBaseExportModeland defines fields for individual line items of a journal entry. Thecreate_or_update_objectclass method creates or updates line items for a journal entry. It retrieves the category mapping for each expense and constructs a comment using theget_expense_commentmethod. The method usesupdate_or_createsimilarly to theJournalEntrymodel and assumes the same structure for thedescriptionfield ofaccounting_export.apps/workspaces/serializers.py (2)
67-68: The
createmethod in theWorkspaceSerializerclass creates anAccountingExportSummaryobject after creating a workspace. This is a straightforward creation of a related object upon the creation of a workspace.113-121: In the
ExportSettingsSerializerclass, thecreatemethod checks if the export settings are for journal entries and if thejournal_entry_folder_idis not set. If these conditions are met, it creates a journal entry folder using theBusinessCentralConnector. This is a logical extension of the export settings creation process to ensure that necessary resources are created in Business Central.apps/business_central/utils.py (5)
75-75: The
_sync_datamethod in theBusinessCentralConnectorclass has been updated to useitem['number']if available, otherwiseitem['id']. This change accommodates a potential difference in the data structure coming from Business Central. The conditional logic seems correct and should handle the data as expected.97-97: The methods
sync_accounts,sync_vendors,sync_employees, andsync_locationshave been updated to useset_company_idinstead of directly accessing thecompany_idproperty. This change is likely due to a change in theDynamicsclass API. It's important to ensure that theset_company_idmethod exists and functions as expected.Also applies to: 109-109, 121-121, 133-133
140-150: The
create_journal_entry_foldermethod is a new addition that creates a default journal entry folder in Business Central. The payload and the POST request seem to be correctly formed. The method returns the ID of the created folder, which is the expected behavior.152-162: The
bulk_post_journal_lineitemsmethod is responsible for posting journal line items in bulk to Business Central. It retrieves the export settings and uses thejournal_entry_folder_idto post the payload. The method assumes that thebulk_postmethod of thejournal_line_itemsobject in theDynamicsclass exists and works as expected.164-188: The
post_purchase_invoicemethod posts a purchase invoice and its line items to Business Central. It handles exceptions by deleting the created purchase invoice if an error occurs during the posting of line items. This is a good use of a try-except block to ensure clean-up in case of a failure. The method assumes that thepurchase_invoicesandpurchase_invoice_line_itemsobjects in theDynamicsclass havepostandbulk_postmethods, respectively.apps/workspaces/models.py (3)
132-135: The
EXPORT_MODE_CHOICEStuple has been added to provide choices for the export mode. This is a standard Django practice for defining choices for a model field.220-220: The
AdvancedSettingmodel has been updated to renameschedule_start_datetimetostart_datetime. This change is likely for consistency and clarity. The field definition remains the same, which is appropriate.231-244: The
LastExportDetailclass is a new addition to the models. It stores details about the last export, including the time, mode, and counts of successful and failed exports. This is a useful addition for tracking export operations and their outcomes.
| def validate_accounting_export(accounting_export: AccountingExport): | ||
| bulk_errors = [] | ||
| row = 0 | ||
|
|
||
| expenses = accounting_export.expenses.all() | ||
|
|
||
| for lineitem in expenses: | ||
| category = lineitem.category if (lineitem.category == lineitem.sub_category or lineitem.sub_category == None) else '{0} / {1}'.format( | ||
| lineitem.category, lineitem.sub_category) | ||
|
|
||
| category_attribute = ExpenseAttribute.objects.filter( | ||
| value=category, | ||
| workspace_id=accounting_export.workspace_id, | ||
| attribute_type='CATEGORY' | ||
| ).first() | ||
|
|
||
| account = CategoryMapping.objects.filter( | ||
| source_category_id=category_attribute.id, | ||
| workspace_id=accounting_export.workspace_id | ||
| ).first() | ||
|
|
||
| if not account: | ||
| bulk_errors.append({ | ||
| 'row': row, | ||
| 'accounting_export_id': accounting_export.id, | ||
| 'value': category, | ||
| 'type': 'Category Mapping', | ||
| 'message': 'Category Mapping not found' | ||
| }) | ||
|
|
||
| if category_attribute: | ||
| Error.objects.update_or_create( | ||
| workspace_id=accounting_export.workspace_id, | ||
| expense_attribute=category_attribute, | ||
| defaults={ | ||
| 'type': 'CATEGORY_MAPPING', | ||
| 'error_title': category_attribute.value, | ||
| 'error_detail': 'Category mapping is missing', | ||
| 'is_resolved': False | ||
| } | ||
| ) | ||
|
|
||
| row = row + 1 | ||
|
|
||
| if bulk_errors: | ||
| raise BulkError('Mappings are missing', bulk_errors) |
There was a problem hiding this comment.
The function validate_accounting_export is responsible for validating the accounting_export object. It checks if the category mapping exists for each expense and logs an error if not found. The use of a bulk_errors list to collect all errors before raising a BulkError exception is a good practice as it allows for batch processing of errors. However, the variable row is incremented outside the conditional block, which means it will increment even if there is no error. This could lead to misleading row indices in the error messages.
- row = row + 1
+ if not account:
+ row = row + 1Committable 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.
| def validate_accounting_export(accounting_export: AccountingExport): | |
| bulk_errors = [] | |
| row = 0 | |
| expenses = accounting_export.expenses.all() | |
| for lineitem in expenses: | |
| category = lineitem.category if (lineitem.category == lineitem.sub_category or lineitem.sub_category == None) else '{0} / {1}'.format( | |
| lineitem.category, lineitem.sub_category) | |
| category_attribute = ExpenseAttribute.objects.filter( | |
| value=category, | |
| workspace_id=accounting_export.workspace_id, | |
| attribute_type='CATEGORY' | |
| ).first() | |
| account = CategoryMapping.objects.filter( | |
| source_category_id=category_attribute.id, | |
| workspace_id=accounting_export.workspace_id | |
| ).first() | |
| if not account: | |
| bulk_errors.append({ | |
| 'row': row, | |
| 'accounting_export_id': accounting_export.id, | |
| 'value': category, | |
| 'type': 'Category Mapping', | |
| 'message': 'Category Mapping not found' | |
| }) | |
| if category_attribute: | |
| Error.objects.update_or_create( | |
| workspace_id=accounting_export.workspace_id, | |
| expense_attribute=category_attribute, | |
| defaults={ | |
| 'type': 'CATEGORY_MAPPING', | |
| 'error_title': category_attribute.value, | |
| 'error_detail': 'Category mapping is missing', | |
| 'is_resolved': False | |
| } | |
| ) | |
| row = row + 1 | |
| if bulk_errors: | |
| raise BulkError('Mappings are missing', bulk_errors) | |
| def validate_accounting_export(accounting_export: AccountingExport): | |
| bulk_errors = [] | |
| row = 0 | |
| expenses = accounting_export.expenses.all() | |
| for lineitem in expenses: | |
| category = lineitem.category if (lineitem.category == lineitem.sub_category or lineitem.sub_category == None) else '{0} / {1}'.format( | |
| lineitem.category, lineitem.sub_category) | |
| category_attribute = ExpenseAttribute.objects.filter( | |
| value=category, | |
| workspace_id=accounting_export.workspace_id, | |
| attribute_type='CATEGORY' | |
| ).first() | |
| account = CategoryMapping.objects.filter( | |
| source_category_id=category_attribute.id, | |
| workspace_id=accounting_export.workspace_id | |
| ).first() | |
| if not account: | |
| bulk_errors.append({ | |
| 'row': row, | |
| 'accounting_export_id': accounting_export.id, | |
| 'value': category, | |
| 'type': 'Category Mapping', | |
| 'message': 'Category Mapping not found' | |
| }) | |
| if category_attribute: | |
| Error.objects.update_or_create( | |
| workspace_id=accounting_export.workspace_id, | |
| expense_attribute=category_attribute, | |
| defaults={ | |
| 'type': 'CATEGORY_MAPPING', | |
| 'error_title': category_attribute.value, | |
| 'error_detail': 'Category mapping is missing', | |
| 'is_resolved': False | |
| } | |
| ) | |
| row = row + 1 | |
| if bulk_errors: | |
| raise BulkError('Mappings are missing', bulk_errors) |
| def load_attachments( | ||
| business_central_connection: BusinessCentralConnector, | ||
| ref_id: str, | ||
| ref_type: str, | ||
| expense: Expense, | ||
| accounting_export: AccountingExport, | ||
| ): | ||
| """ | ||
| Get attachments from fyle | ||
| :param business_central_connection: Business Central Connection | ||
| :param ref_id: object id | ||
| :param ref_type: type of object | ||
| :param expense_group: Expense group | ||
| """ | ||
| try: | ||
| fyle_credentials = FyleCredential.objects.get( | ||
| workspace_id=accounting_export.workspace_id | ||
| ) | ||
| platform = PlatformConnector(fyle_credentials) | ||
|
|
||
| files_list = [] | ||
|
|
||
| if expense.file_ids and len(expense.file_ids): | ||
| for file_id in expense.file_ids: | ||
| if file_id: | ||
| file_object = {"id": file_id} | ||
| files_list.append(file_object) | ||
|
|
||
| if files_list: | ||
| attachments = platform.files.bulk_generate_file_urls(files_list) | ||
|
|
||
| response = business_central_connection.post_attachments(ref_type, ref_id, attachments) | ||
| print(response) | ||
|
|
||
| except Exception: | ||
| error = traceback.format_exc() | ||
| logger.info( | ||
| "Attachment failed for accounting export id %s / workspace id %s \n Error: %s", | ||
| accounting_export.id, | ||
| accounting_export.workspace_id, | ||
| {"error": error}, | ||
| ) |
There was a problem hiding this comment.
The function load_attachments is designed to upload attachments to Business Central. It retrieves Fyle credentials, constructs a list of file objects, and then uses the PlatformConnector to generate URLs for these files. The attachments are then posted to Business Central. The function includes error handling that logs the exception and uses a try-except block to catch any issues during the process. However, the print(response) statement on line 121 should be replaced with a logger call to ensure that the response is logged appropriately in production environments.
- print(response)
+ logger.info("Attachment upload response: %s", response)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.
| def load_attachments( | |
| business_central_connection: BusinessCentralConnector, | |
| ref_id: str, | |
| ref_type: str, | |
| expense: Expense, | |
| accounting_export: AccountingExport, | |
| ): | |
| """ | |
| Get attachments from fyle | |
| :param business_central_connection: Business Central Connection | |
| :param ref_id: object id | |
| :param ref_type: type of object | |
| :param expense_group: Expense group | |
| """ | |
| try: | |
| fyle_credentials = FyleCredential.objects.get( | |
| workspace_id=accounting_export.workspace_id | |
| ) | |
| platform = PlatformConnector(fyle_credentials) | |
| files_list = [] | |
| if expense.file_ids and len(expense.file_ids): | |
| for file_id in expense.file_ids: | |
| if file_id: | |
| file_object = {"id": file_id} | |
| files_list.append(file_object) | |
| if files_list: | |
| attachments = platform.files.bulk_generate_file_urls(files_list) | |
| response = business_central_connection.post_attachments(ref_type, ref_id, attachments) | |
| print(response) | |
| except Exception: | |
| error = traceback.format_exc() | |
| logger.info( | |
| "Attachment failed for accounting export id %s / workspace id %s \n Error: %s", | |
| accounting_export.id, | |
| accounting_export.workspace_id, | |
| {"error": error}, | |
| ) | |
| def load_attachments( | |
| business_central_connection: BusinessCentralConnector, | |
| ref_id: str, | |
| ref_type: str, | |
| expense: Expense, | |
| accounting_export: AccountingExport, | |
| ): | |
| """ | |
| Get attachments from fyle | |
| :param business_central_connection: Business Central Connection | |
| :param ref_id: object id | |
| :param ref_type: type of object | |
| :param expense_group: Expense group | |
| """ | |
| try: | |
| fyle_credentials = FyleCredential.objects.get( | |
| workspace_id=accounting_export.workspace_id | |
| ) | |
| platform = PlatformConnector(fyle_credentials) | |
| files_list = [] | |
| if expense.file_ids and len(expense.file_ids): | |
| for file_id in expense.file_ids: | |
| if file_id: | |
| file_object = {"id": file_id} | |
| files_list.append(file_object) | |
| if files_list: | |
| attachments = platform.files.bulk_generate_file_urls(files_list) | |
| response = business_central_connection.post_attachments(ref_type, ref_id, attachments) | |
| logger.info("Attachment upload response: %s", response) | |
| except Exception: | |
| error = traceback.format_exc() | |
| logger.info( | |
| "Attachment failed for accounting export id %s / workspace id %s \n Error: %s", | |
| accounting_export.id, | |
| accounting_export.workspace_id, | |
| {"error": error}, | |
| ) |
| def get_expense_comment(workspace_id, lineitem: Expense, category: str, advance_setting: AdvancedSetting) -> str: | ||
| workspace = Workspace.objects.get(id=workspace_id) | ||
| org_id = workspace.org_id | ||
|
|
||
| fyle_credentials = FyleCredential.objects.get(workspace_id=workspace_id) | ||
| cluster_domain = fyle_credentials.cluster_domain | ||
| workspace.cluster_domain = cluster_domain | ||
| workspace.save() | ||
|
|
||
| expense_link = '{0}/app/main/#/enterprise/view_expense/{1}?org_id={2}'.format( | ||
| cluster_domain, lineitem.expense_id, org_id | ||
| ) | ||
|
|
||
| memo_structure = advance_setting.expense_memo_structure | ||
|
|
||
| details = { | ||
| 'employee_email': lineitem.employee_email, | ||
| 'merchant': '{0}'.format(lineitem.vendor) if lineitem.vendor else '', | ||
| 'category': '{0}'.format(category) if lineitem.category else '', | ||
| 'purpose': '{0}'.format(lineitem.purpose) if lineitem.purpose else '', | ||
| 'report_number': '{0}'.format(lineitem.claim_number), | ||
| 'spent_on': '{0}'.format(lineitem.spent_at.date()) if lineitem.spent_at else '', | ||
| 'expense_link': expense_link | ||
| } | ||
|
|
||
| purpose = '' | ||
|
|
||
| for id, field in enumerate(memo_structure): | ||
| if field in details: | ||
| purpose += details[field] | ||
| if id + 1 != len(memo_structure): | ||
| purpose = '{0} - '.format(purpose) | ||
|
|
||
| return purpose |
There was a problem hiding this comment.
The get_expense_comment method constructs a comment for an expense based on the expense_memo_structure from AdvancedSetting. It dynamically builds the comment by iterating over the memo structure and appending details from the expense. The method correctly handles the case where certain details may not be present. However, there is a bug in the loop that concatenates the fields; it appends a hyphen after every field, which will result in an incorrect comment format.
- purpose = '{0} - '.format(purpose)
+ if id + 1 != len(memo_structure):
+ purpose += ' - '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.
| def get_expense_comment(workspace_id, lineitem: Expense, category: str, advance_setting: AdvancedSetting) -> str: | |
| workspace = Workspace.objects.get(id=workspace_id) | |
| org_id = workspace.org_id | |
| fyle_credentials = FyleCredential.objects.get(workspace_id=workspace_id) | |
| cluster_domain = fyle_credentials.cluster_domain | |
| workspace.cluster_domain = cluster_domain | |
| workspace.save() | |
| expense_link = '{0}/app/main/#/enterprise/view_expense/{1}?org_id={2}'.format( | |
| cluster_domain, lineitem.expense_id, org_id | |
| ) | |
| memo_structure = advance_setting.expense_memo_structure | |
| details = { | |
| 'employee_email': lineitem.employee_email, | |
| 'merchant': '{0}'.format(lineitem.vendor) if lineitem.vendor else '', | |
| 'category': '{0}'.format(category) if lineitem.category else '', | |
| 'purpose': '{0}'.format(lineitem.purpose) if lineitem.purpose else '', | |
| 'report_number': '{0}'.format(lineitem.claim_number), | |
| 'spent_on': '{0}'.format(lineitem.spent_at.date()) if lineitem.spent_at else '', | |
| 'expense_link': expense_link | |
| } | |
| purpose = '' | |
| for id, field in enumerate(memo_structure): | |
| if field in details: | |
| purpose += details[field] | |
| if id + 1 != len(memo_structure): | |
| purpose = '{0} - '.format(purpose) | |
| return purpose | |
| def get_expense_comment(workspace_id, lineitem: Expense, category: str, advance_setting: AdvancedSetting) -> str: | |
| workspace = Workspace.objects.get(id=workspace_id) | |
| org_id = workspace.org_id | |
| fyle_credentials = FyleCredential.objects.get(workspace_id=workspace_id) | |
| cluster_domain = fyle_credentials.cluster_domain | |
| workspace.cluster_domain = cluster_domain | |
| workspace.save() | |
| expense_link = '{0}/app/main/#/enterprise/view_expense/{1}?org_id={2}'.format( | |
| cluster_domain, lineitem.expense_id, org_id | |
| ) | |
| memo_structure = advance_setting.expense_memo_structure | |
| details = { | |
| 'employee_email': lineitem.employee_email, | |
| 'merchant': '{0}'.format(lineitem.vendor) if lineitem.vendor else '', | |
| 'category': '{0}'.format(category) if lineitem.category else '', | |
| 'purpose': '{0}'.format(lineitem.purpose) if lineitem.purpose else '', | |
| 'report_number': '{0}'.format(lineitem.claim_number), | |
| 'spent_on': '{0}'.format(lineitem.spent_at.date()) if lineitem.spent_at else '', | |
| 'expense_link': expense_link | |
| } | |
| purpose = '' | |
| for id, field in enumerate(memo_structure): | |
| if field in details: | |
| purpose += details[field] | |
| if id + 1 != len(memo_structure): | |
| purpose += ' - ' | |
| return purpose |
| def post_attachments( | ||
| self, ref_type: str, ref_id: str, attachments: List[Dict] | ||
| ) -> List: | ||
| """ | ||
| Link attachments to objects Business Central | ||
| :param ref_id: object id | ||
| :param ref_type: type of object | ||
| :param attachments: attachment[dict()] | ||
| """ | ||
| responses = [] | ||
| if len(attachments): | ||
| for attachment in attachments: | ||
| data = { | ||
| "parentId": ref_id, | ||
| "fileName": "{0}_{1}".format(attachment["id"], attachment["name"]), | ||
| "parentType": ref_type | ||
| } | ||
| post_response = self.connection.attachments.post(data) | ||
|
|
||
| self.connection.attachments.upload( | ||
| post_response["id"], | ||
| attachment["content_type"], | ||
| base64.b64decode(attachment["download_url"]) | ||
| ) | ||
|
|
||
| responses.append(post_response) | ||
|
|
||
| return post_response |
There was a problem hiding this comment.
The post_attachments method uploads attachments to Business Central. It iterates over the provided attachments and posts each one. The method uses base64 decoding to handle the attachment content, which is correct. However, the method should return responses instead of post_response to return all responses for the attachments.
- return post_response
+ return responsesCommittable 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.
| def post_attachments( | |
| self, ref_type: str, ref_id: str, attachments: List[Dict] | |
| ) -> List: | |
| """ | |
| Link attachments to objects Business Central | |
| :param ref_id: object id | |
| :param ref_type: type of object | |
| :param attachments: attachment[dict()] | |
| """ | |
| responses = [] | |
| if len(attachments): | |
| for attachment in attachments: | |
| data = { | |
| "parentId": ref_id, | |
| "fileName": "{0}_{1}".format(attachment["id"], attachment["name"]), | |
| "parentType": ref_type | |
| } | |
| post_response = self.connection.attachments.post(data) | |
| self.connection.attachments.upload( | |
| post_response["id"], | |
| attachment["content_type"], | |
| base64.b64decode(attachment["download_url"]) | |
| ) | |
| responses.append(post_response) | |
| return post_response | |
| def post_attachments( | |
| self, ref_type: str, ref_id: str, attachments: List[Dict] | |
| ) -> List: | |
| """ | |
| Link attachments to objects Business Central | |
| :param ref_id: object id | |
| :param ref_type: type of object | |
| :param attachments: attachment[dict()] | |
| """ | |
| responses = [] | |
| if len(attachments): | |
| for attachment in attachments: | |
| data = { | |
| "parentId": ref_id, | |
| "fileName": "{0}_{1}".format(attachment["id"], attachment["name"]), | |
| "parentType": ref_type | |
| } | |
| post_response = self.connection.attachments.post(data) | |
| self.connection.attachments.upload( | |
| post_response["id"], | |
| attachment["content_type"], | |
| base64.b64decode(attachment["download_url"]) | |
| ) | |
| responses.append(post_response) | |
| return responses |
ruuushhh
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (3)
- apps/business_central/exceptions.py (1 hunks)
- apps/business_central/exports/journal_entry/queues.py (1 hunks)
- apps/business_central/exports/journal_entry/tasks.py (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- apps/business_central/exports/journal_entry/tasks.py
| def check_accounting_export_and_start_import(workspace_id: int, accounting_export_ids: List[str]): | ||
| """ | ||
| Check accounting export group and start export | ||
| """ | ||
|
|
||
| fyle_credentials = FyleCredential.objects.filter(workspace_id=workspace_id).first() | ||
|
|
||
| accounting_exports = AccountingExport.objects.filter(~Q(status__in=['IN_PROGRESS', 'COMPLETE', 'EXPORT_QUEUED']), | ||
| workspace_id=workspace_id, id__in=accounting_export_ids, journal_entry__id__isnull=True, | ||
| exported_at__isnull=True).all() | ||
|
|
||
| chain = Chain() | ||
| chain.append('apps.fyle.helpers.sync_dimensions', fyle_credentials) | ||
|
|
||
| for index, accounting_export_group in enumerate(accounting_exports): | ||
| accounting_export, _ = AccountingExport.objects.update_or_create( | ||
| workspace_id=accounting_export_group.workspace_id, | ||
| id=accounting_export_group.id, | ||
| defaults={ | ||
| 'status': 'ENQUEUED', | ||
| 'type': 'JOURNAL_ENTRY' | ||
| } | ||
| ) | ||
|
|
||
| if accounting_export.status not in ['IN_PROGRESS', 'ENQUEUED']: | ||
| accounting_export.status = 'ENQUEUED' | ||
| accounting_export.save() | ||
|
|
||
| last_export = False | ||
| if accounting_exports.count() == index + 1: | ||
| last_export = True | ||
|
|
||
| chain.append('apps.business_central.exports.journal_entry.tasks.create_journal_entry', accounting_export, last_export) | ||
|
|
||
| if chain.length() > 1: | ||
| chain.run() |
There was a problem hiding this comment.
The function check_accounting_export_and_start_import is designed to enqueue accounting exports for processing. However, there are a few points to consider:
- Line 17-19: The filter condition is complex and may benefit from being broken out for clarity.
- Line 21-45: The use of
Chainfromdjango_qis appropriate for creating a sequence of tasks. However, the condition on line 34 seems redundant since the status is set to 'ENQUEUED' in theupdate_or_createdefaults. - Line 38-40: The logic to determine
last_exportis correct, but it's inefficient to callaccounting_exports.count()inside the loop. This should be calculated once before the loop starts. - Line 42: The task
create_journal_entryis appended to the chain correctly, but it's important to ensure that the task is defined to handle thelast_exportflag appropriately.
Consider refactoring the filter condition for readability and efficiency. Also, review the necessity of the status check on line 34, and calculate accounting_exports.count() once before the loop to avoid repeated database calls.
| def handle_business_central_error(exception, accounting_export: AccountingExport, export_type: str): | ||
| logger.info(exception.response) | ||
|
|
||
| business_central_error = exception.response | ||
| error_msg = 'Failed to create {0}'.format(export_type) | ||
|
|
||
| Error.objects.update_or_create(workspace_id=accounting_export.workspace_id, accounting_export=accounting_export, defaults={'error_title': error_msg, 'type': 'BUSINESS_CENTRAL_ERROR', 'error_detail': business_central_error, 'is_resolved': False}) | ||
|
|
||
| accounting_export.status = 'FAILED' | ||
| accounting_export.detail = None | ||
| accounting_export.business_central_errors = business_central_error | ||
| accounting_export.save() | ||
|
|
||
|
|
||
| def handle_business_central_exceptions(): | ||
| def decorator(func): | ||
| def new_fn(*args): | ||
|
|
||
| accounting_export = args[0] | ||
|
|
||
| try: | ||
| return func(*args) | ||
| except (FyleCredential.DoesNotExist): | ||
| logger.info('Fyle credentials not found %s', accounting_export.workspace_id) | ||
| accounting_export.detail = {'message': 'Fyle credentials do not exist in workspace'} | ||
| accounting_export.status = 'FAILED' | ||
|
|
||
| accounting_export.save() | ||
|
|
||
| except BusinessCentralCredentials.DoesNotExist: | ||
| logger.info('Sage300 Account not connected / token expired for workspace_id %s / accounting export %s', accounting_export.workspace_id, accounting_export.id) | ||
| detail = {'accounting_export_id': accounting_export.id, 'message': 'Sage300 Account not connected / token expired'} | ||
| accounting_export.status = 'FAILED' | ||
| accounting_export.detail = detail | ||
|
|
||
| accounting_export.save() | ||
|
|
||
| except WrongParamsError as exception: | ||
| handle_business_central_error(exception, accounting_export, 'Purchase Invoice') | ||
|
|
||
| except BulkError as exception: | ||
| logger.info(exception.response) | ||
| detail = exception.response | ||
| accounting_export.status = 'FAILED' | ||
| accounting_export.detail = detail | ||
|
|
||
| accounting_export.save() | ||
|
|
||
| except Exception as error: | ||
| error = traceback.format_exc() | ||
| accounting_export.detail = {'error': error} | ||
| accounting_export.status = 'FATAL' | ||
|
|
||
| accounting_export.save() | ||
| logger.error('Something unexpected happened workspace_id: %s %s', accounting_export.workspace_id, accounting_export.detail) | ||
|
|
||
| update_accounting_export_summary(accounting_export.workspace_id) |
There was a problem hiding this comment.
The function handle_business_central_exceptions is a decorator designed to handle various exceptions that can occur during the export process. The following points need attention:
- Line 16-27: The function
handle_business_central_errorlogs the error and updates theAccountingExportobject. It's important to ensure that theexception.responsecontains the expected format to be stored inbusiness_central_errors. - Line 30-76: The decorator
handle_business_central_exceptionsis well-structured to catch different exceptions. However, the error messages and handling seem to be specific to Business Central and Sage300, which might be a copy-paste error (line 46 mentions Sage300 instead of Business Central). - Line 72: The call to
update_accounting_export_summaryshould be inside afinallyblock to ensure it's called even if an exception is not raised.
Correct the potential copy-paste error in the exception message on line 46 to reflect the correct system (Business Central). Also, consider moving the update_accounting_export_summary call to a finally block to ensure it's always executed.
* Purchase Invoice Export * Tests resolved * Comments resolved * Vendor Employee Id export logic (#64) * Vendor Employee Id export logic * Test resolved * Validate employee mapping * Validate employee mapping * Validate employee mapping * Fix comments * Comments resolved * Import Employee mapping Module (#65) * Import Employee mapping Module * Added triggers for import and advanced settings (#66) * Added triggers from import and advanced settings * Tests Fixed * location support and import account bug fix (#68) * location support and import account bug fix * location support and import account bug fix * comments resolved
ruuushhh
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 4
Configuration used: CodeRabbit UI
Files selected for processing (33)
- apps/business_central/exports/accounting_export.py (1 hunks)
- apps/business_central/exports/base_model.py (1 hunks)
- apps/business_central/exports/helpers.py (1 hunks)
- apps/business_central/exports/journal_entry/models.py (1 hunks)
- apps/business_central/exports/purchase_invoice/models.py (1 hunks)
- apps/business_central/exports/purchase_invoice/queues.py (1 hunks)
- apps/business_central/exports/purchase_invoice/tasks.py (1 hunks)
- apps/business_central/migrations/0002_purchaseinvoice_journalentry_employee_account_id_and_more.py (1 hunks)
- apps/business_central/migrations/0003_remove_purchaseinvoice_description.py (1 hunks)
- apps/business_central/migrations/0004_alter_purchaseinvoice_accounting_export.py (1 hunks)
- apps/business_central/migrations/0005_rename_employee_account_id_journalentry_account_id_and_more.py (1 hunks)
- apps/business_central/migrations/0006_purchaseinvoicelineitems_location_id.py (1 hunks)
- apps/business_central/models.py (1 hunks)
- apps/business_central/utils.py (6 hunks)
- apps/fyle/urls.py (1 hunks)
- apps/mappings/imports/modules/base.py (1 hunks)
- apps/mappings/imports/modules/employees.py (1 hunks)
- apps/mappings/imports/queues.py (1 hunks)
- apps/mappings/imports/tasks.py (2 hunks)
- apps/workspaces/migrations/0003_rename_default_journal_entry_folder_id_exportsetting_journal_entry_folder_id_and_more.py (1 hunks)
- apps/workspaces/migrations/0004_exportsetting_import_vendors_as_merchants_and_more.py (1 hunks)
- apps/workspaces/migrations/0005_rename_emoloyee_mapping_exportsetting_employee_mapping.py (1 hunks)
- apps/workspaces/migrations/0006_alter_importsetting_workspace.py (1 hunks)
- apps/workspaces/migrations/0006_rename_employee_mapping_exportsetting_employee_field_mapping.py (1 hunks)
- apps/workspaces/models.py (7 hunks)
- apps/workspaces/serializers.py (7 hunks)
- apps/workspaces/tasks.py (1 hunks)
- apps/workspaces/triggers.py (1 hunks)
- apps/workspaces/views.py (4 hunks)
- requirements.txt (1 hunks)
- tests/conftest.py (1 hunks)
- tests/test_fyle/fixtures.py (1 hunks)
- tests/test_workspaces/test_view.py (8 hunks)
Files skipped from review as they are similar to previous changes (5)
- apps/business_central/exports/helpers.py
- apps/business_central/exports/journal_entry/models.py
- apps/business_central/utils.py
- apps/workspaces/models.py
- apps/workspaces/serializers.py
Additional comments: 35
apps/business_central/models.py (1)
- 1-2: The imports for
JournalEntry,JournalEntryLineItems,PurchaseInvoice, andPurchaseInvoiceLineitemshave been added. Ensure that these models are indeed located in the specified modules and that they are used within this file or its related files.Verification successful
The search results confirm that the models
JournalEntry,JournalEntryLineItems,PurchaseInvoice, andPurchaseInvoiceLineitemsare indeed used within theapps/business_central/directory. The models are referenced in migration files, which indicates that they are part of the database schema changes. Additionally, they are used in thetasks.pyandmodels.pyfiles within theexports/journal_entryandexports/purchase_invoicesubdirectories, suggesting that these models are part of the export functionality for journal entries and purchase invoices.Given this information, the imports in
apps/business_central/models.pyare justified as these models are utilized in related files within thebusiness_centralapp.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the imported models are used in the current file or related files. rg --type python "JournalEntry|JournalEntryLineItems|PurchaseInvoice|PurchaseInvoiceLineitems" apps/business_central/models.pyLength of output: 159
Script:
#!/bin/bash # Search for the usage of the imported models in the apps/business_central/models.py file and related files within the apps/business_central/ directory. rg "JournalEntry|JournalEntryLineItems|PurchaseInvoice|PurchaseInvoiceLineitems" apps/business_central/Length of output: 4264
apps/business_central/migrations/0003_remove_purchaseinvoice_description.py (1)
- 13-16: The migration removes the
descriptionfield from thePurchaseInvoicemodel. Ensure that this change is intentional and that the removal of this field does not affect any existing functionality or data integrity.apps/workspaces/migrations/0005_rename_emoloyee_mapping_exportsetting_employee_mapping.py (1)
- 13-16: The migration renames the field
emoloyee_mappingtoemployee_mappingin theExportSettingmodel. This is likely a typo correction. Ensure that all references to the old field name are updated across the codebase.Verification successful
The occurrences of the old field name
emoloyee_mappingare found within the migration files, which is expected as they document the history of database schema changes. There is no indication that the old field name is used in the current operational code. The migration process in Django does not require updating old migration files when renaming fields, as these files serve as a historical record.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the old field name is not used anywhere in the codebase. rg --type python "emoloyee_mapping" .Length of output: 68
Script:
#!/bin/bash # Verify that the old field name is not used anywhere in the codebase. rg "emoloyee_mapping" .Length of output: 780
apps/business_central/migrations/0006_purchaseinvoicelineitems_location_id.py (1)
- 14-17: The migration adds a new field
location_idto thePurchaseInvoiceLineitemsmodel. Ensure that this field is appropriately used in the application and that any necessary validations or business logic related to this field are implemented.apps/workspaces/migrations/0006_alter_importsetting_workspace.py (1)
- 14-17: The migration alters the
workspacefield in theImportSettingmodel to use aOneToOneFieldwith aPROTECTon delete. Confirm that this change is in line with the application's data model and that it won't cause any issues with existing data.apps/business_central/migrations/0004_alter_purchaseinvoice_accounting_export.py (1)
- 15-18: The migration alters the
accounting_exportfield in thePurchaseInvoicemodel to use aOneToOneFieldwith aPROTECTon delete. Ensure that this change is consistent with the application's requirements and that it won't negatively impact existing data or functionality.Verification successful
The shell script has successfully found references to
accounting_exportin themodels.pyfile within theapps/business_centraldirectory. The output shows thataccounting_exportis indeed used as aOneToOneFieldrelated to theAccountingExportmodel, and there are several functions that interact with this field, indicating that the change to aOneToOneFieldis already integrated into the application's logic.Based on this information, it can be inferred that the migration altering the
accounting_exportfield to aOneToOneFieldwith aPROTECTon delete is consistent with the application's requirements, as the application logic is already using this field in the expected manner.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the change to a OneToOneField is consistent with the application's requirements. rg --type python "accounting_export" apps/business_central/models.pyLength of output: 99
Script:
#!/bin/bash # Re-run the search for "accounting_export" in the models.py file within the apps/business_central directory rg "accounting_export" apps/business_central/models.pyLength of output: 54
Script:
#!/bin/bash # Find the models.py file within the apps/business_central directory and search for "accounting_export" fd models.py apps/business_central --exec rg "accounting_export" {}Length of output: 3421
requirements.txt (2)
- 31-31: The
ms-dynamics-business-central-sdkpackage has been updated to version1.3.3. Confirm that this version is compatible with the application and that all necessary changes related to the update have been made.- 35-35: The
fyle-accounting-mappingspackage has been updated to version1.29.1. Ensure that this version is compatible with the application and that any necessary updates have been made to accommodate changes in the package.apps/business_central/migrations/0005_rename_employee_account_id_journalentry_account_id_and_more.py (3)
- 14-22: The migration renames the
employee_account_idfield toaccount_idin bothJournalEntryandJournalEntryLineitemsmodels. Confirm that this renaming is consistent with the intended usage and that all references to the old field name are updated.Verification successful
The search results indicate that the term
employee_account_idappears only within migration files. This is expected as migrations keep a record of historical changes to the database schema. The presence ofemployee_account_idin these files does not imply that the field is still being referenced in the active codebase; it merely reflects the state of the database before the migration was applied.Given this context, it seems that the renaming of
employee_account_idtoaccount_idhas been carried out correctly in the active codebase, as there are no references to the old field name outside of the migration files.* 24-27: The migration adds a new field `account_type` to the `JournalEntry` model. Ensure that this field is appropriately used in the application and that any necessary validations or business logic related to this field are implemented. * 29-32: The migration adds a new field `account_type` to the `JournalEntryLineitems` model. Ensure that this field is appropriately used in the application and that any necessary validations or business logic related to this field are implemented.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the old field name is not used anywhere in the codebase. rg --type python "employee_account_id" apps/business_central/Length of output: 92
Script:
#!/bin/bash # Correct the previous script to search for the old field name without specifying the file type rg "employee_account_id" apps/business_central/Length of output: 963
Verification successful
The search results indicate that the
account_typefield is present in thejournal_entry/models.pyandjournal_entry/tasks.pyfiles within theapps/business_central/exports/directory. The field is defined in theJournalEntryLineitemsmodel and is used in methods that seem to be related to setting up and exporting data. It is also referenced in the tasks file, where it appears to be included in the data being prepared for export.Given this information, it seems that the
account_typefield is indeed being utilized in the application, specifically within the context of journal entry exports. However, without further context on the business logic or validations, we cannot fully verify if it is used "correctly" as per the review comment's request.To ensure that the field is used with the necessary validations or business logic, we would need to examine the methods where
account_typeis used, such asget_account_id_typeand the tasks where it is included in the export payload. If more detailed verification is required, additional scripts could be run to inspect these methods and their usage ofaccount_type.For now, based on the search results, we can conclude that the
account_typefield is actively used in the application's journal entry export functionality.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new account_type field is being used correctly in the application. rg --type python "account_type" apps/business_central/Length of output: 85
Script:
#!/bin/bash # Verify that the new account_type field is being used correctly in the application. # Using rg with a glob pattern to search for Python files rg "account_type" --glob "*.py" apps/business_central/Length of output: 1404
apps/workspaces/migrations/0004_exportsetting_import_vendors_as_merchants_and_more.py (3)
- 14-17: The migration adds a new field
import_vendors_as_merchantsto theExportSettingmodel. Confirm that this field is used correctly in the application and that any related functionality is implemented.- 19-22: The migration adds a new field
name_in_journal_entryto theExportSettingmodel. Ensure that this field is used correctly in the application and that any related functionality is implemented.Verification successful
The
name_in_journal_entryfield is referenced in the following files:
apps/workspaces/serializers.py: It appears in a conditional statement, which suggests that there is logic that depends on the value of this field.apps/workspaces/models.py: The field is defined in theExportSettingmodel.apps/workspaces/migrations/0004_exportsetting_import_vendors_as_merchants_and_more.py: The field is added in a migration file.Based on the output, the field is indeed being used in the application, specifically within the
workspacesapp. The usage inserializers.pyindicates that there is some functionality that relies on the value of this field.* 24-27: The migration alters the `emoloyee_mapping` field in the `ExportSetting` model. This seems to be a typo and should be `employee_mapping`. Confirm that this is a typo and that it is corrected in the codebase.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the new name_in_journal_entry field is being used correctly in the application. rg --type python "name_in_journal_entry" apps/workspaces/Length of output: 88
Script:
#!/bin/bash # Verify that the new name_in_journal_entry field is being used correctly in the application. # Correcting the previous script by removing the incorrect --type flag rg "name_in_journal_entry" apps/workspaces/Length of output: 423
Verification successful
The typo in the field name
emoloyee_mappingwas indeed present in the codebase. However, it has been corrected in a subsequent migration, as indicated by the presence of a migration file named0005_rename_emoloyee_mapping_exportsetting_employee_mapping.pywhich changes the field name fromemoloyee_mappingtoemployee_mapping. This correction is also referenced in other migration files, confirming that the typo has been addressed.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the typo in the field name is corrected in the codebase. rg --type python "emoloyee_mapping" apps/workspaces/Length of output: 83
Script:
#!/bin/bash # Re-run the search for "emoloyee_mapping" without specifying the file type. rg "emoloyee_mapping" apps/workspaces/Length of output: 785
apps/mappings/imports/tasks.py (2)
- 3-3: The import for
Employeefrom theemployeesmodule has been added. Ensure that theEmployeemodel is indeed located in the specified module and that it is used within this file or its related files.Verification successful
The shell script output confirms that the
Employeemodel is imported inapps/mappings/imports/tasks.pyand is also referenced within theENTITY_MAPdictionary in the same file. This verifies that the import is indeed used within the file.* 14-14: The `ENTITY_MAP` dictionary has been updated to include `'EMPLOYEE': Employee`. Confirm that this addition is consistent with the application's data model and that the `Employee` entity is handled correctly where this map is used.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the imported Employee model is used in the current file or related files. rg --type python "Employee" apps/mappings/imports/tasks.pyLength of output: 89
Script:
#!/bin/bash # Verify that the imported Employee model is used in the current file or related files. rg "Employee" apps/mappings/imports/tasks.pyLength of output: 131
apps/mappings/imports/queues.py (1)
- 34-34: The condition for triggering an import via schedule in the
chain_import_fields_to_fylefunction has been updated to include the 'EMPLOYEE' field. Confirm that this change is intended and that the 'EMPLOYEE' field is handled correctly in the import process.Verification successful
The extended script output provides more context on the
chain_import_fields_to_fylefunction. It shows that the function is setting up a chain of import tasks based on theimport_settingsandmapping_settingsfor a givenworkspace_id. TheEMPLOYEEfield is included in a conditional check alongsidePROJECTandCOST_CENTER, and if this condition is met, a task is appended to the chain to trigger an import via schedule.The output, however, is still truncated and does not show the complete handling of the 'EMPLOYEE' field within the function. To ensure that the 'EMPLOYEE' field is handled correctly, we need to see the complete logic after the condition that includes the 'EMPLOYEE' field. This would typically involve checking if there are any specific actions or methods called for the 'EMPLOYEE' field that differ from the other fields, or if there's additional logic further down in the function that pertains to the 'EMPLOYEE' field.
Given the current output, we can infer that the 'EMPLOYEE' field is considered for scheduling an import task, but without the complete function body, we cannot fully verify the correct handling of the 'EMPLOYEE' field in the import process. Therefore, we need to extend the script further to capture the rest of the function's content.
The script output now provides the complete content of the
chain_import_fields_to_fylefunction. The function sets up a chain of import tasks based on the settings for a givenworkspace_id. The handling of the 'EMPLOYEE' field is part of a loop that iterates overmapping_settings. If thesource_fieldof amapping_settingis in the list['PROJECT', 'COST_CENTER', 'EMPLOYEE'], the function appends a task to the chain to trigger an import via schedule, using themapping_setting.destination_fieldandmapping_setting.source_fieldas parameters.The function also handles custom field mapping settings in a similar manner, appending tasks to the chain for each custom field that is set to be imported to Fyle.
Finally, if the chain has tasks appended to it (i.e., its length is greater than 0), the chain is executed with the
chain.run()call.Based on this information, we can verify that the 'EMPLOYEE' field is included in the import process in the same way as 'PROJECT' and 'COST_CENTER' fields. There is no special or different handling for the 'EMPLOYEE' field indicated in the function's logic. The change to include the 'EMPLOYEE' field in the condition for triggering an import via schedule is consistent with the rest of the function's implementation.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the 'EMPLOYEE' field is handled correctly in the import process. rg --type python "chain_import_fields_to_fyle" apps/mappings/imports/Length of output: 100
Script:
#!/bin/bash # Verify that the 'EMPLOYEE' field is handled correctly in the import process. # Removing the --type flag to avoid the previous error and ensure the search is conducted. rg "chain_import_fields_to_fyle" apps/mappings/imports/Length of output: 358
Script:
#!/bin/bash # Examine the implementation of the function 'chain_import_fields_to_fyle' in 'queues.py' # to understand how the 'EMPLOYEE' field is handled. ast-grep --lang python --pattern $'def chain_import_fields_to_fyle($_) { $$$ }' apps/mappings/imports/queues.pyLength of output: 578
Script:
#!/bin/bash # Correcting the pattern syntax for ast-grep to examine the 'chain_import_fields_to_fyle' function. ast-grep --lang python --pattern 'def chain_import_fields_to_fyle($workspace_id) { $$$ }' apps/mappings/imports/queues.pyLength of output: 600
Script:
#!/bin/bash # Using rg to extract the content of 'chain_import_fields_to_fyle' function from 'queues.py'. rg -A 10 "def chain_import_fields_to_fyle" apps/mappings/imports/queues.pyLength of output: 578
Script:
#!/bin/bash # Extending the range of lines to capture more of the 'chain_import_fields_to_fyle' function's content. rg -A 30 "def chain_import_fields_to_fyle" apps/mappings/imports/queues.pyLength of output: 1240
Script:
#!/bin/bash # Retrieve the full content of the 'chain_import_fields_to_fyle' function to verify the handling of the 'EMPLOYEE' field. rg -A 50 "def chain_import_fields_to_fyle" apps/mappings/imports/queues.pyLength of output: 1734
apps/fyle/urls.py (2)
- 29-29: The
SkippedExpenseViewhas been added to the list of exported entities. Ensure that this view is implemented correctly and that the corresponding URL path is properly configured.Verification successful
The
SkippedExpenseViewis implemented inapps/fyle/views.pyas aListAPIViewand is correctly referenced inapps/fyle/urls.py, where it is also associated with the URL path'expenses/'. This confirms that the view is implemented and has a corresponding URL path configured.* 34-34: The URL path for the `AccoutingExportSyncView` has been modified from `'expense_groups/sync/'` to `'accounting_exports/sync/'`. Confirm that this change is reflected in the frontend or any other services that might consume this endpoint.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the SkippedExpenseView is implemented and configured correctly. rg --type python "SkippedExpenseView" apps/fyle/Length of output: 79
Script:
#!/bin/bash # Verify that the SkippedExpenseView is implemented and configured correctly. # Removing the --type option as it caused an unrecognized file type error. rg "SkippedExpenseView" apps/fyle/Length of output: 231
apps/business_central/exports/purchase_invoice/queues.py (1)
- 10-10: The function
check_accounting_export_and_start_importis called to start the import process for accounting exports. Ensure that this function is implemented correctly and that it handles theaccounting_export_idsparameter appropriately.Verification successful
The function
check_accounting_export_and_start_importinqueues.pyis implemented to handle theaccounting_export_idsparameter by filteringAccountingExportobjects and queuing tasks for each export. The function updates the status of the exports and initiates the export process as expected.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the check_accounting_export_and_start_import function is implemented correctly. rg --type python "check_accounting_export_and_start_import" apps/business_central/exports/purchase_invoice/Length of output: 138
Script:
#!/bin/bash # Verify that the check_accounting_export_and_start_import function is implemented correctly by searching for its usage and definition. rg "check_accounting_export_and_start_import" apps/business_central/exports/purchase_invoice/Length of output: 554
Script:
#!/bin/bash # Review the implementation of the function check_accounting_export_and_start_import in queues.py cat apps/business_central/exports/purchase_invoice/queues.pyLength of output: 1707
apps/workspaces/migrations/0003_rename_default_journal_entry_folder_id_exportsetting_journal_entry_folder_id_and_more.py (2)
- 14-17: The migration renames the field
default_journal_entry_folder_idtojournal_entry_folder_idin theExportSettingmodel. Confirm that this renaming is consistent with the intended usage and that all references to the old field name are updated.Verification successful
The migration renaming
default_journal_entry_folder_idtojournal_entry_folder_idhas been correctly reflected in the codebase. The old field name only appears within the migration files, which is expected as they document the schema evolution. There are no references to the old field name in the active application code.* 19-45: Multiple fields are being removed from the `ExportSetting` model. Ensure that the removal of these fields is intentional and that it does not affect any existing functionality or data integrity.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the old field name is not used anywhere in the codebase. rg --type python "default_journal_entry_folder_id" apps/workspaces/Length of output: 98
Script:
#!/bin/bash # Correct the previous script by using the proper file type for Python files. rg "default_journal_entry_folder_id" apps/workspaces/ --type pyLength of output: 726
apps/workspaces/triggers.py (2)
- 19-41: The
post_save_mapping_settingsmethod in theImportSettingsTriggerclass deletes mapping settings that are not required. Ensure that this deletion logic is correct and does not inadvertently remove necessary settings.- 53-63: The
run_post_advance_settings_triggersmethod in theAdvancedSettingsTriggersclass schedules a sync based on advanced settings. Confirm that the scheduling logic is correct and that it respects the user's configuration.apps/business_central/exports/accounting_export.py (1)
- 30-75: The
create_business_central_objectmethod in theAccountingDataExporterclass creates an accounting object and posts it to an external system. Ensure that the implementation of this method is correct and that it handles transactions and error resolution appropriately.Verification successful
The
create_business_central_objectmethod in theAccountingDataExporterclass has been successfully extracted from theaccounting_export.pyfile. The method includes the following operations:
- Retrieval of advanced settings for the current workspace.
- Checking and updating the status of the accounting export.
- Validation of the accounting export.
- Use of a database transaction to ensure atomic operations for:
- Creating or updating the main body of the accounting object.
- Creating or updating line items for the accounting object, if applicable.
- Posting the data to the external accounting system.
- Updating the accounting export details and status.
The method concludes by saving the updated accounting export object and resolving any errors related to the exported accounting export.
Based on the provided code, the method appears to handle transactions and error resolution appropriately, as it uses a database transaction to ensure that the operations are atomic and includes error resolution at the end of the process.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the create_business_central_object method is implemented correctly. rg --type python "create_business_central_object" apps/business_central/exports/accounting_export.pyLength of output: 131
Script:
#!/bin/bash # Verify that the create_business_central_object method is implemented correctly. rg "create_business_central_object" apps/business_central/exports/accounting_export.pyLength of output: 169
Script:
#!/bin/bash # Display the contents of the accounting_export.py file starting from the definition of create_business_central_object method. cat apps/business_central/exports/accounting_export.py | sed -n '/def create_business_central_object/,/^$/p'Length of output: 277
Script:
#!/bin/bash # Attempt to display the entire body of the create_business_central_object method. # Using awk to print lines from the start of create_business_central_object method until the next def or end of file. awk '/def create_business_central_object/,/def |^$/' apps/business_central/exports/accounting_export.pyLength of output: 188
Script:
#!/bin/bash # Attempt to display the entire body of the create_business_central_object method. # Using sed to print lines from the start of create_business_central_object method until the next def or an empty line that indicates the end of the method. sed -n '/def create_business_central_object/,/^$/p' apps/business_central/exports/accounting_export.pyLength of output: 271
Script:
#!/bin/bash # Display the entire content of the accounting_export.py file and then extract the create_business_central_object method. cat apps/business_central/exports/accounting_export.py | sed -n '/def create_business_central_object/,/def/p'Length of output: 2340
apps/workspaces/views.py (2)
- 112-119: The
ImportSettingViewclass has been modified to inherit fromgenerics.RetrieveUpdateAPIViewand theget_objectmethod has been customized. Confirm that this change is consistent with the application's requirements and that theImportSettingmodel is correctly retrieved and updated.- 144-150: The
TriggerExportsViewclass has been added to trigger export creation. Ensure that theexport_to_business_centraltask is called correctly and that the export process is initiated as expected.Verification successful
The
TriggerExportsViewclass'spostmethod correctly calls theexport_to_business_centralfunction with theworkspace_idparameter extracted fromkwargs. This indicates that the export process is initiated as expected when this view'spostmethod is called.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the TriggerExportsView class correctly initiates the export process. rg --type python "TriggerExportsView" apps/workspaces/views.pyLength of output: 93
Script:
#!/bin/bash # Re-run the verification for TriggerExportsView class without specifying the file type. rg "class TriggerExportsView" apps/workspaces/views.pyLength of output: 107
Script:
#!/bin/bash # Verify the implementation of the post method within the TriggerExportsView class. ast-grep --lang python --pattern $'class TriggerExportsView { def post($_, $_, $_, $_) { $$$ } }'Length of output: 598
Script:
#!/bin/bash # Correct the ast-grep pattern to match the post method within the TriggerExportsView class. ast-grep --lang python --pattern $'class $_ { def post($$$) { $$$ } }' --source 'apps/workspaces/views.py'Length of output: 305
Script:
#!/bin/bash # Adjust the ast-grep command to correctly search for the post method within the TriggerExportsView class in the views.py file. ast-grep --lang python --pattern $'class $_ { def post($$$) { $$$ } }' apps/workspaces/views.pyLength of output: 441
Script:
#!/bin/bash # Use a simpler grep command to verify the presence and correct usage of the export_to_business_central function within the post method of the TriggerExportsView class. grep -A 5 "class TriggerExportsView" apps/workspaces/views.pyLength of output: 206
Script:
#!/bin/bash # Extend the grep command to capture more lines following the class definition to see the complete implementation of the post method. grep -A 10 "class TriggerExportsView" apps/workspaces/views.pyLength of output: 353
apps/business_central/migrations/0002_purchaseinvoice_journalentry_employee_account_id_and_more.py (3)
- 18-32: The migration creates a new
PurchaseInvoicemodel with various fields. Confirm that the fields and their types are appropriate for the data they are meant to store and that they align with the application's data model.- 37-45: The migration adds new fields to the
JournalEntryandJournalEntryLineitemsmodels. Ensure that these fields are used correctly in the application and that any necessary validations or business logic related to these fields are implemented.- 47-63: The migration creates a new
PurchaseInvoiceLineitemsmodel with various fields. Confirm that the fields and their types are appropriate for the data they are meant to store and that they align with the application's data model.Verification successful
The fields and their types for the
PurchaseInvoiceLineitemsmodel have been successfully located in theapps/business_central/exports/purchase_invoice/models.pyfile. The fields are as follows:
accounts_payable_account_id: UsesStringNullField, which seems appropriate for storing an ID as a string and allows null values.purchase_invoice: Usesmodels.ForeignKey, referencingPurchaseInvoice, which is standard for establishing a many-to-one relationship.expense: Usesmodels.OneToOneField, referencingExpense, which is standard for a one-to-one relationship with theExpensemodel.amount: UsesFloatNullField, which is suitable for storing numerical amounts with the possibility of null values.description: UsesTextNotNullField, which is suitable for storing text descriptions and does not allow null values.location_id: UsesStringNullField, similar toaccounts_payable_account_id, suitable for storing an ID as a string with null values allowed.The types of fields used are consistent with common practices for modeling such data in Django. The use of null-allowed fields where appropriate suggests consideration for cases where data may not be present.
Based on the information provided, the fields and their types in the
PurchaseInvoiceLineitemsmodel appear to be appropriate and align with the application's data model.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the fields in the PurchaseInvoiceLineitems model are appropriate and align with the application's data model. rg --type python "PurchaseInvoiceLineitems" apps/business_central/models.pyLength of output: 106
Script:
#!/bin/bash # Verify that the fields in the PurchaseInvoiceLineitems model are appropriate and align with the application's data model. rg "PurchaseInvoiceLineitems" apps/business_central/models.pyLength of output: 177
Script:
#!/bin/bash # Locate the definition of the PurchaseInvoiceLineitems model to verify the fields and their types. rg "class PurchaseInvoiceLineitems" -A 20 apps/business_central/exports/purchase_invoice/models.pyLength of output: 1096
apps/business_central/exports/base_model.py (4)
- 23-56: The
get_expense_commentmethod has been updated to include logic for constructing a URL to view an expense in Fyle and to build a comment based on a memo structure. The method now saves thecluster_domainto the workspace and uses it to construct theexpense_link. The loop for building the comment has been corrected to append a hyphen only between fields, not after every field, which addresses the previous issue raised in the comments.- 58-74: The
get_total_amountmethod correctly calculates the total amount of expenses associated with anAccountingExportobject. It handles the case where there are no expenses by returning 0.0, which is a good practice for avoidingNoneTypeerrors in arithmetic operations.- 101-129: The
get_account_id_typemethod determines the account ID and type based on theAccountingExportobject andExportSetting. It handles different scenarios for reimbursable expenses and credit card expenses, including default values and merchant-based logic. Ensure that all possible paths are covered by tests, especially since this method involves multiple conditional branches.- 152-178: The
get_location_idmethod retrieves the location ID based on theAccountingExportobject and a line item's project or cost center. It usesMappingSettingto determine the source field and then finds the correspondingMappingobject. This method should be tested to ensure that it handles all possible configurations and returns the correct location ID.apps/mappings/imports/modules/base.py (1)
- 173-173: The inclusion of
'employees'in thesync_expense_attributesmethod's condition is a logical update to ensure that employee data is synchronized along with other expense attributes. This change aligns with the PR's objective to enhance the export functionality.
| def get_invoice_date(accounting_export: AccountingExport) -> str: | ||
| """ | ||
| Get the invoice date from the provided AccountingExport. | ||
|
|
||
| Parameters: | ||
| - accounting_export (AccountingExport): The AccountingExport instance containing the description field. | ||
|
|
||
| Returns: | ||
| - str: The invoice date as a string in the format '%Y-%m-%dT%H:%M:%S'. | ||
| """ | ||
| # Check for specific keys in the 'description' field and return the corresponding value | ||
| if 'spent_at' in accounting_export.description and accounting_export.description['spent_at']: | ||
| return accounting_export.description['spent_at'] | ||
| elif 'approved_at' in accounting_export.description and accounting_export.description['approved_at']: | ||
| return accounting_export.description['approved_at'] | ||
| elif 'verified_at' in accounting_export.description and accounting_export.description['verified_at']: | ||
| return accounting_export.description['verified_at'] | ||
| elif 'last_spent_at' in accounting_export.description and accounting_export.description['last_spent_at']: | ||
| return accounting_export.description['last_spent_at'] | ||
| elif 'posted_at' in accounting_export.description and accounting_export.description['posted_at']: | ||
| return accounting_export.description['posted_at'] | ||
|
|
||
| # If none of the expected keys are present or if the values are empty, return the current date and time | ||
| return datetime.now().strftime("%Y-%m-%d") |
There was a problem hiding this comment.
The extended search output confirms that the get_invoice_date method in base_model.py returns the current date in the format '%Y-%m-%d' when none of the expected keys are present or their values are empty. This format does not match the format specified in the method's docstring ('%Y-%m-%dT%H:%M:%S'). Therefore, there is an inconsistency between the docstring and the actual behavior of the method.
The review comment is correct in pointing out that the method should ensure that the date format returned is consistent with the expected format mentioned in the docstring. The current implementation does not do this when it falls back to the current date.
Analysis chain
The get_invoice_date method extracts the invoice date from various fields within the description of an AccountingExport object. It falls back to the current date if none of the expected keys are present. This method should ensure that the date format returned is consistent and matches the expected format '%Y-%m-%dT%H:%M:%S' as mentioned in the docstring.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that the date format returned by get_invoice_date is consistent with '%Y-%m-%dT%H:%M:%S'
# This script should be run in a test environment where the method's behavior can be safely evaluated.
Length of output: 0
Script:
#!/bin/bash
# Search for the implementation of the get_invoice_date method in base_model.py
# and display the relevant code block that handles the return of the current date.
rg "def get_invoice_date" -A 20 apps/business_central/exports/base_model.py
Length of output: 1421
Script:
#!/bin/bash
# Extend the search to include the entire get_invoice_date method to see the fallback date format.
rg "def get_invoice_date" -A 30 apps/business_central/exports/base_model.py
Length of output: 1926
| def get_expense_purpose(lineitem: Expense, category: str, advance_setting: AdvancedSetting) -> str: | ||
| memo_structure = advance_setting.expense_memo_structure | ||
|
|
||
| details = { | ||
| 'employee_email': lineitem.employee_email, | ||
| 'merchant': '{0}'.format(lineitem.vendor) if lineitem.vendor else '', | ||
| 'category': '{0}'.format(category) if lineitem.category else '', | ||
| 'purpose': '{0}'.format(lineitem.purpose) if lineitem.purpose else '', | ||
| 'report_number': '{0}'.format(lineitem.claim_number), | ||
| 'spent_on': '{0}'.format(lineitem.spent_at.date()) if lineitem.spent_at else '', | ||
| } | ||
|
|
||
| purpose = '' | ||
|
|
||
| for id, field in enumerate(memo_structure): | ||
| if field in details: | ||
| purpose += details[field] | ||
| if id + 1 != len(memo_structure): | ||
| purpose = '{0} - '.format(purpose) | ||
|
|
||
| return purpose |
There was a problem hiding this comment.
The get_expense_purpose method appears to be a duplicate of the get_expense_comment method. This redundancy should be addressed to follow the DRY principle. The method should be refactored to reuse the common logic, or one of the methods should be removed if they are indeed identical.
- def get_expense_purpose(lineitem: Expense, category: str, advance_setting: AdvancedSetting) -> str:
- memo_structure = advance_setting.expense_memo_structure
- ...
- return purpose
+ # Removed duplicate method get_expense_purpose, use get_expense_comment instead.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.
| def get_expense_purpose(lineitem: Expense, category: str, advance_setting: AdvancedSetting) -> str: | |
| memo_structure = advance_setting.expense_memo_structure | |
| details = { | |
| 'employee_email': lineitem.employee_email, | |
| 'merchant': '{0}'.format(lineitem.vendor) if lineitem.vendor else '', | |
| 'category': '{0}'.format(category) if lineitem.category else '', | |
| 'purpose': '{0}'.format(lineitem.purpose) if lineitem.purpose else '', | |
| 'report_number': '{0}'.format(lineitem.claim_number), | |
| 'spent_on': '{0}'.format(lineitem.spent_at.date()) if lineitem.spent_at else '', | |
| } | |
| purpose = '' | |
| for id, field in enumerate(memo_structure): | |
| if field in details: | |
| purpose += details[field] | |
| if id + 1 != len(memo_structure): | |
| purpose = '{0} - '.format(purpose) | |
| return purpose | |
| # Removed duplicate method get_expense_purpose, use get_expense_comment instead. |
| migrations.RenameField( | ||
| model_name='exportsetting', | ||
| old_name='employee_mapping', | ||
| new_name='employee_field_mapping', |
There was a problem hiding this comment.
The search results show that the term employee_mapping is still present in the active codebase, specifically within the employees.py module and the helpers.py file in the business_central/exports directory. These references are not part of migration files or comments, which suggests that there are indeed active parts of the code that may still be using the old field name.
This indicates that the renaming of the field employee_mapping to employee_field_mapping in the ExportSetting model may not be fully consistent throughout the codebase, and there could be areas where the old field name is still being used. This could lead to errors if the code attempts to reference a field that has been renamed.
Based on this evidence, it seems that the review comment is correct, and there are indeed references to the old field name that need to be updated to reflect the new field name.
Analysis chain
The migration renames the field employee_mapping to employee_field_mapping in the ExportSetting model. Confirm that this renaming is consistent with the intended usage and that all references to the old field name are updated.
The search results indicate that the term employee_mapping is still present in various files within the codebase. Specifically, it appears in migration files, a helper file, and within comments in the employees.py module.
Given this information, it seems that not all references to the old field name employee_mapping have been updated. This could potentially lead to issues if the code is still referencing the old field name outside of the migration context.
To provide a more accurate response, I will need to differentiate between references that are part of historical migrations (which are expected and acceptable) and references that are part of the active codebase (which would need to be updated).
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that the old field name is not used anywhere in the codebase.
rg --type python "employee_mapping" .
Length of output: 68
Script:
#!/bin/bash
# Correct the previous script to search for the old field name "employee_mapping" without specifying the file type.
rg "employee_mapping" .
Length of output: 1371
Script:
#!/bin/bash
# Check the usage of "employee_mapping" in active codebase files, excluding migrations and comments.
rg "employee_mapping" --glob '!**/migrations/*' --glob '!*.pyc' --glob '!*.pyo' --glob '!__pycache__/*' --glob '!*.py~' --glob '!*.py#'
Length of output: 860
| migrations.AddField( | ||
| model_name='exportsetting', | ||
| name='emoloyee_mapping', | ||
| field=ms_business_central_api.models.fields.StringNullField(help_text='Employee Mapping', max_length=255, null=True), |
There was a problem hiding this comment.
The search results indicate that the correct field name employee_mapping does not appear in apps/workspaces/models.py, which suggests that the latest models file does not contain the corrected field name. Additionally, the incorrect field name emoloyee_mapping still appears in several migration files outside of the specific migrations that were meant to fix the typo.
Given this information, it seems that the typo has not been fully corrected in the codebase, as the incorrect field name still exists in migrations that are not related to the renaming. This suggests that the review comment is correct in pointing out the typo, and it has not been fully addressed in the codebase.
Analysis chain
The migration adds a new field emoloyee_mapping to the ExportSetting model. This seems to be a typo and should be employee_mapping. Confirm that this is a typo and that it is corrected in the codebase.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify that the typo in the field name is corrected in the codebase.
rg --type python "emoloyee_mapping" apps/workspaces/
Length of output: 83
Script:
#!/bin/bash
# Verify that the typo in the field name is corrected in the codebase.
rg "emoloyee_mapping" apps/workspaces/
Length of output: 785
Script:
#!/bin/bash
# Verify that the typo in the field name has been corrected in the latest state of the codebase.
# Search for the correct field name "employee_mapping" in the models.
rg "employee_mapping" apps/workspaces/models.py
# Search for the incorrect field name "emoloyee_mapping" outside of the specific migrations that fixed it.
rg "emoloyee_mapping" --glob '!apps/workspaces/migrations/0005_rename_emoloyee_mapping_exportsetting_employee_mapping.py' --glob '!apps/workspaces/migrations/0006_rename_employee_mapping_exportsetting_employee_field_mapping.py'
Length of output: 708
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Documentation
Refactor
Dependencies
ms-dynamics-business-central-sdkandfyle-accounting-mappingsto newer versions.Tests