Refund Order should return propper error message (#1726)

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,

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

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(

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)