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

22655 - Wire in reverse_invoice for consolidated invoices #1702

Merged
merged 66 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 63 commits
Commits
Show all changes
66 commits
Select commit Hold shift + click to select a range
ac78d3b
Wire in reverse_invoice for consolidated invoices plus unit test
seeker25 Aug 22, 2024
544f31a
small fix
seeker25 Aug 22, 2024
8c5f62c
small text change
seeker25 Aug 22, 2024
0bb63d8
move statement up a bit
seeker25 Aug 22, 2024
8b382d2
clean up
seeker25 Aug 22, 2024
db77b79
lint fix
seeker25 Aug 22, 2024
3a2456b
Remove WIRE code
seeker25 Aug 26, 2024
2527c73
Revert back to using reverse_invoice instead of adjust_invoice
seeker25 Aug 26, 2024
39b5331
remove unused function
seeker25 Aug 26, 2024
19a1850
Lint fixes
seeker25 Aug 26, 2024
ea4026d
Throw in auth_account_override so we don't worry about locking other …
seeker25 Aug 26, 2024
f44062a
cleanup unused task
seeker25 Aug 26, 2024
b99ae7e
fix override for overdue flow
seeker25 Aug 26, 2024
22761aa
add in auth_account_override to overdue status updater
seeker25 Aug 26, 2024
ecafe65
spacing
seeker25 Aug 26, 2024
7b4fed0
spacing
seeker25 Aug 26, 2024
4502c3f
unit test fixes
seeker25 Aug 26, 2024
6e3336d
Fix unlock + receipt details
seeker25 Aug 26, 2024
761ca96
Statement changes
seeker25 Aug 27, 2024
e24a660
Merge branch 'main' of https://github.com/bcgov/sbc-pay into 22655
seeker25 Aug 27, 2024
23a0728
Build new statementDTO class
seeker25 Aug 27, 2024
25c4485
small clean up
seeker25 Aug 27, 2024
dc867c8
Add in statements to payload
seeker25 Aug 27, 2024
7604a08
Merge branch 'main' of https://github.com/bcgov/sbc-pay into 22655
seeker25 Aug 27, 2024
0e36f8e
Merge branch 'main' of https://github.com/bcgov/sbc-pay into 22655
seeker25 Aug 27, 2024
aab88f0
Merge branch 'main' of https://github.com/bcgov/sbc-pay into 22655
seeker25 Aug 27, 2024
1b498fd
Merge branch 'main' of https://github.com/bcgov/sbc-pay into 22655
seeker25 Aug 27, 2024
27bd817
put back statement settings
seeker25 Aug 27, 2024
1ff423a
put back statements
seeker25 Aug 27, 2024
e981987
Merge branch 'main' of https://github.com/bcgov/sbc-pay into 22655
seeker25 Aug 28, 2024
cf7b2ce
revert change
seeker25 Aug 28, 2024
42d9043
revert
seeker25 Aug 28, 2024
3e60006
Filter out consolidated invoices for EFT_TASK, also only reverse exis…
seeker25 Aug 28, 2024
cf4dee9
Changes for consolidation
seeker25 Aug 28, 2024
80aaf58
Code to detect consolidated invoices
seeker25 Aug 28, 2024
f206e3a
Handle consolidated invoices in eft_Task, handle creating a unique co…
seeker25 Aug 28, 2024
007b294
update comment
seeker25 Aug 28, 2024
51f01cb
fix reversing consolidated invoices
seeker25 Aug 28, 2024
33928a3
small hint fix
seeker25 Aug 28, 2024
1e5e176
comment changes
seeker25 Aug 28, 2024
11586fc
comment changes
seeker25 Aug 28, 2024
8de4afd
Small refactor
seeker25 Aug 29, 2024
9cdbbdc
Fix todo, other general clean up
seeker25 Aug 29, 2024
0d2014c
use is_consolidated
seeker25 Aug 29, 2024
b9589ed
Suggestions from feedback
seeker25 Aug 29, 2024
dcd4cf1
small tweaks
seeker25 Aug 29, 2024
4c6648c
lint fixes
seeker25 Aug 29, 2024
482b820
is consolidated true
seeker25 Aug 29, 2024
70ae7d9
incorrect
seeker25 Aug 29, 2024
58ebb91
use payment.invoice_number
seeker25 Aug 29, 2024
3f78317
Add in condition for PAD/EFT
seeker25 Aug 29, 2024
8eb7eb8
Merge branch 'main' of https://github.com/bcgov/sbc-pay into 22655
seeker25 Aug 29, 2024
5582825
add mapper fields
seeker25 Aug 29, 2024
6b87734
Unit tests for eft_task
seeker25 Aug 29, 2024
1c6796a
cleanup
seeker25 Aug 29, 2024
d7bae63
add in is_consolidated
seeker25 Aug 29, 2024
dfb2f6b
update pay-queue refs
seeker25 Aug 29, 2024
26bf445
Poetry lock fix
seeker25 Aug 29, 2024
e047813
test cleanup
seeker25 Aug 30, 2024
d8f4d88
A bit more cleanup and changes
seeker25 Aug 30, 2024
c864806
unit test fixes
seeker25 Aug 30, 2024
4ccaffe
final test fixes
seeker25 Aug 30, 2024
c91afb1
rename
seeker25 Aug 30, 2024
f20cae2
test and lint fixes, finally
seeker25 Aug 30, 2024
02de543
payment_account_id missing for payment
seeker25 Aug 30, 2024
5a3f769
small refactor
seeker25 Aug 30, 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
6 changes: 3 additions & 3 deletions jobs/payment-jobs/poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion jobs/payment-jobs/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ readme = "README.md"

