diff --git a/.github/workflows/docker-publish.yml b/.github/workflows/docker-publish.yml deleted file mode 100644 index 0452cf22..00000000 --- a/.github/workflows/docker-publish.yml +++ /dev/null @@ -1,22 +0,0 @@ -name: Push Docker Images - -on: - push: - branches: - - main -jobs: - # Push image to GitHub Packages. - # See also https://docs.docker.com/docker-hub/builds/ - push: - runs-on: ubuntu-latest - if: github.event_name == 'push' - - steps: - - name: Checkout - uses: actions/checkout@v2 - - - name: Build and Push docker image - env: - DOCKERHUB_PASSWORD: ${{ secrets.DOCKERHUB_PASSWORD }} - DOCKERHUB_USERNAME: ${{ secrets.DOCKERHUB_USERNAME }} - run : make github_docker_push diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index 082d7435..00000000 --- a/Dockerfile +++ /dev/null @@ -1,66 +0,0 @@ -FROM ubuntu:focal as app -MAINTAINER sre@edx.org - - -# Packages installed: - -# gcc; for compiling python extensions distributed with python packages like mysql-client -# git; for pulling requirements from GitHub. -# language-pack-en locales; ubuntu locale support so that system utilities have a consistent language and time zone. -# libmysqlclient-dev; to install header files needed to use native C implementation for MySQL-python for performance gains. -# libssl-dev; # mysqlclient wont install without this. -# pkg-config is now required for libmysqlclient-dev and its python dependencies -# python3-dev; to install header files for python extensions; much wheel-building depends on this -# python3-pip; install pip to install application requirements.txt files -# python; ubuntu doesnt ship with python, so this is the python we will use to run the application - -# If you add a package here please include a comment above describing what it is used for -RUN apt-get update && apt-get -qy install --no-install-recommends \ - gcc \ - git \ - language-pack-en \ - libmysqlclient-dev \ - libssl-dev \ - locales \ - pkg-config \ - python3-dev \ - python3-pip \ - python3.8 - - -RUN pip install --upgrade pip setuptools -# delete apt package lists because we do not need them inflating our image -RUN rm -rf /var/lib/apt/lists/* - -RUN ln -s /usr/bin/python3 /usr/bin/python - -RUN locale-gen en_US.UTF-8 -ENV LANG en_US.UTF-8 -ENV LANGUAGE en_US:en -ENV LC_ALL en_US.UTF-8 -ENV DJANGO_SETTINGS_MODULE commerce_coordinator.settings.production - -EXPOSE 8140 -RUN useradd -m --shell /bin/false app - -WORKDIR /edx/app/commerce-coordinator - -# Copy the requirements explicitly even though we copy everything below -# this prevents the image cache from busting unless the dependencies have changed. -COPY requirements/production.txt /edx/app/commerce-coordinator/requirements/production.txt - -# Dependencies are installed as root so they cannot be modified by the application user. -RUN pip install -r requirements/production.txt - -RUN mkdir -p /edx/var/log - -# Code is owned by root so it cannot be modified by the application user. -# So we copy it before changing users. -USER app - -# Gunicorn 19 does not log to stdout or stderr by default. Once we are past gunicorn 19, the logging to STDOUT need not be specified. -CMD gunicorn --workers=2 --name commerce-coordinator -c /edx/app/commerce-coordinator/commerce_coordinator/docker_gunicorn_configuration.py --log-file - --max-requests=1000 commerce-coordinator.wsgi:application - -# This line is after the requirements so that changes to the code will not -# bust the image cache -COPY . /edx/app/commerce-coordinator diff --git a/Makefile b/Makefile index 30c79300..5e9eb997 100644 --- a/Makefile +++ b/Makefile @@ -155,17 +155,11 @@ start-devstack: ## run a local development copy of the server open-devstack: ## open a shell on the server started by start-devstack docker exec -it commerce-coordinator /edx/app/commerce-coordinator/devstack.sh open -pkg-devstack: ## build the commerce-coordinator image from the latest configuration and code - docker build -t commerce-coordinator:latest -f docker/build/commerce-coordinator/Dockerfile git://github.com/openedx/configuration - detect_changed_source_translations: ## check if translation files are up-to-date cd commerce_coordinator && i18n_tool changed validate_translations: fake_translations detect_changed_source_translations ## install fake translations and check if translation files are up-to-date -docker_build: - docker build . -f Dockerfile -t openedx/commerce-coordinator - dev.provision_docker: # start LMS, and Setup a clean commerce-coordinator stack. bash provision-commerce-coordinator.sh @@ -176,9 +170,6 @@ dev.run_test_query: dev.up: # Starts all containers docker-compose up -d -dev.up.build: - docker-compose up -d --build - dev.down: # Kills containers and all of their data that isn't in volumes docker-compose down @@ -187,7 +178,7 @@ dev.stop: # Stops containers so they can be restarted dev.multistack.up: bash find-start-lms.sh - docker-compose up -d --build + docker-compose up -d dev.multistack.stop: docker compose -p devstack stop @@ -212,19 +203,6 @@ db-shell: # Run the app shell as root, enter the app's database %-attach: docker attach commerce_coordinator.$* -github_docker_build: - docker build . -f Dockerfile --target app -t openedx/commerce-coordinator - -github_docker_tag: github_docker_build - docker tag openedx/commerce-coordinator openedx/commerce-coordinator:${GITHUB_SHA} - -github_docker_auth: - echo "$$DOCKERHUB_PASSWORD" | docker login -u "$$DOCKERHUB_USERNAME" --password-stdin - -github_docker_push: github_docker_tag github_docker_auth ## push to docker hub - docker push 'openedx/commerce-coordinator:latest' - docker push "openedx/commerce-coordinator:${GITHUB_SHA}" - selfcheck: ## check that the Makefile is well-formed @echo "The Makefile is well-formed." 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/docker-compose.yml b/docker-compose.yml index 636dfc26..69f7ab62 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -14,8 +14,7 @@ services: container_name: commerce-coordinator.memcache app: - image: devstack # this should exist locally from previous devstack, we will build our Dockerfile - build: . # Build Dockerfile if we need to. + image: edxops/commerce-coordinator-dev container_name: commerce-coordinator.app volumes: - .:/edx/app/commerce-coordinator/ 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 +""" diff --git a/provision-commerce-coordinator.sh b/provision-commerce-coordinator.sh index dfdef86d..d81b7ae6 100644 --- a/provision-commerce-coordinator.sh +++ b/provision-commerce-coordinator.sh @@ -12,10 +12,7 @@ NC="\x1B[0m" bash ./find-start-lms.sh docker-compose down -v # its ok if this fails. -docker-compose up -d --build - -# Install requirements -# Can be skipped right now because we're using the --build flag on docker-compose. This will need to be changed once we move to devstack. +docker-compose up -d # Wait for MySQL echo "Waiting for MySQL"