Revert StudentModule.module_state_key to field rather than computed property

lms/djangoapps/class_dashboard/dashboard_data.py

@@ -32,13 +32,13 @@ def get_problem_grade_distribution(course_id):
         course_id__exact=course_id,
         grade__isnull=False,
         module_type__exact="problem",
-    ).values('module_id', 'grade', 'max_grade').annotate(count_grade=Count('grade'))
+    ).values('module_state_key', 'grade', 'max_grade').annotate(count_grade=Count('grade'))
 
     prob_grade_distrib = {}
 
     # Loop through resultset building data for each problem
     for row in db_query:
-        curr_problem = course_id.make_usage_key_from_deprecated_string(row['module_id'])
+        curr_problem = course_id.make_usage_key_from_deprecated_string(row['module_state_key'])
 
         # Build set of grade distributions for each problem that has student responses
         if curr_problem in prob_grade_distrib:
@@ -70,12 +70,12 @@ def get_sequential_open_distrib(course_id):
     db_query = models.StudentModule.objects.filter(
         course_id__exact=course_id,
         module_type__exact="sequential",
-    ).values('module_id').annotate(count_sequential=Count('module_id'))
+    ).values('module_state_key').annotate(count_sequential=Count('module_state_key'))
 
     # Build set of "opened" data for each subsection that has "opened" data
     sequential_open_distrib = {}
     for row in db_query:
-        row_loc = course_id.make_usage_key_from_deprecated_string(row['module_id'])
+        row_loc = course_id.make_usage_key_from_deprecated_string(row['module_state_key'])
         sequential_open_distrib[row_loc] = row['count_sequential']
 
     return sequential_open_distrib
@@ -101,18 +101,18 @@ def get_problem_set_grade_distrib(course_id, problem_set):
         course_id__exact=course_id,
         grade__isnull=False,
         module_type__exact="problem",
-        module_id__in=problem_set,
+        module_state_key__in=problem_set,
     ).values(
-        'module_id',
+        'module_state_key',
         'grade',
         'max_grade',
-    ).annotate(count_grade=Count('grade')).order_by('module_id', 'grade')
+    ).annotate(count_grade=Count('grade')).order_by('module_state_key', 'grade')
 
     prob_grade_distrib = {}
 
     # Loop through resultset building data for each problem
     for row in db_query:
