From c8898b0afc6c4f32f680e897e212afaf106c318b Mon Sep 17 00:00:00 2001 From: John Eskew Date: Thu, 2 Oct 2014 11:32:17 -0400 Subject: [PATCH 1/8] Use: CourseLocator.from_string() instead of: SlashSeparatedCourseKey.from_deprecated_string() to construct the course_key for which the auto-created user is registered. --- common/djangoapps/student/views.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 6adca7a26c..02d9ca587b 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -57,6 +57,7 @@ from dark_lang.models import DarkLangConfig from xmodule.modulestore.django import modulestore from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.locator import CourseLocator from xmodule.modulestore import ModuleStoreEnum from collections import namedtuple @@ -1674,7 +1675,7 @@ def auto_auth(request): course_id = request.GET.get('course_id', None) course_key = None if course_id: - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) + course_key = CourseLocator.from_string(course_id) role_names = [v.strip() for v in request.GET.get('roles', '').split(',') if v.strip()] # Get or create the user object From c4dc7376ad7af1e8f552085e96413be78c545393 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Fri, 3 Oct 2014 11:18:24 -0400 Subject: [PATCH 2/8] Add bulk_operations() wrapper around course progress view. --- lms/djangoapps/courseware/views.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index c216b0ceea..22d95de96c 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -805,8 +805,9 @@ def progress(request, course_id, student_id=None): course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - with grades.manual_transaction(): - return _progress(request, course_key, student_id) + with modulestore().bulk_operations(course_key): + with grades.manual_transaction(): + return _progress(request, course_key, student_id) def _progress(request, course_key, student_id): From 0c795934c31c1d4e99cea949c0e30acd39e32264 Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Fri, 3 Oct 2014 17:09:43 -0400 Subject: [PATCH 3/8] Don't fetch python libs if there's no python code in the capa problem. --- common/lib/capa/capa/capa_problem.py | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/common/lib/capa/capa/capa_problem.py b/common/lib/capa/capa/capa_problem.py index f3eb115ad6..a854e1109f 100644 --- a/common/lib/capa/capa/capa_problem.py +++ b/common/lib/capa/capa/capa_problem.py @@ -646,14 +646,14 @@ class LoncapaProblem(object): code = unescape(script.text, XMLESC) all_code += code - # An asset named python_lib.zip can be imported by Python code. extra_files = [] - zip_lib = self.capa_system.get_python_lib_zip() - if zip_lib is not None: - extra_files.append(("python_lib.zip", zip_lib)) - python_path.append("python_lib.zip") - if all_code: + # An asset named python_lib.zip can be imported by Python code. + zip_lib = self.capa_system.get_python_lib_zip() + if zip_lib is not None: + extra_files.append(("python_lib.zip", zip_lib)) + python_path.append("python_lib.zip") + try: safe_exec( all_code, From 0ed4ee32eb9383eb8ce4fdf5c36d23d2794d766c Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Fri, 3 Oct 2014 17:28:16 -0400 Subject: [PATCH 4/8] Use bulk operations on get_course_by_id and courseware --- lms/djangoapps/courseware/courses.py | 3 ++- lms/djangoapps/courseware/views.py | 5 +++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lms/djangoapps/courseware/courses.py b/lms/djangoapps/courseware/courses.py index 3b6d614f91..27ceb530f3 100644 --- a/lms/djangoapps/courseware/courses.py +++ b/lms/djangoapps/courseware/courses.py @@ -66,7 +66,8 @@ def get_course_by_id(course_key, depth=0): depth: The number of levels of children for the modulestore to cache. None means infinite depth """ - course = modulestore().get_course(course_key, depth=depth) + with modulestore().bulk_operations(course_key): + course = modulestore().get_course(course_key, depth=depth) if course: return course else: diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 22d95de96c..18013af6dc 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -285,6 +285,11 @@ def index(request, course_id, chapter=None, section=None, return redirect(reverse('dashboard')) request.user = user # keep just one instance of User + with modulestore().bulk_operations(course_key): + return _index_bulk_op(request, user, course_key, chapter, section, position) + + +def _index_bulk_op(request, user, course_key, chapter, section, position): course = get_course_with_access(user, 'load', course_key, depth=2) staff_access = has_access(user, 'staff', course) registered = registered_for_course(course, user) From 18822bdb4e3011a11b63a695a279a55f65eb490a Mon Sep 17 00:00:00 2001 From: John Eskew Date: Sun, 5 Oct 2014 10:42:23 -0400 Subject: [PATCH 5/8] Wrap course info and dashboard in bulk ops wrapper to reduce Mongo calls. --- common/djangoapps/student/views.py | 32 ++++++++-------- lms/djangoapps/courseware/views.py | 61 +++++++++++++++--------------- 2 files changed, 48 insertions(+), 45 deletions(-) diff --git a/common/djangoapps/student/views.py b/common/djangoapps/student/views.py index 02d9ca587b..d1417970f7 100644 --- a/common/djangoapps/student/views.py +++ b/common/djangoapps/student/views.py @@ -246,23 +246,25 @@ def get_course_enrollment_pairs(user, course_org_filter, org_filter_out_set): a student's dashboard. """ for enrollment in CourseEnrollment.enrollments_for_user(user): - course = modulestore().get_course(enrollment.course_id) - if course and not isinstance(course, ErrorDescriptor): + store = modulestore() + with store.bulk_operations(enrollment.course_id): + course = store.get_course(enrollment.course_id) + if course and not isinstance(course, ErrorDescriptor): - # if we are in a Microsite, then filter out anything that is not - # attributed (by ORG) to that Microsite - if course_org_filter and course_org_filter != course.location.org: - continue - # Conversely, if we are not in a Microsite, then let's filter out any enrollments - # with courses attributed (by ORG) to Microsites - elif course.location.org in org_filter_out_set: - continue + # if we are in a Microsite, then filter out anything that is not + # attributed (by ORG) to that Microsite + if course_org_filter and course_org_filter != course.location.org: + continue + # Conversely, if we are not in a Microsite, then let's filter out any enrollments + # with courses attributed (by ORG) to Microsites + elif course.location.org in org_filter_out_set: + continue - yield (course, enrollment) - else: - log.error("User {0} enrolled in {2} course {1}".format( - user.username, enrollment.course_id, "broken" if course else "non-existent" - )) + yield (course, enrollment) + else: + log.error("User {0} enrolled in {2} course {1}".format( + user.username, enrollment.course_id, "broken" if course else "non-existent" + )) def _cert_info(user, course, cert_status): diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 18013af6dc..f92326690a 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -559,41 +559,42 @@ def course_info(request, course_id): course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - course = get_course_with_access(request.user, 'load', course_key) - staff_access = has_access(request.user, 'staff', course) - masq = setup_masquerade(request, staff_access) # allow staff to toggle masquerade on info page - reverifications = fetch_reverify_banner_info(request, course_key) - studio_url = get_studio_url(course, 'course_info') + with modulestore().bulk_operations(course_key): + course = get_course_with_access(request.user, 'load', course_key) + staff_access = has_access(request.user, 'staff', course) + masq = setup_masquerade(request, staff_access) # allow staff to toggle masquerade on info page + reverifications = fetch_reverify_banner_info(request, course_key) + studio_url = get_studio_url(course, 'course_info') - # link to where the student should go to enroll in the course: - # about page if there is not marketing site, SITE_NAME if there is - url_to_enroll = reverse(course_about, args=[course_id]) - if settings.FEATURES.get('ENABLE_MKTG_SITE'): - url_to_enroll = marketing_link('COURSES') + # link to where the student should go to enroll in the course: + # about page if there is not marketing site, SITE_NAME if there is + url_to_enroll = reverse(course_about, args=[course_id]) + if settings.FEATURES.get('ENABLE_MKTG_SITE'): + url_to_enroll = marketing_link('COURSES') - show_enroll_banner = request.user.is_authenticated() and not CourseEnrollment.is_enrolled(request.user, course.id) + show_enroll_banner = request.user.is_authenticated() and not CourseEnrollment.is_enrolled(request.user, course.id) - context = { - 'request': request, - 'course_id': course_key.to_deprecated_string(), - 'cache': None, - 'course': course, - 'staff_access': staff_access, - 'masquerade': masq, - 'studio_url': studio_url, - 'reverifications': reverifications, - 'show_enroll_banner': show_enroll_banner, - 'url_to_enroll': url_to_enroll, - } + context = { + 'request': request, + 'course_id': course_key.to_deprecated_string(), + 'cache': None, + 'course': course, + 'staff_access': staff_access, + 'masquerade': masq, + 'studio_url': studio_url, + 'reverifications': reverifications, + 'show_enroll_banner': show_enroll_banner, + 'url_to_enroll': url_to_enroll, + } - now = datetime.now(UTC()) - effective_start = _adjust_start_date_for_beta_testers(request.user, course, course_key) - if staff_access and now < effective_start: - # Disable student view button if user is staff and - # course is not yet visible to students. - context['disable_student_access'] = True + now = datetime.now(UTC()) + effective_start = _adjust_start_date_for_beta_testers(request.user, course, course_key) + if staff_access and now < effective_start: + # Disable student view button if user is staff and + # course is not yet visible to students. + context['disable_student_access'] = True - return render_to_response('courseware/info.html', context) + return render_to_response('courseware/info.html', context) @ensure_csrf_cookie From c58e68813c7d941f89026c31e393e01b5572ff6c Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 6 Oct 2014 11:20:33 -0400 Subject: [PATCH 6/8] Cache modules in split bulk ops --- .../xmodule/combined_open_ended_module.py | 2 +- .../split_mongo/caching_descriptor_system.py | 19 ++++++--- .../xmodule/modulestore/split_mongo/split.py | 42 ++++++++++++++++++- .../combined_open_ended_modulev1.py | 1 - 4 files changed, 55 insertions(+), 9 deletions(-) diff --git a/common/lib/xmodule/xmodule/combined_open_ended_module.py b/common/lib/xmodule/xmodule/combined_open_ended_module.py index c8a3d8860c..3a184ffec0 100644 --- a/common/lib/xmodule/xmodule/combined_open_ended_module.py +++ b/common/lib/xmodule/xmodule/combined_open_ended_module.py @@ -240,7 +240,7 @@ class CombinedOpenEndedFields(object): help=_("The number of times the student can try to answer this problem."), default=1, scope=Scope.settings, - values={"min": 1 } + values={"min": 1} ) accept_file_upload = Boolean( display_name=_("Allow File Uploads"), diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py index 34455a068d..73159bf221 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/caching_descriptor_system.py @@ -76,6 +76,11 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): @contract(usage_key="BlockUsageLocator | BlockKey", course_entry_override="CourseEnvelope | None") def _load_item(self, usage_key, course_entry_override=None, **kwargs): + """ + Instantiate the xblock fetching it either from the cache or from the structure + + :param course_entry_override: the course_info with the course_key to use (defaults to cached) + """ # usage_key is either a UsageKey or just the block_key. if a usage_key, if isinstance(usage_key, BlockUsageLocator): @@ -90,21 +95,25 @@ class CachingDescriptorSystem(MakoDescriptorSystem, EditInfoRuntimeMixin): raise ItemNotFoundError else: block_key = BlockKey.from_usage_key(usage_key) + version_guid = self.course_entry.course_key.version_guid else: block_key = usage_key course_info = course_entry_override or self.course_entry course_key = course_info.course_key + version_guid = course_key.version_guid - if course_entry_override: - structure_id = course_entry_override.structure.get('_id') - else: - structure_id = self.course_entry.structure.get('_id') + # look in cache + cached_module = self.modulestore.get_cached_block(course_key, version_guid, block_key) + if cached_module: + return cached_module json_data = self.get_module_data(block_key, course_key) class_ = self.load_block_type(json_data.get('block_type')) - return self.xblock_from_json(class_, course_key, block_key, json_data, course_entry_override, **kwargs) + block = self.xblock_from_json(class_, course_key, block_key, json_data, course_entry_override, **kwargs) + self.modulestore.cache_block(course_key, version_guid, block_key, block) + return block @contract(block_key=BlockKey, course_key=CourseLocator) def get_module_data(self, block_key, course_key): diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 03ed1d5d37..912c3b6a0a 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -117,6 +117,8 @@ class SplitBulkWriteRecord(BulkOpsRecord): self.index = None self.structures = {} self.structures_in_db = set() + # dict(version_guid, dict(BlockKey, module)) + self.modules = defaultdict(dict) self.definitions = {} self.definitions_in_db = set() @@ -309,6 +311,38 @@ class SplitBulkWriteMixin(BulkOperationsMixin): else: self.db_connection.insert_structure(structure) + def get_cached_block(self, course_key, version_guid, block_id): + """ + If there's an active bulk_operation, see if it's cached this module and just return it + Don't do any extra work to get the ones which are not cached. Make the caller do the work & cache them. + """ + bulk_write_record = self._get_bulk_ops_record(course_key) + if bulk_write_record.active: + return bulk_write_record.modules[version_guid].get(block_id, None) + else: + return None + + def cache_block(self, course_key, version_guid, block_key, block): + """ + The counterpart to :method `get_cached_block` which caches a block. + Returns nothing. + """ + bulk_write_record = self._get_bulk_ops_record(course_key) + if bulk_write_record.active: + bulk_write_record.modules[version_guid][block_key] = block + + def decache_block(self, course_key, version_guid, block_key): + """ + Write operations which don't write from blocks must remove the target blocks from the cache. + Returns nothing. + """ + bulk_write_record = self._get_bulk_ops_record(course_key) + if bulk_write_record.active: + try: + del bulk_write_record.modules[version_guid][block_key] + except KeyError: + pass + def get_definition(self, course_key, definition_guid): """ Retrieve a single definition by id, respecting the active bulk operation @@ -637,8 +671,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): @contract(course_entry=CourseEnvelope, block_keys="list(BlockKey)", depth="int | None") def _load_items(self, course_entry, block_keys, depth=0, lazy=True, **kwargs): ''' - Load & cache the given blocks from the course. Prefetch down to the - given depth. Load the definitions into each block if lazy is False; + Load & cache the given blocks from the course. May return the blocks in any order. + + Load the definitions into each block if lazy is False; otherwise, use the lazy definition placeholder. ''' runtime = self._get_cache(course_entry.structure['_id']) @@ -646,6 +681,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): runtime = self.create_runtime(course_entry, lazy) self._add_cache(course_entry.structure['_id'], runtime) self.cache_items(runtime, block_keys, course_entry.course_key, depth, lazy) + return [runtime.load_item(block_key, course_entry, **kwargs) for block_key in block_keys] def _get_cache(self, course_version_guid): @@ -1364,6 +1400,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): # if the parent hadn't been previously changed in this bulk transaction, indicate that it's # part of the bulk transaction self.version_block(parent, user_id, new_structure['_id']) + self.decache_block(parent_usage_key.course_key, new_structure['_id'], block_id) # db update self.update_structure(parent_usage_key.course_key, new_structure) @@ -1957,6 +1994,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase): parent_block['edit_info']['edited_by'] = user_id parent_block['edit_info']['previous_version'] = parent_block['edit_info']['update_version'] parent_block['edit_info']['update_version'] = new_id + self.decache_block(usage_locator.course_key, new_id, parent_block_key) self._remove_subtree(BlockKey.from_usage_key(usage_locator), new_blocks) diff --git a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py index cfa9db908b..3fbda59f79 100644 --- a/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py +++ b/common/lib/xmodule/xmodule/open_ended_grading_classes/combined_open_ended_modulev1.py @@ -1229,7 +1229,6 @@ class CombinedOpenEndedV1Descriptor(): return {'task_xml': parse_task('task'), 'prompt': parse('prompt'), 'rubric': parse('rubric')} - def definition_to_xml(self, resource_fs): '''Return an xml element representing this definition.''' elt = etree.Element('combinedopenended') From 41d4b1286f5f5bae2e09891d609353f7425d5734 Mon Sep 17 00:00:00 2001 From: John Eskew Date: Mon, 6 Oct 2014 15:37:25 -0400 Subject: [PATCH 7/8] Add tests for Split course keys and CourseLocators --- .../student/tests/test_auto_auth.py | 47 ++++++++++++------- 1 file changed, 31 insertions(+), 16 deletions(-) diff --git a/common/djangoapps/student/tests/test_auto_auth.py b/common/djangoapps/student/tests/test_auto_auth.py index 6692b336fa..884729a84a 100644 --- a/common/djangoapps/student/tests/test_auto_auth.py +++ b/common/djangoapps/student/tests/test_auto_auth.py @@ -7,14 +7,25 @@ from django_comment_common.utils import seed_permissions_roles from student.models import CourseEnrollment, UserProfile from util.testing import UrlResetMixin from opaque_keys.edx.locations import SlashSeparatedCourseKey +from opaque_keys.edx.locator import CourseLocator from mock import patch +import ddt - +@ddt.ddt class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): """ Tests for the Auto auth view that we have for load testing. """ + COURSE_ID_MONGO = 'edX/Test101/2014_Spring' + COURSE_ID_SPLIT = 'course-v1:edX+Test101+2014_Spring' + COURSE_IDS_DDT = ( + (COURSE_ID_MONGO, SlashSeparatedCourseKey.from_deprecated_string(COURSE_ID_MONGO)), + (COURSE_ID_SPLIT, SlashSeparatedCourseKey.from_deprecated_string(COURSE_ID_SPLIT)), + (COURSE_ID_MONGO, CourseLocator.from_string(COURSE_ID_MONGO)), + (COURSE_ID_SPLIT, CourseLocator.from_string(COURSE_ID_SPLIT)), + ) + @patch.dict("django.conf.settings.FEATURES", {"AUTOMATIC_AUTH_FOR_TESTING": True}) def setUp(self): # Patching the settings.FEATURES['AUTOMATIC_AUTH_FOR_TESTING'] @@ -24,8 +35,6 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): super(AutoAuthEnabledTestCase, self).setUp() self.url = '/auto_auth' self.client = Client() - self.course_id = 'edX/Test101/2014_Spring' - self.course_key = SlashSeparatedCourseKey.from_deprecated_string(self.course_id) def test_create_user(self): """ @@ -83,42 +92,48 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): user = User.objects.get(username='test') self.assertFalse(user.is_staff) - def test_course_enrollment(self): + @ddt.data(*COURSE_IDS_DDT) + @ddt.unpack + def test_course_enrollment(self, course_id, course_key): # Create a user and enroll in a course - self._auto_auth(username='test', course_id=self.course_id) + self._auto_auth(username='test', course_id=course_id) # Check that a course enrollment was created for the user self.assertEqual(CourseEnrollment.objects.count(), 1) - enrollment = CourseEnrollment.objects.get(course_id=self.course_key) + enrollment = CourseEnrollment.objects.get(course_id=course_key) self.assertEqual(enrollment.user.username, "test") - def test_double_enrollment(self): + @ddt.data(*COURSE_IDS_DDT) + @ddt.unpack + def test_double_enrollment(self, course_id, course_key): # Create a user and enroll in a course - self._auto_auth(username='test', course_id=self.course_id) + self._auto_auth(username='test', course_id=course_id) # Make the same call again, re-enrolling the student in the same course - self._auto_auth(username='test', course_id=self.course_id) + self._auto_auth(username='test', course_id=course_id) # Check that only one course enrollment was created for the user self.assertEqual(CourseEnrollment.objects.count(), 1) - enrollment = CourseEnrollment.objects.get(course_id=self.course_key) + enrollment = CourseEnrollment.objects.get(course_id=course_key) self.assertEqual(enrollment.user.username, "test") - def test_set_roles(self): - seed_permissions_roles(self.course_key) - course_roles = dict((r.name, r) for r in Role.objects.filter(course_id=self.course_key)) + @ddt.data(*COURSE_IDS_DDT) + @ddt.unpack + def test_set_roles(self, course_id, course_key): + seed_permissions_roles(course_key) + course_roles = dict((r.name, r) for r in Role.objects.filter(course_id=course_key)) self.assertEqual(len(course_roles), 4) # sanity check # Student role is assigned by default on course enrollment. - self._auto_auth(username='a_student', course_id=self.course_id) + self._auto_auth(username='a_student', course_id=course_id) user = User.objects.get(username='a_student') user_roles = user.roles.all() self.assertEqual(len(user_roles), 1) self.assertEqual(user_roles[0], course_roles[FORUM_ROLE_STUDENT]) - self._auto_auth(username='a_moderator', course_id=self.course_id, roles='Moderator') + self._auto_auth(username='a_moderator', course_id=course_id, roles='Moderator') user = User.objects.get(username='a_moderator') user_roles = user.roles.all() self.assertEqual( @@ -127,7 +142,7 @@ class AutoAuthEnabledTestCase(UrlResetMixin, TestCase): course_roles[FORUM_ROLE_MODERATOR]])) # check multiple roles work. - self._auto_auth(username='an_admin', course_id=self.course_id, + self._auto_auth(username='an_admin', course_id=course_id, roles='{},{}'.format(FORUM_ROLE_MODERATOR, FORUM_ROLE_ADMINISTRATOR)) user = User.objects.get(username='an_admin') user_roles = user.roles.all() From eb33f0e0eb282e63dad4c5682e5804b7222bc4cc Mon Sep 17 00:00:00 2001 From: Don Mitchell Date: Mon, 6 Oct 2014 16:51:07 -0400 Subject: [PATCH 8/8] Cons up new dict to not hurt cached value --- cms/djangoapps/models/settings/course_grading.py | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/cms/djangoapps/models/settings/course_grading.py b/cms/djangoapps/models/settings/course_grading.py index 35a279e28a..f00a5863ec 100644 --- a/cms/djangoapps/models/settings/course_grading.py +++ b/cms/djangoapps/models/settings/course_grading.py @@ -205,10 +205,11 @@ class CourseGradingModel(object): @staticmethod def jsonize_grader(i, grader): - grader['id'] = i - if grader['weight']: - grader['weight'] *= 100 - if not 'short_label' in grader: - grader['short_label'] = "" - - return grader + return { + "id": i, + "type": grader["type"], + "min_count": grader.get('min_count', 0), + "drop_count": grader.get('drop_count', 0), + "short_label": grader.get('short_label', ""), + "weight": grader.get('weight', 0) * 100, + }