diff --git a/common/djangoapps/entitlements/api/v1/tests/test_views.py b/common/djangoapps/entitlements/api/v1/tests/test_views.py index 6b535ac642..e508fbffed 100644 --- a/common/djangoapps/entitlements/api/v1/tests/test_views.py +++ b/common/djangoapps/entitlements/api/v1/tests/test_views.py @@ -22,6 +22,7 @@ if settings.ROOT_URLCONF == 'lms.urls': from entitlements.tests.factories import CourseEntitlementFactory from entitlements.models import CourseEntitlement from entitlements.api.v1.serializers import CourseEntitlementSerializer + from entitlements.signals import REFUND_ENTITLEMENT @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -429,3 +430,48 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase): assert response.status_code == 400 assert response.data['message'] == expected_message # pylint: disable=no-member assert not CourseEnrollment.is_enrolled(self.user, fake_course_key) + + @patch('lms.djangoapps.commerce.signals.refund_entitlement', return_value=[1]) + @patch("entitlements.api.v1.views.get_course_runs_for_course") + def test_user_can_revoke_and_refund(self, mock_get_course_runs, mock_refund_entitlement): + course_entitlement = CourseEntitlementFactory.create(user=self.user) + mock_get_course_runs.return_value = self.return_values + + url = reverse( + self.ENTITLEMENTS_ENROLLMENT_NAMESPACE, + args=[str(course_entitlement.uuid)] + ) + assert course_entitlement.enrollment_course_run is None + + data = { + 'course_run_id': str(self.course.id) + } + response = self.client.post( + url, + data=json.dumps(data), + content_type='application/json', + ) + course_entitlement.refresh_from_db() + + assert response.status_code == 201 + assert CourseEnrollment.is_enrolled(self.user, self.course.id) + + # Unenroll with Revoke for refund + with patch('lms.djangoapps.commerce.signals.handle_refund_entitlement') as mock_refund_handler: + REFUND_ENTITLEMENT.connect(mock_refund_handler) + + # pre_db_changes_entitlement = course_entitlement + revoke_url = url + '?is_refund=true' + response = self.client.delete( + revoke_url, + content_type='application/json', + ) + assert response.status_code == 204 + + course_entitlement.refresh_from_db() + assert mock_refund_handler.called + assert (CourseEntitlementSerializer(mock_refund_handler.call_args[1]['course_entitlement']).data == + CourseEntitlementSerializer(course_entitlement).data) + assert not CourseEnrollment.is_enrolled(self.user, self.course.id) + assert course_entitlement.enrollment_course_run is None + assert course_entitlement.expired_at is not None diff --git a/common/djangoapps/entitlements/api/v1/views.py b/common/djangoapps/entitlements/api/v1/views.py index 13aa941a05..8b41b891f9 100644 --- a/common/djangoapps/entitlements/api/v1/views.py +++ b/common/djangoapps/entitlements/api/v1/views.py @@ -14,6 +14,7 @@ from entitlements.api.v1.filters import CourseEntitlementFilter from entitlements.api.v1.permissions import IsAdminOrAuthenticatedReadOnly from entitlements.api.v1.serializers import CourseEntitlementSerializer from entitlements.models import CourseEntitlement +from entitlements.signals import REFUND_ENTITLEMENT from openedx.core.djangoapps.catalog.utils import get_course_runs_for_course from openedx.core.djangoapps.cors_csrf.authentication import SessionAuthenticationCrossDomainCsrf from student.models import CourseEnrollment @@ -149,7 +150,7 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): except AlreadyEnrolledError: enrollment = CourseEnrollment.get_enrollment(user, course_run_key) if enrollment.mode == entitlement.mode: - CourseEntitlement.set_enrollment(entitlement, enrollment) + entitlement.set_enrollment(enrollment) # Else the User is already enrolled in another Mode and we should # not do anything else related to Entitlements. except CourseEnrollmentException: @@ -167,7 +168,7 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): data={'message': message} ) - CourseEntitlement.set_enrollment(entitlement, enrollment) + entitlement.set_enrollment(enrollment) return None def _unenroll_entitlement(self, entitlement, course_run_key, user): @@ -175,7 +176,7 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): Internal method to handle the details of Unenrolling a User in a Course Run. """ CourseEnrollment.unenroll(user, course_run_key, skip_refund=True) - CourseEntitlement.set_enrollment(entitlement, None) + entitlement.set_enrollment(None) def create(self, request, uuid): """ @@ -196,7 +197,7 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): data='The Course Run ID was not provided.' ) - # Verify that the user has an Entitlement for the provided Course UUID. + # Verify that the user has an Entitlement for the provided Entitlement UUID. try: entitlement = CourseEntitlement.objects.get(uuid=uuid, user=request.user, expired_at=None) except CourseEntitlement.DoesNotExist: @@ -205,7 +206,7 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): data='The Entitlement for this UUID does not exist or is Expired.' ) - # Verify the course run ID is of the same type as the Course entitlement. + # Verify the course run ID is of the same Course as the Course entitlement. course_run_valid = self._verify_course_run_for_entitlement(entitlement, course_run_id) if not course_run_valid: return Response( @@ -257,7 +258,13 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): def destroy(self, request, uuid): """ On DELETE call to this API we will unenroll the course enrollment for the provided uuid + + If is_refund parameter is provided then unenroll the user, set Entitlement expiration, and issue + a refund """ + is_refund = True if request.query_params.get('is_refund', 'false') == 'true' else False + + # Retrieve the entitlement for the UUID belongs to the current user. try: entitlement = CourseEntitlement.objects.get(uuid=uuid, user=request.user, expired_at=None) except CourseEntitlement.DoesNotExist: @@ -266,12 +273,42 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): data='The Entitlement for this UUID does not exist or is Expired.' ) - if entitlement.enrollment_course_run is None: - return Response(status=status.HTTP_204_NO_CONTENT) + if is_refund and entitlement.is_entitlement_refundable(): + with transaction.atomic(): + # Revoke and refund the entitlement + if entitlement.enrollment_course_run is not None: + self._unenroll_entitlement( + entitlement=entitlement, + course_run_key=entitlement.enrollment_course_run.course_id, + user=request.user + ) + + # Revoke the Course Entitlement and issue Refund + log.info( + 'Entitlement Refund requested for Course Entitlement[%s]', + str(entitlement.uuid) + ) + + REFUND_ENTITLEMENT.send(sender=None, course_entitlement=entitlement) + entitlement.expired_at_datetime = timezone.now() + entitlement.save() + + log.info( + 'Set expired_at to [%s] for course entitlement [%s]', + entitlement.expired_at, + entitlement.uuid + ) + elif not is_refund: + if entitlement.enrollment_course_run is not None: + self._unenroll_entitlement( + entitlement=entitlement, + course_run_key=entitlement.enrollment_course_run.course_id, + user=request.user + ) + else: + log.info( + 'Entitlement Refund failed for Course Entitlement [%s]. Entitlement is not refundable', + str(entitlement.uuid) + ) - self._unenroll_entitlement( - entitlement=entitlement, - course_run_key=entitlement.enrollment_course_run.course_id, - user=request.user - ) return Response(status=status.HTTP_204_NO_CONTENT) diff --git a/common/djangoapps/entitlements/models.py b/common/djangoapps/entitlements/models.py index fa80ac0301..14813fdc81 100644 --- a/common/djangoapps/entitlements/models.py +++ b/common/djangoapps/entitlements/models.py @@ -221,12 +221,12 @@ class CourseEntitlement(TimeStampedModel): 'expired_at': self.expired_at } - @classmethod - def set_enrollment(cls, entitlement, enrollment): + def set_enrollment(self, enrollment): """ Fulfills an entitlement by specifying a session. """ - cls.objects.filter(id=entitlement.id).update(enrollment_course_run=enrollment) + self.enrollment_course_run = enrollment + self.save() @classmethod def unexpired_entitlements_for_user(cls, user): diff --git a/common/djangoapps/entitlements/signals.py b/common/djangoapps/entitlements/signals.py new file mode 100644 index 0000000000..8783699f9e --- /dev/null +++ b/common/djangoapps/entitlements/signals.py @@ -0,0 +1,6 @@ +""" +Enrollment track related signals. +""" +from django.dispatch import Signal + +REFUND_ENTITLEMENT = Signal(providing_args=['course_entitlement']) diff --git a/lms/djangoapps/commerce/signals.py b/lms/djangoapps/commerce/signals.py index 01b4e9146d..f9ca113942 100644 --- a/lms/djangoapps/commerce/signals.py +++ b/lms/djangoapps/commerce/signals.py @@ -14,12 +14,12 @@ from django.contrib.auth.models import AnonymousUser from django.dispatch import receiver from django.utils.translation import ugettext as _ +from entitlements.signals import REFUND_ENTITLEMENT from openedx.core.djangoapps.commerce.utils import ecommerce_api_client, is_commerce_service_configured from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers from openedx.core.djangoapps.theming import helpers as theming_helpers from request_cache.middleware import RequestCache from student.signals import REFUND_ORDER - from .models import CommerceConfiguration log = logging.getLogger(__name__) @@ -46,7 +46,7 @@ def handle_refund_order(sender, course_enrollment=None, **kwargs): # there's certainly no need to inform Otto about this request. return refund_seat(course_enrollment) - except: # pylint: disable=bare-except + except Exception: # pylint: disable=broad-except # don't assume the signal was fired with `send_robust`. # avoid blowing up other signal handlers by gracefully # trapping the Exception and logging an error. @@ -57,6 +57,30 @@ def handle_refund_order(sender, course_enrollment=None, **kwargs): ) +# pylint: disable=unused-argument +@receiver(REFUND_ENTITLEMENT) +def handle_refund_entitlement(sender, course_entitlement=None, **kwargs): + if not is_commerce_service_configured(): + return + + if course_entitlement and course_entitlement.is_entitlement_refundable(): + try: + request_user = get_request_user() + if request_user and course_entitlement.user == request_user: + refund_entitlement(course_entitlement) + except Exception as exc: # pylint: disable=broad-except + # don't assume the signal was fired with `send_robust`. + # avoid blowing up other signal handlers by gracefully + # trapping the Exception and logging an error. + log.exception( + "Unexpected exception while attempting to initiate refund for user [%s], " + "course entitlement [%s] message: [%s]", + course_entitlement.user.id, + course_entitlement.uuid, + str(exc) + ) + + def get_request_user(): """ Helper to get the authenticated user from the current HTTP request (if @@ -69,6 +93,57 @@ def get_request_user(): return getattr(request, 'user', None) +def _process_refund(refund_ids, api_client, course_product, is_entitlement=False): + """ + Helper method to process a refund for a given course_product + """ + config = CommerceConfiguration.current() + + if config.enable_automatic_refund_approval: + refunds_requiring_approval = [] + + for refund_id in refund_ids: + try: + # NOTE: Approve payment only because the user has already been unenrolled. Additionally, this + # ensures we don't tie up an additional web worker when the E-Commerce Service tries to unenroll + # the learner + api_client.refunds(refund_id).process.put({'action': 'approve_payment_only'}) + log.info('Refund [%d] successfully approved.', refund_id) + except: # pylint: disable=bare-except + log.exception('Failed to automatically approve refund [%d]!', refund_id) + refunds_requiring_approval.append(refund_id) + else: + refunds_requiring_approval = refund_ids + + if refunds_requiring_approval: + # XCOM-371: this is a temporary measure to suppress refund-related email + # notifications to students and support for free enrollments. This + # condition should be removed when the CourseEnrollment.refundable() logic + # is updated to be more correct, or when we implement better handling (and + # notifications) in Otto for handling reversal of $0 transactions. + if course_product.mode != 'verified': + # 'verified' is the only enrollment mode that should presently + # result in opening a refund request. + msg = 'Skipping refund email notification for non-verified mode for user [%s], course [%s], mode: [%s]' + course_identifier = course_product.course_id + if is_entitlement: + course_identifier = str(course_product.uuid) + msg = ('Skipping refund email notification for non-verified mode for user [%s], ' + 'course entitlement [%s], mode: [%s]') + log.info( + msg, + course_product.user.id, + course_identifier, + course_product.mode, + ) + else: + try: + send_refund_notification(course_product, refunds_requiring_approval) + except: # pylint: disable=bare-except + # don't break, just log a warning + log.warning('Could not send email notification for refund.', exc_info=True) + + def refund_seat(course_enrollment): """ Attempt to initiate a refund for any orders associated with the seat being unenrolled, using the commerce service. @@ -98,51 +173,64 @@ def refund_seat(course_enrollment): if refund_ids: log.info('Refund successfully opened for user [%s], course [%s]: %r', enrollee.id, course_key_str, refund_ids) - config = CommerceConfiguration.current() - - if config.enable_automatic_refund_approval: - refunds_requiring_approval = [] - - for refund_id in refund_ids: - try: - # NOTE: Approve payment only because the user has already been unenrolled. Additionally, this - # ensures we don't tie up an additional web worker when the E-Commerce Service tries to unenroll - # the learner - api_client.refunds(refund_id).process.put({'action': 'approve_payment_only'}) - log.info('Refund [%d] successfully approved.', refund_id) - except: # pylint: disable=bare-except - log.exception('Failed to automatically approve refund [%d]!', refund_id) - refunds_requiring_approval.append(refund_id) - else: - refunds_requiring_approval = refund_ids - - if refunds_requiring_approval: - # XCOM-371: this is a temporary measure to suppress refund-related email - # notifications to students and support for free enrollments. This - # condition should be removed when the CourseEnrollment.refundable() logic - # is updated to be more correct, or when we implement better handling (and - # notifications) in Otto for handling reversal of $0 transactions. - if course_enrollment.mode != 'verified': - # 'verified' is the only enrollment mode that should presently - # result in opening a refund request. - log.info( - 'Skipping refund email notification for non-verified mode for user [%s], course [%s], mode: [%s]', - course_enrollment.user.id, - course_enrollment.course_id, - course_enrollment.mode, - ) - else: - try: - send_refund_notification(course_enrollment, refunds_requiring_approval) - except: # pylint: disable=bare-except - # don't break, just log a warning - log.warning('Could not send email notification for refund.', exc_info=True) + _process_refund( + refund_ids=refund_ids, + api_client=api_client, + course_product=course_enrollment, + ) else: log.info('No refund opened for user [%s], course [%s]', enrollee.id, course_key_str) return refund_ids +def refund_entitlement(course_entitlement): + """ + Attempt a refund of a course entitlement + :param course_entitlement: + :return: + """ + user_model = get_user_model() + enrollee = course_entitlement.user + entitlement_uuid = str(course_entitlement.uuid) + + service_user = user_model.objects.get(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME) + api_client = ecommerce_api_client(service_user) + + log.info( + 'Attempting to create a refund for user [%s], course entitlement [%s]...', + enrollee.username, + entitlement_uuid + ) + + refund_ids = api_client.refunds.post( + { + 'order_number': course_entitlement.order_number, + 'username': enrollee.username, + 'entitlement_uuid': entitlement_uuid, + } + ) + + if refund_ids: + log.info( + 'Refund successfully opened for user [%s], course entitlement [%s]: %r', + enrollee.username, + entitlement_uuid, + refund_ids, + ) + + _process_refund( + refund_ids=refund_ids, + api_client=api_client, + course_product=course_entitlement, + is_entitlement=True + ) + else: + log.info('No refund opened for user [%s], course entitlement [%s]', enrollee.id, entitlement_uuid) + + return refund_ids + + def create_zendesk_ticket(requester_name, requester_email, subject, body, tags=None): """ Create a Zendesk ticket via API. """ if not (settings.ZENDESK_URL and settings.ZENDESK_USER and settings.ZENDESK_API_KEY): @@ -206,7 +294,7 @@ def generate_refund_notification_body(student, refund_ids): # pylint: disable=i return '{msg}\n\n{urls}'.format(msg=msg, urls='\n'.join(refund_urls)) -def send_refund_notification(course_enrollment, refund_ids): +def send_refund_notification(course_product, refund_ids): """ Notify the support team of the refund request. """ tags = ['auto_refund'] @@ -215,7 +303,7 @@ def send_refund_notification(course_enrollment, refund_ids): # this is not presently supported with the external service. raise NotImplementedError("Unable to send refund processing emails to support teams.") - student = course_enrollment.user + student = course_product.user subject = _("[Refund] User-Requested Refund") body = generate_refund_notification_body(student, refund_ids) requester_name = student.profile.name or student.username diff --git a/lms/djangoapps/commerce/tests/test_signals.py b/lms/djangoapps/commerce/tests/test_signals.py index 55b1d15d53..d2fd5e8fef 100644 --- a/lms/djangoapps/commerce/tests/test_signals.py +++ b/lms/djangoapps/commerce/tests/test_signals.py @@ -19,13 +19,14 @@ from opaque_keys.edx.keys import CourseKey from requests import Timeout from course_modes.models import CourseMode +from entitlements.signals import REFUND_ENTITLEMENT +from entitlements.tests.factories import CourseEntitlementFactory from student.signals import REFUND_ORDER from student.tests.factories import CourseEnrollmentFactory, UserFactory - from . import JSON +from .mocks import mock_create_refund, mock_process_refund from ..models import CommerceConfiguration from ..signals import create_zendesk_ticket, generate_refund_notification_body, send_refund_notification -from .mocks import mock_create_refund, mock_process_refund ZENDESK_URL = 'http://zendesk.example.com/' ZENDESK_USER = 'test@example.com' @@ -320,3 +321,139 @@ class TestRefundSignal(TestCase): } } self.assertDictEqual(json.loads(last_request.body), expected) + + +@override_settings(ZENDESK_URL=ZENDESK_URL, ZENDESK_USER=ZENDESK_USER, ZENDESK_API_KEY=ZENDESK_API_KEY) +class TestRevokeEntitlementSignal(TestCase): + """ + Exercises logic triggered by the REVOKE_ENTITLEMENT signal. + """ + + def setUp(self): + super(TestRevokeEntitlementSignal, self).setUp() + + # Ensure the E-Commerce service user exists + UserFactory(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME, is_staff=True) + + self.requester = UserFactory(username="test-requester") + self.student = UserFactory( + username="test-student", + email="test-student@example.com", + ) + self.course_entitlement = CourseEntitlementFactory( + user=self.student, + mode=CourseMode.VERIFIED + ) + + self.config = CommerceConfiguration.current() + self.config.enable_automatic_refund_approval = True + self.config.save() + + def send_signal(self): + """ + DRY helper: emit the REVOKE_ENTITLEMENT signal, as is done in + common.djangoapps.entitlements.views after a successful unenrollment and revoke of the entitlement. + """ + REFUND_ENTITLEMENT.send(sender=None, course_entitlement=self.course_entitlement) + + @override_settings( + ECOMMERCE_PUBLIC_URL_ROOT=None, + ECOMMERCE_API_URL=None, + ) + def test_no_service(self): + """ + Ensure that the receiver quietly bypasses attempts to initiate + refunds when there is no external service configured. + """ + with mock.patch('lms.djangoapps.commerce.signals.refund_seat') as mock_refund_entitlement: + self.send_signal() + self.assertFalse(mock_refund_entitlement.called) + + @mock.patch('lms.djangoapps.commerce.signals.get_request_user') + @mock.patch('lms.djangoapps.commerce.signals.refund_entitlement') + def test_receiver(self, mock_refund_entitlement, mock_get_user): + """ + Ensure that the REVOKE_ENTITLEMENT signal triggers correct calls to + refund_entitlement(), when it is appropriate to do so. + """ + mock_get_user.return_value = self.student + self.send_signal() + self.assertTrue(mock_refund_entitlement.called) + self.assertEqual(mock_refund_entitlement.call_args[0], (self.course_entitlement,)) + + # if the course_entitlement is not refundable, we should not try to initiate a refund. + mock_refund_entitlement.reset_mock() + self.course_entitlement.is_entitlement_refundable = mock.Mock(return_value=False) + self.send_signal() + self.assertFalse(mock_refund_entitlement.called) + + @mock.patch('lms.djangoapps.commerce.signals.refund_entitlement') + @mock.patch('lms.djangoapps.commerce.signals.get_request_user', return_value=None) + def test_requester(self, mock_get_request_user, mock_refund_entitlement): + """ + Ensure the right requester is specified when initiating refunds. + """ + # no HTTP request/user: No Refund called. + self.send_signal() + self.assertFalse(mock_refund_entitlement.called) + + # HTTP user is the student: auth to commerce service as the unenrolled student and refund. + mock_get_request_user.return_value = self.student + mock_refund_entitlement.reset_mock() + self.send_signal() + self.assertTrue(mock_refund_entitlement.called) + self.assertEqual(mock_refund_entitlement.call_args[0], (self.course_entitlement,)) + + # HTTP user is another user: No refund invalid user. + mock_get_request_user.return_value = self.requester + mock_refund_entitlement.reset_mock() + self.send_signal() + self.assertFalse(mock_refund_entitlement.called) + + # HTTP user is another server (AnonymousUser): do not try to initiate a refund at all. + mock_get_request_user.return_value = AnonymousUser() + mock_refund_entitlement.reset_mock() + self.send_signal() + self.assertFalse(mock_refund_entitlement.called) + + @mock.patch('lms.djangoapps.commerce.signals.get_request_user',) + @mock.patch('lms.djangoapps.commerce.signals.send_refund_notification') + def test_notification_when_approval_fails(self, mock_send_notification, mock_get_user): + """ + Ensure the notification function is triggered when refunds are initiated, and cannot be automatically approved. + """ + refund_id = 1 + failed_refund_id = 2 + + with mock_create_refund(status=201, response=[refund_id, failed_refund_id]): + with mock_process_refund(refund_id, reset_on_exit=False): + with mock_process_refund(failed_refund_id, status=500, reset_on_exit=False): + mock_get_user.return_value = self.student + self.send_signal() + self.assertTrue(mock_send_notification.called) + mock_send_notification.assert_called_with(self.course_entitlement, [failed_refund_id]) + + @mock.patch('lms.djangoapps.commerce.signals.get_request_user') + @mock.patch('lms.djangoapps.commerce.signals.send_refund_notification') + def test_notification_if_automatic_approval_disabled(self, mock_send_notification, mock_get_user): + """ + Ensure the notification is always sent if the automatic approval functionality is disabled. + """ + refund_id = 1 + self.config.enable_automatic_refund_approval = False + self.config.save() + + with mock_create_refund(status=201, response=[refund_id]): + mock_get_user.return_value = self.student + self.send_signal() + self.assertTrue(mock_send_notification.called) + mock_send_notification.assert_called_with(self.course_entitlement, [refund_id]) + + @mock.patch('openedx.core.djangoapps.theming.helpers.is_request_in_themed_site', return_value=True) + def test_notification_themed_site(self, mock_is_request_in_themed_site): # pylint: disable=unused-argument + """ + Ensure the notification function raises an Exception if used in the + context of themed site. + """ + with self.assertRaises(NotImplementedError): + send_refund_notification(self.course_entitlement, [1, 2, 3])