-        row_loc = course_id.make_usage_key_from_deprecated_string(row['module_id'])
+        row_loc = course_id.make_usage_key_from_deprecated_string(row['module_state_key'])
         if row_loc not in prob_grade_distrib:
             prob_grade_distrib[row_loc] = {
                 'max_grade': 0,
@@ -418,12 +418,12 @@ def get_students_opened_subsection(request, csv=False):
     If 'csv' is True, returns a header array, and an array of arrays in the format:
     student names, usernames for CSV download.
     """
-    module_id = Location.from_deprecated_string(request.GET.get('module_id'))
+    module_state_key = Location.from_deprecated_string(request.GET.get('module_id'))
     csv = request.GET.get('csv')
 
     # Query for "opened a subsection" students
     students = models.StudentModule.objects.select_related('student').filter(
-        module_id__exact=module_id,
+        module_state_key__exact=module_state_key,
         module_type__exact='sequential',
     ).values('student__username', 'student__profile__name').order_by('student__profile__name')
 
@@ -468,12 +468,12 @@ def get_students_problem_grades(request, csv=False):
     If 'csv' is True, returns a header array, and an array of arrays in the format:
     student names, usernames, grades, percents for CSV download.
     """
-    module_id = Location.from_deprecated_string(request.GET.get('module_id'))
+    module_state_key = Location.from_deprecated_string(request.GET.get('module_id'))
     csv = request.GET.get('csv')
 
     # Query for "problem grades" students
     students = models.StudentModule.objects.select_related('student').filter(
-        module_id__exact=module_id,
+        module_state_key=module_state_key,
         module_type__exact='problem',
         grade__isnull=False,
     ).values('student__username', 'student__profile__name', 'grade', 'max_grade').order_by('student__profile__name')

lms/djangoapps/courseware/grades.py

@@ -116,7 +116,7 @@ def answer_distributions(course_key):
             continue
 
         try:
-            url, display_name = url_and_display_name(module.module_id.map_into_course(course_key))
+            url, display_name = url_and_display_name(module.module_state_key.map_into_course(course_key))
             # Each problem part has an ID that is derived from the
             # module.module_state_key (with some suffix appended)
             for problem_part_id, raw_answer in raw_answers.items():
@@ -210,7 +210,7 @@ def _grade(student, request, course, keep_raw_scores):
                 with manual_transaction():
                     should_grade_section = StudentModule.objects.filter(
                         student=student,
-                        module_id__in=[
+                        module_state_key__in=[
                             descriptor.location for descriptor in section['xmoduledescriptors']
                         ]
                     ).exists()
@@ -447,7 +447,7 @@ def get_score(course_id, user, problem_descriptor, module_creator, scores_cache=
         student_module = StudentModule.objects.get(
             student=user,
             course_id=course_id,
-            module_id=problem_descriptor.location
+            module_state_key=problem_descriptor.location
         )
     except StudentModule.DoesNotExist:
         student_module = None

lms/djangoapps/courseware/model_data.py

@@ -144,7 +144,7 @@ class FieldDataCache(object):
         if scope == Scope.user_state:
             return self._chunked_query(
                 StudentModule,
-                'module_id__in',
+                'module_state_key__in',
                 (descriptor.scope_ids.usage_id for descriptor in self.descriptors),
                 course_id=self.course_id,
                 student=self.user.pk,
@@ -248,7 +248,7 @@ class FieldDataCache(object):
             field_object, _ = StudentModule.objects.get_or_create(
                 course_id=self.course_id,
                 student=User.objects.get(id=key.user_id),
-                module_id=key.block_scope_id.replace(run=None),
+                module_state_key=key.block_scope_id,
                 defaults={
                     'state': json.dumps({}),
                     'module_type': key.block_scope_id.category,

lms/djangoapps/courseware/models.py

@@ -40,31 +40,13 @@ class StudentModule(models.Model):
     # but for abtests and the like, this can be set to a shared value
     # for many instances of the module.
     # Filename for homeworks, etc.
-    module_id = LocationKeyField(max_length=255, db_index=True, db_column='module_id')
+    module_state_key = LocationKeyField(max_length=255, db_index=True, db_column='module_id')
     student = models.ForeignKey(User, db_index=True)
-    # TODO: This is a lie; course_id now represents something more like a course_key.  We may
-    # or may not want to change references to this to something like course_key or course_key_field in
-    # this file.  (Certain changes would require a DB migration which is probably not what we want.)
-    # Someone should look at this and reevaluate before the final merge into master.
-    course_id = CourseKeyField(max_length=255, db_index=True)
-
-    @property
-    def module_state_key(self):
-        """
-        Returns a Location based on module_id and course_id
-        """
-        return self.course_id.make_usage_key(self.module_id.category, self.module_id.name)
 
-    @module_state_key.setter
-    def module_state_key(self, usage_key):
-        """
-        Set the module_id and course_id from the passed UsageKey
-        """
-        self.course_id = usage_key.course_key
-        self.module_id = usage_key
+    course_id = CourseKeyField(max_length=255, db_index=True)
 
     class Meta:
-        unique_together = (('student', 'module_id', 'course_id'),)
+        unique_together = (('student', 'module_state_key', 'course_id'),)
 
     ## Internal state of the object
     state = models.TextField(null=True, blank=True)

lms/djangoapps/courseware/tests/test_model_data.py

@@ -7,7 +7,7 @@ from functools import partial
 
 from courseware.model_data import DjangoKeyValueStore
 from courseware.model_data import InvalidScopeError, FieldDataCache
-from courseware.models import StudentModule, XModuleUserStateSummaryField
+from courseware.models import StudentModule
 from courseware.models import XModuleStudentInfoField, XModuleStudentPrefsField
 
 from student.tests.factories import UserFactory
@@ -200,7 +200,7 @@ class TestMissingStudentModule(TestCase):
         student_module = StudentModule.objects.all()[0]
         self.assertEquals({'a_field': 'a_value'}, json.loads(student_module.state))
         self.assertEquals(self.user, student_module.student)
-        self.assertEquals(location('usage_id'), student_module.module_state_key)
+        self.assertEquals(location('usage_id').replace(run=None), student_module.module_state_key)
         self.assertEquals(course_id, student_module.course_id)
 
     def test_delete_field_from_missing_student_module(self):

lms/djangoapps/courseware/tests/test_module_render.py

@@ -726,7 +726,7 @@ class TestStaffDebugInfo(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course.id,
-            module_id=self.location,
+            module_state_key=self.location,
             student=UserFactory(),
             grade=1,
             max_grade=1,

lms/djangoapps/courseware/views.py

@@ -755,7 +755,7 @@ def submission_history(request, course_id, student_username, location):
         student = User.objects.get(username=student_username)
         student_module = StudentModule.objects.get(
             course_id=course_key,
-            module_id=usage_key,
+            module_state_key=usage_key,
             student_id=student.id
         )
     except User.DoesNotExist:

lms/djangoapps/instructor/enrollment.py

@@ -198,7 +198,7 @@ def reset_student_attempts(course_id, student, module_state_key, delete_module=F
     module_to_reset = StudentModule.objects.get(
         student_id=student.id,
         course_id=course_id,
-        module_id=module_state_key
+        module_state_key=module_state_key
     )
 
     if delete_module:

lms/djangoapps/instructor/management/commands/openended_stats.py

@@ -81,7 +81,7 @@ def calculate_task_statistics(students, course, location, task_number, write_to_
     students_with_graded_submissions = []  # pylint: disable=invalid-name
     students_with_no_state = []
 
-    student_modules = StudentModule.objects.filter(module_id=location, student__in=students).order_by('student')
+    student_modules = StudentModule.objects.filter(module_state_key=location, student__in=students).order_by('student')
     print "Total student modules: {0}".format(student_modules.count())
 
     for index, student_module in enumerate(student_modules):

lms/djangoapps/instructor/management/tests/test_openended_commands.py

@@ -43,7 +43,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             student=self.student_on_initial,
             grade=0,
             max_grade=1,
@@ -52,7 +52,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             student=self.student_on_accessing,
             grade=0,
             max_grade=1,
@@ -61,7 +61,7 @@ class OpenEndedPostTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             student=self.student_on_post_assessment,
             grade=0,
             max_grade=1,
@@ -139,7 +139,7 @@ class OpenEndedStatsTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             student=self.student_on_initial,
             grade=0,
             max_grade=1,
@@ -148,7 +148,7 @@ class OpenEndedStatsTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             student=self.student_on_accessing,
             grade=0,
             max_grade=1,
@@ -157,7 +157,7 @@ class OpenEndedStatsTest(ModuleStoreTestCase):
 
         StudentModuleFactory.create(
             course_id=self.course_id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             student=self.student_on_post_assessment,
             grade=0,
             max_grade=1,

lms/djangoapps/instructor/tests/test_api.py

@@ -120,7 +120,7 @@ class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase):
         _module = StudentModule.objects.create(
             student=self.user,
             course_id=self.course.id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             state=json.dumps({'attempts': 10}),
         )
 
@@ -1489,7 +1489,7 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase)
         self.module_to_reset = StudentModule.objects.create(
             student=self.student,
             course_id=self.course.id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             state=json.dumps({'attempts': 10}),
         )
 
@@ -1748,7 +1748,7 @@ class TestInstructorAPITaskLists(ModuleStoreTestCase, LoginEnrollmentTestCase):
         self.module = StudentModule.objects.create(
             student=self.student,
             course_id=self.course.id,
-            module_id=self.problem_location,
+            module_state_key=self.problem_location,
             state=json.dumps({'attempts': 10}),
         )
         mock_factory = MockCompletionInfo()
@@ -1988,46 +1988,46 @@ class TestDueDateExtensions(ModuleStoreTestCase, LoginEnrollmentTestCase):
             state='{}',
             student_id=user1.id,
             course_id=course.id,
-            module_id=week1.location).save()
+            module_state_key=week1.location).save()
         StudentModule(
             state='{}',
             student_id=user1.id,
             course_id=course.id,
-            module_id=week2.location).save()
+            module_state_key=week2.location).save()
         StudentModule(
             state='{}',
             student_id=user1.id,
             course_id=course.id,
-            module_id=week3.location).save()
+            module_state_key=week3.location).save()
         StudentModule(
             state='{}',
             student_id=user1.id,
             course_id=course.id,
-            module_id=homework.location).save()
+            module_state_key=homework.location).save()
 
         user2 = UserFactory.create()
         StudentModule(
             state='{}',
             student_id=user2.id,
             course_id=course.id,
-            module_id=week1.location).save()
+            module_state_key=week1.location).save()
         StudentModule(
             state='{}',
             student_id=user2.id,
             course_id=course.id,
-            module_id=homework.location).save()
+            module_state_key=homework.location).save()
 
         user3 = UserFactory.create()
         StudentModule(
             state='{}',
             student_id=user3.id,
             course_id=course.id,
-            module_id=week1.location).save()
+            module_state_key=week1.location).save()
         StudentModule(
             state='{}',
             student_id=user3.id,
             course_id=course.id,
-            module_id=homework.location).save()
+            module_state_key=homework.location).save()
 
         self.course = course
         self.week1 = week1

