diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 96d325d1a7..b4ce13cf80 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -60,6 +60,7 @@ from util.query import use_read_replica_if_available UNENROLL_DONE = Signal(providing_args=["course_enrollment", "skip_refund"]) ENROLL_STATUS_CHANGE = Signal(providing_args=["event", "user", "course_id", "mode", "cost", "currency"]) +REFUND_ORDER = Signal(providing_args=["course_enrollment"]) log = logging.getLogger(__name__) AUDIT_LOG = logging.getLogger("audit") SessionStore = import_module(settings.SESSION_ENGINE).SessionStore # pylint: disable=invalid-name diff --git a/common/djangoapps/student/tests/test_views.py b/common/djangoapps/student/tests/test_views.py index 5cd9a8d340..a5cd234665 100644 --- a/common/djangoapps/student/tests/test_views.py +++ b/common/djangoapps/student/tests/test_views.py @@ -18,7 +18,7 @@ from pyquery import PyQuery as pq from student.cookies import get_user_info_cookie_data from student.helpers import DISABLE_UNENROLL_CERT_STATES -from student.models import CourseEnrollment, UserProfile +from student.models import CourseEnrollment, REFUND_ORDER, UserProfile from student.tests.factories import CourseEnrollmentFactory, UserFactory from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase @@ -91,16 +91,19 @@ class TestStudentDashboardUnenrollments(SharedModuleStoreTestCase): self.cert_status = cert_status with patch('student.views.cert_info', side_effect=self.mock_cert): - response = self.client.post( - reverse('change_enrollment'), - {'enrollment_action': 'unenroll', 'course_id': self.course.id} - ) + with patch('commerce.signals.handle_refund_order') as mock_refund_handler: + REFUND_ORDER.connect(mock_refund_handler) + response = self.client.post( + reverse('change_enrollment'), + {'enrollment_action': 'unenroll', 'course_id': self.course.id} + ) - self.assertEqual(response.status_code, status_code) - if status_code == 200: - course_enrollment.assert_called_with(self.user, self.course.id) - else: - course_enrollment.assert_not_called() + self.assertEqual(response.status_code, status_code) + if status_code == 200: + course_enrollment.assert_called_with(self.user, self.course.id) + self.assertTrue(mock_refund_handler.called) + else: + course_enrollment.assert_not_called() def test_no_cert_status(self): """ Assert that the dashboard loads when cert_status is None.""" diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index f871ee08ee..5166592aad 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -119,7 +119,8 @@ from student.models import ( UserStanding, anonymous_id_for_user, create_comments_service_user, - unique_id_for_user + unique_id_for_user, + REFUND_ORDER ) from student.tasks import send_activation_email from third_party_auth import pipeline, provider @@ -1267,6 +1268,7 @@ def change_enrollment(request, check_access=True): return HttpResponseBadRequest(_("Your certificate prevents you from unenrolling from this course")) CourseEnrollment.unenroll(user, course_id) + REFUND_ORDER.send(sender=None, course_enrollment=enrollment) return HttpResponse() else: return HttpResponseBadRequest(_("Enrollment action is invalid")) diff --git a/lms/djangoapps/commerce/signals.py b/lms/djangoapps/commerce/signals.py index b288553b09..a9c8feb62e 100644 --- a/lms/djangoapps/commerce/signals.py +++ b/lms/djangoapps/commerce/signals.py @@ -19,21 +19,19 @@ from openedx.core.djangoapps.commerce.utils import ecommerce_api_client, is_comm 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.models import UNENROLL_DONE +from student.models import REFUND_ORDER log = logging.getLogger(__name__) # pylint: disable=unused-argument -@receiver(UNENROLL_DONE) -def handle_unenroll_done(sender, course_enrollment=None, skip_refund=False, **kwargs): +@receiver(REFUND_ORDER) +def handle_refund_order(sender, course_enrollment=None, **kwargs): """ Signal receiver for unenrollments, used to automatically initiate refunds when applicable. - - N.B. this signal is also consumed by lms.djangoapps.shoppingcart. """ - if not is_commerce_service_configured() or skip_refund: + if not is_commerce_service_configured(): return if course_enrollment and course_enrollment.refundable(): diff --git a/lms/djangoapps/commerce/tests/test_signals.py b/lms/djangoapps/commerce/tests/test_signals.py index e90d45512d..2954f22d2b 100644 --- a/lms/djangoapps/commerce/tests/test_signals.py +++ b/lms/djangoapps/commerce/tests/test_signals.py @@ -23,7 +23,7 @@ from commerce.signals import create_zendesk_ticket, generate_refund_notification from commerce.tests import JSON from commerce.tests.mocks import mock_create_refund, mock_process_refund from course_modes.models import CourseMode -from student.models import UNENROLL_DONE +from student.models import REFUND_ORDER from student.tests.factories import CourseEnrollmentFactory, UserFactory ZENDESK_URL = 'http://zendesk.example.com/' @@ -35,7 +35,7 @@ ZENDESK_API_KEY = 'abc123' @override_settings(ZENDESK_URL=ZENDESK_URL, ZENDESK_USER=ZENDESK_USER, ZENDESK_API_KEY=ZENDESK_API_KEY) class TestRefundSignal(TestCase): """ - Exercises logic triggered by the UNENROLL_DONE signal. + Exercises logic triggered by the REFUND_ORDER signal. """ def setUp(self): @@ -60,12 +60,12 @@ class TestRefundSignal(TestCase): self.config.enable_automatic_refund_approval = True self.config.save() - def send_signal(self, skip_refund=False): + def send_signal(self): """ - DRY helper: emit the UNENROLL_DONE signal, as is done in + DRY helper: emit the REFUND_ORDER signal, as is done in common.djangoapps.student.models after a successful unenrollment. """ - UNENROLL_DONE.send(sender=None, course_enrollment=self.course_enrollment, skip_refund=skip_refund) + REFUND_ORDER.send(sender=None, course_enrollment=self.course_enrollment) @override_settings( ECOMMERCE_PUBLIC_URL_ROOT=None, @@ -83,7 +83,7 @@ class TestRefundSignal(TestCase): @mock.patch('commerce.signals.refund_seat') def test_receiver(self, mock_refund_seat): """ - Ensure that the UNENROLL_DONE signal triggers correct calls to + Ensure that the REFUND_ORDER signal triggers correct calls to refund_seat(), when it is appropriate to do so. TODO (jsa): ideally we would assert that the signal receiver got wired @@ -94,11 +94,6 @@ class TestRefundSignal(TestCase): self.assertTrue(mock_refund_seat.called) self.assertEqual(mock_refund_seat.call_args[0], (self.course_enrollment,)) - # if skip_refund is set to True in the signal, we should not try to initiate a refund. - mock_refund_seat.reset_mock() - self.send_signal(skip_refund=True) - self.assertFalse(mock_refund_seat.called) - # if the course_enrollment is not refundable, we should not try to initiate a refund. mock_refund_seat.reset_mock() self.course_enrollment.refundable = mock.Mock(return_value=False)