From 218961beec6cc8246c17821ff78a14f7cb444911 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Fri, 20 Sep 2024 08:31:40 -0700 Subject: [PATCH 01/16] WIP --- pay-api/setup.cfg | 1 + pay-api/src/pay_api/dtos/__init__.py | 1 + pay-api/src/pay_api/dtos/eft_shortname.py | 87 ++++++++ pay-api/src/pay_api/models/__init__.py | 10 +- pay-api/src/pay_api/models/base_model.py | 4 + pay-api/src/pay_api/models/cfs_account.py | 2 +- pay-api/src/pay_api/models/credit.py | 2 +- .../src/pay_api/models/distribution_code.py | 2 +- pay-api/src/pay_api/models/eft_credit.py | 2 +- pay-api/src/pay_api/models/eft_refund.py | 11 +- .../models/eft_short_names_historical.py | 2 +- pay-api/src/pay_api/models/refund.py | 2 +- .../pay_api/resources/v1/eft_short_names.py | 124 ++++++----- .../schemas/schemas/refund_shortname.json | 10 + pay-api/src/pay_api/services/__init__.py | 2 +- pay-api/src/pay_api/services/eft_refund.py | 198 ++++++++++++++++++ pay-api/src/pay_api/services/eft_service.py | 169 +-------------- .../services/eft_short_name_historical.py | 16 +- pay-api/src/pay_api/services/statement.py | 2 +- .../pay_api/services/statement_recipients.py | 2 +- .../pay_api/services/statement_settings.py | 2 +- pay-api/src/pay_api/utils/enums.py | 1 + .../unit/api/test_eft_short_name_history.py | 2 +- .../tests/unit/api/test_eft_short_names.py | 27 ++- .../models/test_eft_short_names_historical.py | 10 +- .../tests/unit/services/test_eft_service.py | 11 +- .../test_eft_short_name_historical.py | 4 +- 27 files changed, 446 insertions(+), 260 deletions(-) create mode 100644 pay-api/src/pay_api/dtos/__init__.py create mode 100644 pay-api/src/pay_api/dtos/eft_shortname.py create mode 100644 pay-api/src/pay_api/services/eft_refund.py diff --git a/pay-api/setup.cfg b/pay-api/setup.cfg index ccbd8d7ce..7113f1574 100755 --- a/pay-api/setup.cfg +++ b/pay-api/setup.cfg @@ -71,6 +71,7 @@ notes=FIXME,XXX,TODO ignored-modules=flask_sqlalchemy,sqlalchemy,SQLAlchemy,alembic,scoped_session ignored-classes=scoped_session min-similarity-lines=15 +max-attributes=15 disable=C0301,W0511,R0903 good-names= b, diff --git a/pay-api/src/pay_api/dtos/__init__.py b/pay-api/src/pay_api/dtos/__init__.py new file mode 100644 index 000000000..14e8999c9 --- /dev/null +++ b/pay-api/src/pay_api/dtos/__init__.py @@ -0,0 +1 @@ +"""Init.""" diff --git a/pay-api/src/pay_api/dtos/eft_shortname.py b/pay-api/src/pay_api/dtos/eft_shortname.py new file mode 100644 index 000000000..381334a77 --- /dev/null +++ b/pay-api/src/pay_api/dtos/eft_shortname.py @@ -0,0 +1,87 @@ +"""Rationale behind creating these DTOS below. + +1. To ensure that the request and response payloads are validated before they are passed to the service layer. +2. To ensure that the request and response payloads are consistent across the application. +3. To ensure that the request and response payloads are consistent with the API documentation. + +In the near future, will find a library that generates our API spec based off of these DTOs. +""" +from decimal import Decimal +from attrs import define +from typing import List + +from pay_api.utils.converter import Converter + + +@define +class EFTShortNameGetRequest: + """EFT Short name search.""" + + short_name: str = None + short_name_id: int = None + short_name_type: str = None + amount_owing: Decimal = None + statement_id: int = None + state: str = None + page: int = 1 + limit: int = 10 + account_id: int = None + account_name: str = None + account_branch: str = None + account_id_list: str = None + + @classmethod + def from_dict(cls, data: dict): + """Convert from request args to EFTShortNameSearchDTO.""" + dto = Converter(camel_to_snake_case=True).structure(data, EFTShortNameGetRequest) + # In the future, we'll need a cleaner way to handle this. + dto.state = dto.state.split(',') if dto.state else None + dto.account_id_list = dto.account_id_list.split(',') if dto.account_id_list else None + return dto + + +@define +class EFTShortNameSummaryGetRequest: + """EFT Short name summary search.""" + + short_name: str = None + short_name_id: int = None + short_name_type: str = None + credits_remaining: Decimal = None + linked_accounts_count: int = None + payment_received_start_date: str = None + payment_received_end_date: str = None + page: int = 1 + limit: int = 10 + + @classmethod + def from_dict(cls, data: dict): + """Convert from request args to EFTShortNameSummarySearchDTO.""" + dto = Converter(camel_to_snake_case=True).structure(data, EFTShortNameSummaryGetRequest) + return dto + + +@define +class EFTShortNameRefundPatchRequest: + """EFT Short name refund DTO.""" + + comment: str + declined_reason: str + status: str + + @classmethod + def from_dict(cls, data: dict): + """Convert from request json to EFTShortNameRefundDTO.""" + return Converter(camel_to_snake_case=True).structure(data, EFTShortNameRefundPatchRequest) + + +@define +class EFTShortNameRefundGetRequest: + """EFT Short name refund DTO.""" + + statuses: List[str] + + @classmethod + def from_dict(cls, data: dict): + """Convert from request json to EFTShortNameRefundDTO.""" + EFTShortNameRefundGetRequest(statuses=data.get('status', [])) diff --git a/pay-api/src/pay_api/models/__init__.py b/pay-api/src/pay_api/models/__init__.py index 9a886309a..49dd644b0 100755 --- a/pay-api/src/pay_api/models/__init__.py +++ b/pay-api/src/pay_api/models/__init__.py @@ -31,10 +31,11 @@ from .eft_credit_invoice_link import EFTCreditInvoiceLink from .eft_file import EFTFile from .eft_process_status_code import EFTProcessStatusCode -from .eft_short_names import EFTShortnames, EFTShortnameSchema, EFTShortnameSummarySchema -from .eft_short_names_historical import EFTShortnamesHistorical from .eft_refund import EFTRefund +from .eft_refund_email_list import EFTRefundEmailList from .eft_short_name_links import EFTShortnameLinks, EFTShortnameLinkSchema +from .eft_short_names import EFTShortnames, EFTShortnameSchema, EFTShortnameSummarySchema +from .eft_short_names_historical import EFTShortnameHistorySchema, EFTShortNamesHistorical from .eft_transaction import EFTTransaction, EFTTransactionSchema from .ejv_file import EjvFile from .ejv_header import EjvHeader @@ -61,13 +62,12 @@ from .refunds_partial import RefundPartialLine, RefundsPartial from .routing_slip import RoutingSlip, RoutingSlipSchema from .routing_slip_status_code import RoutingSlipStatusCode, RoutingSlipStatusCodeSchema -from .statement import StatementDTO, Statement, StatementSchema +from .statement import Statement, StatementDTO, StatementSchema from .statement_invoices import StatementInvoices, StatementInvoicesSchema # noqa: I005 from .statement_recipients import StatementRecipients, StatementRecipientsSchema from .statement_settings import StatementSettings, StatementSettingsSchema from .transaction_status_code import TransactionStatusCode, TransactionStatusCodeSchema -from .comment import Comment, CommentSchema -from .eft_refund_email_list import EFTRefundEmailList +from .comment import Comment, CommentSchema # This has to be at the bottom otherwise FeeSchedule errors event.listen(Engine, 'before_cursor_execute', DBTracing.query_tracing) diff --git a/pay-api/src/pay_api/models/base_model.py b/pay-api/src/pay_api/models/base_model.py index 1cedfeb7c..1a3135960 100644 --- a/pay-api/src/pay_api/models/base_model.py +++ b/pay-api/src/pay_api/models/base_model.py @@ -52,6 +52,10 @@ def delete(self): db.session.delete(self) db.session.commit() + def to_dict(self): + """Quick and easy way to convert to a dict.""" + return {c.name: getattr(self, c.name) for c in self.__table__.columns} + @staticmethod def rollback(): """RollBack.""" diff --git a/pay-api/src/pay_api/models/cfs_account.py b/pay-api/src/pay_api/models/cfs_account.py index 745ff21cf..244c2a7a4 100644 --- a/pay-api/src/pay_api/models/cfs_account.py +++ b/pay-api/src/pay_api/models/cfs_account.py @@ -29,7 +29,7 @@ from .db import db -class CfsAccount(Versioned, BaseModel): # pylint:disable=too-many-instance-attributes +class CfsAccount(Versioned, BaseModel): """This class manages all of the base data about PayBC Account.""" __tablename__ = 'cfs_accounts' diff --git a/pay-api/src/pay_api/models/credit.py b/pay-api/src/pay_api/models/credit.py index 6018d878a..6cc0f7cf6 100644 --- a/pay-api/src/pay_api/models/credit.py +++ b/pay-api/src/pay_api/models/credit.py @@ -19,7 +19,7 @@ from .db import db, ma -class Credit(BaseModel): # pylint:disable=too-many-instance-attributes +class Credit(BaseModel): """This class manages all of the base data about Credit.""" __tablename__ = 'credits' diff --git a/pay-api/src/pay_api/models/distribution_code.py b/pay-api/src/pay_api/models/distribution_code.py index 02528ea1f..96b1afa50 100644 --- a/pay-api/src/pay_api/models/distribution_code.py +++ b/pay-api/src/pay_api/models/distribution_code.py @@ -72,7 +72,7 @@ def bulk_save_links(cls, links: list): BaseModel.commit() -class DistributionCode(Audit, Versioned, BaseModel): # pylint:disable=too-many-instance-attributes +class DistributionCode(Audit, Versioned, BaseModel): """This class manages all of the base data about distribution code. Distribution code holds details on the codes for how the collected payment is going to be distributed. diff --git a/pay-api/src/pay_api/models/eft_credit.py b/pay-api/src/pay_api/models/eft_credit.py index 11214445e..186d29b79 100644 --- a/pay-api/src/pay_api/models/eft_credit.py +++ b/pay-api/src/pay_api/models/eft_credit.py @@ -22,7 +22,7 @@ from .db import db -class EFTCredit(BaseModel): # pylint:disable=too-many-instance-attributes +class EFTCredit(BaseModel): """This class manages all of the base data for EFT credits.""" __tablename__ = 'eft_credits' diff --git a/pay-api/src/pay_api/models/eft_refund.py b/pay-api/src/pay_api/models/eft_refund.py index 880c14460..27b92a1fe 100644 --- a/pay-api/src/pay_api/models/eft_refund.py +++ b/pay-api/src/pay_api/models/eft_refund.py @@ -20,7 +20,7 @@ from .db import db -class EFTRefund(BaseModel): # pylint: disable=too-many-instance-attributes +class EFTRefund(BaseModel): """This class manages the file data for EFT Refunds.""" __tablename__ = 'eft_refunds' @@ -66,3 +66,12 @@ class EFTRefund(BaseModel): # pylint: disable=too-many-instance-attributes updated_by = db.Column('updated_by', db.String(100), nullable=True) updated_by_name = db.Column('updated_by_name', db.String(100), nullable=True) updated_on = db.Column('updated_on', db.DateTime, nullable=True) + + + @classmethod + def find_refunds(cls, statuses: List[str]): + query = EFTRefundModel.query + if data.statuses: + query = query.filter(EFTRefundModel.status.in_(data.statuses)) + refunds = query.all() + return [refund.to_dict() for refund in refunds] diff --git a/pay-api/src/pay_api/models/eft_short_names_historical.py b/pay-api/src/pay_api/models/eft_short_names_historical.py index e87dfd624..d5b508be2 100644 --- a/pay-api/src/pay_api/models/eft_short_names_historical.py +++ b/pay-api/src/pay_api/models/eft_short_names_historical.py @@ -23,7 +23,7 @@ from .db import db -class EFTShortnamesHistorical(BaseModel): # pylint:disable=too-many-instance-attributes +class EFTShortNamesHistorical(BaseModel): """This class manages all EFT Short name historical data.""" __tablename__ = 'eft_short_names_historical' diff --git a/pay-api/src/pay_api/models/refund.py b/pay-api/src/pay_api/models/refund.py index bba73faed..78010f926 100644 --- a/pay-api/src/pay_api/models/refund.py +++ b/pay-api/src/pay_api/models/refund.py @@ -21,7 +21,7 @@ from .db import db, ma -class Refund(BaseModel): # pylint:disable=too-many-instance-attributes +class Refund(BaseModel): """This class manages all of the base data about Refunds.""" __tablename__ = 'refunds' diff --git a/pay-api/src/pay_api/resources/v1/eft_short_names.py b/pay-api/src/pay_api/resources/v1/eft_short_names.py index 60330542c..30fa1b7d3 100644 --- a/pay-api/src/pay_api/resources/v1/eft_short_names.py +++ b/pay-api/src/pay_api/resources/v1/eft_short_names.py @@ -17,13 +17,14 @@ from flask import Blueprint, current_app, jsonify, request from flask_cors import cross_origin +from pay_api.dtos.eft_shortname import EFTShortNameGetRequest, EFTShortNameRefundGetRequest, EFTShortNameRefundPatchRequest, EFTShortNameSummaryGetRequest from pay_api.exceptions import BusinessException, error_to_response from pay_api.schemas import utils as schema_utils -from pay_api.services.eft_service import EftService -from pay_api.services.eft_short_names import EFTShortnames as EFTShortnameService +from pay_api.services.eft_refund import EFTRefund as EFTRefundService from pay_api.services.eft_short_name_summaries import EFTShortnameSummaries as EFTShortnameSummariesService +from pay_api.services.eft_short_names import EFTShortnames as EFTShortnameService from pay_api.services.eft_short_names import EFTShortnamesSearch -from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTShortnameHistoryService +from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTShortnameHistoryService from pay_api.services.eft_short_name_historical import EFTShortnameHistorySearch from pay_api.utils.auth import jwt as _jwt from pay_api.utils.endpoints_enums import EndpointEnum @@ -31,6 +32,7 @@ from pay_api.utils.errors import Error from pay_api.utils.util import string_to_date, string_to_decimal, string_to_int + bp = Blueprint('EFT_SHORT_NAMES', __name__, url_prefix=f'{EndpointEnum.API_V1.value}/eft-shortnames') @@ -41,36 +43,21 @@ def get_eft_shortnames(): """Get all eft short name records.""" current_app.logger.info('get_eft_shortnames') - return jsonify(response), status @@ -81,26 +68,17 @@ def get_eft_shortnames(): def get_eft_shortname_summaries(): """Get all eft short name summaries.""" current_app.logger.info('get_eft_shortname_summaries') return jsonify(response), status @@ -245,10 +223,22 @@ def post_eft_statement_payment(short_name_id: int): return jsonify(response), status +@bp.route('/shortname-refund', methods=['GET', 'OPTIONS']) +@cross_origin(origins='*', methods=['GET', 'POST', 'PATCH']) +@_jwt.has_one_of_roles([Role.EFT_REFUND.value, Role.EFT_REFUND_APPROVER.value]) +def get_shortname_refunds(): + """Get all short name refunds.""" + # TODO unit test spec update + request_data = EFTShortNameRefundGetRequest.from_dict(request.args.to_dict()) + current_app.logger.info('post_fas_refund') + current_app.logger.debug('>post_shortname_refund') + return jsonify(response), status + + +@bp.route('/shortname-refund/', methods=['PATCH']) +@cross_origin(origins='*') +@_jwt.requires_auth +@_jwt.has_one_of_roles([Role.EFT_REFUND_APPROVER.value]) +def patch_shortname_refund(eft_refund_id: int): + """Patch EFT short name link.""" + current_app.logger.info('patch_eft_shortname_link') return jsonify(response), status diff --git a/pay-api/src/pay_api/schemas/schemas/refund_shortname.json b/pay-api/src/pay_api/schemas/schemas/refund_shortname.json index 05c2b06f4..b6835a378 100644 --- a/pay-api/src/pay_api/schemas/schemas/refund_shortname.json +++ b/pay-api/src/pay_api/schemas/schemas/refund_shortname.json @@ -57,6 +57,16 @@ "test@gmail.com" ] }, + "status": { + "$id":"#/properties/status", + "type":"string", + "title":"Status", + "description":"Status.", + "default":"", + "examples":[ + "ACTIVE" + ] + }, "comment":{ "$id":"#/properties/comment", "type":"string", diff --git a/pay-api/src/pay_api/services/__init__.py b/pay-api/src/pay_api/services/__init__.py index 96d9aec93..d2dc098e2 100755 --- a/pay-api/src/pay_api/services/__init__.py +++ b/pay-api/src/pay_api/services/__init__.py @@ -16,7 +16,7 @@ from .cfs_service import CFSService from .distribution_code import DistributionCode from .eft_service import EftService -from .eft_short_name_historical import EFTShortnameHistorical as EFTShortNameHistoricalService +from .eft_short_name_historical import EFTShortNameHistorical as EFTHistoryService from .eft_short_name_historical import EFTShortnameHistory, EFTShortnameHistorySearch from .eft_short_name_summaries import EFTShortnameSummaries as EFTShortNameSummaryService from .eft_short_names import EFTShortnames as EFTShortNamesService diff --git a/pay-api/src/pay_api/services/eft_refund.py b/pay-api/src/pay_api/services/eft_refund.py new file mode 100644 index 000000000..69c0323b8 --- /dev/null +++ b/pay-api/src/pay_api/services/eft_refund.py @@ -0,0 +1,198 @@ +"""Module for EFT refunds that go tghrough the AP module via EFT.""" +from decimal import Decimal +from typing import List +from flask import current_app +from pay_api.dtos.eft_shortname import EFTShortNameRefundGetRequest, EFTShortNameRefundPatchRequest +from pay_api.exceptions import BusinessException, Error +from pay_api.models import Invoice as InvoiceModel +from pay_api.models import EFTRefund as EFTRefundModel +from pay_api.models import PaymentAccount +from pay_api.models.eft_refund_email_list import EFTRefundEmailList +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.models import EFTShortNamesHistorical as EFTHistoryModel +from pay_api.services.email_service import _render_shortname_details_body, send_email +from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTHistoryService +from pay_api.utils.enums import EFTCreditInvoiceStatus, InvoiceStatus +from pay_api.utils.user_context import user_context +from pay_api.utils.util import get_str_by_path + + +class EFTRefund: + """Service to manage EFT Refunds.""" + + @staticmethod + @user_context + def create_shortname_refund(request: dict, **kwargs): + """Create refund.""" + # This method isn't for invoices, it's for shortname only. + shortname_id = get_str_by_path(request, 'shortNameId') + shortname = get_str_by_path(request, 'shortName') + amount = get_str_by_path(request, 'refundAmount') + comment = get_str_by_path(request, 'comment') + + current_app.logger.debug(f'Starting shortname refund : {shortname_id}') + + refund = EFTRefund._create_refund_model(request, shortname_id, amount, comment) + EFTRefund.refund_eft_credits(int(shortname_id), amount) + + history = EFTHistoryService.create_shortname_refund( + EFTHistoryModel(short_name_id=shortname_id, + amount=amount, + credit_balance=EFTCreditModel.get_eft_credit_balance(int(shortname_id)), + eft_refund_id=refund.id, + is_processing=False, + hidden=False)).flush() + + recipients = EFTRefundEmailList.find_all_emails() + subject = f'Pending Refund Request for Short Name {shortname}' + html_body = _render_shortname_details_body(shortname, amount, comment, shortname_id) + + send_email(recipients, subject, html_body, **kwargs) + history.save() + refund.save() + + return refund.to_dict() + + @staticmethod + def get_shortname_refunds(data: EFTShortNameRefundGetRequest): + """Get all refunds.""" + refunds = EFTRefundModel.find_refunds(data.statuses) + return [refund.to_dict() for refund in refunds] + + @staticmethod + def find_refund_by_id(refund_id: int) -> EFTRefundModel: + """Find refund by id.""" + return EFTRefundModel.find_by_id(refund_id) + + @staticmethod + def handle_invoice_refund(invoice: InvoiceModel, + payment_account: PaymentAccount, + cils: List[EFTCreditInvoiceLinkModel]) -> InvoiceStatus: + """Create EFT Short name funds received historical record.""" + # 2. No EFT Credit Link - Job needs to reverse invoice in CFS + # (Invoice needs to be reversed, receipt doesn't exist.) + if not cils: + return InvoiceStatus.REFUND_REQUESTED.value + + latest_link = cils[0] + sibling_cils = [cil for cil in cils if cil.link_group_id == latest_link.link_group_id] + latest_eft_credit = EFTCreditModel.find_by_id(latest_link.eft_credit_id) + link_group_id = EFTCreditInvoiceLinkModel.get_next_group_link_seq() + existing_balance = EFTCreditModel.get_eft_credit_balance(latest_eft_credit.short_name_id) + + match latest_link.status_code: + case EFTCreditInvoiceStatus.PENDING.value: + # 3. EFT Credit Link - PENDING, CANCEL that link - restore balance to EFT credit existing call + # (Invoice needs to be reversed, receipt doesn't exist.) + for cil in sibling_cils: + EFTRefund.return_eft_credit(cil, EFTCreditInvoiceStatus.CANCELLED.value) + cil.link_group_id = link_group_id + cil.flush() + case EFTCreditInvoiceStatus.COMPLETED.value: + # 4. EFT Credit Link - COMPLETED + # (Invoice needs to be reversed and receipt needs to be reversed.) + # reversal_total = Decimal('0') + for cil in sibling_cils: + EFTRefund.return_eft_credit(cil) + EFTCreditInvoiceLinkModel( + eft_credit_id=cil.eft_credit_id, + status_code=EFTCreditInvoiceStatus.PENDING_REFUND.value, + amount=cil.amount, + receipt_number=cil.receipt_number, + invoice_id=invoice.id, + link_group_id=link_group_id).flush() + # if corp_type := CorpTypeModel.find_by_code(invoice.corp_type_code): + # if corp_type.has_partner_disbursements: + # reversal_total += cil.amount + + # if reversal_total > 0: + # PartnerDisbursementsModel( + # amount=reversal_total, + # 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() + + 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) + EFTHistoryService.create_invoice_refund( + EFTHistoryModel(short_name_id=latest_eft_credit.short_name_id, + amount=invoice.total, + credit_balance=current_balance, + payment_account_id=payment_account.id, + related_group_link_id=link_group_id, + statement_number=short_name_history.statement_number if short_name_history else None, + invoice_id=invoice.id, + is_processing=True, + hidden=False)).flush() + + return InvoiceStatus.REFUND_REQUESTED.value + + @staticmethod + def refund_eft_credits(shortname_id: int, amount: str): + """Refund the amount to eft_credits table based on short_name_id.""" + refund_amount = Decimal(amount) + eft_credits = EFTCreditModel.get_eft_credits(shortname_id) + eft_credit_balance = EFTCreditModel.get_eft_credit_balance(shortname_id) + + if refund_amount > eft_credit_balance: + raise BusinessException(Error.INVALID_REFUND) + + for credit in eft_credits: + if refund_amount <= 0: + break + credit_amount = Decimal(credit.remaining_amount) + if credit_amount <= 0: + continue + + deduction = min(refund_amount, credit_amount) + credit.remaining_amount -= deduction + refund_amount -= deduction + + credit.save() + + @staticmethod + def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest) -> EFTRefundModel: + """Update the refund status.""" + refund = EFTRefundModel.find_pending_refund(refund_id) + refund.comment = data.comment + refund.decline_reason = data.declined_reason + refund.status = data.status + refund.save() + return refund + + @staticmethod + def _create_refund_model(request: dict, shortname_id: str, amount: str, comment: str) -> EFTRefundModel: + """Create and return the EFTRefundModel instance.""" + # AP refund job should pick up this row and send back the amount in the refund via cheque. + # For example if we had $500 on the EFT Shortname credits and we want to refund $300, + # then the AP refund job should send a cheque for $300 to the supplier while leaving $200 on the credits. + refund = EFTRefundModel( + short_name_id=shortname_id, + refund_amount=amount, + cas_supplier_number=get_str_by_path(request, 'casSupplierNum'), + refund_email=get_str_by_path(request, 'refundEmail'), + comment=comment + ) + refund.status = EFTCreditInvoiceStatus.PENDING_REFUND + refund.flush() + return refund + + @staticmethod + def return_eft_credit(eft_credit_link: EFTCreditInvoiceLinkModel, + update_status: str = None) -> EFTCreditModel: + """Return EFT Credit Invoice Link amount to EFT Credit.""" + eft_credit = EFTCreditModel.find_by_id(eft_credit_link.eft_credit_id) + eft_credit.remaining_amount += eft_credit_link.amount + + if eft_credit.remaining_amount > eft_credit.amount: + raise BusinessException(Error.EFT_CREDIT_AMOUNT_UNEXPECTED) + + if update_status: + eft_credit_link.status_code = update_status + + return eft_credit diff --git a/pay-api/src/pay_api/services/eft_service.py b/pay-api/src/pay_api/services/eft_service.py index a4835caf8..263b1eb77 100644 --- a/pay-api/src/pay_api/services/eft_service.py +++ b/pay-api/src/pay_api/services/eft_service.py @@ -15,7 +15,6 @@ from __future__ import annotations from datetime import datetime, timezone -from decimal import Decimal from typing import Any, Dict, List from flask import current_app @@ -25,10 +24,8 @@ from pay_api.models import CfsAccount as CfsAccountModel from pay_api.models import EFTCredit as EFTCreditModel from pay_api.models import EFTCreditInvoiceLink as EFTCreditInvoiceLinkModel -from pay_api.models import EFTRefund as EFTRefundModel -from pay_api.models import EFTRefundEmailList from pay_api.models import EFTShortnameLinks as EFTShortnameLinksModel -from pay_api.models import EFTShortnamesHistorical as EFTHistoryModel +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 Payment as PaymentModel @@ -44,13 +41,13 @@ PaymentSystem) from pay_api.utils.errors import Error from pay_api.utils.user_context import user_context -from pay_api.utils.util import get_str_by_path from .auth import get_account_admin_users from .deposit_service import DepositService -from .eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService +from .eft_short_name_historical import EFTShortNameHistorical as EFTHistoryService from .eft_short_name_historical import EFTShortnameHistory as EFTHistory -from .email_service import _render_payment_reversed_template, _render_shortname_details_body, send_email +from .eft_refund import EFTRefund as EFTRefundService +from .email_service import _render_payment_reversed_template, send_email from .invoice import Invoice from .invoice_reference import InvoiceReference from .payment_account import PaymentAccount @@ -124,7 +121,7 @@ def process_cfs_refund(self, invoice: InvoiceModel, # Also untested with EFT. We want to be able to refund back to the original payment method. raise BusinessException(Error.INVALID_CONSOLIDATED_REFUND) - return EftService._handle_invoice_refund(invoice, payment_account, cils) + return EFTRefundService.handle_invoice_refund(invoice, payment_account, cils) @staticmethod def create_invoice_reference(invoice: InvoiceModel, invoice_number: str, @@ -150,36 +147,6 @@ def create_receipt(invoice: InvoiceModel, payment: PaymentModel) -> ReceiptModel receipt_number=payment.receipt_number) return receipt - @staticmethod - @user_context - def create_shortname_refund(request: Dict[str, str], **kwargs) -> Dict[str, str]: - """Create refund.""" - # This method isn't for invoices, it's for shortname only. - shortname_id = get_str_by_path(request, 'shortNameId') - shortname = get_str_by_path(request, 'shortName') - amount = get_str_by_path(request, 'refundAmount') - comment = get_str_by_path(request, 'comment') - - current_app.logger.debug(f'Starting shortname refund : {shortname_id}') - - refund = EftService._create_refund_model(request, shortname_id, amount, comment).flush() - EftService._refund_eft_credits(int(shortname_id), amount) - - EFTHistoryService.create_shortname_refund( - EFTHistory(short_name_id=shortname_id, - amount=amount, - credit_balance=EFTCreditModel.get_eft_credit_balance(int(shortname_id)), - eft_refund_id=refund.id, - is_processing=False, - hidden=False)).flush() - - recipients = EFTRefundEmailList.find_all_emails() - subject = f'Pending Refund Request for Short Name {shortname}' - html_body = _render_shortname_details_body(shortname, amount, comment, shortname_id) - - send_email(recipients, subject, html_body, **kwargs) - db.session.commit() - @staticmethod def apply_payment_action(short_name_id: int, auth_account_id: str): """Apply EFT payments to outstanding payments.""" @@ -204,7 +171,7 @@ def cancel_payment_action(short_name_id: int, auth_account_id: str, invoice_id: statuses=[EFTCreditInvoiceStatus.PENDING.value]) link_group_ids = set() for credit_link in credit_links: - eft_credit = EftService._return_eft_credit(credit_link, EFTCreditInvoiceStatus.CANCELLED.value) + eft_credit = EFTRefundService.return_eft_credit(credit_link, EFTCreditInvoiceStatus.CANCELLED.value) if credit_link.link_group_id: link_group_ids.add(credit_link.link_group_id) db.session.add(eft_credit) @@ -247,7 +214,7 @@ def reverse_payment_action(short_name_id: int, statement_id: int): f'- {invoice.id} in status {invoice.invoice_status_code}.') raise BusinessException(Error.EFT_PAYMENT_INVOICE_REVERSE_UNEXPECTED_STATUS) - eft_credit = EftService._return_eft_credit(current_link) + eft_credit = EFTRefundService.return_eft_credit(current_link) eft_credit.flush() reversed_credits += current_link.amount @@ -431,88 +398,6 @@ def apply_eft_credit(invoice_id: int, eft_credit.remaining_amount = 0 eft_credit.save_or_add(auto_save) - @staticmethod - def _return_eft_credit(eft_credit_link: EFTCreditInvoiceLinkModel, - update_status: str = None) -> EFTCreditModel: - """Return EFT Credit Invoice Link amount to EFT Credit.""" - eft_credit = EFTCreditModel.find_by_id(eft_credit_link.eft_credit_id) - eft_credit.remaining_amount += eft_credit_link.amount - - if eft_credit.remaining_amount > eft_credit.amount: - raise BusinessException(Error.EFT_CREDIT_AMOUNT_UNEXPECTED) - - if update_status: - eft_credit_link.status_code = update_status - - return eft_credit - - @staticmethod - def _handle_invoice_refund(invoice: InvoiceModel, - payment_account: PaymentAccount, - cils: List[EFTCreditInvoiceLinkModel]) -> InvoiceStatus: - """Create EFT Short name funds received historical record.""" - # 2. No EFT Credit Link - Job needs to reverse invoice in CFS - # (Invoice needs to be reversed, receipt doesn't exist.) - if not cils: - return InvoiceStatus.REFUND_REQUESTED.value - - latest_link = cils[0] - sibling_cils = [cil for cil in cils if cil.link_group_id == latest_link.link_group_id] - latest_eft_credit = EFTCreditModel.find_by_id(latest_link.eft_credit_id) - link_group_id = EFTCreditInvoiceLinkModel.get_next_group_link_seq() - existing_balance = EFTCreditModel.get_eft_credit_balance(latest_eft_credit.short_name_id) - - match latest_link.status_code: - case EFTCreditInvoiceStatus.PENDING.value: - # 3. EFT Credit Link - PENDING, CANCEL that link - restore balance to EFT credit existing call - # (Invoice needs to be reversed, receipt doesn't exist.) - for cil in sibling_cils: - EftService._return_eft_credit(cil, EFTCreditInvoiceStatus.CANCELLED.value) - cil.link_group_id = link_group_id - cil.flush() - case EFTCreditInvoiceStatus.COMPLETED.value: - # 4. EFT Credit Link - COMPLETED - # (Invoice needs to be reversed and receipt needs to be reversed.) - # reversal_total = Decimal('0') - for cil in sibling_cils: - EftService._return_eft_credit(cil) - EFTCreditInvoiceLinkModel( - eft_credit_id=cil.eft_credit_id, - status_code=EFTCreditInvoiceStatus.PENDING_REFUND.value, - amount=cil.amount, - receipt_number=cil.receipt_number, - invoice_id=invoice.id, - link_group_id=link_group_id).flush() - # if corp_type := CorpTypeModel.find_by_code(invoice.corp_type_code): - # if corp_type.has_partner_disbursements: - # reversal_total += cil.amount - - # if reversal_total > 0: - # PartnerDisbursementsModel( - # amount=reversal_total, - # 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() - - 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) - EFTHistoryService.create_invoice_refund( - EFTHistory(short_name_id=latest_eft_credit.short_name_id, - amount=invoice.total, - credit_balance=current_balance, - payment_account_id=payment_account.id, - related_group_link_id=link_group_id, - statement_number=short_name_history.statement_number if short_name_history else None, - invoice_id=invoice.id, - is_processing=True, - hidden=False)).flush() - - return InvoiceStatus.REFUND_REQUESTED.value - @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.""" @@ -561,46 +446,6 @@ def process_owing_statements(short_name_id: int, auth_account_id: str, is_new_li current_app.logger.debug('>process_owing_statements') - @staticmethod - def _refund_eft_credits(shortname_id: int, amount: str): - """Refund the amount to eft_credits table based on short_name_id.""" - refund_amount = Decimal(amount) - eft_credits = EFTCreditModel.get_eft_credits(shortname_id) - eft_credit_balance = EFTCreditModel.get_eft_credit_balance(shortname_id) - - if refund_amount > eft_credit_balance: - raise BusinessException(Error.INVALID_REFUND) - - for credit in eft_credits: - if refund_amount <= 0: - break - credit_amount = Decimal(credit.remaining_amount) - if credit_amount <= 0: - continue - - deduction = min(refund_amount, credit_amount) - credit.remaining_amount -= deduction - refund_amount -= deduction - - credit.flush() - - @staticmethod - def _create_refund_model(request: Dict[str, str], - shortname_id: str, amount: str, comment: str) -> EFTRefundModel: - """Create and return the EFTRefundModel instance.""" - # AP refund job should pick up this row and send back the amount in the refund via cheque. - # For example if we had $500 on the EFT Shortname credits and we want to refund $300, - # then the AP refund job should send a cheque for $300 to the supplier while leaving $200 on the credits. - refund = EFTRefundModel( - short_name_id=shortname_id, - refund_amount=amount, - cas_supplier_number=get_str_by_path(request, 'casSupplierNum'), - refund_email=get_str_by_path(request, 'refundEmail'), - comment=comment - ) - refund.status = EFTCreditInvoiceStatus.PENDING_REFUND - return refund - @staticmethod def _get_statement_invoices_owing(auth_account_id: str, statement_id: int = None) -> List[InvoiceModel]: """Return statement invoices that have not been fully paid.""" diff --git a/pay-api/src/pay_api/services/eft_short_name_historical.py b/pay-api/src/pay_api/services/eft_short_name_historical.py index 6092fa5f5..db05c3bdc 100644 --- a/pay-api/src/pay_api/services/eft_short_name_historical.py +++ b/pay-api/src/pay_api/services/eft_short_name_historical.py @@ -20,10 +20,10 @@ from sqlalchemy import and_, case, exists, false, func, select from sqlalchemy.orm import aliased -from pay_api.models import EFTShortnamesHistorical as EFTShortnamesHistoricalModel +from pay_api.models import EFTShortNamesHistorical as EFTShortnamesHistoricalModel from pay_api.models import PaymentAccount as PaymentAccountModel from pay_api.models import db -from pay_api.models.eft_short_names_historical import EFTShortnameHistorySchema +from pay_api.models import EFTShortnameHistorySchema from pay_api.utils.enums import EFTHistoricalTypes from pay_api.utils.user_context import user_context from pay_api.utils.util import unstructure_schema_items @@ -53,7 +53,7 @@ class EFTShortnameHistorySearch: limit: Optional[int] = 10 -class EFTShortnameHistorical: +class EFTShortNameHistorical: """Service to manage EFT Short name historical data.""" @staticmethod @@ -66,7 +66,7 @@ def create_funds_received(history: EFTShortnameHistory) -> EFTShortnamesHistoric hidden=history.hidden, is_processing=history.is_processing, short_name_id=history.short_name_id, - transaction_date=EFTShortnameHistorical.transaction_date_now(), + transaction_date=EFTShortNameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.FUNDS_RECEIVED.value ) @@ -84,7 +84,7 @@ def create_statement_paid(history: EFTShortnameHistory, **kwargs) -> EFTShortnam related_group_link_id=history.related_group_link_id, short_name_id=history.short_name_id, statement_number=history.statement_number, - transaction_date=EFTShortnameHistorical.transaction_date_now(), + transaction_date=EFTShortNameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.STATEMENT_PAID.value ) @@ -102,7 +102,7 @@ def create_statement_reverse(history: EFTShortnameHistory, **kwargs) -> EFTShort related_group_link_id=history.related_group_link_id, short_name_id=history.short_name_id, statement_number=history.statement_number, - transaction_date=EFTShortnameHistorical.transaction_date_now(), + transaction_date=EFTShortNameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.STATEMENT_REVERSE.value ) @@ -121,7 +121,7 @@ def create_invoice_refund(history: EFTShortnameHistory, **kwargs) -> EFTShortnam short_name_id=history.short_name_id, statement_number=history.statement_number, invoice_id=history.invoice_id, - transaction_date=EFTShortnameHistorical.transaction_date_now(), + transaction_date=EFTShortNameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.INVOICE_REFUND.value ) @@ -137,7 +137,7 @@ def create_shortname_refund(history: EFTShortnameHistory, **kwargs) -> EFTShortn is_processing=history.is_processing, short_name_id=history.short_name_id, eft_refund_id=history.eft_refund_id, - transaction_date=EFTShortnameHistorical.transaction_date_now(), + transaction_date=EFTShortNameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.SN_REFUND_PENDING_APPROVAL.value ) diff --git a/pay-api/src/pay_api/services/statement.py b/pay-api/src/pay_api/services/statement.py index e22e87763..b88d9eef3 100644 --- a/pay-api/src/pay_api/services/statement.py +++ b/pay-api/src/pay_api/services/statement.py @@ -43,7 +43,7 @@ from .payment import PaymentReportInput -class Statement: # pylint:disable=too-many-instance-attributes,too-many-public-methods +class Statement: # pylint:disable=too-many-public-methods """Service to manage statement related operations.""" def __init__(self): diff --git a/pay-api/src/pay_api/services/statement_recipients.py b/pay-api/src/pay_api/services/statement_recipients.py index 6a05f6d63..3f62afecb 100644 --- a/pay-api/src/pay_api/services/statement_recipients.py +++ b/pay-api/src/pay_api/services/statement_recipients.py @@ -22,7 +22,7 @@ from pay_api.models.payment_account import PaymentAccount as PaymentAccountModel -class StatementRecipients: # pylint:disable=too-many-instance-attributes +class StatementRecipients: """Service to manage statement related operations.""" def __init__(self): diff --git a/pay-api/src/pay_api/services/statement_settings.py b/pay-api/src/pay_api/services/statement_settings.py index 43120d0bc..2c8efe7f3 100644 --- a/pay-api/src/pay_api/services/statement_settings.py +++ b/pay-api/src/pay_api/services/statement_settings.py @@ -26,7 +26,7 @@ from pay_api.utils.util import current_local_time, get_first_and_last_dates_of_month, get_week_start_and_end_date -class StatementSettings: # pylint:disable=too-many-instance-attributes +class StatementSettings: """Service to manage statement related operations.""" def __init__(self): diff --git a/pay-api/src/pay_api/utils/enums.py b/pay-api/src/pay_api/utils/enums.py index 191441394..922e538c8 100644 --- a/pay-api/src/pay_api/utils/enums.py +++ b/pay-api/src/pay_api/utils/enums.py @@ -139,6 +139,7 @@ class Role(Enum): VIEW_ALL_TRANSACTIONS = 'view_all_transactions' MANAGE_EFT = 'manage_eft' EFT_REFUND = 'eft_refund' + EFT_REFUND_APPROVER = 'eft_refund_approver' class Code(Enum): diff --git a/pay-api/tests/unit/api/test_eft_short_name_history.py b/pay-api/tests/unit/api/test_eft_short_name_history.py index db1198995..d2cd80cf7 100755 --- a/pay-api/tests/unit/api/test_eft_short_name_history.py +++ b/pay-api/tests/unit/api/test_eft_short_name_history.py @@ -21,7 +21,7 @@ import pytest from freezegun import freeze_time -from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService +from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTHistoryService from pay_api.services.eft_short_name_historical import EFTShortnameHistory as EFTHistory from pay_api.utils.enums import EFTHistoricalTypes, InvoiceStatus, PaymentMethod, Role from tests.utilities.base_test import ( diff --git a/pay-api/tests/unit/api/test_eft_short_names.py b/pay-api/tests/unit/api/test_eft_short_names.py index a39e099d0..693341c52 100755 --- a/pay-api/tests/unit/api/test_eft_short_names.py +++ b/pay-api/tests/unit/api/test_eft_short_names.py @@ -27,7 +27,7 @@ from pay_api.models import EFTFile as EFTFileModel from pay_api.models import EFTShortnameLinks as EFTShortnameLinksModel from pay_api.models import EFTShortnames as EFTShortnamesModel -from pay_api.models import EFTShortnamesHistorical as EFTShortnamesHistoryModel +from pay_api.models import EFTShortNamesHistorical as EFTShortnamesHistoryModel from pay_api.models import EFTTransaction as EFTTransactionModel from pay_api.models import PaymentAccount as PaymentAccountModel from pay_api.models.eft_refund import EFTRefund as EFTRefundModel @@ -899,7 +899,7 @@ def test_search_eft_short_names(session, client, jwt, app): data_dict['single-linked']['statement_summary'][0]) -def test_post_shortname_refund_success(db, session, client, jwt, app): +def test_post_shortname_refund_success(db, session, client, jwt): """Test successful creation of a shortname refund.""" token = jwt.create_jwt(get_claims(roles=[Role.EFT_REFUND.value]), token_header) headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} @@ -918,7 +918,7 @@ def test_post_shortname_refund_success(db, session, client, jwt, app): 'refundEmail': 'test@example.com', 'comment': 'Refund for overpayment' } - with patch('pay_api.services.eft_service.send_email') as mock_email: + with patch('pay_api.services.eft_refund.send_email') as mock_email: rv = client.post('/api/v1/eft-shortnames/shortname-refund', headers=headers, json=data) assert rv.status_code == 202 mock_email.assert_called_once() @@ -939,22 +939,35 @@ def test_post_shortname_refund_success(db, session, client, jwt, app): assert history_record.transaction_type == EFTHistoricalTypes.SN_REFUND_PENDING_APPROVAL.value -def test_post_shortname_refund_invalid_request(client, mocker, jwt, app): +def test_post_shortname_refund_invalid_request(client, mocker, jwt): """Test handling of invalid request format.""" data = { 'invalid_field': 'invalid_value' } - token = jwt.create_jwt(get_claims(roles=[Role.EFT_REFUND.value]), token_header) headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} - rv = client.post('/api/v1/eft-shortnames/shortname-refund', headers=headers, json=data) assert rv.status_code == 400 assert 'INVALID_REQUEST' in rv.json['type'] -def test_patch_shortname(session, client, jwt, app): +def test_get_shortname_refund(client, jwt): + token = jwt.create_jwt(get_claims(roles=[Role.EFT_REFUND.value]), token_header) + headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} + rv = client.get('/api/v1/eft-shortnames/shortname-refund', headers=headers) + assert rv.status_code == 200 + + +def test_patch_shortname_refund(client): + data = { + 'invalid_field': 'invalid_value' + } + token = jwt.create_jwt(get_claims(roles=[Role.EFT_REFUND.value]), token_header) + headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} + rv = client.post('/api/v1/eft-shortnames/shortname-refund', headers=headers, json=data) + +def test_patch_shortname(session, client, jwt): """Test patch EFT Short name.""" data = {'email': 'invalid_email', 'casSupplierNumber': '1234567ABC'} diff --git a/pay-api/tests/unit/models/test_eft_short_names_historical.py b/pay-api/tests/unit/models/test_eft_short_names_historical.py index d2447e431..b773b0df7 100644 --- a/pay-api/tests/unit/models/test_eft_short_names_historical.py +++ b/pay-api/tests/unit/models/test_eft_short_names_historical.py @@ -18,7 +18,7 @@ """ from datetime import datetime, timezone -from pay_api.models import EFTShortnamesHistorical +from pay_api.models import EFTShortNamesHistorical from pay_api.utils.enums import EFTHistoricalTypes from tests.utilities.base_test import factory_eft_shortname, factory_payment_account @@ -34,7 +34,7 @@ def test_eft_short_names_historical(session): eft_short_name.save() now_date = datetime.now(tz=timezone.utc).date() - default_historical = EFTShortnamesHistorical( + default_historical = EFTShortNamesHistorical( amount=151.50, created_by='USER1', credit_balance=1234.50, @@ -43,7 +43,7 @@ def test_eft_short_names_historical(session): transaction_type=EFTHistoricalTypes.FUNDS_RECEIVED.value ).save() - default_historical = EFTShortnamesHistorical.find_by_id(default_historical.id) + default_historical = EFTShortNamesHistorical.find_by_id(default_historical.id) assert default_historical.id is not None assert default_historical.amount == 151.50 assert default_historical.created_on.date() == now_date @@ -58,7 +58,7 @@ def test_eft_short_names_historical(session): assert default_historical.transaction_date.date() == now_date assert default_historical.transaction_type == EFTHistoricalTypes.FUNDS_RECEIVED.value - short_name_historical = EFTShortnamesHistorical( + short_name_historical = EFTShortNamesHistorical( amount=123.50, created_by='USER1', credit_balance=456.50, @@ -72,7 +72,7 @@ def test_eft_short_names_historical(session): transaction_type=EFTHistoricalTypes.STATEMENT_PAID.value ).save() - short_name_historical = EFTShortnamesHistorical.find_by_id(short_name_historical.id) + short_name_historical = EFTShortNamesHistorical.find_by_id(short_name_historical.id) assert short_name_historical.id is not None assert short_name_historical.amount == 123.50 diff --git a/pay-api/tests/unit/services/test_eft_service.py b/pay-api/tests/unit/services/test_eft_service.py index cf936de55..06d422e2a 100644 --- a/pay-api/tests/unit/services/test_eft_service.py +++ b/pay-api/tests/unit/services/test_eft_service.py @@ -24,8 +24,9 @@ from pay_api.exceptions import BusinessException from pay_api.models import EFTCreditInvoiceLink as EFTCreditInvoiceLinkModel -from pay_api.models import EFTShortnamesHistorical as EFTHistoryModel +from pay_api.models import EFTShortNamesHistorical as EFTHistoryModel from pay_api.services.eft_service import EftService +from pay_api.services.eft_refund import EFTRefund as EFTRefundService from pay_api.utils.enums import ( EFTCreditInvoiceStatus, EFTHistoricalTypes, InvoiceReferenceStatus, InvoiceStatus, PaymentMethod) from pay_api.utils.errors import Error @@ -77,7 +78,7 @@ def test_refund_eft_credits(session): with patch('pay_api.models.EFTCredit.get_eft_credits', return_value=[credit1, credit2, credit3]), \ patch('pay_api.models.EFTCredit.get_eft_credit_balance', return_value=9): - EftService._refund_eft_credits(1, '8') + EFTRefundService.refund_eft_credits(1, '8') assert credit1.remaining_amount == 0 assert credit2.remaining_amount == 0 assert credit3.remaining_amount == 1 @@ -86,7 +87,7 @@ def test_refund_eft_credits(session): credit2.remaining_amount = 5 with patch('pay_api.models.EFTCredit.get_eft_credit_balance', return_value=10): - EftService._refund_eft_credits(1, '7') + EFTRefundService.refund_eft_credits(1, '7') assert credit1.remaining_amount == 0 assert credit2.remaining_amount == 3 @@ -94,7 +95,7 @@ def test_refund_eft_credits(session): credit2.remaining_amount = 2 with patch('pay_api.models.EFTCredit.get_eft_credit_balance', return_value=7): - EftService._refund_eft_credits(1, '1') + EFTRefundService.refund_eft_credits(1, '1') assert credit1.remaining_amount == 4 assert credit2.remaining_amount == 2 @@ -110,7 +111,7 @@ def test_refund_eft_credits_exceed_balance(session): patch('pay_api.models.EFTCredit.get_eft_credit_balance', return_value=8): with pytest.raises(BusinessException) as excinfo: - EftService._refund_eft_credits(1, '20') + EFTRefundService.refund_eft_credits(1, '20') assert excinfo.value.code == Error.INVALID_REFUND.name diff --git a/pay-api/tests/unit/services/test_eft_short_name_historical.py b/pay-api/tests/unit/services/test_eft_short_name_historical.py index e37e48f0b..fc8fc7ab0 100644 --- a/pay-api/tests/unit/services/test_eft_short_name_historical.py +++ b/pay-api/tests/unit/services/test_eft_short_name_historical.py @@ -20,8 +20,8 @@ from freezegun import freeze_time -from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTShortnameHistoryService -from pay_api.services.eft_short_name_historical import EFTShortnameHistory +from pay_api.models.eft_short_names_historical import EFTShortNamesHistorical as EFTShortnameHistory +from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTShortnameHistoryService from pay_api.utils.enums import EFTHistoricalTypes, InvoiceStatus, PaymentMethod from tests.utilities.base_test import ( factory_eft_refund, factory_eft_shortname, factory_invoice, factory_payment_account) From 24d15ad3cba4b3fc02a0e58cd50dec3754c312ab Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Fri, 20 Sep 2024 14:32:04 -0700 Subject: [PATCH 02/16] implement patch, get routes for refunds --- .../versions/2024_09_20_75a39e02c746_.py | 45 +++++++++++ pay-api/src/pay_api/dtos/eft_shortname.py | 12 ++- pay-api/src/pay_api/models/__init__.py | 2 +- pay-api/src/pay_api/models/base_model.py | 3 +- pay-api/src/pay_api/models/eft_refund.py | 24 +++--- .../pay_api/resources/v1/eft_short_names.py | 15 ++-- .../schemas/schemas/refund_shortname.json | 10 --- pay-api/src/pay_api/services/eft_refund.py | 11 ++- .../services/eft_short_name_summaries.py | 2 +- pay-api/src/pay_api/utils/converter.py | 31 +++++++- pay-api/src/pay_api/utils/enums.py | 4 +- pay-api/src/pay_api/utils/errors.py | 1 + .../tests/unit/api/test_eft_short_names.py | 74 ++++++++++++++++--- pay-api/tests/unit/models/test_eft_refund.py | 8 +- pay-api/tests/utilities/base_test.py | 9 ++- 15 files changed, 185 insertions(+), 66 deletions(-) create mode 100644 pay-api/migrations/versions/2024_09_20_75a39e02c746_.py diff --git a/pay-api/migrations/versions/2024_09_20_75a39e02c746_.py b/pay-api/migrations/versions/2024_09_20_75a39e02c746_.py new file mode 100644 index 000000000..7af4fee6a --- /dev/null +++ b/pay-api/migrations/versions/2024_09_20_75a39e02c746_.py @@ -0,0 +1,45 @@ +"""Fix audit columns for eft_refunds. + +Revision ID: 75a39e02c746 +Revises: 29f59e6f147b +Create Date: 2024-09-20 13:45:35.132557 + +""" +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 = '75a39e02c746' +down_revision = '29f59e6f147b' +branch_labels = None +depends_on = None + + +def upgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('eft_refunds', schema=None) as batch_op: + batch_op.add_column(sa.Column('created_name', sa.String(length=100), nullable=True)) + batch_op.add_column(sa.Column('updated_name', sa.String(length=50), nullable=True)) + batch_op.alter_column('updated_by', + existing_type=sa.VARCHAR(length=100), + type_=sa.String(length=50), + existing_nullable=True) + batch_op.drop_column('updated_by_name') + # ### end Alembic commands ### + + +def downgrade(): + # ### commands auto generated by Alembic - please adjust! ### + with op.batch_alter_table('eft_refunds', schema=None) as batch_op: + batch_op.add_column(sa.Column('updated_by_name', sa.VARCHAR(length=100), autoincrement=False, nullable=True)) + batch_op.alter_column('updated_by', + existing_type=sa.String(length=50), + type_=sa.VARCHAR(length=100), + existing_nullable=True) + batch_op.drop_column('updated_name') + batch_op.drop_column('created_name') + # ### end Alembic commands ### diff --git a/pay-api/src/pay_api/dtos/eft_shortname.py b/pay-api/src/pay_api/dtos/eft_shortname.py index 381334a77..caef8c4ee 100644 --- a/pay-api/src/pay_api/dtos/eft_shortname.py +++ b/pay-api/src/pay_api/dtos/eft_shortname.py @@ -7,8 +7,8 @@ In the near future, will find a library that generates our API spec based off of these DTOs. """ from decimal import Decimal -from attrs import define from typing import List +from attrs import define from pay_api.utils.converter import Converter @@ -66,7 +66,7 @@ class EFTShortNameRefundPatchRequest: """EFT Short name refund DTO.""" comment: str - declined_reason: str + decline_reason: str status: str @classmethod @@ -74,6 +74,10 @@ def from_dict(cls, data: dict): """Convert from request json to EFTShortNameRefundDTO.""" return Converter(camel_to_snake_case=True).structure(data, EFTShortNameRefundPatchRequest) + def to_dict(self): + """Convert from EFTShortNameRefundDTO to request json.""" + return Converter(snake_case_to_camel=True).unstructure(self) + @define class EFTShortNameRefundGetRequest: @@ -84,4 +88,6 @@ class EFTShortNameRefundGetRequest: @classmethod def from_dict(cls, data: dict): """Convert from request json to EFTShortNameRefundDTO.""" - EFTShortNameRefundGetRequest(statuses=data.get('status', [])) + input_string = data.get('status', '') + statuses = input_string.split(',') if input_string else [] + return EFTShortNameRefundGetRequest(statuses=statuses) diff --git a/pay-api/src/pay_api/models/__init__.py b/pay-api/src/pay_api/models/__init__.py index 49dd644b0..3da025f5e 100755 --- a/pay-api/src/pay_api/models/__init__.py +++ b/pay-api/src/pay_api/models/__init__.py @@ -67,7 +67,7 @@ from .statement_recipients import StatementRecipients, StatementRecipientsSchema from .statement_settings import StatementSettings, StatementSettingsSchema from .transaction_status_code import TransactionStatusCode, TransactionStatusCodeSchema -from .comment import Comment, CommentSchema # This has to be at the bottom otherwise FeeSchedule errors +from .comment import Comment, CommentSchema # This has to be at the bottom otherwise FeeSchedule errors event.listen(Engine, 'before_cursor_execute', DBTracing.query_tracing) diff --git a/pay-api/src/pay_api/models/base_model.py b/pay-api/src/pay_api/models/base_model.py index 1a3135960..beb34054c 100644 --- a/pay-api/src/pay_api/models/base_model.py +++ b/pay-api/src/pay_api/models/base_model.py @@ -54,7 +54,8 @@ def delete(self): def to_dict(self): """Quick and easy way to convert to a dict.""" - return {c.name: getattr(self, c.name) for c in self.__table__.columns} + # We need a better way to do this in the future. + return {c.name: str(getattr(self, c.name)) for c in self.__table__.columns if getattr(self, c.name) is not None} @staticmethod def rollback(): diff --git a/pay-api/src/pay_api/models/eft_refund.py b/pay-api/src/pay_api/models/eft_refund.py index 27b92a1fe..f94ac1993 100644 --- a/pay-api/src/pay_api/models/eft_refund.py +++ b/pay-api/src/pay_api/models/eft_refund.py @@ -12,15 +12,15 @@ # See the License for the specific language governing permissions and # limitations under the License. """Model to handle EFT REFUNDS, this is picked up by the AP job to mail out.""" -from datetime import datetime, timezone +from typing import List from sqlalchemy import ForeignKey -from .base_model import BaseModel +from .audit import Audit from .db import db -class EFTRefund(BaseModel): +class EFTRefund(Audit): """This class manages the file data for EFT Refunds.""" __tablename__ = 'eft_refunds' @@ -40,6 +40,7 @@ class EFTRefund(BaseModel): 'cas_supplier_number', 'comment', 'created_by', + 'created_name' 'created_on', 'decline_reason', 'id', @@ -47,8 +48,8 @@ class EFTRefund(BaseModel): 'refund_email', 'short_name_id', 'status', - 'updated_by_name', 'updated_by', + 'updated_name', 'updated_on' ] } @@ -57,21 +58,16 @@ class EFTRefund(BaseModel): comment = db.Column(db.String(), nullable=False) decline_reason = db.Column(db.String(), nullable=True) created_by = db.Column('created_by', db.String(100), nullable=True) - created_on = db.Column('created_on', db.DateTime, nullable=False, default=lambda: datetime.now(tz=timezone.utc)) id = db.Column(db.Integer, primary_key=True, autoincrement=True) refund_amount = db.Column(db.Numeric(), nullable=False) refund_email = db.Column(db.String(100), nullable=False) short_name_id = db.Column(db.Integer, ForeignKey('eft_short_names.id'), nullable=False) status = db.Column(db.String(25), nullable=True) - updated_by = db.Column('updated_by', db.String(100), nullable=True) - updated_by_name = db.Column('updated_by_name', db.String(100), nullable=True) - updated_on = db.Column('updated_on', db.DateTime, nullable=True) - @classmethod def find_refunds(cls, statuses: List[str]): - query = EFTRefundModel.query - if data.statuses: - query = query.filter(EFTRefundModel.status.in_(data.statuses)) - refunds = query.all() - return [refund.to_dict() for refund in refunds] + """Return all refunds by status.""" + query = cls.query + if statuses: + query = cls.query.filter(EFTRefund.status.in_(statuses)) + return query.all() diff --git a/pay-api/src/pay_api/resources/v1/eft_short_names.py b/pay-api/src/pay_api/resources/v1/eft_short_names.py index 30fa1b7d3..bfc160ef3 100644 --- a/pay-api/src/pay_api/resources/v1/eft_short_names.py +++ b/pay-api/src/pay_api/resources/v1/eft_short_names.py @@ -17,15 +17,16 @@ from flask import Blueprint, current_app, jsonify, request from flask_cors import cross_origin -from pay_api.dtos.eft_shortname import EFTShortNameGetRequest, EFTShortNameRefundGetRequest, EFTShortNameRefundPatchRequest, EFTShortNameSummaryGetRequest +from pay_api.dtos.eft_shortname import ( + EFTShortNameGetRequest, EFTShortNameRefundGetRequest, EFTShortNameRefundPatchRequest, EFTShortNameSummaryGetRequest) from pay_api.exceptions import BusinessException, error_to_response from pay_api.schemas import utils as schema_utils from pay_api.services.eft_refund import EFTRefund as EFTRefundService +from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTShortnameHistoryService +from pay_api.services.eft_short_name_historical import EFTShortnameHistorySearch from pay_api.services.eft_short_name_summaries import EFTShortnameSummaries as EFTShortnameSummariesService from pay_api.services.eft_short_names import EFTShortnames as EFTShortnameService from pay_api.services.eft_short_names import EFTShortnamesSearch -from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTShortnameHistoryService -from pay_api.services.eft_short_name_historical import EFTShortnameHistorySearch from pay_api.utils.auth import jwt as _jwt from pay_api.utils.endpoints_enums import EndpointEnum from pay_api.utils.enums import Role @@ -228,7 +229,6 @@ def post_eft_statement_payment(short_name_id: int): @_jwt.has_one_of_roles([Role.EFT_REFUND.value, Role.EFT_REFUND_APPROVER.value]) def get_shortname_refunds(): """Get all short name refunds.""" - # TODO unit test spec update request_data = EFTShortNameRefundGetRequest.from_dict(request.args.to_dict()) current_app.logger.info(' EFTRefundModel: """Update the refund status.""" - refund = EFTRefundModel.find_pending_refund(refund_id) + refund = EFTRefundModel.find_by_id(refund_id) + if refund.status != EFTShortnameRefundStatus.PENDING_APPROVAL.value: + raise BusinessException(Error.REFUND_ALREADY_FINALIZED) refund.comment = data.comment - refund.decline_reason = data.declined_reason + if data.status == EFTShortnameRefundStatus.REJECTED.value: + refund.decline_reason = data.declined_reason refund.status = data.status refund.save() - return refund + return refund.to_dict() @staticmethod def _create_refund_model(request: dict, shortname_id: str, amount: str, comment: str) -> EFTRefundModel: diff --git a/pay-api/src/pay_api/services/eft_short_name_summaries.py b/pay-api/src/pay_api/services/eft_short_name_summaries.py index 705cbe3b0..ae44971ee 100644 --- a/pay-api/src/pay_api/services/eft_short_name_summaries.py +++ b/pay-api/src/pay_api/services/eft_short_name_summaries.py @@ -82,7 +82,7 @@ def get_linked_count_query(): def get_shortname_refund_query(): """Query for EFT shortname refund.""" return (db.session.query(EFTRefundModel.short_name_id, EFTRefundModel.status) - .filter(EFTRefundModel.status.in_([EFTShortnameRefundStatus.PENDING_REFUND.value])) + .filter(EFTRefundModel.status.in_([EFTShortnameRefundStatus.PENDING_APPROVAL.value])) .distinct(EFTRefundModel.short_name_id)) @staticmethod diff --git a/pay-api/src/pay_api/utils/converter.py b/pay-api/src/pay_api/utils/converter.py index 9697f13c3..57cbca56d 100644 --- a/pay-api/src/pay_api/utils/converter.py +++ b/pay-api/src/pay_api/utils/converter.py @@ -12,7 +12,7 @@ class Converter(cattrs.Converter): """Addon to cattr converter.""" - def __init__(self, camel_to_snake_case: bool = False, enum_to_value: bool = False): + def __init__(self, camel_to_snake_case: bool = False, snake_case_to_camel=False, enum_to_value: bool = False): """Initialize function, add in extra unstructure hooks.""" super().__init__() # More from cattrs-extras/blob/master/src/cattrs_extras/converter.py @@ -31,6 +31,15 @@ def __init__(self, camel_to_snake_case: bool = False, enum_to_value: bool = Fals has, self._to_snake_case_structure ) + if snake_case_to_camel: + self.register_unstructure_hook_factory( + has, self._to_camel_case_unstructure + ) + + self.register_structure_hook_factory( + has, self._to_camel_case_structure + ) + def _to_snake_case(self, camel_str: str) -> str: return re.sub(r'(? str: components = snake_str.split('_') return components[0] + ''.join(x.title() for x in components[1:]) + def _to_camel_case_unstructure(self, cls): + return make_dict_unstructure_fn( + cls, + self, + **{ + a.name: override(rename=self._to_camel_case(a.name)) + for a in fields(cls) + } + ) + + def _to_camel_case_structure(self, cls): + return make_dict_structure_fn( + cls, + self, + **{ + a.name: override(rename=self._to_camel_case(a.name)) + for a in fields(cls) + } + ) + def _to_snake_case_unstructure(self, cls): return make_dict_unstructure_fn( cls, diff --git a/pay-api/src/pay_api/utils/enums.py b/pay-api/src/pay_api/utils/enums.py index 922e538c8..149bc9ff5 100644 --- a/pay-api/src/pay_api/utils/enums.py +++ b/pay-api/src/pay_api/utils/enums.py @@ -369,7 +369,9 @@ class EFTShortnameStatus(Enum): class EFTShortnameRefundStatus(Enum): """EFT Short name refund statuses.""" - PENDING_REFUND = 'PENDING_REFUND' + APPROVED = 'APPROVED' + PENDING_APPROVAL = 'PENDING_APPROVAL' + REJECTED = 'REJECTED' class EFTPaymentActions(Enum): diff --git a/pay-api/src/pay_api/utils/errors.py b/pay-api/src/pay_api/utils/errors.py index 219eb08f4..a2a64db77 100644 --- a/pay-api/src/pay_api/utils/errors.py +++ b/pay-api/src/pay_api/utils/errors.py @@ -103,6 +103,7 @@ class Error(Enum): ROUTING_SLIP_REFUND = 'ROUTING_SLIP_REFUND', HTTPStatus.BAD_REQUEST NO_FEE_REFUND = 'NO_FEE_REFUND', HTTPStatus.BAD_REQUEST INVALID_CONSOLIDATED_REFUND = 'INVALID_CONSOLIDATED_REFUND', HTTPStatus.BAD_REQUEST + REFUND_ALREADY_FINALIZED = 'REFUND_ALREADY_FINALIZED', HTTPStatus.BAD_REQUEST RS_ALREADY_A_PARENT = 'RS_ALREADY_A_PARENT', HTTPStatus.BAD_REQUEST RS_ALREADY_LINKED = 'RS_ALREADY_LINKED', HTTPStatus.BAD_REQUEST diff --git a/pay-api/tests/unit/api/test_eft_short_names.py b/pay-api/tests/unit/api/test_eft_short_names.py index 693341c52..c5b4b0d0d 100755 --- a/pay-api/tests/unit/api/test_eft_short_names.py +++ b/pay-api/tests/unit/api/test_eft_short_names.py @@ -22,6 +22,9 @@ from decimal import Decimal from unittest.mock import patch +import pytest + +from pay_api.dtos.eft_shortname import EFTShortNameRefundPatchRequest from pay_api.models import EFTCredit as EFTCreditModel from pay_api.models import EFTCreditInvoiceLink as EFTCreditInvoiceModel from pay_api.models import EFTFile as EFTFileModel @@ -36,9 +39,9 @@ EFTShortnameStatus, EFTShortnameType, InvoiceStatus, PaymentMethod, Role, StatementFrequency) from pay_api.utils.errors import Error from tests.utilities.base_test import ( - factory_eft_credit, factory_eft_file, factory_eft_shortname, factory_eft_shortname_link, factory_invoice, - factory_payment_account, factory_statement, factory_statement_invoices, factory_statement_settings, get_claims, - token_header) + factory_eft_credit, factory_eft_file, factory_eft_refund, factory_eft_shortname, factory_eft_shortname_link, + factory_invoice, factory_payment_account, factory_statement, factory_statement_invoices, factory_statement_settings, + get_claims, token_header) def test_create_eft_short_name_link(session, client, jwt, app): @@ -552,7 +555,7 @@ def create_eft_summary_search_data(): cas_supplier_number='123', refund_email='test@example.com', comment='Test comment', - status=EFTShortnameRefundStatus.PENDING_REFUND.value + status=EFTShortnameRefundStatus.PENDING_APPROVAL.value ).save() return short_name_1, s1_transaction1, short_name_2, s2_transaction1, s1_refund @@ -952,20 +955,67 @@ def test_post_shortname_refund_invalid_request(client, mocker, jwt): assert 'INVALID_REQUEST' in rv.json['type'] -def test_get_shortname_refund(client, jwt): +@pytest.mark.parametrize('query_string, test_name, count', [ + ('', 'get_all', 3), + ('?status=APPROVED,PENDING_APPROVAL', 'status_filter_multiple', 2), + ('?status=REJECTED', 'status_filter_rejected', 1), +]) +def test_get_shortname_refund(session, client, jwt, query_string, test_name, count): + """Test get short name refund.""" + short_name = factory_eft_shortname('TEST_SHORTNAME').save() + factory_eft_refund(short_name_id=short_name.id, + status=EFTShortnameRefundStatus.APPROVED.value) + factory_eft_refund(short_name_id=short_name.id, + status=EFTShortnameRefundStatus.PENDING_APPROVAL.value) + factory_eft_refund(short_name_id=short_name.id, + status=EFTShortnameRefundStatus.REJECTED.value) token = jwt.create_jwt(get_claims(roles=[Role.EFT_REFUND.value]), token_header) headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} - rv = client.get('/api/v1/eft-shortnames/shortname-refund', headers=headers) + rv = client.get(f'/api/v1/eft-shortnames/shortname-refund{query_string}', headers=headers) assert rv.status_code == 200 + assert len(rv.json) == count -def test_patch_shortname_refund(client): - data = { - 'invalid_field': 'invalid_value' - } - token = jwt.create_jwt(get_claims(roles=[Role.EFT_REFUND.value]), token_header) +@pytest.mark.parametrize('payload, test_name, role', [ + (EFTShortNameRefundPatchRequest( + comment='Test comment', + decline_reason='Test reason', + status=EFTShortnameRefundStatus.APPROVED.value + ).to_dict(), 'valid_full_patch_refund', Role.EFT_REFUND_APPROVER.value), + ({}, 'unauthorized', Role.EFT_REFUND.value), + (EFTShortNameRefundPatchRequest( + comment='Test comment', + decline_reason='Test reason', + status=EFTShortnameRefundStatus.PENDING_APPROVAL.value + ).to_dict( + ), 'bad_transition', Role.EFT_REFUND_APPROVER.value), +]) +def test_patch_shortname_refund(session, client, jwt, payload, test_name, role): + """Test patch short name refund.""" + short_name = factory_eft_shortname('TEST_SHORTNAME').save() + refund = factory_eft_refund(short_name_id=short_name.id, + status=EFTShortnameRefundStatus.PENDING_APPROVAL.value) + if test_name == 'bad_transition': + refund.status = EFTShortnameRefundStatus.APPROVED.value + refund.save() + token = jwt.create_jwt(get_claims(roles=[role]), token_header) headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} - rv = client.post('/api/v1/eft-shortnames/shortname-refund', headers=headers, json=data) + rv = client.patch(f'/api/v1/eft-shortnames/shortname-refund/{refund.id}', headers=headers, json=payload) + match test_name: + case 'unauthorized': + assert rv.status_code == 401 + case 'bad_transition' | 'invalid_patch_refund': + assert rv.status_code == 400 + assert 'REFUND_ALREADY_FINALIZED' in rv.json['type'] + case 'valid_full_patch_refund': + assert rv.status_code == 200 + assert rv.json['status'] == EFTShortnameRefundStatus.APPROVED.value + assert rv.json['comment'] == 'Test comment' + # Can't set declined reason on an approved. + assert 'declineReason' not in rv.json + case _: + raise ValueError(f'Invalid test case {test_name}') + def test_patch_shortname(session, client, jwt): """Test patch EFT Short name.""" diff --git a/pay-api/tests/unit/models/test_eft_refund.py b/pay-api/tests/unit/models/test_eft_refund.py index 37b8bee59..15f96349c 100644 --- a/pay-api/tests/unit/models/test_eft_refund.py +++ b/pay-api/tests/unit/models/test_eft_refund.py @@ -52,7 +52,7 @@ def test_eft_refund_defaults(session): assert eft_refund.comment == 'Test comment' assert eft_refund.status is None assert eft_refund.updated_by is None - assert eft_refund.updated_by_name is None + assert eft_refund.updated_name is None def test_eft_refund_all_attributes(session): @@ -67,7 +67,7 @@ def test_eft_refund_all_attributes(session): created_by = 'user111' decline_reason = 'Decline reason comment' updated_by = 'user123' - updated_by_name = 'User Name' + updated_name = 'User Name' eft_refund = EFTRefundModel( short_name_id=short_name.id, @@ -79,7 +79,7 @@ def test_eft_refund_all_attributes(session): status=status, created_by=created_by, updated_by=updated_by, - updated_by_name=updated_by_name, + updated_name=updated_name, ) eft_refund.save() @@ -95,4 +95,4 @@ def test_eft_refund_all_attributes(session): assert eft_refund.status == status assert eft_refund.created_by == created_by assert eft_refund.updated_by == updated_by - assert eft_refund.updated_by_name == updated_by_name + assert eft_refund.updated_name == updated_name diff --git a/pay-api/tests/utilities/base_test.py b/pay-api/tests/utilities/base_test.py index ad7e1db7f..8f1c9ea6d 100644 --- a/pay-api/tests/utilities/base_test.py +++ b/pay-api/tests/utilities/base_test.py @@ -931,16 +931,17 @@ def factory_eft_credit(eft_file_id, short_name_id, amount=10.00, remaining_amoun ) -def factory_eft_refund(short_name_id, refund_amount, cas_supplier_number='1234567', - refund_email='test@test.com', comment='test comment'): +def factory_eft_refund(short_name_id, refund_amount=10.00, cas_supplier_number='1234567', + refund_email='test@test.com', comment='test comment', status='PENDING'): """Return an EFT Refund.""" return EFTRefund( short_name_id=short_name_id, refund_amount=refund_amount, cas_supplier_number=cas_supplier_number, refund_email=refund_email, - comment=comment - ) + comment=comment, + status=status + ).save() def factory_eft_credit_invoice_link(eft_credit_id, invoice_id, status_code, amount=1, link_group_id=None): From 424cea829479e359f8fdd976aac4737a10d63781 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Mon, 23 Sep 2024 13:17:53 -0700 Subject: [PATCH 03/16] Fix rejected flow so it moves money back to credit --- pay-api/src/pay_api/dtos/eft_shortname.py | 2 +- pay-api/src/pay_api/models/eft_credit.py | 1 + .../models/eft_short_names_historical.py | 2 +- .../pay_api/resources/v1/eft_short_names.py | 2 +- pay-api/src/pay_api/services/__init__.py | 2 +- pay-api/src/pay_api/services/eft_refund.py | 28 ++++++++----- pay-api/src/pay_api/services/eft_service.py | 2 +- .../services/eft_short_name_historical.py | 12 +++--- .../unit/api/test_eft_short_name_history.py | 2 +- .../tests/unit/api/test_eft_short_names.py | 39 +++++++++++++------ .../tests/unit/services/test_eft_service.py | 8 ++-- .../test_eft_short_name_historical.py | 2 +- 12 files changed, 65 insertions(+), 37 deletions(-) diff --git a/pay-api/src/pay_api/dtos/eft_shortname.py b/pay-api/src/pay_api/dtos/eft_shortname.py index caef8c4ee..25ea52925 100644 --- a/pay-api/src/pay_api/dtos/eft_shortname.py +++ b/pay-api/src/pay_api/dtos/eft_shortname.py @@ -66,8 +66,8 @@ class EFTShortNameRefundPatchRequest: """EFT Short name refund DTO.""" comment: str - decline_reason: str status: str + decline_reason: str = None @classmethod def from_dict(cls, data: dict): diff --git a/pay-api/src/pay_api/models/eft_credit.py b/pay-api/src/pay_api/models/eft_credit.py index 186d29b79..d5bb37dfd 100644 --- a/pay-api/src/pay_api/models/eft_credit.py +++ b/pay-api/src/pay_api/models/eft_credit.py @@ -80,5 +80,6 @@ def get_eft_credits(cls, short_name_id: int) -> List[Self]: return (cls.query .filter(EFTCredit.remaining_amount > 0) .filter(EFTCredit.short_name_id == short_name_id) + .with_for_update() .order_by(EFTCredit.created_on.asc()) .all()) diff --git a/pay-api/src/pay_api/models/eft_short_names_historical.py b/pay-api/src/pay_api/models/eft_short_names_historical.py index d5b508be2..83d5b8ef1 100644 --- a/pay-api/src/pay_api/models/eft_short_names_historical.py +++ b/pay-api/src/pay_api/models/eft_short_names_historical.py @@ -81,7 +81,7 @@ def find_by_related_group_link_id(cls, group_link_id: int) -> Self: @classmethod def find_by_eft_refund_id(cls, eft_refund_id: int) -> Self: """Find historical records by EFT refund id.""" - return cls.query.filter_by(eft_refund_id=eft_refund_id).one_or_none() + return cls.query.filter_by(eft_refund_id=eft_refund_id).order_by(EFTShortNamesHistorical.id.desc()).all() @define diff --git a/pay-api/src/pay_api/resources/v1/eft_short_names.py b/pay-api/src/pay_api/resources/v1/eft_short_names.py index bfc160ef3..8087044cb 100644 --- a/pay-api/src/pay_api/resources/v1/eft_short_names.py +++ b/pay-api/src/pay_api/resources/v1/eft_short_names.py @@ -22,7 +22,7 @@ from pay_api.exceptions import BusinessException, error_to_response from pay_api.schemas import utils as schema_utils from pay_api.services.eft_refund import EFTRefund as EFTRefundService -from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTShortnameHistoryService +from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTShortnameHistoryService from pay_api.services.eft_short_name_historical import EFTShortnameHistorySearch from pay_api.services.eft_short_name_summaries import EFTShortnameSummaries as EFTShortnameSummariesService from pay_api.services.eft_short_names import EFTShortnames as EFTShortnameService diff --git a/pay-api/src/pay_api/services/__init__.py b/pay-api/src/pay_api/services/__init__.py index d2dc098e2..a9cdfe65a 100755 --- a/pay-api/src/pay_api/services/__init__.py +++ b/pay-api/src/pay_api/services/__init__.py @@ -16,7 +16,7 @@ from .cfs_service import CFSService from .distribution_code import DistributionCode from .eft_service import EftService -from .eft_short_name_historical import EFTShortNameHistorical as EFTHistoryService +from .eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService from .eft_short_name_historical import EFTShortnameHistory, EFTShortnameHistorySearch from .eft_short_name_summaries import EFTShortnameSummaries as EFTShortNameSummaryService from .eft_short_names import EFTShortnames as EFTShortNamesService diff --git a/pay-api/src/pay_api/services/eft_refund.py b/pay-api/src/pay_api/services/eft_refund.py index 6eada8b35..b756bce91 100644 --- a/pay-api/src/pay_api/services/eft_refund.py +++ b/pay-api/src/pay_api/services/eft_refund.py @@ -12,7 +12,7 @@ from pay_api.models.eft_credit_invoice_link import EFTCreditInvoiceLink as EFTCreditInvoiceLinkModel from pay_api.models import EFTShortNamesHistorical as EFTHistoryModel from pay_api.services.email_service import _render_shortname_details_body, send_email -from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTHistoryService +from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService from pay_api.utils.enums import EFTCreditInvoiceStatus, EFTShortnameRefundStatus, InvoiceStatus from pay_api.utils.user_context import user_context from pay_api.utils.util import get_str_by_path @@ -28,9 +28,12 @@ def create_shortname_refund(request: dict, **kwargs): # This method isn't for invoices, it's for shortname only. shortname_id = get_str_by_path(request, 'shortNameId') shortname = get_str_by_path(request, 'shortName') - amount = get_str_by_path(request, 'refundAmount') + amount = Decimal(get_str_by_path(request, 'refundAmount')) comment = get_str_by_path(request, 'comment') + if amount <= 0: + raise BusinessException(Error.INVALID_REFUND) + current_app.logger.debug(f'Starting shortname refund : {shortname_id}') refund = EFTRefund._create_refund_model(request, shortname_id, amount, comment) @@ -133,9 +136,8 @@ def handle_invoice_refund(invoice: InvoiceModel, return InvoiceStatus.REFUND_REQUESTED.value @staticmethod - def refund_eft_credits(shortname_id: int, amount: str): + def refund_eft_credits(shortname_id: int, refund_amount: Decimal): """Refund the amount to eft_credits table based on short_name_id.""" - refund_amount = Decimal(amount) eft_credits = EFTCreditModel.get_eft_credits(shortname_id) eft_credit_balance = EFTCreditModel.get_eft_credit_balance(shortname_id) @@ -143,7 +145,7 @@ def refund_eft_credits(shortname_id: int, amount: str): raise BusinessException(Error.INVALID_REFUND) for credit in eft_credits: - if refund_amount <= 0: + if refund_amount == 0: break credit_amount = Decimal(credit.remaining_amount) if credit_amount <= 0: @@ -162,14 +164,22 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest if refund.status != EFTShortnameRefundStatus.PENDING_APPROVAL.value: raise BusinessException(Error.REFUND_ALREADY_FINALIZED) refund.comment = data.comment - if data.status == EFTShortnameRefundStatus.REJECTED.value: - refund.decline_reason = data.declined_reason refund.status = data.status - refund.save() + refund.decline_reason = data.decline_reason + refund.save_or_add(auto_save=False) + if data.status == EFTShortnameRefundStatus.REJECTED.value: + EFTRefund.refund_eft_credits(refund.short_name_id, -refund.refund_amount) + EFTHistoryService.create_shortname_refund( + EFTHistoryModel(short_name_id=refund.short_name_id, + amount=-refund.refund_amount, + credit_balance=EFTCreditModel.get_eft_credit_balance(refund.short_name_id), + eft_refund_id=refund.id, + is_processing=False, + hidden=False)).save() return refund.to_dict() @staticmethod - def _create_refund_model(request: dict, shortname_id: str, amount: str, comment: str) -> EFTRefundModel: + def _create_refund_model(request: dict, shortname_id: str, amount: Decimal, comment: str) -> EFTRefundModel: """Create and return the EFTRefundModel instance.""" # AP refund job should pick up this row and send back the amount in the refund via cheque. # For example if we had $500 on the EFT Shortname credits and we want to refund $300, diff --git a/pay-api/src/pay_api/services/eft_service.py b/pay-api/src/pay_api/services/eft_service.py index 263b1eb77..8a6355bdf 100644 --- a/pay-api/src/pay_api/services/eft_service.py +++ b/pay-api/src/pay_api/services/eft_service.py @@ -44,7 +44,7 @@ from .auth import get_account_admin_users from .deposit_service import DepositService -from .eft_short_name_historical import EFTShortNameHistorical as EFTHistoryService +from .eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService from .eft_short_name_historical import EFTShortnameHistory as EFTHistory from .eft_refund import EFTRefund as EFTRefundService from .email_service import _render_payment_reversed_template, send_email diff --git a/pay-api/src/pay_api/services/eft_short_name_historical.py b/pay-api/src/pay_api/services/eft_short_name_historical.py index db05c3bdc..a392136b0 100644 --- a/pay-api/src/pay_api/services/eft_short_name_historical.py +++ b/pay-api/src/pay_api/services/eft_short_name_historical.py @@ -53,7 +53,7 @@ class EFTShortnameHistorySearch: limit: Optional[int] = 10 -class EFTShortNameHistorical: +class EFTShortnameHistorical: """Service to manage EFT Short name historical data.""" @staticmethod @@ -66,7 +66,7 @@ def create_funds_received(history: EFTShortnameHistory) -> EFTShortnamesHistoric hidden=history.hidden, is_processing=history.is_processing, short_name_id=history.short_name_id, - transaction_date=EFTShortNameHistorical.transaction_date_now(), + transaction_date=EFTShortnameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.FUNDS_RECEIVED.value ) @@ -84,7 +84,7 @@ def create_statement_paid(history: EFTShortnameHistory, **kwargs) -> EFTShortnam related_group_link_id=history.related_group_link_id, short_name_id=history.short_name_id, statement_number=history.statement_number, - transaction_date=EFTShortNameHistorical.transaction_date_now(), + transaction_date=EFTShortnameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.STATEMENT_PAID.value ) @@ -102,7 +102,7 @@ def create_statement_reverse(history: EFTShortnameHistory, **kwargs) -> EFTShort related_group_link_id=history.related_group_link_id, short_name_id=history.short_name_id, statement_number=history.statement_number, - transaction_date=EFTShortNameHistorical.transaction_date_now(), + transaction_date=EFTShortnameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.STATEMENT_REVERSE.value ) @@ -121,7 +121,7 @@ def create_invoice_refund(history: EFTShortnameHistory, **kwargs) -> EFTShortnam short_name_id=history.short_name_id, statement_number=history.statement_number, invoice_id=history.invoice_id, - transaction_date=EFTShortNameHistorical.transaction_date_now(), + transaction_date=EFTShortnameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.INVOICE_REFUND.value ) @@ -137,7 +137,7 @@ def create_shortname_refund(history: EFTShortnameHistory, **kwargs) -> EFTShortn is_processing=history.is_processing, short_name_id=history.short_name_id, eft_refund_id=history.eft_refund_id, - transaction_date=EFTShortNameHistorical.transaction_date_now(), + transaction_date=EFTShortnameHistorical.transaction_date_now(), transaction_type=EFTHistoricalTypes.SN_REFUND_PENDING_APPROVAL.value ) diff --git a/pay-api/tests/unit/api/test_eft_short_name_history.py b/pay-api/tests/unit/api/test_eft_short_name_history.py index d2cd80cf7..db1198995 100755 --- a/pay-api/tests/unit/api/test_eft_short_name_history.py +++ b/pay-api/tests/unit/api/test_eft_short_name_history.py @@ -21,7 +21,7 @@ import pytest from freezegun import freeze_time -from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTHistoryService +from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService from pay_api.services.eft_short_name_historical import EFTShortnameHistory as EFTHistory from pay_api.utils.enums import EFTHistoricalTypes, InvoiceStatus, PaymentMethod, Role from tests.utilities.base_test import ( diff --git a/pay-api/tests/unit/api/test_eft_short_names.py b/pay-api/tests/unit/api/test_eft_short_names.py index c5b4b0d0d..20df0e868 100755 --- a/pay-api/tests/unit/api/test_eft_short_names.py +++ b/pay-api/tests/unit/api/test_eft_short_names.py @@ -976,25 +976,32 @@ def test_get_shortname_refund(session, client, jwt, query_string, test_name, cou assert len(rv.json) == count -@pytest.mark.parametrize('payload, test_name, role', [ - (EFTShortNameRefundPatchRequest( +@pytest.mark.parametrize('test_name, payload, role', [ + ('valid_approved_refund', EFTShortNameRefundPatchRequest( comment='Test comment', decline_reason='Test reason', status=EFTShortnameRefundStatus.APPROVED.value - ).to_dict(), 'valid_full_patch_refund', Role.EFT_REFUND_APPROVER.value), - ({}, 'unauthorized', Role.EFT_REFUND.value), - (EFTShortNameRefundPatchRequest( + ).to_dict(), Role.EFT_REFUND_APPROVER.value), + ('valid_rejected_refund', EFTShortNameRefundPatchRequest( + comment='Test comment', + decline_reason='Test reason', + status=EFTShortnameRefundStatus.REJECTED.value + ).to_dict(), Role.EFT_REFUND_APPROVER.value), + ('unauthorized', {}, Role.EFT_REFUND.value), + ('bad_transition', EFTShortNameRefundPatchRequest( comment='Test comment', decline_reason='Test reason', status=EFTShortnameRefundStatus.PENDING_APPROVAL.value - ).to_dict( - ), 'bad_transition', Role.EFT_REFUND_APPROVER.value), + ).to_dict(), Role.EFT_REFUND_APPROVER.value), ]) def test_patch_shortname_refund(session, client, jwt, payload, test_name, role): """Test patch short name refund.""" short_name = factory_eft_shortname('TEST_SHORTNAME').save() - refund = factory_eft_refund(short_name_id=short_name.id, + refund = factory_eft_refund(short_name_id=short_name.id, refund_amount=10, status=EFTShortnameRefundStatus.PENDING_APPROVAL.value) + eft_file = factory_eft_file().save() + eft_credit = EFTCreditModel(eft_file_id=eft_file.id, short_name_id=short_name.id, + amount=100, remaining_amount=90).save() if test_name == 'bad_transition': refund.status = EFTShortnameRefundStatus.APPROVED.value refund.save() @@ -1007,12 +1014,22 @@ def test_patch_shortname_refund(session, client, jwt, payload, test_name, role): case 'bad_transition' | 'invalid_patch_refund': assert rv.status_code == 400 assert 'REFUND_ALREADY_FINALIZED' in rv.json['type'] - case 'valid_full_patch_refund': + case 'valid_approved_refund': assert rv.status_code == 200 assert rv.json['status'] == EFTShortnameRefundStatus.APPROVED.value assert rv.json['comment'] == 'Test comment' - # Can't set declined reason on an approved. - assert 'declineReason' not in rv.json + case 'valid_rejected_refund': + assert rv.status_code == 200 + assert rv.json['status'] == EFTShortnameRefundStatus.REJECTED.value + assert rv.json['comment'] == 'Test comment' + # assert EFTHistoryModel + history = EFTShortnamesHistoryModel.find_by_eft_refund_id(refund.id)[0] + assert history + assert history.amount == -10 + assert history.credit_balance == 90 + 10 + eft_credit = EFTCreditModel.find_by_id(eft_credit.id) + assert eft_credit.remaining_amount == 90 + 10 + assert rv.json['declineReason'] case _: raise ValueError(f'Invalid test case {test_name}') diff --git a/pay-api/tests/unit/services/test_eft_service.py b/pay-api/tests/unit/services/test_eft_service.py index 06d422e2a..6366afb06 100644 --- a/pay-api/tests/unit/services/test_eft_service.py +++ b/pay-api/tests/unit/services/test_eft_service.py @@ -78,7 +78,7 @@ def test_refund_eft_credits(session): with patch('pay_api.models.EFTCredit.get_eft_credits', return_value=[credit1, credit2, credit3]), \ patch('pay_api.models.EFTCredit.get_eft_credit_balance', return_value=9): - EFTRefundService.refund_eft_credits(1, '8') + EFTRefundService.refund_eft_credits(1, 8) assert credit1.remaining_amount == 0 assert credit2.remaining_amount == 0 assert credit3.remaining_amount == 1 @@ -87,7 +87,7 @@ def test_refund_eft_credits(session): credit2.remaining_amount = 5 with patch('pay_api.models.EFTCredit.get_eft_credit_balance', return_value=10): - EFTRefundService.refund_eft_credits(1, '7') + EFTRefundService.refund_eft_credits(1, 7) assert credit1.remaining_amount == 0 assert credit2.remaining_amount == 3 @@ -95,7 +95,7 @@ def test_refund_eft_credits(session): credit2.remaining_amount = 2 with patch('pay_api.models.EFTCredit.get_eft_credit_balance', return_value=7): - EFTRefundService.refund_eft_credits(1, '1') + EFTRefundService.refund_eft_credits(1, 1) assert credit1.remaining_amount == 4 assert credit2.remaining_amount == 2 @@ -111,7 +111,7 @@ def test_refund_eft_credits_exceed_balance(session): patch('pay_api.models.EFTCredit.get_eft_credit_balance', return_value=8): with pytest.raises(BusinessException) as excinfo: - EFTRefundService.refund_eft_credits(1, '20') + EFTRefundService.refund_eft_credits(1, 20) assert excinfo.value.code == Error.INVALID_REFUND.name diff --git a/pay-api/tests/unit/services/test_eft_short_name_historical.py b/pay-api/tests/unit/services/test_eft_short_name_historical.py index fc8fc7ab0..abe3643d8 100644 --- a/pay-api/tests/unit/services/test_eft_short_name_historical.py +++ b/pay-api/tests/unit/services/test_eft_short_name_historical.py @@ -21,7 +21,7 @@ from freezegun import freeze_time from pay_api.models.eft_short_names_historical import EFTShortNamesHistorical as EFTShortnameHistory -from pay_api.services.eft_short_name_historical import EFTShortNameHistorical as EFTShortnameHistoryService +from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTShortnameHistoryService from pay_api.utils.enums import EFTHistoricalTypes, InvoiceStatus, PaymentMethod from tests.utilities.base_test import ( factory_eft_refund, factory_eft_shortname, factory_invoice, factory_payment_account) From c50ec5a019ce6fbe34477d02a6a3f8df43ecfacb Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Mon, 23 Sep 2024 13:18:28 -0700 Subject: [PATCH 04/16] Update pay-queue to point at new pay-api --- pay-queue/poetry.lock | 8 ++++---- pay-queue/pyproject.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/pay-queue/poetry.lock b/pay-queue/poetry.lock index 3742d490e..498722818 100644 --- a/pay-queue/poetry.lock +++ b/pay-queue/poetry.lock @@ -1979,9 +1979,9 @@ werkzeug = "3.0.3" [package.source] type = "git" -url = "https://github.com/ochiu/sbc-pay.git" -reference = "22982-EFT-Wire" -resolved_reference = "a2fe75dfc54f2531981ddfc95d4aad74c4e15a84" +url = "https://github.com/seeker25/sbc-pay.git" +reference = "21537" +resolved_reference = "424cea829479e359f8fdd976aac4737a10d63781" subdirectory = "pay-api" [[package]] @@ -3104,4 +3104,4 @@ multidict = ">=4.0" [metadata] lock-version = "2.0" python-versions = "^3.12" -content-hash = "2a400928cdd8f903afdf9a3938f1ba9864605f09a1be75649296ec3615b40507" +content-hash = "1f549058e6873465c87ef26cd34c5e532b607e0ff6cc77c09707f258168e84da" diff --git a/pay-queue/pyproject.toml b/pay-queue/pyproject.toml index d2a1518e4..cef3b8a33 100644 --- a/pay-queue/pyproject.toml +++ b/pay-queue/pyproject.toml @@ -20,7 +20,7 @@ jinja2 = "^3.1.3" protobuf = "4.25.3" launchdarkly-server-sdk = "^8.2.1" cachecontrol = "^0.14.0" -pay-api = {git = "https://github.com/ochiu/sbc-pay.git", subdirectory = "pay-api", branch = "22982-EFT-Wire"} +pay-api = {git = "https://github.com/seeker25/sbc-pay.git", subdirectory = "pay-api", branch = "21537"} pg8000 = "^1.30.5" From 3a6cd30a996eff2a44f7c7ff2622bc24dc2c84ac Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Mon, 23 Sep 2024 13:24:56 -0700 Subject: [PATCH 05/16] minor change --- pay-api/src/pay_api/models/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pay-api/src/pay_api/models/__init__.py b/pay-api/src/pay_api/models/__init__.py index 3da025f5e..8ad38ca14 100755 --- a/pay-api/src/pay_api/models/__init__.py +++ b/pay-api/src/pay_api/models/__init__.py @@ -35,7 +35,7 @@ from .eft_refund_email_list import EFTRefundEmailList from .eft_short_name_links import EFTShortnameLinks, EFTShortnameLinkSchema from .eft_short_names import EFTShortnames, EFTShortnameSchema, EFTShortnameSummarySchema -from .eft_short_names_historical import EFTShortnameHistorySchema, EFTShortNamesHistorical +from .eft_short_names_historical import EFTShortnameHistorySchema, EFTShortnamesHistorical from .eft_transaction import EFTTransaction, EFTTransactionSchema from .ejv_file import EjvFile from .ejv_header import EjvHeader From 5dee7413398eadac4560f8dc134e0572a4d1b642 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Mon, 23 Sep 2024 13:25:51 -0700 Subject: [PATCH 06/16] update Poetry --- pay-queue/poetry.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pay-queue/poetry.lock b/pay-queue/poetry.lock index 498722818..b0816857b 100644 --- a/pay-queue/poetry.lock +++ b/pay-queue/poetry.lock @@ -1981,7 +1981,7 @@ werkzeug = "3.0.3" type = "git" url = "https://github.com/seeker25/sbc-pay.git" reference = "21537" -resolved_reference = "424cea829479e359f8fdd976aac4737a10d63781" +resolved_reference = "3a6cd30a996eff2a44f7c7ff2622bc24dc2c84ac" subdirectory = "pay-api" [[package]] From 31090b710c00f879fa366c350f23adce7c39748b Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Mon, 23 Sep 2024 13:30:40 -0700 Subject: [PATCH 07/16] Fix naming --- .../src/pay_api/models/eft_short_names_historical.py | 4 ++-- pay-api/src/pay_api/services/eft_refund.py | 2 +- pay-api/src/pay_api/services/eft_service.py | 2 +- .../src/pay_api/services/eft_short_name_historical.py | 2 +- pay-api/tests/unit/api/test_eft_short_names.py | 2 +- .../unit/models/test_eft_short_names_historical.py | 10 +++++----- pay-api/tests/unit/services/test_eft_service.py | 2 +- .../unit/services/test_eft_short_name_historical.py | 2 +- 8 files changed, 13 insertions(+), 13 deletions(-) diff --git a/pay-api/src/pay_api/models/eft_short_names_historical.py b/pay-api/src/pay_api/models/eft_short_names_historical.py index 83d5b8ef1..402432f3c 100644 --- a/pay-api/src/pay_api/models/eft_short_names_historical.py +++ b/pay-api/src/pay_api/models/eft_short_names_historical.py @@ -23,7 +23,7 @@ from .db import db -class EFTShortNamesHistorical(BaseModel): +class EFTShortnamesHistorical(BaseModel): """This class manages all EFT Short name historical data.""" __tablename__ = 'eft_short_names_historical' @@ -81,7 +81,7 @@ def find_by_related_group_link_id(cls, group_link_id: int) -> Self: @classmethod def find_by_eft_refund_id(cls, eft_refund_id: int) -> Self: """Find historical records by EFT refund id.""" - return cls.query.filter_by(eft_refund_id=eft_refund_id).order_by(EFTShortNamesHistorical.id.desc()).all() + return cls.query.filter_by(eft_refund_id=eft_refund_id).order_by(EFTShortnamesHistorical.id.desc()).all() @define diff --git a/pay-api/src/pay_api/services/eft_refund.py b/pay-api/src/pay_api/services/eft_refund.py index b756bce91..78c296215 100644 --- a/pay-api/src/pay_api/services/eft_refund.py +++ b/pay-api/src/pay_api/services/eft_refund.py @@ -10,7 +10,7 @@ from pay_api.models.eft_refund_email_list import EFTRefundEmailList 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.models import EFTShortNamesHistorical as EFTHistoryModel +from pay_api.models import EFTShortnamesHistorical as EFTHistoryModel from pay_api.services.email_service import _render_shortname_details_body, send_email from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService from pay_api.utils.enums import EFTCreditInvoiceStatus, EFTShortnameRefundStatus, InvoiceStatus diff --git a/pay-api/src/pay_api/services/eft_service.py b/pay-api/src/pay_api/services/eft_service.py index 8a6355bdf..2a891dcaa 100644 --- a/pay-api/src/pay_api/services/eft_service.py +++ b/pay-api/src/pay_api/services/eft_service.py @@ -25,7 +25,7 @@ from pay_api.models import EFTCredit as EFTCreditModel from pay_api.models import EFTCreditInvoiceLink as EFTCreditInvoiceLinkModel from pay_api.models import EFTShortnameLinks as EFTShortnameLinksModel -from pay_api.models import EFTShortNamesHistorical as EFTHistoryModel +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 Payment as PaymentModel diff --git a/pay-api/src/pay_api/services/eft_short_name_historical.py b/pay-api/src/pay_api/services/eft_short_name_historical.py index a392136b0..6a9ee33bc 100644 --- a/pay-api/src/pay_api/services/eft_short_name_historical.py +++ b/pay-api/src/pay_api/services/eft_short_name_historical.py @@ -20,7 +20,7 @@ from sqlalchemy import and_, case, exists, false, func, select from sqlalchemy.orm import aliased -from pay_api.models import EFTShortNamesHistorical as EFTShortnamesHistoricalModel +from pay_api.models import EFTShortnamesHistorical as EFTShortnamesHistoricalModel from pay_api.models import PaymentAccount as PaymentAccountModel from pay_api.models import db from pay_api.models import EFTShortnameHistorySchema diff --git a/pay-api/tests/unit/api/test_eft_short_names.py b/pay-api/tests/unit/api/test_eft_short_names.py index 20df0e868..0e41087bb 100755 --- a/pay-api/tests/unit/api/test_eft_short_names.py +++ b/pay-api/tests/unit/api/test_eft_short_names.py @@ -30,7 +30,7 @@ from pay_api.models import EFTFile as EFTFileModel from pay_api.models import EFTShortnameLinks as EFTShortnameLinksModel from pay_api.models import EFTShortnames as EFTShortnamesModel -from pay_api.models import EFTShortNamesHistorical as EFTShortnamesHistoryModel +from pay_api.models import EFTShortnamesHistorical as EFTShortnamesHistoryModel from pay_api.models import EFTTransaction as EFTTransactionModel from pay_api.models import PaymentAccount as PaymentAccountModel from pay_api.models.eft_refund import EFTRefund as EFTRefundModel diff --git a/pay-api/tests/unit/models/test_eft_short_names_historical.py b/pay-api/tests/unit/models/test_eft_short_names_historical.py index b773b0df7..d2447e431 100644 --- a/pay-api/tests/unit/models/test_eft_short_names_historical.py +++ b/pay-api/tests/unit/models/test_eft_short_names_historical.py @@ -18,7 +18,7 @@ """ from datetime import datetime, timezone -from pay_api.models import EFTShortNamesHistorical +from pay_api.models import EFTShortnamesHistorical from pay_api.utils.enums import EFTHistoricalTypes from tests.utilities.base_test import factory_eft_shortname, factory_payment_account @@ -34,7 +34,7 @@ def test_eft_short_names_historical(session): eft_short_name.save() now_date = datetime.now(tz=timezone.utc).date() - default_historical = EFTShortNamesHistorical( + default_historical = EFTShortnamesHistorical( amount=151.50, created_by='USER1', credit_balance=1234.50, @@ -43,7 +43,7 @@ def test_eft_short_names_historical(session): transaction_type=EFTHistoricalTypes.FUNDS_RECEIVED.value ).save() - default_historical = EFTShortNamesHistorical.find_by_id(default_historical.id) + default_historical = EFTShortnamesHistorical.find_by_id(default_historical.id) assert default_historical.id is not None assert default_historical.amount == 151.50 assert default_historical.created_on.date() == now_date @@ -58,7 +58,7 @@ def test_eft_short_names_historical(session): assert default_historical.transaction_date.date() == now_date assert default_historical.transaction_type == EFTHistoricalTypes.FUNDS_RECEIVED.value - short_name_historical = EFTShortNamesHistorical( + short_name_historical = EFTShortnamesHistorical( amount=123.50, created_by='USER1', credit_balance=456.50, @@ -72,7 +72,7 @@ def test_eft_short_names_historical(session): transaction_type=EFTHistoricalTypes.STATEMENT_PAID.value ).save() - short_name_historical = EFTShortNamesHistorical.find_by_id(short_name_historical.id) + short_name_historical = EFTShortnamesHistorical.find_by_id(short_name_historical.id) assert short_name_historical.id is not None assert short_name_historical.amount == 123.50 diff --git a/pay-api/tests/unit/services/test_eft_service.py b/pay-api/tests/unit/services/test_eft_service.py index 6366afb06..c9b6a8f57 100644 --- a/pay-api/tests/unit/services/test_eft_service.py +++ b/pay-api/tests/unit/services/test_eft_service.py @@ -24,7 +24,7 @@ from pay_api.exceptions import BusinessException from pay_api.models import EFTCreditInvoiceLink as EFTCreditInvoiceLinkModel -from pay_api.models import EFTShortNamesHistorical as EFTHistoryModel +from pay_api.models import EFTShortnamesHistorical as EFTHistoryModel from pay_api.services.eft_service import EftService from pay_api.services.eft_refund import EFTRefund as EFTRefundService from pay_api.utils.enums import ( diff --git a/pay-api/tests/unit/services/test_eft_short_name_historical.py b/pay-api/tests/unit/services/test_eft_short_name_historical.py index abe3643d8..2829a1cf8 100644 --- a/pay-api/tests/unit/services/test_eft_short_name_historical.py +++ b/pay-api/tests/unit/services/test_eft_short_name_historical.py @@ -20,7 +20,7 @@ from freezegun import freeze_time -from pay_api.models.eft_short_names_historical import EFTShortNamesHistorical as EFTShortnameHistory +from pay_api.models.eft_short_names_historical import EFTShortnamesHistorical as EFTShortnameHistory from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTShortnameHistoryService from pay_api.utils.enums import EFTHistoricalTypes, InvoiceStatus, PaymentMethod from tests.utilities.base_test import ( From 5a504e1885b8c77d337afd034a1620e657dd4734 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Mon, 23 Sep 2024 13:31:02 -0700 Subject: [PATCH 08/16] update poetry --- pay-queue/poetry.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pay-queue/poetry.lock b/pay-queue/poetry.lock index b0816857b..c9ec6e384 100644 --- a/pay-queue/poetry.lock +++ b/pay-queue/poetry.lock @@ -1981,7 +1981,7 @@ werkzeug = "3.0.3" type = "git" url = "https://github.com/seeker25/sbc-pay.git" reference = "21537" -resolved_reference = "3a6cd30a996eff2a44f7c7ff2622bc24dc2c84ac" +resolved_reference = "31090b710c00f879fa366c350f23adce7c39748b" subdirectory = "pay-api" [[package]] From 9afe3d18a393e473d07cb12c008a5a815df6931d Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Tue, 24 Sep 2024 06:55:04 -0700 Subject: [PATCH 09/16] Refactor into dataclass, remove short_name from request grab from database instead, rename REJECTED - >DECLINED so it matches business verbage, build serializable base class to assist with ATTRS serialization --- pay-api/src/pay_api/dtos/eft_shortname.py | 25 ++----- .../schemas/schemas/refund_shortname.json | 10 --- pay-api/src/pay_api/services/eft_refund.py | 75 ++++++++++++++----- pay-api/src/pay_api/services/eft_service.py | 2 +- pay-api/src/pay_api/services/email_service.py | 44 ++++++----- pay-api/src/pay_api/utils/enums.py | 4 +- pay-api/src/pay_api/utils/serializable.py | 15 ++++ .../tests/unit/api/test_eft_short_names.py | 11 +-- 8 files changed, 111 insertions(+), 75 deletions(-) create mode 100644 pay-api/src/pay_api/utils/serializable.py diff --git a/pay-api/src/pay_api/dtos/eft_shortname.py b/pay-api/src/pay_api/dtos/eft_shortname.py index 25ea52925..76f2a54ec 100644 --- a/pay-api/src/pay_api/dtos/eft_shortname.py +++ b/pay-api/src/pay_api/dtos/eft_shortname.py @@ -10,11 +10,11 @@ from typing import List from attrs import define -from pay_api.utils.converter import Converter +from pay_api.utils.serializable import Serializable @define -class EFTShortNameGetRequest: +class EFTShortNameGetRequest(Serializable): """EFT Short name search.""" short_name: str = None @@ -33,7 +33,7 @@ class EFTShortNameGetRequest: @classmethod def from_dict(cls, data: dict): """Convert from request args to EFTShortNameSearchDTO.""" - dto = Converter(camel_to_snake_case=True).structure(data, EFTShortNameGetRequest) + dto = super().from_dict(data) # In the future, we'll need a cleaner way to handle this. dto.state = dto.state.split(',') if dto.state else None dto.account_id_list = dto.account_id_list.split(',') if dto.account_id_list else None @@ -41,7 +41,7 @@ def from_dict(cls, data: dict): @define -class EFTShortNameSummaryGetRequest: +class EFTShortNameSummaryGetRequest(Serializable): """EFT Short name summary search.""" short_name: str = None @@ -54,30 +54,15 @@ class EFTShortNameSummaryGetRequest: page: int = 1 limit: int = 10 - @classmethod - def from_dict(cls, data: dict): - """Convert from request args to EFTShortNameSummarySearchDTO.""" - dto = Converter(camel_to_snake_case=True).structure(data, EFTShortNameSummaryGetRequest) - return dto - @define -class EFTShortNameRefundPatchRequest: +class EFTShortNameRefundPatchRequest(Serializable): """EFT Short name refund DTO.""" comment: str status: str decline_reason: str = None - @classmethod - def from_dict(cls, data: dict): - """Convert from request json to EFTShortNameRefundDTO.""" - return Converter(camel_to_snake_case=True).structure(data, EFTShortNameRefundPatchRequest) - - def to_dict(self): - """Convert from EFTShortNameRefundDTO to request json.""" - return Converter(snake_case_to_camel=True).unstructure(self) - @define class EFTShortNameRefundGetRequest: diff --git a/pay-api/src/pay_api/schemas/schemas/refund_shortname.json b/pay-api/src/pay_api/schemas/schemas/refund_shortname.json index 05c2b06f4..ad6246be9 100644 --- a/pay-api/src/pay_api/schemas/schemas/refund_shortname.json +++ b/pay-api/src/pay_api/schemas/schemas/refund_shortname.json @@ -17,16 +17,6 @@ "24" ] }, - "short_name":{ - "$id":"#/properties/shortName", - "type":"string", - "title":"Short Name", - "description":"short name.", - "default":"", - "examples":[ - "24" - ] - }, "refund_amount":{ "$id":"#/properties/refundAmount", "type":"number", diff --git a/pay-api/src/pay_api/services/eft_refund.py b/pay-api/src/pay_api/services/eft_refund.py index 78c296215..dd2af3df7 100644 --- a/pay-api/src/pay_api/services/eft_refund.py +++ b/pay-api/src/pay_api/services/eft_refund.py @@ -11,7 +11,8 @@ 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.models import EFTShortnamesHistorical as EFTHistoryModel -from pay_api.services.email_service import _render_shortname_details_body, send_email +from pay_api.models import EFTShortnames as EFTShortnamesModel +from pay_api.services.email_service import ShortNameRefundEmailContent, send_email from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService from pay_api.utils.enums import EFTCreditInvoiceStatus, EFTShortnameRefundStatus, InvoiceStatus from pay_api.utils.user_context import user_context @@ -26,8 +27,7 @@ class EFTRefund: def create_shortname_refund(request: dict, **kwargs): """Create refund.""" # This method isn't for invoices, it's for shortname only. - shortname_id = get_str_by_path(request, 'shortNameId') - shortname = get_str_by_path(request, 'shortName') + shortname_id = int(get_str_by_path(request, 'shortNameId')) amount = Decimal(get_str_by_path(request, 'refundAmount')) comment = get_str_by_path(request, 'comment') @@ -36,21 +36,29 @@ def create_shortname_refund(request: dict, **kwargs): current_app.logger.debug(f'Starting shortname refund : {shortname_id}') + shortname = EFTShortnamesModel.find_by_id(shortname_id) refund = EFTRefund._create_refund_model(request, shortname_id, amount, comment) - EFTRefund.refund_eft_credits(int(shortname_id), amount) + EFTRefund.refund_eft_credits(shortname_id, amount) history = EFTHistoryService.create_shortname_refund( EFTHistoryModel(short_name_id=shortname_id, amount=amount, - credit_balance=EFTCreditModel.get_eft_credit_balance(int(shortname_id)), + credit_balance=EFTCreditModel.get_eft_credit_balance(shortname_id), eft_refund_id=refund.id, is_processing=False, hidden=False)).flush() recipients = EFTRefundEmailList.find_all_emails() - subject = f'Pending Refund Request for Short Name {shortname}' - html_body = _render_shortname_details_body(shortname, amount, comment, shortname_id) - + subject = f'Pending Refund Request for Short Name {shortname.short_name}' + html_body = ShortNameRefundEmailContent( + comment=comment, + decline_reason=refund.decline_reason, + refund_amount=amount, + short_name_id=shortname_id, + short_name=shortname.short_name, + status=EFTShortnameRefundStatus.PENDING_APPROVAL.value, + url=f"{current_app.config.get('AUTH_WEB_URL')}/pay/shortname-details/{shortname_id}", + ).render_body() send_email(recipients, subject, html_body, **kwargs) history.save() refund.save() @@ -158,7 +166,8 @@ def refund_eft_credits(shortname_id: int, refund_amount: Decimal): credit.save() @staticmethod - def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest) -> EFTRefundModel: + @user_context + def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest, **kwargs) -> EFTRefundModel: """Update the refund status.""" refund = EFTRefundModel.find_by_id(refund_id) if refund.status != EFTShortnameRefundStatus.PENDING_APPROVAL.value: @@ -167,19 +176,47 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest refund.status = data.status refund.decline_reason = data.decline_reason refund.save_or_add(auto_save=False) - if data.status == EFTShortnameRefundStatus.REJECTED.value: - EFTRefund.refund_eft_credits(refund.short_name_id, -refund.refund_amount) - EFTHistoryService.create_shortname_refund( - EFTHistoryModel(short_name_id=refund.short_name_id, - amount=-refund.refund_amount, - credit_balance=EFTCreditModel.get_eft_credit_balance(refund.short_name_id), - eft_refund_id=refund.id, - is_processing=False, - hidden=False)).save() + shortname = EFTShortnamesModel.find_by_id(refund.short_name_id) + recipients = EFTRefundEmailList.find_all_emails() + match data.status: + case EFTShortnameRefundStatus.DECLINED.value: + EFTRefund.refund_eft_credits(refund.short_name_id, -refund.refund_amount) + EFTHistoryService.create_shortname_refund( + EFTHistoryModel(short_name_id=refund.short_name_id, + amount=-refund.refund_amount, + credit_balance=EFTCreditModel.get_eft_credit_balance(refund.short_name_id), + eft_refund_id=refund.id, + is_processing=False, + hidden=False)).save() + subject = f'Declined Refund Request for Short Name {shortname.short_name}' + body = ShortNameRefundEmailContent( + comment=refund.comment, + decline_reason=refund.decline_reason, + refund_amount=refund.refund_amount, + short_name_id=refund.short_name_id, + short_name=shortname.short_name, + status=data.status, + url=f"{current_app.config.get('AUTH_WEB_URL')}/pay/shortname-details/{refund.short_name_id}", + ).render_body() + send_email(recipients, subject, body, **kwargs) + case EFTShortnameRefundStatus.APPROVED.value: + subject = f'Approved Refund Request for Short Name {shortname.short_name}' + body = ShortNameRefundEmailContent( + comment=refund.comment, + decline_reason=refund.decline_reason, + refund_amount=refund.refund_amount, + short_name_id=refund.short_name_id, + short_name=shortname.short_name, + status=data.status, + url=f"{current_app.config.get('AUTH_WEB_URL')}/pay/shortname-details/{refund.short_name_id}", + ).render_body() + send_email(recipients, subject, body, **kwargs) + case _: + pass return refund.to_dict() @staticmethod - def _create_refund_model(request: dict, shortname_id: str, amount: Decimal, comment: str) -> EFTRefundModel: + def _create_refund_model(request: dict, shortname_id: int, amount: Decimal, comment: str) -> EFTRefundModel: """Create and return the EFTRefundModel instance.""" # AP refund job should pick up this row and send back the amount in the refund via cheque. # For example if we had $500 on the EFT Shortname credits and we want to refund $300, diff --git a/pay-api/src/pay_api/services/eft_service.py b/pay-api/src/pay_api/services/eft_service.py index 2a891dcaa..2abb9218f 100644 --- a/pay-api/src/pay_api/services/eft_service.py +++ b/pay-api/src/pay_api/services/eft_service.py @@ -308,7 +308,7 @@ def _send_reversed_payment_notification(statement: StatementModel, reversed_amou send_email(recipients=recipients, subject='Outstanding Balance Adjustment Notice', - html_body=_render_payment_reversed_template(email_params), + body=_render_payment_reversed_template(email_params), **kwargs) @staticmethod diff --git a/pay-api/src/pay_api/services/email_service.py b/pay-api/src/pay_api/services/email_service.py index 3b055982f..00d9e3356 100644 --- a/pay-api/src/pay_api/services/email_service.py +++ b/pay-api/src/pay_api/services/email_service.py @@ -15,18 +15,22 @@ """This manages all of the email notification service.""" import os from typing import Dict +from decimal import Decimal +from attr import define from flask import current_app from jinja2 import Environment, FileSystemLoader from pay_api.exceptions import ServiceUnavailableException from pay_api.services.oauth_service import OAuthService from pay_api.utils.enums import AuthHeaderType, ContentType +from pay_api.utils.serializable import Serializable from pay_api.utils.user_context import user_context @user_context -def send_email(recipients: list, subject: str, html_body: str, **kwargs): +def send_email(recipients: list, subject: str, body: str, **kwargs): """Send the email notification.""" + # Note if we send HTML in the body, we aren't sending through GCNotify, ideally we'd like to send through GCNotify. token = kwargs['user'].bearer_token current_app.logger.info(f'>send_email to recipients: {recipients}') notify_url = current_app.config.get('NOTIFY_API_ENDPOINT') + 'notify/' @@ -38,7 +42,7 @@ def send_email(recipients: list, subject: str, html_body: str, **kwargs): 'recipients': recipient, 'content': { 'subject': subject, - 'body': html_body + 'body': body } } @@ -56,22 +60,26 @@ def send_email(recipients: list, subject: str, html_body: str, **kwargs): return success -def _render_shortname_details_body(shortname: str, amount: str, comment: str, shortname_id: str) -> str: - """Render the email body using the provided template.""" - current_dir = os.path.dirname(os.path.abspath(__file__)) - project_root_dir = os.path.dirname(current_dir) - templates_dir = os.path.join(project_root_dir, 'templates') - env = Environment(loader=FileSystemLoader(templates_dir), autoescape=True) - template = env.get_template('eft_refund_notification.html') - - url = f"{current_app.config.get('AUTH_WEB_URL')}/pay/shortname-details/{shortname_id}" - params = { - 'shortname': shortname, - 'refundAmount': amount, - 'comment': comment, - 'url': url - } - return template.render(params) +@define +class ShortNameRefundEmailContent(Serializable): + """Short name refund email.""" + + comment: str + decline_reason: str + refund_amount: Decimal + short_name_id: int + short_name: str + status: str + url: str + + def render_body(self) -> str: + """Render the email body using the provided template.""" + current_dir = os.path.dirname(os.path.abspath(__file__)) + project_root_dir = os.path.dirname(current_dir) + templates_dir = os.path.join(project_root_dir, 'templates') + env = Environment(loader=FileSystemLoader(templates_dir), autoescape=True) + template = env.get_template('eft_refund_notification.html') + return template.render(self.to_dict()) def _render_payment_reversed_template(params: Dict) -> str: diff --git a/pay-api/src/pay_api/utils/enums.py b/pay-api/src/pay_api/utils/enums.py index 149bc9ff5..eae692e90 100644 --- a/pay-api/src/pay_api/utils/enums.py +++ b/pay-api/src/pay_api/utils/enums.py @@ -371,7 +371,7 @@ class EFTShortnameRefundStatus(Enum): APPROVED = 'APPROVED' PENDING_APPROVAL = 'PENDING_APPROVAL' - REJECTED = 'REJECTED' + DECLINED = 'DECLINED' class EFTPaymentActions(Enum): @@ -393,7 +393,7 @@ class EFTHistoricalTypes(Enum): # Short name refund statuses SN_REFUND_PENDING_APPROVAL = 'SN_REFUND_PENDING_APPROVAL' SN_REFUND_APPROVED = 'SN_REFUND_APPROVED' - SN_REFUND_REJECTED = 'SN_REFUND_REJECTED' + SN_REFUND_DECLINED = 'SN_REFUND_DECLINED' class PaymentDetailsGlStatus(Enum): diff --git a/pay-api/src/pay_api/utils/serializable.py b/pay-api/src/pay_api/utils/serializable.py new file mode 100644 index 000000000..9ba2355b9 --- /dev/null +++ b/pay-api/src/pay_api/utils/serializable.py @@ -0,0 +1,15 @@ +"""Serializable class for cattr structure and unstructure.""" +from pay_api.utils.converter import Converter + + +class Serializable: + """Helper for cattr structure and unstructure (serialization/deserialization).""" + + @classmethod + def from_dict(cls, data: dict): + """Convert from dictionary to object.""" + return Converter(camel_to_snake_case=True).structure(data, cls) + + def to_dict(self): + """Convert from object to dictionary.""" + return Converter(snake_case_to_camel=True).unstructure(self) diff --git a/pay-api/tests/unit/api/test_eft_short_names.py b/pay-api/tests/unit/api/test_eft_short_names.py index 0e41087bb..198684821 100755 --- a/pay-api/tests/unit/api/test_eft_short_names.py +++ b/pay-api/tests/unit/api/test_eft_short_names.py @@ -957,8 +957,9 @@ def test_post_shortname_refund_invalid_request(client, mocker, jwt): @pytest.mark.parametrize('query_string, test_name, count', [ ('', 'get_all', 3), - ('?status=APPROVED,PENDING_APPROVAL', 'status_filter_multiple', 2), - ('?status=REJECTED', 'status_filter_rejected', 1), + (f'?status={EFTShortnameRefundStatus.APPROVED.value},{ + EFTShortnameRefundStatus.PENDING_APPROVAL.value}', 'status_filter_multiple', 2), + (f'?status={EFTShortnameRefundStatus.DECLINED.value}', 'status_filter_rejected', 1), ]) def test_get_shortname_refund(session, client, jwt, query_string, test_name, count): """Test get short name refund.""" @@ -968,7 +969,7 @@ def test_get_shortname_refund(session, client, jwt, query_string, test_name, cou factory_eft_refund(short_name_id=short_name.id, status=EFTShortnameRefundStatus.PENDING_APPROVAL.value) factory_eft_refund(short_name_id=short_name.id, - status=EFTShortnameRefundStatus.REJECTED.value) + status=EFTShortnameRefundStatus.DECLINED.value) token = jwt.create_jwt(get_claims(roles=[Role.EFT_REFUND.value]), token_header) headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} rv = client.get(f'/api/v1/eft-shortnames/shortname-refund{query_string}', headers=headers) @@ -985,7 +986,7 @@ def test_get_shortname_refund(session, client, jwt, query_string, test_name, cou ('valid_rejected_refund', EFTShortNameRefundPatchRequest( comment='Test comment', decline_reason='Test reason', - status=EFTShortnameRefundStatus.REJECTED.value + status=EFTShortnameRefundStatus.DECLINED.value ).to_dict(), Role.EFT_REFUND_APPROVER.value), ('unauthorized', {}, Role.EFT_REFUND.value), ('bad_transition', EFTShortNameRefundPatchRequest( @@ -1020,7 +1021,7 @@ def test_patch_shortname_refund(session, client, jwt, payload, test_name, role): assert rv.json['comment'] == 'Test comment' case 'valid_rejected_refund': assert rv.status_code == 200 - assert rv.json['status'] == EFTShortnameRefundStatus.REJECTED.value + assert rv.json['status'] == EFTShortnameRefundStatus.DECLINED.value assert rv.json['comment'] == 'Test comment' # assert EFTHistoryModel history = EFTShortnamesHistoryModel.find_by_eft_refund_id(refund.id)[0] From 5851cc8b01c9259efd4471987505f3ef5768a74d Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Tue, 24 Sep 2024 09:18:40 -0700 Subject: [PATCH 10/16] Unit test for new credit reversal --- pay-api/src/pay_api/models/eft_credit.py | 4 +- pay-api/src/pay_api/services/eft_refund.py | 38 ++++++++++++++----- pay-api/src/pay_api/utils/errors.py | 9 +---- .../tests/unit/services/test_eft_service.py | 32 ++++++++++++++++ pay-api/tests/utilities/base_test.py | 8 ++-- 5 files changed, 69 insertions(+), 22 deletions(-) diff --git a/pay-api/src/pay_api/models/eft_credit.py b/pay-api/src/pay_api/models/eft_credit.py index d5bb37dfd..a067c9ec9 100644 --- a/pay-api/src/pay_api/models/eft_credit.py +++ b/pay-api/src/pay_api/models/eft_credit.py @@ -75,10 +75,10 @@ def get_eft_credit_balance(cls, short_name_id: int) -> Decimal: return Decimal(result.credit_balance) if result else 0 @classmethod - def get_eft_credits(cls, short_name_id: int) -> List[Self]: + def get_eft_credits(cls, short_name_id: int, include_zero_remaining=False) -> List[Self]: """Get EFT Credits with a remaining amount.""" return (cls.query - .filter(EFTCredit.remaining_amount > 0) + .filter(include_zero_remaining or EFTCredit.remaining_amount > 0) .filter(EFTCredit.short_name_id == short_name_id) .with_for_update() .order_by(EFTCredit.created_on.asc()) diff --git a/pay-api/src/pay_api/services/eft_refund.py b/pay-api/src/pay_api/services/eft_refund.py index dd2af3df7..0c1682fef 100644 --- a/pay-api/src/pay_api/services/eft_refund.py +++ b/pay-api/src/pay_api/services/eft_refund.py @@ -144,26 +144,46 @@ def handle_invoice_refund(invoice: InvoiceModel, return InvoiceStatus.REFUND_REQUESTED.value @staticmethod - def refund_eft_credits(shortname_id: int, refund_amount: Decimal): + def reverse_eft_credits(shortname_id: int, amount: Decimal): + """Reverse the amount to eft_credits table based on short_name_id.""" + eft_credits = EFTCreditModel.get_eft_credits(shortname_id, include_zero_remaining=True) + for credit in eft_credits: + if credit.remaining_amount == credit.amount: + continue + if credit.remaining_amount > credit.amount: + raise BusinessException(Error.EFT_CREDIT_AMOUNT_UNEXPECTED) + + credit_adjustment = min(amount, credit.amount - credit.remaining_amount) + amount -= credit_adjustment + credit.remaining_amount += credit_adjustment + credit.flush() + + # Scenario where we tried to reverse an amount all the credits, + # but for some reason our refund was more than the original amount on the credits. + if amount > 0: + raise BusinessException(Error.INVALID_REFUND) + + @staticmethod + def refund_eft_credits(shortname_id: int, amount: Decimal): """Refund the amount to eft_credits table based on short_name_id.""" eft_credits = EFTCreditModel.get_eft_credits(shortname_id) eft_credit_balance = EFTCreditModel.get_eft_credit_balance(shortname_id) - - if refund_amount > eft_credit_balance: + if amount > eft_credit_balance or amount <= 0: raise BusinessException(Error.INVALID_REFUND) for credit in eft_credits: - if refund_amount == 0: - break credit_amount = Decimal(credit.remaining_amount) if credit_amount <= 0: continue - deduction = min(refund_amount, credit_amount) + deduction = min(amount, credit_amount) credit.remaining_amount -= deduction - refund_amount -= deduction + amount -= deduction - credit.save() + credit.flush() + # Scenario where we couldn't subtract the remaining amount from the credits. + if amount > 0: + raise BusinessException(Error.INVALID_REFUND) @staticmethod @user_context @@ -180,7 +200,7 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest recipients = EFTRefundEmailList.find_all_emails() match data.status: case EFTShortnameRefundStatus.DECLINED.value: - EFTRefund.refund_eft_credits(refund.short_name_id, -refund.refund_amount) + EFTRefund.reverse_eft_credits(refund.short_name_id, refund.refund_amount) EFTHistoryService.create_shortname_refund( EFTHistoryModel(short_name_id=refund.short_name_id, amount=-refund.refund_amount, diff --git a/pay-api/src/pay_api/utils/errors.py b/pay-api/src/pay_api/utils/errors.py index a2a64db77..c3ad4756e 100644 --- a/pay-api/src/pay_api/utils/errors.py +++ b/pay-api/src/pay_api/utils/errors.py @@ -59,6 +59,7 @@ class Error(Enum): SERVICE_UNAVAILABLE = 'SERVICE_UNAVAILABLE', HTTPStatus.SERVICE_UNAVAILABLE INVALID_REQUEST = 'INVALID_REQUEST', HTTPStatus.BAD_REQUEST, 'Invalid Request' + PATCH_INVALID_ACTION = 'PATCH_INVALID_ACTION', HTTPStatus.BAD_REQUEST PAYMENT_SEARCH_TOO_MANY_RECORDS = 'PAYMENT_SEARCH_TOO_MANY_RECORDS', HTTPStatus.BAD_REQUEST @@ -72,7 +73,6 @@ class Error(Enum): CFS_INVOICES_MISMATCH = 'CFS_INVOICES_MISMATCH', HTTPStatus.BAD_REQUEST - # EFT Errors EFT_CREDIT_AMOUNT_UNEXPECTED = 'EFT_CREDIT_AMOUNT_UNEXPECTED', HTTPStatus.BAD_REQUEST EFT_INSUFFICIENT_CREDITS = 'EFT_INSUFFICIENT_CREDITS', HTTPStatus.BAD_REQUEST EFT_PAYMENT_ACTION_ACCOUNT_ID_REQUIRED = 'EFT_PAYMENT_ACTION_ACCOUNT_ID_REQUIRED', HTTPStatus.BAD_REQUEST @@ -93,13 +93,11 @@ class Error(Enum): EFT_SHORT_NAME_OUTSTANDING_BALANCE = 'EFT_SHORT_NAME_OUTSTANDING_BALANCE', HTTPStatus.BAD_REQUEST EFT_INVOICES_OVERDUE = 'EFT_INVOICES_OVERDUE', HTTPStatus.BAD_REQUEST - # FAS Errors FAS_INVALID_PAYMENT_METHOD = 'FAS_INVALID_PAYMENT_METHOD', HTTPStatus.BAD_REQUEST FAS_INVALID_ROUTING_SLIP_NUMBER = 'FAS_INVALID_ROUTING_SLIP_NUMBER', HTTPStatus.BAD_REQUEST FAS_INVALID_ROUTING_SLIP_DIGITS = 'FAS_INVALID_ROUTING_SLIP_DIGITS', HTTPStatus.BAD_REQUEST - PATCH_INVALID_ACTION = 'PATCH_INVALID_ACTION', HTTPStatus.BAD_REQUEST + FAS_INVALID_RS_STATUS_CHANGE = 'FAS_INVALID_RS_STATUS_CHANGE', HTTPStatus.BAD_REQUEST - # Refund Errors ROUTING_SLIP_REFUND = 'ROUTING_SLIP_REFUND', HTTPStatus.BAD_REQUEST NO_FEE_REFUND = 'NO_FEE_REFUND', HTTPStatus.BAD_REQUEST INVALID_CONSOLIDATED_REFUND = 'INVALID_CONSOLIDATED_REFUND', HTTPStatus.BAD_REQUEST @@ -113,12 +111,9 @@ class Error(Enum): RS_IN_INVALID_STATUS = 'RS_IN_INVALID_STATUS', HTTPStatus.BAD_REQUEST RS_CANT_LINK_NSF = 'RS_CANT_LINK_NSF', HTTPStatus.BAD_REQUEST RS_HAS_TRANSACTIONS = 'RS_HAS_TRANSACTIONS', HTTPStatus.BAD_REQUEST - - # routing slip RS_INSUFFICIENT_FUNDS = 'RS_INSUFFICIENT_FUNDS', HTTPStatus.BAD_REQUEST RS_DOESNT_EXIST = 'RS_DOESNT_EXIST', HTTPStatus.BAD_REQUEST RS_NOT_ACTIVE = 'RS_NOT_ACTIVE', HTTPStatus.BAD_REQUEST - FAS_INVALID_RS_STATUS_CHANGE = 'FAS_INVALID_RS_STATUS_CHANGE', HTTPStatus.BAD_REQUEST def __new__(cls, code, status, message=None, details=None): """Attributes for the enum.""" diff --git a/pay-api/tests/unit/services/test_eft_service.py b/pay-api/tests/unit/services/test_eft_service.py index c9b6a8f57..fab54b374 100644 --- a/pay-api/tests/unit/services/test_eft_service.py +++ b/pay-api/tests/unit/services/test_eft_service.py @@ -23,6 +23,7 @@ import pytest from pay_api.exceptions import BusinessException +from pay_api.models import EFTCredit as EFTCreditModel from pay_api.models import EFTCreditInvoiceLink as EFTCreditInvoiceLinkModel from pay_api.models import EFTShortnamesHistorical as EFTHistoryModel from pay_api.services.eft_service import EftService @@ -100,6 +101,37 @@ def test_refund_eft_credits(session): assert credit2.remaining_amount == 2 +@pytest.mark.parametrize('test_name', [ + ('reverse_eft_credit_success'), + ('reverse_eft_credit_leftover_fail'), + ('reverse_eft_credit_remaining_higher_than_original_amount_fail'), +]) +def test_refund_eft_credit_reversal(session, test_name): + """Test refund eft credit reversal.""" + file = factory_eft_file().save() + shortname = factory_eft_shortname(short_name='TESTSHORTNAME123').save() + match test_name: + case 'reverse_eft_credit_success': + factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=10) + factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=7) + factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=9) + factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=1, remaining_amount=0) + EFTRefundService.reverse_eft_credits(shortname.id, 5) + assert EFTCreditModel.query.filter_by(remaining_amount=10).count() == 3 + assert EFTCreditModel.query.filter_by(remaining_amount=1).count() == 1 + case 'reverse_eft_credit_leftover_fail': + factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=10) + factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=9) + with pytest.raises(BusinessException) as excinfo: + EFTRefundService.reverse_eft_credits(shortname.id, 5) + assert excinfo.value.code == Error.INVALID_REFUND.name + case 'reverse_eft_credit_remaining_higher_than_original_amount_fail': + factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=15) + with pytest.raises(BusinessException) as excinfo: + EFTRefundService.reverse_eft_credits(shortname.id, 5) + assert excinfo.value.code == Error.INVALID_REFUND.name + + def test_refund_eft_credits_exceed_balance(session): """Test refund amount exceeds eft_credit_balance.""" credit1 = MagicMock(remaining_amount=2) diff --git a/pay-api/tests/utilities/base_test.py b/pay-api/tests/utilities/base_test.py index 8f1c9ea6d..d584521cf 100644 --- a/pay-api/tests/utilities/base_test.py +++ b/pay-api/tests/utilities/base_test.py @@ -893,7 +893,7 @@ def factory_comments(routing_slip_number: str, username: str = 'comment_user', c comment = Comment(submitter_name=username, routing_slip_number=routing_slip_number, comment=comment - ) + ).save() return comment @@ -904,7 +904,7 @@ def factory_eft_file(file_ref: str = 'test_ref.txt'): def factory_eft_shortname(short_name: str, short_name_type: str = EFTShortnameType.EFT.value): """Return an EFT short name model.""" - return EFTShortnames(short_name=short_name, type=short_name_type) + return EFTShortnames(short_name=short_name, type=short_name_type).save() def factory_eft_shortname_link(short_name_id: int, auth_account_id: str = '1234', @@ -917,7 +917,7 @@ def factory_eft_shortname_link(short_name_id: int, auth_account_id: str = '1234' updated_by=updated_by, updated_by_name=updated_by, updated_on=updated_on - ) + ).save() def factory_eft_credit(eft_file_id, short_name_id, amount=10.00, remaining_amount=10.00): @@ -928,7 +928,7 @@ def factory_eft_credit(eft_file_id, short_name_id, amount=10.00, remaining_amoun remaining_amount=remaining_amount, eft_file_id=eft_file_id, short_name_id=short_name_id - ) + ).save() def factory_eft_refund(short_name_id, refund_amount=10.00, cas_supplier_number='1234567', From 4d9798f1f81c5771c728bc6c05c1647a48a8d982 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Tue, 24 Sep 2024 10:43:05 -0700 Subject: [PATCH 11/16] update templates --- .../templates/eft_refund_notification.html | 15 --------------- .../eft_refund_notification_client.html | 9 +++++++++ .../templates/eft_refund_notification_staff.html | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 15 deletions(-) delete mode 100644 pay-api/src/pay_api/templates/eft_refund_notification.html create mode 100644 pay-api/src/pay_api/templates/eft_refund_notification_client.html create mode 100644 pay-api/src/pay_api/templates/eft_refund_notification_staff.html diff --git a/pay-api/src/pay_api/templates/eft_refund_notification.html b/pay-api/src/pay_api/templates/eft_refund_notification.html deleted file mode 100644 index 7fdbf6bc8..000000000 --- a/pay-api/src/pay_api/templates/eft_refund_notification.html +++ /dev/null @@ -1,15 +0,0 @@ - - - - Pending Refund Request - - -