lms/djangoapps/instructor/tests/test_enrollment.py

@@ -297,9 +297,9 @@ class TestInstructorEnrollmentStudentModule(TestCase):
         user = UserFactory()
         msk = self.course_key.make_usage_key('dummy', 'module')
         original_state = json.dumps({'attempts': 32, 'otherstuff': 'alsorobots'})
-        StudentModule.objects.create(student=user, course_id=self.course_key, module_id=msk, state=original_state)
+        StudentModule.objects.create(student=user, course_id=self.course_key, module_state_key=msk, state=original_state)
         # lambda to reload the module state from the database
-        module = lambda: StudentModule.objects.get(student=user, course_id=self.course_key, module_id=msk)
+        module = lambda: StudentModule.objects.get(student=user, course_id=self.course_key, module_state_key=msk)
         self.assertEqual(json.loads(module().state)['attempts'], 32)
         reset_student_attempts(self.course_key, user, msk)
         self.assertEqual(json.loads(module().state)['attempts'], 0)
@@ -308,10 +308,10 @@ class TestInstructorEnrollmentStudentModule(TestCase):
         user = UserFactory()
         msk = self.course_key.make_usage_key('dummy', 'module')
         original_state = json.dumps({'attempts': 32, 'otherstuff': 'alsorobots'})
