From 6f11091d2148946422cb96852346dd1cfc56daaa Mon Sep 17 00:00:00 2001 From: Anna Gavrilman Date: Fri, 7 Jul 2023 16:23:32 +0200 Subject: [PATCH] Refund Order should return propper error message (#1726) --- ecommerce/admin.py | 2 +- ecommerce/api.py | 19 +++++++++++-------- ecommerce/api_test.py | 18 +++++++++++------- sheets/refunds_plugin.py | 6 +++--- 4 files changed, 26 insertions(+), 19 deletions(-) diff --git a/ecommerce/admin.py b/ecommerce/admin.py index 4e2d00b021..cde31bd6c7 100644 --- a/ecommerce/admin.py +++ b/ecommerce/admin.py @@ -335,7 +335,7 @@ def post(self, request): refund_amount = refund_form.cleaned_data.get("refund_amount") # Call the refund CyberSource API with provided reason and amount - refund_api_success = refund_order( + refund_api_success, _ = refund_order( order_id=order.id, refund_amount=refund_amount, refund_reason=refund_reason, diff --git a/ecommerce/api.py b/ecommerce/api.py index adfcbba86d..61497be0bf 100644 --- a/ecommerce/api.py +++ b/ecommerce/api.py @@ -397,23 +397,26 @@ def refund_order(*, order_id: int = None, reference_number: str = None, **kwargs refund_amount = kwargs.get("refund_amount") refund_reason = kwargs.get("refund_reason", "") unenroll = kwargs.get("unenroll", False) - + message = "" if reference_number is not None: order = FulfilledOrder.objects.get(reference_number=reference_number) elif order_id is not None: order = FulfilledOrder.objects.get(pk=order_id) else: - log.error("Either order_id or reference_number is required to fetch the Order.") - return False + message = "Either order_id or reference_number is required to fetch the Order." + log.error(message) + return False, message if order.state != Order.STATE.FULFILLED: - log.debug("Order with order_id %s is not in fulfilled state.", order.id) - return False + message = f"Order with order_id {order.id} is not in fulfilled state." + log.error(message) + return False, message order_recent_transaction = order.transactions.first() if not order_recent_transaction: - log.error("There is no associated transaction against order_id %s", order.id) - return False + message = f"There is no associated transaction against order_id {order.id}" + log.error(message) + return False, message transaction_dict = order_recent_transaction.data @@ -458,7 +461,7 @@ def refund_order(*, order_id: int = None, reference_number: str = None, **kwargs if unenroll: perform_downgrade_from_order.delay(order.id) - return True + return True, message def downgrade_learner_from_order(order_id): diff --git a/ecommerce/api_test.py b/ecommerce/api_test.py index 636df0ebc0..e40edcb4d1 100644 --- a/ecommerce/api_test.py +++ b/ecommerce/api_test.py @@ -161,18 +161,20 @@ def test_cybersource_refund_no_fulfilled_order(order_state): the given order_id""" unfulfilled_order = OrderFactory.create(state=order_state) - refund_response = refund_order(order_id=unfulfilled_order.id) + refund_response, message = refund_order(order_id=unfulfilled_order.id) assert f"Order with order_id {unfulfilled_order.id} is not in fulfilled state." assert refund_response is False + assert "is not in fulfilled state." in message def test_cybersource_refund_no_order_id(): """Test that refund_order returns logs properly and False when there is no Fulfilled order against the given order_id""" - refund_response = refund_order() + refund_response, message = refund_order() assert f"Either order_id or reference_number is required to fetch the Order." assert refund_response is False + assert "Either order_id or reference_number" in message def test_cybersource_order_no_transaction(fulfilled_order): @@ -182,9 +184,10 @@ def test_cybersource_order_no_transaction(fulfilled_order): """ fulfilled_order = OrderFactory.create(state=Order.STATE.FULFILLED) - refund_response = refund_order(order_id=fulfilled_order.id) + refund_response, message = refund_order(order_id=fulfilled_order.id) assert f"There is no associated transaction against order_id {fulfilled_order.id}." assert refund_response is False + assert "There is no associated transaction" in message @pytest.mark.parametrize( @@ -220,13 +223,13 @@ def test_order_refund_success(mocker, order_state, unenroll, fulfilled_transacti if order_state == ProcessorResponse.STATE_DUPLICATE: with pytest.raises(Exception) as e: - refund_success = refund_order( + refund_success, _ = refund_order( order_id=fulfilled_transaction.order.id, unenroll=unenroll ) return else: - refund_success = refund_order( + refund_success, _ = refund_order( order_id=fulfilled_transaction.order.id, unenroll=unenroll ) @@ -288,7 +291,7 @@ def test_order_refund_success_with_ref_num(mocker, unenroll, fulfilled_transacti "mitol.payment_gateway.api.PaymentGateway.start_refund", return_value=sample_response, ) - refund_success = refund_order( + refund_success, message = refund_order( reference_number=fulfilled_transaction.order.reference_number, unenroll=unenroll ) # There should be two transaction objects (One for payment and other for refund) @@ -307,6 +310,7 @@ def test_order_refund_success_with_ref_num(mocker, unenroll, fulfilled_transacti == 1 ) assert refund_success is True + assert message is "" # Unenrollment task should only run if unenrollment was requested if unenroll: @@ -338,7 +342,7 @@ def test_order_refund_failure(mocker, fulfilled_transaction): ) with pytest.raises(ApiException): - refund_response = refund_order(order_id=fulfilled_transaction.order.id) + refund_response, message = refund_order(order_id=fulfilled_transaction.order.id) assert refund_response is False assert ( Transaction.objects.filter( diff --git a/sheets/refunds_plugin.py b/sheets/refunds_plugin.py index 7966c1b721..7436acce37 100644 --- a/sheets/refunds_plugin.py +++ b/sheets/refunds_plugin.py @@ -11,10 +11,10 @@ def refunds_process_request( self, refund_request_row: RefundRequestRow ) -> RefundResult: - refund_api_success = refund_order( + refund_success, message = refund_order( reference_number=refund_request_row.order_ref_num, unenroll=True ) - if refund_api_success: + if refund_success: return RefundResult(ResultType.PROCESSED) - return RefundResult(ResultType.FAILED) + return RefundResult(ResultType.FAILED, message)