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.

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

Base Export Structure added #50

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
22 changes: 22 additions & 0 deletions apps/business_central/actions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
from django.db.models import Q

from apps.accounting_exports.models import AccountingExport
from apps.workspaces.models import LastExportDetail


def update_last_export_details(workspace_id):
last_export_detail = LastExportDetail.objects.get(workspace_id=workspace_id)

failed_exports = AccountingExport.objects.filter(~Q(type__in=['FETCHING_REIMBURSABLE_EXPENSES', 'FETCHING_CREDIT_CARD_EXPENSES']), workspace_id=workspace_id, status__in=['FAILED', 'FATAL']).count()

successful_exports = AccountingExport.objects.filter(
~Q(type__in=['FETCHING_REIMBURSABLE_EXPENSES', 'FETCHING_CREDIT_CARD_EXPENSES']),
workspace_id=workspace_id, status='COMPLETE',
).count()

last_export_detail.failed_accounting_exports_count = failed_exports
last_export_detail.successful_accounting_exports_count = successful_exports
last_export_detail.total_accounting_exports_count = failed_exports + successful_exports
last_export_detail.save()

return last_export_detail
Comment on lines +7 to +22
Copy link

Choose a reason for hiding this comment

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

The function update_last_export_details uses LastExportDetail.objects.get(workspace_id=workspace_id) without handling the potential DoesNotExist exception that is raised if no LastExportDetail instance is found for the given workspace_id. It's important to handle this case to avoid unhandled exceptions which could disrupt the flow of the application.

 def update_last_export_details(workspace_id):
+    try:
         last_export_detail = LastExportDetail.objects.get(workspace_id=workspace_id)
+    except LastExportDetail.DoesNotExist:
+        # Handle the exception, e.g., create a new LastExportDetail instance or log an error
+        pass

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.

Suggested change
def update_last_export_details(workspace_id):
last_export_detail = LastExportDetail.objects.get(workspace_id=workspace_id)
failed_exports = AccountingExport.objects.filter(~Q(type__in=['FETCHING_REIMBURSABLE_EXPENSES', 'FETCHING_CREDIT_CARD_EXPENSES']), workspace_id=workspace_id, status__in=['FAILED', 'FATAL']).count()
successful_exports = AccountingExport.objects.filter(
~Q(type__in=['FETCHING_REIMBURSABLE_EXPENSES', 'FETCHING_CREDIT_CARD_EXPENSES']),
workspace_id=workspace_id, status='COMPLETE',
).count()
last_export_detail.failed_accounting_exports_count = failed_exports
last_export_detail.successful_accounting_exports_count = successful_exports
last_export_detail.total_accounting_exports_count = failed_exports + successful_exports
last_export_detail.save()
return last_export_detail
def update_last_export_details(workspace_id):
try:
last_export_detail = LastExportDetail.objects.get(workspace_id=workspace_id)
except LastExportDetail.DoesNotExist:
# Handle the exception, e.g., create a new LastExportDetail instance or log an error
pass
failed_exports = AccountingExport.objects.filter(~Q(type__in=['FETCHING_REIMBURSABLE_EXPENSES', 'FETCHING_CREDIT_CARD_EXPENSES']), workspace_id=workspace_id, status__in=['FAILED', 'FATAL']).count()
successful_exports = AccountingExport.objects.filter(
~Q(type__in=['FETCHING_REIMBURSABLE_EXPENSES', 'FETCHING_CREDIT_CARD_EXPENSES']),
workspace_id=workspace_id, status='COMPLETE',
).count()
last_export_detail.failed_accounting_exports_count = failed_exports
last_export_detail.successful_accounting_exports_count = successful_exports
last_export_detail.total_accounting_exports_count = failed_exports + successful_exports
last_export_detail.save()
return last_export_detail

76 changes: 76 additions & 0 deletions apps/business_central/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@

import logging
import traceback

from dynamics.exceptions.dynamics_exceptions import WrongParamsError

from apps.accounting_exports.models import AccountingExport, Error
from apps.business_central.actions import update_last_export_details
from apps.workspaces.models import BusinessCentralCredentials, FyleCredential
from ms_business_central_api.exceptions import BulkError

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


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]
Copy link

Choose a reason for hiding this comment

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

Consider using keyword arguments or explicit argument names to access accounting_export within the decorator to avoid potential errors if the function signature changes in the future.


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('Business Central 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': 'Business Central 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_last_export_details(accounting_export.workspace_id)
Comment on lines +36 to +72
Copy link

Choose a reason for hiding this comment

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

There is repeated code for updating accounting_export status and saving it within each except block. Consider refactoring to a common function or using a finally block to reduce duplication and improve maintainability.


return new_fn

return decorator
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
Empty file.
21 changes: 21 additions & 0 deletions apps/workspaces/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,11 @@ class Meta:
('CREATED_AT', 'created_at')
)

EXPORT_MODE_CHOICES = (
('MANUAL', 'MANUAL'),
('AUTO', 'AUTO')
)


class BusinessCentralCredentials(BaseModel):
"""
Expand Down Expand Up @@ -227,3 +232,19 @@ class AdvancedSetting(BaseModel):

class Meta:
db_table = 'advanced_settings'


class LastExportDetail(BaseModel):
"""
Table to store Last Export Details
"""

id = models.AutoField(primary_key=True)
last_exported_at = models.DateTimeField(help_text='Last exported at datetime', null=True)
export_mode = models.CharField(max_length=50, help_text='Mode of the export Auto / Manual', choices=EXPORT_MODE_CHOICES, null=True)
total_accounting_exports_count = models.IntegerField(help_text='Total count of accounting exports exported', null=True)
successful_accounting_exports_count = models.IntegerField(help_text='count of successful accounting_exports ', null=True)
failed_accounting_exports_count = models.IntegerField(help_text='count of failed accounting_exports ', null=True)

class Meta:
db_table = 'last_export_details'
Comment on lines +237 to +250
Copy link

Choose a reason for hiding this comment

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

The LastExportDetail model is well-defined with appropriate field types, help texts, and nullability. However, consider adding indexes to frequently queried fields like last_exported_at and export_mode to improve database performance.

class LastExportDetail(BaseModel):
    ...
    last_exported_at = models.DateTimeField(help_text='Last exported at datetime', null=True, db_index=True)
    export_mode = models.CharField(max_length=50, help_text='Mode of the export Auto / Manual', choices=EXPORT_MODE_CHOICES, null=True, db_index=True)
    ...

3 changes: 3 additions & 0 deletions apps/workspaces/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
ExportSetting,
FyleCredential,
ImportSetting,
LastExportDetail,
Workspace,
)
from ms_business_central_api.utils import assert_valid
Expand Down Expand Up @@ -63,6 +64,8 @@ def create(self, validated_data):

cluster_domain = get_cluster_domain(auth_tokens.refresh_token)

LastExportDetail.objects.create(workspace_id=workspace.id)

FyleCredential.objects.update_or_create(
refresh_token=auth_tokens.refresh_token,
workspace_id=workspace.id,
Expand Down
22 changes: 22 additions & 0 deletions ms_business_central_api/exceptions.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@

"""
Exceptions
"""


class BulkError(Exception):
"""
Bulk Error Exception.

Parameters:
msg (str): Short description of the error.
response: Error response.
"""

def __init__(self, msg, response=None):
super().__init__(msg)
self.message = msg
self.response = response

def __str__(self):
return repr(self.message)
Loading