-        StudentModule.objects.create(student=user, course_id=self.course_key, module_id=msk, state=original_state)
-        self.assertEqual(StudentModule.objects.filter(student=user, course_id=self.course_key, module_id=msk).count(), 1)
+        StudentModule.objects.create(student=user, course_id=self.course_key, module_state_key=msk, state=original_state)
+        self.assertEqual(StudentModule.objects.filter(student=user, course_id=self.course_key, module_state_key=msk).count(), 1)
         reset_student_attempts(self.course_key, user, msk, delete_module=True)
-        self.assertEqual(StudentModule.objects.filter(student=user, course_id=self.course_key, module_id=msk).count(), 0)
+        self.assertEqual(StudentModule.objects.filter(student=user, course_id=self.course_key, module_state_key=msk).count(), 0)
 
     def test_delete_submission_scores(self):
         user = UserFactory()

lms/djangoapps/instructor/tests/test_legacy_gradebook.py

@@ -65,7 +65,7 @@ class TestGradebook(ModuleStoreTestCase):
                     max_grade=1,
                     student=user,
                     course_id=self.course.id,
-                    module_id=item.location
+                    module_state_key=item.location
                 )
 
         self.response = self.client.get(reverse(

lms/djangoapps/instructor/tests/test_tools.py

@@ -329,7 +329,7 @@ def get_extended_due(course, unit, student):
     student_module = StudentModule.objects.get(
         student_id=student.id,
         course_id=course.id,
-        module_id=unit.location
+        module_state_key=unit.location
     )
 
     state = json.loads(student_module.state)

lms/djangoapps/instructor/views/legacy.py

@@ -401,7 +401,7 @@ def instructor_dashboard(request, course_id):
                     student_module = StudentModule.objects.get(
                         student_id=student.id,
                         course_id=course_key,
-                        module_id=module_state_key
+                        module_state_key=module_state_key
                     )
                     msg += _("Found module.  ")
 

lms/djangoapps/instructor/views/tools.py

@@ -148,7 +148,7 @@ def set_due_date_extension(course, unit, student, due_date):
             student_module = StudentModule.objects.get(
                 student_id=student.id,
                 course_id=course.id,
-                module_id=node.location
+                module_state_key=node.location
             )
 
             state = json.loads(student_module.state)
@@ -173,7 +173,7 @@ def dump_module_extensions(course, unit):
     header = [_("Username"), _("Full Name"), _("Extended Due Date")]
     query = StudentModule.objects.filter(
         course_id=course.id,
-        module_id=unit.location)
+        module_state_key=unit.location)
     for module in query:
         state = json.loads(module.state)
         extended_due = state.get("extended_due")
@@ -208,14 +208,16 @@ def dump_student_extensions(course, student):
         student_id=student.id)
     for module in query:
         state = json.loads(module.state)
