From 1c839cc09d2aa941c4868c3e28a6e38c8cbc4125 Mon Sep 17 00:00:00 2001 From: Matt Drayer Date: Fri, 16 Jan 2015 12:45:16 -0500 Subject: [PATCH] Addressed several outstanding issues related to initial entrance exams feature delivery --- .../contentstore/views/entrance_exam.py | 19 +-- cms/djangoapps/contentstore/views/helpers.py | 108 +++++++++++++++++ cms/djangoapps/contentstore/views/item.py | 110 +++--------------- .../contentstore/views/tests/test_item.py | 14 +++ cms/static/js/spec/models/xblock_info_spec.js | 10 ++ .../js/spec/views/settings/main_spec.js | 73 +++++++++--- common/lib/xmodule/xmodule/seq_module.py | 2 +- 7 files changed, 212 insertions(+), 124 deletions(-) diff --git a/cms/djangoapps/contentstore/views/entrance_exam.py b/cms/djangoapps/contentstore/views/entrance_exam.py index caa94a2cc4..7c8658a4fe 100644 --- a/cms/djangoapps/contentstore/views/entrance_exam.py +++ b/cms/djangoapps/contentstore/views/entrance_exam.py @@ -10,7 +10,8 @@ from django_future.csrf import ensure_csrf_cookie from django.http import HttpResponse from django.test import RequestFactory -from contentstore.views.item import create_item, delete_item +from contentstore.views.helpers import create_xblock +from contentstore.views.item import delete_item from milestones import api as milestones_api from models.settings.course_metadata import CourseMetadata from opaque_keys.edx.keys import CourseKey, UsageKey @@ -108,10 +109,14 @@ def _create_entrance_exam(request, course_key, entrance_exam_minimum_score_pct=N 'is_entrance_exam': True, 'in_entrance_exam': True, } - factory = RequestFactory() - internal_request = factory.post('/', json.dumps(payload), content_type="application/json") - internal_request.user = request.user - created_item = json.loads(create_item(internal_request).content) + parent_locator = unicode(course.location) + created_block = create_xblock( + parent_locator=parent_locator, + user=request.user, + category='chapter', + display_name='Entrance Exam', + is_entrance_exam=True + ) # Set the entrance exam metadata flags for this course # Reload the course so we don't overwrite the new child reference @@ -119,7 +124,7 @@ def _create_entrance_exam(request, course_key, entrance_exam_minimum_score_pct=N metadata = { 'entrance_exam_enabled': True, 'entrance_exam_minimum_score_pct': entrance_exam_minimum_score_pct / 100, - 'entrance_exam_id': created_item['locator'], + 'entrance_exam_id': unicode(created_block.location), } CourseMetadata.update_from_dict(metadata, course, request.user) @@ -146,7 +151,7 @@ def _create_entrance_exam(request, course_key, entrance_exam_minimum_score_pct=N ) milestones_api.add_course_content_milestone( unicode(course.id), - created_item['locator'], + unicode(created_block.location), relationship_types['FULFILLS'], milestone ) diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 3769c81978..b5c79f59ae 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -4,16 +4,22 @@ Helper methods for Studio views. from __future__ import absolute_import +from uuid import uuid4 import urllib from django.conf import settings from django.http import HttpResponse from django.shortcuts import redirect from django.utils.translation import ugettext as _ + from edxmako.shortcuts import render_to_string, render_to_response +from opaque_keys.edx.keys import UsageKey from xblock.core import XBlock from xmodule.modulestore.django import modulestore +from xmodule.tabs import StaticTab + from contentstore.utils import reverse_course_url, reverse_library_url, reverse_usage_url +from models.settings.course_grading import CourseGradingModel __all__ = ['edge', 'event', 'landing'] @@ -154,3 +160,105 @@ def xblock_primary_child_category(xblock): elif category == 'sequential': return 'vertical' return None + + +def usage_key_with_run(usage_key_string): + """ + Converts usage_key_string to a UsageKey, adding a course run if necessary + """ + usage_key = UsageKey.from_string(usage_key_string) + usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key)) + return usage_key + + +def create_xblock(parent_locator, user, category, display_name, boilerplate=None, is_entrance_exam=False): + """ + Performs the actual grunt work of creating items/xblocks -- knows nothing about requests, views, etc. + """ + store = modulestore() + usage_key = usage_key_with_run(parent_locator) + with store.bulk_operations(usage_key.course_key): + parent = store.get_item(usage_key) + dest_usage_key = usage_key.replace(category=category, name=uuid4().hex) + + # get the metadata, display_name, and definition from the caller + metadata = {} + data = None + template_id = boilerplate + if template_id: + clz = parent.runtime.load_block_type(category) + if clz is not None: + template = clz.get_template(template_id) + if template is not None: + metadata = template.get('metadata', {}) + data = template.get('data') + + if display_name is not None: + metadata['display_name'] = display_name + + # We should use the 'fields' kwarg for newer module settings/values (vs. metadata or data) + fields = {} + + # Entrance Exams: Chapter module positioning + child_position = None + if settings.FEATURES.get('ENTRANCE_EXAMS', False): + if category == 'chapter' and is_entrance_exam: + fields['is_entrance_exam'] = is_entrance_exam + fields['in_entrance_exam'] = True # Inherited metadata, all children will have it + child_position = 0 + + # TODO need to fix components that are sending definition_data as strings, instead of as dicts + # For now, migrate them into dicts here. + if isinstance(data, basestring): + data = {'data': data} + + created_block = store.create_child( + user.id, + usage_key, + dest_usage_key.block_type, + block_id=dest_usage_key.block_id, + fields=fields, + definition_data=data, + metadata=metadata, + runtime=parent.runtime, + position=child_position, + ) + + # Entrance Exams: Grader assignment + if settings.FEATURES.get('ENTRANCE_EXAMS', False): + course = store.get_course(usage_key.course_key) + if hasattr(course, 'entrance_exam_enabled') and course.entrance_exam_enabled: + if category == 'sequential' and parent_locator == course.entrance_exam_id: + grader = { + "type": "Entrance Exam", + "min_count": 0, + "drop_count": 0, + "short_label": "Entrance", + "weight": 0 + } + grading_model = CourseGradingModel.update_grader_from_json( + course.id, + grader, + user + ) + CourseGradingModel.update_section_grader_type( + created_block, + grading_model['type'], + user + ) + + # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so + # if we add one then we need to also add it to the policy information (i.e. metadata) + # we should remove this once we can break this reference from the course to static tabs + if category == 'static_tab': + display_name = display_name or _("Empty") # Prevent name being None + course = store.get_course(dest_usage_key.course_key) + course.tabs.append( + StaticTab( + name=display_name, + url_slug=dest_usage_key.name, + ) + ) + store.update_item(course, user.id) + + return created_block diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 8610559f1f..86cb005f17 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -42,7 +42,7 @@ from student.auth import has_studio_write_access, has_studio_read_access from contentstore.utils import find_release_date_source, find_staff_lock_source, is_currently_visible_to_students, \ ancestor_has_staff_lock, has_children_visible_to_specific_content_groups from contentstore.views.helpers import is_unit, xblock_studio_url, xblock_primary_child_category, \ - xblock_type_display_name, get_parent_xblock + xblock_type_display_name, get_parent_xblock, create_xblock, usage_key_with_run from contentstore.views.preview import get_preview_fragment from edxmako.shortcuts import render_to_string from models.settings.course_grading import CourseGradingModel @@ -79,15 +79,6 @@ def hash_resource(resource): return md5.hexdigest() -def usage_key_with_run(usage_key_string): - """ - Converts usage_key_string to a UsageKey, adding a course run if necessary - """ - usage_key = UsageKey.from_string(usage_key_string) - usage_key = usage_key.replace(course_key=modulestore().fill_in_run(usage_key.course_key)) - return usage_key - - def _filter_entrance_exam_grader(graders): """ If the entrance exams feature is enabled we need to hide away the grader from @@ -536,13 +527,12 @@ def create_item(request): @expect_json def _create_item(request): """View for create items.""" - usage_key = usage_key_with_run(request.json['parent_locator']) + parent_locator = request.json['parent_locator'] + usage_key = usage_key_with_run(parent_locator) if not has_studio_write_access(request.user, usage_key.course_key): raise PermissionDenied() category = request.json['category'] - display_name = request.json.get('display_name') - if isinstance(usage_key, LibraryUsageLocator): # Only these categories are supported at this time. if category not in ['html', 'problem', 'video']: @@ -550,91 +540,17 @@ def _create_item(request): "Category '%s' not supported for Libraries" % category, content_type='text/plain' ) - store = modulestore() - with store.bulk_operations(usage_key.course_key): - parent = store.get_item(usage_key) - dest_usage_key = usage_key.replace(category=category, name=uuid4().hex) + created_block = create_xblock( + parent_locator=parent_locator, + user=request.user, + category=category, + display_name=request.json.get('display_name'), + boilerplate=request.json.get('boilerplate') + ) - # get the metadata, display_name, and definition from the request - metadata = {} - data = None - template_id = request.json.get('boilerplate') - if template_id: - clz = parent.runtime.load_block_type(category) - if clz is not None: - template = clz.get_template(template_id) - if template is not None: - metadata = template.get('metadata', {}) - data = template.get('data') - - if display_name is not None: - metadata['display_name'] = display_name - - # Entrance Exams: Chapter module positioning - child_position = None - if settings.FEATURES.get('ENTRANCE_EXAMS', False): - is_entrance_exam = request.json.get('is_entrance_exam', False) - if category == 'chapter' and is_entrance_exam: - metadata['is_entrance_exam'] = is_entrance_exam - metadata['in_entrance_exam'] = True # Inherited metadata, all children will have it - child_position = 0 - - # TODO need to fix components that are sending definition_data as strings, instead of as dicts - # For now, migrate them into dicts here. - if isinstance(data, basestring): - data = {'data': data} - - created_block = store.create_child( - request.user.id, - usage_key, - dest_usage_key.block_type, - block_id=dest_usage_key.block_id, - definition_data=data, - metadata=metadata, - runtime=parent.runtime, - position=child_position - ) - - # Entrance Exams: Grader assignment - if settings.FEATURES.get('ENTRANCE_EXAMS', False): - course = store.get_course(usage_key.course_key) - if hasattr(course, 'entrance_exam_enabled') and course.entrance_exam_enabled: - if category == 'sequential' and request.json.get('parent_locator') == course.entrance_exam_id: - grader = { - "type": "Entrance Exam", - "min_count": 0, - "drop_count": 0, - "short_label": "Entrance", - "weight": 0 - } - grading_model = CourseGradingModel.update_grader_from_json( - course.id, - grader, - request.user - ) - CourseGradingModel.update_section_grader_type( - created_block, - grading_model['type'], - request.user - ) - - # VS[compat] cdodge: This is a hack because static_tabs also have references from the course module, so - # if we add one then we need to also add it to the policy information (i.e. metadata) - # we should remove this once we can break this reference from the course to static tabs - if category == 'static_tab': - display_name = display_name or _("Empty") # Prevent name being None - course = store.get_course(dest_usage_key.course_key) - course.tabs.append( - StaticTab( - name=display_name, - url_slug=dest_usage_key.name, - ) - ) - store.update_item(course, request.user.id) - - return JsonResponse( - {"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)} - ) + return JsonResponse( + {"locator": unicode(created_block.location), "courseKey": unicode(created_block.location.course_key)} + ) def _duplicate_item(parent_usage_key, duplicate_source_usage_key, user, display_name=None): diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index dd8a609968..2ca21b71f4 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -1378,6 +1378,20 @@ class TestXBlockInfo(ItemTest): json_response = json.loads(resp.content) self.validate_course_xblock_info(json_response, course_outline=True) + def test_chapter_entrance_exam_xblock_info(self): + chapter = ItemFactory.create( + parent_location=self.course.location, category='chapter', display_name="Entrance Exam", + user_id=self.user.id, is_entrance_exam=True + ) + chapter = modulestore().get_item(chapter.location) + xblock_info = create_xblock_info( + chapter, + include_child_info=True, + include_children_predicate=ALWAYS, + ) + self.assertEqual(xblock_info['override_type'], {'is_entrance_exam': True}) + self.assertEqual(xblock_info['display_name'], 'Entrance Exam') + def test_chapter_xblock_info(self): chapter = modulestore().get_item(self.chapter.location) xblock_info = create_xblock_info( diff --git a/cms/static/js/spec/models/xblock_info_spec.js b/cms/static/js/spec/models/xblock_info_spec.js index 5074407478..c55c8baa7e 100644 --- a/cms/static/js/spec/models/xblock_info_spec.js +++ b/cms/static/js/spec/models/xblock_info_spec.js @@ -7,6 +7,16 @@ define(['backbone', 'js/models/xblock_info'], expect(new XBlockInfo({'category': 'sequential'}).isEditableOnCourseOutline()).toBe(true); expect(new XBlockInfo({'category': 'vertical'}).isEditableOnCourseOutline()).toBe(true); }); + + it('cannot delete an entrance exam', function(){ + expect(new XBlockInfo({'category': 'chapter', 'override_type': {'is_entrance_exam':true}}) + .canBeDeleted()).toBe(false); + }); + + it('can delete module rather then entrance exam', function(){ + expect(new XBlockInfo({'category': 'chapter', 'override_type': {'is_entrance_exam':false}}).canBeDeleted()).toBe(true); + expect(new XBlockInfo({'category': 'chapter', 'override_type': {}}).canBeDeleted()).toBe(true); + }); }); } ); diff --git a/cms/static/js/spec/views/settings/main_spec.js b/cms/static/js/spec/views/settings/main_spec.js index 81c10613fe..e04be25c2a 100644 --- a/cms/static/js/spec/views/settings/main_spec.js +++ b/cms/static/js/spec/views/settings/main_spec.js @@ -3,6 +3,13 @@ define([ 'js/common_helpers/ajax_helpers' ], function($, CourseDetailsModel, MainView, AjaxHelpers) { 'use strict'; + + var SELECTORS = { + entrance_exam_min_score: '#entrance-exam-minimum-score-pct', + entrance_exam_enabled_field: '#entrance-exam-enabled', + grade_requirement_div: '.div-grade-requirements div' + }; + describe('Settings/Main', function () { var urlRoot = '/course/settings/org/DemoX/Demo_Course', modelData = { @@ -79,12 +86,52 @@ define([ AjaxHelpers.respondWithJson(requests, expectedJson); }); - it('should save entrance exam course details information correctly', function () { - var entrance_exam_minimum_score_pct = '60'; - var entrance_exam_enabled = 'true'; + it('should disallow save with an invalid minimum score percentage', function(){ + var entrance_exam_enabled_field = this.view.$(SELECTORS.entrance_exam_enabled_field), + entrance_exam_min_score = this.view.$(SELECTORS.entrance_exam_min_score); - var entrance_exam_min_score = this.view.$('#entrance-exam-minimum-score-pct'); - var entrance_exam_enabled_field = this.view.$('#entrance-exam-enabled'); + //input some invalid values. + expect(entrance_exam_min_score.val('101').trigger('input')).toHaveClass("error"); + expect(entrance_exam_min_score.val('invalidVal').trigger('input')).toHaveClass("error"); + + }); + + it('should provide a default value for the minimum score percentage', function(){ + + var entrance_exam_min_score = this.view.$(SELECTORS.entrance_exam_min_score); + + //if input an empty value, model should be populated with the default value. + entrance_exam_min_score.val('').trigger('input'); + expect(this.model.get('entrance_exam_minimum_score_pct')) + .toEqual(this.model.defaults.entrance_exam_minimum_score_pct); + }); + + it('show and hide the grade requirement section when the check box is selected and deselected respectively', function(){ + + var entrance_exam_enabled_field = this.view.$(SELECTORS.entrance_exam_enabled_field); + + // select the entrance-exam-enabled checkbox. grade requirement section should be visible. + entrance_exam_enabled_field + .attr('checked', 'true') + .trigger('change'); + + this.view.render(); + expect(this.view.$(SELECTORS.grade_requirement_div)).toBeVisible(); + + // deselect the entrance-exam-enabled checkbox. grade requirement section should be hidden. + entrance_exam_enabled_field + .removeAttr('checked') + .trigger('change'); + + expect(this.view.$(SELECTORS.grade_requirement_div)).toBeHidden(); + + }); + + it('should save entrance exam course details information correctly', function () { + var entrance_exam_minimum_score_pct = '60', + entrance_exam_enabled = 'true', + entrance_exam_min_score = this.view.$(SELECTORS.entrance_exam_min_score), + entrance_exam_enabled_field = this.view.$(SELECTORS.entrance_exam_enabled_field); var requests = AjaxHelpers.requests(this), expectedJson = $.extend(true, {}, modelData, { @@ -92,22 +139,10 @@ define([ entrance_exam_minimum_score_pct: entrance_exam_minimum_score_pct }); - expect(this.view.$('.div-grade-requirements div')).toBeHidden(); - - // select the entrance-exam-enabled checkbox. grade requirement section should be visible + // select the entrance-exam-enabled checkbox. entrance_exam_enabled_field - .attr('checked', entrance_exam_enabled) + .attr('checked', 'true') .trigger('change'); - expect(this.view.$('.div-grade-requirements div')).toBeVisible(); - - //input some invalid values. - expect(entrance_exam_min_score.val('101').trigger('input')).toHaveClass("error"); - expect(entrance_exam_min_score.val('invalidVal').trigger('input')).toHaveClass("error"); - - //if input an empty value, model should be populated with the default value. - entrance_exam_min_score.val('').trigger('input'); - expect(this.model.get('entrance_exam_minimum_score_pct')) - .toEqual(this.model.defaults.entrance_exam_minimum_score_pct); // input a valid value for entrance exam minimum score. entrance_exam_min_score.val(entrance_exam_minimum_score_pct).trigger('input'); diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index eab9168c1a..1f1b383dc9 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -52,7 +52,7 @@ class SequenceFields(object): "Tag this course module as an Entrance Exam. " + "Note, you must enable Entrance Exams for this course setting to take effect." ), - scope=Scope.settings, + scope=Scope.content, )