Fix Edit Expenses changes #204
Conversation
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
WalkthroughThe primary updates include adding a 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 (
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- apps/fyle/queue.py (1 hunks)
Additional comments not posted (1)
apps/fyle/queue.py (1)
86-86: Ensure no sensitive information is logged.While logging the body of the request can be useful for debugging, ensure that no sensitive information (e.g., personal data, financial information) is included in the logs. Consider sanitizing the data before logging.
apps/fyle/queue.py
Outdated
| @@ -83,6 +83,7 @@ def async_handle_webhook_callback(body: dict, workspace_id: int) -> None: | |||
|
|
|||
| elif body.get('action') == 'UPDATED_AFTER_APPROVAL' and body.get('data') and body.get('resource') == 'EXPENSE': | |||
| org_id = body['data']['org_id'] | |||
| logger.info("Body: {}".format(body)) | |||
| logger.info("| Updating non-exported expenses through webhook | Content: {{WORKSPACE_ID: {} Payload: {}}}".format(workspace_id, body.get('data'))) | |||
There was a problem hiding this comment.
Correct the logging format.
The logging statement uses double curly braces which might be a formatting error. The correct format should be a single pair of curly braces.
- logger.info("| Updating non-exported expenses through webhook | Content: {{WORKSPACE_ID: {} Payload: {}}}".format(workspace_id, body.get('data')))
+ logger.info("| Updating non-exported expenses through webhook | Content: {WORKSPACE_ID: {} Payload: {}}".format(workspace_id, body.get('data')))Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| logger.info("| Updating non-exported expenses through webhook | Content: {{WORKSPACE_ID: {} Payload: {}}}".format(workspace_id, body.get('data'))) | |
| logger.info("| Updating non-exported expenses through webhook | Content: {WORKSPACE_ID: {} Payload: {}}".format(workspace_id, body.get('data'))) |
Hrishabh17
had a problem deploying
to
CI Environment
GitHub Actions
Failure
Hrishabh17
had a problem deploying
to
CI Environment
GitHub Actions
Failure
|
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- apps/fyle/models.py (4 hunks)
- apps/fyle/tasks.py (1 hunks)
- requirements.txt (1 hunks)
- tests/test_fyle/fixtures.py (1 hunks)
Files skipped from review due to trivial changes (1)
- requirements.txt
Additional comments not posted (3)
apps/fyle/tasks.py (1)
148-148: LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
update_non_exported_expensesare correct and non-breaking.apps/fyle/models.py (1)
Line range hint
119-140:
LGTM! But verify the function usage in the codebase.The code changes are approved.
However, ensure that all function calls to
create_expense_objectsmatch the new signature and are non-breaking.Verification successful
Ensure all function calls to
create_expense_objectsmatch the new signature and are non-breaking.
apps/fyle/tasks.py:
- Line: 47:
expense_objects = Expense.create_expense_objects(expenses, workspace_id)- Line: 57:
Expense.create_expense_objects(expense_objects, expense.workspace_id, skip_update=True)The function calls in
apps/fyle/tasks.pymatch the new signature ofcreate_expense_objectswith theskip_updateparameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `create_expense_objects` in the codebase. # Test: Search for the function usage. Expect: Only occurances of the new signature. rg --type python -A 5 $'create_expense_objects'Length of output: 77
Script:
#!/bin/bash # Description: Verify all function calls to `create_expense_objects` in the codebase. # Test: Search for the function usage. Expect: Only occurrences of the new signature. rg 'create_expense_objects' --glob '**/*.py' -A 5Length of output: 8236
tests/test_fyle/fixtures.py (1)
Line range hint
1-1:
Ensure tests are updated to reflect the removal of thereportfield.The removal of the
reportdictionary field might impact tests that rely on this data. Ensure the tests are updated accordingly.
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- apps/fyle/models.py (2 hunks)
- apps/fyle/tasks.py (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- apps/fyle/models.py
- apps/fyle/tasks.py
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
Hrishabh17
temporarily deployed
to
CI Environment
GitHub Actions
Inactive
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- apps/fyle/models.py (2 hunks)
- apps/fyle/tasks.py (1 hunks)
- sage_desktop_api/settings.py (1 hunks)
- sage_desktop_api/tests/settings.py (1 hunks)
- tests/test_fyle/fixtures.py (3 hunks)
Files skipped from review due to trivial changes (2)
- sage_desktop_api/settings.py
- sage_desktop_api/tests/settings.py
Files skipped from review as they are similar to previous changes (3)
- apps/fyle/models.py
- apps/fyle/tasks.py
- tests/test_fyle/fixtures.py
* fixing payload logging * fix edit expenses * bump version of platform-connector, add fix for expense update * add logger * remove extra loggers * increase poll * fix created_at, updated_at null constraint
Summary by CodeRabbit
New Features
skip_updateparameter in expense object creation to control update behavior.Performance Improvements
Dependencies
fyle-integrations-platform-connectorto version1.38.4.Tests