-        if module.module_state_key not in units:
+        # temporary hack: module_state_key is missing the run but units are not. fix module_state_key
+        module_loc = module.map_into_course(module.course_id)
+        if module_loc not in units:
             continue
         extended_due = state.get("extended_due")
         if not extended_due:
             continue
         extended_due = DATE_FIELD.from_json(extended_due)
         extended_due = extended_due.strftime("%Y-%m-%d %H:%M")
-        title = title_or_url(units[module.module_state_key])
+        title = title_or_url(units[module_loc])
         data.append(dict(zip(header, (title, extended_due))))
     return {
         "header": header,

lms/djangoapps/instructor_task/tasks_helper.py

@@ -251,7 +251,7 @@ def perform_module_state_update(update_fcn, filter_fcn, _entry_id, course_id, ta
     module_descriptor = modulestore().get_item(usage_key)
 
     # find the module in question
-    modules_to_update = StudentModule.objects.filter(course_id=course_id, module_id=usage_key)
+    modules_to_update = StudentModule.objects.filter(course_id=course_id, module_state_key=usage_key)
 
     # give the option of updating an individual student. If not specified,
     # then updates all students who have responded to a problem so far

lms/djangoapps/instructor_task/tests/test_base.py

@@ -215,5 +215,5 @@ class InstructorTaskModuleTestCase(InstructorTaskCourseTestCase):
         return StudentModule.objects.get(course_id=self.course.id,
                                          student=User.objects.get(username=username),
                                          module_type=descriptor.location.category,
-                                         module_id=descriptor.location,
+                                         module_state_key=descriptor.location,
                                          )

lms/djangoapps/instructor_task/tests/test_tasks.py

@@ -128,7 +128,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         for student in students:
             CourseEnrollmentFactory.create(course_id=self.course.id, user=student)
             StudentModuleFactory.create(course_id=self.course.id,
-                                        module_id=self.location,
+                                        module_state_key=self.location,
                                         student=student,
                                         grade=grade,
                                         max_grade=max_grade,
@@ -140,7 +140,7 @@ class TestInstructorTasks(InstructorTaskModuleTestCase):
         for student in students:
             module = StudentModule.objects.get(course_id=self.course.id,
                                                student=student,
-                                               module_id=self.location)
+                                               module_state_key=self.location)
             state = json.loads(module.state)
             self.assertEquals(state['attempts'], num_attempts)
 
@@ -357,7 +357,7 @@ class TestResetAttemptsInstructorTask(TestInstructorTasks):
         for student in students:
             module = StudentModule.objects.get(course_id=self.course.id,
                                                student=student,
-                                               module_id=self.location)
+                                               module_state_key=self.location)
             state = json.loads(module.state)
             self.assertEquals(state['attempts'], initial_attempts)
 
@@ -383,7 +383,7 @@ class TestResetAttemptsInstructorTask(TestInstructorTasks):
         for index, student in enumerate(students):
             module = StudentModule.objects.get(course_id=self.course.id,
                                                student=student,
-                                               module_id=self.location)
+                                               module_state_key=self.location)
             state = json.loads(module.state)
             if index == 3:
                 self.assertEquals(state['attempts'], 0)
@@ -430,11 +430,11 @@ class TestDeleteStateInstructorTask(TestInstructorTasks):
         for student in students:
             StudentModule.objects.get(course_id=self.course.id,
                                       student=student,
-                                      module_id=self.location)
+                                      module_state_key=self.location)
         self._test_run_with_task(delete_problem_state, 'deleted', num_students)
         # confirm that no state can be found anymore:
         for student in students:
             with self.assertRaises(StudentModule.DoesNotExist):
                 StudentModule.objects.get(course_id=self.course.id,
                                           student=student,
-                                          module_id=self.location)
+                                          module_state_key=self.location)

lms/djangoapps/psychometrics/models.py

@@ -15,9 +15,7 @@ class PsychometricData(models.Model):
 
     Links to instances of StudentModule, but only those for capa problems.
 
-    Note that StudentModule.module_state_key is nominally a Location instance (url string).
-    That means it is of the form {tag}://{org}/{course}/{category}/{name}[@{revision}]
-    and for capa problems, category = "problem".
+    Note that StudentModule.module_state_key is a :class:`Location` instance.
 
     checktimes is extracted from tracking logs, or added by capa module via psychometrics callback.
     """