From f6265eac2185d8714d61ff31bde83ede1a029499 Mon Sep 17 00:00:00 2001 From: Travis Semple Date: Tue, 7 Jan 2025 12:41:28 -0800 Subject: [PATCH] =?UTF-8?q?Remove=20unused=20code,=20add=20in=20check=20fo?= =?UTF-8?q?r=20system=20-=20service=20accounts=20have=20a=E2=80=A6=20(#187?= =?UTF-8?q?2)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- pay-api/poetry.lock | 2 +- .../src/pay_api/services/payment_service.py | 6 +- pay-api/src/pay_api/services/refund.py | 3 - pay-api/src/pay_api/utils/user_context.py | 1 + pay-api/tests/unit/api/test_partial_refund.py | 104 +++++++++--------- pay-api/tests/unit/models/test_corp_type.py | 7 +- pay-api/tests/unit/services/test_code.py | 8 +- .../unit/services/test_payment_service.py | 15 --- 8 files changed, 59 insertions(+), 87 deletions(-) diff --git a/pay-api/poetry.lock b/pay-api/poetry.lock index fd2fd7137..1b433caba 100644 --- a/pay-api/poetry.lock +++ b/pay-api/poetry.lock @@ -920,7 +920,7 @@ six = "^1.16.0" type = "git" url = "https://github.com/seeker25/flask-jwt-oidc.git" reference = "HEAD" -resolved_reference = "d208d4643e3b17358f7295bee0f955e67ba6ac88" +resolved_reference = "563f01ef6453eb0ea1cc0a2d71c6665350c853ff" [[package]] name = "flask-marshmallow" diff --git a/pay-api/src/pay_api/services/payment_service.py b/pay-api/src/pay_api/services/payment_service.py index 7383e1d2f..ba397e0a4 100644 --- a/pay-api/src/pay_api/services/payment_service.py +++ b/pay-api/src/pay_api/services/payment_service.py @@ -31,7 +31,6 @@ from .base_payment_system import PaymentSystemService from .fee_schedule import FeeSchedule -from .flags import flags from .invoice import Invoice from .invoice_reference import InvoiceReference from .payment import Payment @@ -74,11 +73,8 @@ def create_invoice( payment_account = cls._find_payment_account(authorization) payment_method = _get_payment_method(payment_request, payment_account) - if payment_method == PaymentMethod.EFT.value and not flags.is_on("enable-eft-payment-method", default=False): - raise BusinessException(Error.INVALID_PAYMENT_METHOD) - user: UserContext = kwargs["user"] - if user.is_api_user() and not user.is_sandbox(): + if user.is_api_user() and (not user.is_sandbox() and not user.is_system()): if payment_method in [PaymentMethod.DIRECT_PAY.value, PaymentMethod.ONLINE_BANKING.value]: raise BusinessException(Error.INVALID_PAYMENT_METHOD) diff --git a/pay-api/src/pay_api/services/refund.py b/pay-api/src/pay_api/services/refund.py index fa003f202..53b1aba6f 100644 --- a/pay-api/src/pay_api/services/refund.py +++ b/pay-api/src/pay_api/services/refund.py @@ -30,7 +30,6 @@ from pay_api.models import RoutingSlip as RoutingSlipModel from pay_api.models import db from pay_api.services.base_payment_system import PaymentSystemService -from pay_api.services.flags import flags from pay_api.services.payment_account import PaymentAccount from pay_api.utils.constants import REFUND_SUCCESS_MESSAGES from pay_api.utils.converter import Converter @@ -304,8 +303,6 @@ def _validate_partial_refund_lines(refund_revenue: List[RefundPartialLine], invo @classmethod def _validate_allow_partial_refund(cls, refund_revenue, invoice: InvoiceModel): if refund_revenue: - if not flags.is_on("enable-partial-refunds", default=False): - raise BusinessException(Error.INVALID_REQUEST) if invoice.corp_type.has_partner_disbursements: raise BusinessException(Error.PARTIAL_REFUND_DISBURSEMENTS_UNSUPPORTED) diff --git a/pay-api/src/pay_api/utils/user_context.py b/pay-api/src/pay_api/utils/user_context.py index a0987efa1..fbab709fe 100644 --- a/pay-api/src/pay_api/utils/user_context.py +++ b/pay-api/src/pay_api/utils/user_context.py @@ -120,6 +120,7 @@ def is_sandbox(self) -> bool: def is_api_user(self) -> bool: """Return True if the user is an api_user.""" + # Note it's possible some of our service accounts could fall under API users, so check for system as well. return Role.API_USER.value in self._roles if self._roles else False @property diff --git a/pay-api/tests/unit/api/test_partial_refund.py b/pay-api/tests/unit/api/test_partial_refund.py index c01570d13..86fef228d 100644 --- a/pay-api/tests/unit/api/test_partial_refund.py +++ b/pay-api/tests/unit/api/test_partial_refund.py @@ -101,30 +101,29 @@ def test_create_refund(session, client, jwt, app, monkeypatch): assert payload["refundRevenue"][0]["lineNumber"] == "1" assert payload["refundRevenue"][0]["refundAmount"] == refund_partial[0].refund_amount - with patch("pay_api.services.payment_service.flags.is_on", return_value=True): - rv = client.post( - f"/api/v1/payment-requests/{inv_id}/refunds", - data=json.dumps({"reason": "Test", "refundRevenue": refund_revenue}), - headers=headers, - ) - assert rv.status_code == 202 - assert rv.json.get("message") == REFUND_SUCCESS_MESSAGES["DIRECT_PAY.PAID"] - assert RefundModel.find_by_invoice_id(inv_id) is not None - - refunds_partial: List[RefundPartialModel] = RefundService.get_refund_partials_by_invoice_id(inv_id) - assert refunds_partial - assert len(refunds_partial) == 1 - - refund = refunds_partial[0] - assert refund.id is not None - assert refund.payment_line_item_id == payment_line_items[0].id - assert refund.refund_amount == refund_amount - assert refund.refund_type == RefundsPartialType.BASE_FEES.value - - invoice = InvoiceModel.find_by_id(invoice.id) - assert invoice.invoice_status_code == InvoiceStatus.PAID.value - assert invoice.refund_date.date() == datetime.now(tz=timezone.utc).date() - assert invoice.refund == refund_amount + rv = client.post( + f"/api/v1/payment-requests/{inv_id}/refunds", + data=json.dumps({"reason": "Test", "refundRevenue": refund_revenue}), + headers=headers, + ) + assert rv.status_code == 202 + assert rv.json.get("message") == REFUND_SUCCESS_MESSAGES["DIRECT_PAY.PAID"] + assert RefundModel.find_by_invoice_id(inv_id) is not None + + refunds_partial: List[RefundPartialModel] = RefundService.get_refund_partials_by_invoice_id(inv_id) + assert refunds_partial + assert len(refunds_partial) == 1 + + refund = refunds_partial[0] + assert refund.id is not None + assert refund.payment_line_item_id == payment_line_items[0].id + assert refund.refund_amount == refund_amount + assert refund.refund_type == RefundsPartialType.BASE_FEES.value + + invoice = InvoiceModel.find_by_id(invoice.id) + assert invoice.invoice_status_code == InvoiceStatus.PAID.value + assert invoice.refund_date.date() == datetime.now(tz=timezone.utc).date() + assert invoice.refund == refund_amount def test_create_refund_fails(session, client, jwt, app, monkeypatch): @@ -172,19 +171,18 @@ def test_create_refund_fails(session, client, jwt, app, monkeypatch): token = jwt.create_jwt(get_claims(app_request=app, role=Role.SYSTEM.value), token_header) headers = {"Authorization": f"Bearer {token}", "content-type": "application/json"} - with patch("pay_api.services.payment_service.flags.is_on", return_value=True): - rv = client.post( - f"/api/v1/payment-requests/{inv_id}/refunds", - data=json.dumps({"reason": "Test", "refundRevenue": refund_revenue}), - headers=headers, - ) - assert rv.status_code == 400 - assert rv.json.get("type") == Error.INVALID_REQUEST.name - assert RefundModel.find_by_invoice_id(inv_id) is None + rv = client.post( + f"/api/v1/payment-requests/{inv_id}/refunds", + data=json.dumps({"reason": "Test", "refundRevenue": refund_revenue}), + headers=headers, + ) + assert rv.status_code == 400 + assert rv.json.get("type") == Error.INVALID_REQUEST.name + assert RefundModel.find_by_invoice_id(inv_id) is None - refunds_partial: List[RefundPartialModel] = RefundService.get_refund_partials_by_invoice_id(inv_id) - assert not refunds_partial - assert len(refunds_partial) == 0 + refunds_partial: List[RefundPartialModel] = RefundService.get_refund_partials_by_invoice_id(inv_id) + assert not refunds_partial + assert len(refunds_partial) == 0 def test_refund_validation_for_disbursements(session, client, jwt, app, monkeypatch): @@ -215,15 +213,14 @@ def test_refund_validation_for_disbursements(session, client, jwt, app, monkeypa } ] - with patch("pay_api.services.payment_service.flags.is_on", return_value=True): - rv = client.post( - f"/api/v1/payment-requests/{inv_id}/refunds", - data=json.dumps({"reason": "Test", "refundRevenue": refund_revenue}), - headers=headers, - ) - assert rv.status_code == 400 - assert rv.json.get("type") == Error.PARTIAL_REFUND_DISBURSEMENTS_UNSUPPORTED.name - assert RefundModel.find_by_invoice_id(inv_id) is None + rv = client.post( + f"/api/v1/payment-requests/{inv_id}/refunds", + data=json.dumps({"reason": "Test", "refundRevenue": refund_revenue}), + headers=headers, + ) + assert rv.status_code == 400 + assert rv.json.get("type") == Error.PARTIAL_REFUND_DISBURSEMENTS_UNSUPPORTED.name + assert RefundModel.find_by_invoice_id(inv_id) is None @pytest.mark.parametrize( @@ -295,15 +292,14 @@ def test_create_refund_validation(session, client, jwt, app, monkeypatch, fee_ty mock_get.return_value.status_code = 200 mock_get.return_value.json.return_value = base_paybc_response - with patch("pay_api.services.payment_service.flags.is_on", return_value=True): - rv = client.post( - f"/api/v1/payment-requests/{inv_id}/refunds", - data=json.dumps({"reason": "Test", "refundRevenue": refund_revenue}), - headers=headers, - ) - assert rv.status_code == 400 - assert rv.json.get("type") == Error.REFUND_AMOUNT_INVALID.name - assert RefundModel.find_by_invoice_id(inv_id) is None + rv = client.post( + f"/api/v1/payment-requests/{inv_id}/refunds", + data=json.dumps({"reason": "Test", "refundRevenue": refund_revenue}), + headers=headers, + ) + assert rv.status_code == 400 + assert rv.json.get("type") == Error.REFUND_AMOUNT_INVALID.name + assert RefundModel.find_by_invoice_id(inv_id) is None def _get_base_paybc_response(): diff --git a/pay-api/tests/unit/models/test_corp_type.py b/pay-api/tests/unit/models/test_corp_type.py index c872751ba..41651997f 100644 --- a/pay-api/tests/unit/models/test_corp_type.py +++ b/pay-api/tests/unit/models/test_corp_type.py @@ -64,14 +64,11 @@ def test_corp_type_by_invalid_code(session): def test_payment_methods(session): """Assert that payment methods are stored and retrieved correctly.""" business_corp = factory_corp_type( - "XX", - "Business", - product="BUSINESS", - payment_methods=['PAD', 'DIRECT_PAY', 'ONLINE_BANKING', 'DRAWDOWN'] + "XX", "Business", product="BUSINESS", payment_methods=["PAD", "DIRECT_PAY", "ONLINE_BANKING", "DRAWDOWN"] ) session.add(business_corp) session.commit() retrieved_corp = CorpType.find_by_code("XX") assert retrieved_corp is not None - assert retrieved_corp.payment_methods == ['PAD', 'DIRECT_PAY', 'ONLINE_BANKING', 'DRAWDOWN'] + assert retrieved_corp.payment_methods == ["PAD", "DIRECT_PAY", "ONLINE_BANKING", "DRAWDOWN"] diff --git a/pay-api/tests/unit/services/test_code.py b/pay-api/tests/unit/services/test_code.py index 4f198cf17..dcfb23777 100644 --- a/pay-api/tests/unit/services/test_code.py +++ b/pay-api/tests/unit/services/test_code.py @@ -66,10 +66,10 @@ def test_find_valid_payment_methods_by_product_code(session): assert payment_methods is not None assert isinstance(payment_methods, dict) - business_payment_methods = CodeService.find_valid_payment_methods_by_product_code('BUSINESS') + business_payment_methods = CodeService.find_valid_payment_methods_by_product_code("BUSINESS") assert business_payment_methods is not None - assert 'BUSINESS' in business_payment_methods - assert isinstance(business_payment_methods['BUSINESS'], list) + assert "BUSINESS" in business_payment_methods + assert isinstance(business_payment_methods["BUSINESS"], list) - invalid_payment_methods = CodeService.find_valid_payment_methods_by_product_code('INVALID') + invalid_payment_methods = CodeService.find_valid_payment_methods_by_product_code("INVALID") assert invalid_payment_methods == {} diff --git a/pay-api/tests/unit/services/test_payment_service.py b/pay-api/tests/unit/services/test_payment_service.py index 5689eb0fa..f06faa1d0 100644 --- a/pay-api/tests/unit/services/test_payment_service.py +++ b/pay-api/tests/unit/services/test_payment_service.py @@ -352,21 +352,6 @@ def test_create_eft_payment(session, public_user_mock): assert payment_response.get("status_code") == InvoiceStatus.APPROVED.value -def test_create_eft_payment_ff_disabled(session, public_user_mock): - """Assert that the payment method EFT feature flag properly disables record creation.""" - factory_payment_account(payment_method_code=PaymentMethod.EFT.value).save() - - with patch("pay_api.services.payment_service.flags.is_on", return_value=False): - with pytest.raises(BusinessException) as exception: - PaymentService.create_invoice( - get_payment_request_with_service_fees(business_identifier="CP0002000"), - get_auth_premium_user(), - ) - - assert exception is not None - assert exception.value.code == Error.INVALID_PAYMENT_METHOD.name - - def test_internal_rs_back_active(session, public_user_mock): """12033 - Scenario 2.