[tool.poetry.dependencies]
python = "^3.12"
pay-api = {git = "https://github.com/seeker25/sbc-pay.git", branch = "22992", subdirectory = "pay-api"}
pay-api = {git = "https://github.com/seeker25/sbc-pay.git", branch = "22655", subdirectory = "pay-api"}
gunicorn = "^21.2.0"
flask = "^3.0.2"
flask-sqlalchemy = "^3.1.1"
Expand Down
64 changes: 38 additions & 26 deletions jobs/payment-jobs/tasks/eft_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
from pay_api.services.cfs_service import CFSService
from pay_api.services.eft_service import EftService
from pay_api.services.invoice import Invoice as InvoiceService
from pay_api.utils.constants import CFS_ADJ_ACTIVITY_NAME
from pay_api.utils.enums import (
CfsAccountStatus, DisbursementStatus, EFTCreditInvoiceStatus, InvoiceReferenceStatus, InvoiceStatus, PaymentMethod,
PaymentStatus, PaymentSystem, ReverseOperation)
Expand Down Expand Up @@ -116,16 +115,15 @@ class EFTCILRollup:
@classmethod
def link_electronic_funds_transfers_cfs(cls) -> dict:
"""Replicate linked EFT's as receipts inside of CFS and mark invoices as paid."""
target_status = EFTCreditInvoiceStatus.PENDING.value
credit_invoice_links = cls.get_eft_credit_invoice_links_by_status(target_status)
credit_invoice_links = cls.get_eft_credit_invoice_links_by_status(EFTCreditInvoiceStatus.PENDING.value)
cls.history_group_ids = set()
for invoice, cfs_account, cil_rollup in credit_invoice_links:
try:
current_app.logger.info(f'PayAccount: {invoice.payment_account_id} Id: {cil_rollup.id} -'
f' Invoice Id: {invoice.id} - Amount: {cil_rollup.rollup_amount}')
receipt_number = f'EFTCIL{cil_rollup.id}'
if invoice.invoice_status_code == InvoiceStatus.OVERDUE.value:
cls.overdue_account_ids[invoice.payment_account_id] = cfs_account.payment_account
receipt_number = f'EFTCIL{cil_rollup.id}'
cls._create_receipt_and_invoice(cfs_account, cil_rollup, invoice, receipt_number)
cls._update_cil_and_shortname_history(cil_rollup, receipt_number=receipt_number)
db.session.commit()
Expand Down Expand Up @@ -181,10 +179,13 @@ def handle_unlinked_refund_requested_invoices(cls):
for invoice in invoices:
cfs_account = CfsAccountModel.find_effective_by_payment_method(invoice.payment_account_id,
PaymentMethod.EFT.value)
if not cfs_account:
current_app.logger.error(f'No EFT CFS Account found for pay account id={invoice.payment_account_id}')
continue
invoice_reference = InvoiceReferenceModel.find_by_invoice_id_and_status(
invoice.id, InvoiceReferenceStatus.ACTIVE.value)
try:
cls._handle_invoice_refund(cfs_account, invoice, invoice_reference)
cls._handle_invoice_refund(invoice, invoice_reference)
db.session.commit()
except Exception as e: # NOQA # pylint: disable=broad-except
capture_message(
Expand Down Expand Up @@ -248,8 +249,29 @@ def _create_receipt_and_invoice(cls,
if not (invoice_reference := InvoiceReferenceModel.find_by_invoice_id_and_status(
cil_rollup.invoice_id, InvoiceReferenceStatus.ACTIVE.value
)):
raise Exception(f'Active Invoice reference not ' # pylint: disable=broad-exception-raised
f'found for invoice id: {invoice.id}')
raise LookupError(f'Active Invoice reference not '
f'found for invoice id: {invoice.id}')
if invoice_reference.is_consolidated:
original_invoice_reference = InvoiceReferenceModel.find_by_invoice_id_and_status(
cil_rollup.invoice_id, InvoiceReferenceStatus.CANCELLED.value, exclude_consolidated=True
)
if not original_invoice_reference:
raise LookupError(f'Non consolidated cancelled invoice reference not '
f'found for invoice id: {invoice.id}')
invoice_response = CFSService.get_invoice(cfs_account=cfs_account,
inv_number=original_invoice_reference.invoice_number)
cfs_total = Decimal(invoice_response.get('total', '0'))
invoice_total_matches = cfs_total == invoice.total
if not invoice_total_matches:
raise ValueError(f'SBC-PAY Invoice total {invoice.total} does not match CFS total {cfs_total}')
# Note we do the opposite of this in payment_account.
current_app.logger.info(f'Consolidated invoice found, reversing consolidated '
f'invoice {invoice_reference.invoice_number}.')
CFSService.reverse_invoice(invoice_reference.invoice_number)
invoice_reference.status_code = InvoiceReferenceStatus.CANCELLED.value
invoice_reference.flush()
invoice_reference = original_invoice_reference

invoice_reference.status_code = InvoiceReferenceStatus.COMPLETED.value
invoice_reference.flush()
# Note: Not creating the entire EFT as a receipt because it can be mapped to multiple CFS accounts.
Expand Down Expand Up @@ -284,7 +306,7 @@ def _create_receipt_and_invoice(cls,
def _rollback_receipt_and_invoice(cls, cfs_account: CfsAccountModel,
invoice: InvoiceModel,
receipt_number: str,
cil_status_code):
cil_status_code: str):
"""Rollback receipt in CFS and reset invoice status."""
invoice_reference_requirement = {
EFTCreditInvoiceStatus.PENDING_REFUND.value: InvoiceReferenceStatus.COMPLETED.value,
Expand All @@ -294,14 +316,16 @@ def _rollback_receipt_and_invoice(cls, cfs_account: CfsAccountModel,
invoice_reference = InvoiceReferenceModel.find_by_invoice_id_and_status(
invoice.id, invoice_reference_status
)
if invoice_reference and invoice_reference.is_consolidated:
raise ValueError(f'Cannot reverse a consolidated invoice {invoice_reference.invoice_number}')
if cil_status_code != EFTCreditInvoiceStatus.CANCELLED.value and not invoice_reference:
raise Exception(f'{invoice_reference_status} invoice reference ' # pylint: disable=broad-exception-raised
f'not found for invoice id: {invoice.id} - {invoice.invoice_status_code}')
raise LookupError(f'{invoice_reference_status} invoice reference '
f'not found for invoice id: {invoice.id} - {invoice.invoice_status_code}')
is_invoice_refund = invoice.invoice_status_code == InvoiceStatus.REFUND_REQUESTED.value
is_reversal = not is_invoice_refund
CFSService.reverse_rs_receipt_in_cfs(cfs_account, receipt_number, ReverseOperation.VOID.value)
if is_invoice_refund:
cls._handle_invoice_refund(cfs_account, invoice, invoice_reference)
cls._handle_invoice_refund(invoice, invoice_reference)
else:
invoice_reference.status_code = InvoiceReferenceStatus.ACTIVE.value
invoice.paid = 0
Expand All @@ -317,28 +341,16 @@ def _rollback_receipt_and_invoice(cls, cfs_account: CfsAccountModel,

@classmethod
def _handle_invoice_refund(cls,
cfs_account: CfsAccountModel,
invoice: InvoiceModel,
invoice_reference: InvoiceReferenceModel):
"""Handle invoice refunds adjustment on a non-rolled up invoice."""
if invoice_reference:
adjustment_lines = cls._build_reversal_adjustment_lines(invoice)
CFSService.adjust_invoice(cfs_account, invoice_reference.invoice_number, adjustment_lines=adjustment_lines)
if invoice_reference.is_consolidated:
raise ValueError(f'Cannot reverse a consolidated invoice: {invoice_reference.invoice_number}')
CFSService.reverse_invoice(invoice_reference.invoice_number)
invoice_reference.status_code = InvoiceReferenceStatus.CANCELLED.value
invoice_reference.flush()
invoice.invoice_status_code = InvoiceStatus.REFUNDED.value
invoice.refund_date = datetime.now(tz=timezone.utc)
invoice.refund = invoice.total
invoice.flush()

@classmethod
def _build_reversal_adjustment_lines(cls, invoice: InvoiceModel) -> list:
"""Build the adjustment lines for the invoice."""
return [
{
'line_number': line['line_number'],
'adjustment_amount': line['unit_price'],
'activity_name': CFS_ADJ_ACTIVITY_NAME
}
for line in CFSService.build_lines(invoice.payment_line_items, negate=True)
]
6 changes: 4 additions & 2 deletions jobs/payment-jobs/tests/jobs/factory.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,11 +164,13 @@ def factory_payment_line_item(invoice_id: str, fee_schedule_id: int, filing_fees


def factory_invoice_reference(invoice_id: int, invoice_number: str = '10021',
status_code=InvoiceReferenceStatus.ACTIVE.value):
status_code=InvoiceReferenceStatus.ACTIVE.value,
is_consolidated=False):
"""Return Factory."""
return InvoiceReference(invoice_id=invoice_id,
status_code=status_code,
invoice_number=invoice_number).save()
invoice_number=invoice_number,
is_consolidated=is_consolidated).save()


def factory_create_online_banking_account(auth_account_id='1234', status=CfsAccountStatus.PENDING.value,
Expand Down
69 changes: 62 additions & 7 deletions jobs/payment-jobs/tests/jobs/test_eft_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,9 @@ def test_eft_credit_invoice_links_by_status(session, test_name, payment_method,
assert len(results) == 1


def test_link_electronic_funds_transfers(session):
@pytest.mark.parametrize('test_name', ('happy_path', 'consolidated_happy', 'consolidated_mismatch',
'normal_invoice_missing'))
def test_link_electronic_funds_transfers(session, test_name):
"""Test link electronic funds transfers."""
auth_account_id, eft_file, short_name_id, eft_transaction_id = setup_eft_credit_invoice_links_test()
payment_account = factory_create_eft_account(auth_account_id=auth_account_id, status=CfsAccountStatus.ACTIVE.value)
Expand All @@ -169,13 +171,48 @@ def test_link_electronic_funds_transfers(session):

cfs_account = CfsAccountModel.find_effective_by_payment_method(
payment_account.id, PaymentMethod.EFT.value)
return_value = {}
original_invoice_reference = None

with patch('pay_api.services.CFSService.create_cfs_receipt') as mock_create_cfs:
EFTTask.link_electronic_funds_transfers_cfs()
mock_create_cfs.assert_called()
match test_name:
case 'consolidated_happy' | 'consolidated_mismatch':
invoice_reference.is_consolidated = True
invoice_reference.save()
original_invoice_reference = factory_invoice_reference(invoice_id=invoice.id,
is_consolidated=False,
status_code=InvoiceReferenceStatus.CANCELLED.value) \
.save()
return_value = {'total': 10.00}
if test_name == 'consolidated_mismatch':
return_value = {'total': 10.01}
case 'normal_invoice_missing':
invoice_reference.is_consolidated = True
invoice_reference.save()
case _:
pass

if test_name in ['consolidated_mismatch', 'normal_invoice_missing']:
with patch('pay_api.services.CFSService.get_invoice', return_value=return_value) as mock_get_invoice:
EFTTask.link_electronic_funds_transfers_cfs()
# No change, the amount didn't match or normal invoice was missing.
assert invoice_reference.status_code == InvoiceReferenceStatus.ACTIVE.value
return

with patch('pay_api.services.CFSService.reverse_invoice') as mock_reverse_invoice:
with patch('pay_api.services.CFSService.create_cfs_receipt') as mock_create_receipt:
with patch('pay_api.services.CFSService.get_invoice', return_value=return_value) as mock_get_invoice:
EFTTask.link_electronic_funds_transfers_cfs()
if test_name == 'consolidated_happy':
mock_reverse_invoice.assert_called()
mock_get_invoice.assert_called()
mock_create_receipt.assert_called()

assert cfs_account.status == CfsAccountStatus.ACTIVE.value
assert invoice_reference.status_code == InvoiceReferenceStatus.COMPLETED.value
if test_name == 'consolidated_happy':
assert invoice_reference.status_code == InvoiceReferenceStatus.CANCELLED.value
assert original_invoice_reference.status_code == InvoiceReferenceStatus.COMPLETED.value
else:
assert invoice_reference.status_code == InvoiceReferenceStatus.COMPLETED.value
receipt = ReceiptModel.find_all_receipts_for_invoice(invoice.id)[0]
assert receipt
assert receipt.receipt_amount == credit_invoice_link.amount + credit_invoice_link2.amount
Expand Down Expand Up @@ -259,7 +296,7 @@ def test_reverse_electronic_funds_transfers(session):
session.commit()

with patch('pay_api.services.CFSService.reverse_rs_receipt_in_cfs') as mock_reverse:
with patch('pay_api.services.CFSService.adjust_invoice') as mock_invoice:
with patch('pay_api.services.CFSService.reverse_invoice') as mock_invoice:
EFTTask.reverse_electronic_funds_transfers_cfs()
mock_invoice.assert_called()
mock_reverse.assert_called()
Expand Down Expand Up @@ -328,7 +365,7 @@ def test_handle_unlinked_refund_requested_invoices(session):
invoice_ref_2 = factory_invoice_reference(invoice_id=invoice_2.id).save()
invoice_3 = factory_invoice(payment_account=payment_account, status_code=InvoiceStatus.REFUND_REQUESTED.value,
payment_method_code=PaymentMethod.EFT.value, total=10).save()
with patch('pay_api.services.CFSService.adjust_invoice') as mock_invoice:
with patch('pay_api.services.CFSService.reverse_invoice') as mock_invoice:
EFTTask.handle_unlinked_refund_requested_invoices()
mock_invoice.assert_called()
# Has CIL so it's excluded
Expand All @@ -340,3 +377,21 @@ def test_handle_unlinked_refund_requested_invoices(session):
assert invoice_ref_2.status_code == InvoiceReferenceStatus.CANCELLED.value
# Has no invoice reference, should still move to REFUNDED
assert invoice_3.invoice_status_code == InvoiceStatus.REFUNDED.value


def test_rollback_consolidated_invoice():
"""Ensure we can't rollback a consolidated invoice."""
payment_account = factory_create_eft_account(status=CfsAccountStatus.ACTIVE.value)
invoice_1 = factory_invoice(payment_account=payment_account).save()
invoice_reference = factory_invoice_reference(invoice_id=invoice_1.id,
status_code=InvoiceReferenceStatus.COMPLETED.value,
is_consolidated=True).save()
with pytest.raises(Exception) as excinfo:
EFTTask._rollback_receipt_and_invoice(None, # pylint: disable=protected-access
invoice_1,
None,
cil_status_code=EFTCreditInvoiceStatus.PENDING_REFUND.value)
assert 'Cannot reverse a consolidated invoice' in excinfo.value.args
with pytest.raises(Exception) as excinfo:
EFTTask._handle_invoice_refund(None, invoice_reference) # pylint: disable=protected-access
assert 'Cannot reverse a consolidated invoice' in excinfo.value.args
35 changes: 35 additions & 0 deletions pay-api/migrations/versions/2024_08_29_2097573390f1_.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
"""Add is_consolidated and created_on columns to invoice_references

Revision ID: 2097573390f1
Revises: fc32e7db4493
Create Date: 2024-08-29 12:01:51.061253

"""
from alembic import op
import sqlalchemy as sa


# revision identifiers, used by Alembic.
# Note you may see foreign keys with distribution_codes_history
# For disbursement_distribution_code_id, service_fee_distribution_code_id
# Please ignore those lines and don't include in migration.

revision = '2097573390f1'
down_revision = 'fc32e7db4493'
branch_labels = None
depends_on = None


def upgrade():
with op.batch_alter_table('invoice_references', schema=None) as batch_op:
batch_op.add_column(sa.Column('created_on', sa.DateTime(), nullable=True))
batch_op.add_column(sa.Column('is_consolidated', sa.Boolean(), nullable=False))
batch_op.create_index(batch_op.f('ix_invoice_references_is_consolidated'), ['is_consolidated'], unique=False)
op.execute("update invoice_references set is_consolidated = 't' where invoice_number like '%-C'")


def downgrade():
with op.batch_alter_table('invoice_references', schema=None) as batch_op:
batch_op.drop_index(batch_op.f('ix_invoice_references_is_consolidated'))
batch_op.drop_column('is_consolidated')
batch_op.drop_column('created_on')
7 changes: 4 additions & 3 deletions pay-api/src/pay_api/models/invoice.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,11 @@ def find_by_business_identifier(cls, business_identifier: str):
return cls.query.filter_by(business_identifier=business_identifier).all()

@classmethod
def find_invoices_by_status_for_account(cls, pay_account_id: int, invoice_statuses: List[str]):
def find_invoices_by_status_for_account(cls, pay_account_id: int, invoice_statuses: List[str]) -> List[Invoice]:
"""Return invoices by status for an account."""
query = cls.query.filter_by(payment_account_id=pay_account_id). \
filter(Invoice.invoice_status_code.in_(invoice_statuses))
query = cls.query.filter_by(payment_account_id=pay_account_id) \
.filter(Invoice.invoice_status_code.in_(invoice_statuses)) \
.order_by(Invoice.id)

return query.all()

Expand Down
Loading
Loading