Remove deprecated SubfieldBase.

Also fix:
  - double-serialization of keys
  - using course keys instead of ids
  - using ids instead of course keys
  - empty course keys returning empty string instead of None
Other consequences:
  - If a model object is created using a string for an OpaqueKeyField,
    the field will stay the string, even after a save(). When the object
    is pulled back from the DB, the field will then be an Opaque Key
    object.
  - If a model is created without providing an opaque key for the
    OpaqueKeyField, the model instance returns an empty string
    for the key - it used to return None.

common/djangoapps/student/models.py

@@ -1529,7 +1529,7 @@ class CourseEnrollment(models.Model):
 
         if not status_hash:
             enrollments = cls.enrollments_for_user(user).values_list('course_id', 'mode')
-            enrollments = [(e[0].lower(), e[1].lower()) for e in enrollments]
+            enrollments = [(unicode(e[0]).lower(), e[1].lower()) for e in enrollments]
             enrollments = sorted(enrollments, key=lambda e: e[0])
             hash_elements = [user.username]
             hash_elements += ['{course_id}={mode}'.format(course_id=e[0], mode=e[1]) for e in enrollments]

common/djangoapps/util/models.py

@@ -44,8 +44,6 @@ class CompressedTextField(models.TextField):
     """ TextField that transparently compresses data when saving to the database, and decompresses the data
     when retrieving it from the database. """
 
-    __metaclass__ = models.SubfieldBase
-
     def get_prep_value(self, value):
         """ Compress the text data. """
         if value is not None:
@@ -59,5 +57,7 @@ class CompressedTextField(models.TextField):
         """ Decompresses the value from the database. """
         if isinstance(value, unicode):
             value = decompress_string(value)
-
         return value
+
+    def from_db_value(self, value, expression, connection, context):
+        return self.to_python(value)

lms/djangoapps/badges/models.py

@@ -82,7 +82,7 @@ class BadgeClass(models.Model):
         if course_id and not modulestore().get_course(course_id).issue_badges:
             raise CourseBadgesDisabledError("This course does not have badges enabled.")
         if not course_id:
-            course_id = CourseKeyField.Empty
+            course_id = ''
         try:
             return cls.objects.get(slug=slug, issuing_component=issuing_component, course_id=course_id)
         except cls.DoesNotExist:
@@ -97,6 +97,7 @@ class BadgeClass(models.Model):
             description=description,
             criteria=criteria,
         )
+
         badge_class.image.save(image_file_handle.name, image_file_handle)
         badge_class.full_clean()
         badge_class.save()

lms/djangoapps/class_dashboard/dashboard_data.py

@@ -45,7 +45,7 @@ def get_problem_grade_distribution(course_id):
 
     # Loop through resultset building data for each problem
     for row in db_query:
-        curr_problem = UsageKey.from_string(row['module_state_key']).map_into_course(course_id)
+        curr_problem = row['module_state_key'].map_into_course(course_id)
 
         # Build set of grade distributions for each problem that has student responses
         if curr_problem in prob_grade_distrib:
@@ -85,7 +85,7 @@ def get_sequential_open_distrib(course_id):
     # Build set of "opened" data for each subsection that has "opened" data
     sequential_open_distrib = {}
     for row in db_query:
-        row_loc = UsageKey.from_string(row['module_state_key']).map_into_course(course_id)
+        row_loc = row['module_state_key'].map_into_course(course_id)
         sequential_open_distrib[row_loc] = row['count_sequential']
 
     return sequential_open_distrib
@@ -122,7 +122,7 @@ def get_problem_set_grade_distrib(course_id, problem_set):
 
     # Loop through resultset building data for each problem
     for row in db_query:
