Merge pull request #1052 from edx/iivic/SOL-2168

[SOL-2168] Add Backend Validation for Course Seat Types

ecommerce/extensions/api/v2/tests/views/test_coupons.py

@@ -731,6 +731,21 @@ class CouponViewSetFunctionalTest(CouponMixin, CourseCatalogTestMixin, CourseCat
         })
         self.assert_post_response_status(self.data)
 
+    @ddt.data(8, 'string', {'dict': 'type'})
+    def test_creating_coupon_with_wrong_course_seat_types(self, course_seat_types):
+        """ Verify creating coupon with course seat types not a list returns bad request. """
+        self.data.update({'course_seat_types': course_seat_types})
+        self.assert_post_response_status(self.data)
+
+    def test_creating_coupon_with_course_seat_types(self):
+        """ Verify creating coupon with course seat types list creates coupon. """
+        self.data.update({
+            'catalog_query': 'test',
+            'course_seat_types': ['verified'],
+            'stock_record_ids': None
+        })
+        self.assert_post_response_status(self.data, status.HTTP_200_OK)
+
 
 class CouponCategoriesListViewTests(TestCase):
     """ Tests for the coupon category list view. """

ecommerce/extensions/api/v2/views/coupons.py

@@ -8,6 +8,7 @@ from django.core.exceptions import ValidationError
 from django.db import IntegrityError, transaction
 from django.http import Http404
 from django.shortcuts import get_object_or_404
+from django.utils.translation import ugettext as _
 from oscar.core.loading import get_model
 from rest_framework import filters, generics, serializers, status, viewsets
 from rest_framework.permissions import IsAdminUser, IsAuthenticated
@@ -44,7 +45,7 @@ class CouponViewSet(EdxOrderPlacementMixin, viewsets.ModelViewSet):
     """ Coupon resource. """
     queryset = Product.objects.filter(product_class__name='Coupon')
     permission_classes = (IsAuthenticated, IsAdminUser)
-    filter_backends = (filters.DjangoFilterBackend, )
+    filter_backends = (filters.DjangoFilterBackend,)
     filter_class = ProductFilter
 
     def get_serializer_class(self):
@@ -94,7 +95,14 @@ class CouponViewSet(EdxOrderPlacementMixin, viewsets.ModelViewSet):
                         pass
 
                 if course_seat_types:
-                    course_seat_types = prepare_course_seat_types(course_seat_types)
+                    try:
+                        course_seat_types = prepare_course_seat_types(course_seat_types)
+                    except (AttributeError, TypeError) as e:
+                        logger.exception('Failed to create coupon. Invalid course seat types data: %s', e.message)
+                        return Response(
+                            _('Invalid course seat types data: {}').format(e.message),
+                            status=status.HTTP_400_BAD_REQUEST
+                        )
 
                 try:
                     category = Category.objects.get(name=category_data['name'])

ecommerce/extensions/offer/models.py

@@ -94,16 +94,33 @@ class ConditionalOffer(AbstractConditionalOffer):
         return super(ConditionalOffer, self).is_condition_satisfied(basket)  # pylint: disable=bad-super-call
 
 
-def validate_credit_seat_type(value):
-    if len(value.split(',')) > 1 and 'credit' in value:
+def validate_credit_seat_type(course_seat_types, allowed_seat_types):
+    if not isinstance(course_seat_types, basestring):
+        logger.exception('Failed to create Range. Credit seat types must be type str or unicode.')
+        raise ValidationError(_('Credit seat types must be type str or unicode.'))
+
+    course_seat_types_list = course_seat_types.split(',')
+
+    if len(course_seat_types_list) > 1 and 'credit' in course_seat_types_list:
+        logger.exception('Failed to create Range. Credit seat type cannot be paired with other seat types.')
         raise ValidationError('Credit seat types cannot be paired with other seat types.')
 
+    if not set(course_seat_types_list).issubset(set(allowed_seat_types)):
+        logger.exception(
+            'Failed to create Range. Not allowed course seat types %s. Allowed values for course seat types are %s',
+            course_seat_types_list, allowed_seat_types
+        )
+        raise ValidationError(_(
+            'Not allowed course seat types {}. Allowed values for course seat types are {}'
+        ).format(course_seat_types_list, allowed_seat_types))
+
 
 class Range(AbstractRange):
     UPDATABLE_RANGE_FIELDS = [
         'catalog_query',
         'course_seat_types',
     ]
