Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support for editing expenses #379

Merged
merged 3 commits into from
Jun 26, 2024
Merged

Add support for editing expenses #379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23bec31
Add support for editing expenses
Hrishabh17 Jun 26, 2024
588c337
Update status code
Hrishabh17 Jun 26, 2024
e1687f7
Merge branch 'master' into allow-expenses-edit
Hrishabh17 Jun 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions apps/exceptions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from rest_framework.response import Response
from rest_framework.views import status
from rest_framework.exceptions import ValidationError
from xerosdk.exceptions import (
InternalServerError,
InvalidClientError,
Expand Down Expand Up @@ -101,6 +102,13 @@ def new_fn(*args, **kwargs):
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
)

except ValidationError as e:
logger.exception(e)
return Response(
{"message": e.detail},
status=status.HTTP_400_BAD_REQUEST,
)

except Exception as exception:
logger.exception(exception)
return Response(
Expand Down
14 changes: 13 additions & 1 deletion apps/fyle/helpers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,12 @@

import requests
from django.conf import settings
from rest_framework.exceptions import ValidationError


from apps.fyle.models import Expense, ExpenseGroup, ExpenseGroupSettings
from apps.tasks.models import TaskLog
from apps.workspaces.models import WorkspaceGeneralSettings
from apps.workspaces.models import Workspace, WorkspaceGeneralSettings

import django_filters

Expand Down Expand Up @@ -187,6 +189,16 @@ def get_batched_expenses(batched_payload: List[dict], workspace_id: int) -> List
return Expense.objects.filter(expense_id__in=expense_ids, workspace_id=workspace_id)


def assert_valid_request(workspace_id:int, fyle_org_id:str):
"""
Assert if the request is valid by checking
the url_workspace_id and fyle_org_id workspace
"""
workspace = Workspace.objects.get(fyle_org_id=fyle_org_id)
if workspace.id != workspace_id:
raise ValidationError('Workspace mismatch')

Comment on lines +192 to +200
Copy link

Choose a reason for hiding this comment

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

New validation function: assert_valid_request

This function provides essential validation for workspace and organization ID matching, which is crucial for security and data integrity. However, the function could potentially raise an unhandled Workspace.DoesNotExist exception if no workspace matches the provided fyle_org_id.

- workspace = Workspace.objects.get(fyle_org_id=fyle_org_id)
+ workspace = Workspace.objects.filter(fyle_org_id=fyle_org_id).first()
+ if not workspace:
+     raise ValidationError('Workspace not found')
Committable suggestion

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

Suggested change
def assert_valid_request(workspace_id:int, fyle_org_id:str):
"""
Assert if the request is valid by checking
the url_workspace_id and fyle_org_id workspace
"""
workspace = Workspace.objects.get(fyle_org_id=fyle_org_id)
if workspace.id != workspace_id:
raise ValidationError('Workspace mismatch')
def assert_valid_request(workspace_id:int, fyle_org_id:str):
"""
Assert if the request is valid by checking
the url_workspace_id and fyle_org_id workspace
"""
workspace = Workspace.objects.filter(fyle_org_id=fyle_org_id).first()
if not workspace:
raise ValidationError('Workspace not found')
if workspace.id != workspace_id:
raise ValidationError('Workspace mismatch')


class AdvanceSearchFilter(django_filters.FilterSet):
def filter_queryset(self, queryset):
or_filtered_queryset = queryset.none()
Expand Down
14 changes: 13 additions & 1 deletion apps/fyle/queue.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,9 @@
import logging
from django_q.tasks import async_task
from apps.fyle.helpers import assert_valid_request

logger = logging.getLogger(__name__)
logger.level = logging.INFO


def async_post_accounting_export_summary(org_id: str, workspace_id: int) -> None:
Expand All @@ -12,7 +17,7 @@ def async_post_accounting_export_summary(org_id: str, workspace_id: int) -> None
async_task('apps.fyle.tasks.post_accounting_export_summary', org_id, workspace_id)


def async_import_and_export_expenses(body: dict) -> None:
def async_import_and_export_expenses(body: dict, workspace_id: int) -> None:
"""
Async'ly import and export expenses
:param body: body
Expand All @@ -21,4 +26,11 @@ def async_import_and_export_expenses(body: dict) -> None:
if body.get('action') == 'ACCOUNTING_EXPORT_INITIATED' and body.get('data'):
report_id = body['data']['id']
org_id = body['data']['org_id']
assert_valid_request(workspace_id=workspace_id, fyle_org_id=org_id)
async_task('apps.fyle.tasks.import_and_export_expenses', report_id, org_id)

elif body.get('action') == 'UPDATED_AFTER_APPROVAL' and body.get('data') and body.get('resource') == 'EXPENSE':
org_id = body['data']['org_id']
logger.info("| Updating non-exported expenses through webhook | Content: {{WORKSPACE_ID: {} Payload: {}}}".format(workspace_id, body.get('data')))
assert_valid_request(workspace_id=workspace_id, fyle_org_id=org_id)
Copy link
Contributor

Choose a reason for hiding this comment

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

let's move this out of the if block since this is repeated in both blocks and common for both

async_task('apps.fyle.tasks.update_non_exported_expenses', body['data'])
26 changes: 26 additions & 0 deletions apps/fyle/tasks.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from fyle.platform.exceptions import RetryException
from fyle_accounting_mappings.models import ExpenseAttribute
from fyle_integrations_platform_connector import PlatformConnector
from fyle_integrations_platform_connector.apis.expenses import Expenses as FyleExpenses

from apps.fyle.actions import create_generator_and_post_in_batches
from apps.fyle.enums import ExpenseStateEnum, FundSourceEnum, PlatformExpensesEnum
Expand Down Expand Up @@ -308,3 +309,28 @@ def post_accounting_export_summary(org_id: str, workspace_id: int, fund_source:
accounting_export_summary_batches
)
create_generator_and_post_in_batches(accounting_export_summary_batches, platform, workspace_id)


def update_non_exported_expenses(data: Dict) -> None:
"""
To update expenses not in COMPLETE, IN_PROGRESS state
"""
expense_state = None
org_id = data['org_id']
expense_id = data['id']
workspace = Workspace.objects.get(fyle_org_id = org_id)
expense = Expense.objects.filter(workspace_id=workspace.id, expense_id=expense_id).first()

if expense:
if 'state' in expense.accounting_export_summary:
expense_state = expense.accounting_export_summary['state']
else:
expense_state = 'NOT_EXPORTED'

if expense_state and expense_state not in ['COMPLETE', 'IN_PROGRESS']:
expense_obj = []
expense_obj.append(data)
expense_objects = FyleExpenses().construct_expense_object(expense_obj, expense.workspace_id)
Expense.create_expense_objects(
expense_objects, expense.workspace_id
)
Comment on lines +314 to +336
Copy link

Choose a reason for hiding this comment

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

New function to update non-exported expenses

This function updates expenses that are not in the COMPLETE or IN_PROGRESS state. It's a valuable addition for handling edge cases in expense management. However, the implementation can be optimized by using .get() for dictionary access with a default value.

- if 'state' in expense.accounting_export_summary:
-     expense_state = expense.accounting_export_summary['state']
- else:
-     expense_state = 'NOT_EXPORTED'
+ expense_state = expense.accounting_export_summary.get('state', 'NOT_EXPORTED')
Committable suggestion

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

Suggested change
def update_non_exported_expenses(data: Dict) -> None:
"""
To update expenses not in COMPLETE, IN_PROGRESS state
"""
expense_state = None
org_id = data['org_id']
expense_id = data['id']
workspace = Workspace.objects.get(fyle_org_id = org_id)
expense = Expense.objects.filter(workspace_id=workspace.id, expense_id=expense_id).first()
if expense:
if 'state' in expense.accounting_export_summary:
expense_state = expense.accounting_export_summary['state']
else:
expense_state = 'NOT_EXPORTED'
if expense_state and expense_state not in ['COMPLETE', 'IN_PROGRESS']:
expense_obj = []
expense_obj.append(data)
expense_objects = FyleExpenses().construct_expense_object(expense_obj, expense.workspace_id)
Expense.create_expense_objects(
expense_objects, expense.workspace_id
)
def update_non_exported_expenses(data: Dict) -> None:
"""
To update expenses not in COMPLETE, IN_PROGRESS state
"""
expense_state = None
org_id = data['org_id']
expense_id = data['id']
workspace = Workspace.objects.get(fyle_org_id = org_id)
expense = Expense.objects.filter(workspace_id=workspace.id, expense_id=expense_id).first()
if expense:
expense_state = expense.accounting_export_summary.get('state', 'NOT_EXPORTED')
if expense_state and expense_state not in ['COMPLETE', 'IN_PROGRESS']:
expense_obj = []
expense_obj.append(data)
expense_objects = FyleExpenses().construct_expense_object(expense_obj, expense.workspace_id)
Expense.create_expense_objects(
expense_objects, expense.workspace_id
)
Tools
Ruff

325-328: Use expense_state = expense.accounting_export_summary.get("state", "NOT_EXPORTED") instead of an if block (SIM401)

Replace with expense_state = expense.accounting_export_summary.get("state", "NOT_EXPORTED")

3 changes: 2 additions & 1 deletion apps/fyle/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,8 @@ class ExportView(generics.CreateAPIView):
authentication_classes = []
permission_classes = []

@handle_view_exceptions()
def post(self, request, *args, **kwargs):
async_import_and_export_expenses(request.data)
async_import_and_export_expenses(request.data, int(kwargs['workspace_id']))

return Response(data={}, status=status.HTTP_200_OK)
2 changes: 1 addition & 1 deletion requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ enum34==1.1.10
future==0.18.2
fyle==0.37.0
fyle-accounting-mappings==1.32.1
fyle-integrations-platform-connector==1.38.0
fyle-integrations-platform-connector==1.38.1
fyle-rest-auth==1.7.2
gevent==23.9.1
gunicorn==20.1.0
Expand Down
Loading
Loading