Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #16934 from edx/aj/LEARNER-3629_refund_api_refactor

Browse Source 
Refactor of the CourseEntitlement Refund API to handle refund failures
This commit is contained in:
Albert (AJ) St. Aubin
2018-01-04 12:16:35 -05:00
committed by GitHub
parent 64b7fb88b8 417ef0583b
commit 14ddb59f90
8 changed files with 613 additions and 517 deletions
Download Patch File Download Diff File Expand all files Collapse all files

108
common/djangoapps/entitlements/api/v1/tests/test_views.py
View File

@@ -25,7 +25,6 @@ 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')
@@ -345,6 +344,8 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase):
def setUp(self):
super(EntitlementEnrollmentViewSetTest, self).setUp()
self.user = UserFactory()
UserFactory(username=settings.ECOMMERCE_SERVICE_WORKER_USERNAME, is_staff=True)
self.client.login(username=self.user.username, password=TEST_PASSWORD)
self.course = CourseFactory.create(org='edX', number='DemoX', display_name='Demo_Course')
self.course2 = CourseFactory.create(org='edX', number='DemoX2', display_name='Demo_Course 2')
@@ -504,8 +505,8 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase):
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")
@patch('entitlements.api.v1.views.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):
course_entitlement = CourseEntitlementFactory.create(user=self.user)
mock_get_course_runs.return_value = self.return_values
@@ -530,28 +531,24 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase):
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)
revoke_url = url + '?is_refund=true'
response = self.client.delete(
revoke_url,
content_type='application/json',
)
assert response.status_code == 204
# 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
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 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('lms.djangoapps.commerce.signals.refund_entitlement', return_value=[1])
@patch("entitlements.api.v1.views.get_course_runs_for_course")
@patch('entitlements.api.v1.views.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,
mock_get_course_runs,
@@ -581,18 +578,59 @@ class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase):
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)
revoke_url = url + '?is_refund=true'
response = self.client.delete(
revoke_url,
content_type='application/json',
)
assert response.status_code == 400
revoke_url = url + '?is_refund=true'
response = self.client.delete(
revoke_url,
content_type='application/json',
)
assert response.status_code == 400
course_entitlement.refresh_from_db()
assert CourseEnrollment.is_enrolled(self.user, self.course.id)
assert course_entitlement.enrollment_course_run is not None
assert course_entitlement.expired_at is None
course_entitlement.refresh_from_db()
assert not mock_refund_handler.called
assert CourseEnrollment.is_enrolled(self.user, self.course.id)
assert course_entitlement.enrollment_course_run is not None
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.api.v1.views.get_course_runs_for_course")
def test_user_is_not_unenrolled_on_failed_refund(
self,
mock_get_course_runs,
mock_refund_entitlement,
mock_is_refundable
):
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
# Enroll the User
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
revoke_url = url + '?is_refund=true'
response = self.client.delete(
revoke_url,
content_type='application/json',
)
assert response.status_code == 500
course_entitlement.refresh_from_db()
assert CourseEnrollment.is_enrolled(self.user, self.course.id)
assert course_entitlement.enrollment_course_run is not None
assert course_entitlement.expired_at is None

155
common/djangoapps/entitlements/api/v1/views.py
View File

