Merge pull request #6678 from edx/mattdrayer/entexams-follow-up

Make entrance exam field non-editable

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
     )

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

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)
-
-        # 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)}
-        )
+    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')
+    )
+
+    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):

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(

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);
+            });
         });
     }
 );

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);
+
+            //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();
 
-            var entrance_exam_min_score = this.view.$('#entrance-exam-minimum-score-pct');
-            var entrance_exam_enabled_field = this.view.$('#entrance-exam-enabled');
+        });
+
+        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');

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,
     )