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

17534 - nsf tweaks #1385

Merged
merged 10 commits into from
Jan 24, 2024
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
33 changes: 33 additions & 0 deletions pay-api/migrations/versions/2024_01_23_0b1f9c35b1fb_.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
"""Add cfs_account and modify invoice_number to be non-nullable in non_sufficient_funds table

Revision ID: 0b1f9c35b1fb
Revises: fccdab259e05
Create Date: 2024-01-23 14:26:29.427462

"""
from alembic import op
import sqlalchemy as sa
from sqlalchemy.dialects import postgresql

# revision identifiers, used by Alembic.
revision = '0b1f9c35b1fb'
down_revision = 'fccdab259e05'
branch_labels = None
depends_on = None


def upgrade():
op.add_column('non_sufficient_funds', sa.Column('cfs_account', sa.String(length=50), nullable=True,
comment='CFS Account number'))
op.alter_column('non_sufficient_funds', 'invoice_number',
existing_type=sa.VARCHAR(length=50),
nullable=False,
existing_comment='CFS Invoice number')


def downgrade():
op.alter_column('non_sufficient_funds', 'invoice_number',
existing_type=sa.VARCHAR(length=50),
nullable=True,
existing_comment='CFS Invoice number')
op.drop_column('non_sufficient_funds', 'cfs_account')
9 changes: 6 additions & 3 deletions pay-api/src/pay_api/models/non_sufficient_funds.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,15 @@ class NonSufficientFundsModel(BaseModel): # pylint: disable=too-many-instance-a
__mapper_args__ = {
'include_properties': [
'id',
'cfs_account',
'description',
'invoice_id',
'invoice_number'
]
}

id = db.Column(db.Integer, primary_key=True, autoincrement=True)
cfs_account = db.Column(db.String(50), nullable=True, comment='CFS Account number')
description = db.Column(db.String(50), nullable=True)
invoice_id = db.Column(db.Integer, ForeignKey('invoices.id'), nullable=False)
invoice_number = db.Column(db.String(50), nullable=False, index=True, comment='CFS Invoice number')
Expand All @@ -55,15 +57,16 @@ class NonSufficientFundsSchema: # pylint: disable=too-few-public-methods
"""Used to search for NSF records."""

id: int
description: str
cfs_account: int
invoice_id: int
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still need invoice id?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We could do away with it if there is no need for OPS to have this information about a specific invoice, and we're currently still using it on the DB queries (but could query around it).

Copy link
Collaborator

@seeker25 seeker25 Jan 24, 2024

Choose a reason for hiding this comment

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

I think it's a bit confusing to leave it in there (or at least as invoice_id), because the NSF pertains to multiple invoice ids (grouped up by the invoice_number) and you're not creating multiple rows.. Maybe we could use a better name for it instead?
nsf_fee_invoice_id? that way it's distinct from the other invoice_ids that are NSF

invoice_number: str
description: str

@classmethod
def from_row(cls, row: NonSufficientFundsModel):
"""From row is used so we don't tightly couple to our database class.

https://www.attrs.org/en/stable/init.html
"""
return cls(id=row.id, invoice_id=row.invoice_id, invoice_number=row.invoice_number,
description=row.description)
return cls(id=row.id, cfs_account=row.cfs_account, description=row.description,
invoice_id=row.invoice_id, invoice_number=row.invoice_number)
47 changes: 35 additions & 12 deletions pay-api/src/pay_api/services/non_sufficient_funds.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,45 +53,67 @@ def populate(value: NonSufficientFundsModel):
return non_sufficient_funds_service

@staticmethod
def save_non_sufficient_funds(invoice_id: int, invoice_number: str, description: str) -> NonSufficientFundsService:
def save_non_sufficient_funds(invoice_id: int, invoice_number: str, cfs_account: int,
description: str) -> NonSufficientFundsService:
"""Create Non-Sufficient Funds record."""
current_app.logger.debug('<save_non_sufficient_funds')
non_sufficient_funds_service = NonSufficientFundsService()

non_sufficient_funds_service.dao.description = description
non_sufficient_funds_service.dao.invoice_id = invoice_id
non_sufficient_funds_service.dao.invoice_number = invoice_number
non_sufficient_funds_service.dao.cfs_account = cfs_account
non_sufficient_funds_dao = non_sufficient_funds_service.dao.save()

non_sufficient_funds_service = NonSufficientFundsService.populate(non_sufficient_funds_dao)
current_app.logger.debug('>save_non_sufficient_funds')
return NonSufficientFundsService.asdict(non_sufficient_funds_service)

@staticmethod
def exists_for_invoice_number(invoice_number: str) -> bool:
"""Return boolean if a row exists for the invoice number."""
return (db.session.query(NonSufficientFundsModel)
.filter(NonSufficientFundsModel.invoice_number == invoice_number)
.count()
) > 0

