From e5d1f04a897bdb16c3d2287e390fea04c1c93aa7 Mon Sep 17 00:00:00 2001 From: McKenzie Welter Date: Wed, 7 Mar 2018 12:30:12 -0500 Subject: [PATCH] move support actions to existing entitlement api --- .../entitlements/api/v1/permissions.py | 10 +- .../entitlements/api/v1/serializers.py | 31 ++--- .../api/v1/tests/test_serializers.py | 1 + .../entitlements/api/v1/tests/test_views.py | 63 ++++++++- .../djangoapps/entitlements/api/v1/views.py | 127 ++++++++++++------ common/djangoapps/entitlements/models.py | 17 --- lms/djangoapps/support/tests/test_views.py | 5 - lms/djangoapps/support/urls.py | 2 +- .../support/views/course_entitlements.py | 108 --------------- 9 files changed, 164 insertions(+), 200 deletions(-) diff --git a/common/djangoapps/entitlements/api/v1/permissions.py b/common/djangoapps/entitlements/api/v1/permissions.py index 16361f3592..d5271a69da 100644 --- a/common/djangoapps/entitlements/api/v1/permissions.py +++ b/common/djangoapps/entitlements/api/v1/permissions.py @@ -5,15 +5,17 @@ requiring Superuser access for all other Request types on an API endpoint. from rest_framework.permissions import BasePermission, SAFE_METHODS +from courseware.access import has_access -class IsAdminOrAuthenticatedReadOnly(BasePermission): + +class IsAdminOrSupportOrAuthenticatedReadOnly(BasePermission): """ - Method that will require staff access for all methods not + Method that will require admin or support access for all methods not in the SAFE_METHODS list. For example GET requests will not - require a Staff or Admin user. + require an Admin or Support user. """ def has_permission(self, request, view): if request.method in SAFE_METHODS: return request.user.is_authenticated else: - return request.user.is_staff + return request.user.is_staff or has_access(request.user, "support", "global") diff --git a/common/djangoapps/entitlements/api/v1/serializers.py b/common/djangoapps/entitlements/api/v1/serializers.py index bb7660626e..9fbe4c32dc 100644 --- a/common/djangoapps/entitlements/api/v1/serializers.py +++ b/common/djangoapps/entitlements/api/v1/serializers.py @@ -15,6 +15,14 @@ class CourseEntitlementSerializer(serializers.ModelSerializer): source='enrollment_course_run.course_id', read_only=True ) + support_details = serializers.SerializerMethodField() + + def get_support_details(self, model): + """ + Returns a serialized set of all support interactions with the course entitlement + """ + qset = CourseEntitlementSupportDetail.objects.filter(entitlement=model).order_by('-created') + return CourseEntitlementSupportDetailSerializer(qset, many=True).data class Meta: model = CourseEntitlement @@ -27,7 +35,8 @@ class CourseEntitlementSerializer(serializers.ModelSerializer): 'created', 'modified', 'mode', - 'order_number' + 'order_number', + 'support_details' ) @@ -44,25 +53,7 @@ class CourseEntitlementSupportDetailSerializer(serializers.ModelSerializer): model = CourseEntitlementSupportDetail fields = ( 'support_user', - 'reason', + 'action', 'comments', 'unenrolled_run' ) - - -class SupportCourseEntitlementSerializer(CourseEntitlementSerializer): - """ - Serialize a learner's course entitlement with details from all support team interactions with that entitlement. - """ - support_details = serializers.SerializerMethodField() - - def get_support_details(self, model): - """ - Returns a serialized set of all support interactions with the course entitlement - """ - qset = CourseEntitlementSupportDetail.objects.filter(entitlement=model).order_by('-created') - return CourseEntitlementSupportDetailSerializer(qset, many=True).data - - class Meta: - model = CourseEntitlement - fields = CourseEntitlementSerializer.Meta.fields + ('support_details', ) diff --git a/common/djangoapps/entitlements/api/v1/tests/test_serializers.py b/common/djangoapps/entitlements/api/v1/tests/test_serializers.py index ff4c0a2de0..1073bd4c9c 100644 --- a/common/djangoapps/entitlements/api/v1/tests/test_serializers.py +++ b/common/djangoapps/entitlements/api/v1/tests/test_serializers.py @@ -31,6 +31,7 @@ class EntitlementsSerializerTests(ModuleStoreTestCase): 'order_number': entitlement.order_number, 'created': entitlement.created.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), 'modified': entitlement.modified.strftime('%Y-%m-%dT%H:%M:%S.%fZ'), + 'support_details': [], } assert serializer.data == expected diff --git a/common/djangoapps/entitlements/api/v1/tests/test_views.py b/common/djangoapps/entitlements/api/v1/tests/test_views.py index faf8742be4..8524214f57 100644 --- a/common/djangoapps/entitlements/api/v1/tests/test_views.py +++ b/common/djangoapps/entitlements/api/v1/tests/test_views.py @@ -2,7 +2,7 @@ import json import logging import unittest import uuid -from datetime import timedelta +from datetime import datetime, timedelta from django.conf import settings from django.core.urlresolvers import reverse @@ -53,7 +53,7 @@ class EntitlementViewSetTest(ModuleStoreTestCase): "user": user.username, "mode": "verified", "course_uuid": course_uuid, - "order_number": "EDX-1001" + "order_number": "EDX-1001", } def test_auth_required(self): @@ -124,6 +124,33 @@ class EntitlementViewSetTest(ModuleStoreTestCase): ) assert results == CourseEntitlementSerializer(course_entitlement).data + def test_add_entitlement_with_support_detail(self): + """ + Verify that an EntitlementSupportDetail entry is made when the request includes support interaction information. + """ + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + entitlement_data['support_details'] = [ + { + "action": "CREATE", + "comments": "Family emergency." + }, + ] + + response = self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + assert response.status_code == 201 + results = response.data + + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + assert results == CourseEntitlementSerializer(course_entitlement).data + @patch("entitlements.api.v1.views.get_course_runs_for_course") def test_add_entitlement_and_upgrade_audit_enrollment(self, mock_get_course_runs): """ @@ -333,6 +360,38 @@ class EntitlementViewSetTest(ModuleStoreTestCase): assert course_entitlement.expired_at is not None assert course_entitlement.enrollment_course_run is None + def test_reinstate_entitlement(self): + enrollment = CourseEnrollmentFactory(user=self.user, is_active=True) + expired_entitlement = CourseEntitlementFactory.create( + user=self.user, enrollment_course_run=enrollment, expired_at=datetime.now() + ) + url = reverse(self.ENTITLEMENTS_DETAILS_PATH, args=[str(expired_entitlement.uuid)]) + + update_data = { + 'expired_at': None, + 'enrollment_course_run': None, + 'support_details': [ + { + 'unenrolled_run': str(enrollment.course.id), + 'action': 'REISSUE', + 'comments': 'Severe illness.' + } + ] + } + + response = self.client.patch( + url, + data=json.dumps(update_data), + content_type='application/json' + ) + assert response.status_code == 200 + + results = response.data + reinstated_entitlement = CourseEntitlement.objects.get( + uuid=expired_entitlement.uuid + ) + assert results == CourseEntitlementSerializer(reinstated_entitlement).data + @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') class EntitlementEnrollmentViewSetTest(ModuleStoreTestCase): diff --git a/common/djangoapps/entitlements/api/v1/views.py b/common/djangoapps/entitlements/api/v1/views.py index 7c62c482ce..0577e8fc65 100644 --- a/common/djangoapps/entitlements/api/v1/views.py +++ b/common/djangoapps/entitlements/api/v1/views.py @@ -1,6 +1,7 @@ import logging from django.db import IntegrityError, transaction +from django.http import HttpResponseBadRequest from django.utils import timezone from django_filters.rest_framework import DjangoFilterBackend from edx_rest_framework_extensions.authentication import JwtAuthentication @@ -12,12 +13,13 @@ from rest_framework.authentication import SessionAuthentication from rest_framework.response import Response from entitlements.api.v1.filters import CourseEntitlementFilter -from entitlements.api.v1.permissions import IsAdminOrAuthenticatedReadOnly +from entitlements.api.v1.permissions import IsAdminOrSupportOrAuthenticatedReadOnly from entitlements.api.v1.serializers import CourseEntitlementSerializer -from entitlements.models import CourseEntitlement +from entitlements.models import CourseEntitlement, 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 from student.models import CourseEnrollment from student.models import CourseEnrollmentException, AlreadyEnrolledError @@ -91,7 +93,7 @@ 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}' authentication_classes = (JwtAuthentication, SessionAuthenticationCrossDomainCsrf,) - permission_classes = (permissions.IsAuthenticated, IsAdminOrAuthenticatedReadOnly,) + permission_classes = (permissions.IsAuthenticated, IsAdminOrSupportOrAuthenticatedReadOnly,) lookup_value_regex = ENTITLEMENT_UUID4_REGEX lookup_field = 'uuid' serializer_class = CourseEntitlementSerializer @@ -119,53 +121,61 @@ class EntitlementViewSet(viewsets.ModelViewSet): return CourseEntitlement.objects.all().select_related('user').select_related('enrollment_course_run') def create(self, request, *args, **kwargs): + support_details = request.data.pop('support_details', []) serializer = self.get_serializer(data=request.data) serializer.is_valid(raise_exception=True) self.perform_create(serializer) entitlement = serializer.instance - user = entitlement.user - # find all course_runs within the course - course_runs = get_course_runs_for_course(entitlement.course_uuid) - - # check if the user has enrollments for any of the course_runs - user_run_enrollments = [ - CourseEnrollment.get_enrollment(user, CourseKey.from_string(course_run.get('key'))) - for course_run - in course_runs - if CourseEnrollment.get_enrollment(user, CourseKey.from_string(course_run.get('key'))) - ] - - # filter to just enrollments that can be upgraded. - upgradeable_enrollments = [ - enrollment - for enrollment - in user_run_enrollments - if enrollment.is_active and enrollment.upgrade_deadline and enrollment.upgrade_deadline > timezone.now() - ] - - # if there is only one upgradeable enrollment, convert it from audit to the entitlement.mode - # if there is any ambiguity about which enrollment to upgrade - # (i.e. multiple upgradeable enrollments or no available upgradeable enrollment), dont enroll - if len(upgradeable_enrollments) == 1: - enrollment = upgradeable_enrollments[0] - log.info( - 'Upgrading enrollment [%s] from %s to %s while adding entitlement for user [%s] for course [%s]', - enrollment, - enrollment.mode, - serializer.data.get('mode'), - user.username, - serializer.data.get('course_uuid') - ) - enrollment.update_enrollment(mode=entitlement.mode) - entitlement.set_enrollment(enrollment) + if support_details: + for support_detail in support_details: + support_detail['entitlement'] = entitlement + support_detail['support_user'] = request.user + CourseEntitlementSupportDetail.objects.create(**support_detail) else: - log.info( - 'No enrollment upgraded while adding entitlement for user [%s] for course [%s] ', - user.username, - serializer.data.get('course_uuid') - ) + user = entitlement.user + + # find all course_runs within the course + course_runs = get_course_runs_for_course(entitlement.course_uuid) + + # check if the user has enrollments for any of the course_runs + user_run_enrollments = [ + CourseEnrollment.get_enrollment(user, CourseKey.from_string(course_run.get('key'))) + for course_run + in course_runs + if CourseEnrollment.get_enrollment(user, CourseKey.from_string(course_run.get('key'))) + ] + + # filter to just enrollments that can be upgraded. + upgradeable_enrollments = [ + enrollment + for enrollment + in user_run_enrollments + if enrollment.is_active and enrollment.upgrade_deadline and enrollment.upgrade_deadline > timezone.now() + ] + + # if there is only one upgradeable enrollment, convert it from audit to the entitlement.mode + # if there is any ambiguity about which enrollment to upgrade + # (i.e. multiple upgradeable enrollments or no available upgradeable enrollment), dont enroll + if len(upgradeable_enrollments) == 1: + enrollment = upgradeable_enrollments[0] + log.info( + 'Upgrading enrollment [%s] from %s to %s while adding entitlement for user [%s] for course [%s]', + enrollment, + enrollment.mode, + serializer.data.get('mode'), + user.username, + serializer.data.get('course_uuid') + ) + enrollment.update_enrollment(mode=entitlement.mode) + entitlement.set_enrollment(enrollment) + else: + log.info( + 'No enrollment upgraded while adding entitlement for user [%s] for course [%s] ', + user.username, + serializer.data.get('course_uuid') + ) headers = self.get_success_headers(serializer.data) # Note, the entitlement is re-serialized before getting added to the Response, @@ -221,6 +231,37 @@ class EntitlementViewSet(viewsets.ModelViewSet): # 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) + def partial_update(self, request, *args, **kwargs): + entitlement_uuid = kwargs.get('uuid', None) + + try: + entitlement = CourseEntitlement.objects.get(uuid=entitlement_uuid) + except CourseEntitlement.DoesNotExist: + return HttpResponseBadRequest( + u'Could not find entitlement {entitlement_uuid} to update'.format( + entitlement_uuid=entitlement_uuid + ) + ) + support_details = request.data.pop('support_details', []) + + for support_detail in support_details: + support_detail['entitlement'] = entitlement + support_detail['support_user'] = request.user + unenrolled_run_id = support_detail.get('unenrolled_run', None) + if unenrolled_run_id: + try: + unenrolled_run_course_key = CourseKey.from_string(unenrolled_run_id) + _unenroll_entitlement(entitlement, unenrolled_run_course_key) + support_detail['unenrolled_run'] = CourseOverview.objects.get(id=unenrolled_run_course_key) + except (InvalidKeyError, CourseOverview.DoesNotExist) as error: + return HttpResponseBadRequest( + u'Error raised while trying to unenroll user {user} from course run {course_id}: {error}' + .format(user=entitlement.user.username, course_id=unenrolled_run_id, error=error) + ) + CourseEntitlementSupportDetail.objects.create(**support_detail) + + return super(EntitlementViewSet, self).partial_update(request, *args, **kwargs) + class EntitlementEnrollmentViewSet(viewsets.GenericViewSet): """ diff --git a/common/djangoapps/entitlements/models.py b/common/djangoapps/entitlements/models.py index 5dc44362b4..5eaa07fab3 100644 --- a/common/djangoapps/entitlements/models.py +++ b/common/djangoapps/entitlements/models.py @@ -263,23 +263,6 @@ class CourseEntitlement(TimeStampedModel): self.enrollment_course_run = enrollment self.save() - def reinstate(self): - """ - Unenrolls a user from the run in which they have spent the given entitlement and - sets the entitlement's expired_at date to null. - - Returns: - CourseOverview: course run from which the user has been unenrolled - """ - unenrolled_run = self.enrollment_course_run.course - self.expired_at = None - CourseEnrollment.unenroll( - user=self.enrollment_course_run.user, course_id=unenrolled_run.id, skip_refund=True - ) - self.enrollment_course_run = None - self.save() - return unenrolled_run - @classmethod def unexpired_entitlements_for_user(cls, user): return cls.objects.filter(user=user, expired_at=None).select_related('user') diff --git a/lms/djangoapps/support/tests/test_views.py b/lms/djangoapps/support/tests/test_views.py index 9e853c9081..587f112630 100644 --- a/lms/djangoapps/support/tests/test_views.py +++ b/lms/djangoapps/support/tests/test_views.py @@ -7,7 +7,6 @@ import itertools import json import re from datetime import datetime, timedelta -from uuid import uuid4 import ddt import pytest @@ -20,8 +19,6 @@ from pytz import UTC from common.test.utils import disable_signal from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory -from entitlements.models import CourseEntitlementSupportDetail -from entitlements.tests.factories import CourseEntitlementFactory from lms.djangoapps.verify_student.models import VerificationDeadline from student.models import ENROLLED_TO_ENROLLED, CourseEnrollment, ManualEnrollmentAudit from student.roles import GlobalStaff, SupportStaffRole @@ -111,7 +108,6 @@ class SupportViewAccessTests(SupportViewTestCase): 'support:enrollment_list', 'support:manage_user', 'support:manage_user_detail', - 'support:enrollment_list' ), ( (GlobalStaff, True), (SupportStaffRole, True), @@ -140,7 +136,6 @@ class SupportViewAccessTests(SupportViewTestCase): "support:enrollment_list", "support:manage_user", "support:manage_user_detail", - "support:enrollment_list" ) def test_require_login(self, url_name): url = reverse(url_name) diff --git a/lms/djangoapps/support/urls.py b/lms/djangoapps/support/urls.py index 494bfcb1a4..a959fe0a04 100644 --- a/lms/djangoapps/support/urls.py +++ b/lms/djangoapps/support/urls.py @@ -17,8 +17,8 @@ urlpatterns = [ url(r'^$', index, name="index"), url(r'^certificates/?$', CertificatesSupportView.as_view(), name="certificates"), url(r'^refund/?$', RefundSupportView.as_view(), name="refund"), - url(r'^course_entitlement/?$', COURSE_ENTITLEMENTS_VIEW, name="course_entitlement"), url(r'^enrollment/?$', EnrollmentSupportView.as_view(), name="enrollment"), + url(r'^course_entitlement/?$', COURSE_ENTITLEMENTS_VIEW, name="course_entitlement"), url(r'^contact_us/?$', ContactUsView.as_view(), name="contact_us"), url( r'^enrollment/(?P[\w.@+-]+)?$', diff --git a/lms/djangoapps/support/views/course_entitlements.py b/lms/djangoapps/support/views/course_entitlements.py index 6a93c96464..ce39d059c2 100644 --- a/lms/djangoapps/support/views/course_entitlements.py +++ b/lms/djangoapps/support/views/course_entitlements.py @@ -12,8 +12,6 @@ from rest_framework import permissions, status, viewsets from rest_framework.response import Response from edxmako.shortcuts import render_to_response -from entitlements.api.v1.permissions import IsAdminOrAuthenticatedReadOnly -from entitlements.api.v1.serializers import SupportCourseEntitlementSerializer from entitlements.models import CourseEntitlement, CourseEntitlementSupportDetail from lms.djangoapps.commerce.utils import EcommerceService from lms.djangoapps.support.decorators import require_support_permission @@ -40,109 +38,3 @@ class EntitlementSupportView(View): 'support_actions': support_actions } return render_to_response('support/entitlement.html', context) - - -class EntitlementSupportListView(viewsets.ModelViewSet): - """ - Allows viewing and changing learner course entitlements, used the support team. - """ - authentication_classes = (JwtAuthentication, SessionAuthenticationCrossDomainCsrf,) - permission_classes = (permissions.IsAuthenticated, IsAdminOrAuthenticatedReadOnly,) - queryset = CourseEntitlement.objects.all() - serializer_class = SupportCourseEntitlementSerializer - - @method_decorator(require_support_permission) - def list(self, request, username_or_email): # pylint: disable=unused-argument - """ - Returns a list of course entitlements for the given user, along with details of any - support team interactions with each of the course entitlements. - """ - try: - user = User.objects.get(Q(username=username_or_email) | Q(email=username_or_email)) - except User.DoesNotExist: - return Response([]) - - return Response(self.serializer_class(self.queryset.filter(user=user), many=True).data) - - @method_decorator(require_support_permission) - def update(self, request, username_or_email): # pylint: disable=unused-argument - """ Allows support staff to update an existing course entitlement. """ - support_user = request.user - entitlement_uuid = request.data.get('entitlement_uuid') - if not entitlement_uuid: - return HttpResponseBadRequest(u'The field {fieldname} is required.'.format(fieldname='entitlement_uuid')) - reason = request.data.get('reason') - if not reason: - return HttpResponseBadRequest(u'The field {fieldname} is required.'.format(fieldname='reason')) - comments = request.data.get('comments', None) - try: - entitlement = CourseEntitlement.objects.get(uuid=entitlement_uuid) - except CourseEntitlement.DoesNotExist: - return HttpResponseBadRequest( - u'Could not find entitlement {entitlement_uuid} for update'.format( - entitlement_uuid=entitlement_uuid - ) - ) - if reason == CourseEntitlementSupportDetail.LEAVE_SESSION: - return self._reinstate_entitlement(support_user, entitlement, comments) - - def _reinstate_entitlement(self, support_user, entitlement, comments): - """ Allows support staff to unexpire a user's entitlement.""" - if entitlement.enrollment_course_run is None: - return HttpResponseBadRequest( - u"Entitlement {entitlement} has not been spent on a course run.".format( - entitlement=entitlement - ) - ) - try: - with transaction.atomic(): - unenrolled_run = entitlement.reinstate() - CourseEntitlementSupportDetail.objects.create( - entitlement=entitlement, reason=CourseEntitlementSupportDetail.LEAVE_SESSION, comments=comments, - unenrolled_run=unenrolled_run, support_user=support_user - ) - return Response( - data=SupportCourseEntitlementSerializer(instance=entitlement).data - ) - except DatabaseError: - return HttpResponseBadRequest( - u'Failed to reinstate entitlement {entitlement}'.format(entitlement=entitlement)) - - @method_decorator(require_support_permission) - def create(self, request, username_or_email): # pylint: disable=arguments-differ - """ Allows support staff to grant a user a new entitlement for a course. """ - support_user = request.user - comments = request.data.get('comments', None) - - creation_fields = {} - missing_fields_string = '' - for field in REQUIRED_CREATION_FIELDS: - creation_fields[field] = request.data.get(field) - if not creation_fields.get(field): - missing_fields_string = missing_fields_string + ' ' + field - if missing_fields_string: - return HttpResponseBadRequest( - u'The following required fields are missing from the request:{missing_fields}'.format( - missing_fields=missing_fields_string - ) - ) - - try: - user = User.objects.get(Q(username=username_or_email) | Q(email=username_or_email)) - except User.DoesNotExist: - return HttpResponseBadRequest( - u'Could not find user {username_or_email}.'.format( - username_or_email=username_or_email, - ) - ) - - entitlement = CourseEntitlement.objects.create( - user=user, course_uuid=creation_fields['course_uuid'], mode=creation_fields['mode'] - ) - CourseEntitlementSupportDetail.objects.create( - entitlement=entitlement, reason=creation_fields['reason'], comments=comments, support_user=support_user - ) - return Response( - status=status.HTTP_201_CREATED, - data=SupportCourseEntitlementSerializer(instance=entitlement).data - )