-        row_loc = UsageKey.from_string(row['module_state_key']).map_into_course(course_id)
+        row_loc = row['module_state_key'].map_into_course(course_id)
         if row_loc not in prob_grade_distrib:
             prob_grade_distrib[row_loc] = {
                 'max_grade': 0,

lms/djangoapps/courseware/model_data.py

@@ -960,7 +960,7 @@ class ScoresClient(object):
         # attached to them (since old mongo identifiers don't include runs).
         # So we have to add that info back in before we put it into our lookup.
         self._locations_to_scores.update({
-            UsageKey.from_string(location).map_into_course(self.course_key): self.Score(correct, total, created)
+            location.map_into_course(self.course_key): self.Score(correct, total, created)
             for location, correct, total, created
             in scores_qset.values_list('module_state_key', 'grade', 'max_grade', 'created')
         })

lms/djangoapps/instructor/tests/test_ecommerce.py

@@ -308,7 +308,7 @@ class TestECommerceDashboardViews(SiteMixin, SharedModuleStoreTestCase):
         Test Update Coupon Info Scenarios. Handle all the HttpResponses return by update_coupon view
         """
         coupon = Coupon(
-            code='AS452', description='asdsadsa', course_id=self.course.id.to_deprecated_string(),
+            code='AS452', description='asdsadsa', course_id=self.course.id,
             percentage_discount=10, created_by=self.instructor
         )
         coupon.save()

lms/djangoapps/instructor_analytics/basic.py

@@ -198,6 +198,7 @@ def issued_certificates(course_key, features):
 
     # Report run date
     for data in generated_certificates:
+        data['course_id'] = unicode(data['course_id'])
         data['report_run_date'] = report_run_date
 
     return generated_certificates
@@ -488,7 +489,6 @@ def course_registration_features(features, registration_codes, csv_type):
             except ObjectDoesNotExist:
                 pass
 
-        course_registration_dict['course_id'] = course_registration_dict['course_id'].to_deprecated_string()
         return course_registration_dict
     return [extract_course_registration(code, features, csv_type) for code in registration_codes]
 

lms/djangoapps/instructor_analytics/tests/test_basic.py

@@ -574,7 +574,7 @@ class TestCourseRegistrationCodeAnalyticsBasic(ModuleStoreTestCase):
             self.assertIn(course_registration['code'], [registration_code.code for registration_code in registration_codes])
             self.assertIn(
                 course_registration['course_id'],
-                [registration_code.course_id.to_deprecated_string() for registration_code in registration_codes]
+                [registration_code.course_id for registration_code in registration_codes]
             )
             self.assertIn(
                 course_registration['company_name'],

lms/djangoapps/shoppingcart/models.py

@@ -2159,7 +2159,7 @@ class Donation(OrderItem):
             "line_desc": description
         }
 
-        if course_id is not None:
+        if course_id is not None and course_id != '':
             params["course_id"] = course_id
             params["donation_type"] = "course"
         else:

lms/djangoapps/shoppingcart/tests/test_models.py

@@ -1144,7 +1144,7 @@ class DonationTest(ModuleStoreTestCase):
         with self.assertRaises(CourseDoesNotExistException):
             Donation.add_to_order(self.cart, self.COST, course_id=fake_course_id)
 
-    def _assert_donation(self, donation, donation_type=None, course_id=None, unit_cost=None, line_desc=None):
+    def _assert_donation(self, donation, donation_type=None, course_id='', unit_cost=None, line_desc=None):
         """Verify the donation fields and that the donation can be purchased. """
         self.assertEqual(donation.order, self.cart)
         self.assertEqual(donation.user, self.user)

lms/djangoapps/teams/views.py

@@ -1055,7 +1055,7 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView):
                     CourseAccessRole.objects.filter(user=request.user, role='staff').values_list('course_id', flat=True)
                 )
                 accessible_course_ids = [item for sublist in (enrolled_courses, staff_courses) for item in sublist]
-                if requested_course_id is not None and requested_course_id not in accessible_course_ids:
+                if requested_course_key is not None and requested_course_key not in accessible_course_ids:
                     return Response(status=status.HTTP_400_BAD_REQUEST)
 
         if not specified_username_or_team:
@@ -1068,7 +1068,7 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView):
         if requested_course_key is not None:
             course_keys = [requested_course_key]
         elif accessible_course_ids is not None:
-            course_keys = [CourseKey.from_string(course_string) for course_string in accessible_course_ids]
+            course_keys = accessible_course_ids
 
         queryset = CourseTeamMembership.get_memberships(username, course_keys, team_id)
         page = self.paginate_queryset(queryset)

openedx/core/djangoapps/content/course_overviews/models.py

@@ -552,8 +552,7 @@ class CourseOverview(TimeStampedModel):
         Returns all course keys from course overviews.
         """
         return [
-            CourseKey.from_string(course_overview['id'])
-            for course_overview in CourseOverview.objects.values('id')
+            course_overview['id'] for course_overview in CourseOverview.objects.values('id')
         ]
 
     def is_discussion_tab_enabled(self):

openedx/core/djangoapps/course_groups/models.py

@@ -80,7 +80,9 @@ class CohortMembership(models.Model):
         unique_together = (('user', 'course_id'), )
 
     def clean_fields(self, *args, **kwargs):
-        if self.course_id is None:
+        if self.course_id is None and self.course_user_group.course_id is None:
+            raise ValidationError("CohortMembership course_id and course_user_group course_id are both None")
+        if self.course_id in ('', None):
             self.course_id = self.course_user_group.course_id
         super(CohortMembership, self).clean_fields(*args, **kwargs)
 
@@ -91,9 +93,8 @@ class CohortMembership(models.Model):
             raise ValidationError("Non-matching course_ids provided")
 
     def save(self, *args, **kwargs):
-        self.full_clean(validate_unique=False)
-
         log.info("Saving CohortMembership for user '%s' in '%s'", self.user.id, self.course_id)
+        self.full_clean(validate_unique=False)
 
         # Avoid infinite recursion if creating from get_or_create() call below.
         # This block also allows middleware to use CohortMembership.get_or_create without worrying about outer_atomic

openedx/core/djangoapps/credit/tests/factories.py

@@ -6,6 +6,7 @@ import uuid  # pylint:disable=unused-import
 from django.contrib.auth.models import User
 import factory
 from factory.fuzzy import FuzzyText
+from opaque_keys.edx.keys import CourseKey
 import pytz
 
 from openedx.core.djangoapps.credit.models import CreditProvider, CreditEligibility, CreditCourse, CreditRequest
@@ -50,7 +51,9 @@ class CreditRequestFactory(factory.DjangoModelFactory):
         Sets up parameters field.
         """
         if not obj.parameters:
-            course_key = obj.course.course_key
+            # The factory is creating these keys as strings, in Django 1.9+ they don't get automatically
+            # marshalled to CourseKey objects, so this ensures they are created as expected.
+            course_key = CourseKey.from_string(obj.course.course_key)
             user = User.objects.get(username=obj.username)
             user_profile = user.profile
 

openedx/core/djangoapps/credit/tests/test_views.py

@@ -346,7 +346,7 @@ class CreditProviderRequestCreateViewTests(ApiTestCaseMixin, UserMixin, TestCase
         """ Verify the endpoint can create a new credit request. """
         username = self.user.username
         course = self.eligibility.course
-        course_key = course.course_key
+        course_key = CourseKey.from_string(course.course_key)
         final_grade = 0.95
 
         # Enable provider integration
@@ -521,6 +521,7 @@ class CreditProviderCallbackViewTests(UserMixin, TestCase):
     def _create_credit_request_and_get_uuid(self, username=None, course_key=None):
         """ Initiate a request for credit and return the request UUID. """
         username = username or self.user.username
+
         course = CreditCourse.objects.get(course_key=course_key) if course_key else self.eligibility.course
         credit_request = CreditRequestFactory(username=username, course=course, provider=self.provider)
         return credit_request.uuid

openedx/core/djangoapps/xmodule_django/models.py

@@ -81,8 +81,6 @@ class OpaqueKeyField(models.CharField):
     """
     description = "An OpaqueKey object, saved to the DB in the form of a string."
 
-    __metaclass__ = models.SubfieldBase
-
     Empty = object()
     KEY_CLASS = None
 
@@ -117,6 +115,9 @@ class OpaqueKeyField(models.CharField):
         else:
             return value
 
+    def from_db_value(self, value, expression, connection, context):
+        return self.to_python(value)
+
     def get_prep_lookup(self, lookup, value):
         if lookup == 'isnull':
             raise TypeError('Use {0}.Empty rather than None to query for a missing {0}'.format(self.__class__.__name__))
@@ -131,7 +132,8 @@ class OpaqueKeyField(models.CharField):
         if value is self.Empty or value is None:
             return ''  # CharFields should use '' as their empty value, rather than None
 
-        assert isinstance(value, self.KEY_CLASS), "%s is not an instance of %s" % (value, self.KEY_CLASS)
+        assert isinstance(value, (basestring, self.KEY_CLASS)), \
+            "%s is not an instance of basestring or %s" % (value, self.KEY_CLASS)
         serialized_key = unicode(_strip_value(value))
         if serialized_key.endswith('\n'):
             # An opaque key object serialized to a string with a trailing newline.