From 05221318d3ea4477a348d1d9639852dd4c15316b Mon Sep 17 00:00:00 2001 From: Syed Sajjad Hussain Shah <52817156+syedsajjadkazmii@users.noreply.github.com> Date: Wed, 4 Dec 2024 16:48:32 +0500 Subject: [PATCH] feat: send enrollment/fulfillment failure email on unsupported course mode (#302) * chore: added logs to note time for commercetools calls (#304) * feat: add tests * fix: docs error * fix: quality error * fix: handle review comments * fix: handle final review --------- Co-authored-by: NoyanAziz --- .../apps/commercetools/serializers.py | 3 + .../apps/commercetools/sub_messages/tasks.py | 5 +- .../apps/commercetools/tests/conftest.py | 1 + .../apps/commercetools/tests/test_utils.py | 55 ++++++++++++++++ .../apps/commercetools/utils.py | 21 +++++++ .../apps/lms/signal_handlers.py | 5 +- commerce_coordinator/apps/lms/tasks.py | 62 ++++++++++++++++++- .../apps/lms/tests/constants.py | 5 +- .../apps/lms/tests/test_signals.py | 5 +- .../apps/lms/tests/test_tasks.py | 46 +++++++++++++- commerce_coordinator/settings/base.py | 1 + commerce_coordinator/settings/local.py | 1 + docs/conf.py | 7 +++ 13 files changed, 208 insertions(+), 9 deletions(-) diff --git a/commerce_coordinator/apps/commercetools/serializers.py b/commerce_coordinator/apps/commercetools/serializers.py index fafac844..5a377581 100644 --- a/commerce_coordinator/apps/commercetools/serializers.py +++ b/commerce_coordinator/apps/commercetools/serializers.py @@ -87,6 +87,9 @@ class OrderFulfillViewInputSerializer(CoordinatorSerializer): line_item_state_id = serializers.CharField(allow_null=False) edx_lms_user_id = serializers.IntegerField(allow_null=False) message_id = serializers.CharField(allow_null=False) + course_title = serializers.CharField(allow_null=False) + user_first_name = serializers.CharField(allow_null=False) + user_email = serializers.EmailField(allow_null=False) class OrderReturnedViewMessageLineItemReturnItemSerializer(CoordinatorSerializer): diff --git a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py index 0c0e9c3d..b6505d6f 100644 --- a/commerce_coordinator/apps/commercetools/sub_messages/tasks.py +++ b/commerce_coordinator/apps/commercetools/sub_messages/tasks.py @@ -125,7 +125,10 @@ def fulfill_order_placed_message_signal_task( 'course_mode': get_course_mode_from_ct_order(item), 'item_quantity': item.quantity, 'line_item_state_id': line_item_state_id, - 'message_id': message_id + 'message_id': message_id, + 'user_first_name': customer.first_name, + 'user_email': customer.email, + 'course_title': item.name.get('en-US', '') }) # the following throws and thus doesn't need to be a conditional diff --git a/commerce_coordinator/apps/commercetools/tests/conftest.py b/commerce_coordinator/apps/commercetools/tests/conftest.py index c173843c..61b7f639 100644 --- a/commerce_coordinator/apps/commercetools/tests/conftest.py +++ b/commerce_coordinator/apps/commercetools/tests/conftest.py @@ -284,6 +284,7 @@ def gen_example_customer() -> CTCustomer: def gen_customer(email: str, un: str): return CTCustomer( + first_name='John', email=email, custom=CTCustomFields( type=CTTypeReference( diff --git a/commerce_coordinator/apps/commercetools/tests/test_utils.py b/commerce_coordinator/apps/commercetools/tests/test_utils.py index a0c30dec..43c7e53b 100644 --- a/commerce_coordinator/apps/commercetools/tests/test_utils.py +++ b/commerce_coordinator/apps/commercetools/tests/test_utils.py @@ -28,6 +28,7 @@ has_full_refund_transaction, has_refund_transaction, send_order_confirmation_email, + send_unsupported_mode_fulfillment_error_email, translate_stripe_refund_status_to_transaction_status ) @@ -126,6 +127,60 @@ def test_send_order_confirmation_email_failure(self, mock_logger, mock_get_braze mock_logger.assert_called_once_with('Encountered exception sending Order confirmation email. ' 'Exception: Error sending Braze email') + @override_settings( + BRAZE_API_KEY="braze_api_key", + BRAZE_API_SERVER="braze_api_server", + BRAZE_CT_FULFILLMENT_UNSUPPORTED_MODE_ERROR_CANVAS_ID="dummy_canvas" + ) + @patch('commerce_coordinator.apps.commercetools.utils.get_braze_client') + def test_send_unsupported_mode_fulfillment_error_email_success(self, mock_get_braze_client): + mock_braze_client = Mock() + mock_get_braze_client.return_value = mock_braze_client + + canvas_entry_properties = {} + lms_user_id = 'user123' + lms_user_email = 'user@example.com' + + with patch.object(mock_braze_client, 'send_canvas_message') as mock_send_canvas_message: + send_unsupported_mode_fulfillment_error_email( + lms_user_id, lms_user_email, canvas_entry_properties + ) + + mock_send_canvas_message.assert_called_once_with( + canvas_id='dummy_canvas', + recipients=[{"external_user_id": lms_user_id, "attributes": {"email": lms_user_email}}], + canvas_entry_properties=canvas_entry_properties, + ) + + @override_settings( + BRAZE_API_KEY="braze_api_key", + BRAZE_API_SERVER="braze_api_server", + BRAZE_CT_FULFILLMENT_UNSUPPORTED_MODE_ERROR_CANVAS_ID="dummy_canvas" + ) + @patch('commerce_coordinator.apps.commercetools.utils.get_braze_client') + @patch('commerce_coordinator.apps.commercetools.utils.logger.exception') + def test_send_unsupported_mode_fulfillment_error_email_failure(self, mock_logger, mock_get_braze_client): + mock_braze_client = Mock() + mock_get_braze_client.return_value = mock_braze_client + + canvas_entry_properties = {} + lms_user_id = 'user123' + lms_user_email = 'user@example.com' + + with patch.object(mock_braze_client, 'send_canvas_message') as mock_send_canvas_message: + mock_send_canvas_message.side_effect = Exception('Error sending Braze email') + send_unsupported_mode_fulfillment_error_email( + lms_user_id, lms_user_email, canvas_entry_properties + ) + + mock_send_canvas_message.assert_called_once_with( + canvas_id='dummy_canvas', + recipients=[{"external_user_id": lms_user_id, "attributes": {"email": lms_user_email}}], + canvas_entry_properties=canvas_entry_properties, + ) + mock_logger.assert_called_once_with('Encountered exception sending Fulfillment unsupported mode error ' + 'email. Exception: Error sending Braze email') + def test_extract_ct_product_information_for_braze_canvas(self): order = gen_order(EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD['order_number']) line_item = order.line_items[0] diff --git a/commerce_coordinator/apps/commercetools/utils.py b/commerce_coordinator/apps/commercetools/utils.py index 8f4f6ca6..f7e8b415 100644 --- a/commerce_coordinator/apps/commercetools/utils.py +++ b/commerce_coordinator/apps/commercetools/utils.py @@ -65,6 +65,27 @@ def send_order_confirmation_email( logger.exception(f"Encountered exception sending Order confirmation email. Exception: {exc}") +def send_unsupported_mode_fulfillment_error_email( + lms_user_id, lms_user_email, canvas_entry_properties +): + """ Sends fulfillment error email via Braze. """ + recipients = [{"external_user_id": lms_user_id, "attributes": { + "email": lms_user_email, + }}] + canvas_id = settings.BRAZE_CT_FULFILLMENT_UNSUPPORTED_MODE_ERROR_CANVAS_ID + + try: + braze_client = get_braze_client() + if braze_client: + braze_client.send_canvas_message( + canvas_id=canvas_id, + recipients=recipients, + canvas_entry_properties=canvas_entry_properties, + ) + except Exception as exc: # pylint: disable=broad-exception-caught + logger.exception(f"Encountered exception sending Fulfillment unsupported mode error email. Exception: {exc}") + + def format_amount_for_braze_canvas(centAmount): """ Utility to convert amount to dollar with 2 decimals percision. Also adds the Dollar signs to resulting value. diff --git a/commerce_coordinator/apps/lms/signal_handlers.py b/commerce_coordinator/apps/lms/signal_handlers.py index e9cccfef..da72e353 100644 --- a/commerce_coordinator/apps/lms/signal_handlers.py +++ b/commerce_coordinator/apps/lms/signal_handlers.py @@ -28,6 +28,9 @@ def fulfill_order_placed_send_enroll_in_course(**kwargs): line_item_id=kwargs['line_item_id'], item_quantity=kwargs['item_quantity'], line_item_state_id=kwargs['line_item_state_id'], - message_id=kwargs['message_id'] + message_id=kwargs['message_id'], + user_first_name=kwargs['user_first_name'], + user_email=kwargs['user_email'], + course_title=kwargs['course_title'] ) return async_result.id diff --git a/commerce_coordinator/apps/lms/tasks.py b/commerce_coordinator/apps/lms/tasks.py index 5a825256..c276f97b 100644 --- a/commerce_coordinator/apps/lms/tasks.py +++ b/commerce_coordinator/apps/lms/tasks.py @@ -2,15 +2,17 @@ LMS Celery tasks """ +import json from datetime import datetime -from celery import shared_task +from celery import Task, shared_task from celery.utils.log import get_task_logger from django.contrib.auth import get_user_model from requests import RequestException from commerce_coordinator.apps.commercetools.catalog_info.constants import TwoUKeys from commerce_coordinator.apps.commercetools.clients import CommercetoolsAPIClient +from commerce_coordinator.apps.commercetools.utils import send_unsupported_mode_fulfillment_error_email from commerce_coordinator.apps.lms.clients import LMSAPIClient # Use the special Celery logger for our tasks @@ -18,7 +20,58 @@ User = get_user_model() -@shared_task(bind=True, autoretry_for=(RequestException,), retry_kwargs={'max_retries': 5, 'countdown': 3}) +class CourseEnrollTaskAfterReturn(Task): # pylint: disable=abstract-method + """ + Base class for fulfill_order_placed_send_enroll_in_course_task + """ + + def on_failure(self, exc, task_id, args, kwargs, einfo): + edx_lms_user_id = kwargs.get('edx_lms_user_id') + user_email = kwargs.get('user_email') + order_number = kwargs.get('order_number') + user_first_name = kwargs.get('user_first_name') + course_title = kwargs.get('course_title') + + error_message = ( + json.loads(exc.response.text).get('message', '') + if isinstance(exc, RequestException) and exc.response is not None + else str(exc) + ) + + logger.error( + f"Post-purchase fulfillment task {self.name} failed after max " + f"retries with the error message: {error_message} " + f"for user with user Id: {edx_lms_user_id}, email: {user_email}, " + f"order number: {order_number}, and course title: {course_title}" + ) + + # This error is returned from LMS if the course mode is unsupported + # https://github.com/openedx/edx-platform/blob/master/openedx/core/djangoapps/enrollments/views.py#L870 + course_mode_expired_error = "course mode is expired or otherwise unavailable for course run" + + if course_mode_expired_error in error_message: + logger.info( + f"Sending unsupported course mode fulfillment error email " + f"for the user with user ID: {edx_lms_user_id}, email: {user_email}, " + f"order number: {order_number}, and course title: {course_title}" + ) + + canvas_entry_properties = { + 'order_number': order_number, + 'product_type': 'course', # TODO: Fetch product type from commercetools product object + 'product_name': course_title, + 'first_name': user_first_name, + } + # Send failure notification email + send_unsupported_mode_fulfillment_error_email(edx_lms_user_id, user_email, canvas_entry_properties) + + +@shared_task( + bind=True, + autoretry_for=(RequestException,), + retry_kwargs={'max_retries': 5, 'countdown': 3}, + base=CourseEnrollTaskAfterReturn, +) def fulfill_order_placed_send_enroll_in_course_task( self, course_id, @@ -34,7 +87,10 @@ def fulfill_order_placed_send_enroll_in_course_task( line_item_id, item_quantity, line_item_state_id, - message_id + message_id, + user_first_name, # pylint: disable=unused-argument + user_email, # pylint: disable=unused-argument + course_title # pylint: disable=unused-argument ): """ Celery task for order placed fulfillment and enrollment via LMS Enrollment API. diff --git a/commerce_coordinator/apps/lms/tests/constants.py b/commerce_coordinator/apps/lms/tests/constants.py index a9f05f19..7241cb1c 100644 --- a/commerce_coordinator/apps/lms/tests/constants.py +++ b/commerce_coordinator/apps/lms/tests/constants.py @@ -29,7 +29,10 @@ 'line_item_id': '822d77c4-00a6-4fb9-909b-094ef0b8c4b9', 'item_quantity': 1, 'line_item_state_id': '8f2e888e-9777-4557-9a7f-c649153770c2', - 'message_id': '1063f19c-08f3-41a4-a952-a8577374373c' + 'message_id': '1063f19c-08f3-41a4-a952-a8577374373c', + 'user_first_name': 'test', + 'user_email': 'test@example.com', + 'course_title': 'Demonstration Course', } EXAMPLE_FULFILLMENT_REQUEST_PAYLOAD = { diff --git a/commerce_coordinator/apps/lms/tests/test_signals.py b/commerce_coordinator/apps/lms/tests/test_signals.py index 10336858..98a23da6 100644 --- a/commerce_coordinator/apps/lms/tests/test_signals.py +++ b/commerce_coordinator/apps/lms/tests/test_signals.py @@ -35,7 +35,10 @@ class FulfillOrderPlacedSendEnrollInCourseTest(CoordinatorSignalReceiverTestCase 'line_item_id': 11, 'item_quantity': 1, 'line_item_state_id': 12, - 'message_id': 13 + 'message_id': 13, + 'user_first_name': 14, + 'user_email': 15, + 'course_title': 16, } def test_correct_arguments_passed(self, mock_task): diff --git a/commerce_coordinator/apps/lms/tests/test_tasks.py b/commerce_coordinator/apps/lms/tests/test_tasks.py index 595d68a2..120bd07a 100644 --- a/commerce_coordinator/apps/lms/tests/test_tasks.py +++ b/commerce_coordinator/apps/lms/tests/test_tasks.py @@ -3,7 +3,7 @@ """ import logging -from unittest.mock import patch, sentinel +from unittest.mock import Mock, patch, sentinel from django.test import TestCase from requests import RequestException @@ -49,7 +49,10 @@ def unpack_for_uut(values): values['line_item_id'], values['item_quantity'], values['line_item_state_id'], - values['message_id'] + values['message_id'], + values['user_first_name'], + values['user_email'], + values['course_title'] ) def setUp(self): @@ -130,3 +133,42 @@ def test_retry_logic(self, mock_ct_get_order, mock_ct_get_state, mock_client): mock_ct_get_state.assert_called_with(TwoUKeys.FAILURE_FULFILMENT_STATE) mock_ct_get_order.assert_called_with(EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD.get('order_id')) + + @patch('commerce_coordinator.apps.lms.tasks.send_unsupported_mode_fulfillment_error_email') + @patch.object(fulfill_order_placed_send_enroll_in_course_task, 'max_retries', 5) + def test_fulfillment_error_email_is_sent_on_failure( + self, mock_send_email, mock_client + ): # pylint: disable=unused-argument + """ + Test that `on_failure` sends the appropriate failure email. + """ + mock_response = Mock() + mock_response.text = '{"message": "course mode is expired or otherwise unavailable for course run"}' + exception = RequestException("400 Bad Request") + exception.response = mock_response + + exc = exception + task_id = "test_task_id" + args = [] + kwargs = EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD + einfo = Mock() + + fulfill_order_placed_send_enroll_in_course_task.push_request(retries=5) + fulfill_order_placed_send_enroll_in_course_task.on_failure( + exc=exc, + task_id=task_id, + args=args, + kwargs=kwargs, + einfo=einfo + ) + + mock_send_email.assert_called_once_with( + EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD['edx_lms_user_id'], + EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD['user_email'], + { + 'order_number': EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD['order_number'], + 'product_type': 'course', + 'product_name': EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD['course_title'], + 'first_name': EXAMPLE_FULFILLMENT_SIGNAL_PAYLOAD['user_first_name'], + } + ) diff --git a/commerce_coordinator/settings/base.py b/commerce_coordinator/settings/base.py index 491d659b..de5d3b0f 100644 --- a/commerce_coordinator/settings/base.py +++ b/commerce_coordinator/settings/base.py @@ -468,6 +468,7 @@ def root(*path_fragments): BRAZE_API_KEY = None BRAZE_API_SERVER = None BRAZE_CT_ORDER_CONFIRMATION_CANVAS_ID = '' +BRAZE_CT_FULFILLMENT_UNSUPPORTED_MODE_ERROR_CANVAS_ID = '' # SEGMENT WRITE KEY SEGMENT_KEY = None diff --git a/commerce_coordinator/settings/local.py b/commerce_coordinator/settings/local.py index 17bd5c97..6d7803d6 100644 --- a/commerce_coordinator/settings/local.py +++ b/commerce_coordinator/settings/local.py @@ -7,6 +7,7 @@ 'host.docker.internal', 'localhost', '.ngrok-free.app', + '.share.zrok.io', ) INSTALLED_APPS += ( diff --git a/docs/conf.py b/docs/conf.py index 05376dbe..7ff2388e 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -556,3 +556,10 @@ def setup(app): """Sphinx extension: run sphinx-apidoc.""" event = 'builder-inited' app.connect(event, on_init) + +# celery.Task has some roles inside the library that are not recognized by Sphinx +# and causing errors, so we add them here to avoid the errors. +rst_prolog = """ +.. role:: setting +.. role:: sig +"""