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

Small fixes and tweaks for partner reversals #1775

Merged
merged 22 commits into from
Oct 11, 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
8 changes: 4 additions & 4 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/bcgov/sbc-pay.git", branch = "main", subdirectory = "pay-api"}
pay-api = {git = "https://github.com/seeker25/sbc-pay.git", branch = "partner_disbursement_fixes", subdirectory = "pay-api"}
flask = "^3.0.2"
flask-sqlalchemy = "^3.1.1"
sqlalchemy = "^2.0.28"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ def process_overpayment_notification(cls, date_override=None):
@classmethod
def _send_notifications(cls):
"""Send over payment notification."""
if cls.short_names == {}:
if not cls.short_names:
return

qualified_receiver_recipients = get_emails_with_keycloak_role(Role.EFT_REFUND.value)
Expand Down
2 changes: 1 addition & 1 deletion jobs/payment-jobs/tasks/ejv_partner_distribution_task.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ def get_disbursement_by_distribution_for_partner(partner):
DistributionCodeModel,
DistributionCodeModel.distribution_code_id == PaymentLineItemModel.fee_distribution_id,
)
.filter(PartnerDisbursementsModel.status_code == DisbursementStatus.WAITING_FOR_RECEIPT.value)
.filter(PartnerDisbursementsModel.status_code == DisbursementStatus.WAITING_FOR_JOB.value)
.filter(PartnerDisbursementsModel.partner_code == partner.code)
.filter(DistributionCodeModel.stop_ejv.is_(False) | DistributionCodeModel.stop_ejv.is_(None))
.filter(~InvoiceModel.receipts.any(cast(ReceiptModel.receipt_date, Date) >= disbursement_date.date()))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ def test_disbursement_for_partners(session, monkeypatch, client_code, batch_type
amount=10,
is_reversal=False,
partner_code=eft_invoice.corp_type_code,
status_code=DisbursementStatus.WAITING_FOR_RECEIPT.value,
status_code=DisbursementStatus.WAITING_FOR_JOB.value,
target_id=eft_invoice.id,
target_type=EJVLinkType.INVOICE.value,
).save()
Expand Down Expand Up @@ -162,7 +162,7 @@ def test_disbursement_for_partners(session, monkeypatch, client_code, batch_type
invoice.invoice_status_code = InvoiceStatus.REFUNDED.value
invoice.refund_date = datetime.now(tz=timezone.utc)
invoice.save()
partner_disbursement.status = DisbursementStatus.WAITING_FOR_RECEIPT.value
partner_disbursement.status = DisbursementStatus.WAITING_FOR_JOB.value
partner_disbursement.is_reversal = True
partner_disbursement.save()

Expand Down
2 changes: 1 addition & 1 deletion pay-api/migrations/versions/2024_10_01_56c4542db0d7_.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ def upgrade():
op.execute(
f"""insert into partner_disbursements (amount, created_on, partner_code, is_reversal, status_code, target_id, target_type)
select (i.total - i.service_fees) as amount, now() as created_on, i.corp_type_code as partner_code, 'f' as is_reversal,
'{DisbursementStatus.WAITING_FOR_RECEIPT.value}' as status_code, i.id as target_id, '{EJVLinkType.INVOICE.value}' as target_type from invoices i where invoice_status_code in ('APPROVED', 'PAID')
'{DisbursementStatus.WAITING_FOR_JOB.value}' as status_code, i.id as target_id, '{EJVLinkType.INVOICE.value}' as target_type from invoices i where invoice_status_code in ('APPROVED', 'PAID')
and corp_type_code in ('CSO','VS') and payment_method_code = 'EFT'
"""
)
Expand Down
32 changes: 32 additions & 0 deletions pay-api/migrations/versions/2024_10_10_ed487561aeeb_.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
"""Small fix for status code

Revision ID: ed487561aeeb
Revises: d1996caed682
Create Date: 2024-10-10 11:36:29.069307

"""
from alembic import op
import sqlalchemy as sa

from pay_api.utils.enums import DisbursementStatus


# 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 = 'ed487561aeeb'
down_revision = 'd1996caed682'
branch_labels = None
depends_on = None


def upgrade():
op.execute(
f"update partner_disbursements set status_code = '{DisbursementStatus.WAITING_FOR_JOB.value}' where status_code = 'WAITING_FOR_RECEIPT'"
seeker25 marked this conversation as resolved.
Show resolved Hide resolved
)


def downgrade():
pass
1 change: 1 addition & 0 deletions pay-api/pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,7 @@ extend-exclude = '''
migrations
| devops
| .history
| lib
)/
'''

Expand Down
12 changes: 12 additions & 0 deletions pay-api/src/pay_api/models/partner_disbursements.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

from datetime import datetime, timezone

from pay_api.utils.enums import DisbursementStatus

from .base_model import BaseModel
from .db import db

Expand Down Expand Up @@ -76,3 +78,13 @@ class PartnerDisbursements(BaseModel): # pylint: disable=too-many-instance-attr
def find_by_target(cls, target_id: int, target_type: str):
"""Find the Partner Disbursement by target."""
return cls.query.filter_by(target_id=target_id, target_type=target_type).first()

@classmethod
def find_by_target_latest_exclude_cancelled(cls, target_id: int, target_type: str):
"""Find the latest Partner Disbursement by target."""
return (
cls.query.filter_by(target_id=target_id, target_type=target_type)
.filter(PartnerDisbursements.status_code != DisbursementStatus.CANCELLED.value)
.order_by(PartnerDisbursements.id.desc())
.first()
)
1 change: 1 addition & 0 deletions pay-api/src/pay_api/services/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
from .internal_pay_service import InternalPayService
from .invoice import Invoice as InvoiceService
from .non_sufficient_funds import NonSufficientFundsService
from .partner_disbursements import PartnerDisbursements
from .payment import Payment
from .payment_service import PaymentService
from .payment_transaction import PaymentTransaction as TransactionService
Expand Down
15 changes: 0 additions & 15 deletions pay-api/src/pay_api/services/eft_refund.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,20 @@

from pay_api.dtos.eft_shortname import EFTShortNameRefundGetRequest, EFTShortNameRefundPatchRequest
from pay_api.exceptions import BusinessException, Error
from pay_api.models import CorpType as CorpTypeModel
from pay_api.models import EFTRefund as EFTRefundModel
from pay_api.models import EFTShortnames as EFTShortnamesModel
from pay_api.models import EFTShortnamesHistorical as EFTHistoryModel
from pay_api.models import Invoice as InvoiceModel
from pay_api.models import PartnerDisbursements as PartnerDisbursementsModel
from pay_api.models import PaymentAccount
from pay_api.models.eft_credit import EFTCredit as EFTCreditModel
from pay_api.models.eft_credit_invoice_link import EFTCreditInvoiceLink as EFTCreditInvoiceLinkModel
from pay_api.services.auth import get_emails_with_keycloak_role
from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService
from pay_api.services.email_service import ShortNameRefundEmailContent, send_email
from pay_api.utils.enums import (
DisbursementStatus,
EFTCreditInvoiceStatus,
EFTHistoricalTypes,
EFTShortnameRefundStatus,
EJVLinkType,
InvoiceStatus,
Role,
)
Expand Down Expand Up @@ -137,17 +133,6 @@ def handle_invoice_refund(
if reversal_total != invoice.total:
raise BusinessException(Error.EFT_PARTIAL_REFUND)

if corp_type := CorpTypeModel.find_by_code(invoice.corp_type_code):
if corp_type.has_partner_disbursements and invoice.total - invoice.service_fees > 0:
PartnerDisbursementsModel(
amount=invoice.total - invoice.service_fees,
is_reversal=True,
partner_code=invoice.corp_type_code,
status_code=DisbursementStatus.WAITING_FOR_RECEIPT.value,
target_id=invoice.id,
target_type=EJVLinkType.INVOICE.value,
).flush()

current_balance = EFTCreditModel.get_eft_credit_balance(latest_eft_credit.short_name_id)
if existing_balance != current_balance:
short_name_history = EFTHistoryModel.find_by_related_group_link_id(latest_link.link_group_id)
Expand Down
29 changes: 7 additions & 22 deletions pay-api/src/pay_api/services/eft_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
from pay_api.models import EFTShortnamesHistorical as EFTHistoryModel
from pay_api.models import Invoice as InvoiceModel
from pay_api.models import InvoiceReference as InvoiceReferenceModel
from pay_api.models import PartnerDisbursements as PartnerDisbursementsModel
from pay_api.models import Payment as PaymentModel
from pay_api.models import PaymentAccount as PaymentAccountModel
from pay_api.models import Receipt as ReceiptModel
Expand All @@ -39,9 +38,7 @@
from pay_api.models import db
from pay_api.utils.enums import (
CfsAccountStatus,
DisbursementStatus,
EFTCreditInvoiceStatus,
EJVLinkType,
InvoiceReferenceStatus,
InvoiceStatus,
PaymentMethod,
Expand All @@ -59,6 +56,7 @@
from .email_service import _render_payment_reversed_template, send_email
from .invoice import Invoice
from .invoice_reference import InvoiceReference
from .partner_disbursements import PartnerDisbursements
from .payment_account import PaymentAccount
from .payment_line_item import PaymentLineItem
from .statement import Statement as StatementService
Expand Down Expand Up @@ -99,16 +97,7 @@ def create_invoice(
) -> None:
"""Do nothing here, we create invoice references on the create CFS_INVOICES job."""
self.ensure_no_payment_blockers(payment_account)
if corp_type := CorpTypeModel.find_by_code(invoice.corp_type_code):
if corp_type.has_partner_disbursements and invoice.total - invoice.service_fees > 0:
PartnerDisbursementsModel(
amount=invoice.total - invoice.service_fees,
is_reversal=False,
partner_code=invoice.corp_type_code,
status_code=DisbursementStatus.WAITING_FOR_RECEIPT.value,
target_id=invoice.id,
target_type=EJVLinkType.INVOICE.value,
).flush()
PartnerDisbursements.handle_payment(invoice)

def complete_post_invoice(self, invoice: Invoice, invoice_reference: InvoiceReference) -> None:
"""Complete any post invoice activities if needed."""
Expand Down Expand Up @@ -147,6 +136,7 @@ def process_cfs_refund(
refund_partial: List[RefundPartialLine],
): # pylint:disable=unused-argument
"""Process refund in CFS."""
PartnerDisbursements.handle_reversal(invoice)
cils = EFTCreditInvoiceLinkModel.find_by_invoice_id(invoice.id)
# 1. Possible to have no CILs and no invoice_reference, nothing to reverse.
if (
Expand Down Expand Up @@ -290,16 +280,8 @@ def reverse_payment_action(short_name_id: int, statement_id: int):
for invoice, total_amount in invoice_disbursements.items():
if total_amount != invoice.total:
raise BusinessException(Error.EFT_PARTIAL_REFUND)
PartnerDisbursements.handle_reversal(invoice)

if total_amount - invoice.service_fees > 0:
PartnerDisbursementsModel(
amount=total_amount - invoice.service_fees,
is_reversal=True,
partner_code=invoice.corp_type_code,
status_code=DisbursementStatus.WAITING_FOR_RECEIPT.value,
target_id=invoice.id,
target_type=EJVLinkType.INVOICE.value,
).flush()
statement = StatementModel.find_by_id(statement_id)
EFTHistoryService.create_statement_reverse(
EFTHistory(
Expand Down Expand Up @@ -486,6 +468,9 @@ def apply_eft_credit(invoice_id: int, short_name_id: int, link_group_id: int, au
eft_credit.remaining_amount = 0
eft_credit.save_or_add(auto_save)

if invoice_balance == 0:
PartnerDisbursements.handle_payment(invoice)

@staticmethod
def process_owing_statements(short_name_id: int, auth_account_id: str, is_new_link: bool = False):
"""Process outstanding statement invoices for an EFT Short name."""
Expand Down
81 changes: 81 additions & 0 deletions pay-api/src/pay_api/services/partner_disbursements.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
"""Partner Disbursements service."""

from datetime import datetime, timezone

from flask import current_app

from pay_api.models.corp_type import CorpType as CorpTypeModel
from pay_api.models.invoice import Invoice as InvoiceModel
from pay_api.models.partner_disbursements import PartnerDisbursements as PartnerDisbursementsModel
from pay_api.utils.enums import DisbursementStatus, EJVLinkType


class PartnerDisbursements:
"""Partner Disbursements service."""

@staticmethod
def _skip_partner_disbursement(invoice: InvoiceModel) -> bool:
"""Determine if partner disbursement should be skipped."""
return (
invoice.total - invoice.service_fees <= 0
or bool(CorpTypeModel.find_by_code(invoice.corp_type_code).has_partner_disbursements) is False
)

@staticmethod
def handle_payment(invoice: InvoiceModel):
"""Insert a partner disbursement row if necessary with is_reversal as False."""
if PartnerDisbursements._skip_partner_disbursement(invoice):
return

latest_active_disbursement = PartnerDisbursementsModel.find_by_target_latest_exclude_cancelled(
invoice.id, EJVLinkType.INVOICE.value
)

if latest_active_disbursement is None or latest_active_disbursement.is_reversal:
PartnerDisbursementsModel(
amount=invoice.total - invoice.service_fees,
is_reversal=False,
partner_code=invoice.corp_type_code,
status_code=DisbursementStatus.WAITING_FOR_JOB.value,
target_id=invoice.id,
target_type=EJVLinkType.INVOICE.value,
).flush()
else:
# If this was already called at invoice creation, it might be called again when mapping credits.
# If we're mapping credits after invoice creation, we don't want to create a new row.
current_app.logger.info(f"Skipping Partner Disbursement Payment creation for {invoice.id} already exists.")

@staticmethod
def handle_reversal(invoice: InvoiceModel):
"""Cancel existing row or insert new row if non reversal is found."""
if PartnerDisbursements._skip_partner_disbursement(invoice):
return

if not (
latest_active_disbursement := PartnerDisbursementsModel.find_by_target_latest_exclude_cancelled(
invoice.id, EJVLinkType.INVOICE.value
)
):
current_app.logger.error(f"Existing Partner Disbursement not found for invoice {invoice.id}")
return

if latest_active_disbursement.is_reversal is True:
current_app.logger.error(f"Duplicate Existing Partner Disbursement Reversal for invoice {invoice.id}")
return

match latest_active_disbursement.status_code:
case DisbursementStatus.WAITING_FOR_JOB.value:
# Note we never CANCEL a reversal.
latest_active_disbursement.status_code = DisbursementStatus.CANCELLED.value
latest_active_disbursement.processed_on = datetime.now(tz=timezone.utc)
latest_active_disbursement.flush()
case _:
# We'll assume errored status should be fixed in the future to COMPLETED hopefully.
PartnerDisbursementsModel(
amount=invoice.total - invoice.service_fees,
is_reversal=True,
partner_code=invoice.corp_type_code,
status_code=DisbursementStatus.WAITING_FOR_JOB.value,
target_id=invoice.id,
target_type=EJVLinkType.INVOICE.value,
).flush()
2 changes: 1 addition & 1 deletion pay-api/src/pay_api/utils/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ class DisbursementStatus(Enum):
REVERSED = "REVERSED"
UPLOADED = "UPLOADED"
# Could be waiting for receipt in the job.
WAITING_FOR_RECEIPT = "WAITING_FOR_RECEIPT"
WAITING_FOR_JOB = "WAITING_FOR_JOB"


class DisbursementMethod(Enum):
Expand Down
5 changes: 4 additions & 1 deletion pay-api/tests/unit/api/test_eft_payment_actions.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
from pay_api.models import Statement as StatementModel
from pay_api.services import EftService
from pay_api.utils.enums import (
DisbursementStatus,
EFTCreditInvoiceStatus,
EFTPaymentActions,
InvoiceStatus,
Expand All @@ -46,6 +47,7 @@
factory_eft_shortname,
factory_eft_shortname_link,
factory_invoice,
factory_partner_disbursement,
factory_payment_account,
factory_statement,
factory_statement_invoices,
Expand Down Expand Up @@ -394,6 +396,7 @@ def test_eft_reverse_payment_action(db, session, client, jwt, app, admin_users_m
)
credit_invoice_links[0].receipt_number = "ABC123"
credit_invoice_links[0].save()
factory_partner_disbursement(invoices[0], status_code=DisbursementStatus.COMPLETED.value).save()
with patch("pay_api.services.eft_service.send_email") as mock_email:
rv = client.post(
f"/api/v1/eft-shortnames/{short_name.id}/payment",
Expand Down Expand Up @@ -424,6 +427,6 @@ def test_eft_reverse_payment_action(db, session, client, jwt, app, admin_users_m
assert credit_invoice_links[1].receipt_number == credit_invoice_links[0].receipt_number
assert EFTCreditModel.get_eft_credit_balance(short_name.id) == 100

partner_disbursement = PartnerDisbursements.query.first()
partner_disbursement = PartnerDisbursements.query.order_by(PartnerDisbursements.id.desc()).first()
assert partner_disbursement.is_reversal is True
assert partner_disbursement.amount == 100
Loading
Loading