diff --git a/askbot b/askbot index 1c3381046c..e56ae38084 160000 --- a/askbot +++ b/askbot @@ -1 +1 @@ -Subproject commit 1c3381046c78e055439ba1c78e0df48410fcc13e +Subproject commit e56ae380846f7c6cdaeacfc58880fab103540491 diff --git a/cms/djangoapps/contentstore/views.py b/cms/djangoapps/contentstore/views.py index 0305795e52..31be96ad7b 100644 --- a/cms/djangoapps/contentstore/views.py +++ b/cms/djangoapps/contentstore/views.py @@ -249,7 +249,7 @@ def load_preview_module(request, preview_id, descriptor, instance_state, shared_ module = descriptor.xmodule_constructor(system)(instance_state, shared_state) module.get_html = replace_static_urls( wrap_xmodule(module.get_html, module, "xmodule_display.html"), - module.metadata['data_dir'] + module.metadata['data_dir'], module ) save_preview_state(request, preview_id, descriptor.location.url(), module.get_instance_state(), module.get_shared_state()) diff --git a/cms/envs/common.py b/cms/envs/common.py index 3f8f4440c5..6faecafec1 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -83,7 +83,7 @@ TEMPLATE_CONTEXT_PROCESSORS = ( 'django.core.context_processors.request', 'django.core.context_processors.static', 'django.contrib.messages.context_processors.messages', - 'django.core.context_processors.auth', # this is required for admin + 'django.contrib.auth.context_processors.auth', # this is required for admin 'django.core.context_processors.csrf', # necessary for csrf protection ) @@ -121,6 +121,7 @@ MIDDLEWARE_CLASSES = ( ) ############################ SIGNAL HANDLERS ################################ +# This is imported to register the exception signal handling that logs exceptions import monitoring.exceptions # noqa ############################ DJANGO_BUILTINS ################################ 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", } } diff --git a/cms/static/coffee/spec/main_spec.coffee b/cms/static/coffee/spec/main_spec.coffee index c8f6976fed..72800cec7f 100644 --- a/cms/static/coffee/spec/main_spec.coffee +++ b/cms/static/coffee/spec/main_spec.coffee @@ -2,7 +2,7 @@ describe "CMS", -> beforeEach -> CMS.unbind() - it "should iniitalize Models", -> + it "should initialize Models", -> expect(CMS.Models).toBeDefined() it "should initialize Views", -> diff --git a/cms/static/coffee/spec/models/module_spec.coffee b/cms/static/coffee/spec/models/module_spec.coffee index 43ebdf420a..8fd552d93c 100644 --- a/cms/static/coffee/spec/models/module_spec.coffee +++ b/cms/static/coffee/spec/models/module_spec.coffee @@ -11,14 +11,25 @@ describe "CMS.Models.Module", -> @fakeModule = jasmine.createSpy("fakeModuleObject") window.FakeModule = jasmine.createSpy("FakeModule").andReturn(@fakeModule) @module = new CMS.Models.Module(type: "FakeModule") - @stubElement = $("
") - @module.loadModule(@stubElement) + @stubDiv = $('
') + @stubElement = $('
') + @stubElement.data('type', "FakeModule") + + @stubDiv.append(@stubElement) + @module.loadModule(@stubDiv) afterEach -> window.FakeModule = undefined it "initialize the module", -> - expect(window.FakeModule).toHaveBeenCalledWith(@stubElement) + expect(window.FakeModule).toHaveBeenCalled() + # Need to compare underlying nodes, because jquery selectors + # aren't equal even when they point to the same node. + # http://stackoverflow.com/questions/9505437/how-to-test-jquery-with-jasmine-for-element-id-if-used-as-this + expectedNode = @stubElement[0] + actualNode = window.FakeModule.mostRecentCall.args[0][0] + + expect(actualNode).toEqual(expectedNode) expect(@module.module).toEqual(@fakeModule) describe "when the module does not exists", -> @@ -32,7 +43,8 @@ describe "CMS.Models.Module", -> window.console = @previousConsole it "print out error to log", -> - expect(window.console.error).toHaveBeenCalledWith("Unable to load HTML.") + expect(window.console.error).toHaveBeenCalled() + expect(window.console.error.mostRecentCall.args[0]).toMatch("^Unable to load") describe "editUrl", -> diff --git a/cms/static/coffee/spec/views/module_edit_spec.coffee b/cms/static/coffee/spec/views/module_edit_spec.coffee index 693353ff70..067d169bca 100644 --- a/cms/static/coffee/spec/views/module_edit_spec.coffee +++ b/cms/static/coffee/spec/views/module_edit_spec.coffee @@ -8,11 +8,11 @@ describe "CMS.Views.ModuleEdit", -> cancel
  1. - submodule + submodule
- """ + """ #" CMS.unbind() describe "defaults", -> @@ -27,7 +27,7 @@ describe "CMS.Views.ModuleEdit", -> @stubModule.editUrl.andReturn("/edit_item?id=stub_module") new CMS.Views.ModuleEdit(el: $("#module-edit"), model: @stubModule) - it "load the edit from via ajax and pass to the model", -> + it "load the edit via ajax and pass to the model", -> expect($.fn.load).toHaveBeenCalledWith("/edit_item?id=stub_module", jasmine.any(Function)) if $.fn.load.mostRecentCall $.fn.load.mostRecentCall.args[1]() @@ -37,9 +37,9 @@ describe "CMS.Views.ModuleEdit", -> beforeEach -> @stubJqXHR = jasmine.createSpy("stubJqXHR") @stubJqXHR.success = jasmine.createSpy("stubJqXHR.success").andReturn(@stubJqXHR) - @stubJqXHR.error= jasmine.createSpy("stubJqXHR.success").andReturn(@stubJqXHR) + @stubJqXHR.error = jasmine.createSpy("stubJqXHR.error").andReturn(@stubJqXHR) @stubModule.save = jasmine.createSpy("stubModule.save").andReturn(@stubJqXHR) - new CMS.Views.ModuleEdit(el: $("#module-edit"), model: @stubModule) + new CMS.Views.ModuleEdit(el: $(".module-edit"), model: @stubModule) spyOn(window, "alert") $(".save-update").click() @@ -77,5 +77,5 @@ describe "CMS.Views.ModuleEdit", -> expect(CMS.pushView).toHaveBeenCalledWith @view expect(CMS.Views.ModuleEdit).toHaveBeenCalledWith model: @model expect(CMS.Models.Module).toHaveBeenCalledWith - id: "i4x://mitx.edu/course/module" + id: "i4x://mitx/course/html/module" type: "html" diff --git a/cms/static/coffee/spec/views/module_spec.coffee b/cms/static/coffee/spec/views/module_spec.coffee index a42c06856c..826263bc41 100644 --- a/cms/static/coffee/spec/views/module_spec.coffee +++ b/cms/static/coffee/spec/views/module_spec.coffee @@ -1,7 +1,7 @@ describe "CMS.Views.Module", -> beforeEach -> setFixtures """ -
+ """ @@ -20,5 +20,5 @@ describe "CMS.Views.Module", -> expect(CMS.replaceView).toHaveBeenCalledWith @view expect(CMS.Views.ModuleEdit).toHaveBeenCalledWith model: @model expect(CMS.Models.Module).toHaveBeenCalledWith - id: "i4x://mitx.edu/course/module" + id: "i4x://mitx/course/html/module" type: "html" diff --git a/cms/static/coffee/spec/views/week_spec.coffee b/cms/static/coffee/spec/views/week_spec.coffee index 74b8c22fde..d5256b0a57 100644 --- a/cms/static/coffee/spec/views/week_spec.coffee +++ b/cms/static/coffee/spec/views/week_spec.coffee @@ -1,7 +1,7 @@ describe "CMS.Views.Week", -> beforeEach -> setFixtures """ -
+
edit diff --git a/cms/static/coffee/src/models/module.coffee b/cms/static/coffee/src/models/module.coffee index 3abea41e35..159172e852 100644 --- a/cms/static/coffee/src/models/module.coffee +++ b/cms/static/coffee/src/models/module.coffee @@ -4,7 +4,8 @@ class CMS.Models.Module extends Backbone.Model data: '' loadModule: (element) -> - @module = XModule.loadModule($(element).find('.xmodule_edit')) + elt = $(element).find('.xmodule_edit').first() + @module = XModule.loadModule(elt) editUrl: -> "/edit_item?#{$.param(id: @get('id'))}" diff --git a/cms/static/coffee/src/views/module_edit.coffee b/cms/static/coffee/src/views/module_edit.coffee index d47bc3a90c..d212f7cb17 100644 --- a/cms/static/coffee/src/views/module_edit.coffee +++ b/cms/static/coffee/src/views/module_edit.coffee @@ -13,6 +13,16 @@ class CMS.Views.ModuleEdit extends Backbone.View # Load preview modules XModule.loadModules('display') + @enableDrag() + + enableDrag: -> + # Enable dragging things in the #sortable div (if there is one) + if $("#sortable").length > 0 + $("#sortable").sortable({ + placeholder: "ui-state-highlight" + }) + $("#sortable").disableSelection(); + save: (event) -> event.preventDefault() @@ -32,6 +42,7 @@ class CMS.Views.ModuleEdit extends Backbone.View cancel: (event) -> event.preventDefault() CMS.popView() + @enableDrag() editSubmodule: (event) -> event.preventDefault() @@ -42,3 +53,4 @@ class CMS.Views.ModuleEdit extends Backbone.View id: $(event.target).data('id') type: if moduleType == 'None' then null else moduleType previewType: if previewType == 'None' then null else previewType + @enableDrag() diff --git a/cms/templates/widgets/sequence-edit.html b/cms/templates/widgets/sequence-edit.html index c623eb4ec2..d92ccbb7a7 100644 --- a/cms/templates/widgets/sequence-edit.html +++ b/cms/templates/widgets/sequence-edit.html @@ -33,7 +33,7 @@
  1. -
      +
        % for child in module.get_children():
      1. -<%def name='url(file)'>${staticfiles_storage.url(file)} +<%def name='url(file)'> +<% +try: + url = staticfiles_storage.url(file) +except: + url = file +%>${url} <%def name='css(group)'> % if settings.MITX_FEATURES['USE_DJANGO_PIPELINE']: diff --git a/common/djangoapps/static_replace.py b/common/djangoapps/static_replace.py index f9660e7f5e..ba3cfb302a 100644 --- a/common/djangoapps/static_replace.py +++ b/common/djangoapps/static_replace.py @@ -1,6 +1,26 @@ -from staticfiles.storage import staticfiles_storage +import logging import re +from staticfiles.storage import staticfiles_storage +from staticfiles import finders +from django.conf import settings + +log = logging.getLogger(__name__) + +def try_staticfiles_lookup(path): + """ + Try to lookup a path in staticfiles_storage. If it fails, return + a dead link instead of raising an exception. + """ + try: + url = staticfiles_storage.url(path) + except Exception as err: + log.warning("staticfiles_storage couldn't find path {}: {}".format( + path, str(err))) + # Just return a dead link--don't kill everything. + url = "file_not_found" + return url + def replace(static_url, prefix=None): if prefix is None: @@ -9,10 +29,19 @@ def replace(static_url, prefix=None): prefix = prefix + '/' quote = static_url.group('quote') - if staticfiles_storage.exists(static_url.group('rest')): + + servable = ( + # If in debug mode, we'll serve up anything that the finders can find + (settings.DEBUG and finders.find(static_url.group('rest'), True)) or + # Otherwise, we'll only serve up stuff that the storages can find + staticfiles_storage.exists(static_url.group('rest')) + ) + + if servable: return static_url.group(0) else: - url = staticfiles_storage.url(prefix + static_url.group('rest')) + # don't error if file can't be found + url = try_staticfiles_lookup(prefix + static_url.group('rest')) return "".join([quote, url, quote]) diff --git a/common/djangoapps/student/management/commands/create_random_users.py b/common/djangoapps/student/management/commands/create_random_users.py new file mode 100644 index 0000000000..c6cf452a43 --- /dev/null +++ b/common/djangoapps/student/management/commands/create_random_users.py @@ -0,0 +1,36 @@ +## +## A script to create some dummy users + +from django.core.management.base import BaseCommand +from django.conf import settings +from django.contrib.auth.models import User +from student.models import UserProfile, CourseEnrollment + +from student.views import _do_create_account, get_random_post_override + +def create(n, course_id): + """Create n users, enrolling them in course_id if it's not None""" + for i in range(n): + (user, user_profile, _) = _do_create_account(get_random_post_override()) + if course_id is not None: + CourseEnrollment.objects.create(user=user, course_id=course_id) + +class Command(BaseCommand): + help = """Create N new users, with random parameters. + +Usage: create_random_users.py N [course_id_to_enroll_in]. + +Examples: + create_random_users.py 1 + create_random_users.py 10 MITx/6.002x/2012_Fall + create_random_users.py 100 HarvardX/CS50x/2012 +""" + + def handle(self, *args, **options): + if len(args) < 1 or len(args) > 2: + print Command.help + return + + n = int(args[0]) + course_id = args[1] if len(args) == 2 else None + create(n, course_id) diff --git a/common/djangoapps/student/models.py b/common/djangoapps/student/models.py index 49d3381303..0fbe70c0b3 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1,5 +1,30 @@ """ -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.) + +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 file and check it in at the same time as your model changes. To do that, @@ -10,16 +35,41 @@ 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): + """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" @@ -203,3 +253,154 @@ def add_user_to_default_group(user, group): utg.save() utg.users.add(User.objects.get(username=user)) 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, user_obj.id) + +@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. + """ + if not is_portal(): + return + + enrollment_obj = kwargs['instance'] + log.debug("Replicating user because of new enrollment") + replicate_user(enrollment_obj.user, enrollment_obj.course_id) + + 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, 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_profile_obj.user_id) + + +######### 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 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(id=portal_user.id) + for field in USER_FIELDS_TO_COPY: + 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, instance, user_id): + """ + model_method is the model action that we want replicated. For instance, + UserProfile.save + """ + 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): + """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' + +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 + that to decide.""" + return len(settings.DATABASES) > 1 + +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" + .format(instance)) + return False + if not is_portal(): + log.debug("{0} should not be replicated because we're not a portal." + .format(instance)) + return False + return True + + + + + + + diff --git a/common/djangoapps/student/tests.py b/common/djangoapps/student/tests.py index 501deb776c..ad7ddb70d1 100644 --- a/common/djangoapps/student/tests.py +++ b/common/djangoapps/student/tests.py @@ -4,13 +4,178 @@ 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 + + if hasattr(portal_user, 'seen_response_count'): + 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 + )) + + 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). + # 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. + 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 modifying User data after you've enrolled.""" + # Create our User + 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() + + 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) + 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/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 87490786c1..1951426ea7 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -94,8 +94,9 @@ def main_index(extra_context = {}, user=None): context.update(extra_context) return render_to_response('index.html', context) -def course_from_id(id): - course_loc = CourseDescriptor.id_to_location(id) +def course_from_id(course_id): + """Return the CourseDescriptor corresponding to this course_id""" + course_loc = CourseDescriptor.id_to_location(course_id) return modulestore().get_item(course_loc) @@ -127,7 +128,7 @@ def dashboard(request): try: courses.append(course_from_id(enrollment.course_id)) except ItemNotFoundError: - log.error("User {0} enrolled in non-existant course {1}" + log.error("User {0} enrolled in non-existent course {1}" .format(user.username, enrollment.course_id)) message = "" @@ -158,15 +159,19 @@ def try_change_enrollment(request): @login_required def change_enrollment_view(request): + """Delegate to change_enrollment to actually do the work.""" return HttpResponse(json.dumps(change_enrollment(request))) - def change_enrollment(request): if request.method != "POST": raise Http404 - action = request.POST.get("enrollment_action", "") user = request.user + if not user.is_authenticated(): + raise Http404 + + action = request.POST.get("enrollment_action", "") + course_id = request.POST.get("course_id", None) if course_id == None: return HttpResponse(json.dumps({'success': False, 'error': 'There was an error receiving the course id.'})) @@ -177,14 +182,14 @@ def change_enrollment(request): try: course = course_from_id(course_id) except ItemNotFoundError: - log.error("User {0} tried to enroll in non-existant course {1}" + log.warning("User {0} tried to enroll in non-existant course {1}" .format(user.username, enrollment.course_id)) return {'success': False, 'error': 'The course requested does not exist.'} if settings.MITX_FEATURES.get('ACCESS_REQUIRE_STAFF_FOR_COURSE'): # require that user be in the staff_* group (or be an overall admin) to be able to enroll # eg staff_6.002x or staff_6.00x - if not has_staff_access_to_course(user,course): + if not has_staff_access_to_course(user, course): staff_group = course_staff_group_name(course) log.debug('user %s denied enrollment to %s ; not in %s' % (user,course.location.url(),staff_group)) return {'success': False, 'error' : '%s membership required to access course.' % staff_group} @@ -264,6 +269,7 @@ def logout_user(request): def change_setting(request): ''' JSON call to change a profile setting: Right now, location ''' + # TODO (vshnayder): location is no longer used up = UserProfile.objects.get(user=request.user) # request.user.profile_cache if 'location' in request.POST: up.location = request.POST['location'] @@ -272,6 +278,59 @@ def change_setting(request): return HttpResponse(json.dumps({'success': True, 'location': up.location, })) +def _do_create_account(post_vars): + """ + Given cleaned post variables, create the User and UserProfile objects, as well as the + registration for this user. + + Returns a tuple (User, UserProfile, Registration). + + Note: this function is also used for creating test users. + """ + user = User(username=post_vars['username'], + email=post_vars['email'], + is_active=False) + user.set_password(post_vars['password']) + registration = Registration() + # TODO: Rearrange so that if part of the process fails, the whole process fails. + # Right now, we can have e.g. no registration e-mail sent out and a zombie account + try: + user.save() + except IntegrityError: + js = {'success': False} + # Figure out the cause of the integrity error + if len(User.objects.filter(username=post_vars['username'])) > 0: + js['value'] = "An account with this username already exists." + js['field'] = 'username' + return HttpResponse(json.dumps(js)) + + if len(User.objects.filter(email=post_vars['email'])) > 0: + js['value'] = "An account with this e-mail already exists." + js['field'] = 'email' + return HttpResponse(json.dumps(js)) + + raise + + registration.register(user) + + profile = UserProfile(user=user) + profile.name = post_vars['name'] + profile.level_of_education = post_vars.get('level_of_education') + profile.gender = post_vars.get('gender') + profile.mailing_address = post_vars.get('mailing_address') + profile.goals = post_vars.get('goals') + + try: + profile.year_of_birth = int(post_vars['year_of_birth']) + except (ValueError, KeyError): + profile.year_of_birth = None # If they give us garbage, just ignore it instead + # of asking them to put an integer. + try: + profile.save() + except Exception: + log.exception("UserProfile creation failed for user {0}.".format(user.id)) + return (user, profile, registration) + @ensure_csrf_cookie def create_account(request, post_override=None): @@ -343,50 +402,14 @@ def create_account(request, post_override=None): js['field'] = 'username' return HttpResponse(json.dumps(js)) - u = User(username=post_vars['username'], - email=post_vars['email'], - is_active=False) - u.set_password(post_vars['password']) - r = Registration() - # TODO: Rearrange so that if part of the process fails, the whole process fails. - # Right now, we can have e.g. no registration e-mail sent out and a zombie account - try: - u.save() - except IntegrityError: - # Figure out the cause of the integrity error - if len(User.objects.filter(username=post_vars['username'])) > 0: - js['value'] = "An account with this username already exists." - js['field'] = 'username' - return HttpResponse(json.dumps(js)) - - if len(User.objects.filter(email=post_vars['email'])) > 0: - js['value'] = "An account with this e-mail already exists." - js['field'] = 'email' - return HttpResponse(json.dumps(js)) - - raise - - r.register(u) - - up = UserProfile(user=u) - up.name = post_vars['name'] - up.level_of_education = post_vars.get('level_of_education') - up.gender = post_vars.get('gender') - up.mailing_address = post_vars.get('mailing_address') - up.goals = post_vars.get('goals') - - try: - up.year_of_birth = int(post_vars['year_of_birth']) - except (ValueError, KeyError): - up.year_of_birth = None # If they give us garbage, just ignore it instead - # of asking them to put an integer. - try: - up.save() - except Exception: - log.exception("UserProfile creation failed for user {0}.".format(u.id)) + # Ok, looks like everything is legit. Create the account. + ret = _do_create_account(post_vars) + if isinstance(ret,HttpResponse): # if there was an error then return that + return ret + (user, profile, registration) = ret d = {'name': post_vars['name'], - 'key': r.activation_key, + 'key': registration.activation_key, } # composes activation email @@ -398,10 +421,11 @@ def create_account(request, post_override=None): try: if settings.MITX_FEATURES.get('REROUTE_ACTIVATION_EMAIL'): dest_addr = settings.MITX_FEATURES['REROUTE_ACTIVATION_EMAIL'] - message = "Activation for %s (%s): %s\n" % (u, u.email, up.name) + '-' * 80 + '\n\n' + message + message = ("Activation for %s (%s): %s\n" % (user, user.email, profile.name) + + '-' * 80 + '\n\n' + message) send_mail(subject, message, settings.DEFAULT_FROM_EMAIL, [dest_addr], fail_silently=False) elif not settings.GENERATE_RANDOM_USER_CREDENTIALS: - res = u.email_user(subject, message, settings.DEFAULT_FROM_EMAIL) + res = user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL) except: log.exception(sys.exc_info()) js['value'] = 'Could not send activation e-mail.' @@ -431,24 +455,30 @@ def create_account(request, post_override=None): return HttpResponse(json.dumps(js), mimetype="application/json") -def create_random_account(create_account_function): - +def get_random_post_override(): + """ + Return a dictionary suitable for passing to post_vars of _do_create_account or post_override + of create_account, with random user info. + """ def id_generator(size=6, chars=string.ascii_uppercase + string.ascii_lowercase + string.digits): return ''.join(random.choice(chars) for x in range(size)) - def inner_create_random_account(request): - post_override = {'username': "random_" + id_generator(), - 'email': id_generator(size=10, chars=string.ascii_lowercase) + "_dummy_test@mitx.mit.edu", - 'password': id_generator(), - 'location': id_generator(size=5, chars=string.ascii_uppercase), - 'name': id_generator(size=5, chars=string.ascii_lowercase) + " " + id_generator(size=7, chars=string.ascii_lowercase), - 'honor_code': u'true', - 'terms_of_service': u'true', } + return {'username': "random_" + id_generator(), + 'email': id_generator(size=10, chars=string.ascii_lowercase) + "_dummy_test@mitx.mit.edu", + 'password': id_generator(), + 'name': (id_generator(size=5, chars=string.ascii_lowercase) + " " + + id_generator(size=7, chars=string.ascii_lowercase)), + 'honor_code': u'true', + 'terms_of_service': u'true', } - return create_account_function(request, post_override=post_override) + +def create_random_account(create_account_function): + def inner_create_random_account(request): + return create_account_function(request, post_override=get_random_post_override()) return inner_create_random_account +# TODO (vshnayder): do we need GENERATE_RANDOM_USER_CREDENTIALS for anything? if settings.GENERATE_RANDOM_USER_CREDENTIALS: create_account = create_random_account(create_account) @@ -514,7 +544,7 @@ def reactivation_email(request): subject = ''.join(subject.splitlines()) message = render_to_string('reactivation_email.txt', d) - res = u.email_user(subject, message, settings.DEFAULT_FROM_EMAIL) + res = user.email_user(subject, message, settings.DEFAULT_FROM_EMAIL) return HttpResponse(json.dumps({'success': True})) diff --git a/common/djangoapps/util/json_request.py b/common/djangoapps/util/json_request.py index c2fad16d70..391905e574 100644 --- a/common/djangoapps/util/json_request.py +++ b/common/djangoapps/util/json_request.py @@ -9,7 +9,7 @@ def expect_json(view_function): if request.META['CONTENT_TYPE'] == "application/json": cloned_request = copy.copy(request) cloned_request.POST = cloned_request.POST.copy() - cloned_request.POST.update(json.loads(request.raw_post_data)) + cloned_request.POST.update(json.loads(request.body)) return view_function(cloned_request, *args, **kwargs) else: return view_function(request, *args, **kwargs) diff --git a/common/djangoapps/util/middleware.py b/common/djangoapps/util/middleware.py deleted file mode 100644 index ce7b961766..0000000000 --- a/common/djangoapps/util/middleware.py +++ /dev/null @@ -1,16 +0,0 @@ -import logging - -from django.conf import settings -from django.http import HttpResponseServerError - -log = logging.getLogger("mitx") - - -class ExceptionLoggingMiddleware(object): - """Just here to log unchecked exceptions that go all the way up the Django - stack""" - - if not settings.TEMPLATE_DEBUG: - def process_exception(self, request, exception): - log.exception(exception) - return HttpResponseServerError("Server Error - Please try again later.") diff --git a/common/djangoapps/xmodule_modifiers.py b/common/djangoapps/xmodule_modifiers.py index 597d74ce6f..0aeaa59d69 100644 --- a/common/djangoapps/xmodule_modifiers.py +++ b/common/djangoapps/xmodule_modifiers.py @@ -34,7 +34,7 @@ def wrap_xmodule(get_html, module, template): return _get_html -def replace_static_urls(get_html, prefix): +def replace_static_urls(get_html, prefix, module): """ Updates the supplied module with a new get_html function that wraps the old get_html function and substitutes urls of the form /static/... @@ -69,14 +69,14 @@ def grade_histogram(module_id): return grades -def add_histogram(get_html, module): +def add_histogram(get_html, module, user): """ Updates the supplied module with a new get_html function that wraps the output of the old get_html function with additional information for admin users only, including a histogram of student answers and the definition of the xmodule - Does nothing if module is a SequenceModule + Does nothing if module is a SequenceModule or a VerticalModule. """ @wraps(get_html) def _get_html(): @@ -90,19 +90,27 @@ def add_histogram(get_html, module): # TODO (ichuang): Remove after fall 2012 LMS migration done if settings.MITX_FEATURES.get('ENABLE_LMS_MIGRATION'): - [filepath, filename] = module.definition.get('filename','') + [filepath, filename] = module.definition.get('filename', ['', None]) osfs = module.system.filestore if filename is not None and osfs.exists(filename): - filepath = filename # if original, unmangled filename exists then use it (github doesn't like symlinks) + # if original, unmangled filename exists then use it (github + # doesn't like symlinks) + filepath = filename data_dir = osfs.root_path.rsplit('/')[-1] - edit_link = "https://github.com/MITx/%s/tree/master/%s" % (data_dir,filepath) + giturl = module.metadata.get('giturl','https://github.com/MITx') + edit_link = "%s/%s/tree/master/%s" % (giturl,data_dir,filepath) else: edit_link = False staff_context = {'definition': module.definition.get('data'), 'metadata': json.dumps(module.metadata, indent=4), - 'element_id': module.location.html_id(), + 'location': module.location, + 'xqa_key': module.metadata.get('xqa_key',''), + 'category': str(module.__class__.__name__), + 'element_id': module.location.html_id().replace('-','_'), 'edit_link': edit_link, + 'user': user, + 'xqa_server' : settings.MITX_FEATURES.get('USE_XQA_SERVER','http://xqa:server@content-qa.mitx.mit.edu/xqa'), 'histogram': json.dumps(histogram), 'render_histogram': render_histogram, 'module_content': get_html()} diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index 2dc059e7d7..8945db4ff6 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -203,8 +203,9 @@ class LoncapaProblem(object): cmap.update(self.correct_map) for responder in self.responders.values(): if hasattr(responder, 'update_score'): - # Each LoncapaResponse will update the specific entries of 'cmap' that it's responsible for - cmap = responder.update_score(score_msg, cmap, queuekey) + # Each LoncapaResponse will update its specific entries in cmap + # cmap is passed by reference + responder.update_score(score_msg, cmap, queuekey) self.correct_map.set_dict(cmap.get_dict()) return cmap @@ -228,14 +229,14 @@ class LoncapaProblem(object): Calls the Response for each question in this problem, to do the actual grading. ''' - + self.student_answers = convert_files_to_filenames(answers) - + oldcmap = self.correct_map # old CorrectMap newcmap = CorrectMap() # start new with empty CorrectMap # log.debug('Responders: %s' % self.responders) for responder in self.responders.values(): # Call each responsetype instance to do actual grading - if 'filesubmission' in responder.allowed_inputfields: # File objects are passed only if responsetype + if 'filesubmission' in responder.allowed_inputfields: # File objects are passed only if responsetype # explicitly allows for file submissions results = responder.evaluate_answers(answers, oldcmap) else: @@ -294,9 +295,9 @@ class LoncapaProblem(object): try: ifp = self.system.filestore.open(file) # open using ModuleSystem OSFS filestore except Exception as err: - log.error('Error %s in problem xml include: %s' % ( + log.warning('Error %s in problem xml include: %s' % ( err, etree.tostring(inc, pretty_print=True))) - log.error('Cannot find file %s in %s' % ( + log.warning('Cannot find file %s in %s' % ( file, self.system.filestore)) # if debugging, don't fail - just log error # TODO (vshnayder): need real error handling, display to users @@ -305,11 +306,11 @@ class LoncapaProblem(object): else: continue try: - incxml = etree.XML(ifp.read()) # read in and convert to XML + incxml = etree.XML(ifp.read()) # read in and convert to XML except Exception as err: - log.error('Error %s in problem xml include: %s' % ( + log.warning('Error %s in problem xml include: %s' % ( err, etree.tostring(inc, pretty_print=True))) - log.error('Cannot parse XML in %s' % (file)) + log.warning('Cannot parse XML in %s' % (file)) # if debugging, don't fail - just log error # TODO (vshnayder): same as above if not self.system.get('DEBUG'): @@ -392,9 +393,10 @@ class LoncapaProblem(object): context['script_code'] += code # store code source in context try: exec code in context, context # use "context" for global context; thus defs in code are global within code - except Exception: + except Exception as err: log.exception("Error while execing script code: " + code) - raise responsetypes.LoncapaProblemError("Error while executing script code") + msg = "Error while executing script code: %s" % str(err).replace('<','<') + raise responsetypes.LoncapaProblemError(msg) finally: sys.path = original_path diff --git a/common/lib/capa/capa/inputtypes.py b/common/lib/capa/capa/inputtypes.py index 2d1da53625..8c513e7aec 100644 --- a/common/lib/capa/capa/inputtypes.py +++ b/common/lib/capa/capa/inputtypes.py @@ -205,7 +205,7 @@ def extract_choices(element): raise Exception("[courseware.capa.inputtypes.extract_choices] \ Expected a tag; got %s instead" % choice.tag) - choice_text = ''.join([etree.tostring(x) for x in choice]) + choice_text = ''.join([x.text for x in choice]) choices.append((choice.get("name"), choice_text)) @@ -336,9 +336,19 @@ def filesubmission(element, value, status, render_template, msg=''): Upload a single file (e.g. for programming assignments) ''' eid = element.get('id') - context = { 'id': eid, 'state': status, 'msg': msg, 'value': value, } + + # Check if problem has been queued + queue_len = 0 + if status == 'incomplete': # Flag indicating that the problem has been queued, 'msg' is length of queue + status = 'queued' + queue_len = msg + msg = 'Submitted to grader. (Queue length: %s)' % queue_len + + context = { 'id': eid, 'state': status, 'msg': msg, 'value': value, + 'queue_len': queue_len + } html = render_template("filesubmission.html", context) - return etree.XML(html) + return etree.XML(html) #----------------------------------------------------------------------------- @@ -359,9 +369,16 @@ def textbox(element, value, status, render_template, msg=''): if not value: value = element.text # if no student input yet, then use the default input given by the problem + # Check if problem has been queued + queue_len = 0 + if status == 'incomplete': # Flag indicating that the problem has been queued, 'msg' is length of queue + status = 'queued' + queue_len = msg + msg = 'Submitted to grader. (Queue length: %s)' % queue_len + # For CodeMirror - mode = element.get('mode') or 'python' # mode, eg "python" or "xml" - linenumbers = element.get('linenumbers','true') # for CodeMirror + mode = element.get('mode','python') + linenumbers = element.get('linenumbers','true') tabsize = element.get('tabsize','4') tabsize = int(tabsize) @@ -369,6 +386,7 @@ def textbox(element, value, status, render_template, msg=''): 'mode': mode, 'linenumbers': linenumbers, 'rows': rows, 'cols': cols, 'hidden': hidden, 'tabsize': tabsize, + 'queue_len': queue_len, } html = render_template("textbox.html", context) try: diff --git a/common/lib/capa/capa/responsetypes.py b/common/lib/capa/capa/responsetypes.py index 484be315ba..91d8a8cadc 100644 --- a/common/lib/capa/capa/responsetypes.py +++ b/common/lib/capa/capa/responsetypes.py @@ -990,6 +990,12 @@ class CodeResponse(LoncapaResponse): ''' Grade student code using an external queueing server, called 'xqueue' + Expects 'xqueue' dict in ModuleSystem with the following keys: + system.xqueue = { 'interface': XqueueInterface object, + 'callback_url': Per-StudentModule callback URL where results are posted (string), + 'default_queuename': Default queuename to submit request (string) + } + External requests are only submitted for student submission grading (i.e. and not for getting reference answers) ''' @@ -999,10 +1005,27 @@ class CodeResponse(LoncapaResponse): max_inputfields = 1 def setup_response(self): + ''' + Configure CodeResponse from XML. Supports both CodeResponse and ExternalResponse XML + + TODO: Determines whether in synchronous or asynchronous (queued) mode + ''' xml = self.xml + self.url = xml.get('url', None) # XML can override external resource (grader/queue) URL self.queue_name = xml.get('queuename', self.system.xqueue['default_queuename']) - answer = xml.find('answer') + self._parse_externalresponse_xml() + + def _parse_externalresponse_xml(self): + ''' + VS[compat]: Suppport for old ExternalResponse XML format. When successful, sets: + self.code + self.tests + self.answer + self.initial_display + ''' + answer = self.xml.find('answer') + if answer is not None: answer_src = answer.get('src') if answer_src is not None: @@ -1016,7 +1039,7 @@ class CodeResponse(LoncapaResponse): msg += "\nSee XML source line %s" % getattr(self.xml, 'sourceline', '') raise LoncapaProblemError(msg) - self.tests = xml.get('tests') + self.tests = self.xml.get('tests') # Extract 'answer' and 'initial_display' from XML. Note that the code to be exec'ed here is: # (1) Internal edX code, i.e. NOT student submissions, and @@ -1063,15 +1086,16 @@ class CodeResponse(LoncapaResponse): 'edX_cmd': 'get_score', 'edX_tests': self.tests, 'processor': self.code, - 'edX_student_response': unicode(submission), # unicode on File object returns its filename } - # Submit request - if hasattr(submission, 'read'): # Test for whether submission is a file + # Submit request. When successful, 'msg' is the prior length of the queue + if is_file(submission): + contents.update({'edX_student_response': submission.name}) (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents), file_to_upload=submission) else: + contents.update({'edX_student_response': submission}) (error, msg) = qinterface.send_to_queue(header=xheader, body=json.dumps(contents)) @@ -1080,33 +1104,31 @@ class CodeResponse(LoncapaResponse): cmap.set(self.answer_id, queuekey=None, msg='Unable to deliver your submission to grader. (Reason: %s.) Please try again later.' % msg) else: - # Non-null CorrectMap['queuekey'] indicates that the problem has been queued - cmap.set(self.answer_id, queuekey=queuekey, msg='Submitted to grader. (Queue length: %s)' % msg) + # Queueing mechanism flags: + # 1) Backend: Non-null CorrectMap['queuekey'] indicates that the problem has been queued + # 2) Frontend: correctness='incomplete' eventually trickles down through inputtypes.textbox + # and .filesubmission to inform the browser to poll the LMS + cmap.set(self.answer_id, queuekey=queuekey, correctness='incomplete', msg=msg) return cmap def update_score(self, score_msg, oldcmap, queuekey): - # Parse 'score_msg' as XML - try: - rxml = etree.fromstring(score_msg) - except Exception as err: - msg = 'Error in CodeResponse %s: cannot parse response from xworker r.text=%s' % (err, score_msg) - raise Exception(err) - # The following process is lifted directly from ExternalResponse - ad = rxml.find('awarddetail').text - admap = {'EXACT_ANS': 'correct', # TODO: handle other loncapa responses - 'WRONG_FORMAT': 'incorrect', - } - self.context['correct'] = ['correct'] - if ad in admap: - self.context['correct'][0] = admap[ad] + (valid_score_msg, correct, score, msg) = self._parse_score_msg(score_msg) + if not valid_score_msg: + oldcmap.set(self.answer_id, msg='Error: Invalid grader reply.') + return oldcmap + + correctness = 'incorrect' + if correct: + correctness = 'correct' + + self.context['correct'] = correctness # TODO: Find out how this is used elsewhere, if any # Replace 'oldcmap' with new grading results if queuekey matches. # If queuekey does not match, we keep waiting for the score_msg whose key actually matches if oldcmap.is_right_queuekey(self.answer_id, queuekey): - msg = rxml.find('message').text.replace(' ', ' ') - oldcmap.set(self.answer_id, correctness=self.context['correct'][0], msg=msg, queuekey=None) # Queuekey is consumed + oldcmap.set(self.answer_id, correctness=correctness, msg=msg.replace(' ', ' '), queuekey=None) # Queuekey is consumed else: log.debug('CodeResponse: queuekey %s does not match for answer_id=%s.' % (queuekey, self.answer_id)) @@ -1119,6 +1141,31 @@ class CodeResponse(LoncapaResponse): def get_initial_display(self): return {self.answer_id: self.initial_display} + def _parse_score_msg(self, score_msg): + ''' + Grader reply is a JSON-dump of the following dict + { 'correct': True/False, + 'score': # TODO -- Partial grading + 'msg': grader_msg } + + Returns (valid_score_msg, correct, score, msg): + valid_score_msg: Flag indicating valid score_msg format (Boolean) + correct: Correctness of submission (Boolean) + score: # TODO: Implement partial grading + msg: Message from grader to display to student (string) + ''' + fail = (False, False, -1, '') + try: + score_result = json.loads(score_msg) + except (TypeError, ValueError): + return fail + if not isinstance(score_result, dict): + return fail + for tag in ['correct', 'score', 'msg']: + if not score_result.has_key(tag): + return fail + return (True, score_result['correct'], score_result['score'], score_result['msg']) + #----------------------------------------------------------------------------- diff --git a/common/lib/capa/capa/templates/filesubmission.html b/common/lib/capa/capa/templates/filesubmission.html index ff9fc992fd..d3d57ee318 100644 --- a/common/lib/capa/capa/templates/filesubmission.html +++ b/common/lib/capa/capa/templates/filesubmission.html @@ -6,8 +6,9 @@ % elif state == 'incorrect': - % elif state == 'incomplete': - + % elif state == 'queued': + + % endif (${state})
        diff --git a/common/lib/capa/capa/templates/textbox.html b/common/lib/capa/capa/templates/textbox.html index f201bd6947..647f4fe4e8 100644 --- a/common/lib/capa/capa/templates/textbox.html +++ b/common/lib/capa/capa/templates/textbox.html @@ -13,11 +13,12 @@ % elif state == 'incorrect': - % elif state == 'incomplete': - + % elif state == 'queued': + + % endif % if hidden: -
        +
        % endif
        (${state}) diff --git a/common/lib/capa/capa/util.py b/common/lib/capa/capa/util.py index 1dc113cd20..005494e8c0 100644 --- a/common/lib/capa/capa/util.py +++ b/common/lib/capa/capa/util.py @@ -39,5 +39,18 @@ def convert_files_to_filenames(answers): ''' new_answers = dict() for answer_id in answers.keys(): - new_answers[answer_id] = unicode(answers[answer_id]) + if is_file(answers[answer_id]): + new_answers[answer_id] = answers[answer_id].name + else: + new_answers[answer_id] = answers[answer_id] return new_answers + +def is_file(file_to_test): + ''' + Duck typing to check if 'file_to_test' is a File object + ''' + is_file = True + for method in ['read', 'name']: + if not hasattr(file_to_test, method): + is_file = False + return is_file diff --git a/common/lib/capa/capa/xqueue_interface.py b/common/lib/capa/capa/xqueue_interface.py index 82e4fba5cc..70f086120e 100644 --- a/common/lib/capa/capa/xqueue_interface.py +++ b/common/lib/capa/capa/xqueue_interface.py @@ -67,7 +67,6 @@ class XqueueInterface: self.url = url self.auth = auth self.session = requests.session() - self._login() def send_to_queue(self, header, body, file_to_upload=None): ''' diff --git a/common/lib/xmodule/xmodule/abtest_module.py b/common/lib/xmodule/xmodule/abtest_module.py index 8d6604b06b..035413a402 100644 --- a/common/lib/xmodule/xmodule/abtest_module.py +++ b/common/lib/xmodule/xmodule/abtest_module.py @@ -11,18 +11,19 @@ DEFAULT = "_DEFAULT_GROUP" def group_from_value(groups, v): - ''' Given group: (('a',0.3),('b',0.4),('c',0.3)) And random value + """ + Given group: (('a', 0.3), ('b', 0.4), ('c', 0.3)) and random value v in [0,1], return the associated group (in the above case, return - 'a' if v<0.3, 'b' if 0.3<=v<0.7, and 'c' if v>0.7 -''' + 'a' if v < 0.3, 'b' if 0.3 <= v < 0.7, and 'c' if v > 0.7 + """ sum = 0 for (g, p) in groups: sum = sum + p if sum > v: return g - # Round off errors might cause us to run to the end of the list - # If the do, return the last element + # Round off errors might cause us to run to the end of the list. + # If the do, return the last element. return g @@ -31,8 +32,8 @@ class ABTestModule(XModule): Implements an A/B test with an aribtrary number of competing groups """ - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) if shared_state is None: @@ -103,7 +104,9 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor): experiment = xml_object.get('experiment') if experiment is None: - raise InvalidDefinitionError("ABTests must specify an experiment. Not found in:\n{xml}".format(xml=etree.tostring(xml_object, pretty_print=True))) + raise InvalidDefinitionError( + "ABTests must specify an experiment. Not found in:\n{xml}" + .format(xml=etree.tostring(xml_object, pretty_print=True))) definition = { 'data': { @@ -127,7 +130,9 @@ class ABTestDescriptor(RawDescriptor, XmlDescriptor): definition['data']['group_content'][name] = child_content_urls definition['children'].extend(child_content_urls) - default_portion = 1 - sum(portion for (name, portion) in definition['data']['group_portions'].items()) + default_portion = 1 - sum( + portion for (name, portion) in definition['data']['group_portions'].items()) + if default_portion < 0: raise InvalidDefinitionError("ABTest portions must add up to less than or equal to 1") diff --git a/common/lib/xmodule/xmodule/capa_module.py b/common/lib/xmodule/xmodule/capa_module.py index 7ce76def32..f12b1c2be4 100644 --- a/common/lib/xmodule/xmodule/capa_module.py +++ b/common/lib/xmodule/xmodule/capa_module.py @@ -11,13 +11,13 @@ from datetime import timedelta from lxml import etree from pkg_resources import resource_string -from xmodule.x_module import XModule -from xmodule.raw_module import RawDescriptor -from xmodule.exceptions import NotFoundError -from progress import Progress from capa.capa_problem import LoncapaProblem from capa.responsetypes import StudentInputError from capa.util import convert_files_to_filenames +from progress import Progress +from xmodule.x_module import XModule +from xmodule.raw_module import RawDescriptor +from xmodule.exceptions import NotFoundError log = logging.getLogger("mitx.courseware") @@ -80,9 +80,9 @@ class CapaModule(XModule): js_module_name = "Problem" css = {'scss': [resource_string(__name__, 'css/capa/display.scss')]} - def __init__(self, system, location, definition, instance_state=None, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) self.attempts = 0 @@ -119,9 +119,9 @@ class CapaModule(XModule): if self.show_answer == "": self.show_answer = "closed" - if instance_state != None: + if instance_state is not None: instance_state = json.loads(instance_state) - if instance_state != None and 'attempts' in instance_state: + if instance_state is not None and 'attempts' in instance_state: self.attempts = instance_state['attempts'] self.name = only_one(dom2.xpath('/problem/@name')) @@ -130,16 +130,18 @@ class CapaModule(XModule): if weight_string: self.weight = float(weight_string) else: - self.weight = 1 + self.weight = None if self.rerandomize == 'never': seed = 1 - elif self.rerandomize == "per_student" and hasattr(system, 'id'): + elif self.rerandomize == "per_student" and hasattr(self.system, 'id'): seed = system.id else: seed = None try: + # TODO (vshnayder): move as much as possible of this work and error + # checking to descriptor load time self.lcp = LoncapaProblem(self.definition['data'], self.location.html_id(), instance_state, seed=seed, system=self.system) except Exception as err: @@ -148,7 +150,7 @@ class CapaModule(XModule): # TODO (vshnayder): do modules need error handlers too? # We shouldn't be switching on DEBUG. if self.system.DEBUG: - log.error(msg) + log.warning(msg) # TODO (vshnayder): This logic should be general, not here--and may # want to preserve the data instead of replacing it. # e.g. in the CMS @@ -238,7 +240,7 @@ class CapaModule(XModule): content = {'name': self.metadata['display_name'], 'html': html, 'weight': self.weight, - } + } # We using strings as truthy values, because the terminology of the # check button is context-specific. @@ -426,7 +428,7 @@ class CapaModule(XModule): event_info = dict() event_info['state'] = self.lcp.get_state() event_info['problem_id'] = self.location.url() - + answers = self.make_dict_of_responses(get) event_info['answers'] = convert_files_to_filenames(answers) @@ -563,6 +565,14 @@ class CapaDescriptor(RawDescriptor): module_class = CapaModule + stores_state = True + has_score = True + + # Capa modules have some additional metadata: + # TODO (vshnayder): do problems have any other metadata? Do they + # actually use type and points? + metadata_attributes = RawDescriptor.metadata_attributes + ('type', 'points') + # VS[compat] # TODO (cpennington): Delete this method once all fall 2012 course are being # edited in the cms @@ -572,8 +582,3 @@ class CapaDescriptor(RawDescriptor): 'problems/' + path[8:], path[8:], ] - - @classmethod - def split_to_file(cls, xml_object): - '''Problems always written in their own files''' - return True diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index acdc574220..7ec9f54cd2 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -3,6 +3,7 @@ import time import dateutil.parser import logging +from xmodule.util.decorators import lazyproperty from xmodule.graders import load_grading_policy from xmodule.modulestore import Location from xmodule.seq_module import SequenceDescriptor, SequenceModule @@ -12,13 +13,9 @@ log = logging.getLogger(__name__) class CourseDescriptor(SequenceDescriptor): module_class = SequenceModule - metadata_attributes = SequenceDescriptor.metadata_attributes + ('org', 'course') def __init__(self, system, definition=None, **kwargs): super(CourseDescriptor, self).__init__(system, definition, **kwargs) - - self._grader = None - self._grade_cutoffs = None msg = None try: @@ -39,34 +36,84 @@ class CourseDescriptor(SequenceDescriptor): def has_started(self): return time.gmtime() > self.start - + @property def grader(self): - self.__load_grading_policy() - return self._grader - + return self.__grading_policy['GRADER'] + @property def grade_cutoffs(self): - self.__load_grading_policy() - return self._grade_cutoffs - - - def __load_grading_policy(self): - if not self._grader or not self._grade_cutoffs: - policy_string = "" - - try: - with self.system.resources_fs.open("grading_policy.json") as grading_policy_file: - policy_string = grading_policy_file.read() - except (IOError, ResourceNotFoundError): - log.warning("Unable to load course settings file from grading_policy.json in course " + self.id) - - grading_policy = load_grading_policy(policy_string) - - self._grader = grading_policy['GRADER'] - self._grade_cutoffs = grading_policy['GRADE_CUTOFFS'] - - + return self.__grading_policy['GRADE_CUTOFFS'] + + @lazyproperty + def __grading_policy(self): + policy_string = "" + + try: + with self.system.resources_fs.open("grading_policy.json") as grading_policy_file: + policy_string = grading_policy_file.read() + except (IOError, ResourceNotFoundError): + log.warning("Unable to load course settings file from grading_policy.json in course " + self.id) + + grading_policy = load_grading_policy(policy_string) + + return grading_policy + + + @lazyproperty + def grading_context(self): + """ + This returns a dictionary with keys necessary for quickly grading + a student. They are used by grades.grade() + + The grading context has two keys: + graded_sections - This contains the sections that are graded, as + well as all possible children modules that can affect the + grading. This allows some sections to be skipped if the student + hasn't seen any part of it. + + The format is a dictionary keyed by section-type. The values are + arrays of dictionaries containing + "section_descriptor" : The section descriptor + "xmoduledescriptors" : An array of xmoduledescriptors that + could possibly be in the section, for any student + + all_descriptors - This contains a list of all xmodules that can + effect grading a student. This is used to efficiently fetch + all the xmodule state for a StudentModuleCache without walking + the descriptor tree again. + + + """ + + all_descriptors = [] + graded_sections = {} + + def yield_descriptor_descendents(module_descriptor): + for child in module_descriptor.get_children(): + yield child + for module_descriptor in yield_descriptor_descendents(child): + yield module_descriptor + + for c in self.get_children(): + sections = [] + for s in c.get_children(): + if s.metadata.get('graded', False): + xmoduledescriptors = list(yield_descriptor_descendents(s)) + + # The xmoduledescriptors included here are only the ones that have scores. + section_description = { 'section_descriptor' : s, 'xmoduledescriptors' : filter(lambda child: child.has_score, xmoduledescriptors) } + + section_format = s.metadata.get('format', "") + graded_sections[ section_format ] = graded_sections.get( section_format, [] ) + [section_description] + + all_descriptors.extend(xmoduledescriptors) + all_descriptors.append(s) + + return { 'graded_sections' : graded_sections, + 'all_descriptors' : all_descriptors,} + + @staticmethod def id_to_location(course_id): '''Convert the given course_id (org/course/name) to a location object. diff --git a/common/lib/xmodule/xmodule/css/capa/display.scss b/common/lib/xmodule/xmodule/css/capa/display.scss index 2088e8baa3..6b1c32ae65 100644 --- a/common/lib/xmodule/xmodule/css/capa/display.scss +++ b/common/lib/xmodule/xmodule/css/capa/display.scss @@ -49,6 +49,8 @@ padding-left: flex-gutter(9); } } + + div { p.status { text-indent: -9999px; @@ -64,6 +66,16 @@ div { } } + &.processing { + p.status { + @include inline-block(); + background: url('../images/spinner.gif') center center no-repeat; + height: 20px; + width: 20px; + text-indent: -9999px; + } + } + &.correct, &.ui-icon-check { p.status { @include inline-block(); @@ -134,6 +146,15 @@ div { width: 14px; } + &.processing, &.ui-icon-check { + @include inline-block(); + background: url('../images/spinner.gif') center center no-repeat; + height: 20px; + position: relative; + top: 6px; + width: 25px; + } + &.correct, &.ui-icon-check { @include inline-block(); background: url('../images/correct-icon.png') center center no-repeat; diff --git a/common/lib/xmodule/xmodule/css/sequence/display.scss b/common/lib/xmodule/xmodule/css/sequence/display.scss index 6e7c5d24a7..97a8e0e4b3 100644 --- a/common/lib/xmodule/xmodule/css/sequence/display.scss +++ b/common/lib/xmodule/xmodule/css/sequence/display.scss @@ -3,9 +3,9 @@ nav.sequence-nav { // import from external sources. @extend .topbar; border-bottom: 1px solid $border-color; + @include border-top-right-radius(4px); margin: (-(lh())) (-(lh())) lh() (-(lh())); position: relative; - @include border-top-right-radius(4px); ol { @include box-sizing(border-box); @@ -242,9 +242,11 @@ nav.sequence-bottom { border: 1px solid $border-color; @include border-radius(3px); @include inline-block(); + width: 100px; li { float: left; + width: 50%; &.prev, &.next { margin-bottom: 0; @@ -252,12 +254,11 @@ nav.sequence-bottom { a { background-position: center center; background-repeat: no-repeat; - border-bottom: none; + border: none; display: block; padding: lh(.5) 4px; text-indent: -9999px; @include transition(all, .2s, $ease-in-out-quad); - width: 45px; &:hover { background-color: #ddd; @@ -275,7 +276,7 @@ nav.sequence-bottom { &.prev { a { background-image: url('../images/sequence-nav/previous-icon.png'); - border-right: 1px solid lighten($border-color, 10%); + border-right: 1px solid lighten(#c6c6c6, 10%); &:hover { background-color: none; diff --git a/common/lib/xmodule/xmodule/css/video/display.scss b/common/lib/xmodule/xmodule/css/video/display.scss index 789a267755..235e2e3277 100644 --- a/common/lib/xmodule/xmodule/css/video/display.scss +++ b/common/lib/xmodule/xmodule/css/video/display.scss @@ -16,7 +16,6 @@ div.video { height: 0; overflow: hidden; padding-bottom: 56.25%; - padding-top: 30px; position: relative; object, iframe { @@ -207,7 +206,7 @@ div.video { h3 { color: #999; float: left; - font-size: 12px; + font-size: em(14); font-weight: normal; letter-spacing: 1px; padding: 0 lh(.25) 0 lh(.5); @@ -221,6 +220,7 @@ div.video { margin-bottom: 0; padding: 0 lh(.5) 0 0; line-height: 46px; + color: #fff; } &:hover, &:active, &:focus { @@ -462,7 +462,8 @@ div.video { } ol.subtitles { - width: 0px; + width: 0; + height: 0; } } diff --git a/common/lib/xmodule/xmodule/error_module.py b/common/lib/xmodule/xmodule/error_module.py index ecc90873b9..20301ee460 100644 --- a/common/lib/xmodule/xmodule/error_module.py +++ b/common/lib/xmodule/xmodule/error_module.py @@ -1,5 +1,8 @@ -import sys +import hashlib import logging +import random +import string +import sys from pkg_resources import resource_string from lxml import etree @@ -24,6 +27,14 @@ class ErrorModule(XModule): 'is_staff' : self.system.is_staff, }) + def displayable_items(self): + """Hide errors in the profile and table of contents for non-staff + users. + """ + if self.system.is_staff: + return [self] + return [] + class ErrorDescriptor(EditingDescriptor): """ Module that provides a raw editing view of broken xml. @@ -35,7 +46,8 @@ class ErrorDescriptor(EditingDescriptor): error_msg='Error not available'): '''Create an instance of this descriptor from the supplied data. - Does not try to parse the data--just stores it. + Does not require that xml_data be parseable--just stores it and exports + as-is if not. Takes an extra, optional, parameter--the error that caused an issue. (should be a string, or convert usefully into one). @@ -45,6 +57,13 @@ class ErrorDescriptor(EditingDescriptor): definition = {'data': inner} inner['error_msg'] = str(error_msg) + # Pick a unique url_name -- the sha1 hash of the xml_data. + # NOTE: We could try to pull out the url_name of the errored descriptor, + # but url_names aren't guaranteed to be unique between descriptor types, + # and ErrorDescriptor can wrap any type. When the wrapped module is fixed, + # it will be written out with the original url_name. + url_name = hashlib.sha1(xml_data).hexdigest() + try: # If this is already an error tag, don't want to re-wrap it. xml_obj = etree.fromstring(xml_data) @@ -63,8 +82,9 @@ class ErrorDescriptor(EditingDescriptor): inner['contents'] = xml_data # TODO (vshnayder): Do we need a unique slug here? Just pick a random # 64-bit num? - location = ['i4x', org, course, 'error', 'slug'] - metadata = {} # stays in the xml_data + location = ['i4x', org, course, 'error', url_name] + # real metadata stays in the xml_data, but add a display name + metadata = {'display_name': 'Error ' + url_name} return cls(system, definition, location=location, metadata=metadata) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index 260b84278b..5e7c1f7e3f 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -13,13 +13,14 @@ from .html_checker import check_html log = logging.getLogger("mitx.courseware") + class HtmlModule(XModule): def get_html(self): return self.html - def __init__(self, system, location, definition, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) self.html = self.definition['data'] @@ -36,18 +37,19 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): # are being edited in the cms @classmethod def backcompat_paths(cls, path): - origpath = path if path.endswith('.html.xml'): - path = path[:-9] + '.html' #backcompat--look for html instead of xml + path = path[:-9] + '.html' # backcompat--look for html instead of xml candidates = [] while os.sep in path: candidates.append(path) _, _, path = path.partition(os.sep) # also look for .html versions instead of .xml - if origpath.endswith('.xml'): - candidates.append(origpath[:-4] + '.html') - return candidates + nc = [] + for candidate in candidates: + if candidate.endswith('.xml'): + nc.append(candidate[:-4] + '.html') + return candidates + nc # NOTE: html descriptors are special. We do not want to parse and # export them ourselves, because that can break things (e.g. lxml @@ -69,7 +71,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): if filename is None: definition_xml = copy.deepcopy(xml_object) cls.clean_metadata_from_xml(definition_xml) - return {'data' : stringify_children(definition_xml)} + return {'data': stringify_children(definition_xml)} else: filepath = cls._format_filepath(xml_object.tag, filename) @@ -80,7 +82,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): # online and has imported all current (fall 2012) courses from xml if not system.resources_fs.exists(filepath): candidates = cls.backcompat_paths(filepath) - #log.debug("candidates = {0}".format(candidates)) + log.debug("candidates = {0}".format(candidates)) for candidate in candidates: if system.resources_fs.exists(candidate): filepath = candidate @@ -95,7 +97,7 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): log.warning(msg) system.error_tracker("Warning: " + msg) - definition = {'data' : html} + definition = {'data': html} # TODO (ichuang): remove this after migration # for Fall 2012 LMS migration: keep filename (and unmangled filename) @@ -109,17 +111,11 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): # add more info and re-raise raise Exception(msg), None, sys.exc_info()[2] - @classmethod - def split_to_file(cls, xml_object): - '''Never include inline html''' - return True - - # TODO (vshnayder): make export put things in the right places. def definition_to_xml(self, resource_fs): '''If the contents are valid xml, write them to filename.xml. Otherwise, - write just the tag to filename.xml, and the html + write just to filename.xml, and the html string to filename.html. ''' try: @@ -138,4 +134,3 @@ class HtmlDescriptor(XmlDescriptor, EditingDescriptor): elt = etree.Element('html') elt.set("filename", self.url_name) return elt - diff --git a/common/lib/xmodule/xmodule/js/src/capa/display.coffee b/common/lib/xmodule/xmodule/js/src/capa/display.coffee index 333dac745e..6bdd8e1c36 100644 --- a/common/lib/xmodule/xmodule/js/src/capa/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/capa/display.coffee @@ -13,7 +13,10 @@ class @Problem bind: => MathJax.Hub.Queue ["Typeset", MathJax.Hub] window.update_schematics() - @inputs = @$("[id^=input_#{@element_id.replace(/problem_/, '')}_]") + + problem_prefix = @element_id.replace(/problem_/,'') + @inputs = @$("[id^=input_#{problem_prefix}_]") + @$('section.action input:button').click @refreshAnswers @$('section.action input.check').click @check_fd #@$('section.action input.check').click @check @@ -27,18 +30,40 @@ class @Problem @el.attr progress: response.progress_status @el.trigger('progressChanged') + queueing: => + @queued_items = @$(".xqueue") + if @queued_items.length > 0 + if window.queuePollerID # Only one poller 'thread' per Problem + window.clearTimeout(window.queuePollerID) + window.queuePollerID = window.setTimeout(@poll, 100) + + poll: => + $.postWithPrefix "#{@url}/problem_get", (response) => + @el.html(response.html) + @executeProblemScripts() + @bind() + + @queued_items = @$(".xqueue") + if @queued_items.length == 0 + delete window.queuePollerID + else + # TODO: Dynamically adjust timeout interval based on @queued_items.value + window.queuePollerID = window.setTimeout(@poll, 1000) + render: (content) -> if content @el.html(content) @executeProblemScripts () => @setupInputTypes() @bind() + @queueing() else $.postWithPrefix "#{@url}/problem_get", (response) => @el.html(response.html) @executeProblemScripts () => @setupInputTypes() @bind() + @queueing() # TODO add hooks for problem types here by inspecting response.html and doing # stuff if a div w a class is found diff --git a/common/lib/xmodule/xmodule/js/src/sequence/display.coffee b/common/lib/xmodule/xmodule/js/src/sequence/display.coffee index 0b17111d81..832a5ec7eb 100644 --- a/common/lib/xmodule/xmodule/js/src/sequence/display.coffee +++ b/common/lib/xmodule/xmodule/js/src/sequence/display.coffee @@ -91,6 +91,13 @@ class @Sequence event.preventDefault() new_position = $(event.target).data('element') Logger.log "seq_goto", old: @position, new: new_position, id: @id + + # On Sequence chage, destroy any existing polling thread + # for queued submissions, see ../capa/display.coffee + if window.queuePollerID + window.clearTimeout(window.queuePollerID) + delete window.queuePollerID + @render new_position next: (event) => 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/common/lib/xmodule/xmodule/modulestore/mongo.py b/common/lib/xmodule/xmodule/modulestore/mongo.py index 1cec6c7f87..b6101a6929 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo.py @@ -55,6 +55,9 @@ class CachingDescriptorSystem(MakoDescriptorSystem): if json_data is None: return self.modulestore.get_item(location) else: + # TODO (vshnayder): metadata inheritance is somewhat broken because mongo, doesn't + # always load an entire course. We're punting on this until after launch, and then + # will build a proper course policy framework. return XModuleDescriptor.load_from_json(json_data, self, self.default_class) diff --git a/common/lib/xmodule/xmodule/modulestore/xml.py b/common/lib/xmodule/xmodule/modulestore/xml.py index 46fcf19469..8c4c373d4f 100644 --- a/common/lib/xmodule/xmodule/modulestore/xml.py +++ b/common/lib/xmodule/xmodule/modulestore/xml.py @@ -50,8 +50,9 @@ class ImportSystem(XMLParsingSystem, MakoDescriptorSystem): # have been imported into the cms from xml xml = clean_out_mako_templating(xml) xml_data = etree.fromstring(xml) - except: - log.exception("Unable to parse xml: {xml}".format(xml=xml)) + except Exception as err: + log.warning("Unable to parse xml: {err}, xml: {xml}".format( + err=str(err), xml=xml)) raise # VS[compat]. Take this out once course conversion is done @@ -188,26 +189,37 @@ class XMLModuleStore(ModuleStoreBase): course_file = StringIO(clean_out_mako_templating(course_file.read())) course_data = etree.parse(course_file).getroot() + org = course_data.get('org') if org is None: - log.error("No 'org' attribute set for course in {dir}. " + msg = ("No 'org' attribute set for course in {dir}. " "Using default 'edx'".format(dir=course_dir)) + log.warning(msg) + tracker(msg) org = 'edx' course = course_data.get('course') if course is None: - log.error("No 'course' attribute set for course in {dir}." + msg = ("No 'course' attribute set for course in {dir}." " Using default '{default}'".format( dir=course_dir, default=course_dir )) + log.warning(msg) + tracker(msg) course = course_dir system = ImportSystem(self, org, course, course_dir, tracker) course_descriptor = system.process_xml(etree.tostring(course_data)) + # NOTE: The descriptors end up loading somewhat bottom up, which + # breaks metadata inheritance via get_children(). Instead + # (actually, in addition to, for now), we do a final inheritance pass + # after we have the course descriptor. + XModuleDescriptor.compute_inherited_metadata(course_descriptor) + log.debug('========> Done with course import from {0}'.format(course_dir)) return course_descriptor diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index 5f7f41bf8d..2986c948d3 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -25,9 +25,9 @@ class SequenceModule(XModule): css = {'scss': [resource_string(__name__, 'css/sequence/display.scss')]} js_module_name = "Sequence" - def __init__(self, system, location, definition, instance_state=None, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) self.position = 1 @@ -107,6 +107,8 @@ class SequenceModule(XModule): class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): mako_template = 'widgets/sequence-edit.html' module_class = SequenceModule + + stores_state = True # For remembering where in the sequence the student is @classmethod def definition_from_xml(cls, xml_object, system): @@ -122,16 +124,3 @@ class SequenceDescriptor(MakoModuleDescriptor, XmlDescriptor): etree.fromstring(child.export_to_xml(resource_fs))) return xml_object - @classmethod - def split_to_file(cls, xml_object): - # Note: if we end up needing subclasses, can port this logic there. - yes = ('chapter',) - no = ('course',) - - if xml_object.tag in yes: - return True - elif xml_object.tag in no: - return False - - # otherwise maybe--delegate to superclass. - return XmlDescriptor.split_to_file(xml_object) diff --git a/common/lib/xmodule/xmodule/template_module.py b/common/lib/xmodule/xmodule/template_module.py index 260ad009f9..13eab038ec 100644 --- a/common/lib/xmodule/xmodule/template_module.py +++ b/common/lib/xmodule/xmodule/template_module.py @@ -26,12 +26,23 @@ class CustomTagModule(XModule): More information given in         the text """ - def __init__(self, system, location, definition, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) - xmltree = etree.fromstring(self.definition['data']) + def get_html(self): + return self.descriptor.rendered_html + + +class CustomTagDescriptor(RawDescriptor): + """ Descriptor for custom tags. Loads the template when created.""" + module_class = CustomTagModule + + @staticmethod + def render_template(system, xml_data): + '''Render the template, given the definition xml_data''' + xmltree = etree.fromstring(xml_data) if 'impl' in xmltree.attrib: template_name = xmltree.attrib['impl'] else: @@ -45,13 +56,20 @@ class CustomTagModule(XModule): .format(location)) params = dict(xmltree.items()) - with self.system.filestore.open( - 'custom_tags/{name}'.format(name=template_name)) as template: - self.html = Template(template.read()).render(**params) - - def get_html(self): - return self.html + with system.resources_fs.open('custom_tags/{name}' + .format(name=template_name)) as template: + return Template(template.read()).render(**params) -class CustomTagDescriptor(RawDescriptor): - module_class = CustomTagModule + def __init__(self, system, definition, **kwargs): + '''Render and save the template for this descriptor instance''' + super(CustomTagDescriptor, self).__init__(system, definition, **kwargs) + self.rendered_html = self.render_template(system, definition['data']) + + def export_to_file(self): + """ + Custom tags are special: since they're already pointers, we don't want + to export them in a file with yet another layer of indirection. + """ + return False + diff --git a/common/lib/xmodule/xmodule/tests/__init__.py b/common/lib/xmodule/xmodule/tests/__init__.py index bed7d37181..7230795163 100644 --- a/common/lib/xmodule/xmodule/tests/__init__.py +++ b/common/lib/xmodule/xmodule/tests/__init__.py @@ -10,12 +10,14 @@ import os import fs import json +import json import numpy import xmodule import capa.calc as calc import capa.capa_problem as lcp from capa.correctmap import CorrectMap +from capa.util import convert_files_to_filenames from xmodule import graders, x_module from xmodule.x_module import ModuleSystem from xmodule.graders import Score, aggregate_scores @@ -32,7 +34,7 @@ i4xs = ModuleSystem( user=Mock(), filestore=fs.osfs.OSFS(os.path.dirname(os.path.realpath(__file__))+"/test_files"), debug=True, - xqueue=None, + xqueue={'interface':None, 'callback_url':'/', 'default_queuename': 'testqueue'}, is_staff=False, node_path=os.environ.get("NODE_PATH", "/usr/local/lib/node_modules") ) @@ -280,7 +282,6 @@ class StringResponseWithHintTest(unittest.TestCase): class CodeResponseTest(unittest.TestCase): ''' Test CodeResponse - ''' def test_update_score(self): problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" @@ -293,9 +294,14 @@ class CodeResponseTest(unittest.TestCase): for i in range(numAnswers): old_cmap.update(CorrectMap(answer_id=answer_ids[i], queuekey=1000 + i)) - # Message format inherited from ExternalResponse - correct_score_msg = "EXACT_ANSMESSAGE" - incorrect_score_msg = "WRONG_FORMATMESSAGE" + # TODO: Message format inherited from ExternalResponse + #correct_score_msg = "EXACT_ANSMESSAGE" + #incorrect_score_msg = "WRONG_FORMATMESSAGE" + + # New message format common to external graders + correct_score_msg = json.dumps({'correct':True, 'score':1, 'msg':'MESSAGE'}) + incorrect_score_msg = json.dumps({'correct':False, 'score':0, 'msg':'MESSAGE'}) + xserver_msgs = {'correct': correct_score_msg, 'incorrect': incorrect_score_msg, } @@ -329,7 +335,18 @@ class CodeResponseTest(unittest.TestCase): self.assertFalse(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be dequeued, message delivered else: self.assertTrue(test_lcp.correct_map.is_queued(answer_ids[j])) # Should be queued, message undelivered - + + def test_convert_files_to_filenames(self): + problem_file = os.path.dirname(__file__) + "/test_files/coderesponse.xml" + fp = open(problem_file) + answers_with_file = {'1_2_1': 'String-based answer', + '1_3_1': ['answer1', 'answer2', 'answer3'], + '1_4_1': fp} + answers_converted = convert_files_to_filenames(answers_with_file) + self.assertEquals(answers_converted['1_2_1'], 'String-based answer') + self.assertEquals(answers_converted['1_3_1'], ['answer1', 'answer2', 'answer3']) + self.assertEquals(answers_converted['1_4_1'], fp.name) + class ChoiceResponseTest(unittest.TestCase): @@ -712,6 +729,6 @@ class ModuleProgressTest(unittest.TestCase): ''' def test_xmodule_default(self): '''Make sure default get_progress exists, returns None''' - xm = x_module.XModule(i4xs, 'a://b/c/d/e', {}) + xm = x_module.XModule(i4xs, 'a://b/c/d/e', None, {}) p = xm.get_progress() self.assertEqual(p, None) diff --git a/common/lib/xmodule/xmodule/tests/test_export.py b/common/lib/xmodule/xmodule/tests/test_export.py index eacf8352be..268c3d1062 100644 --- a/common/lib/xmodule/xmodule/tests/test_export.py +++ b/common/lib/xmodule/xmodule/tests/test_export.py @@ -1,36 +1,113 @@ -from xmodule.modulestore.xml import XMLModuleStore -from nose.tools import assert_equals -from nose import SkipTest -from tempfile import mkdtemp +import unittest + from fs.osfs import OSFS +from nose.tools import assert_equals, assert_true +from path import path +from tempfile import mkdtemp +from shutil import copytree + +from xmodule.modulestore.xml import XMLModuleStore + +# from ~/mitx_all/mitx/common/lib/xmodule/xmodule/tests/ +# to ~/mitx_all/mitx/common/test +TEST_DIR = path(__file__).abspath().dirname() +for i in range(4): + TEST_DIR = TEST_DIR.dirname() +TEST_DIR = TEST_DIR / 'test' + +DATA_DIR = TEST_DIR / 'data' -def check_export_roundtrip(data_dir): - print "Starting import" - initial_import = XMLModuleStore('org', 'course', data_dir, eager=True) - initial_course = initial_import.course +def strip_metadata(descriptor, key): + """ + Recursively strips tag from all children. + """ + print "strip {key} from {desc}".format(key=key, desc=descriptor.location.url()) + descriptor.metadata.pop(key, None) + for d in descriptor.get_children(): + strip_metadata(d, key) - print "Starting export" - export_dir = mkdtemp() - fs = OSFS(export_dir) - xml = initial_course.export_to_xml(fs) - with fs.open('course.xml', 'w') as course_xml: - course_xml.write(xml) - - print "Starting second import" - second_import = XMLModuleStore('org', 'course', export_dir, eager=True) - - print "Checking key equality" - assert_equals(initial_import.modules.keys(), second_import.modules.keys()) - - print "Checking module equality" - for location in initial_import.modules.keys(): - print "Checking", location - assert_equals(initial_import.modules[location], second_import.modules[location]) +def strip_filenames(descriptor): + """ + Recursively strips 'filename' from all children's definitions. + """ + print "strip filename from {desc}".format(desc=descriptor.location.url()) + descriptor.definition.pop('filename', None) + for d in descriptor.get_children(): + strip_filenames(d) -def test_toy_roundtrip(): - dir = "" - # TODO: add paths and make this run. - raise SkipTest() - check_export_roundtrip(dir) + +class RoundTripTestCase(unittest.TestCase): + '''Check that our test courses roundtrip properly''' + def check_export_roundtrip(self, data_dir, course_dir): + + root_dir = path(mkdtemp()) + print "Copying test course to temp dir {0}".format(root_dir) + + data_dir = path(data_dir) + copytree(data_dir / course_dir, root_dir / course_dir) + + print "Starting import" + initial_import = XMLModuleStore(root_dir, eager=True, course_dirs=[course_dir]) + + courses = initial_import.get_courses() + self.assertEquals(len(courses), 1) + initial_course = courses[0] + + # export to the same directory--that way things like the custom_tags/ folder + # will still be there. + print "Starting export" + fs = OSFS(root_dir) + export_fs = fs.makeopendir(course_dir) + + xml = initial_course.export_to_xml(export_fs) + with export_fs.open('course.xml', 'w') as course_xml: + course_xml.write(xml) + + print "Starting second import" + second_import = XMLModuleStore(root_dir, eager=True, course_dirs=[course_dir]) + + courses2 = second_import.get_courses() + self.assertEquals(len(courses2), 1) + exported_course = courses2[0] + + print "Checking course equality" + # HACK: data_dir metadata tags break equality because they + # aren't real metadata, and depend on paths. Remove them. + strip_metadata(initial_course, 'data_dir') + strip_metadata(exported_course, 'data_dir') + + # HACK: filenames change when changing file formats + # during imports from old-style courses. Ignore them. + strip_filenames(initial_course) + strip_filenames(exported_course) + + self.assertEquals(initial_course, exported_course) + + print "Checking key equality" + self.assertEquals(sorted(initial_import.modules.keys()), + sorted(second_import.modules.keys())) + + print "Checking module equality" + for location in initial_import.modules.keys(): + print "Checking", location + if location.category == 'html': + print ("Skipping html modules--they can't import in" + " final form without writing files...") + continue + self.assertEquals(initial_import.modules[location], + second_import.modules[location]) + + + def setUp(self): + self.maxDiff = None + + def test_toy_roundtrip(self): + self.check_export_roundtrip(DATA_DIR, "toy") + + def test_simple_roundtrip(self): + self.check_export_roundtrip(DATA_DIR, "simple") + + def test_full_roundtrip(self): + self.check_export_roundtrip(DATA_DIR, "full") diff --git a/common/lib/xmodule/xmodule/tests/test_import.py b/common/lib/xmodule/xmodule/tests/test_import.py index 0d3e2260fb..1da618f6a4 100644 --- a/common/lib/xmodule/xmodule/tests/test_import.py +++ b/common/lib/xmodule/xmodule/tests/test_import.py @@ -5,10 +5,14 @@ from fs.memoryfs import MemoryFS from lxml import etree from xmodule.x_module import XMLParsingSystem, XModuleDescriptor +from xmodule.xml_module import is_pointer_tag from xmodule.errortracker import make_error_tracker from xmodule.modulestore import Location +from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore.exceptions import ItemNotFoundError +from .test_export import DATA_DIR + ORG = 'test_org' COURSE = 'test_course' @@ -46,22 +50,17 @@ class DummySystem(XMLParsingSystem): raise Exception("Shouldn't be called") - - class ImportTestCase(unittest.TestCase): '''Make sure module imports work properly, including for malformed inputs''' - - @staticmethod def get_system(): '''Get a dummy system''' return DummySystem() def test_fallback(self): - '''Make sure that malformed xml loads as an ErrorDescriptor.''' + '''Check that malformed xml loads as an ErrorDescriptor.''' bad_xml = '''''' - system = self.get_system() descriptor = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', 'course', @@ -70,6 +69,22 @@ class ImportTestCase(unittest.TestCase): self.assertEqual(descriptor.__class__.__name__, 'ErrorDescriptor') + + def test_unique_url_names(self): + '''Check that each error gets its very own url_name''' + bad_xml = '''''' + bad_xml2 = '''''' + system = self.get_system() + + descriptor1 = XModuleDescriptor.load_from_xml(bad_xml, system, 'org', + 'course', None) + + descriptor2 = XModuleDescriptor.load_from_xml(bad_xml2, system, 'org', + 'course', None) + + self.assertNotEqual(descriptor1.location, descriptor2.location) + + def test_reimport(self): '''Make sure an already-exported error xml tag loads properly''' @@ -111,30 +126,84 @@ class ImportTestCase(unittest.TestCase): xml_out = etree.fromstring(xml_str_out) self.assertEqual(xml_out.tag, 'sequential') - def test_metadata_inherit(self): - """Make sure metadata inherits properly""" + def test_metadata_import_export(self): + """Two checks: + - unknown metadata is preserved across import-export + - inherited metadata doesn't leak to children. + """ system = self.get_system() - v = "1 hour" - start_xml = ''' - - Two houses, ... - '''.format(grace=v) + v = '1 hour' + org = 'foo' + course = 'bbhh' + url_name = 'test1' + start_xml = ''' + + + Two houses, ... + + '''.format(grace=v, org=org, course=course, url_name=url_name) descriptor = XModuleDescriptor.load_from_xml(start_xml, system, - 'org', 'course') + org, course) - print "Errors: {0}".format(system.errorlog.errors) print descriptor, descriptor.metadata self.assertEqual(descriptor.metadata['graceperiod'], v) + self.assertEqual(descriptor.metadata['unicorn'], 'purple') - # Check that the child inherits correctly + # Check that the child inherits graceperiod correctly child = descriptor.get_children()[0] self.assertEqual(child.metadata['graceperiod'], v) - # Now export and see if the chapter tag has a graceperiod attribute + # check that the child does _not_ inherit any unicorns + self.assertTrue('unicorn' not in child.metadata) + + # Now export and check things resource_fs = MemoryFS() exported_xml = descriptor.export_to_xml(resource_fs) + + # Check that the exported xml is just a pointer print "Exported xml:", exported_xml - root = etree.fromstring(exported_xml) - chapter_tag = root[0] - self.assertEqual(chapter_tag.tag, 'chapter') - self.assertFalse('graceperiod' in chapter_tag.attrib) + pointer = etree.fromstring(exported_xml) + self.assertTrue(is_pointer_tag(pointer)) + # but it's a special case course pointer + self.assertEqual(pointer.attrib['course'], course) + self.assertEqual(pointer.attrib['org'], org) + + # Does the course still have unicorns? + with resource_fs.open('course/{url_name}.xml'.format(url_name=url_name)) as f: + course_xml = etree.fromstring(f.read()) + + self.assertEqual(course_xml.attrib['unicorn'], 'purple') + + # the course and org tags should be _only_ in the pointer + self.assertTrue('course' not in course_xml.attrib) + self.assertTrue('org' not in course_xml.attrib) + + # did we successfully strip the url_name from the definition contents? + self.assertTrue('url_name' not in course_xml.attrib) + + # Does the chapter tag now have a graceperiod attribute? + # hardcoded path to child + with resource_fs.open('chapter/ch.xml') as f: + chapter_xml = etree.fromstring(f.read()) + self.assertEqual(chapter_xml.tag, 'chapter') + self.assertFalse('graceperiod' in chapter_xml.attrib) + + def test_metadata_inherit(self): + """Make sure that metadata is inherited properly""" + + print "Starting import" + initial_import = XMLModuleStore(DATA_DIR, eager=True, course_dirs=['toy']) + + courses = initial_import.get_courses() + self.assertEquals(len(courses), 1) + course = courses[0] + + def check_for_key(key, node): + "recursive check for presence of key" + print "Checking {}".format(node.location.url()) + self.assertTrue(key in node.metadata) + for c in node.get_children(): + check_for_key(key, c) + + check_for_key('graceperiod', course) diff --git a/common/lib/xmodule/xmodule/util/__init__.py b/common/lib/xmodule/xmodule/util/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/lib/xmodule/xmodule/util/decorators.py b/common/lib/xmodule/xmodule/util/decorators.py new file mode 100644 index 0000000000..81ab747a3e --- /dev/null +++ b/common/lib/xmodule/xmodule/util/decorators.py @@ -0,0 +1,36 @@ + + +def lazyproperty(fn): + """ + Use this decorator for lazy generation of properties that + are expensive to compute. From http://stackoverflow.com/a/3013910/86828 + + + Example: + class Test(object): + + @lazyproperty + def a(self): + print 'generating "a"' + return range(5) + + Interactive Session: + >>> t = Test() + >>> t.__dict__ + {} + >>> t.a + generating "a" + [0, 1, 2, 3, 4] + >>> t.__dict__ + {'_lazy_a': [0, 1, 2, 3, 4]} + >>> t.a + [0, 1, 2, 3, 4] + """ + + attr_name = '_lazy_' + fn.__name__ + @property + def _lazyprop(self): + if not hasattr(self, attr_name): + setattr(self, attr_name, fn(self)) + return getattr(self, attr_name) + return _lazyprop \ No newline at end of file diff --git a/common/lib/xmodule/xmodule/vertical_module.py b/common/lib/xmodule/xmodule/vertical_module.py index 884fd0a056..52e5825e43 100644 --- a/common/lib/xmodule/xmodule/vertical_module.py +++ b/common/lib/xmodule/xmodule/vertical_module.py @@ -10,8 +10,8 @@ class_priority = ['video', 'problem'] class VerticalModule(XModule): ''' Layout module for laying out submodules vertically.''' - def __init__(self, system, location, definition, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, instance_state, shared_state, **kwargs) + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) self.contents = None def get_html(self): diff --git a/common/lib/xmodule/xmodule/video_module.py b/common/lib/xmodule/xmodule/video_module.py index fb68ba982b..7d11dea283 100644 --- a/common/lib/xmodule/xmodule/video_module.py +++ b/common/lib/xmodule/xmodule/video_module.py @@ -23,9 +23,9 @@ class VideoModule(XModule): css = {'scss': [resource_string(__name__, 'css/video/display.scss')]} js_module_name = "Video" - def __init__(self, system, location, definition, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): - XModule.__init__(self, system, location, definition, + XModule.__init__(self, system, location, definition, descriptor, instance_state, shared_state, **kwargs) xmltree = etree.fromstring(self.definition['data']) self.youtube = xmltree.get('youtube') @@ -80,3 +80,5 @@ class VideoModule(XModule): class VideoDescriptor(RawDescriptor): module_class = VideoModule + + stores_state = True diff --git a/common/lib/xmodule/xmodule/x_module.py b/common/lib/xmodule/xmodule/x_module.py index 538f27be2e..9f99f5a526 100644 --- a/common/lib/xmodule/xmodule/x_module.py +++ b/common/lib/xmodule/xmodule/x_module.py @@ -6,6 +6,7 @@ from fs.errors import ResourceNotFoundError from functools import partial from lxml import etree from lxml.etree import XMLSyntaxError +from pprint import pprint from xmodule.modulestore import Location from xmodule.errortracker import exc_info_to_str @@ -142,7 +143,7 @@ class XModule(HTMLSnippet): # in the module icon_class = 'other' - def __init__(self, system, location, definition, + def __init__(self, system, location, definition, descriptor, instance_state=None, shared_state=None, **kwargs): ''' Construct a new xmodule @@ -165,6 +166,10 @@ class XModule(HTMLSnippet): 'children': is a list of Location-like values for child modules that this module depends on + descriptor: the XModuleDescriptor that this module is an instance of. + TODO (vshnayder): remove the definition parameter and location--they + can come from the descriptor. + instance_state: A string of serialized json that contains the state of this module for current student accessing the system, or None if no state has been saved @@ -188,6 +193,7 @@ class XModule(HTMLSnippet): self.system = system self.location = Location(location) self.definition = definition + self.descriptor = descriptor self.instance_state = instance_state self.shared_state = shared_state self.id = self.location.url() @@ -303,10 +309,18 @@ class XModuleDescriptor(Plugin, HTMLSnippet): entry_point = "xmodule.v1" module_class = XModule + # Attributes for inpsection of the descriptor + stores_state = False # Indicates whether the xmodule state should be + # stored in a database (independent of shared state) + has_score = False # This indicates whether the xmodule is a problem-type. + # It should respond to max_score() and grade(). It can be graded or ungraded + # (like a practice problem). + # A list of metadata that this module can inherit from its parent module inheritable_metadata = ( 'graded', 'start', 'due', 'graceperiod', 'showanswer', 'rerandomize', - + # TODO (ichuang): used for Fall 2012 xqa server access + 'xqa_key', # TODO: This is used by the XMLModuleStore to provide for locations for # static files, and will need to be removed when that code is removed 'data_dir' @@ -390,6 +404,18 @@ class XModuleDescriptor(Plugin, HTMLSnippet): return dict((k,v) for k,v in self.metadata.items() if k not in self._inherited_metadata) + @staticmethod + def compute_inherited_metadata(node): + """Given a descriptor, traverse all of its descendants and do metadata + inheritance. Should be called on a CourseDescriptor after importing a + course. + + NOTE: This means that there is no such thing as lazy loading at the + moment--this accesses all the children.""" + for c in node.get_children(): + c.inherit_metadata(node.metadata) + XModuleDescriptor.compute_inherited_metadata(c) + def inherit_metadata(self, metadata): """ Updates this module with metadata inherited from a containing module. @@ -410,6 +436,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet): self._child_instances = [] for child_loc in self.definition.get('children', []): child = self.system.load_item(child_loc) + # TODO (vshnayder): this should go away once we have + # proper inheritance support in mongo. The xml + # datastore does all inheritance on course load. child.inherit_metadata(self.metadata) self._child_instances.append(child) @@ -426,6 +455,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet): system, self.location, self.definition, + self, metadata=self.metadata ) @@ -493,7 +523,7 @@ class XModuleDescriptor(Plugin, HTMLSnippet): # Put import here to avoid circular import errors from xmodule.error_module import ErrorDescriptor msg = "Error loading from xml." - log.exception(msg) + log.warning(msg + " " + str(err)) system.error_tracker(msg) err_msg = msg + "\n" + exc_info_to_str(sys.exc_info()) descriptor = ErrorDescriptor.from_xml(xml_data, system, org, course, @@ -550,9 +580,9 @@ class XModuleDescriptor(Plugin, HTMLSnippet): if not eq: for attr in self.equality_attributes: - print(getattr(self, attr, None), - getattr(other, attr, None), - getattr(self, attr, None) == getattr(other, attr, None)) + pprint((getattr(self, attr, None), + getattr(other, attr, None), + getattr(self, attr, None) == getattr(other, attr, None))) return eq @@ -586,9 +616,10 @@ class DescriptorSystem(object): try: x = access_some_resource() check_some_format(x) - except SomeProblem: - msg = 'Grommet {0} is broken'.format(x) - log.exception(msg) # don't rely on handler to log + except SomeProblem as err: + msg = 'Grommet {0} is broken: {1}'.format(x, str(err)) + log.warning(msg) # don't rely on tracker to log + # NOTE: we generally don't want content errors logged as errors self.system.error_tracker(msg) # work around return 'Oops, couldn't load grommet' @@ -643,7 +674,7 @@ class ModuleSystem(object): user=None, filestore=None, debug=False, - xqueue = None, + xqueue=None, is_staff=False, node_path=""): ''' @@ -668,7 +699,7 @@ class ModuleSystem(object): filestore - A filestore ojbect. Defaults to an instance of OSFS based at settings.DATA_DIR. - xqueue - Dict containing XqueueInterface object, as well as parameters + xqueue - Dict containing XqueueInterface object, as well as parameters for the specific StudentModule replace_urls - TEMPORARY - A function like static_replace.replace_urls diff --git a/common/lib/xmodule/xmodule/xml_module.py b/common/lib/xmodule/xmodule/xml_module.py index 7a12ed869d..399d5d3f91 100644 --- a/common/lib/xmodule/xmodule/xml_module.py +++ b/common/lib/xmodule/xmodule/xml_module.py @@ -1,6 +1,7 @@ from xmodule.x_module import XModuleDescriptor from xmodule.modulestore import Location from lxml import etree +import json import copy import logging import traceback @@ -11,22 +12,52 @@ import sys log = logging.getLogger(__name__) -_AttrMapBase = namedtuple('_AttrMap', 'metadata_key to_metadata from_metadata') + +def is_pointer_tag(xml_obj): + """ + Check if xml_obj is a pointer tag: . + No children, one attribute named url_name. + + Special case for course roots: the pointer is + + + xml_obj: an etree Element + + Returns a bool. + """ + if xml_obj.tag != "course": + expected_attr = set(['url_name']) + else: + expected_attr = set(['url_name', 'course', 'org']) + + actual_attr = set(xml_obj.attrib.keys()) + return len(xml_obj) == 0 and actual_attr == expected_attr + +def get_metadata_from_xml(xml_object, remove=True): + meta = xml_object.find('meta') + if meta is None: + return '' + dmdata = meta.text + log.debug('meta for %s loaded: %s' % (xml_object,dmdata)) + if remove: + xml_object.remove(meta) + return dmdata + +_AttrMapBase = namedtuple('_AttrMap', 'from_xml to_xml') class AttrMap(_AttrMapBase): """ - A class that specifies a metadata_key, and two functions: + A class that specifies two functions: - to_metadata: convert value from the xml representation into + from_xml: convert value from the xml representation into an internal python representation - from_metadata: convert the internal python representation into + to_xml: convert the internal python representation into the value to store in the xml. """ - def __new__(_cls, metadata_key, - to_metadata=lambda x: x, - from_metadata=lambda x: x): - return _AttrMapBase.__new__(_cls, metadata_key, to_metadata, from_metadata) + def __new__(_cls, from_xml=lambda x: x, + to_xml=lambda x: x): + return _AttrMapBase.__new__(_cls, from_xml, to_xml) class XmlDescriptor(XModuleDescriptor): @@ -39,19 +70,29 @@ class XmlDescriptor(XModuleDescriptor): # The attributes will be removed from the definition xml passed # to definition_from_xml, and from the xml returned by definition_to_xml + + # Note -- url_name isn't in this list because it's handled specially on + # import and export. + + # TODO (vshnayder): Do we need a list of metadata we actually + # understand? And if we do, is this the place? + # Related: What's the right behavior for clean_metadata? metadata_attributes = ('format', 'graceperiod', 'showanswer', 'rerandomize', 'start', 'due', 'graded', 'display_name', 'url_name', 'hide_from_toc', - 'ispublic', # if True, then course is listed for all users; see + 'ispublic', # if True, then course is listed for all users; see + 'xqa_key', # for xqaa server access # VS[compat] Remove once unused. 'name', 'slug') + metadata_to_strip = ('data_dir', + # VS[compat] -- remove the below attrs once everything is in the CMS + 'course', 'org', 'url_name', 'filename') # A dictionary mapping xml attribute names AttrMaps that describe how # to import and export them xml_attribute_map = { # type conversion: want True/False in python, "true"/"false" in xml - 'graded': AttrMap('graded', - lambda val: val == 'true', + 'graded': AttrMap(lambda val: val == 'true', lambda val: str(val).lower()), } @@ -101,12 +142,32 @@ class XmlDescriptor(XModuleDescriptor): """ return etree.parse(file_object).getroot() + @classmethod + def load_file(cls, filepath, fs, location): + ''' + Open the specified file in fs, and call cls.file_to_xml on it, + returning the lxml object. + + Add details and reraise on error. + ''' + try: + with fs.open(filepath) as file: + return cls.file_to_xml(file) + except Exception as err: + # Add info about where we are, but keep the traceback + msg = 'Unable to load file contents at path %s for item %s: %s ' % ( + filepath, location.url(), str(err)) + raise Exception, msg, sys.exc_info()[2] + + @classmethod def load_definition(cls, xml_object, system, location): '''Load a descriptor definition from the specified xml_object. Subclasses should not need to override this except in special cases (e.g. html module)''' + # VS[compat] -- the filename tag should go away once everything is + # converted. (note: make sure html files still work once this goes away) filename = xml_object.get('filename') if filename is None: definition_xml = copy.deepcopy(xml_object) @@ -120,25 +181,20 @@ class XmlDescriptor(XModuleDescriptor): # again in the correct format. This should go away once the CMS is # online and has imported all current (fall 2012) courses from xml if not system.resources_fs.exists(filepath) and hasattr( - cls, - 'backcompat_paths'): + cls, 'backcompat_paths'): candidates = cls.backcompat_paths(filepath) for candidate in candidates: if system.resources_fs.exists(candidate): filepath = candidate break - try: - with system.resources_fs.open(filepath) as file: - definition_xml = cls.file_to_xml(file) - except Exception: - msg = 'Unable to load file contents at path %s for item %s' % ( - filepath, location.url()) - # Add info about where we are, but keep the traceback - raise Exception, msg, sys.exc_info()[2] + definition_xml = cls.load_file(filepath, system.resources_fs, location) + definition_metadata = get_metadata_from_xml(definition_xml) cls.clean_metadata_from_xml(definition_xml) definition = cls.definition_from_xml(definition_xml, system) + if definition_metadata: + definition['definition_metadata'] = definition_metadata # TODO (ichuang): remove this after migration # for Fall 2012 LMS migration: keep filename (and unmangled filename) @@ -146,6 +202,28 @@ class XmlDescriptor(XModuleDescriptor): return definition + @classmethod + def load_metadata(cls, xml_object): + """ + Read the metadata attributes from this xml_object. + + Returns a dictionary {key: value}. + """ + metadata = {} + for attr in xml_object.attrib: + val = xml_object.get(attr) + if val is not None: + # VS[compat]. Remove after all key translations done + attr = cls._translate(attr) + + if attr in cls.metadata_to_strip: + # don't load these + continue + + attr_map = cls.xml_attribute_map.get(attr, AttrMap()) + metadata[attr] = attr_map.from_xml(val) + return metadata + @classmethod def from_xml(cls, xml_data, system, org=None, course=None): @@ -160,26 +238,38 @@ class XmlDescriptor(XModuleDescriptor): url identifiers """ xml_object = etree.fromstring(xml_data) - # VS[compat] -- just have the url_name lookup once translation is done - slug = xml_object.get('url_name', xml_object.get('slug')) - location = Location('i4x', org, course, xml_object.tag, slug) + # VS[compat] -- just have the url_name lookup, once translation is done + url_name = xml_object.get('url_name', xml_object.get('slug')) + location = Location('i4x', org, course, xml_object.tag, url_name) - def load_metadata(): - metadata = {} - for attr in cls.metadata_attributes: - val = xml_object.get(attr) - if val is not None: - # VS[compat]. Remove after all key translations done - attr = cls._translate(attr) + # VS[compat] -- detect new-style each-in-a-file mode + if is_pointer_tag(xml_object): + # new style: + # read the actual definition file--named using url_name + filepath = cls._format_filepath(xml_object.tag, url_name) + definition_xml = cls.load_file(filepath, system.resources_fs, location) + else: + definition_xml = xml_object # this is just a pointer, not the real definition content - attr_map = cls.xml_attribute_map.get(attr, AttrMap(attr)) - metadata[attr_map.metadata_key] = attr_map.to_metadata(val) - return metadata + definition = cls.load_definition(definition_xml, system, location) # note this removes metadata + # VS[compat] -- make Ike's github preview links work in both old and + # new file layouts + if is_pointer_tag(xml_object): + # new style -- contents actually at filepath + definition['filename'] = [filepath, filepath] + + metadata = cls.load_metadata(definition_xml) + + # move definition metadata into dict + dmdata = definition.get('definition_metadata','') + if dmdata: + metadata['definition_metadata_raw'] = dmdata + try: + metadata.update(json.loads(dmdata)) + except Exception as err: + log.debug('Error %s in loading metadata %s' % (err,dmdata)) + metadata['definition_metadata_err'] = str(err) - definition = cls.load_definition(xml_object, system, location) - metadata = load_metadata() - # VS[compat] -- just have the url_name lookup once translation is done - slug = xml_object.get('url_name', xml_object.get('slug')) return cls( system, definition, @@ -193,19 +283,14 @@ class XmlDescriptor(XModuleDescriptor): name=name, ext=cls.filename_extension) - @classmethod - def split_to_file(cls, xml_object): - ''' - Decide whether to write this object to a separate file or not. + def export_to_file(self): + """If this returns True, write the definition of this descriptor to a separate + file. - xml_object: an xml definition of an instance of cls. + NOTE: Do not override this without a good reason. It is here specifically for customtag... + """ + return True - This default implementation will split if this has more than 7 - descendant tags. - - Can be overridden by subclasses. - ''' - return len(list(xml_object.iter())) > 7 def export_to_xml(self, resource_fs): """ @@ -227,42 +312,43 @@ class XmlDescriptor(XModuleDescriptor): xml_object = self.definition_to_xml(resource_fs) self.__class__.clean_metadata_from_xml(xml_object) - # Set the tag first, so it's right if writing to a file + # Set the tag so we get the file path right xml_object.tag = self.category - # Write it to a file if necessary - if self.split_to_file(xml_object): - # Put this object in its own file + def val_for_xml(attr): + """Get the value for this attribute that we want to store. + (Possible format conversion through an AttrMap). + """ + attr_map = self.xml_attribute_map.get(attr, AttrMap()) + return attr_map.to_xml(self.own_metadata[attr]) + + # Add the non-inherited metadata + for attr in sorted(self.own_metadata): + # don't want e.g. data_dir + if attr not in self.metadata_to_strip: + xml_object.set(attr, val_for_xml(attr)) + + if self.export_to_file(): + # Write the definition to a file filepath = self.__class__._format_filepath(self.category, self.url_name) resource_fs.makedir(os.path.dirname(filepath), allow_recreate=True) with resource_fs.open(filepath, 'w') as file: file.write(etree.tostring(xml_object, pretty_print=True)) - # ...and remove all of its children here - for child in xml_object: - xml_object.remove(child) - # also need to remove the text of this object. - xml_object.text = '' - # and the tail for good measure... - xml_object.tail = '' + # And return just a pointer with the category and filename. + record_object = etree.Element(self.category) + else: + record_object = xml_object - xml_object.set('filename', self.url_name) + record_object.set('url_name', self.url_name) - # Add the metadata - xml_object.set('url_name', self.url_name) - for attr in self.metadata_attributes: - attr_map = self.xml_attribute_map.get(attr, AttrMap(attr)) - metadata_key = attr_map.metadata_key + # Special case for course pointers: + if self.category == 'course': + # add org and course attributes on the pointer tag + record_object.set('org', self.location.org) + record_object.set('course', self.location.course) - if (metadata_key not in self.metadata or - metadata_key in self._inherited_metadata): - continue - - val = attr_map.from_metadata(self.metadata[metadata_key]) - xml_object.set(attr, val) - - # Now we just have to make it beautiful - return etree.tostring(xml_object, pretty_print=True) + return etree.tostring(record_object, pretty_print=True) def definition_to_xml(self, resource_fs): """ diff --git a/common/static/images/sequence-nav/document-icon-current.png b/common/static/images/sequence-nav/document-icon-current.png index f82f38ed59..4eeedd8f76 100644 Binary files a/common/static/images/sequence-nav/document-icon-current.png and b/common/static/images/sequence-nav/document-icon-current.png differ diff --git a/common/static/images/sequence-nav/document-icon-normal.png b/common/static/images/sequence-nav/document-icon-normal.png index 41c7e3af93..d508136647 100644 Binary files a/common/static/images/sequence-nav/document-icon-normal.png and b/common/static/images/sequence-nav/document-icon-normal.png differ diff --git a/common/static/images/sequence-nav/document-icon-visited.png b/common/static/images/sequence-nav/document-icon-visited.png index df8ff0fa7e..c0a1c7d099 100644 Binary files a/common/static/images/sequence-nav/document-icon-visited.png and b/common/static/images/sequence-nav/document-icon-visited.png differ diff --git a/common/static/images/sequence-nav/edit.png b/common/static/images/sequence-nav/edit.png index 6d7f0632c3..f7f5d3d944 100644 Binary files a/common/static/images/sequence-nav/edit.png and b/common/static/images/sequence-nav/edit.png differ diff --git a/common/static/images/sequence-nav/history.png b/common/static/images/sequence-nav/history.png index a65c4398d9..c063fa2240 100644 Binary files a/common/static/images/sequence-nav/history.png and b/common/static/images/sequence-nav/history.png differ diff --git a/common/static/images/sequence-nav/list-icon-current.png b/common/static/images/sequence-nav/list-icon-current.png index be95f52ac5..602f0da61b 100644 Binary files a/common/static/images/sequence-nav/list-icon-current.png and b/common/static/images/sequence-nav/list-icon-current.png differ diff --git a/common/static/images/sequence-nav/list-icon-normal.png b/common/static/images/sequence-nav/list-icon-normal.png index 42c473e82b..09867e09c8 100644 Binary files a/common/static/images/sequence-nav/list-icon-normal.png and b/common/static/images/sequence-nav/list-icon-normal.png differ diff --git a/common/static/images/sequence-nav/list-icon-visited.png b/common/static/images/sequence-nav/list-icon-visited.png index ab911cfa8b..dca52dd3cc 100644 Binary files a/common/static/images/sequence-nav/list-icon-visited.png and b/common/static/images/sequence-nav/list-icon-visited.png differ diff --git a/common/static/images/sequence-nav/next-icon.png b/common/static/images/sequence-nav/next-icon.png index 135d4fa715..ece431dcb8 100644 Binary files a/common/static/images/sequence-nav/next-icon.png and b/common/static/images/sequence-nav/next-icon.png differ diff --git a/common/static/images/sequence-nav/other-icon.png b/common/static/images/sequence-nav/other-icon.png index 822a415972..01c0bab93c 100644 Binary files a/common/static/images/sequence-nav/other-icon.png and b/common/static/images/sequence-nav/other-icon.png differ diff --git a/common/static/images/sequence-nav/previous-icon.png b/common/static/images/sequence-nav/previous-icon.png index 004a69ac4a..3b995df1f0 100644 Binary files a/common/static/images/sequence-nav/previous-icon.png and b/common/static/images/sequence-nav/previous-icon.png differ diff --git a/common/static/images/sequence-nav/problem-icon-alt.png b/common/static/images/sequence-nav/problem-icon-alt.png index c276ba223f..364e041bad 100644 Binary files a/common/static/images/sequence-nav/problem-icon-alt.png and b/common/static/images/sequence-nav/problem-icon-alt.png differ diff --git a/common/static/images/sequence-nav/vertical-icon.png b/common/static/images/sequence-nav/vertical-icon.png index be17eab195..064c6ba046 100644 Binary files a/common/static/images/sequence-nav/vertical-icon.png and b/common/static/images/sequence-nav/vertical-icon.png differ diff --git a/common/static/images/sequence-nav/video-icon-current.png b/common/static/images/sequence-nav/video-icon-current.png index 8bbc910f61..a7a787ee8b 100644 Binary files a/common/static/images/sequence-nav/video-icon-current.png and b/common/static/images/sequence-nav/video-icon-current.png differ diff --git a/common/static/images/sequence-nav/video-icon-normal.png b/common/static/images/sequence-nav/video-icon-normal.png index 52321ebf7b..df907e83a0 100644 Binary files a/common/static/images/sequence-nav/video-icon-normal.png and b/common/static/images/sequence-nav/video-icon-normal.png differ diff --git a/common/static/images/sequence-nav/video-icon-visited.png b/common/static/images/sequence-nav/video-icon-visited.png index b980c5b73e..49ab808567 100644 Binary files a/common/static/images/sequence-nav/video-icon-visited.png and b/common/static/images/sequence-nav/video-icon-visited.png differ diff --git a/common/static/images/sequence-nav/video-icon.png b/common/static/images/sequence-nav/video-icon.png index afb0ad86af..715a38eb96 100644 Binary files a/common/static/images/sequence-nav/video-icon.png and b/common/static/images/sequence-nav/video-icon.png differ diff --git a/common/static/images/sequence-nav/view.png b/common/static/images/sequence-nav/view.png index 8905b519a2..bc85d47c23 100644 Binary files a/common/static/images/sequence-nav/view.png and b/common/static/images/sequence-nav/view.png differ diff --git a/common/static/images/spinner.gif b/common/static/images/spinner.gif new file mode 100644 index 0000000000..b2f94cd12c Binary files /dev/null and b/common/static/images/spinner.gif differ diff --git a/common/test/data/simple/course.xml b/common/test/data/simple/course.xml index b92158cdb7..86dc8df45c 100644 --- a/common/test/data/simple/course.xml +++ b/common/test/data/simple/course.xml @@ -2,7 +2,7 @@
- + \ No newline at end of file diff --git a/lms/templates/grade_summary.html b/lms/templates/grade_summary.html new file mode 100644 index 0000000000..3fdcd910ae --- /dev/null +++ b/lms/templates/grade_summary.html @@ -0,0 +1,16 @@ +<%inherit file="main.html" /> +<%! from django.core.urlresolvers import reverse %> +<%namespace name='static' file='static_content.html'/> + +<%include file="course_navigation.html" args="active_page=''" /> + +
+
+
+

Grade summary

+ +

Not implemented yet

+ +
+
+
diff --git a/lms/templates/gradebook.html b/lms/templates/gradebook.html index bcc0456711..a4a81a6868 100644 --- a/lms/templates/gradebook.html +++ b/lms/templates/gradebook.html @@ -1,4 +1,5 @@ <%inherit file="main.html" /> +<%! from django.core.urlresolvers import reverse %> <%namespace name='static' file='static_content.html'/> <%block name="js_extra"> @@ -8,30 +9,32 @@ <%block name="headextra"> - + <%static:css group='course'/> + - + -<%include file="navigation.html" args="active_page=''" /> +<%include file="course_navigation.html" args="active_page=''" /> +

Gradebook

- + %if len(students) > 0: <% - templateSummary = students[0]['grade_info']['grade_summary'] + templateSummary = students[0]['grade_summary'] %> - - + + %for section in templateSummary['section_breakdown']: @@ -39,29 +42,32 @@ %endfor - - <%def name="percent_data(percentage)"> + + <%def name="percent_data(fraction)"> <% - data_class = "grade_none" - if percentage > .87: - data_class = "grade_a" - elif percentage > .70: - data_class = "grade_b" - elif percentage > .6: - data_class = "grade_c" - elif percentage > 0: - data_class = "grade_f" + letter_grade = 'None' + if fraction > 0: + letter_grade = 'F' + for grade in ['A', 'B', 'C']: + if fraction >= course.grade_cutoffs[grade]: + letter_grade = grade + break + + data_class = "grade_" + letter_grade %> - + - + %for student in students: - - %for section in student['grade_info']['grade_summary']['section_breakdown']: + + %for section in student['grade_summary']['section_breakdown']: ${percent_data( section['percent'] )} %endfor - + %endfor
StudentTotal
${ "{0:.0%}".format( percentage ) }${ "{0:.0f}".format( 100 * fraction ) }
${student['username']} + ${student['username']}${percent_data( student['grade_info']['grade_summary']['percent'])}${percent_data( student['grade_summary']['percent'])}
diff --git a/lms/templates/instructor_dashboard.html b/lms/templates/instructor_dashboard.html new file mode 100644 index 0000000000..6b87f63031 --- /dev/null +++ b/lms/templates/instructor_dashboard.html @@ -0,0 +1,24 @@ +<%inherit file="main.html" /> +<%! from django.core.urlresolvers import reverse %> +<%namespace name='static' file='static_content.html'/> + +<%block name="headextra"> + <%static:css group='course'/> + + +<%include file="course_navigation.html" args="active_page='instructor'" /> + +
+
+
+

Instructor Dashboard

+ +

+ Gradebook + +

+ Grade summary + +

+
+
diff --git a/lms/templates/login_modal.html b/lms/templates/login_modal.html index c89931695c..8652f457e6 100644 --- a/lms/templates/login_modal.html +++ b/lms/templates/login_modal.html @@ -27,9 +27,11 @@ Not enrolled? Forgot password?

+% if settings.MITX_FEATURES.get('AUTH_USE_OPENID'):

login via openid

+% endif
diff --git a/lms/templates/main.html b/lms/templates/main.html index fb502bfe22..891d770637 100644 --- a/lms/templates/main.html +++ b/lms/templates/main.html @@ -25,9 +25,9 @@ <%include file="navigation.html" />
${self.body()} + <%block name="bodyextra"/>
- <%block name="bodyextra"/> <%include file="footer.html" /> <%static:js group='application'/> diff --git a/lms/templates/navigation.html b/lms/templates/navigation.html index f628e346f2..6c4cfc853b 100644 --- a/lms/templates/navigation.html +++ b/lms/templates/navigation.html @@ -7,7 +7,12 @@
%endif diff --git a/lms/templates/portal/course_about.html b/lms/templates/portal/course_about.html index a3bf8dd755..36ec33607f 100644 --- a/lms/templates/portal/course_about.html +++ b/lms/templates/portal/course_about.html @@ -20,7 +20,7 @@ if(json.success) { location.href="${reverse('dashboard')}"; }else{ - $('#register_message).html("

" + json.error + "

") + $('#register_message').html("

" + json.error + "

"); } }); })(this) diff --git a/lms/templates/profile.html b/lms/templates/profile.html index 8107bb1923..ca27920a1b 100644 --- a/lms/templates/profile.html +++ b/lms/templates/profile.html @@ -18,99 +18,98 @@ - <%include file="course_navigation.html" args="active_page='profile'" />
@@ -138,20 +137,27 @@ $(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']} - %if 'due' in section and section['due']!="": - due ${section['due']} - %endif +

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

+

+ ${section['format']} + + %if 'due' in section and section['due']!="": + + due ${section['due']} + + %endif +

%if len(section['scores']) > 0: -
    - ${ "Problem Scores: " if section['graded'] else "Practice Scores: "} - %for score in section['scores']: -
  1. ${"{0:.3n}/{1:.3n}".format(float(score.earned),float(score.possible))}
    2. - %endfor -
+
+

${ "Problem Scores: " if section['graded'] else "Practice Scores: "}

+
    + %for score in section['scores']: +
  1. ${"{0:.3n}/{1:.3n}".format(float(score.earned),float(score.possible))}
    2. + %endfor +
+
%endif @@ -202,7 +208,7 @@ $(function() {
-
+ -
-

Apply to change your name

-
-
-
-

To uphold the credibility of edX certificates, name changes must go through an approval process. A member of the course staff will review your request, and if approved, update your information. Please allow up to a week for your request to be processed. Thank you.

-
    -
  • - - -
    • -
  • - - -
    • -
  • - -
    • -
-
-
+ -
-

Change e-mail

+ -
  • - - -
    • -
  • -

    We will send a confirmation to both ${email} and your new e-mail as part of the process.

    - -
    • - + -
    -

    Deactivate edX Account

    -

    Once you deactivate you’re MITx account you will no longer recieve updates and new class announcements from MITx.

    -

    If you’d like to still get updates and new class announcements you can just unenroll and keep your account active.

    + diff --git a/lms/templates/profile_graphs.js b/lms/templates/profile_graphs.js index 58dbeb8ed9..9826250331 100644 --- a/lms/templates/profile_graphs.js +++ b/lms/templates/profile_graphs.js @@ -1,6 +1,7 @@ -<%page args="grade_summary, graph_div_id, **kwargs"/> +<%page args="grade_summary, grade_cutoffs, graph_div_id, **kwargs"/> <%! import json + import math %> $(function () { @@ -89,8 +90,16 @@ $(function () { ticks += [ [overviewBarX, "Total"] ] tickIndex += 1 + sectionSpacer - totalScore = grade_summary['percent'] + totalScore = math.floor(grade_summary['percent'] * 100) / 100 #We floor it to the nearest percent, 80.9 won't show up like a 90 (an A) detail_tooltips['Dropped Scores'] = dropped_score_tooltips + + + ## ----------------------------- Grade cutoffs ------------------------- ## + + grade_cutoff_ticks = [ [1, "100%"], [0, "0%"] ] + for grade in ['A', 'B', 'C']: + percent = grade_cutoffs[grade] + grade_cutoff_ticks.append( [ percent, "{0} {1:.0%}".format(grade, percent) ] ) %> var series = ${ json.dumps( series ) }; @@ -98,6 +107,7 @@ $(function () { var bottomTicks = ${ json.dumps(bottomTicks) }; var detail_tooltips = ${ json.dumps(detail_tooltips) }; var droppedScores = ${ json.dumps(droppedScores) }; + var grade_cutoff_ticks = ${ json.dumps(grade_cutoff_ticks) } //Alwasy be sure that one series has the xaxis set to 2, or the second xaxis labels won't show up series.push( {label: 'Dropped Scores', data: droppedScores, points: {symbol: "cross", show: true, radius: 3}, bars: {show: false}, color: "#333"} ); @@ -107,10 +117,10 @@ $(function () { lines: {show: false, steps: false }, bars: {show: true, barWidth: 0.8, align: 'center', lineWidth: 0, fill: .8 },}, xaxis: {tickLength: 0, min: 0.0, max: ${tickIndex - sectionSpacer}, ticks: ticks, labelAngle: 90}, - yaxis: {ticks: [[1, "100%"], [0.87, "A 87%"], [0.7, "B 70%"], [0.6, "C 60%"], [0, "0%"]], min: 0.0, max: 1.0, labelWidth: 50}, + yaxis: {ticks: grade_cutoff_ticks, min: 0.0, max: 1.0, labelWidth: 50}, grid: { hoverable: true, clickable: true, borderWidth: 1, - markings: [ {yaxis: {from: 0.87, to: 1 }, color: "#ddd"}, {yaxis: {from: 0.7, to: 0.87 }, color: "#e9e9e9"}, - {yaxis: {from: 0.6, to: 0.7 }, color: "#f3f3f3"}, ] }, + markings: [ {yaxis: {from: ${grade_cutoffs['A']}, to: 1 }, color: "#ddd"}, {yaxis: {from: ${grade_cutoffs['B']}, to: ${grade_cutoffs['A']} }, color: "#e9e9e9"}, + {yaxis: {from: ${grade_cutoffs['C']}, to: ${grade_cutoffs['B']} }, color: "#f3f3f3"}, ] }, legend: {show: false}, }; diff --git a/lms/templates/simplewiki/simplewiki_base.html b/lms/templates/simplewiki/simplewiki_base.html index 6777af1b82..04a239b6c3 100644 --- a/lms/templates/simplewiki/simplewiki_base.html +++ b/lms/templates/simplewiki/simplewiki_base.html @@ -71,9 +71,9 @@ }); - + <%block name="wiki_head"/> - + <%block name="bodyextra"> @@ -86,7 +86,7 @@
    <%block name="wiki_panel">
    -

    Course Wiki

    +

    Course Wiki

    • @@ -101,12 +101,12 @@
      <% - baseURL = wiki_reverse("wiki_create", course=course, kwargs={"article_path" : namespace + "/" }) + baseURL = wiki_reverse("wiki_create", course=course, kwargs={"article_path" : namespace + "/" }) %>
      -
      +
      • @@ -130,31 +130,31 @@
        - %if wiki_article is not UNDEFINED: -
        - %if wiki_article.locked: -

        This article has been locked

        - %endif -

        Last modified: ${wiki_article.modified_on.strftime("%b %d, %Y, %I:%M %p")}

        - %endif - - %if wiki_article is not UNDEFINED: - -
        + %if wiki_article is not UNDEFINED: +
        + %if wiki_article.locked: +

        This article has been locked

        %endif +

        Last modified: ${wiki_article.modified_on.strftime("%b %d, %Y, %I:%M %p")}

        + %endif + + %if wiki_article is not UNDEFINED: + +
        + %endif <%block name="wiki_page_title"/> <%block name="wiki_body"/> diff --git a/lms/templates/simplewiki/simplewiki_edit.html b/lms/templates/simplewiki/simplewiki_edit.html index 7618dd629c..856dfb9cc8 100644 --- a/lms/templates/simplewiki/simplewiki_edit.html +++ b/lms/templates/simplewiki/simplewiki_edit.html @@ -65,12 +65,10 @@ ${"Edit " + wiki_title + " - " if wiki_title is not UNDEFINED else ""}MITx 6.002
      ${wiki_form} %if create_article: - + %else: - %endif - <%include file="simplewiki_instructions.html"/> diff --git a/lms/templates/simplewiki/simplewiki_searchresults.html b/lms/templates/simplewiki/simplewiki_searchresults.html index d94cbf9c25..e64a01ae62 100644 --- a/lms/templates/simplewiki/simplewiki_searchresults.html +++ b/lms/templates/simplewiki/simplewiki_searchresults.html @@ -26,7 +26,7 @@ Displaying all articles

    • ${article.title} ${'(Deleted)' if article_deleted else ''}

      • %endfor -%if not wiki_search_results: +%if not wiki_search_results: No articles matching ${wiki_search_query if wiki_search_query is not UNDEFINED else ""} ! %endif

    diff --git a/lms/templates/staff_problem_info.html b/lms/templates/staff_problem_info.html index c9b92c51db..f91decf876 100644 --- a/lms/templates/staff_problem_info.html +++ b/lms/templates/staff_problem_info.html @@ -1,11 +1,122 @@ ${module_content} %if edit_link: - + % endif -
    + + + + +
    + + +
    + +## leanModal needs to be included here otherwise this breaks when in a + + + + diff --git a/lms/templates/static_templates/jobs.html b/lms/templates/static_templates/jobs.html index 2ed341b625..392d13719a 100644 --- a/lms/templates/static_templates/jobs.html +++ b/lms/templates/static_templates/jobs.html @@ -29,49 +29,75 @@
    -

    EdX Fellow

    -

    EdX Fellows are immersed in developing innovative solutions for online teaching, learning and research. They partner with faculty and staff from edX universities in the development, implementation and evaluation of online courses. We're looking for candidates with recent masters or doctorate degrees in the social sciences, humanities, natural sciences, engineering, or education. We welcome new ways of thinking about both the promises and practices of online learning.

    +

    EdX Fellows focus on the development of innovative solutions for online teaching, learning, and research. They create and manage partnerships with faculty and staff from edX universities in the development, implementation, and evaluation of online courses and related learning products. EdX is seeking candidates with doctoral degrees in the social sciences, humanities, natural sciences, engineering, or education, who are committed to the development of innovative pedagogies to improve online teaching and learning.

    +

    An ideal candidate will have:

    +
      +
    • experience in teaching and developing online courses, preferably in higher education
      • +
    • exceptional written and communication skills
      • +
    • experience in facilitating and convening teams of higher education faculty
      • +
    • a broad knowledge of, and experience with, research in online learning
    • exceptional organizational and communication skills
      • +
    • proven success in digital project management.
      • +
    • strong background in working with LMS & CMS environments
      • +

    Ability to work in a fast-paced, highly collaborative environment is essential.

    -

    If you are interested in this position, please send an email to jobs@edx.org.

    +

    If you are interested in this position, please send an email to jobs@edx.org.

    +
    +
    +
    +
    +

    EdX Course Manager

    +

    Course Managers support edX Fellows and related content staff in the creation and implementation of online courses and other learning products. Course Managers are involved in the complete life-cycle of edX courses, from initial concept through development, launch, and data collection. EdX is seeking Course Managers who have a masters or doctorate degree.

    +

    An ideal candidate will have:

    +
      +
    • significant operational experience with online teaching and learning environments; CMS, LMS systems, and with API feature sets.
      • +
    • a broad knowledge of higher education content disciplines
      • +
    • experience with innovative instructional design practices
      • +
    • exceptional organizational and communication skills
      • +
    • proven success in digital project management
      • +
    • a working knowledge of basic computer programming skills, e.g. Python, XML, HTML5
      • +
    +

    Ability to work in a fast-paced, highly collaborative environment is essential.

    +

    If you are interested in this position, please send an email to jobs@edx.org

    +
    +
    +
    +
    +

    EdX Content Engineer

    +

    Content Engineers support edX Fellows and edX Course Managers in the overall technical development of course content, assessments, and domain-specific online tools. Tasks include developing graders for rich problems, designing automated tools for import of problems from other formats, as well as creating new ways for students to interact with domain-specific problems in the system.

    +

    A candidate must have:

    +
      +
    • Python or JavaScript development experience
      • +
    • A deep interest in pedagogy and education
      • +
    +

    Knowledge of GWT or Backbone.js a plus.

    If you are interested in this position, please send an email to jobs@edx.org.

    -

    Platform Developer

    Platform Developers build the core learning platform that powers edX, writing both front-end and back-end code. They tackle a wide range of technical challenges, and so the best candidates will have a strong background in one or more of the following areas: machine learning, education, user interaction design, big data, social network analysis, and devops. Specialists are encouraged to apply, but team members often wear many hats. Our ideal candidate would have excellent coding skills, a proven history of delivering projects, and a deep research background.

    -

    If you are interested in this position, please send an email to jobs@edx.org

    -
    -
    - -
    -
    -

    Content Engineer

    -

    Content Engineers develop sophisticated, domain-specific tools that enable professors to deliver the best possible educational experience in their classes. Examples include circuit schematic editors, scientific simulators of every kind, and peer collaboration tools. Content Engineers are dedicated to pushing the boundaries of what can be taught and assessed online, and will work closely with edX Fellows and course staff.

    -

    Strong JavaScript skills are required. A deep interest and background in pedagogy and education is highly desired. Knowledge of GWT, Backbone.js, and Python a plus.

    -

    If you are interested in this position, please send an email to jobs@edx.org.

    +

    If you are interested in this position, please send an email to jobs@edx.org

    -

    Positions

    How to Apply

    E-mail your resume, coverletter and any other materials to jobs@edx.org

    Our Location

    -

    11 Cambridge Center
    +

    11 Cambridge Center
    Cambridge, MA 02142

    - diff --git a/lms/templates/staticbook.html b/lms/templates/staticbook.html index eae70cdd84..2a0a6f03bb 100644 --- a/lms/templates/staticbook.html +++ b/lms/templates/staticbook.html @@ -4,6 +4,7 @@ <%block name="headextra"> <%static:css group='course'/> +<%static:js group='courseware'/> <%block name="js_extra"> @@ -71,7 +72,31 @@ $("#open_close_accordion a").click(function(){ @@ -89,17 +114,6 @@ $("#open_close_accordion a").click(function(){ - -
    diff --git a/lms/urls.py b/lms/urls.py index 9dc317039e..aaeba1b51e 100644 --- a/lms/urls.py +++ b/lms/urls.py @@ -1,8 +1,7 @@ from django.conf import settings -from django.conf.urls.defaults import patterns, include, url +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: @@ -15,7 +14,7 @@ urlpatterns = ('', url(r'^dashboard$', 'student.views.dashboard', name="dashboard"), url(r'^admin_dashboard$', 'dashboard.views.dashboard'), - + url(r'^change_email$', 'student.views.change_email_request'), url(r'^email_confirm/(?P[^/]*)$', 'student.views.confirm_email_change'), url(r'^change_name$', 'student.views.change_name_request'), @@ -85,7 +84,6 @@ urlpatterns = ('', (r'^pressrelease$', 'django.views.generic.simple.redirect_to', {'url': '/press/uc-berkeley-joins-edx'}), - (r'^favicon\.ico$', 'django.views.generic.simple.redirect_to', {'url': '/static/images/favicon.ico'}), # TODO: These urls no longer work. They need to be updated before they are re-enabled @@ -98,16 +96,21 @@ if settings.PERFSTATS: if settings.COURSEWARE_ENABLED: urlpatterns += ( + # Hook django-masquerade, allowing staff to view site as other users 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'^xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', 'courseware.module_render.xqueue_callback'), - url(r'^change_setting$', 'student.views.change_setting'), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/modx/(?P.*?)/(?P[^/]*)$', + 'courseware.module_render.modx_dispatch', + name='modx_dispatch'), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/xqueue/(?P[^/]*)/(?P.*?)/(?P[^/]*)$', + 'courseware.module_render.xqueue_callback', + name='xqueue_callback'), + url(r'^change_setting$', 'student.views.change_setting', + name='change_setting'), # TODO: These views need to be updated before they work # url(r'^calculate$', 'util.views.calculate'), - # url(r'^gradebook$', 'courseware.views.gradebook'), # TODO: We should probably remove the circuit package. I believe it was only used in the old way of saving wiki circuits for the wiki # url(r'^edit_circuit/(?P[^/]*)$', 'circuit.views.edit_circuit'), # url(r'^save_circuit/(?P[^/]*)$', 'circuit.views.save_circuit'), @@ -131,12 +134,24 @@ if settings.COURSEWARE_ENABLED: 'staticbook.views.index_shifted'), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/courseware/?$', 'courseware.views.index', name="courseware"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/courseware/(?P[^/]*)/$', + 'courseware.views.index', name="courseware_chapter"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/courseware/(?P[^/]*)/(?P
    [^/]*)/$', 'courseware.views.index', name="courseware_section"), url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/profile$', 'courseware.views.profile', name="profile"), + # Takes optional student_id for instructor use--shows profile as that student sees it. url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/profile/(?P[^/]*)/$', - 'courseware.views.profile'), + 'courseware.views.profile', name="student_profile"), + + # For the instructor + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/instructor$', + 'courseware.views.instructor_dashboard', name="instructor_dashboard"), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/gradebook$', + 'courseware.views.gradebook', name='gradebook'), + url(r'^courses/(?P[^/]+/[^/]+/[^/]+)/grade_summary$', + 'courseware.views.grade_summary', name='grade_summary'), + ) # Multicourse wiki 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; + } + } +} + + diff --git a/rakefile b/rakefile index 01491ce981..c62c87701e 100644 --- a/rakefile +++ b/rakefile @@ -83,13 +83,21 @@ end task :pylint => "pylint_#{system}" end +$failed_tests = 0 -def run_tests(system, report_dir) +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)) + 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 + $failed_tests += 1 unless ok + end end +TEST_TASKS = [] [:lms, :cms].each do |system| report_dir = File.join(REPORT_DIR, system.to_s) @@ -97,15 +105,16 @@ end # Per System tasks desc "Run all django tests on our djangoapps for the #{system}" - task "test_#{system}" => ["clean_test_files", "#{system}:collectstatic:test", "fasttest_#{system}"] + task "test_#{system}", [:stop_on_failure] => ["clean_test_files", "#{system}:collectstatic:test", "fasttest_#{system}"] # Have a way to run the tests without running collectstatic -- useful when debugging without # messing with static files. - task "fasttest_#{system}" => [report_dir, :predjango] do - run_tests(system, report_dir) + task "fasttest_#{system}", [:stop_on_failure] => [report_dir, :predjango] do |t, args| + args.with_defaults(:stop_on_failure => 'true') + run_tests(system, report_dir, args.stop_on_failure) end - task :test => "test_#{system}" + TEST_TASKS << "test_#{system}" desc <<-desc Start the #{system} locally with the specified environment (defaults to dev). @@ -142,7 +151,17 @@ Dir["common/lib/*"].each do |lib| ENV['NOSE_XUNIT_FILE'] = File.join(report_dir, "nosetests.xml") sh("nosetests #{lib} --cover-erase --with-xunit --with-xcoverage --cover-html --cover-inclusive --cover-package #{File.basename(lib)} --cover-html-dir #{File.join(report_dir, "cover")}") end - task :test => task_name + TEST_TASKS << task_name +end + +task :test do + TEST_TASKS.each do |task| + Rake::Task[task].invoke(false) + end + + if $failed_tests > 0 + abort "Tests failed!" + end end task :runserver => :lms diff --git a/requirements.txt b/requirements.txt index 33b2bfeb05..ef16d2c577 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,4 +1,4 @@ -django<1.4 +django>=1.4,<1.5 pip numpy scipy diff --git a/utility-scripts/create_groups.py b/utility-scripts/create_groups.py index 33c563127f..0e3245bb4d 100644 --- a/utility-scripts/create_groups.py +++ b/utility-scripts/create_groups.py @@ -18,6 +18,7 @@ except Exception as err: from django.conf import settings from django.contrib.auth.models import User, Group from path import path +from lxml import etree data_dir = settings.DATA_DIR print "data_dir = %s" % data_dir @@ -26,7 +27,17 @@ for course_dir in os.listdir(data_dir): # print course_dir if not os.path.isdir(path(data_dir) / course_dir): continue - gname = 'staff_%s' % course_dir + + cxfn = path(data_dir) / course_dir / 'course.xml' + coursexml = etree.parse(cxfn) + cxmlroot = coursexml.getroot() + course = cxmlroot.get('course') + if course is None: + print "oops, can't get course id for %s" % course_dir + continue + print "course=%s for course_dir=%s" % (course,course_dir) + + gname = 'staff_%s' % course if Group.objects.filter(name=gname): print "group exists for %s" % gname continue