From eb7962edd4b3075fc6e5bd0302d82efeb76a1b7f Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Tue, 31 May 2022 09:41:26 -0700 Subject: [PATCH] 12033 - FAS - RS not moving to Completed status when balance is drained to 0 (#943) * Dependency update + migration. * Use quantized for refund amount. Update test for it. * Validate linking. * Unit test changes. * Lint fix. * Remove comment. * Check for routing slip status == active. * Reverse unit test case. * Remove unnecessary code. * Code trim. --- .../7aff4af4be85_rs_complete_changes.py | 25 ++++++++ pay-api/requirements.txt | 14 ++--- pay-api/requirements/prod.txt | 3 +- .../pay_api/services/internal_pay_service.py | 8 ++- pay-api/src/pay_api/services/refund.py | 4 +- pay-api/tests/unit/api/fas/test_refund.py | 2 +- .../tests/unit/api/fas/test_routing_slip.py | 61 ++++++++++++++++++- .../unit/services/test_payment_service.py | 42 ++++++++++++- 8 files changed, 144 insertions(+), 15 deletions(-) create mode 100644 pay-api/migrations/versions/7aff4af4be85_rs_complete_changes.py diff --git a/pay-api/migrations/versions/7aff4af4be85_rs_complete_changes.py b/pay-api/migrations/versions/7aff4af4be85_rs_complete_changes.py new file mode 100644 index 000000000..abd58e355 --- /dev/null +++ b/pay-api/migrations/versions/7aff4af4be85_rs_complete_changes.py @@ -0,0 +1,25 @@ +"""Migration for Routing slip status COMPLETE. + +Revision ID: 7aff4af4be85 +Revises: 6a6b042b831a +Create Date: 2022-05-30 08:23:00.535893 + +""" +from alembic import op +from sqlalchemy import String +from sqlalchemy.sql import column, table + + +# revision identifiers, used by Alembic. +revision = '7aff4af4be85' +down_revision = '6a6b042b831a' +branch_labels = None +depends_on = None + +def upgrade(): + op.execute("UPDATE routing_slips set status = 'COMPLETE' where remaining_amount < 0.01 and status = 'ACTIVE'") + + +def downgrade(): + op.execute("UPDATE routing_slips set status = 'ACTIVE' where remaining_amount < 0.01 and status = 'COMPLETE'") + diff --git a/pay-api/requirements.txt b/pay-api/requirements.txt index cbb0cd0dd..55a3815a4 100644 --- a/pay-api/requirements.txt +++ b/pay-api/requirements.txt @@ -1,4 +1,4 @@ -Flask-Caching==1.10.1 +Flask-Caching==1.11.1 Flask-Migrate==2.7.0 Flask-Moment==1.0.2 Flask-SQLAlchemy==2.5.1 @@ -17,12 +17,12 @@ asyncio-nats-client==0.11.5 asyncio-nats-streaming==0.4.0 attrs==21.4.0 blinker==1.4 -cachelib==0.6.0 -certifi==2021.10.8 +cachelib==0.7.0 +certifi==2022.5.18.1 cffi==1.15.0 charset-normalizer==2.0.12 click==8.1.3 -croniter==1.3.4 +croniter==1.3.5 cryptography==37.0.2 dpath==2.0.6 ecdsa==0.17.0 @@ -31,16 +31,16 @@ flask-marshmallow==0.11.0 flask-restx==0.5.1 gunicorn==20.1.0 idna==3.3 -importlib-metadata==4.11.3 +importlib-metadata==4.11.4 importlib-resources==5.7.1 itsdangerous==2.0.1 jaeger-client==4.8.0 jsonschema==4.5.1 marshmallow-sqlalchemy==0.25.0 -marshmallow==3.15.0 +marshmallow==3.16.0 opentracing==2.4.0 packaging==21.3 -protobuf==3.20.1 +protobuf==3.19.4 psycopg2-binary==2.9.3 pyasn1==0.4.8 pycparser==2.21 diff --git a/pay-api/requirements/prod.txt b/pay-api/requirements/prod.txt index 1a17d7bde..be3ed2c40 100755 --- a/pay-api/requirements/prod.txt +++ b/pay-api/requirements/prod.txt @@ -26,4 +26,5 @@ cryptography sqlalchemy_utils sqlalchemy<1.4 itsdangerous==2.0.1 -Jinja2==3.0.3 \ No newline at end of file +Jinja2==3.0.3 +protobuf~=3.19.0 diff --git a/pay-api/src/pay_api/services/internal_pay_service.py b/pay-api/src/pay_api/services/internal_pay_service.py index c12c6fc18..a9b45044a 100644 --- a/pay-api/src/pay_api/services/internal_pay_service.py +++ b/pay-api/src/pay_api/services/internal_pay_service.py @@ -15,7 +15,6 @@ There are conditions where the payment will be handled internally. For e.g, zero $ or staff payments. """ -import decimal from datetime import datetime from http import HTTPStatus @@ -59,6 +58,8 @@ def create_invoice(self, payment_account: PaymentAccount, line_items: [PaymentLi current_app.logger.info(f'FAS Routing slip found with remaining amount : {routing_slip.remaining_amount}') routing_slip.remaining_amount = routing_slip.remaining_amount - \ get_quantized(invoice.total) + if routing_slip.status == RoutingSlipStatus.ACTIVE.value and routing_slip.remaining_amount < 0.01: + routing_slip.status = RoutingSlipStatus.COMPLETE.value routing_slip.flush() else: invoice_reference = InvoiceReference.create(invoice.id, @@ -111,6 +112,9 @@ def process_cfs_refund(self, invoice: InvoiceModel): payment.payment_status_code = PaymentStatus.REFUNDED.value payment.flush() routing_slip.remaining_amount += get_quantized(invoice.total) + # Move routing slip back to active on refund. + if routing_slip.status == RoutingSlipStatus.COMPLETE.value: + routing_slip.status = RoutingSlipStatus.ACTIVE.value routing_slip.flush() invoice.invoice_status_code = InvoiceStatus.REFUND_REQUESTED.value invoice.flush() @@ -137,7 +141,7 @@ def _validate_routing_slip(routing_slip: RoutingSlipModel, invoice: Invoice): if routing_slip.parent: detail = f'This Routing slip is linked, enter the parent Routing slip: {routing_slip.parent.number}' raise BusinessException(InternalPayService._create_error_object('LINKED_ROUTING_SLIP', detail)) - if routing_slip.remaining_amount < decimal.Decimal(invoice.total).quantize(decimal.Decimal('1.00')): + if routing_slip.remaining_amount < get_quantized(invoice.total): detail = f'There is not enough balance in this Routing slip. ' \ f'The current balance is: ${routing_slip.remaining_amount:.2f}' diff --git a/pay-api/src/pay_api/services/refund.py b/pay-api/src/pay_api/services/refund.py index bf57b6d51..9627cd943 100644 --- a/pay-api/src/pay_api/services/refund.py +++ b/pay-api/src/pay_api/services/refund.py @@ -30,7 +30,7 @@ from pay_api.utils.enums import InvoiceStatus, Role, RoutingSlipStatus from pay_api.utils.errors import Error from pay_api.utils.user_context import UserContext, user_context -from pay_api.utils.util import get_str_by_path +from pay_api.utils.util import get_quantized, get_str_by_path class RefundService: # pylint: disable=too-many-instance-attributes @@ -196,7 +196,7 @@ def create_routing_slip_refund(cls, routing_slip_number: str, request: Dict[str, if (refund_status := get_str_by_path(request, 'status')) is None: raise BusinessException(Error.INVALID_REQUEST) user_name = kwargs['user'].user_name - if rs_model.remaining_amount == 0: + if get_quantized(rs_model.remaining_amount) == 0: raise BusinessException(Error.INVALID_REQUEST) # refund not possible for zero amount routing slips is_refund_finalized = refund_status in (RoutingSlipStatus.REFUND_AUTHORIZED.value, diff --git a/pay-api/tests/unit/api/fas/test_refund.py b/pay-api/tests/unit/api/fas/test_refund.py index eece4d296..8a9b26e66 100644 --- a/pay-api/tests/unit/api/fas/test_refund.py +++ b/pay-api/tests/unit/api/fas/test_refund.py @@ -120,7 +120,7 @@ def test_refund_routing_slips_reject(client, jwt): def test_refund_routing_slips_zero_dollar_error(client, jwt): """Assert zero dollar refund fails.""" - payload = get_routing_slip_request(cheque_receipt_numbers=[('1234567890', PaymentMethod.CHEQUE.value, 0)]) + payload = get_routing_slip_request(cheque_receipt_numbers=[('1234567890', PaymentMethod.CHEQUE.value, 0.005)]) token = jwt.create_jwt(get_claims(roles=[Role.FAS_CREATE.value, Role.FAS_VIEW.value, Role.FAS_REFUND.value]), token_header) headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} diff --git a/pay-api/tests/unit/api/fas/test_routing_slip.py b/pay-api/tests/unit/api/fas/test_routing_slip.py index 85684a304..92619e20a 100755 --- a/pay-api/tests/unit/api/fas/test_routing_slip.py +++ b/pay-api/tests/unit/api/fas/test_routing_slip.py @@ -25,7 +25,7 @@ from flask import current_app from faker import Faker -from pay_api.models import PaymentAccount +from pay_api.models import PaymentAccount, RoutingSlip from pay_api.schemas import utils as schema_utils from pay_api.utils.constants import DT_SHORT_FORMAT from pay_api.services.fas.routing_slip_status_transition_service import RoutingSlipStatusTransitionService @@ -770,3 +770,62 @@ def test_create_routing_slip_null_cheque_date(client, jwt, app): rv = client.post('/api/v1/fas/routing-slips', data=json.dumps(routing_slip_payload), headers=headers) assert rv.status_code == 400 + + +def test_routing_slip_link_attempt(client, jwt, app): + """12033 - Scenario 3. + + Routing slip is Completed, attempt to be linked. + Linking shouldn't be allowed and explaining that completed routing + slip cannot be involved in linking. + """ + token = jwt.create_jwt(get_claims(roles=[Role.FAS_CREATE.value, Role.FAS_LINK.value, Role.FAS_SEARCH.value]), + token_header) + headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} + child = get_routing_slip_request('438607657') + parent1 = get_routing_slip_request('355336710') + client.post('/api/v1/fas/routing-slips', data=json.dumps(child), headers=headers) + client.post('/api/v1/fas/routing-slips', data=json.dumps(parent1), headers=headers) + + rs_model = RoutingSlip.find_by_number('438607657') + rs_model.status = RoutingSlipStatus.COMPLETE.value + rs_model.commit() + + data = {'childRoutingSlipNumber': f"{child.get('number')}", 'parentRoutingSlipNumber': f"{parent1.get('number')}"} + rv = client.post('/api/v1/fas/routing-slips/links', data=json.dumps(data), headers=headers) + assert rv.json.get('type') == 'RS_IN_INVALID_STATUS' + assert rv.status_code == 400 + + # Try the reverse: + data = {'childRoutingSlipNumber': f"{parent1.get('number')}", 'parentRoutingSlipNumber': f"{child.get('number')}"} + rv = client.post('/api/v1/fas/routing-slips/links', data=json.dumps(data), headers=headers) + assert rv.json.get('type') == 'RS_IN_INVALID_STATUS' + assert rv.status_code == 400 + + +def test_routing_slip_status_to_nsf_attempt(client, jwt, app): + """12033 - Scenario 4. + + Routing slip in Completed, + user attempts to move it into another status, can only set to NSF. + """ + token = jwt.create_jwt(get_claims(roles=[Role.FAS_CREATE.value, Role.FAS_LINK.value, + Role.FAS_SEARCH.value, Role.FAS_EDIT.value]), + token_header) + headers = {'Authorization': f'Bearer {token}', 'content-type': 'application/json'} + child = get_routing_slip_request('438607657') + client.post('/api/v1/fas/routing-slips', data=json.dumps(child), headers=headers) + + rs_model = RoutingSlip.find_by_number('438607657') + rs_model.status = RoutingSlipStatus.COMPLETE.value + rs_model.commit() + + # Active shouldn't work. + rv = client.patch(f"/api/v1/fas/routing-slips/{child.get('number')}?action={PatchActions.UPDATE_STATUS.value}", + data=json.dumps({'status': RoutingSlipStatus.ACTIVE.value}), headers=headers) + assert rv.status_code == 400 + + # NSF should work. + rv = client.patch(f"/api/v1/fas/routing-slips/{child.get('number')}?action={PatchActions.UPDATE_STATUS.value}", + data=json.dumps({'status': RoutingSlipStatus.NSF.value}), headers=headers) + assert rv.status_code == 200, 'status changed successfully.' diff --git a/pay-api/tests/unit/services/test_payment_service.py b/pay-api/tests/unit/services/test_payment_service.py index 260c1c389..2c8564dbf 100644 --- a/pay-api/tests/unit/services/test_payment_service.py +++ b/pay-api/tests/unit/services/test_payment_service.py @@ -23,8 +23,9 @@ from pay_api.models import FeeSchedule, Invoice, Payment, PaymentAccount from pay_api.models import RoutingSlip as RoutingSlipModel from pay_api.services import CFSService +from pay_api.services.internal_pay_service import InternalPayService from pay_api.services.payment_service import PaymentService -from pay_api.utils.enums import InvoiceStatus, PaymentMethod, PaymentStatus +from pay_api.utils.enums import InvoiceStatus, PaymentMethod, PaymentStatus, RoutingSlipStatus from requests import Response from requests.exceptions import ConnectionError, ConnectTimeout, HTTPError from tests.utilities.base_test import ( @@ -88,6 +89,12 @@ def test_create_payment_record_with_internal_pay(session, public_user_mock): rs = RoutingSlipModel.find_by_number(rs_number) assert rs.remaining_amount == 0.0 + """12033 - Scenario 1. + + Manual transaction reduces RS to 0.00 + Routing slip status becomes Completed + """ + assert rs.status == RoutingSlipStatus.COMPLETE.name def test_create_payment_record_rollback(session, public_user_mock): @@ -337,3 +344,36 @@ def test_create_wire_payment(session, public_user_mock): assert payment_response is not None assert payment_response.get('payment_method') == PaymentMethod.WIRE.value assert payment_response.get('status_code') == 'CREATED' + + +def test_internal_rs_back_active(session, public_user_mock): + """12033 - Scenario 2. + + Routing slip is complete and a transaction is cancelled + the balance is restored - Should move back to Active + """ + payment_response = PaymentService.create_invoice( + get_routing_slip_payment_request(), get_auth_staff()) + account_model = PaymentAccount.find_by_auth_account_id(get_auth_staff().get('account').get('id')) + account_id = account_model.id + assert account_id is not None + assert payment_response.get('id') is not None + + rs_number = '123456789' + rs = factory_routing_slip(number=rs_number, payment_account_id=account_id, remaining_amount=50.00) + rs.save() + + # Create another invoice with a routing slip. + invoice = PaymentService.create_invoice(get_routing_slip_payment_request(), get_auth_staff()) + account_model = PaymentAccount.find_by_auth_account_id(get_auth_staff().get('account').get('id')) + + assert account_id == account_model.id + + rs = RoutingSlipModel.find_by_number(rs_number) + assert rs.remaining_amount == 0.0 + assert rs.status == RoutingSlipStatus.COMPLETE.name + + invoice = Invoice.find_by_id(invoice['id']) + InternalPayService().process_cfs_refund(invoice) + + assert rs.status == RoutingSlipStatus.ACTIVE.name