17829 - Implement summary route for statements. #1256
Conversation
|
|
||
| # 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) |
There was a problem hiding this comment.
Some cleanup, eventually we want to get rid of premium accounts
| @@ -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) | |||
There was a problem hiding this comment.
Some cleanup, eventually we want to get rid of premium accounts
| .group_by(InvoiceModel.payment_account_id) \ | ||
| .one_or_none() | ||
|
|
||
| total_due = float(result.total_due) if result else 0 |
There was a problem hiding this comment.
Couldn't find a nicer way to do this unfortunately. I tried .first(), .all() etc. Could be replaced by scalars() in SQLAlchemy 2 possibly
| @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. |
There was a problem hiding this comment.
Perhaps worth reading
|
Kudos, SonarCloud Quality Gate passed!
|
| # This is written outside of the model, because we have multiple model references that need to be included. | ||
| # 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. | ||
| result = db.session.query(func.sum(InvoiceModel.total - InvoiceModel.paid).label('total_due'), |
There was a problem hiding this comment.
This should cover partial payments ideally (not implemented currently, maybe for online banking)
| result = db.session.query(func.sum(InvoiceModel.total - InvoiceModel.paid).label('total_due'), | ||
| func.min(InvoiceModel.overdue_date).label('oldest_overdue_date')) \ | ||
| .join(PaymentAccountModel) \ | ||
| .join(StatementInvoicesModel) \ |
There was a problem hiding this comment.
Uses Statement invoices, so this only shows invoices that have statements. Otherwise I would have moved this into the invoice service.
Codecov Report
@@ Coverage Diff @@
## main #1256 +/- ##
==========================================
+ Coverage 91.45% 91.47% +0.02%
==========================================
Files 186 177 -9
Lines 11319 10514 -805
==========================================
- Hits 10352 9618 -734
+ Misses 967 896 -71
Flags with carried forward coverage won't be shown. Click here to find out more.
|
| """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. | ||
| # 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. |
There was a problem hiding this comment.
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.
Issue #:
bcgov/entity#17829
Description of changes:
Return invoices that are included in summaries, for the pending balance.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the sbc-pay license (Apache 2.0).