From 3824e234fe6d539e86c905996757f7ebef74b365 Mon Sep 17 00:00:00 2001 From: Usman Khalid <2200617@gmail.com> Date: Mon, 26 May 2014 18:45:15 +0500 Subject: [PATCH] Do not persist anonymous ids when exporting for all students of a course. For large courses this causes export to take a long time and to time out. LMS-2747 --- common/djangoapps/student/models.py | 25 +++++++++++++------ common/djangoapps/student/tests/tests.py | 1 + lms/djangoapps/instructor/tests/test_api.py | 2 +- .../instructor/tests/test_legacy_anon_csv.py | 2 +- lms/djangoapps/instructor/views/api.py | 4 +-- lms/djangoapps/instructor/views/legacy.py | 4 +-- 6 files changed, 24 insertions(+), 14 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 6faec29c0e..e002d7f1a1 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -67,12 +67,15 @@ class AnonymousUserId(models.Model): unique_together = (user, course_id) -def anonymous_id_for_user(user, course_id): +def anonymous_id_for_user(user, course_id, save=True): """ Return a unique id for a (user, course) pair, suitable for inserting into e.g. personalized survey links. If user is an `AnonymousUser`, returns `None` + + Keyword arguments: + save -- Whether the id should be saved in an AnonymousUserId object. """ # This part is for ability to get xblock instance in xblock_noauth handlers, where user is unauthenticated. if user.is_anonymous(): @@ -90,6 +93,14 @@ def anonymous_id_for_user(user, course_id): hasher.update(course_id.to_deprecated_string()) digest = hasher.hexdigest() + if not hasattr(user, '_anonymous_id'): + user._anonymous_id = {} # pylint: disable=protected-access + + user._anonymous_id[course_id] = digest # pylint: disable=protected-access + + if save is False: + return digest + try: anonymous_user_id, __ = AnonymousUserId.objects.get_or_create( defaults={'anonymous_user_id': digest}, @@ -111,11 +122,6 @@ def anonymous_id_for_user(user, course_id): # continue pass - if not hasattr(user, '_anonymous_id'): - user._anonymous_id = {} - - user._anonymous_id[course_id] = digest - return digest @@ -263,14 +269,17 @@ class UserProfile(models.Model): self.save() -def unique_id_for_user(user): +def unique_id_for_user(user, save=True): """ Return a unique id for a user, suitable for inserting into e.g. personalized survey links. + + Keyword arguments: + save -- Whether the id should be saved in an AnonymousUserId object. """ # Setting course_id to '' makes it not affect the generated hash, # and thus produce the old per-student anonymous id - return anonymous_id_for_user(user, None) + return anonymous_id_for_user(user, None, save=save) # TODO: Should be renamed to generic UserGroup, and possibly diff --git a/common/djangoapps/student/tests/tests.py b/common/djangoapps/student/tests/tests.py index a94c5ada64..b0caef85b4 100644 --- a/common/djangoapps/student/tests/tests.py +++ b/common/djangoapps/student/tests/tests.py @@ -491,3 +491,4 @@ class AnonymousLookupTable(TestCase): anonymous_id = anonymous_id_for_user(self.user, self.course.id) real_user = user_by_anonymous_id(anonymous_id) self.assertEqual(self.user, real_user) + self.assertEqual(anonymous_id, anonymous_id_for_user(self.user, self.course.id, save=False)) diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index c9aaee5861..b5451c5bdc 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -1334,7 +1334,7 @@ class TestInstructorAPILevelsDataDump(ModuleStoreTestCase, LoginEnrollmentTestCa self.assertEqual(response['Content-Type'], 'text/csv') body = response.content.replace('\r', '') self.assertTrue(body.startswith( - '"User ID","Anonymized user ID","Course Specific Anonymized user ID"' + '"User ID","Anonymized User ID","Course Specific Anonymized User ID"' '\n"2","41","42"\n' )) self.assertTrue(body.endswith('"7","41","42"\n')) diff --git a/lms/djangoapps/instructor/tests/test_legacy_anon_csv.py b/lms/djangoapps/instructor/tests/test_legacy_anon_csv.py index 678b96c1df..42fc923f51 100644 --- a/lms/djangoapps/instructor/tests/test_legacy_anon_csv.py +++ b/lms/djangoapps/instructor/tests/test_legacy_anon_csv.py @@ -63,6 +63,6 @@ class TestInstructorDashboardAnonCSV(ModuleStoreTestCase, LoginEnrollmentTestCas body = response.content.replace('\r', '') self.assertEqual( body, - ('"User ID","Anonymized user ID","Course Specific Anonymized user ID"' + ('"User ID","Anonymized User ID","Course Specific Anonymized User ID"' '\n"2","41","42"\n') ) diff --git a/lms/djangoapps/instructor/views/api.py b/lms/djangoapps/instructor/views/api.py index 9a2f23e6fc..f4822fcb52 100644 --- a/lms/djangoapps/instructor/views/api.py +++ b/lms/djangoapps/instructor/views/api.py @@ -630,8 +630,8 @@ def get_anon_ids(request, course_id): # pylint: disable=W0613 students = User.objects.filter( courseenrollment__course_id=course_id, ).order_by('id') - header = ['User ID', 'Anonymized user ID', 'Course Specific Anonymized user ID'] - rows = [[s.id, unique_id_for_user(s), anonymous_id_for_user(s, course_id)] for s in students] + header = ['User ID', 'Anonymized User ID', 'Course Specific Anonymized User ID'] + rows = [[s.id, unique_id_for_user(s, save=False), anonymous_id_for_user(s, course_id, save=False)] for s in students] return csv_response(course_id.to_deprecated_string().replace('/', '-') + '-anon-ids.csv', header, rows) diff --git a/lms/djangoapps/instructor/views/legacy.py b/lms/djangoapps/instructor/views/legacy.py index df23eb3154..36e51a4e1e 100644 --- a/lms/djangoapps/instructor/views/legacy.py +++ b/lms/djangoapps/instructor/views/legacy.py @@ -691,8 +691,8 @@ def instructor_dashboard(request, course_id): courseenrollment__course_id=course_key, ).order_by('id') - datatable = {'header': ['User ID', 'Anonymized user ID', 'Course Specific Anonymized user ID']} - datatable['data'] = [[s.id, unique_id_for_user(s), anonymous_id_for_user(s, course_id)] for s in students] + datatable = {'header': ['User ID', 'Anonymized User ID', 'Course Specific Anonymized User ID']} + datatable['data'] = [[s.id, unique_id_for_user(s, save=False), anonymous_id_for_user(s, course_key, save=False)] for s in students] return return_csv(course_key.to_deprecated_string().replace('/', '-') + '-anon-ids.csv', datatable) #----------------------------------------