@staticmethod
def query_all_non_sufficient_funds_invoices(account_id: str):
"""Return all Non-Sufficient Funds invoices and their aggregate amounts."""
query = (db.session.query(
InvoiceModel, InvoiceReferenceModel)
.join(InvoiceReferenceModel, InvoiceReferenceModel.invoice_id == InvoiceModel.id)
.outerjoin(NonSufficientFundsModel, NonSufficientFundsModel.invoice_id == InvoiceModel.id)
.join(NonSufficientFundsModel,
NonSufficientFundsModel.invoice_number == InvoiceReferenceModel.invoice_number)
.join(PaymentAccountModel, PaymentAccountModel.id == InvoiceModel.payment_account_id)
.filter(PaymentAccountModel.auth_account_id == account_id)
.distinct(InvoiceModel.id)
.group_by(InvoiceModel.id, InvoiceReferenceModel.id)
)

totals_query = (db.session.query(
func.sum(InvoiceModel.total - InvoiceModel.paid).label('total_amount_remaining'),
func.max(case(
[(PaymentLineItemModel.description == ReverseOperation.NSF.value, PaymentLineItemModel.total)],
else_=0))
.label('nsf_amount'),
func.sum(InvoiceModel.total - case(
[(PaymentLineItemModel.description == ReverseOperation.NSF.value, PaymentLineItemModel.total)],
else_=0)).label('total_amount'))
invoice_totals_subquery = (
db.session.query(
InvoiceModel.id.label('invoice_id'),
(InvoiceModel.total - InvoiceModel.paid).label('amount_remaining'),
func.max(case(
[(PaymentLineItemModel.description == ReverseOperation.NSF.value, PaymentLineItemModel.total)],
else_=0)).label('nsf_amount')
)
.join(PaymentAccountModel, PaymentAccountModel.id == InvoiceModel.payment_account_id)
.outerjoin(NonSufficientFundsModel, NonSufficientFundsModel.invoice_id == InvoiceModel.id)
.join(PaymentLineItemModel, PaymentLineItemModel.invoice_id == InvoiceModel.id)
.filter(PaymentAccountModel.auth_account_id == account_id)
.group_by(InvoiceModel.id)
.subquery()
)

totals_query = (
db.session.query(
func.sum(invoice_totals_subquery.c.amount_remaining).label('total_amount_remaining'),
func.sum(invoice_totals_subquery.c.nsf_amount).label('nsf_amount'),
func.sum(invoice_totals_subquery.c.amount_remaining - invoice_totals_subquery.c.nsf_amount)
.label('total_amount')
)
)

aggregate_totals = totals_query.one()
Expand Down Expand Up @@ -134,7 +156,8 @@ def create_non_sufficient_funds_statement_pdf(account_id: str, **kwargs):
endpoint=account_url, token=kwargs['user'].bearer_token,
auth_header_type=AuthHeaderType.BEARER, content_type=ContentType.JSON).json()
template_vars = {
'suspendedOn': datetime.strptime(account['suspendedOn'], '%Y-%m-%dT%H:%M:%S%z').strftime('%B %-d, %Y'),
'suspendedOn': datetime.strptime(account['suspendedOn'], '%Y-%m-%dT%H:%M:%S%z')
.strftime('%B %-d, %Y') if 'suspendedOn' in account else None,
'accountNumber': cfs_account.cfs_account,
'businessName': account['businessName'],
'totalAmountRemaining': invoice['total_amount_remaining'],
Expand Down
4 changes: 2 additions & 2 deletions pay-api/tests/unit/api/test_non_sufficient_funds.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,6 @@ def test_get_non_sufficient_funds(session, client, jwt, app):
assert nsf.status_code == 200
assert len(nsf.json.get('invoices')) == 1
assert nsf.json.get('total') == 1
assert nsf.json.get('totalAmount') == 30
assert nsf.json.get('totalAmountRemaining') == 60
assert nsf.json.get('totalAmount') == 0
assert nsf.json.get('totalAmountRemaining') == 30
assert nsf.json.get('nsfAmount') == 30
5 changes: 3 additions & 2 deletions pay-api/tests/unit/services/test_non_sufficient_funds.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ def test_save_non_sufficient_funds(session):
invoice.save()
non_sufficient_funds = NonSufficientFundsService.save_non_sufficient_funds(invoice_id=invoice.id,
invoice_number=payment.invoice_number,
cfs_account='1234567890',
description='NSF')
assert non_sufficient_funds
assert non_sufficient_funds['description'] == 'NSF'
Expand Down Expand Up @@ -85,6 +86,6 @@ def test_find_all_non_sufficient_funds_invoices(session):
assert 'total' in find_non_sufficient_funds
assert len(find_non_sufficient_funds['invoices']) == 1
assert find_non_sufficient_funds['total'] == 1
assert find_non_sufficient_funds['total_amount'] == 30.0
assert find_non_sufficient_funds['total_amount_remaining'] == 60.0
assert find_non_sufficient_funds['total_amount'] == 0
assert find_non_sufficient_funds['total_amount_remaining'] == 30.0
assert find_non_sufficient_funds['nsf_amount'] == 30.0
Loading