+    ALLOWED_SEAT_TYPES = ['credit', 'professional', 'verified']
     catalog = models.ForeignKey('catalogue.Catalog', blank=True, null=True, related_name='ranges')
     catalog_query = models.TextField(blank=True, null=True)
     course_seat_types = models.CharField(
@@ -135,6 +152,9 @@ class Range(AbstractRange):
             logger.exception(exception_msg)
             raise ValidationError(validation_error_msg)
 
+        if self.course_seat_types:
+            validate_credit_seat_type(self.course_seat_types, self.ALLOWED_SEAT_TYPES)
+
     def run_catalog_query(self, product):
         """
         Retrieve the results from running the query contained in catalog_query field.

ecommerce/extensions/offer/tests/test_models.py

@@ -12,7 +12,6 @@ from oscar.test import factories
 from ecommerce.core.tests.decorators import mock_course_catalog_api_client
 from ecommerce.coupons.tests.mixins import CourseCatalogMockMixin, CouponMixin
 from ecommerce.extensions.catalogue.tests.mixins import CourseCatalogTestMixin
-from ecommerce.extensions.offer.models import validate_credit_seat_type
 from ecommerce.tests.testcases import TestCase
 
 Catalog = get_model('catalogue', 'Catalog')
@@ -75,7 +74,7 @@ class RangeTests(CouponMixin, CourseCatalogTestMixin, CourseCatalogMockMixin, Te
             non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
         """
         self.range.catalog_query = large_query
-        self.range.course_seat_types = ['verified', ]
+        self.range.course_seat_types = 'verified'
         self.range.save()
         self.assertEqual(self.range.catalog_query, large_query)
 
@@ -142,16 +141,6 @@ class RangeTests(CouponMixin, CourseCatalogTestMixin, CourseCatalogMockMixin, Te
         self.range.course_seat_types = 'verified'
         self.assertEqual(len(self.range.all_products()), 0)
 
-    def test_credit_seat_type_validator(self):
-        """
-        Verify the validator raises error for combination of credit and another seat type.
-        """
-        self.assertIsNone(validate_credit_seat_type('verified,professional'))
-        self.assertIsNone(validate_credit_seat_type('credit'))
-
-        with self.assertRaises(ValidationError):
-            validate_credit_seat_type('credit,verified')
-
     @ddt.data(
         {'catalog_query': '*:*'},
         {'catalog_query': '', 'course_seat_types': ['verified']},
@@ -177,13 +166,33 @@ class RangeTests(CouponMixin, CourseCatalogTestMixin, CourseCatalogMockMixin, Te
         """Verify creating range with catalog_query or catalog_seat_types creates range with those values."""
         data = {
             'catalog_query': 'id:testquery',
-            'course_seat_types': ['verified', 'professional']
+            'course_seat_types': 'verified,professional'
         }
         new_range = Range.objects.create(**data)
         self.assertEqual(new_range.catalog_query, data['catalog_query'])
         self.assertEqual(new_range.course_seat_types, data['course_seat_types'])
         self.assertEqual(new_range.catalog, None)
 
+    @ddt.data(5, 'credit,verified', 'verified,not_allowed_value')
+    def test_creating_range_with_wrong_course_seat_types(self, course_seat_types):
+        """ Verify creating range with incorrect course seat types will raise exception. """
+        data = {
+            'catalog_query': '*:*',
+            'course_seat_types': course_seat_types
+        }
+        with self.assertRaises(ValidationError):
+            Range.objects.create(**data)
+
+    @ddt.data('credit', 'professional', 'verified', 'professional,verified')
+    def test_creating_range_with_course_seat_types(self, course_seat_types):
+        """ Verify creating range with allowed course seat types values creates range. """
+        data = {
+            'catalog_query': '*:*',
+            'course_seat_types': course_seat_types
+        }
+        _range = Range.objects.create(**data)
+        self.assertEqual(_range.course_seat_types, course_seat_types)
+
 
 class ConditionalOfferTests(TestCase):
     """Tests for custom ConditionalOffer model."""