Remove unused code, add in check for system - service accounts have a… (

#1872)

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)
 

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)
 

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

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():

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"]

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 == {}

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.