From cc19e50590c6710b1a7de047cb2c3f6ade8377a5 Mon Sep 17 00:00:00 2001 From: McKenzie Welter Date: Wed, 21 Mar 2018 12:31:04 -0400 Subject: [PATCH] add mode field to entitlement policy model and set custom policy on entitlement create --- common/djangoapps/entitlements/admin.py | 4 +- .../entitlements/api/v1/tests/test_views.py | 192 +++++++++++++++++- .../djangoapps/entitlements/api/v1/views.py | 32 ++- .../migrations/0008_auto_20180328_1107.py | 24 +++ common/djangoapps/entitlements/models.py | 17 +- 5 files changed, 250 insertions(+), 19 deletions(-) create mode 100644 common/djangoapps/entitlements/migrations/0008_auto_20180328_1107.py diff --git a/common/djangoapps/entitlements/admin.py b/common/djangoapps/entitlements/admin.py index 9334e7a26b..2767f729fe 100644 --- a/common/djangoapps/entitlements/admin.py +++ b/common/djangoapps/entitlements/admin.py @@ -1,10 +1,11 @@ """Admin forms for Course Entitlements""" from django import forms from django.contrib import admin - from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey + from xmodule.modulestore.django import modulestore + from .models import CourseEntitlement, CourseEntitlementPolicy, CourseEntitlementSupportDetail @@ -72,4 +73,5 @@ class CourseEntitlementPolicyAdmin(admin.ModelAdmin): list_display = ('expiration_period', 'refund_period', 'regain_period', + 'mode', 'site') diff --git a/common/djangoapps/entitlements/api/v1/tests/test_views.py b/common/djangoapps/entitlements/api/v1/tests/test_views.py index 8524214f57..b48e76aef1 100644 --- a/common/djangoapps/entitlements/api/v1/tests/test_views.py +++ b/common/djangoapps/entitlements/api/v1/tests/test_views.py @@ -7,24 +7,25 @@ from datetime import datetime, timedelta from django.conf import settings from django.core.urlresolvers import reverse from django.utils.timezone import now +from mock import patch +from opaque_keys.edx.locator import CourseKey from course_modes.models import CourseMode from course_modes.tests.factories import CourseModeFactory -from mock import patch -from opaque_keys.edx.locator import CourseKey +from openedx.core.djangoapps.site_configuration.tests.factories import SiteFactory +from student.models import CourseEnrollment +from student.tests.factories import TEST_PASSWORD, CourseEnrollmentFactory, UserFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory -from student.models import CourseEnrollment -from student.tests.factories import (TEST_PASSWORD, CourseEnrollmentFactory, UserFactory) - log = logging.getLogger(__name__) # Entitlements is not in CMS' INSTALLED_APPS so these imports will error during test collection if settings.ROOT_URLCONF == 'lms.urls': from entitlements.tests.factories import CourseEntitlementFactory - from entitlements.models import CourseEntitlement + from entitlements.models import CourseEntitlement, CourseEntitlementPolicy from entitlements.api.v1.serializers import CourseEntitlementSerializer + from entitlements.api.v1.views import set_entitlement_policy @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms') @@ -56,6 +57,16 @@ class EntitlementViewSetTest(ModuleStoreTestCase): "order_number": "EDX-1001", } + def _assert_default_policy(self, policy): + """ + Assert that a policy is equal to the default Course Entitlement Policy. + """ + default_policy = CourseEntitlementPolicy() + assert policy.expiration_period == default_policy.expiration_period + assert policy.refund_period == default_policy.refund_period + assert policy.regain_period == default_policy.regain_period + assert policy.mode == default_policy.mode + def test_auth_required(self): self.client.logout() response = self.client.get(self.entitlements_list_url) @@ -124,6 +135,175 @@ class EntitlementViewSetTest(ModuleStoreTestCase): ) assert results == CourseEntitlementSerializer(course_entitlement).data + def test_default_no_policy_entry(self): + """ + Verify that, when there are no entries in the course entitlement policy table, + the default policy is used for a newly created entitlement. + """ + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + + self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + self._assert_default_policy(course_entitlement.policy) + + def test_default_no_matching_policy_entry(self): + """ + Verify that, when no course entitlement policy is found with the same mode or site + as the created entitlement, the default policy is used for the entitlement. + """ + CourseEntitlementPolicy.objects.create(mode=CourseMode.PROFESSIONAL, site=None) + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + + self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + self._assert_default_policy(course_entitlement.policy) + + def test_set_custom_mode_policy_on_create(self): + """ + Verify that, when there does not exist a course entitlement policy with the same mode and site as + a created entitlement, but there does exist a policy with the same mode and a null site, + that policy is assigned to the entitlement. + """ + policy = CourseEntitlementPolicy.objects.create(mode=CourseMode.PROFESSIONAL, site=None) + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + entitlement_data['mode'] = CourseMode.PROFESSIONAL + + self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + assert course_entitlement.policy == policy + + # To verify policy selecting behavior involving site specificity, we interact directly + # with the 'set_entitlement_policy' method due to an inablity to predict or manually assign + # the site associated with the requests made in unittests. + def test_set_custom_site_policy_on_create(self): + """ + Verify that, when there does not exist a course entitlement policy with the same mode and site as + a created entitlement, but there does exist a policy with the same site and a null mode, + that policy is assigned to the entitlement. + """ + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + + self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + + policy_site = SiteFactory.create() + policy = CourseEntitlementPolicy.objects.create(mode=None, site=policy_site) + + set_entitlement_policy(course_entitlement, policy_site) + assert course_entitlement.policy == policy + + def test_set_policy_match_site_over_mode(self): + """ + Verify that, when both a mode-agnostic policy matching the site of a created entitlement and a site-agnostic + policy matching the mode of a created entitlement exist but no policy matching both the site and mode of the + created entitlement exists, the site-specific (mode-agnostic) policy matching the entitlement is selected over + the mode-specific (site-agnostic) policy. + """ + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + + self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + + policy_site = SiteFactory.create() + policy = CourseEntitlementPolicy.objects.create(mode=None, site=policy_site) + CourseEntitlementPolicy.objects.create(mode=entitlement_data['mode'], site=None) + + set_entitlement_policy(course_entitlement, policy_site) + assert course_entitlement.policy == policy + + def test_set_policy_site_and_mode_specific(self): + """ + Verify that, when there exists a policy matching both the mode and site of the a given course entitlement, + it is selected over appropriate site- and mode-specific (mode- and site-agnostic) policies and the default + policy for assignment to the entitlement. + """ + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + entitlement_data['mode'] = CourseMode.PROFESSIONAL + + self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + + policy_site = SiteFactory.create() + policy = CourseEntitlementPolicy.objects.create(mode=entitlement_data['mode'], site=policy_site) + CourseEntitlementPolicy.objects.create(mode=entitlement_data['mode'], site=None) + CourseEntitlementPolicy.objects.create(mode=None, site=policy_site) + + set_entitlement_policy(course_entitlement, policy_site) + assert course_entitlement.policy == policy + + def test_professional_policy_for_no_id_professional(self): + """ + Verify that when there exists a policy with a professional mode that it is assigned + to new entitlements with the mode no-id-professional. + """ + policy = CourseEntitlementPolicy.objects.create(mode=CourseMode.PROFESSIONAL) + course_uuid = uuid.uuid4() + entitlement_data = self._get_data_set(self.user, str(course_uuid)) + entitlement_data['mode'] = CourseMode.NO_ID_PROFESSIONAL_MODE + + self.client.post( + self.entitlements_list_url, + data=json.dumps(entitlement_data), + content_type='application/json', + ) + + course_entitlement = CourseEntitlement.objects.get( + user=self.user, + course_uuid=course_uuid + ) + assert course_entitlement.policy == policy + def test_add_entitlement_with_support_detail(self): """ Verify that an EntitlementSupportDetail entry is made when the request includes support interaction information. diff --git a/common/djangoapps/entitlements/api/v1/views.py b/common/djangoapps/entitlements/api/v1/views.py index 0577e8fc65..15c16a31c6 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.db.models import Q from django.http import HttpResponseBadRequest from django.utils import timezone from django_filters.rest_framework import DjangoFilterBackend @@ -8,22 +9,21 @@ from edx_rest_framework_extensions.authentication import JwtAuthentication from edx_rest_framework_extensions.paginators import DefaultPagination from opaque_keys import InvalidKeyError from opaque_keys.edx.keys import CourseKey -from rest_framework import permissions, viewsets, status +from rest_framework import permissions, status, viewsets from rest_framework.authentication import SessionAuthentication from rest_framework.response import Response +from course_modes.models import CourseMode from entitlements.api.v1.filters import CourseEntitlementFilter from entitlements.api.v1.permissions import IsAdminOrSupportOrAuthenticatedReadOnly from entitlements.api.v1.serializers import CourseEntitlementSerializer -from entitlements.models import CourseEntitlement, CourseEntitlementSupportDetail +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 -from student.models import CourseEnrollment -from student.models import CourseEnrollmentException, AlreadyEnrolledError -from course_modes.models import CourseMode +from student.models import AlreadyEnrolledError, CourseEnrollment, CourseEnrollmentException log = logging.getLogger(__name__) @@ -89,6 +89,27 @@ def _process_revoke_and_unenroll_entitlement(course_entitlement, is_refund=False raise IntegrityError +def set_entitlement_policy(entitlement, site): + """ + Assign the appropriate CourseEntitlementPolicy to the given CourseEntitlement based on its mode and site. + + Arguments: + entitlement: Course Entitlement object + site: string representation of a Site object + + Notes: + Site-specific, mode-agnostic policies take precedence over mode-specific, site-agnostic policies. + If no appropriate CourseEntitlementPolicy is found, the default CourseEntitlementPolicy is assigned. + """ + policy_mode = entitlement.mode + if CourseMode.is_professional_slug(policy_mode): + policy_mode = CourseMode.PROFESSIONAL + filter_query = (Q(site=site) | Q(site__isnull=True)) & (Q(mode=policy_mode) | Q(mode__isnull=True)) + policy = CourseEntitlementPolicy.objects.filter(filter_query).order_by('-site', '-mode').first() + entitlement.policy = policy if policy else None + entitlement.save() + + 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}' @@ -127,6 +148,7 @@ class EntitlementViewSet(viewsets.ModelViewSet): self.perform_create(serializer) entitlement = serializer.instance + set_entitlement_policy(entitlement, request.site) if support_details: for support_detail in support_details: diff --git a/common/djangoapps/entitlements/migrations/0008_auto_20180328_1107.py b/common/djangoapps/entitlements/migrations/0008_auto_20180328_1107.py new file mode 100644 index 0000000000..5e602194b1 --- /dev/null +++ b/common/djangoapps/entitlements/migrations/0008_auto_20180328_1107.py @@ -0,0 +1,24 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ('entitlements', '0007_change_expiration_period_default'), + ] + + operations = [ + migrations.AddField( + model_name='courseentitlementpolicy', + name='mode', + field=models.CharField(max_length=32, null=True, choices=[(None, b'---------'), (b'verified', b'verified'), (b'professional', b'professional')]), + ), + migrations.AlterField( + model_name='courseentitlementpolicy', + name='site', + field=models.ForeignKey(to='sites.Site', null=True), + ), + ] diff --git a/common/djangoapps/entitlements/models.py b/common/djangoapps/entitlements/models.py index 5eaa07fab3..a3acc56abc 100644 --- a/common/djangoapps/entitlements/models.py +++ b/common/djangoapps/entitlements/models.py @@ -6,18 +6,18 @@ from datetime import timedelta from django.conf import settings from django.contrib.sites.models import Site -from django.db import models -from django.db import transaction +from django.db import models, transaction from django.utils.timezone import now +from model_utils import Choices 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 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 -from student.models import CourseEnrollmentException +from student.models import CourseEnrollment, CourseEnrollmentException from util.date_utils import strftime_localized -from entitlements.utils import is_course_run_entitlement_fulfillable log = logging.getLogger("common.entitlements.models") @@ -30,6 +30,7 @@ class CourseEntitlementPolicy(models.Model): DEFAULT_EXPIRATION_PERIOD_DAYS = 730 DEFAULT_REFUND_PERIOD_DAYS = 60 DEFAULT_REGAIN_PERIOD_DAYS = 14 + MODES = Choices((None, '---------'), CourseMode.VERIFIED, CourseMode.PROFESSIONAL) # Use a DurationField to calculate time as it returns a timedelta, useful in performing operations with datetimes expiration_period = models.DurationField( @@ -48,7 +49,8 @@ class CourseEntitlementPolicy(models.Model): "it is no longer able to be regained by a user."), null=False ) - site = models.ForeignKey(Site) + site = models.ForeignKey(Site, null=True) + mode = models.CharField(max_length=32, choices=MODES, null=True) def get_days_until_expiration(self, entitlement): """ @@ -131,11 +133,12 @@ class CourseEntitlementPolicy(models.Model): and not entitlement.expired_at) def __unicode__(self): - return u'Course Entitlement Policy: expiration_period: {}, refund_period: {}, regain_period: {}'\ + return u'Course Entitlement Policy: expiration_period: {}, refund_period: {}, regain_period: {}, mode: {}'\ .format( self.expiration_period, self.refund_period, self.regain_period, + self.mode )