Pending Refund Request for Short Name {{ shortname }}

- -

There is a refund request of ${{ refundAmount }} for short name: {{ shortname }}.

-

{{ comment }}

- -

Please review the information using the following link:

-

Log in to view detail

- - diff --git a/pay-api/src/pay_api/templates/eft_refund_notification_client.html b/pay-api/src/pay_api/templates/eft_refund_notification_client.html new file mode 100644 index 000000000..8b00a25f5 --- /dev/null +++ b/pay-api/src/pay_api/templates/eft_refund_notification_client.html @@ -0,0 +1,9 @@ +# Refund Notice + +A refund of {{ refundAmount }} has been issued to your original payment method. The processing time will be 3-5 business days. + +**Business Registry** +BC Registries and Online Services +Toll Free: [1-877-526-1526](1-877-526-1526) +Victoria Office: [250-387-7848](250-387-7848) +Email: [BCRegistries@gov.bc.ca](BCRegistries@gov.bc.ca) diff --git a/pay-api/src/pay_api/templates/eft_refund_notification_staff.html b/pay-api/src/pay_api/templates/eft_refund_notification_staff.html new file mode 100644 index 000000000..67f2fae5f --- /dev/null +++ b/pay-api/src/pay_api/templates/eft_refund_notification_staff.html @@ -0,0 +1,16 @@ +# {{ state }} Refund Request for Short Name {{ shortname }} + +There is a refund request of ${{ refundAmount }} for short name: {{ shortname }} +{% if state == 'APPROVED' %} + is now approved. +{% elif state == 'DECLINED' %} + is declined with the following message: {{ decline_message }} +{% elif state == 'PENDING' %} + . +{% else %} + . +{% endif %} + +Please review the information using the following link: + +[Log in to view details]({{ url }}) From 49ac0d07497d643200a4b7201e7b3ccf657ea512 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Tue, 24 Sep 2024 16:56:26 -0700 Subject: [PATCH 12/16] Unit tests passing --- .../versions/2024_09_24_87a09ba91c0d_.py | 28 +++++++++ pay-api/src/pay_api/models/__init__.py | 1 - .../pay_api/models/eft_refund_email_list.py | 55 ------------------ pay-api/src/pay_api/services/auth.py | 33 +++++++++++ pay-api/src/pay_api/services/eft_refund.py | 58 ++++++++++--------- pay-api/src/pay_api/services/email_service.py | 14 +++-- pay-api/tests/conftest.py | 23 ++++++++ .../tests/unit/api/test_eft_short_names.py | 5 +- .../tests/unit/services/test_eft_service.py | 26 ++++----- .../tests/unit/services/test_email_service.py | 29 ++-------- .../integration/test_eft_reconciliation.py | 10 ++-- 11 files changed, 150 insertions(+), 132 deletions(-) create mode 100644 pay-api/migrations/versions/2024_09_24_87a09ba91c0d_.py delete mode 100644 pay-api/src/pay_api/models/eft_refund_email_list.py diff --git a/pay-api/migrations/versions/2024_09_24_87a09ba91c0d_.py b/pay-api/migrations/versions/2024_09_24_87a09ba91c0d_.py new file mode 100644 index 000000000..44b948fa9 --- /dev/null +++ b/pay-api/migrations/versions/2024_09_24_87a09ba91c0d_.py @@ -0,0 +1,28 @@ +"""Drop refund email list, use keycloak instead. + +Revision ID: 87a09ba91c0d +Revises: 75a39e02c746 +Create Date: 2024-09-24 14:01:25.238895 + +""" +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +# 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 = '87a09ba91c0d' +down_revision = '75a39e02c746' +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute('DROP SEQUENCE IF EXISTS eft_refund_email_list_id_seq CASCADE') + op.execute('DROP TABLE IF EXISTS eft_refund_email_list') + +def downgrade(): + pass diff --git a/pay-api/src/pay_api/models/__init__.py b/pay-api/src/pay_api/models/__init__.py index 8ad38ca14..17182011a 100755 --- a/pay-api/src/pay_api/models/__init__.py +++ b/pay-api/src/pay_api/models/__init__.py @@ -32,7 +32,6 @@ from .eft_file import EFTFile from .eft_process_status_code import EFTProcessStatusCode from .eft_refund import EFTRefund -from .eft_refund_email_list import EFTRefundEmailList from .eft_short_name_links import EFTShortnameLinks, EFTShortnameLinkSchema from .eft_short_names import EFTShortnames, EFTShortnameSchema, EFTShortnameSummarySchema from .eft_short_names_historical import EFTShortnameHistorySchema, EFTShortnamesHistorical diff --git a/pay-api/src/pay_api/models/eft_refund_email_list.py b/pay-api/src/pay_api/models/eft_refund_email_list.py deleted file mode 100644 index 402889350..000000000 --- a/pay-api/src/pay_api/models/eft_refund_email_list.py +++ /dev/null @@ -1,55 +0,0 @@ -# Copyright © 2024 Province of British Columbia -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Model to handle all operations related to Fee related to accounts.""" -from __future__ import annotations - -from sqlalchemy import Sequence - -from .base_model import BaseModel -from .db import db - - -class EFTRefundEmailList(BaseModel): - """This class manages all of the emails for EFT refund notifications.""" - - __tablename__ = 'eft_refund_email_list' - # this mapper is used so that new and old versions of the service can be run simultaneously, - # making rolling upgrades easier - # This is used by SQLAlchemy to explicitly define which fields we're interested - # so it doesn't freak out and say it can't map the structure if other fields are present. - # This could occur from a failed deploy or during an upgrade. - # The other option is to tell SQLAlchemy to ignore differences, but that is ambiguous - # and can interfere with Alembic upgrades. - # - # NOTE: please keep mapper names in alpha-order, easier to track that way - # Exception, id is always first, _fields first - __mapper_args__ = { - 'include_properties': [ - 'email', - 'first_name', - 'id', - 'last_name', - ] - } - - email = db.Column(db.String(25), nullable=False) - first_name = db.Column(db.String(25), nullable=True) - id = db.Column(db.Integer, Sequence('eft_refund_email_list_id_seq'), primary_key=True) - last_name = db.Column(db.String(25), nullable=True) - - @classmethod - def find_all_emails(cls): - """Return by email_list.""" - email_list = cls.query.with_entities(cls.email).all() - return [email[0] for email in email_list] diff --git a/pay-api/src/pay_api/services/auth.py b/pay-api/src/pay_api/services/auth.py index 33b89199a..4d0c4602c 100644 --- a/pay-api/src/pay_api/services/auth.py +++ b/pay-api/src/pay_api/services/auth.py @@ -13,9 +13,11 @@ # limitations under the License. """This manages all of the authorization service.""" +import base64 from flask import abort, current_app, g from pay_api.services.code import Code as CodeService +from pay_api.services.flags import flags from pay_api.services.oauth_service import OAuthService as RestService from pay_api.utils.enums import AccountType, AuthHeaderType, Code, ContentType, PaymentMethod, Role from pay_api.utils.user_context import UserContext, user_context @@ -111,8 +113,39 @@ def check_auth(business_identifier: str, account_id: str = None, corp_type_code: @user_context def get_account_admin_users(auth_account_id, **kwargs): """Retrieve account admin users.""" + # Only works for STAFF and ADMINS of the org. return RestService.get( current_app.config.get('AUTH_API_ENDPOINT') + f'orgs/{auth_account_id}/members?status=ACTIVE&roles=ADMIN', kwargs['user'].bearer_token, AuthHeaderType.BEARER, ContentType.JSON).json() + + +def get_emails_with_keycloak_role(role: str) -> str: + """Retrieve emails with the specified keycloak role.""" + users = get_users_with_keycloak_role(role) + # Purpose of this is so our TEST users don't get hammered with emails, also our tester can easily switch this on. + if override_emails := flags.is_on('override-eft-refund-emails', default=False): + return override_emails + return ','.join([user['email'] for user in users]) + + +def get_users_with_keycloak_role(role: str) -> dict: + """Retrieve users with the specified keycloak role.""" + url = current_app.config.get('AUTH_API_ENDPOINT') + f'keycloak/users?role={role}' + return RestService.get(url, get_service_account_token(), AuthHeaderType.BEARER, ContentType.JSON).json() + + +def get_service_account_token(): + """Get service account token.""" + issuer_url = current_app.config.get('JWT_OIDC_ISSUER') + token_url = issuer_url + '/protocol/openid-connect/token' + service_account_id = current_app.config.get('KEYCLOAK_SERVICE_ACCOUNT_ID') + service_account_secret = current_app.config.get('KEYCLOAK_SERVICE_ACCOUNT_SECRET') + auth_str = f'{service_account_id}:{service_account_secret}' + basic_auth_encoded = base64.b64encode(auth_str.encode('utf-8')).decode('utf-8') + data = 'grant_type=client_credentials' + token_response = RestService.post(token_url, basic_auth_encoded, AuthHeaderType.BASIC, ContentType.FORM_URL_ENCODED, + data) + bearer_token = token_response.json()['access_token'] + return bearer_token diff --git a/pay-api/src/pay_api/services/eft_refund.py b/pay-api/src/pay_api/services/eft_refund.py index 0c1682fef..2470f1adb 100644 --- a/pay-api/src/pay_api/services/eft_refund.py +++ b/pay-api/src/pay_api/services/eft_refund.py @@ -7,14 +7,14 @@ from pay_api.models import Invoice as InvoiceModel from pay_api.models import EFTRefund as EFTRefundModel from pay_api.models import PaymentAccount -from pay_api.models.eft_refund_email_list import EFTRefundEmailList 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.models import EFTShortnamesHistorical as EFTHistoryModel from pay_api.models import EFTShortnames as EFTShortnamesModel +from pay_api.services.auth import get_emails_with_keycloak_role from pay_api.services.email_service import ShortNameRefundEmailContent, send_email from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService -from pay_api.utils.enums import EFTCreditInvoiceStatus, EFTShortnameRefundStatus, InvoiceStatus +from pay_api.utils.enums import EFTCreditInvoiceStatus, EFTShortnameRefundStatus, InvoiceStatus, Role from pay_api.utils.user_context import user_context from pay_api.utils.util import get_str_by_path @@ -27,39 +27,39 @@ class EFTRefund: def create_shortname_refund(request: dict, **kwargs): """Create refund.""" # This method isn't for invoices, it's for shortname only. - shortname_id = int(get_str_by_path(request, 'shortNameId')) + short_name_id = int(get_str_by_path(request, 'shortNameId')) amount = Decimal(get_str_by_path(request, 'refundAmount')) comment = get_str_by_path(request, 'comment') if amount <= 0: raise BusinessException(Error.INVALID_REFUND) - current_app.logger.debug(f'Starting shortname refund : {shortname_id}') + current_app.logger.debug(f'Starting shortname refund : {short_name_id}') - shortname = EFTShortnamesModel.find_by_id(shortname_id) - refund = EFTRefund._create_refund_model(request, shortname_id, amount, comment) - EFTRefund.refund_eft_credits(shortname_id, amount) + short_name = EFTShortnamesModel.find_by_id(short_name_id) + refund = EFTRefund._create_refund_model(request, short_name_id, amount, comment) + EFTRefund.refund_eft_credits(short_name_id, amount) history = EFTHistoryService.create_shortname_refund( - EFTHistoryModel(short_name_id=shortname_id, + EFTHistoryModel(short_name_id=short_name_id, amount=amount, - credit_balance=EFTCreditModel.get_eft_credit_balance(shortname_id), + credit_balance=EFTCreditModel.get_eft_credit_balance(short_name_id), eft_refund_id=refund.id, is_processing=False, hidden=False)).flush() - recipients = EFTRefundEmailList.find_all_emails() - subject = f'Pending Refund Request for Short Name {shortname.short_name}' + qualified_receiver_recipients = get_emails_with_keycloak_role(Role.EFT_REFUND.value) + subject = f'Pending Refund Request for Short Name {short_name.short_name}' html_body = ShortNameRefundEmailContent( comment=comment, decline_reason=refund.decline_reason, refund_amount=amount, - short_name_id=shortname_id, - short_name=shortname.short_name, + short_name_id=short_name_id, + short_name=short_name.short_name, status=EFTShortnameRefundStatus.PENDING_APPROVAL.value, - url=f"{current_app.config.get('AUTH_WEB_URL')}/pay/shortname-details/{shortname_id}", + url=f"{current_app.config.get('AUTH_WEB_URL')}/pay/shortname-details/{short_name_id}", ).render_body() - send_email(recipients, subject, html_body, **kwargs) + send_email(qualified_receiver_recipients, subject, html_body, **kwargs) history.save() refund.save() @@ -186,8 +186,7 @@ def refund_eft_credits(shortname_id: int, amount: Decimal): raise BusinessException(Error.INVALID_REFUND) @staticmethod - @user_context - def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest, **kwargs) -> EFTRefundModel: + def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest) -> EFTRefundModel: """Update the refund status.""" refund = EFTRefundModel.find_by_id(refund_id) if refund.status != EFTShortnameRefundStatus.PENDING_APPROVAL.value: @@ -196,8 +195,7 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest refund.status = data.status refund.decline_reason = data.decline_reason refund.save_or_add(auto_save=False) - shortname = EFTShortnamesModel.find_by_id(refund.short_name_id) - recipients = EFTRefundEmailList.find_all_emails() + short_name = EFTShortnamesModel.find_by_id(refund.short_name_id) match data.status: case EFTShortnameRefundStatus.DECLINED.value: EFTRefund.reverse_eft_credits(refund.short_name_id, refund.refund_amount) @@ -208,29 +206,35 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest eft_refund_id=refund.id, is_processing=False, hidden=False)).save() - subject = f'Declined Refund Request for Short Name {shortname.short_name}' + subject = f'Declined Refund Request for Short Name {short_name.short_name}' body = ShortNameRefundEmailContent( comment=refund.comment, decline_reason=refund.decline_reason, refund_amount=refund.refund_amount, short_name_id=refund.short_name_id, - short_name=shortname.short_name, + short_name=short_name.short_name, status=data.status, url=f"{current_app.config.get('AUTH_WEB_URL')}/pay/shortname-details/{refund.short_name_id}", ).render_body() - send_email(recipients, subject, body, **kwargs) + expense_authority_recipients = get_emails_with_keycloak_role(Role.EFT_REFUND_APPROVER.value) + send_email(expense_authority_recipients, subject, body) case EFTShortnameRefundStatus.APPROVED.value: - subject = f'Approved Refund Request for Short Name {shortname.short_name}' - body = ShortNameRefundEmailContent( + subject = f'Approved Refund Request for Short Name {short_name.short_name}' + content = ShortNameRefundEmailContent( comment=refund.comment, decline_reason=refund.decline_reason, refund_amount=refund.refund_amount, short_name_id=refund.short_name_id, - short_name=shortname.short_name, + short_name=short_name.short_name, status=data.status, url=f"{current_app.config.get('AUTH_WEB_URL')}/pay/shortname-details/{refund.short_name_id}", - ).render_body() - send_email(recipients, subject, body, **kwargs) + ) + staff_body = content.render_body() + expense_authority_recipients = get_emails_with_keycloak_role(Role.EFT_REFUND_APPROVER.value) + send_email(expense_authority_recipients, subject, staff_body) + client_recipients = refund.refund_email + client_body = content.render_body(is_for_client=True) + send_email(client_recipients, subject, client_body) case _: pass return refund.to_dict() diff --git a/pay-api/src/pay_api/services/email_service.py b/pay-api/src/pay_api/services/email_service.py index 00d9e3356..87b458ad8 100644 --- a/pay-api/src/pay_api/services/email_service.py +++ b/pay-api/src/pay_api/services/email_service.py @@ -21,17 +21,16 @@ from flask import current_app from jinja2 import Environment, FileSystemLoader from pay_api.exceptions import ServiceUnavailableException +from pay_api.services.auth import get_service_account_token from pay_api.services.oauth_service import OAuthService from pay_api.utils.enums import AuthHeaderType, ContentType from pay_api.utils.serializable import Serializable -from pay_api.utils.user_context import user_context -@user_context -def send_email(recipients: list, subject: str, body: str, **kwargs): +def send_email(recipients: list, subject: str, body: str): """Send the email notification.""" # Note if we send HTML in the body, we aren't sending through GCNotify, ideally we'd like to send through GCNotify. - token = kwargs['user'].bearer_token + token = get_service_account_token() current_app.logger.info(f'>send_email to recipients: {recipients}') notify_url = current_app.config.get('NOTIFY_API_ENDPOINT') + 'notify/' @@ -72,13 +71,16 @@ class ShortNameRefundEmailContent(Serializable): status: str url: str - def render_body(self) -> str: + def render_body(self, is_for_client=False) -> str: """Render the email body using the provided template.""" current_dir = os.path.dirname(os.path.abspath(__file__)) project_root_dir = os.path.dirname(current_dir) templates_dir = os.path.join(project_root_dir, 'templates') env = Environment(loader=FileSystemLoader(templates_dir), autoescape=True) - template = env.get_template('eft_refund_notification.html') + if is_for_client: + template = env.get_template('eft_refund_notification_client.html') + else: + template = env.get_template('eft_refund_notification_staff.html') return template.render(self.to_dict()) diff --git a/pay-api/tests/conftest.py b/pay-api/tests/conftest.py index 4a2c25176..203f241d6 100755 --- a/pay-api/tests/conftest.py +++ b/pay-api/tests/conftest.py @@ -298,3 +298,26 @@ def get_account_admin_users(auth_account_id): get_account_admin_users) monkeypatch.setattr('pay_api.services.eft_service.get_account_admin_users', get_account_admin_users) + + +@pytest.fixture() +def emails_with_keycloak_role_mock(monkeypatch): + """Mock auth rest call to get org admins.""" + def get_emails_with_keycloak_role(role): + return 'hello@goodnight.com' + + monkeypatch.setattr('pay_api.services.auth.get_emails_with_keycloak_role', + get_emails_with_keycloak_role) + monkeypatch.setattr('pay_api.services.eft_refund.get_emails_with_keycloak_role', + get_emails_with_keycloak_role) + + +@pytest.fixture() +def send_email_mock(monkeypatch): + """Mock send_email.""" + def send_email(recipients, subject, body): + return True + + # Note this needs to be moved to a prism spec, we need to come up with one for NotifyAPI. + monkeypatch.setattr('pay_api.services.email_service.send_email', send_email) + monkeypatch.setattr('pay_api.services.eft_refund.send_email', send_email) diff --git a/pay-api/tests/unit/api/test_eft_short_names.py b/pay-api/tests/unit/api/test_eft_short_names.py index 198684821..22fdc861f 100755 --- a/pay-api/tests/unit/api/test_eft_short_names.py +++ b/pay-api/tests/unit/api/test_eft_short_names.py @@ -902,7 +902,7 @@ def test_search_eft_short_names(session, client, jwt, app): data_dict['single-linked']['statement_summary'][0]) -def test_post_shortname_refund_success(db, session, client, jwt): +def test_post_shortname_refund_success(db, session, client, jwt, emails_with_keycloak_role_mock): """Test successful creation of a shortname refund.""" token = jwt.create_jwt(get_claims(roles=[Role.EFT_REFUND.value]), token_header) headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} @@ -995,7 +995,8 @@ def test_get_shortname_refund(session, client, jwt, query_string, test_name, cou status=EFTShortnameRefundStatus.PENDING_APPROVAL.value ).to_dict(), Role.EFT_REFUND_APPROVER.value), ]) -def test_patch_shortname_refund(session, client, jwt, payload, test_name, role): +def test_patch_shortname_refund(session, client, jwt, payload, test_name, role, + send_email_mock, emails_with_keycloak_role_mock): """Test patch short name refund.""" short_name = factory_eft_shortname('TEST_SHORTNAME').save() refund = factory_eft_refund(short_name_id=short_name.id, refund_amount=10, diff --git a/pay-api/tests/unit/services/test_eft_service.py b/pay-api/tests/unit/services/test_eft_service.py index fab54b374..19acf4625 100644 --- a/pay-api/tests/unit/services/test_eft_service.py +++ b/pay-api/tests/unit/services/test_eft_service.py @@ -109,26 +109,26 @@ def test_refund_eft_credits(session): def test_refund_eft_credit_reversal(session, test_name): """Test refund eft credit reversal.""" file = factory_eft_file().save() - shortname = factory_eft_shortname(short_name='TESTSHORTNAME123').save() + short_name = factory_eft_shortname(short_name='TESTSHORTNAME123').save() match test_name: case 'reverse_eft_credit_success': - factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=10) - factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=7) - factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=9) - factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=1, remaining_amount=0) - EFTRefundService.reverse_eft_credits(shortname.id, 5) + factory_eft_credit(eft_file_id=file.id, short_name_id=short_name.id, amount=10, remaining_amount=10) + factory_eft_credit(eft_file_id=file.id, short_name_id=short_name.id, amount=10, remaining_amount=7) + factory_eft_credit(eft_file_id=file.id, short_name_id=short_name.id, amount=10, remaining_amount=9) + factory_eft_credit(eft_file_id=file.id, short_name_id=short_name.id, amount=1, remaining_amount=0) + EFTRefundService.reverse_eft_credits(short_name.id, 5) assert EFTCreditModel.query.filter_by(remaining_amount=10).count() == 3 assert EFTCreditModel.query.filter_by(remaining_amount=1).count() == 1 case 'reverse_eft_credit_leftover_fail': - factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=10) - factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=9) + factory_eft_credit(eft_file_id=file.id, short_name_id=short_name.id, amount=10, remaining_amount=10) + factory_eft_credit(eft_file_id=file.id, short_name_id=short_name.id, amount=10, remaining_amount=9) with pytest.raises(BusinessException) as excinfo: - EFTRefundService.reverse_eft_credits(shortname.id, 5) + EFTRefundService.reverse_eft_credits(short_name.id, 5) assert excinfo.value.code == Error.INVALID_REFUND.name case 'reverse_eft_credit_remaining_higher_than_original_amount_fail': - factory_eft_credit(eft_file_id=file.id, short_name_id=shortname.id, amount=10, remaining_amount=15) + factory_eft_credit(eft_file_id=file.id, short_name_id=short_name.id, amount=10, remaining_amount=15) with pytest.raises(BusinessException) as excinfo: - EFTRefundService.reverse_eft_credits(shortname.id, 5) + EFTRefundService.reverse_eft_credits(short_name.id, 5) assert excinfo.value.code == Error.INVALID_REFUND.name @@ -163,9 +163,9 @@ def test_eft_invoice_refund(session, test_name): status_code=InvoiceStatus.APPROVED.value, total=5).save() eft_file = factory_eft_file().save() - shortname = factory_eft_shortname(short_name='TESTSHORTNAME123').save() + short_name = factory_eft_shortname(short_name='TESTSHORTNAME123').save() eft_credit = factory_eft_credit(eft_file_id=eft_file.id, - short_name_id=shortname.id, + short_name_id=short_name.id, amount=10, remaining_amount=1).save() match test_name: diff --git a/pay-api/tests/unit/services/test_email_service.py b/pay-api/tests/unit/services/test_email_service.py index de3c242c3..1484ccf5e 100644 --- a/pay-api/tests/unit/services/test_email_service.py +++ b/pay-api/tests/unit/services/test_email_service.py @@ -15,7 +15,7 @@ """Tests for Email Service.""" import json -from unittest.mock import MagicMock, patch +from unittest.mock import patch from pay_api.services.email_service import send_email from pay_api.utils.enums import AuthHeaderType, ContentType @@ -25,32 +25,15 @@ def test_send_email(app, monkeypatch): """Test send email.""" app.config['NOTIFY_API_ENDPOINT'] = 'http://test_notify_api_endpoint/' - def token_info(): # pylint: disable=unused-argument; mocks of library methods - return { - 'username': 'service account', - 'realm_access': { - 'roles': [ - 'system', - 'edit' - ] - } - } - - def mock_auth(): # pylint: disable=unused-argument; mocks of library methods - return 'test_token' - - monkeypatch.setattr('pay_api.utils.user_context._get_token', mock_auth) - monkeypatch.setattr('pay_api.utils.user_context._get_token_info', token_info) + monkeypatch.setattr('pay_api.services.email_service.get_service_account_token', lambda: 'test') with app.app_context(): with patch('pay_api.services.email_service.OAuthService.post') as mock_post: - mock_user = MagicMock() - mock_user.bearer_token = 'test_token' mock_post.return_value.text = json.dumps({'notifyStatus': 'SUCCESS'}) - result = send_email(['recipient@example.com'], 'Subject', 'Body', user=mock_user) - mock_post.assert_called_once_with( + result = send_email(['recipient@example.com'], 'Subject', 'Body') + mock_post.assert_called_with( 'http://test_notify_api_endpoint/notify/', - token='test_token', + token='test', auth_header_type=AuthHeaderType.BEARER, content_type=ContentType.JSON, data={ @@ -61,4 +44,4 @@ def mock_auth(): # pylint: disable=unused-argument; mocks of library methods } } ) - assert result is True + assert result is True diff --git a/pay-queue/tests/integration/test_eft_reconciliation.py b/pay-queue/tests/integration/test_eft_reconciliation.py index f9357b5f8..c3428e406 100644 --- a/pay-queue/tests/integration/test_eft_reconciliation.py +++ b/pay-queue/tests/integration/test_eft_reconciliation.py @@ -507,7 +507,7 @@ def create_test_data(): eft_shortname: EFTShortnameModel = (EFTShortnameModel(short_name='TESTSHORTNAME', type=EFTShortnameType.EFT.value).save()) EFTShortnameLinksModel( - eft_short_name_id=eft_shortname.id, + eft_short_name_id=eft_short_name.id, auth_account_id=payment_account.auth_account_id, status_code=EFTShortnameStatus.LINKED.value, updated_by='IDIR/JSMITH', @@ -644,7 +644,7 @@ def test_apply_pending_payments(session, app, client): add_file_event_to_queue_and_process(client, file_name=file_name, message_type=QueueMessageTypes.EFT_FILE_UPLOADED.value) - short_name_id = eft_shortname.id + short_name_id = eft_short_name.id eft_credit_balance = EFTCreditModel.get_eft_credit_balance(short_name_id) assert eft_credit_balance == 0 @@ -667,7 +667,7 @@ def test_skip_on_existing_pending_payments(session, app, client): message_type=QueueMessageTypes.EFT_FILE_UPLOADED.value) create_statement_from_invoices(payment_account, [invoice]) - eft_credits = EFTCreditModel.get_eft_credits(eft_shortname.id) + eft_credits = EFTCreditModel.get_eft_credits(eft_short_name.id) # Add an unexpected PENDING record to test that processing skips for this account EFTCreditInvoiceLinkModel( @@ -677,7 +677,7 @@ def test_skip_on_existing_pending_payments(session, app, client): amount=invoice.total, link_group_id=1) - short_name_id = eft_shortname.id + short_name_id = eft_short_name.id eft_credit_balance = EFTCreditModel.get_eft_credit_balance(short_name_id) # Assert credit balance is not spent due to an expected already PENDING state assert eft_credit_balance == 150.50 @@ -696,7 +696,7 @@ def test_skip_on_insufficient_balance(session, app, client): create_statement_from_invoices(payment_account, [invoice]) - short_name_id = eft_shortname.id + short_name_id = eft_short_name.id eft_credit_balance = EFTCreditModel.get_eft_credit_balance(short_name_id) assert eft_credit_balance == 150.50 From a9ecee54beccc8992e212a17ed10a71f18a96078 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Tue, 24 Sep 2024 16:59:16 -0700 Subject: [PATCH 13/16] Fix up pay-queue --- pay-queue/poetry.lock | 2 +- pay-queue/src/pay_queue/auth.py | 34 ---------------- .../services/eft/eft_reconciliation.py | 40 +++++++++---------- .../src/pay_queue/services/email_service.py | 6 +-- .../integration/test_eft_reconciliation.py | 14 +++---- 5 files changed, 31 insertions(+), 65 deletions(-) delete mode 100644 pay-queue/src/pay_queue/auth.py diff --git a/pay-queue/poetry.lock b/pay-queue/poetry.lock index c9ec6e384..66921e69d 100644 --- a/pay-queue/poetry.lock +++ b/pay-queue/poetry.lock @@ -1981,7 +1981,7 @@ werkzeug = "3.0.3" type = "git" url = "https://github.com/seeker25/sbc-pay.git" reference = "21537" -resolved_reference = "31090b710c00f879fa366c350f23adce7c39748b" +resolved_reference = "49ac0d07497d643200a4b7201e7b3ccf657ea512" subdirectory = "pay-api" [[package]] diff --git a/pay-queue/src/pay_queue/auth.py b/pay-queue/src/pay_queue/auth.py deleted file mode 100644 index 8cef1dde0..000000000 --- a/pay-queue/src/pay_queue/auth.py +++ /dev/null @@ -1,34 +0,0 @@ -# Copyright © 2024 Province of British Columbia -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -"""Jobs uses service accounts to retrieve the token.""" - -import requests -from flask import current_app -from requests.auth import HTTPBasicAuth - - -def get_token(): - """Get service account token.""" - issuer_url = current_app.config.get('JWT_OIDC_ISSUER') - - token_url = issuer_url + '/protocol/openid-connect/token' - - auth = HTTPBasicAuth(current_app.config.get('KEYCLOAK_SERVICE_ACCOUNT_ID'), - current_app.config.get('KEYCLOAK_SERVICE_ACCOUNT_SECRET')) - headers = {'content-type': 'application/x-www-form-urlencoded'} - body = 'grant_type=client_credentials' - - token_request = requests.post(token_url, headers=headers, data=body, auth=auth, timeout=500) - bearer_token = token_request.json()['access_token'] - return bearer_token diff --git a/pay-queue/src/pay_queue/services/eft/eft_reconciliation.py b/pay-queue/src/pay_queue/services/eft/eft_reconciliation.py index d9f6592be..1795f4fbe 100644 --- a/pay-queue/src/pay_queue/services/eft/eft_reconciliation.py +++ b/pay-queue/src/pay_queue/services/eft/eft_reconciliation.py @@ -187,16 +187,16 @@ def _apply_eft_pending_payments(context: EFTReconciliation, shortname_balance): """Apply payments to short name links.""" for shortname in shortname_balance.keys(): short_name_type = shortname_balance[shortname]['short_name_type'] - eft_shortname = _get_shortname(shortname, short_name_type) - eft_credit_balance = EFTCreditModel.get_eft_credit_balance(eft_shortname.id) - shortname_links = EFTShortnamesService.get_shortname_links(eft_shortname.id).get('items', []) + eft_short_name = _get_shortname(shortname, short_name_type) + eft_credit_balance = EFTCreditModel.get_eft_credit_balance(eft_short_name.id) + shortname_links = EFTShortnamesService.get_shortname_links(eft_short_name.id).get('items', []) for shortname_link in shortname_links: # We are expecting pending payments to have been cleared since this runs after the # eft task job. Something may have gone wrong, we will skip this link. if shortname_link.get('has_pending_payment'): error_msg = f'Unexpected pending payment on link: {shortname_link.id}' context.eft_error_handling('N/A', error_msg, - table_name=eft_shortname.__tablename__) + table_name=eft_short_name.__tablename__) continue amount_owing = shortname_link.get('amount_owing') @@ -205,7 +205,7 @@ def _apply_eft_pending_payments(context: EFTReconciliation, shortname_balance): try: payload = {'action': EFTPaymentActions.APPLY_CREDITS.value, 'accountId': auth_account_id} - EFTShortnamesService.process_payment_action(eft_shortname.id, payload) + EFTShortnamesService.process_payment_action(eft_short_name.id, payload) except Exception as exception: # NOQA # pylint: disable=broad-except # EFT Short name service handles commit and rollback when the action fails, we just need to make # sure we log the error here @@ -263,14 +263,14 @@ def _process_eft_credits(shortname_balance, eft_file_id): for shortname in shortname_balance.keys(): try: short_name_type = shortname_balance[shortname]['short_name_type'] - eft_shortname = _get_shortname(shortname, short_name_type) + eft_short_name = _get_shortname(shortname, short_name_type) eft_transactions = shortname_balance[shortname]['transactions'] for eft_transaction in eft_transactions: # Check if there is an existing eft credit for this file and transaction eft_credit_model = db.session.query(EFTCreditModel) \ .filter(EFTCreditModel.eft_file_id == eft_file_id) \ - .filter(EFTCreditModel.short_name_id == eft_shortname.id) \ + .filter(EFTCreditModel.short_name_id == eft_short_name.id) \ .filter(EFTCreditModel.eft_transaction_id == eft_transaction['id']) \ .one_or_none() @@ -283,7 +283,7 @@ def _process_eft_credits(shortname_balance, eft_file_id): continue eft_credit_model.eft_file_id = eft_file_id - eft_credit_model.short_name_id = eft_shortname.id + eft_credit_model.short_name_id = eft_short_name.id eft_credit_model.amount = deposit_amount eft_credit_model.remaining_amount = deposit_amount eft_credit_model.eft_transaction_id = eft_transaction['id'] @@ -412,18 +412,18 @@ def _update_transactions_to_complete(eft_file_model: EFTFileModel) -> int: def _get_shortname(short_name: str, short_name_type: str) -> EFTShortnameModel: """Save short name if it doesn't exist.""" - eft_shortname = db.session.query(EFTShortnameModel) \ + eft_short_name = db.session.query(EFTShortnameModel) \ .filter(EFTShortnameModel.short_name == short_name) \ .filter(EFTShortnameModel.type == short_name_type) \ .one_or_none() - if eft_shortname is None: - eft_shortname = EFTShortnameModel() - eft_shortname.short_name = short_name - eft_shortname.type = short_name_type - eft_shortname.save() + if eft_short_name is None: + eft_short_name = EFTShortnameModel() + eft_short_name.short_name = short_name + eft_short_name.type = short_name_type + eft_short_name.save() - return eft_shortname + return eft_short_name def _shortname_balance_as_dict(eft_transactions: List[EFTRecord]) -> Dict: @@ -435,15 +435,15 @@ def _shortname_balance_as_dict(eft_transactions: List[EFTRecord]) -> Dict: if eft_transaction.has_errors(): continue - shortname = eft_transaction.transaction_description + short_name = eft_transaction.transaction_description shortname_type = eft_transaction.short_name_type deposit_amount = eft_transaction.deposit_amount_cad / 100 transaction = {'id': eft_transaction.id, 'deposit_amount': deposit_amount} - shortname_balance.setdefault(shortname, {'balance': 0}) - shortname_balance[shortname]['short_name_type'] = shortname_type - shortname_balance[shortname]['balance'] += deposit_amount - shortname_balance[shortname].setdefault('transactions', []).append(transaction) + shortname_balance.setdefault(short_name, {'balance': 0}) + shortname_balance[short_name]['short_name_type'] = shortname_type + shortname_balance[short_name]['balance'] += deposit_amount + shortname_balance[short_name].setdefault('transactions', []).append(transaction) return shortname_balance diff --git a/pay-queue/src/pay_queue/services/email_service.py b/pay-queue/src/pay_queue/services/email_service.py index 486361140..ce3132da2 100644 --- a/pay-queue/src/pay_queue/services/email_service.py +++ b/pay-queue/src/pay_queue/services/email_service.py @@ -20,11 +20,10 @@ from flask import current_app from jinja2 import Environment, FileSystemLoader from pay_api.exceptions import ServiceUnavailableException +from pay_api.services.auth import get_service_account_token from pay_api.services.oauth_service import OAuthService from pay_api.utils.enums import AuthHeaderType, ContentType -from pay_queue.auth import get_token - @dataclasses.dataclass class EmailParams: @@ -67,7 +66,8 @@ def send_error_email(params: EmailParams): def send_email_service(recipients: list, subject: str, html_body: str): """Send the email notification.""" - token = get_token() + # Refactor this common code to PAY-API. + token = get_service_account_token() current_app.logger.info(f'>send_email to recipients: {recipients}') notify_url = current_app.config.get('NOTIFY_API_ENDPOINT') + 'notify/' diff --git a/pay-queue/tests/integration/test_eft_reconciliation.py b/pay-queue/tests/integration/test_eft_reconciliation.py index c3428e406..62d486044 100644 --- a/pay-queue/tests/integration/test_eft_reconciliation.py +++ b/pay-queue/tests/integration/test_eft_reconciliation.py @@ -503,9 +503,9 @@ def test_eft_tdi17_rerun(session, app, client): def create_test_data(): """Create test seed data.""" - payment_account: PaymentAccountModel = factory_create_eft_account() - eft_shortname: EFTShortnameModel = (EFTShortnameModel(short_name='TESTSHORTNAME', - type=EFTShortnameType.EFT.value).save()) + payment_account = factory_create_eft_account() + eft_short_name = (EFTShortnameModel(short_name='TESTSHORTNAME', + type=EFTShortnameType.EFT.value).save()) EFTShortnameLinksModel( eft_short_name_id=eft_short_name.id, auth_account_id=payment_account.auth_account_id, @@ -521,7 +521,7 @@ def create_test_data(): service_fees=1.50, payment_method_code=PaymentMethod.EFT.value) - return payment_account, eft_shortname, invoice + return payment_account, eft_short_name, invoice def generate_basic_tdi17_file(file_name: str): @@ -636,7 +636,7 @@ def create_statement_from_invoices(account: PaymentAccountModel, invoices: List[ def test_apply_pending_payments(session, app, client): """Test automatically applying a pending eft credit invoice link when there is a credit.""" - payment_account, eft_shortname, invoice = create_test_data() + payment_account, eft_short_name, invoice = create_test_data() create_statement_from_invoices(payment_account, [invoice]) file_name: str = 'test_eft_tdi17.txt' generate_tdi17_file(file_name) @@ -659,7 +659,7 @@ def test_apply_pending_payments(session, app, client): def test_skip_on_existing_pending_payments(session, app, client): """Test auto payment skipping payment when there exists a pending payment.""" - payment_account, eft_shortname, invoice = create_test_data() + payment_account, eft_short_name, invoice = create_test_data() file_name: str = 'test_eft_tdi17.txt' generate_tdi17_file(file_name) add_file_event_to_queue_and_process(client, @@ -685,7 +685,7 @@ def test_skip_on_existing_pending_payments(session, app, client): def test_skip_on_insufficient_balance(session, app, client): """Test auto payment skipping payment when there is an insufficient eft credit balance.""" - payment_account, eft_shortname, invoice = create_test_data() + payment_account, eft_short_name, invoice = create_test_data() invoice.total = 99999 invoice.save() file_name: str = 'test_eft_tdi17.txt' From a226bff57b7f6f70e8e90b80af3adecdcfb73ed0 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Fri, 27 Sep 2024 09:16:52 -0700 Subject: [PATCH 14/16] Small fixes from feedback --- pay-api/src/pay_api/services/eft_refund.py | 17 +++++++++-------- pay-api/src/pay_api/services/email_service.py | 3 +-- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/pay-api/src/pay_api/services/eft_refund.py b/pay-api/src/pay_api/services/eft_refund.py index 2470f1adb..9463bd030 100644 --- a/pay-api/src/pay_api/services/eft_refund.py +++ b/pay-api/src/pay_api/services/eft_refund.py @@ -14,7 +14,8 @@ from pay_api.services.auth import get_emails_with_keycloak_role from pay_api.services.email_service import ShortNameRefundEmailContent, send_email from pay_api.services.eft_short_name_historical import EFTShortnameHistorical as EFTHistoryService -from pay_api.utils.enums import EFTCreditInvoiceStatus, EFTShortnameRefundStatus, InvoiceStatus, Role +from pay_api.utils.enums import ( + EFTCreditInvoiceStatus, EFTHistoricalTypes, EFTShortnameRefundStatus, InvoiceStatus, Role) from pay_api.utils.user_context import user_context from pay_api.utils.util import get_str_by_path @@ -199,13 +200,10 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest match data.status: case EFTShortnameRefundStatus.DECLINED.value: EFTRefund.reverse_eft_credits(refund.short_name_id, refund.refund_amount) - EFTHistoryService.create_shortname_refund( - EFTHistoryModel(short_name_id=refund.short_name_id, - amount=-refund.refund_amount, - credit_balance=EFTCreditModel.get_eft_credit_balance(refund.short_name_id), - eft_refund_id=refund.id, - is_processing=False, - hidden=False)).save() + history = EFTHistoryModel.find_by_eft_refund_id(refund.id) + history.transaction_type = EFTHistoricalTypes.SN_REFUND_DECLINED.value + history.credit_balance = EFTCreditModel.get_eft_credit_balance(refund.short_name_id) + history.save() subject = f'Declined Refund Request for Short Name {short_name.short_name}' body = ShortNameRefundEmailContent( comment=refund.comment, @@ -219,6 +217,9 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest expense_authority_recipients = get_emails_with_keycloak_role(Role.EFT_REFUND_APPROVER.value) send_email(expense_authority_recipients, subject, body) case EFTShortnameRefundStatus.APPROVED.value: + history = EFTHistoryModel.find_by_eft_refund_id(refund.id) + history.transaction_type = EFTHistoricalTypes.SN_REFUND_APPROVED.value + history.save() subject = f'Approved Refund Request for Short Name {short_name.short_name}' content = ShortNameRefundEmailContent( comment=refund.comment, diff --git a/pay-api/src/pay_api/services/email_service.py b/pay-api/src/pay_api/services/email_service.py index 87b458ad8..ca0ba49f7 100644 --- a/pay-api/src/pay_api/services/email_service.py +++ b/pay-api/src/pay_api/services/email_service.py @@ -20,7 +20,6 @@ from attr import define from flask import current_app from jinja2 import Environment, FileSystemLoader -from pay_api.exceptions import ServiceUnavailableException from pay_api.services.auth import get_service_account_token from pay_api.services.oauth_service import OAuthService from pay_api.utils.enums import AuthHeaderType, ContentType @@ -53,7 +52,7 @@ def send_email(recipients: list, subject: str, body: str): if notify_response: current_app.logger.info(f'Successfully sent email to {recipient}') success = True - except ServiceUnavailableException as e: + except Exception as e: # NOQA pylint:disable=broad-except current_app.logger.error(f'Error sending email to {recipient}: {e}') return success From aaec435d35acdb06683937827f0b73e38251f897 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Fri, 27 Sep 2024 10:03:47 -0700 Subject: [PATCH 15/16] fix unit tests --- pay-api/src/pay_api/services/eft_refund.py | 4 ++-- .../tests/unit/api/test_eft_short_names.py | 11 +++++----- pay-api/tests/utilities/base_test.py | 22 ++++++++++++++----- 3 files changed, 25 insertions(+), 12 deletions(-) diff --git a/pay-api/src/pay_api/services/eft_refund.py b/pay-api/src/pay_api/services/eft_refund.py index 9463bd030..99510001d 100644 --- a/pay-api/src/pay_api/services/eft_refund.py +++ b/pay-api/src/pay_api/services/eft_refund.py @@ -200,7 +200,7 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest match data.status: case EFTShortnameRefundStatus.DECLINED.value: EFTRefund.reverse_eft_credits(refund.short_name_id, refund.refund_amount) - history = EFTHistoryModel.find_by_eft_refund_id(refund.id) + history = EFTHistoryModel.find_by_eft_refund_id(refund.id)[0] history.transaction_type = EFTHistoricalTypes.SN_REFUND_DECLINED.value history.credit_balance = EFTCreditModel.get_eft_credit_balance(refund.short_name_id) history.save() @@ -217,7 +217,7 @@ def update_shortname_refund(refund_id: int, data: EFTShortNameRefundPatchRequest expense_authority_recipients = get_emails_with_keycloak_role(Role.EFT_REFUND_APPROVER.value) send_email(expense_authority_recipients, subject, body) case EFTShortnameRefundStatus.APPROVED.value: - history = EFTHistoryModel.find_by_eft_refund_id(refund.id) + history = EFTHistoryModel.find_by_eft_refund_id(refund.id)[0] history.transaction_type = EFTHistoricalTypes.SN_REFUND_APPROVED.value history.save() subject = f'Approved Refund Request for Short Name {short_name.short_name}' diff --git a/pay-api/tests/unit/api/test_eft_short_names.py b/pay-api/tests/unit/api/test_eft_short_names.py index 22fdc861f..2105f026c 100755 --- a/pay-api/tests/unit/api/test_eft_short_names.py +++ b/pay-api/tests/unit/api/test_eft_short_names.py @@ -39,9 +39,9 @@ EFTShortnameStatus, EFTShortnameType, InvoiceStatus, PaymentMethod, Role, StatementFrequency) from pay_api.utils.errors import Error from tests.utilities.base_test import ( - factory_eft_credit, factory_eft_file, factory_eft_refund, factory_eft_shortname, factory_eft_shortname_link, - factory_invoice, factory_payment_account, factory_statement, factory_statement_invoices, factory_statement_settings, - get_claims, token_header) + factory_eft_credit, factory_eft_file, factory_eft_history, factory_eft_refund, factory_eft_shortname, + factory_eft_shortname_link, factory_invoice, factory_payment_account, factory_statement, factory_statement_invoices, + factory_statement_settings, get_claims, token_header) def test_create_eft_short_name_link(session, client, jwt, app): @@ -1004,6 +1004,7 @@ def test_patch_shortname_refund(session, client, jwt, payload, test_name, role, eft_file = factory_eft_file().save() eft_credit = EFTCreditModel(eft_file_id=eft_file.id, short_name_id=short_name.id, amount=100, remaining_amount=90).save() + eft_history = factory_eft_history(short_name.id, refund.id, 10, 10) if test_name == 'bad_transition': refund.status = EFTShortnameRefundStatus.APPROVED.value refund.save() @@ -1020,15 +1021,15 @@ def test_patch_shortname_refund(session, client, jwt, payload, test_name, role, assert rv.status_code == 200 assert rv.json['status'] == EFTShortnameRefundStatus.APPROVED.value assert rv.json['comment'] == 'Test comment' + assert eft_history.transaction_type == EFTHistoricalTypes.SN_REFUND_APPROVED.value case 'valid_rejected_refund': assert rv.status_code == 200 assert rv.json['status'] == EFTShortnameRefundStatus.DECLINED.value assert rv.json['comment'] == 'Test comment' - # assert EFTHistoryModel history = EFTShortnamesHistoryModel.find_by_eft_refund_id(refund.id)[0] assert history - assert history.amount == -10 assert history.credit_balance == 90 + 10 + assert history.transaction_type == EFTHistoricalTypes.SN_REFUND_DECLINED.value eft_credit = EFTCreditModel.find_by_id(eft_credit.id) assert eft_credit.remaining_amount == 90 + 10 assert rv.json['declineReason'] diff --git a/pay-api/tests/utilities/base_test.py b/pay-api/tests/utilities/base_test.py index d584521cf..8efa0a40a 100644 --- a/pay-api/tests/utilities/base_test.py +++ b/pay-api/tests/utilities/base_test.py @@ -26,13 +26,13 @@ from pay_api.models import ( CfsAccount, Comment, DistributionCode, DistributionCodeLink, EFTCredit, EFTCreditInvoiceLink, EFTFile, EFTRefund, - EFTShortnameLinks, EFTShortnames, Invoice, InvoiceReference, NonSufficientFunds, Payment, PaymentAccount, - PaymentLineItem, PaymentTransaction, Receipt, RoutingSlip, Statement, StatementInvoices, StatementSettings) + EFTShortnameLinks, EFTShortnames, EFTShortnamesHistorical, Invoice, InvoiceReference, NonSufficientFunds, Payment, + PaymentAccount, PaymentLineItem, PaymentTransaction, Receipt, RoutingSlip, Statement, StatementInvoices, + StatementSettings) from pay_api.utils.constants import DT_SHORT_FORMAT from pay_api.utils.enums import ( - CfsAccountStatus, EFTShortnameStatus, EFTShortnameType, InvoiceReferenceStatus, InvoiceStatus, LineItemStatus, - PaymentMethod, PaymentStatus, PaymentSystem, Role, RoutingSlipStatus) - + CfsAccountStatus, EFTHistoricalTypes, EFTShortnameStatus, EFTShortnameType, InvoiceReferenceStatus, InvoiceStatus, + LineItemStatus, PaymentMethod, PaymentStatus, PaymentSystem, Role, RoutingSlipStatus) token_header = { 'alg': 'RS256', @@ -931,6 +931,18 @@ def factory_eft_credit(eft_file_id, short_name_id, amount=10.00, remaining_amoun ).save() +def factory_eft_history(short_name_id, refund_id, amount=10.00, credit_balance=10.00): + """Return an EFT Shortnames Historical.""" + return EFTShortnamesHistorical(short_name_id=short_name_id, + amount=amount, + credit_balance=credit_balance, + eft_refund_id=refund_id, + is_processing=False, + hidden=False, + transaction_date=datetime.now(tz=timezone.utc), + transaction_type=EFTHistoricalTypes.SN_REFUND_PENDING_APPROVAL.value).save() + + def factory_eft_refund(short_name_id, refund_amount=10.00, cas_supplier_number='1234567', refund_email='test@test.com', comment='test comment', status='PENDING'): """Return an EFT Refund.""" From 1db4e0b6525ed257a56be29c37056ed368446af2 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Fri, 27 Sep 2024 10:08:43 -0700 Subject: [PATCH 16/16] update pay-queue --- pay-queue/poetry.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pay-queue/poetry.lock b/pay-queue/poetry.lock index 66921e69d..ab49c51dd 100644 --- a/pay-queue/poetry.lock +++ b/pay-queue/poetry.lock @@ -1981,7 +1981,7 @@ werkzeug = "3.0.3" type = "git" url = "https://github.com/seeker25/sbc-pay.git" reference = "21537" -resolved_reference = "49ac0d07497d643200a4b7201e7b3ccf657ea512" +resolved_reference = "aaec435d35acdb06683937827f0b73e38251f897" subdirectory = "pay-api" [[package]]