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

17829 - Implement summary route for statements. #1256

Merged
merged 2 commits into from
Sep 26, 2023
Merged
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
20 changes: 17 additions & 3 deletions pay-api/src/pay_api/resources/v1/account_statements.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,10 @@
@_jwt.requires_auth
def get_account_statements(account_id):
"""Get all statements records for an account."""
current_app.logger.info('<Aget_account_statements')
current_app.logger.info('<get_account_statements')

# Check if user is authorized to perform this action
check_auth(business_identifier=None, account_id=account_id, contains_role=EDIT_ROLE, is_premium=True)
check_auth(business_identifier=None, account_id=account_id, contains_role=EDIT_ROLE)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some cleanup, eventually we want to get rid of premium accounts


page: int = int(request.args.get('page', '1'))
limit: int = int(request.args.get('limit', '10'))
Expand All @@ -60,12 +60,26 @@ def get_account_statement(account_id: str, statement_id: str):
response_content_type = request.headers.get('Accept', ContentType.PDF.value)

# Check if user is authorized to perform this action
auth = check_auth(business_identifier=None, account_id=account_id, contains_role=EDIT_ROLE, is_premium=True)
auth = check_auth(business_identifier=None, account_id=account_id, contains_role=EDIT_ROLE)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some cleanup, eventually we want to get rid of premium accounts


report, report_name = StatementService.get_statement_report(statement_id=statement_id,
content_type=response_content_type, auth=auth)
response = Response(report, 200)
response.headers.set('Content-Disposition', 'attachment', filename=report_name)
response.headers.set('Content-Type', response_content_type)
response.headers.set('Access-Control-Expose-Headers', 'Content-Disposition')
current_app.logger.info('>get_account_statement')
return response


@bp.route('/summary', methods=['GET', 'OPTIONS'])
@cross_origin(origins='*', methods=['GET'])
@_tracing.trace()
@_jwt.requires_auth
def get_account_statement_summary(account_id: str):
"""Create the statement report."""
current_app.logger.info('<get_account_statement_summary')
check_auth(business_identifier=None, account_id=account_id, contains_role=EDIT_ROLE)
response, status = StatementService.get_summary(account_id), HTTPStatus.OK
current_app.logger.info('>get_account_statement_summary')
return jsonify(response), status
32 changes: 30 additions & 2 deletions pay-api/src/pay_api/services/statement.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,17 @@
from datetime import date, datetime

from flask import current_app
from sqlalchemy import func

from pay_api.models import db
from pay_api.models import Invoice as InvoiceModel
from pay_api.models import PaymentAccount as PaymentAccountModel
from pay_api.models import Statement as StatementModel
from pay_api.models import StatementInvoices as StatementInvoicesModel
from pay_api.models import StatementSchema as StatementModelSchema
from pay_api.utils.enums import ContentType, StatementFrequency
from pay_api.utils.enums import ContentType, InvoiceStatus, StatementFrequency
from pay_api.utils.constants import DT_SHORT_FORMAT

from pay_api.utils.util import get_local_formatted_date
from .payment import Payment as PaymentService