@@ -1,6 +1,6 @@
import logging
from django.db import transaction
from django.db import IntegrityError, transaction
from django.utils import timezone
from django_filters.rest_framework import DjangoFilterBackend
from edx_rest_framework_extensions.authentication import JwtAuthentication
@@ -14,7 +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 lms.djangoapps.commerce.utils 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
@@ -23,6 +23,59 @@ from student.models import CourseEnrollmentException, AlreadyEnrolledError
log = logging.getLogger(__name__)
@transaction.atomic
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
def _process_revoke_and_unenroll_entitlement(course_entitlement, is_refund=False):
"""
Process the revoke of the Course Entitlement and refund if needed
Arguments:
course_entitlement: Course Entitlement Object
is_refund (bool): True if a refund should be processed
Exceptions:
IntegrityError if there is an issue that should reverse the database changes
"""
if course_entitlement.expired_at is None:
course_entitlement.expired_at = timezone.now()
log.info(
'Set expired_at to [%s] for course entitlement [%s]',
course_entitlement.expired_at,
course_entitlement.uuid
)
course_entitlement.save()
if course_entitlement.enrollment_course_run is not None:
course_id = course_entitlement.enrollment_course_run.course_id
_unenroll_entitlement(course_entitlement, course_id)
log.info(
'Unenrolled user [%s] from course run [%s] as part of revocation of course entitlement [%s]',
course_entitlement.user.username,
course_id,
course_entitlement.uuid
)
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
class EntitlementViewSet(viewsets.ModelViewSet):
ENTITLEMENT_UUID4_REGEX = '[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}'
@@ -105,7 +158,10 @@ class EntitlementViewSet(viewsets.ModelViewSet):
headers = self.get_success_headers(serializer.data)
# Note, the entitlement is re-serialized before getting added to the Response,
# so that the 'modified' date reflects changes that occur when upgrading enrollment.
return Response(CourseEntitlementSerializer(entitlement).data, status=status.HTTP_201_CREATED, headers=headers)
return Response(
CourseEntitlementSerializer(entitlement).data,
status=status.HTTP_201_CREATED, headers=headers
)
def retrieve(self, request, *args, **kwargs):
"""
@@ -139,31 +195,19 @@ class EntitlementViewSet(viewsets.ModelViewSet):
def perform_destroy(self, instance):
"""
This method is an override and is called by the DELETE method
"""
save_model = False
if instance.expired_at is None:
instance.expired_at = timezone.now()
log.info('Set expired_at to [%s] for course entitlement [%s]', instance.expired_at, instance.uuid)
save_model = True
This method is an override and is called by the destroy method, which is called when a DELETE operation occurs
if instance.enrollment_course_run is not None:
CourseEnrollment.unenroll(
user=instance.user,
course_id=instance.enrollment_course_run.course_id,
skip_refund=True
)
enrollment = instance.enrollment_course_run
instance.enrollment_course_run = None
save_model = True
log.info(
'Unenrolled user [%s] from course run [%s] as part of revocation of course entitlement [%s]',
instance.user.username,
enrollment.course_id,
instance.uuid
)
if save_model:
instance.save()
This method will revoke the User's entitlement and unenroll the user if they are enrolled
in a Course Run
It is assumed the user has already been refunded.
"""
log.info(
'Entitlement Revoke requested for Course Entitlement[%s]',
instance.uuid
)
# This is not called with is_refund=True here because it is assumed the user has already been refunded.
_process_revoke_and_unenroll_entitlement(instance)
class EntitlementEnrollmentViewSet(viewsets.GenericViewSet):
@@ -188,6 +232,7 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet):
return True
return False
@transaction.atomic
def _enroll_entitlement(self, entitlement, course_run_key, user):
"""
Internal method to handle the details of enrolling a User in a Course Run.
@@ -225,13 +270,6 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet):
entitlement.set_enrollment(enrollment)
return None
def _unenroll_entitlement(self, entitlement, course_run_key, user):
"""
Internal method to handle the details of Unenrolling a User in a Course Run.
"""
CourseEnrollment.unenroll(user, course_run_key, skip_refund=True)
entitlement.set_enrollment(None)
def create(self, request, uuid):
"""
On POST this method will be called and will handle enrolling a user in the
@@ -289,10 +327,9 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet):
if response:
return response
elif entitlement.enrollment_course_run.course_id != course_run_id:
self._unenroll_entitlement(
entitlement=entitlement,
_unenroll_entitlement(
course_entitlement=entitlement,
course_run_key=entitlement.enrollment_course_run.course_id,
user=request.user
)
response = self._enroll_entitlement(
entitlement=entitlement,
@@ -328,41 +365,33 @@ class EntitlementEnrollmentViewSet(viewsets.GenericViewSet):
)
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]',
entitlement.uuid
)
# Revoke the Course Entitlement and issue Refund
log.info(
'Entitlement Refund requested for Course Entitlement[%s]',
str(entitlement.uuid)
)
try:
_process_revoke_and_unenroll_entitlement(course_entitlement=entitlement, is_refund=True)
except IntegrityError:
# This state is reached when there was a failure in revoke and refund process resulting
# in a reversion of DB changes
return Response(
status=status.HTTP_500_INTERNAL_SERVER_ERROR,
data={
'message': 'Entitlement revoke and refund failed due to refund internal process failure'
})
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,
_unenroll_entitlement(
course_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)
entitlement.uuid
)
return Response(
status=status.HTTP_400_BAD_REQUEST,

6
common/djangoapps/entitlements/signals.py
View File

@@ -1,6 +0,0 @@
"""
Enrollment track related signals.
"""
from django.dispatch import Signal
REFUND_ENTITLEMENT = Signal(providing_args=['course_entitlement'])