From 2849d8f5722d53444d187cdbdb15b02fe23ec18b Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Tue, 17 Jul 2018 19:05:14 +0500 Subject: [PATCH] Fix unenroll entitlement from instructor dashboard. LEARNER-5620 --- .../entitlements/api/v1/tests/test_views.py | 30 +++++++++---------- .../djangoapps/entitlements/api/v1/views.py | 12 +------- common/djangoapps/entitlements/apps.py | 20 +++++++++++++ common/djangoapps/entitlements/models.py | 29 +++++++++++++++++- common/djangoapps/entitlements/signals.py | 16 ++++++++++ lms/envs/common.py | 2 +- 6 files changed, 81 insertions(+), 28 deletions(-) create mode 100644 common/djangoapps/entitlements/apps.py create mode 100644 common/djangoapps/entitlements/signals.py diff --git a/common/djangoapps/entitlements/api/v1/tests/test_views.py b/common/djangoapps/entitlements/api/v1/tests/test_views.py index 0135aa7264..a52c248a37 100644 --- a/common/djangoapps/entitlements/api/v1/tests/test_views.py +++ b/common/djangoapps/entitlements/api/v1/tests/test_views.py @@ -572,16 +572,13 @@ class EntitlementViewSetTest(ModuleStoreTestCase): course_entitlement.refresh_from_db() assert course_entitlement.expired_at is not None - def test_revoke_unenroll_entitlement(self): - course_entitlement = CourseEntitlementFactory.create() + @patch("entitlements.models.get_course_uuid_for_course") + def test_revoke_unenroll_entitlement(self, mock_course_uuid): + enrollment = CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id, is_active=True) + course_entitlement = CourseEntitlementFactory.create(user=self.user, enrollment_course_run=enrollment) + mock_course_uuid.return_value = course_entitlement.course_uuid url = reverse(self.ENTITLEMENTS_DETAILS_PATH, args=[str(course_entitlement.uuid)]) - enrollment = CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) - - course_entitlement.refresh_from_db() - course_entitlement.enrollment_course_run = enrollment - course_entitlement.save() - assert course_entitlement.enrollment_course_run is not None response = self.client.delete( @@ -754,10 +751,12 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase): assert CourseEnrollment.is_enrolled(self.user, self.course.id) assert course_entitlement.enrollment_course_run is not None + @patch("entitlements.models.get_course_uuid_for_course") @patch("entitlements.api.v1.views.get_course_runs_for_course") - def test_user_can_unenroll(self, mock_get_course_runs): + def test_user_can_unenroll(self, mock_get_course_runs, mock_get_course_uuid): course_entitlement = CourseEntitlementFactory.create(user=self.user, mode=CourseMode.VERIFIED) mock_get_course_runs.return_value = self.return_values + mock_get_course_uuid.return_value = course_entitlement.course_uuid url = reverse( self.ENTITLEMENTS_ENROLLMENT_NAMESPACE, @@ -904,11 +903,13 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase): assert response.data['message'] == expected_message # pylint: disable=no-member assert not CourseEnrollment.is_enrolled(self.user, fake_course_key) - @patch('entitlements.api.v1.views.refund_entitlement', return_value=True) + @patch('entitlements.models.refund_entitlement', return_value=True) @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): + @patch("entitlements.models.get_course_uuid_for_course") + def test_user_can_revoke_and_refund(self, mock_course_uuid, mock_get_course_runs, mock_refund_entitlement): course_entitlement = CourseEntitlementFactory.create(user=self.user, mode=CourseMode.VERIFIED) mock_get_course_runs.return_value = self.return_values + mock_course_uuid.return_value = course_entitlement.course_uuid url = reverse( self.ENTITLEMENTS_ENROLLMENT_NAMESPACE, @@ -939,14 +940,13 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase): course_entitlement.refresh_from_db() assert mock_refund_entitlement.is_called - assert (CourseEntitlementSerializer(mock_refund_entitlement.call_args[1]['course_entitlement']).data == - CourseEntitlementSerializer(course_entitlement).data) + assert mock_refund_entitlement.call_args[1]['course_entitlement'] == course_entitlement 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 @patch('entitlements.api.v1.views.CourseEntitlement.is_entitlement_refundable', return_value=False) - @patch('entitlements.api.v1.views.refund_entitlement', return_value=True) + @patch('entitlements.models.refund_entitlement', return_value=True) @patch('entitlements.api.v1.views.get_course_runs_for_course') def test_user_can_revoke_and_no_refund_available( self, @@ -990,7 +990,7 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase): assert course_entitlement.expired_at is None @patch('entitlements.api.v1.views.CourseEntitlement.is_entitlement_refundable', return_value=True) - @patch('entitlements.api.v1.views.refund_entitlement', return_value=False) + @patch('entitlements.models.refund_entitlement', return_value=False) @patch("entitlements.api.v1.views.get_course_runs_for_course") def test_user_is_not_unenrolled_on_failed_refund( self, diff --git a/common/djangoapps/entitlements/api/v1/views.py b/common/djangoapps/entitlements/api/v1/views.py index 3af1cb69a4..8914365c9e 100644 --- a/common/djangoapps/entitlements/api/v1/views.py +++ b/common/djangoapps/entitlements/api/v1/views.py @@ -19,7 +19,6 @@ from entitlements.api.v1.permissions import IsAdminOrSupportOrAuthenticatedReadO from entitlements.api.v1.serializers import CourseEntitlementSerializer from entitlements.models import CourseEntitlement, CourseEntitlementPolicy, CourseEntitlementSupportDetail from entitlements.utils import is_course_run_entitlement_fulfillable -from lms.djangoapps.commerce.utils import refund_entitlement from openedx.core.djangoapps.catalog.utils import get_course_runs_for_course from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from openedx.core.djangoapps.cors_csrf.authentication import SessionAuthenticationCrossDomainCsrf @@ -42,7 +41,6 @@ def _unenroll_entitlement(course_entitlement, course_run_key): Internal method to handle the details of Unenrolling a User in a Course Run. """ CourseEnrollment.unenroll(course_entitlement.user, course_run_key, skip_refund=True) - course_entitlement.set_enrollment(None) @transaction.atomic @@ -78,15 +76,7 @@ def _process_revoke_and_unenroll_entitlement(course_entitlement, is_refund=False ) if is_refund: - refund_successful = refund_entitlement(course_entitlement=course_entitlement) - if not refund_successful: - # This state is achieved in most cases by a failure in the ecommerce service to process the refund. - log.warn( - 'Entitlement Refund failed for Course Entitlement [%s], alert User', - course_entitlement.uuid - ) - # Force Transaction reset with an Integrity error exception, this will revert all previous transactions - raise IntegrityError + course_entitlement.refund() def set_entitlement_policy(entitlement, site): diff --git a/common/djangoapps/entitlements/apps.py b/common/djangoapps/entitlements/apps.py new file mode 100644 index 0000000000..5e748da974 --- /dev/null +++ b/common/djangoapps/entitlements/apps.py @@ -0,0 +1,20 @@ +""" +Entitlements Application Configuration + +Signal handlers are connected here. +""" + +from django.apps import AppConfig + + +class EntitlementsConfig(AppConfig): + """ + Application Configuration for Entitlements. + """ + name = u'entitlements' + + def ready(self): + """ + Connect handlers to signals. + """ + from . import signals # pylint: disable=unused-variable diff --git a/common/djangoapps/entitlements/models.py b/common/djangoapps/entitlements/models.py index bf2f3cd979..a56d3a00f7 100644 --- a/common/djangoapps/entitlements/models.py +++ b/common/djangoapps/entitlements/models.py @@ -6,7 +6,7 @@ from datetime import timedelta from django.conf import settings from django.contrib.sites.models import Site -from django.db import models, transaction +from django.db import IntegrityError, models, transaction from django.utils.timezone import now from model_utils import Choices from model_utils.models import TimeStampedModel @@ -14,6 +14,7 @@ from model_utils.models import TimeStampedModel from course_modes.models import CourseMode from entitlements.utils import is_course_run_entitlement_fulfillable from lms.djangoapps.certificates.models import GeneratedCertificate +from lms.djangoapps.commerce.utils import refund_entitlement from openedx.core.djangoapps.catalog.utils import get_course_uuid_for_course from openedx.core.djangoapps.content.course_overviews.models import CourseOverview from student.models import CourseEnrollment, CourseEnrollmentException @@ -404,6 +405,32 @@ class CourseEntitlement(TimeStampedModel): return cls.enroll_user_and_fulfill_entitlement(entitlement, course_run_key) return False + @classmethod + def unenroll_entitlement(cls, course_enrollment, skip_refund): + """ + Un-enroll the user from entitlement and refund. + """ + course_uuid = get_course_uuid_for_course(course_enrollment.course_id) + course_entitlement = cls.get_entitlement_if_active(course_enrollment.user, course_uuid) + if course_entitlement: + course_entitlement.set_enrollment(None) + if not skip_refund and course_entitlement.is_entitlement_refundable(): + course_entitlement.refund() + + def refund(self): + """ + Initiate refund process for the entitlement. + """ + refund_successful = refund_entitlement(course_entitlement=self) + if not refund_successful: + # This state is achieved in most cases by a failure in the ecommerce service to process the refund. + log.warn( + 'Entitlement Refund failed for Course Entitlement [%s], alert User', + self.uuid + ) + # Force Transaction reset with an Integrity error exception, this will revert all previous transactions + raise IntegrityError + class CourseEntitlementSupportDetail(TimeStampedModel): """ diff --git a/common/djangoapps/entitlements/signals.py b/common/djangoapps/entitlements/signals.py new file mode 100644 index 0000000000..8cc245f473 --- /dev/null +++ b/common/djangoapps/entitlements/signals.py @@ -0,0 +1,16 @@ +""" +Entitlements related signal handlers. +""" + +from django.dispatch import receiver + +from entitlements.models import CourseEntitlement +from student.signals import UNENROLL_DONE + + +@receiver(UNENROLL_DONE) +def unenroll_entitlement(sender, course_enrollment=None, skip_refund=False, **kwargs): # pylint: disable=unused-argument + """ + Un-enroll user from entitlement upon course run un-enrollment if exist. + """ + CourseEntitlement.unenroll_entitlement(course_enrollment, skip_refund) diff --git a/lms/envs/common.py b/lms/envs/common.py index 7c9aacee30..62390c3bff 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2180,7 +2180,7 @@ INSTALLED_APPS = [ 'enrollment', # Entitlement API - 'entitlements', + 'entitlements.apps.EntitlementsConfig', # Bulk Enrollment API 'bulk_enroll',