Expand Down Expand Up @@ -163,3 +168,26 @@ def get_statement_report(statement_id: str, content_type: str, template_name='st
current_app.logger.debug('>get_statement_report')

return report_response, report_name

@staticmethod
def get_summary(auth_account_id: str):
"""Get summary for statements by account id."""
# This is written outside of the model, because we have multiple model references that need to be included.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Perhaps worth reading

# If we include these references inside of a model, it runs the risk of having circular dependencies.
# It's easier to build out features if our models don't rely on other models.
Copy link
Collaborator Author

@seeker25 seeker25 Sep 26, 2023

Choose a reason for hiding this comment

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

Ideally resources rely on services, should rely on models, models should never rely on models - Otherwise you're in for a world of pain with circular references.

result = db.session.query(func.sum(InvoiceModel.total - InvoiceModel.paid).label('total_due'),
Copy link
Collaborator Author

@seeker25 seeker25 Sep 26, 2023

Choose a reason for hiding this comment

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

This should cover partial payments ideally (not implemented currently, maybe for online banking)

func.min(InvoiceModel.overdue_date).label('oldest_overdue_date')) \
.join(PaymentAccountModel) \
.join(StatementInvoicesModel) \
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Uses Statement invoices, so this only shows invoices that have statements. Otherwise I would have moved this into the invoice service.

.filter(PaymentAccountModel.auth_account_id == auth_account_id) \
.filter(InvoiceModel.invoice_status_code == InvoiceStatus.OVERDUE.value) \
.filter(StatementInvoicesModel.invoice_id == InvoiceModel.id) \
.group_by(InvoiceModel.payment_account_id) \
.one_or_none()

total_due = float(result.total_due) if result else 0
Copy link
Collaborator Author

@seeker25 seeker25 Sep 26, 2023

Choose a reason for hiding this comment

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

Couldn't find a nicer way to do this unfortunately. I tried .first(), .all() etc. Could be replaced by scalars() in SQLAlchemy 2 possibly

oldest_overdue_date = get_local_formatted_date(result.oldest_overdue_date) if result else None
return {
'total_due': total_due,
'oldest_overdue_date': oldest_overdue_date
}
56 changes: 55 additions & 1 deletion pay-api/tests/unit/api/test_statement.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,13 @@
Test-Suite to ensure that the /accounts endpoint is working as expected.
"""

from datetime import datetime
import json

from pay_api.models import PaymentAccount
from pay_api.models.invoice import Invoice
from pay_api.utils.enums import ContentType, StatementFrequency
from pay_api.utils.enums import ContentType, InvoiceStatus, StatementFrequency
from pay_api.utils.util import get_local_formatted_date
from tests.utilities.base_test import (
factory_statement, factory_statement_invoices, factory_statement_settings, get_claims, get_payment_request,
token_header)
Expand Down Expand Up @@ -222,3 +224,55 @@ def test_get_monthly_statement_report_as_csv(session, client, jwt, app):
rv = client.get(f'/api/v1/accounts/{pay_account.auth_account_id}/statements/{statement_model.id}',
headers=headers)
assert rv.status_code == 200


def test_statement_summary(session, client, jwt, app):
"""Assert the statement summary is working."""
headers = {
'Authorization': f'Bearer {jwt.create_jwt(get_claims(), token_header)}',
'content-type': 'application/json'
}

# Check if this works without any invoices in OVERDUE.
rv = client.post('/api/v1/payment-requests',
data=json.dumps(get_payment_request(business_identifier='CP0002000')),
headers=headers)
invoice: Invoice = Invoice.find_by_id(rv.json.get('id'))
payment_account_id = invoice.payment_account_id
pay_account: PaymentAccount = PaymentAccount.find_by_id(payment_account_id)
rv = client.get(f'/api/v1/accounts/{pay_account.auth_account_id}/statements/summary',
headers=headers)
assert rv.status_code == 200
assert rv.json.get('totalDue') == 0
assert rv.json.get('oldestOverdueDate') is None

# Create multiple OVERDUE invoices and check they add up.
total_due = 0
payment_account_id = 0
invoice_ids = []
oldest_overdue_date = datetime.now()
for _ in range(5):
rv = client.post('/api/v1/payment-requests',
data=json.dumps(get_payment_request(business_identifier='CP0002000')),
headers=headers)

invoice: Invoice = Invoice.find_by_id(rv.json.get('id'))
invoice_ids.append(invoice.id)
invoice.invoice_status_code = InvoiceStatus.OVERDUE.value
invoice.overdue_date = oldest_overdue_date
total_due += invoice.total - invoice.paid
invoice.save()

settings_model = factory_statement_settings(payment_account_id=pay_account.id,
frequency=StatementFrequency.MONTHLY.value)
statement_model = factory_statement(payment_account_id=pay_account.id,
frequency=StatementFrequency.MONTHLY.value,
statement_settings_id=settings_model.id)
for invoice_id in invoice_ids:
factory_statement_invoices(statement_id=statement_model.id, invoice_id=invoice_id)

rv = client.get(f'/api/v1/accounts/{pay_account.auth_account_id}/statements/summary',
headers=headers)
assert rv.status_code == 200
assert rv.json.get('totalDue') == float(total_due)
assert rv.json.get('oldestOverdueDate') == get_local_formatted_date(oldest_overdue_date)
Loading