From 0d824c6c02c1f1a43e303d129fd3bc238975429b Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 8 Aug 2012 15:06:03 -0400 Subject: [PATCH 01/16] Move modx_dispatch URL calls into the course --- common/lib/xmodule/xmodule/modulestore/__init__.py | 7 +++++++ lms/djangoapps/courseware/module_render.py | 14 +++++++++++--- lms/urls.py | 5 +++-- 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/common/lib/xmodule/xmodule/modulestore/__init__.py b/common/lib/xmodule/xmodule/modulestore/__init__.py index e59e4bd68e..6c77127d7e 100644 --- a/common/lib/xmodule/xmodule/modulestore/__init__.py +++ b/common/lib/xmodule/xmodule/modulestore/__init__.py @@ -190,6 +190,13 @@ class Location(_LocationBase): return "Location%s" % repr(tuple(self)) + @property + def course_id(self): + """Return the ID of the Course that this item belongs to by looking + at the location URL hierachy""" + return "/".join([self.org, self.course, self.name]) + + class ModuleStore(object): """ An abstract interface for a database backend that stores XModuleDescriptor diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index 852e3d8a77..ad656f8980 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -2,6 +2,7 @@ import json import logging from django.conf import settings +from django.core.urlresolvers import reverse from django.http import Http404 from django.http import HttpResponse from django.views.decorators.csrf import csrf_exempt @@ -125,7 +126,7 @@ def get_module(user, request, location, student_module_cache, position=None): ''' descriptor = modulestore().get_item(location) - + #TODO Only check the cache if this module can possibly have state instance_module = None shared_module = None @@ -146,7 +147,14 @@ def get_module(user, request, location, student_module_cache, position=None): # TODO (vshnayder): fix hardcoded urls (use reverse) # Setup system context for module instance - ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/' + + ajax_url = reverse('modx_dispatch', + kwargs=dict(course_id=descriptor.location.course_id, + id=descriptor.location.url(), + dispatch=''), + ) + + # ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/' # Fully qualified callback URL for external queueing system xqueue_callback_url = (request.build_absolute_uri('/') + settings.MITX_ROOT_URL + @@ -308,7 +316,7 @@ def xqueue_callback(request, userid, id, dispatch): return HttpResponse("") -def modx_dispatch(request, dispatch=None, id=None): +def modx_dispatch(request, dispatch=None, id=None, course_id=None): ''' Generic view for extensions. This is where AJAX calls go. Arguments: diff --git a/lms/urls.py b/lms/urls.py index bb3952b73c..c6cecb74d5 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -2,7 +2,6 @@ from django.conf import settings from django.conf.urls import patterns, include, url from django.contrib import admin from django.conf.urls.static import static - import django.contrib.auth.views # Uncomment the next two lines to enable the admin: @@ -101,7 +100,9 @@ if settings.COURSEWARE_ENABLED: url(r'^masquerade/', include('masquerade.urls')), url(r'^jump_to/(?P.*)$', 'courseware.views.jump_to', name="jump_to"), - url(r'^modx/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch'), #reset_problem'), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/modx/(?P.*?)/(?P[^/]*)$', + 'courseware.module_render.modx_dispatch', + name='modx_dispatch'), url(r'^xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.xqueue_callback'), url(r'^change_setting$', 'student.views.change_setting'), From 165d5dcfd34267a70fcf6a2d8f1b7633e740b223 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 8 Aug 2012 16:13:43 -0400 Subject: [PATCH 02/16] add config files for running different server groups in dev --- lms/envs/devgroups/__init__.py | 0 lms/envs/devgroups/courses.py | 43 ++++++++++++++++++++++++++++++++++ lms/envs/devgroups/h_cs50.py | 3 +++ lms/envs/devgroups/m_6002.py | 3 +++ lms/envs/devgroups/portal.py | 13 ++++++++++ 5 files changed, 62 insertions(+) create mode 100644 lms/envs/devgroups/__init__.py create mode 100644 lms/envs/devgroups/courses.py create mode 100644 lms/envs/devgroups/h_cs50.py create mode 100644 lms/envs/devgroups/m_6002.py create mode 100644 lms/envs/devgroups/portal.py diff --git a/lms/envs/devgroups/__init__.py b/lms/envs/devgroups/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lms/envs/devgroups/courses.py b/lms/envs/devgroups/courses.py new file mode 100644 index 0000000000..bcaae70a99 --- /dev/null +++ b/lms/envs/devgroups/courses.py @@ -0,0 +1,43 @@ +from ..dev import * + +CLASSES_TO_DBS = { + 'BerkeleyX/CS169.1x/2012_Fall' : "cs169.db", + 'BerkeleyX/CS188.1x/2012_Fall' : "cs188_1.db", + 'HarvardX/CS50x/2012' : "cs50.db", + 'HarvardX/PH207x/2012_Fall' : "ph207.db", + 'MITx/3.091x/2012_Fall' : "3091.db", + 'MITx/6.002x/2012_Fall' : "6002.db", + 'MITx/6.00x/2012_Fall' : "600.db", +} + + +CACHES = { + 'default': { + 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'LOCATION': '127.0.0.1:11211', + 'KEY_FUNCTION': 'util.memcache.safe_key', + }, + 'general': { + 'BACKEND': 'django.core.cache.backends.memcached.MemcachedCache', + 'LOCATION': '127.0.0.1:11211', + 'KEY_PREFIX' : 'general', + 'VERSION' : 5, + 'KEY_FUNCTION': 'util.memcache.safe_key', + } +} + +SESSION_ENGINE = 'django.contrib.sessions.backends.cache' + + +def path_for_db(db_name): + return ENV_ROOT / "db" / db_name + +def course_db_for(course_id): + db_name = CLASSES_TO_DBS[course_id] + return { + 'default' : { + 'ENGINE' : 'django.db.backends.sqlite3', + 'NAME' : path_for_db(db_name) + } + } + diff --git a/lms/envs/devgroups/h_cs50.py b/lms/envs/devgroups/h_cs50.py new file mode 100644 index 0000000000..b838b1fdc3 --- /dev/null +++ b/lms/envs/devgroups/h_cs50.py @@ -0,0 +1,3 @@ +from .courses import * + +DATABASES = course_db_for('HarvardX/CS50x/2012') \ No newline at end of file diff --git a/lms/envs/devgroups/m_6002.py b/lms/envs/devgroups/m_6002.py new file mode 100644 index 0000000000..3d8feef764 --- /dev/null +++ b/lms/envs/devgroups/m_6002.py @@ -0,0 +1,3 @@ +from .courses import * + +DATABASES = course_db_for('MITx/6.002x/2012_Fall') \ No newline at end of file diff --git a/lms/envs/devgroups/portal.py b/lms/envs/devgroups/portal.py new file mode 100644 index 0000000000..b674218571 --- /dev/null +++ b/lms/envs/devgroups/portal.py @@ -0,0 +1,13 @@ +""" +Note that for this to work at all, you must have memcached running (or you won't +get shared sessions) +""" +from courses import * + +# Move this to a shared file later: +for class_id, db_name in CLASSES_TO_DBS.items(): + DATABASES[class_id] = { + 'ENGINE': 'django.db.backends.sqlite3', + 'NAME': path_for_db(db_name) + } + From b597b17db1af129d5d8eb71022513ce5bfb13a7c Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Wed, 8 Aug 2012 18:12:33 -0400 Subject: [PATCH 03/16] Add support for replicating course enrollment/unenrollment to course databases --- common/djangoapps/student/models.py | 65 ++++++++++++++++++++++++++++- 1 file changed, 64 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 49d3381303..50175b2ac8 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -10,14 +10,21 @@ file and check it in at the same time as your model changes. To do that, """ from datetime import datetime import json +import logging import uuid -from django.db import models +from django.conf import settings from django.contrib.auth.models import User +from django.db import models +from django.db.models.signals import post_delete, post_save +from django.dispatch import receiver from django_countries import CountryField +from xmodule.modulestore.django import modulestore + #from cache_toolbox import cache_model, cache_relation +log = logging.getLogger(__name__) class UserProfile(models.Model): class Meta: @@ -203,3 +210,59 @@ def add_user_to_default_group(user, group): utg.save() utg.users.add(User.objects.get(username=user)) utg.save() + + + + + +################################# SIGNALS ###################################### + +def is_valid_course_id(course_id): + """We check to both make sure that it's a valid course_id (and not + 'default', or some other non-course DB name) and that we have a mapping + for what database it belongs to.""" + course_ids = set(course.id for course in modulestore().get_courses()) + return (course_id in course_ids) and (course_id in settings.DATABASES) + +def is_portal(): + """Are we in the portal pool? (in which case we'll have to replicate user + updates). Right now, that means we have more than one database defined.""" + return len(settings.DATABASES) > 1 + +def replicate_enrollment(instance_method, **kwargs): + log.debug("########## Enrollment replication called ############") + instance = kwargs['instance'] + + if not is_portal(): + log.debug("replicate_enrollment triggered, but we're not a portal so " + + "we're not propogating") + return + + if not is_valid_course_id(instance.course_id): + log.error("Don't know where to replicate to for course_id: {0}" + .format(instance.course_id)) + return + + # We create a _replicated attribute to differentiate the first save of this + # model vs. the duplicate save we force on to the course database. + if hasattr(instance, '_replicated'): + log.debug("We've already replicated this -- stopping so we don't go " + + "into an infinite loop.") + return + instance._replicated = True + + # instance_method is either CourseEnrollment.save or CourseEnrollment.delete + # using is the entry in DATABASES we push to (we use course_ids for names) + instance_method(instance, using=instance.course_id) + +@receiver(post_save, sender=CourseEnrollment) +def replicate_enrollment_save(sender, **kwargs): + return replicate_enrollment(CourseEnrollment.save, **kwargs) + +@receiver(post_delete, sender=CourseEnrollment) +def replicate_enrollment_delete(sender, **kwargs): + return replicate_enrollment(CourseEnrollment.delete, **kwargs) + + + + From 731e04e013d538d3dd275347f0498b7d08eaff17 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Thu, 9 Aug 2012 17:31:12 -0400 Subject: [PATCH 04/16] Add an explanation of replication --- common/djangoapps/student/models.py | 42 +++++++++++++++++++++++++---- 1 file changed, 37 insertions(+), 5 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 50175b2ac8..5e10e39b51 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1,5 +1,23 @@ """ -WE'RE USING MIGRATIONS! +Models for Student Information + +Replication Notes + +In our live deployment, we intend to run in a scenario where there is a pool of +Portal servers that hold the canoncial user information and that user +information is replicated to slave Course server pools. Each Course has a set of +servers that serves only its content and has users that are relevant only to it. + +We replicate the following tables into the Course DBs where the user is +enrolled. Only the Portal servers should ever write to these models. +* UserProfile +* CourseEnrollment + +We do a partial replication of: +* User -- Askbot extends this and uses the extra fields, so we replicate only + the stuff that comes with basic django_auth and ignore the rest.) + +Migration Notes If you make changes to this model, be sure to create an appropriate migration file and check it in at the same time as your model changes. To do that, @@ -27,6 +45,24 @@ from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) class UserProfile(models.Model): + """This is where we store all the user demographic fields. We have a + separate table for this rather than extending the built-in Django auth_user. + + Notes: + * Some fields are legacy ones from the first run of 6.002, from which + we imported many users. + * Fields like name and address are intentionally open ended, to account + for international variations. An unfortunate side-effect is that we + cannot efficiently sort on last names for instance. + + Replication: + * Only the Portal servers should ever modify this information. + * All fields are replicated into relevant Course databases + + Some of the fields are legacy ones that were captured during the initial + MITx fall prototype. + """ + class Meta: db_table = "auth_userprofile" @@ -211,10 +247,6 @@ def add_user_to_default_group(user, group): utg.users.add(User.objects.get(username=user)) utg.save() - - - - ################################# SIGNALS ###################################### def is_valid_course_id(course_id): From 8d9297ea04f32c6fb27a658cf43527285d07a819 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Fri, 10 Aug 2012 02:02:11 -0400 Subject: [PATCH 05/16] Some refactoring of how user info is copied over when enrollments are created. --- common/djangoapps/student/models.py | 156 +++++++++++++++++++++------- 1 file changed, 121 insertions(+), 35 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 5e10e39b51..1d5926f2be 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -17,6 +17,13 @@ We do a partial replication of: * User -- Askbot extends this and uses the extra fields, so we replicate only the stuff that comes with basic django_auth and ignore the rest.) +There are a couple different scenarios: + +1. There's an update of User or UserProfile -- replicate it to all Course DBs + that the user is enrolled in (found via CourseEnrollment). +2. There's a change in CourseEnrollment. We need to push copies of UserProfile, + CourseEnrollment, and the base fields in User + Migration Notes If you make changes to this model, be sure to create an appropriate migration @@ -247,53 +254,132 @@ def add_user_to_default_group(user, group): utg.users.add(User.objects.get(username=user)) utg.save() -################################# SIGNALS ###################################### +########################## REPLICATION SIGNALS ################################# +@receiver(post_save, sender=CourseEnrollment) +def replicate_enrollment_save(sender, **kwargs): + """This is called when a Student enrolls in a course. It has to do the + following: + + 1. Make sure the User is copied into the Course DB. It may already exist + (someone deleting and re-adding a course). This has to happen first or + the foreign key constraint breaks. + 2. Replicate the CourseEnrollment. + 3. Replicate the UserProfile. + """ + enrollment_obj = kwargs['instance'] + replicate_user(enrollment_obj.user, enrollment_obj.course_id) + replicate_model(CourseEnrollment.save, enrollment_obj.user_id, **kwargs) + replicate_model(UserProfile.save, enrollment_obj.user_id, **kwargs) + +@receiver(post_delete, sender=CourseEnrollment) +def replicate_enrollment_delete(sender, **kwargs): + enrollment_obj = kwargs['instance'] + return replicate_model(CourseEnrollment.delete, enrollment_obj.user_id, **kwargs) + +@receiver(post_save, sender=UserProfile) +def replicate_userprofile_save(sender, **kwargs): + """We just updated the UserProfile (say an update to the name), so push that + change to all Course DBs that we're enrolled in.""" + user_profile_obj = kwargs['instance'] + return replicate_model(UserProfile.save, enrollment_obj.user_id, **kwargs) + +######### Replication functions ######### +def replicate_user(portal_user, course_db_name): + """Replicate a User to the correct Course DB. This is more complicated than + it should be because Askbot extends the auth_user table and adds its own + fields. So we need to only push changes to the standard fields and leave + the rest alone so that Askbot can + """ + try: + # If the user exists in the Course DB, update the appropriate fields and + # save it back out to the Course DB. + course_user = User.objects.using(course_db_name).get(portal_user.id) + fields_to_copy = ["username", "first_name", "last_name", "email", + "password", "is_staff", "is_active", "is_superuser", + "last_login", "date_joined"] + for field in fields_to_copy: + setattr(course_user, field, getattr(portal_user, field)) + + mark_handled(course_user) + course_user.save(using=course_db_name) # Just being explicit. + + except User.DoesNotExist: + # Otherwise, just make a straight copy to the Course DB. + mark_handled(portal_user) + portal_user.save(using=course_db_name) + +def replicate_model(model_method, user_id, **kwargs): + """ + model_method is the model action that we want replicated. For instance, + UserProfile.save + """ + instance = kwargs['instance'] + if not should_replicate(instance): + return + + mark_handled(instance) + course_db_names = db_names_to_replicate_to(user_id) + log.debug("Replicating {0} for user {1} to DBs: {2}" + .format(model_method, user_id, course_db_names)) + + for db_name in course_db_names: + model_method(instance, using=db_name) + +######### Replication Helpers ######### def is_valid_course_id(course_id): """We check to both make sure that it's a valid course_id (and not 'default', or some other non-course DB name) and that we have a mapping for what database it belongs to.""" course_ids = set(course.id for course in modulestore().get_courses()) - return (course_id in course_ids) and (course_id in settings.DATABASES) + is_valid = (course_id in course_ids) and (course_id in settings.DATABASES) + if not is_valid: + log.error("{0} is not a valid DB to replicate to.".format(course_id)) + return is_valid def is_portal(): - """Are we in the portal pool? (in which case we'll have to replicate user - updates). Right now, that means we have more than one database defined.""" + """Are we in the portal pool? Only Portal servers are allowed to replicate + their changes. For now, only Portal servers see multiple DBs, so we use + that to decide.""" return len(settings.DATABASES) > 1 -def replicate_enrollment(instance_method, **kwargs): - log.debug("########## Enrollment replication called ############") - instance = kwargs['instance'] +def db_names_to_replicate_to(user_id): + """Return a list of DB names that this user_id is enrolled in.""" + return [c.course_id + for c in CourseEnrollment.objects.filter(user_id=user_id) + if is_valid_course_id(c.course_id)] +def marked_handled(instance): + """Have we marked this instance as being handled to avoid infinite loops + caused by saving models in post_save hooks for the same models?""" + return hasattr(instance, '_do_not_copy_to_course_db') + +def mark_handled(instance): + """You have to mark your instance with this function or else we'll go into + an infinite loop since we're putting listeners on Model saves/deletes and + the act of replication requires us to call the same model method. + + We create a _replicated attribute to differentiate the first save of this + model vs. the duplicate save we force on to the course database. Kind of + a hack -- suggestions welcome. + """ + instance._do_not_copy_to_course_db = True + +def should_replicate(instance): + """Should this instance be replicated? We need to be a Portal server and + the instance has to not have been marked_handled.""" + if marked_handled(instance): + # Basically, avoid an infinite loop. You should + log.debug("{0} should not be replicated because it's been marked") + return False if not is_portal(): - log.debug("replicate_enrollment triggered, but we're not a portal so " + - "we're not propogating") - return - - if not is_valid_course_id(instance.course_id): - log.error("Don't know where to replicate to for course_id: {0}" - .format(instance.course_id)) - return - - # We create a _replicated attribute to differentiate the first save of this - # model vs. the duplicate save we force on to the course database. - if hasattr(instance, '_replicated'): - log.debug("We've already replicated this -- stopping so we don't go " + - "into an infinite loop.") - return - instance._replicated = True - - # instance_method is either CourseEnrollment.save or CourseEnrollment.delete - # using is the entry in DATABASES we push to (we use course_ids for names) - instance_method(instance, using=instance.course_id) - -@receiver(post_save, sender=CourseEnrollment) -def replicate_enrollment_save(sender, **kwargs): - return replicate_enrollment(CourseEnrollment.save, **kwargs) - -@receiver(post_delete, sender=CourseEnrollment) -def replicate_enrollment_delete(sender, **kwargs): - return replicate_enrollment(CourseEnrollment.delete, **kwargs) + log.debug("{0} should not be replicated because we're not a portal." + .format(instance)) + return False + return True + + + From 172072209f833eec10ae8b5bfdb9d6445acbcc33 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Fri, 10 Aug 2012 10:15:55 -0400 Subject: [PATCH 06/16] Add common djangoapps to tests run --- rakefile | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/rakefile b/rakefile index caf0d58f2f..c62c87701e 100644 --- a/rakefile +++ b/rakefile @@ -88,7 +88,8 @@ $failed_tests = 0 def run_tests(system, report_dir, stop_on_failure=true) ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") ENV['NOSE_COVER_HTML_DIR'] = File.join(report_dir, "cover") - sh(django_admin(system, :test, 'test', *Dir["#{system}/djangoapps/*"].each)) do |ok, res| + dirs = Dir["common/djangoapps/*"] + Dir["#{system}/djangoapps/*"] + sh(django_admin(system, :test, 'test', *dirs.each)) do |ok, res| if !ok and stop_on_failure abort "Test failed!" end From b3676cd760ac41037fc994faf157b6afed5df883 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Fri, 10 Aug 2012 14:43:43 -0400 Subject: [PATCH 07/16] Add replication tests --- common/djangoapps/student/models.py | 38 +++++-- common/djangoapps/student/tests.py | 166 +++++++++++++++++++++++++++- lms/envs/test.py | 11 ++ 3 files changed, 197 insertions(+), 18 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 1d5926f2be..a97bb99cd2 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -255,6 +255,11 @@ def add_user_to_default_group(user, group): utg.save() ########################## REPLICATION SIGNALS ################################# +@receiver(post_save, sender=User) +def replicate_user_save(sender, **kwargs): + user_obj = kwargs['instance'] + return replicate_model(User.save, user_obj.id, **kwargs) + @receiver(post_save, sender=CourseEnrollment) def replicate_enrollment_save(sender, **kwargs): """This is called when a Student enrolls in a course. It has to do the @@ -266,38 +271,44 @@ def replicate_enrollment_save(sender, **kwargs): 2. Replicate the CourseEnrollment. 3. Replicate the UserProfile. """ + if not is_portal(): + return + enrollment_obj = kwargs['instance'] replicate_user(enrollment_obj.user, enrollment_obj.course_id) replicate_model(CourseEnrollment.save, enrollment_obj.user_id, **kwargs) replicate_model(UserProfile.save, enrollment_obj.user_id, **kwargs) - + @receiver(post_delete, sender=CourseEnrollment) def replicate_enrollment_delete(sender, **kwargs): - enrollment_obj = kwargs['instance'] - return replicate_model(CourseEnrollment.delete, enrollment_obj.user_id, **kwargs) - + enrollment_obj = kwargs['instance'] + return replicate_model(CourseEnrollment.delete, enrollment_obj.user_id, **kwargs) + @receiver(post_save, sender=UserProfile) def replicate_userprofile_save(sender, **kwargs): """We just updated the UserProfile (say an update to the name), so push that change to all Course DBs that we're enrolled in.""" user_profile_obj = kwargs['instance'] - return replicate_model(UserProfile.save, enrollment_obj.user_id, **kwargs) + return replicate_model(UserProfile.save, user_profile_obj.user_id, **kwargs) + ######### Replication functions ######### +USER_FIELDS_TO_COPY = ["id", "username", "first_name", "last_name", "email", + "password", "is_staff", "is_active", "is_superuser", + "last_login", "date_joined"] + def replicate_user(portal_user, course_db_name): """Replicate a User to the correct Course DB. This is more complicated than it should be because Askbot extends the auth_user table and adds its own fields. So we need to only push changes to the standard fields and leave - the rest alone so that Askbot can + the rest alone so that Askbot changes at the Course DB level don't get + overridden. """ try: # If the user exists in the Course DB, update the appropriate fields and # save it back out to the Course DB. - course_user = User.objects.using(course_db_name).get(portal_user.id) - fields_to_copy = ["username", "first_name", "last_name", "email", - "password", "is_staff", "is_active", "is_superuser", - "last_login", "date_joined"] - for field in fields_to_copy: + course_user = User.objects.using(course_db_name).get(id=portal_user.id) + for field in USER_FIELDS_TO_COPY: setattr(course_user, field, getattr(portal_user, field)) mark_handled(course_user) @@ -331,6 +342,8 @@ def is_valid_course_id(course_id): """We check to both make sure that it's a valid course_id (and not 'default', or some other non-course DB name) and that we have a mapping for what database it belongs to.""" + return course_id != 'default' + course_ids = set(course.id for course in modulestore().get_courses()) is_valid = (course_id in course_ids) and (course_id in settings.DATABASES) if not is_valid: @@ -370,7 +383,8 @@ def should_replicate(instance): the instance has to not have been marked_handled.""" if marked_handled(instance): # Basically, avoid an infinite loop. You should - log.debug("{0} should not be replicated because it's been marked") + log.debug("{0} should not be replicated because it's been marked" + .format(instance)) return False if not is_portal(): log.debug("{0} should not be replicated because we're not a portal." diff --git a/common/djangoapps/student/tests.py b/common/djangoapps/student/tests.py index 501deb776c..974af6e3b3 100644 --- a/common/djangoapps/student/tests.py +++ b/common/djangoapps/student/tests.py @@ -4,13 +4,167 @@ when you run "manage.py test". Replace this with more appropriate tests for your application. """ +from datetime import datetime from django.test import TestCase +from .models import User, UserProfile, CourseEnrollment, replicate_user, USER_FIELDS_TO_COPY + +COURSE_1 = 'edX/toy/2012_Fall' +COURSE_2 = 'edx/full/6.002_Spring_2012' + +class ReplicationTest(TestCase): + + multi_db = True + + def test_user_replication(self): + """Test basic user replication.""" + portal_user = User.objects.create_user('rusty', 'rusty@edx.org', 'fakepass') + portal_user.first_name='Rusty' + portal_user.last_name='Skids' + portal_user.is_staff=True + portal_user.is_active=True + portal_user.is_superuser=True + portal_user.last_login=datetime(2012, 1, 1) + portal_user.date_joined=datetime(2011, 1, 1) + # This is an Askbot field and will break if askbot is not included + portal_user.seen_response_count = 10 + + portal_user.save(using='default') + + # We replicate this user to Course 1, then pull the same user and verify + # that the fields copied over properly. + replicate_user(portal_user, COURSE_1) + course_user = User.objects.using(COURSE_1).get(id=portal_user.id) + + # Make sure the fields we care about got copied over for this user. + for field in USER_FIELDS_TO_COPY: + self.assertEqual(getattr(portal_user, field), + getattr(course_user, field), + "{0} not copied from {1} to {2}".format( + field, portal_user, course_user + )) + + # Since it's the first copy over of User data, we should have all of it + self.assertEqual(portal_user.seen_response_count, + course_user.seen_response_count) + + # But if we replicate again, the user already exists in the Course DB, + # so it shouldn't update the seen_response_count (which is Askbot + # controlled) + portal_user.seen_response_count = 20 + replicate_user(portal_user, COURSE_1) + course_user = User.objects.using(COURSE_1).get(id=portal_user.id) + self.assertEqual(portal_user.seen_response_count, 20) + self.assertEqual(course_user.seen_response_count, 10) + + # Another replication should work for an email change however, since + # it's a field we care about. + portal_user.email = "clyde@edx.org" + replicate_user(portal_user, COURSE_1) + course_user = User.objects.using(COURSE_1).get(id=portal_user.id) + self.assertEqual(portal_user.email, course_user.email) + + # During this entire time, the user data should never have made it over + # to COURSE_2 + self.assertRaises(User.DoesNotExist, + User.objects.using(COURSE_2).get, + id=portal_user.id) + + + def test_enrollment_for_existing_user_info(self): + """Test the effect of Enrolling in a class if you've already got user + data to be copied over.""" + # Create our User + portal_user = User.objects.create_user('jack', 'jack@edx.org', 'fakepass') + portal_user.first_name = "Jack" + portal_user.save() + + # Set up our UserProfile info + portal_user_profile = UserProfile.objects.create( + user=portal_user, + name="Jack Foo", + level_of_education=None, + gender='m', + mailing_address=None, + goals="World domination", + ) + portal_user_profile.save() + + # Now let's see if creating a CourseEnrollment copies all the relevant + # data. + portal_enrollment = CourseEnrollment.objects.create(user=portal_user, + course_id=COURSE_1) + portal_enrollment.save() + + # Grab all the copies we expect + course_user = User.objects.using(COURSE_1).get(id=portal_user.id) + self.assertEquals(portal_user, course_user) + self.assertRaises(User.DoesNotExist, + User.objects.using(COURSE_2).get, + id=portal_user.id) + + course_enrollment = CourseEnrollment.objects.using(COURSE_1).get(id=portal_enrollment.id) + self.assertEquals(portal_enrollment, course_enrollment) + self.assertRaises(CourseEnrollment.DoesNotExist, + CourseEnrollment.objects.using(COURSE_2).get, + id=portal_enrollment.id) + + course_user_profile = UserProfile.objects.using(COURSE_1).get(id=portal_user_profile.id) + self.assertEquals(portal_user_profile, course_user_profile) + self.assertRaises(UserProfile.DoesNotExist, + UserProfile.objects.using(COURSE_2).get, + id=portal_user_profile.id) + + + def test_enrollment_for_user_info_after_enrollment(self): + """Test the effect of Enrolling in a class if you've already got user + data to be copied over.""" + # Create our User + portal_user = User.objects.create_user('jack', 'jack@edx.org', 'fakepass') + portal_user.first_name = "Jack" + portal_user.save() + + # Now let's see if creating a CourseEnrollment copies all the relevant + # data when things are saved. + portal_enrollment = CourseEnrollment.objects.create(user=portal_user, + course_id=COURSE_1) + portal_enrollment.save() + + # Set up our UserProfile info + portal_user_profile = UserProfile.objects.create( + user=portal_user, + name="Jack Foo", + level_of_education=None, + gender='m', + mailing_address=None, + goals="World domination", + ) + portal_user_profile.save() + + # Grab all the copies we expect, and make sure it doesn't end up in + # places we don't expect. + course_user = User.objects.using(COURSE_1).get(id=portal_user.id) + self.assertEquals(portal_user, course_user) + self.assertRaises(User.DoesNotExist, + User.objects.using(COURSE_2).get, + id=portal_user.id) + + course_enrollment = CourseEnrollment.objects.using(COURSE_1).get(id=portal_enrollment.id) + self.assertEquals(portal_enrollment, course_enrollment) + self.assertRaises(CourseEnrollment.DoesNotExist, + CourseEnrollment.objects.using(COURSE_2).get, + id=portal_enrollment.id) + + course_user_profile = UserProfile.objects.using(COURSE_1).get(id=portal_user_profile.id) + self.assertEquals(portal_user_profile, course_user_profile) + self.assertRaises(UserProfile.DoesNotExist, + UserProfile.objects.using(COURSE_2).get, + id=portal_user_profile.id) + + + + + + -class SimpleTest(TestCase): - def test_basic_addition(self): - """ - Tests that 1 + 1 always equals 2. - """ - self.assertEqual(1 + 1, 2) diff --git a/lms/envs/test.py b/lms/envs/test.py index 7d38e1fbb9..e0caa79ac1 100644 --- a/lms/envs/test.py +++ b/lms/envs/test.py @@ -66,6 +66,17 @@ DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', 'NAME': PROJECT_ROOT / "db" / "mitx.db", + }, + + # The following are for testing purposes... + 'edX/toy/2012_Fall': { + 'ENGINE': 'django.db.backends.sqlite3', + 'NAME': ENV_ROOT / "db" / "course1.db", + }, + + 'edx/full/6.002_Spring_2012': { + 'ENGINE': 'django.db.backends.sqlite3', + 'NAME': ENV_ROOT / "db" / "course2.db", } } From cbfdf59760c6a65052f2896bb6d14d91d0e156f4 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Fri, 10 Aug 2012 16:41:46 -0400 Subject: [PATCH 08/16] Fix test error regarding UserProfiles (a UserProfile must exist before a CourseEnrollment) --- common/djangoapps/student/models.py | 26 ++++++++++++++++------- common/djangoapps/student/tests.py | 32 ++++++++++++++++------------- rakefile | 2 +- 3 files changed, 38 insertions(+), 22 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index a97bb99cd2..4c47e6c67e 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -258,7 +258,7 @@ def add_user_to_default_group(user, group): @receiver(post_save, sender=User) def replicate_user_save(sender, **kwargs): user_obj = kwargs['instance'] - return replicate_model(User.save, user_obj.id, **kwargs) + return replicate_model(User.save, user_obj, user_obj.id) @receiver(post_save, sender=CourseEnrollment) def replicate_enrollment_save(sender, **kwargs): @@ -275,21 +275,27 @@ def replicate_enrollment_save(sender, **kwargs): return enrollment_obj = kwargs['instance'] + log.debug("Replicating user because of new enrollment") replicate_user(enrollment_obj.user, enrollment_obj.course_id) - replicate_model(CourseEnrollment.save, enrollment_obj.user_id, **kwargs) - replicate_model(UserProfile.save, enrollment_obj.user_id, **kwargs) + + log.debug("Replicating enrollment because of new enrollment") + replicate_model(CourseEnrollment.save, enrollment_obj, enrollment_obj.user_id) + + log.debug("Replicating user profile because of new enrollment") + user_profile = UserProfile.objects.get(user_id=enrollment_obj.user_id) + replicate_model(UserProfile.save, user_profile, enrollment_obj.user_id) @receiver(post_delete, sender=CourseEnrollment) def replicate_enrollment_delete(sender, **kwargs): enrollment_obj = kwargs['instance'] - return replicate_model(CourseEnrollment.delete, enrollment_obj.user_id, **kwargs) + return replicate_model(CourseEnrollment.delete, enrollment_obj, enrollment_obj.user_id) @receiver(post_save, sender=UserProfile) def replicate_userprofile_save(sender, **kwargs): """We just updated the UserProfile (say an update to the name), so push that change to all Course DBs that we're enrolled in.""" user_profile_obj = kwargs['instance'] - return replicate_model(UserProfile.save, user_profile_obj.user_id, **kwargs) + return replicate_model(UserProfile.save, user_profile_obj, user_profile_obj.user_id) ######### Replication functions ######### @@ -312,19 +318,25 @@ def replicate_user(portal_user, course_db_name): setattr(course_user, field, getattr(portal_user, field)) mark_handled(course_user) + log.debug("User {0} found in Course DB, replicating fields to {1}" + .format(course_user, course_db_name)) course_user.save(using=course_db_name) # Just being explicit. except User.DoesNotExist: # Otherwise, just make a straight copy to the Course DB. mark_handled(portal_user) + log.debug("User {0} not found in Course DB, creating copy in {1}" + .format(portal_user, course_db_name)) portal_user.save(using=course_db_name) -def replicate_model(model_method, user_id, **kwargs): +def replicate_model(model_method, instance, user_id): """ model_method is the model action that we want replicated. For instance, UserProfile.save """ - instance = kwargs['instance'] + if isinstance(instance, UserProfile): + log.debug("replicate_model called on UserProfile {0}".format(instance)) + if not should_replicate(instance): return diff --git a/common/djangoapps/student/tests.py b/common/djangoapps/student/tests.py index 974af6e3b3..64b1845b7c 100644 --- a/common/djangoapps/student/tests.py +++ b/common/djangoapps/student/tests.py @@ -118,30 +118,34 @@ class ReplicationTest(TestCase): def test_enrollment_for_user_info_after_enrollment(self): - """Test the effect of Enrolling in a class if you've already got user - data to be copied over.""" + """Test the effect of modifying User data after you've enrolled.""" # Create our User - portal_user = User.objects.create_user('jack', 'jack@edx.org', 'fakepass') - portal_user.first_name = "Jack" + portal_user = User.objects.create_user('patty', 'patty@edx.org', 'fakepass') + portal_user.first_name = "Patty" portal_user.save() + # Set up our UserProfile info + portal_user_profile = UserProfile.objects.create( + user=portal_user, + name="Patty Foo", + level_of_education=None, + gender='f', + mailing_address=None, + goals="World peace", + ) + portal_user_profile.save() + # Now let's see if creating a CourseEnrollment copies all the relevant # data when things are saved. portal_enrollment = CourseEnrollment.objects.create(user=portal_user, course_id=COURSE_1) portal_enrollment.save() - # Set up our UserProfile info - portal_user_profile = UserProfile.objects.create( - user=portal_user, - name="Jack Foo", - level_of_education=None, - gender='m', - mailing_address=None, - goals="World domination", - ) + portal_user.last_name = "Bar" + portal_user.save() + portal_user_profile.gender = 'm' portal_user_profile.save() - + # Grab all the copies we expect, and make sure it doesn't end up in # places we don't expect. course_user = User.objects.using(COURSE_1).get(id=portal_user.id) diff --git a/rakefile b/rakefile index c62c87701e..2aeb05dc1e 100644 --- a/rakefile +++ b/rakefile @@ -88,7 +88,7 @@ $failed_tests = 0 def run_tests(system, report_dir, stop_on_failure=true) ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") ENV['NOSE_COVER_HTML_DIR'] = File.join(report_dir, "cover") - dirs = Dir["common/djangoapps/*"] + Dir["#{system}/djangoapps/*"] + dirs = Dir["common/djangoapps/*"] # + Dir["#{system}/djangoapps/*"] sh(django_admin(system, :test, 'test', *dirs.each)) do |ok, res| if !ok and stop_on_failure abort "Test failed!" From 689c761bbc4897518a965b4369470315c8b31494 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Fri, 10 Aug 2012 17:07:03 -0400 Subject: [PATCH 09/16] Remove comment in Rakefile I was just testing. --- rakefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rakefile b/rakefile index 2aeb05dc1e..c62c87701e 100644 --- a/rakefile +++ b/rakefile @@ -88,7 +88,7 @@ $failed_tests = 0 def run_tests(system, report_dir, stop_on_failure=true) ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") ENV['NOSE_COVER_HTML_DIR'] = File.join(report_dir, "cover") - dirs = Dir["common/djangoapps/*"] # + Dir["#{system}/djangoapps/*"] + dirs = Dir["common/djangoapps/*"] + Dir["#{system}/djangoapps/*"] sh(django_admin(system, :test, 'test', *dirs.each)) do |ok, res| if !ok and stop_on_failure abort "Test failed!" From d41f709c92f086957f4d3667964aaa13389da029 Mon Sep 17 00:00:00 2001 From: kimth Date: Fri, 10 Aug 2012 18:51:35 -0400 Subject: [PATCH 10/16] Moved xqueue_callback URLs into the course --- lms/djangoapps/courseware/module_render.py | 12 ++++++++---- lms/urls.py | 4 +++- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/lms/djangoapps/courseware/module_render.py b/lms/djangoapps/courseware/module_render.py index ad656f8980..6d212d6d2c 100644 --- a/lms/djangoapps/courseware/module_render.py +++ b/lms/djangoapps/courseware/module_render.py @@ -157,9 +157,13 @@ def get_module(user, request, location, student_module_cache, position=None): # ajax_url = settings.MITX_ROOT_URL + '/modx/' + descriptor.location.url() + '/' # Fully qualified callback URL for external queueing system - xqueue_callback_url = (request.build_absolute_uri('/') + settings.MITX_ROOT_URL + - 'xqueue/' + str(user.id) + '/' + descriptor.location.url() + '/' + - 'score_update') + xqueue_callback_url = request.build_absolute_uri('/')[:-1] # Trailing slash provided by reverse + xqueue_callback_url += reverse('xqueue_callback', + kwargs=dict(course_id=descriptor.location.course_id, + userid=str(user.id), + id=descriptor.location.url(), + dispatch='score_update'), + ) # Default queuename is course-specific and is derived from the course that # contains the current module. @@ -265,7 +269,7 @@ def get_shared_instance_module(user, module, student_module_cache): return None @csrf_exempt -def xqueue_callback(request, userid, id, dispatch): +def xqueue_callback(request, course_id, userid, id, dispatch): ''' Entry point for graded results from the queueing system. ''' diff --git a/lms/urls.py b/lms/urls.py index c6cecb74d5..ae1609bd0f 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -103,7 +103,9 @@ if settings.COURSEWARE_ENABLED: url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/modx/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.modx_dispatch', name='modx_dispatch'), - url(r'^xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.xqueue_callback'), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', + 'courseware.module_render.xqueue_callback', + name='xqueue_callback'), url(r'^change_setting$', 'student.views.change_setting'), # TODO: These views need to be updated before they work From 54b0a465faca18184dbef40c27a9363141ffbdbe Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Sun, 12 Aug 2012 16:10:08 -0400 Subject: [PATCH 11/16] Ugly hack so that an LMS-specific test of the shared Student djangoapp doesn't break CMS test runs --- common/djangoapps/student/tests.py | 27 +++++++++++++++++---------- 1 file changed, 17 insertions(+), 10 deletions(-) diff --git a/common/djangoapps/student/tests.py b/common/djangoapps/student/tests.py index 64b1845b7c..ad7ddb70d1 100644 --- a/common/djangoapps/student/tests.py +++ b/common/djangoapps/student/tests.py @@ -28,7 +28,9 @@ class ReplicationTest(TestCase): portal_user.last_login=datetime(2012, 1, 1) portal_user.date_joined=datetime(2011, 1, 1) # This is an Askbot field and will break if askbot is not included - portal_user.seen_response_count = 10 + + if hasattr(portal_user, 'seen_response_count'): + portal_user.seen_response_count = 10 portal_user.save(using='default') @@ -45,18 +47,23 @@ class ReplicationTest(TestCase): field, portal_user, course_user )) - # Since it's the first copy over of User data, we should have all of it - self.assertEqual(portal_user.seen_response_count, - course_user.seen_response_count) + if hasattr(portal_user, 'seen_response_count'): + # Since it's the first copy over of User data, we should have all of it + self.assertEqual(portal_user.seen_response_count, + course_user.seen_response_count) # But if we replicate again, the user already exists in the Course DB, # so it shouldn't update the seen_response_count (which is Askbot - # controlled) - portal_user.seen_response_count = 20 - replicate_user(portal_user, COURSE_1) - course_user = User.objects.using(COURSE_1).get(id=portal_user.id) - self.assertEqual(portal_user.seen_response_count, 20) - self.assertEqual(course_user.seen_response_count, 10) + # controlled). + # This hasattr lameness is here because we don't want this test to be + # triggered when we're being run by CMS tests (Askbot doesn't exist + # there, so the test will fail). + if hasattr(portal_user, 'seen_response_count'): + portal_user.seen_response_count = 20 + replicate_user(portal_user, COURSE_1) + course_user = User.objects.using(COURSE_1).get(id=portal_user.id) + self.assertEqual(portal_user.seen_response_count, 20) + self.assertEqual(course_user.seen_response_count, 10) # Another replication should work for an email change however, since # it's a field we care about. From 7711f81303f200752acd924ee0775bd18e698e88 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Sun, 12 Aug 2012 23:35:50 -0400 Subject: [PATCH 12/16] Add nginx.conf file for running a multi-server dev env --- proxy/nginx.conf | 67 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) create mode 100644 proxy/nginx.conf diff --git a/proxy/nginx.conf b/proxy/nginx.conf new file mode 100644 index 0000000000..470c3933ac --- /dev/null +++ b/proxy/nginx.conf @@ -0,0 +1,67 @@ +# Mapping of +# +# From the /mitx directory: +# /usr/local/Cellar/nginx/1.2.2/sbin/nginx -p `pwd`/ -c nginx.conf + +worker_processes 1; + +events { + worker_connections 1024; +} + +http { + ## + # Basic Settings + ## + sendfile on; + tcp_nopush on; + tcp_nodelay on; + keepalive_timeout 65; + types_hash_max_size 2048; + # server_tokens off; + # server_names_hash_bucket_size 64; + # server_name_in_redirect off; + + include /usr/local/etc/nginx/mime.types; + default_type application/octet-stream; + + ## + # Gzip Settings + ## + gzip on; + gzip_disable "msie6"; + + upstream portal { + server localhost:8000; + } + + upstream course_harvardx_cs50_2012 { + server localhost:8001; + } + + upstream course_mitx_6002_2012_fall { + server localhost:8002; + } + + # Mostly copied from our existing server... + server { + listen 8100 default_server; + + rewrite ^(.*)/favicon.ico$ /static/images/favicon.ico last; + + # Our catchall + location / { + proxy_pass http://portal; + } + + location /courses/HarvardX/CS50x/2012/ { + proxy_pass http://course_harvardx_cs50_2012; + } + + location /courses/MITx/6.002x/2012_Fall/ { + proxy_pass http://course_mitx_6002_2012_fall; + } + } +} + + From 510435ada5325a47542c5fc23ed9c6ca425de7a3 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 13 Aug 2012 00:35:51 -0400 Subject: [PATCH 13/16] Sketchy adding of test dbs to the cms test.py config file -- these are required for the Student (shared django app) tests to run --- cms/envs/test.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/cms/envs/test.py b/cms/envs/test.py index bce3c796cf..3823cd9dd9 100644 --- a/cms/envs/test.py +++ b/cms/envs/test.py @@ -55,6 +55,17 @@ DATABASES = { 'default': { 'ENGINE': 'django.db.backends.sqlite3', 'NAME': ENV_ROOT / "db" / "cms.db", + }, + + # The following are for testing purposes... + 'edX/toy/2012_Fall': { + 'ENGINE': 'django.db.backends.sqlite3', + 'NAME': ENV_ROOT / "db" / "course1.db", + }, + + 'edx/full/6.002_Spring_2012': { + 'ENGINE': 'django.db.backends.sqlite3', + 'NAME': ENV_ROOT / "db" / "course2.db", } } From 0aad62d6a82c7157176968f5d011ec90e427216d Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 13 Aug 2012 00:48:35 -0400 Subject: [PATCH 14/16] Remove extra braces that were causing mako syntax errors --- lms/templates/gradebook.html | 4 ++-- lms/templates/instructor_dashboard.html | 4 ++-- lms/templates/profile.html | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lms/templates/gradebook.html b/lms/templates/gradebook.html index 5b88a7f2b6..a4a81a6868 100644 --- a/lms/templates/gradebook.html +++ b/lms/templates/gradebook.html @@ -61,8 +61,8 @@ %for student in students: + kwargs=dict(course_id=course_id, + student_id=student['id']))}"> ${student['username']} %for section in student['grade_summary']['section_breakdown']: ${percent_data( section['percent'] )} diff --git a/lms/templates/instructor_dashboard.html b/lms/templates/instructor_dashboard.html index a5b333a809..6b87f63031 100644 --- a/lms/templates/instructor_dashboard.html +++ b/lms/templates/instructor_dashboard.html @@ -14,10 +14,10 @@

Instructor Dashboard

- Gradebook + Gradebook

- Grade summary + Grade summary diff --git a/lms/templates/profile.html b/lms/templates/profile.html index a6e28c6e57..ca27920a1b 100644 --- a/lms/templates/profile.html +++ b/lms/templates/profile.html @@ -137,7 +137,7 @@ $(function() { percentageString = "{0:.0%}".format( float(earned)/total) if earned > 0 and total > 0 else "" %> -

+

${ section['display_name'] } ${"({0:.3n}/{1:.3n}) {2}".format( float(earned), float(total), percentageString )}

${section['format']} From d24ee256150b368dd19328a168aa46223e9d771f Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 13 Aug 2012 00:57:33 -0400 Subject: [PATCH 15/16] Remove debug checking for UserProfile --- common/djangoapps/student/models.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 4c47e6c67e..257d3cb0f5 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -334,9 +334,6 @@ def replicate_model(model_method, instance, user_id): model_method is the model action that we want replicated. For instance, UserProfile.save """ - if isinstance(instance, UserProfile): - log.debug("replicate_model called on UserProfile {0}".format(instance)) - if not should_replicate(instance): return From 3cabb2dea63a378878a28f35ea838538654a50a8 Mon Sep 17 00:00:00 2001 From: David Ormsbee Date: Mon, 13 Aug 2012 01:00:55 -0400 Subject: [PATCH 16/16] Remove the no-longer-used is_valid_course_id code --- common/djangoapps/student/models.py | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 257d3cb0f5..0fbe70c0b3 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -348,17 +348,14 @@ def replicate_model(model_method, instance, user_id): ######### Replication Helpers ######### def is_valid_course_id(course_id): - """We check to both make sure that it's a valid course_id (and not - 'default', or some other non-course DB name) and that we have a mapping - for what database it belongs to.""" + """Right now, the only database that's not a course database is 'default'. + I had nicer checking in here originally -- it would scan the courses that + were in the system and only let you choose that. But it was annoying to run + tests with, since we don't have course data for some for our course test + databases. Hence the lazy version. + """ return course_id != 'default' - course_ids = set(course.id for course in modulestore().get_courses()) - is_valid = (course_id in course_ids) and (course_id in settings.DATABASES) - if not is_valid: - log.error("{0} is not a valid DB to replicate to.".format(course_id)) - return is_valid - def is_portal(): """Are we in the portal pool? Only Portal servers are allowed to replicate their changes. For now, only Portal servers see multiple DBs, so we use