From 81502176e1c76fab8d2ac5e51583a9a929e690b7 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Fri, 17 Jul 2015 12:06:18 -0400 Subject: [PATCH 01/10] Integrate timed and proctored exam authoring into Studio --- cms/djangoapps/contentstore/proctoring.py | 116 ++++++++++ cms/djangoapps/contentstore/signals.py | 30 ++- .../contentstore/tests/test_proctoring.py | 206 ++++++++++++++++++ cms/djangoapps/contentstore/views/helpers.py | 14 ++ cms/djangoapps/contentstore/views/item.py | 16 +- .../views/tests/test_credit_eligibility.py | 4 +- .../contentstore/views/tests/test_item.py | 32 +++ .../models/settings/course_metadata.py | 4 + cms/envs/aws.py | 6 + cms/envs/common.py | 15 ++ .../spec/views/pages/course_outline_spec.js | 47 +++- cms/static/js/views/course_outline.js | 8 + .../js/views/modals/course_outline_modals.js | 101 ++++++++- cms/static/sass/elements/_modal-window.scss | 46 +++- cms/static/sass/elements/_modules.scss | 12 +- cms/templates/course_outline.html | 2 +- cms/templates/js/course-outline.underscore | 13 +- ...d-examination-preference-editor.underscore | 39 ++++ common/lib/xmodule/xmodule/course_module.py | 9 + common/lib/xmodule/xmodule/seq_module.py | 49 ++++- .../pages/studio/settings_advanced.py | 1 + lms/envs/common.py | 3 + openedx/core/djangoapps/credit/signals.py | 11 +- openedx/core/djangoapps/credit/tasks.py | 49 ++++- .../djangoapps/credit/tests/test_tasks.py | 99 ++++++++- requirements/edx/github.txt | 3 + 26 files changed, 896 insertions(+), 39 deletions(-) create mode 100644 cms/djangoapps/contentstore/proctoring.py create mode 100644 cms/djangoapps/contentstore/tests/test_proctoring.py create mode 100644 cms/templates/js/timed-examination-preference-editor.underscore diff --git a/cms/djangoapps/contentstore/proctoring.py b/cms/djangoapps/contentstore/proctoring.py new file mode 100644 index 0000000000..06be401a28 --- /dev/null +++ b/cms/djangoapps/contentstore/proctoring.py @@ -0,0 +1,116 @@ +""" +Code related to the handling of Proctored Exams in Studio +""" + +import logging + +from django.conf import settings + +from xmodule.modulestore.django import modulestore + +from contentstore.views.helpers import is_item_in_course_tree + +from edx_proctoring.api import ( + get_exam_by_content_id, + update_exam, + create_exam, + get_all_exams_for_course, +) +from edx_proctoring.exceptions import ( + ProctoredExamNotFoundException +) + +log = logging.getLogger(__name__) + + +def register_proctored_exams(course_key): + """ + This is typically called on a course published signal. The course is examined for sequences + that are marked as timed exams. Then these are registered with the edx-proctoring + subsystem. Likewise, if formerly registered exams are unmarked, then those + registred exams are marked as inactive + """ + + if not settings.FEATURES.get('ENABLE_PROCTORED_EXAMS'): + # if feature is not enabled then do a quick exit + return + + course = modulestore().get_course(course_key) + if not course.enable_proctored_exams: + # likewise if course does not have this feature turned on + return + + # get all sequences, since they can be marked as timed/proctored exams + _timed_exams = modulestore().get_items( + course_key, + qualifiers={ + 'category': 'sequential', + }, + settings={ + 'is_time_limited': True, + } + ) + + # filter out any potential dangling sequences + timed_exams = [ + timed_exam + for timed_exam in _timed_exams + if is_item_in_course_tree(timed_exam) + ] + + # enumerate over list of sequences which are time-limited and + # add/update any exam entries in edx-proctoring + for timed_exam in timed_exams: + msg = ( + 'Found {location} as a timed-exam in course structure. Inspecting...'.format( + location=unicode(timed_exam.location) + ) + ) + log.info(msg) + + try: + exam = get_exam_by_content_id(unicode(course_key), unicode(timed_exam.location)) + # update case, make sure everything is synced + update_exam( + exam_id=exam['id'], + exam_name=timed_exam.display_name, + time_limit_mins=timed_exam.default_time_limit_minutes, + is_proctored=timed_exam.is_proctored_enabled, + is_active=True + ) + msg = 'Updated timed exam {exam_id}'.format(exam_id=exam['id']) + log.info(msg) + except ProctoredExamNotFoundException: + exam_id = create_exam( + course_id=unicode(course_key), + content_id=unicode(timed_exam.location), + exam_name=timed_exam.display_name, + time_limit_mins=timed_exam.default_time_limit_minutes, + is_proctored=timed_exam.is_proctored_enabled, + is_active=True + ) + msg = 'Created new timed exam {exam_id}'.format(exam_id=exam_id) + log.info(msg) + + # then see which exams we have in edx-proctoring that are not in + # our current list. That means the the user has disabled it + exams = get_all_exams_for_course(course_key) + + for exam in exams: + if exam['is_active']: + # try to look up the content_id in the sequences location + + search = [ + timed_exam for timed_exam in timed_exams if + unicode(timed_exam.location) == exam['content_id'] + ] + if not search: + # This means it was turned off in Studio, we need to mark + # the exam as inactive (we don't delete!) + msg = 'Disabling timed exam {exam_id}'.format(exam_id=exam['id']) + log.info(msg) + update_exam( + exam_id=exam['id'], + is_proctored=False, + is_active=False, + ) diff --git a/cms/djangoapps/contentstore/signals.py b/cms/djangoapps/contentstore/signals.py index 8ada95bce1..127de92790 100644 --- a/cms/djangoapps/contentstore/signals.py +++ b/cms/djangoapps/contentstore/signals.py @@ -1,4 +1,5 @@ """ receivers of course_published and library_updated events in order to trigger indexing task """ + from datetime import datetime from pytz import UTC @@ -6,16 +7,33 @@ from django.dispatch import receiver from xmodule.modulestore.django import SignalHandler from contentstore.courseware_index import CoursewareSearchIndexer, LibrarySearchIndexer +from contentstore.proctoring import register_proctored_exams +from openedx.core.djangoapps.credit.signals import on_course_publish @receiver(SignalHandler.course_published) def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument """ - Receives signal and kicks off celery task to update search index + Receives publishing signal and performs publishing related workflows, such as + registering proctored exams, building up credit requirements, and performing + search indexing """ - # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded - from .tasks import update_search_index + + # first is to registered exams, the credit subsystem will assume that + # all proctored exams have already been registered, so we have to do that first + register_proctored_exams(course_key) + + # then call into the credit subsystem (in /openedx/djangoapps/credit) + # to perform any 'on_publish' workflow + on_course_publish(course_key) + + # Finally call into the course search subsystem + # to kick off an indexing action + if CoursewareSearchIndexer.indexing_is_enabled(): + # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded + from .tasks import update_search_index + update_search_index.delay(unicode(course_key), datetime.now(UTC).isoformat()) @@ -24,7 +42,9 @@ def listen_for_library_update(sender, library_key, **kwargs): # pylint: disable """ Receives signal and kicks off celery task to update search index """ - # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded - from .tasks import update_library_index + if LibrarySearchIndexer.indexing_is_enabled(): + # import here, because signal is registered at startup, but items in tasks are not yet able to be loaded + from .tasks import update_library_index + update_library_index.delay(unicode(library_key), datetime.now(UTC).isoformat()) diff --git a/cms/djangoapps/contentstore/tests/test_proctoring.py b/cms/djangoapps/contentstore/tests/test_proctoring.py new file mode 100644 index 0000000000..18fb2e53e3 --- /dev/null +++ b/cms/djangoapps/contentstore/tests/test_proctoring.py @@ -0,0 +1,206 @@ +""" +Tests for the edx_proctoring integration into Studio +""" + +from mock import patch +import ddt +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + +from contentstore.signals import listen_for_course_publish + +from edx_proctoring.api import get_all_exams_for_course + + +@ddt.ddt +@patch.dict('django.conf.settings.FEATURES', {'ENABLE_PROCTORED_EXAMS': True}) +class TestProctoredExams(ModuleStoreTestCase): + """ + Tests for the publishing of proctored exams + """ + + def setUp(self): + """ + Initial data setup + """ + super(TestProctoredExams, self).setUp() + + self.course = CourseFactory.create( + org='edX', + course='900', + run='test_run', + enable_proctored_exams=True + ) + + def _verify_exam_data(self, sequence, expected_active): + """ + Helper method to compare the sequence with the stored exam, + which should just be a single one + """ + exams = get_all_exams_for_course(unicode(self.course.id)) + + self.assertEqual(len(exams), 1) + + exam = exams[0] + self.assertEqual(exam['course_id'], unicode(self.course.id)) + self.assertEqual(exam['content_id'], unicode(sequence.location)) + self.assertEqual(exam['exam_name'], sequence.display_name) + self.assertEqual(exam['time_limit_mins'], sequence.default_time_limit_minutes) + self.assertEqual(exam['is_proctored'], sequence.is_proctored_enabled) + self.assertEqual(exam['is_active'], expected_active) + + @ddt.data( + (True, 10, True, True, False), + (True, 10, False, True, False), + (True, 10, True, True, True), + ) + @ddt.unpack + def test_publishing_exam(self, is_time_limited, default_time_limit_minutes, + is_procted_enabled, expected_active, republish): + """ + Happy path testing to see that when a course is published which contains + a proctored exam, it will also put an entry into the exam tables + """ + + chapter = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') + sequence = ItemFactory.create( + parent=chapter, + category='sequential', + display_name='Test Proctored Exam', + graded=True, + is_time_limited=is_time_limited, + default_time_limit_minutes=default_time_limit_minutes, + is_proctored_enabled=is_procted_enabled + ) + + listen_for_course_publish(self, self.course.id) + + self._verify_exam_data(sequence, expected_active) + + if republish: + # update the sequence + sequence.default_time_limit_minutes += sequence.default_time_limit_minutes + self.store.update_item(sequence, self.user.id) + + # simulate a publish + listen_for_course_publish(self, self.course.id) + + # reverify + self._verify_exam_data(sequence, expected_active) + + def test_unpublishing_proctored_exam(self): + """ + Make sure that if we publish and then unpublish a proctored exam, + the exam record stays, but is marked as is_active=False + """ + + chapter = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') + sequence = ItemFactory.create( + parent=chapter, + category='sequential', + display_name='Test Proctored Exam', + graded=True, + is_time_limited=True, + default_time_limit_minutes=10, + is_proctored_enabled=True + ) + + listen_for_course_publish(self, self.course.id) + + exams = get_all_exams_for_course(unicode(self.course.id)) + self.assertEqual(len(exams), 1) + + sequence.is_time_limited = False + sequence.is_proctored_enabled = False + + self.store.update_item(sequence, self.user.id) + + listen_for_course_publish(self, self.course.id) + + self._verify_exam_data(sequence, False) + + def test_dangling_exam(self): + """ + Make sure we filter out all dangling items + """ + + chapter = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') + ItemFactory.create( + parent=chapter, + category='sequential', + display_name='Test Proctored Exam', + graded=True, + is_time_limited=True, + default_time_limit_minutes=10, + is_proctored_enabled=True + ) + + listen_for_course_publish(self, self.course.id) + + exams = get_all_exams_for_course(unicode(self.course.id)) + self.assertEqual(len(exams), 1) + + self.store.delete_item(chapter.location, self.user.id) + + # republish course + listen_for_course_publish(self, self.course.id) + + # look through exam table, the dangling exam + # should be disabled + exams = get_all_exams_for_course(unicode(self.course.id)) + self.assertEqual(len(exams), 1) + + exam = exams[0] + self.assertEqual(exam['is_active'], False) + + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PROCTORED_EXAMS': False}) + def test_feature_flag_off(self): + """ + Make sure the feature flag is honored + """ + + chapter = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') + ItemFactory.create( + parent=chapter, + category='sequential', + display_name='Test Proctored Exam', + graded=True, + is_time_limited=True, + default_time_limit_minutes=10, + is_proctored_enabled=True + ) + + listen_for_course_publish(self, self.course.id) + + exams = get_all_exams_for_course(unicode(self.course.id)) + self.assertEqual(len(exams), 0) + + def test_advanced_setting_off(self): + """ + Make sure the feature flag is honored + """ + + self.course = CourseFactory.create( + org='edX', + course='901', + run='test_run2', + enable_proctored_exams=False + ) + + chapter = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') + ItemFactory.create( + parent=chapter, + category='sequential', + display_name='Test Proctored Exam', + graded=True, + is_time_limited=True, + default_time_limit_minutes=10, + is_proctored_enabled=True + ) + + listen_for_course_publish(self, self.course.id) + + # there shouldn't be any exams because we haven't enabled that + # advanced setting flag + exams = get_all_exams_for_course(unicode(self.course.id)) + self.assertEqual(len(exams), 0) diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 6d5cdf9ba3..59dc39cc30 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -299,3 +299,17 @@ def create_xblock(parent_locator, user, category, display_name, boilerplate=None store.update_item(course, user.id) return created_block + + +def is_item_in_course_tree(item): + """ + Check that the item is in the course tree. + + It's possible that the item is not in the course tree + if its parent has been deleted and is now an orphan. + """ + ancestor = item.get_parent() + while ancestor is not None and ancestor.location.category != "course": + ancestor = ancestor.get_parent() + + return ancestor is not None diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 414d61908b..7a8f0071a5 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -810,6 +810,7 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F # defining the default value 'True' for delete, drag and add new child actions in xblock_actions for each xblock. xblock_actions = {'deletable': True, 'draggable': True, 'childAddable': True} explanatory_message = None + # is_entrance_exam is inherited metadata. if xblock.category == 'chapter' and getattr(xblock, "is_entrance_exam", None): # Entrance exam section should not be deletable, draggable and not have 'New Subsection' button. @@ -846,9 +847,22 @@ def create_xblock_info(xblock, data=None, metadata=None, include_ancestor_info=F "course_graders": json.dumps([grader.get('type') for grader in graders]), "has_changes": has_changes, "actions": xblock_actions, - "explanatory_message": explanatory_message + "explanatory_message": explanatory_message, } + # update xblock_info with proctored_exam information if the feature flag is enabled + if settings.FEATURES.get('ENABLE_PROCTORED_EXAMS'): + if xblock.category == 'course': + xblock_info.update({ + "enable_proctored_exams": xblock.enable_proctored_exams + }) + elif xblock.category == 'sequential': + xblock_info.update({ + "is_proctored_enabled": xblock.is_proctored_enabled, + "is_time_limited": xblock.is_time_limited, + "default_time_limit_minutes": xblock.default_time_limit_minutes + }) + # Entrance exam subsection should be hidden. in_entrance_exam is inherited metadata, all children will have it. if xblock.category == 'sequential' and getattr(xblock, "in_entrance_exam", False): xblock_info["is_header_visible"] = False diff --git a/cms/djangoapps/contentstore/views/tests/test_credit_eligibility.py b/cms/djangoapps/contentstore/views/tests/test_credit_eligibility.py index 6ce0cfce51..a93e7d8641 100644 --- a/cms/djangoapps/contentstore/views/tests/test_credit_eligibility.py +++ b/cms/djangoapps/contentstore/views/tests/test_credit_eligibility.py @@ -10,7 +10,7 @@ from xmodule.modulestore.tests.factories import CourseFactory from openedx.core.djangoapps.credit.api import get_credit_requirements from openedx.core.djangoapps.credit.models import CreditCourse -from openedx.core.djangoapps.credit.signals import listen_for_course_publish +from openedx.core.djangoapps.credit.signals import on_course_publish class CreditEligibilityTest(CourseTestCase): @@ -50,7 +50,7 @@ class CreditEligibilityTest(CourseTestCase): credit_course.save() self.assertEqual(len(get_credit_requirements(self.course.id)), 0) # test that after publishing course, minimum grade requirement is added - listen_for_course_publish(self, self.course.id) + on_course_publish(self.course.id) self.assertEqual(len(get_credit_requirements(self.course.id)), 1) response = self.client.get_html(self.course_details_url) diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 4261042711..d233847a3a 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -1664,6 +1664,38 @@ class TestXBlockInfo(ItemTest): else: self.assertIsNone(xblock_info.get('child_info', None)) + @patch.dict('django.conf.settings.FEATURES', {'ENABLE_PROCTORED_EXAMS': True}) + def test_proctored_exam_xblock_info(self): + self.course.enable_proctored_exams = True + self.course.save() + self.store.update_item(self.course, self.user.id) + + course = modulestore().get_item(self.course.location) + xblock_info = create_xblock_info( + course, + include_child_info=True, + include_children_predicate=ALWAYS, + ) + # exam proctoring should be enabled and time limited. + self.assertEqual(xblock_info['enable_proctored_exams'], True) + + sequential = ItemFactory.create( + parent_location=self.chapter.location, category='sequential', + display_name="Test Lesson 1", user_id=self.user.id, + is_proctored_enabled=True, is_time_limited=True, + default_time_limit_minutes=100 + ) + sequential = modulestore().get_item(sequential.location) + xblock_info = create_xblock_info( + sequential, + include_child_info=True, + include_children_predicate=ALWAYS, + ) + # exam proctoring should be enabled and time limited. + self.assertEqual(xblock_info['is_proctored_enabled'], True) + self.assertEqual(xblock_info['is_time_limited'], True) + self.assertEqual(xblock_info['default_time_limit_minutes'], 100) + class TestLibraryXBlockInfo(ModuleStoreTestCase): """ diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index 54ccf989ac..7eb5713dae 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -46,6 +46,10 @@ class CourseMetadata(object): 'language', 'certificates', 'minimum_grade_credit', + 'default_time_limit_minutes', + 'is_proctored_enabled', + 'is_time_limited', + 'is_practice_exam', ] @classmethod diff --git a/cms/envs/aws.py b/cms/envs/aws.py index fdfaf9cca8..81304c4af8 100644 --- a/cms/envs/aws.py +++ b/cms/envs/aws.py @@ -349,3 +349,9 @@ if FEATURES['ENABLE_COURSEWARE_INDEX'] or FEATURES['ENABLE_LIBRARY_INDEX']: XBLOCK_SETTINGS = ENV_TOKENS.get('XBLOCK_SETTINGS', {}) XBLOCK_SETTINGS.setdefault("VideoDescriptor", {})["licensing_enabled"] = FEATURES.get("LICENSING", False) XBLOCK_SETTINGS.setdefault("VideoModule", {})['YOUTUBE_API_KEY'] = AUTH_TOKENS.get('YOUTUBE_API_KEY', YOUTUBE_API_KEY) + + +################# PROCTORING CONFIGURATION ################## + +PROCTORING_BACKEND_PROVIDER = AUTH_TOKENS.get("PROCTORING_BACKEND_PROVIDER", PROCTORING_BACKEND_PROVIDER) +PROCTORING_SETTINGS = ENV_TOKENS.get("PROCTORING_SETTINGS", PROCTORING_SETTINGS) diff --git a/cms/envs/common.py b/cms/envs/common.py index 8ca08aedcd..04c70e3132 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -181,6 +181,9 @@ FEATURES = { # Can the visibility of the discussion tab be configured on a per-course basis? 'ALLOW_HIDING_DISCUSSION_TAB': False, + + # Timed or Proctored Exams + 'ENABLE_PROCTORED_EXAMS': False, } ENABLE_JASMINE = False @@ -871,6 +874,9 @@ OPTIONAL_APPS = ( # milestones 'milestones', + + # edX Proctoring + 'edx_proctoring', ) @@ -1021,3 +1027,12 @@ CREDIT_PROVIDER_TIMESTAMP_EXPIRATION = 15 * 60 ################################ Deprecated Blocks Info ################################ DEPRECATED_BLOCK_TYPES = ['peergrading', 'combinedopenended'] + + +#### PROCTORING CONFIGURATION DEFAULTS + +PROCTORING_BACKEND_PROVIDER = { + 'class': 'edx_proctoring.backends.NullBackendProvider', + 'options': {}, +} +PROCTORING_SETTINGS = {} diff --git a/cms/static/js/spec/views/pages/course_outline_spec.js b/cms/static/js/spec/views/pages/course_outline_spec.js index 104eb19980..0e9104d9d2 100644 --- a/cms/static/js/spec/views/pages/course_outline_spec.js +++ b/cms/static/js/spec/views/pages/course_outline_spec.js @@ -17,6 +17,7 @@ define(["jquery", "common/js/spec_helpers/ajax_helpers", "js/views/utils/view_ut id: 'mock-course', display_name: 'Mock Course', category: 'course', + enable_proctored_exams: true, studio_url: '/course/slashes:MockCourse', is_container: true, has_changes: false, @@ -214,7 +215,7 @@ define(["jquery", "common/js/spec_helpers/ajax_helpers", "js/views/utils/view_ut 'course-outline', 'xblock-string-field-editor', 'modal-button', 'basic-modal', 'course-outline-modal', 'release-date-editor', 'due-date-editor', 'grading-editor', 'publish-editor', - 'staff-lock-editor' + 'staff-lock-editor', 'timed-examination-preference-editor' ]); appendSetFixtures(mockOutlinePage); mockCourseJSON = createMockCourseJSON({}, [ @@ -582,7 +583,7 @@ define(["jquery", "common/js/spec_helpers/ajax_helpers", "js/views/utils/view_ut }); describe("Subsection", function() { - var getDisplayNameWrapper, setEditModalValues, mockServerValuesJson; + var getDisplayNameWrapper, setEditModalValues, mockServerValuesJson, setModalTimedExaminationPreferenceValues; getDisplayNameWrapper = function() { return getItemHeaders('subsection').find('.wrapper-xblock-field'); @@ -595,6 +596,16 @@ define(["jquery", "common/js/spec_helpers/ajax_helpers", "js/views/utils/view_ut $("#staff_lock").prop('checked', is_locked); }; + setModalTimedExaminationPreferenceValues = function( + is_timed_examination, + time_limit, + is_exam_proctoring_enabled + ){ + $("#id_time_limit").val(time_limit); + $("#id_exam_proctoring").prop('checked', is_exam_proctoring_enabled); + $("#id_timed_examination").prop('checked', is_timed_examination); + }; + // Contains hard-coded dates because dates are presented in different formats. mockServerValuesJson = createMockSectionJSON({ release_date: 'Jan 01, 2970 at 05:00 UTC' @@ -607,7 +618,10 @@ define(["jquery", "common/js/spec_helpers/ajax_helpers", "js/views/utils/view_ut format: "Lab", due: "2014-07-10T00:00:00Z", has_explicit_staff_lock: true, - staff_only_message: true + staff_only_message: true, + "is_time_limited": true, + "is_proctored_enabled": true, + "default_time_limit_minutes": 150 }, [ createMockVerticalJSON({ has_changes: true, @@ -682,6 +696,7 @@ define(["jquery", "common/js/spec_helpers/ajax_helpers", "js/views/utils/view_ut createCourseOutlinePage(this, mockCourseJSON, false); outlinePage.$('.outline-subsection .configure-button').click(); setEditModalValues("7/9/2014", "7/10/2014", "Lab", true); + setModalTimedExaminationPreferenceValues(true, "02:30", true); $(".wrapper-modal-window .action-save").click(); AjaxHelpers.expectJsonRequest(requests, 'POST', '/xblock/mock-subsection', { "graderType":"Lab", @@ -689,7 +704,10 @@ define(["jquery", "common/js/spec_helpers/ajax_helpers", "js/views/utils/view_ut "metadata":{ "visible_to_staff_only": true, "start":"2014-07-09T00:00:00.000Z", - "due":"2014-07-10T00:00:00.000Z" + "due":"2014-07-10T00:00:00.000Z", + "is_time_limited": true, + "is_proctored_enabled": true, + "default_time_limit_minutes": 150 } }); expect(requests[0].requestHeaders['X-HTTP-Method-Override']).toBe('PATCH'); @@ -720,6 +738,27 @@ define(["jquery", "common/js/spec_helpers/ajax_helpers", "js/views/utils/view_ut expect($("#due_date").val()).toBe('7/10/2014'); expect($("#grading_type").val()).toBe('Lab'); expect($("#staff_lock").is(":checked")).toBe(true); + expect($("#id_timed_examination").is(":checked")).toBe(true); + expect($("#id_exam_proctoring").is(":checked")).toBe(true); + expect($("#id_time_limit").val()).toBe("02:30"); + }); + + it('can be edited and enable/disable proctoring fields, when time_limit checkbox value changes', function() { + createCourseOutlinePage(this, mockCourseJSON, false); + outlinePage.$('.outline-subsection .configure-button').click(); + setEditModalValues("7/9/2014", "7/10/2014", "Lab", true); + setModalTimedExaminationPreferenceValues(true, "02:30", true); + var target = $('#id_timed_examination'); + target.attr("checked","checked"); + target.click(); + expect($('#id_exam_proctoring')).toHaveAttr('disabled','disabled'); + expect($('#id_time_limit')).toHaveAttr('disabled','disabled'); + target.removeAttr("checked"); + target.click(); + expect($('#id_exam_proctoring')).not.toHaveAttr('disabled','disabled'); + expect($('#id_time_limit')).not.toHaveAttr('disabled','disabled'); + expect($('#id_time_limit').val()).toBe('00:30'); + expect($('#id_exam_proctoring')).not.toHaveAttr('checked'); }); it('release date, due date, grading type, and staff lock can be cleared.', function() { diff --git a/cms/static/js/views/course_outline.js b/cms/static/js/views/course_outline.js index 1a5e26dde4..a3db955686 100644 --- a/cms/static/js/views/course_outline.js +++ b/cms/static/js/views/course_outline.js @@ -140,9 +140,17 @@ define(["jquery", "underscore", "js/views/xblock_outline", "js/views/utils/view_ }, editXBlock: function() { + var enable_proctored_exams = false; + if (this.model.get('category') === 'sequential' && + this.parentView.parentView.model.has('enable_proctored_exams')) { + + enable_proctored_exams = this.parentView.parentView.model.get('enable_proctored_exams'); + } + var modal = CourseOutlineModalsFactory.getModal('edit', this.model, { onSave: this.refresh.bind(this), parentInfo: this.parentInfo, + enable_proctored_exams: enable_proctored_exams, xblockType: XBlockViewUtils.getXBlockType( this.model.get('category'), this.parentView.model, true ) diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index e46cd710de..9758fd7f0d 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -13,7 +13,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', ) { 'use strict'; var CourseOutlineXBlockModal, SettingsXBlockModal, PublishXBlockModal, AbstractEditor, BaseDateEditor, - ReleaseDateEditor, DueDateEditor, GradingEditor, PublishEditor, StaffLockEditor; + ReleaseDateEditor, DueDateEditor, GradingEditor, PublishEditor, StaffLockEditor, TimedExaminationPreferenceEditor; CourseOutlineXBlockModal = BaseModal.extend({ events : { @@ -257,7 +257,94 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', }; } }); + TimedExaminationPreferenceEditor = AbstractEditor.extend({ + templateName: 'timed-examination-preference-editor', + className: 'edit-settings-timed-examination', + events : { + 'change #id_timed_examination': 'timedExamination', + 'focusout #id_time_limit': 'timeLimitFocusout' + }, + timeLimitFocusout: function(event) { + var selectedTimeLimit = $(event.currentTarget).val(); + if (!this.isValidTimeLimit(selectedTimeLimit)) { + $(event.currentTarget).val("00:30"); + } + }, + timedExamination: function (event) { + event.preventDefault(); + if (!$(event.currentTarget).is(':checked')) { + this.$('#id_exam_proctoring').attr('checked', false); + this.$('#id_time_limit').val('00:30'); + this.$('#id_exam_proctoring').attr('disabled','disabled'); + this.$('#id_time_limit').attr('disabled', 'disabled'); + } + else { + this.$('#id_exam_proctoring').removeAttr('disabled'); + this.$('#id_time_limit').removeAttr('disabled'); + } + return true; + }, + afterRender: function () { + AbstractEditor.prototype.afterRender.call(this); + this.$('input.time').timepicker({ + 'timeFormat' : 'H:i', + 'forceRoundTime': false + }); + this.setExamTime(this.model.get('default_time_limit_minutes')); + this.setExamTmePreference(this.model.get('is_time_limited')); + this.setExamProctoring(this.model.get('is_proctored_enabled')); + }, + setExamProctoring: function(value) { + this.$('#id_exam_proctoring').prop('checked', value); + }, + setExamTime: function(value) { + var time = this.convertTimeLimitMinutesToString(value); + this.$('#id_time_limit').val(time); + }, + setExamTmePreference: function (value) { + this.$('#id_timed_examination').prop('checked', value); + if (!this.$('#id_timed_examination').is(':checked')) { + this.$('#id_exam_proctoring').attr('disabled','disabled'); + this.$('#id_time_limit').attr('disabled', 'disabled'); + } + }, + isExamTimeEnabled: function () { + return this.$('#id_timed_examination').is(':checked'); + }, + isValidTimeLimit: function(time_limit) { + var pattern = new RegExp('^([0-9]|0[0-9]|1[0-9]|2[0-3]):[0-5][0-9]$'); + return pattern.test(time_limit); + }, + getExamTimeLimit: function () { + return this.$('#id_time_limit').val(); + }, + convertTimeLimitMinutesToString: function (timeLimitMinutes) { + var hoursStr = "" + Math.floor(timeLimitMinutes / 60); + var actualMinutesStr = "" + (timeLimitMinutes % 60); + hoursStr = "00".substring(0, 2 - hoursStr.length) + hoursStr; + actualMinutesStr = "00".substring(0, 2 - actualMinutesStr.length) + actualMinutesStr; + return hoursStr + ":" + actualMinutesStr; + }, + convertTimeLimitToMinutes: function (time_limit) { + var time = time_limit.split(':'); + var total_time = (parseInt(time[0]) * 60) + parseInt(time[1]); + return total_time; + }, + isExamProctoringEnabled: function () { + return this.$('#id_exam_proctoring').is(':checked'); + }, + getRequestData: function () { + var time_limit = this.getExamTimeLimit(); + return { + metadata: { + 'is_time_limited': this.isExamTimeEnabled(), + 'is_proctored_enabled': this.isExamProctoringEnabled(), + 'default_time_limit_minutes': this.convertTimeLimitToMinutes(time_limit) + } + }; + } + }); GradingEditor = AbstractEditor.extend({ templateName: 'grading-editor', className: 'edit-settings-grading', @@ -358,11 +445,19 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', if (xblockInfo.isChapter()) { editors = [ReleaseDateEditor, StaffLockEditor]; } else if (xblockInfo.isSequential()) { - editors = [ReleaseDateEditor, GradingEditor, DueDateEditor, StaffLockEditor]; + editors = [ReleaseDateEditor, GradingEditor, DueDateEditor]; + + // since timed/proctored exams are optional + // we want it before the StaffLockEditor + // to keep it closer to the GradingEditor + if (options.enable_proctored_exams) { + editors.push(TimedExaminationPreferenceEditor); + } + + editors.push(StaffLockEditor); } else if (xblockInfo.isVertical()) { editors = [StaffLockEditor]; } - return new SettingsXBlockModal($.extend({ editors: editors, model: xblockInfo diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index ac33a0e8a4..e7d06cf9c0 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -519,9 +519,14 @@ .wrapper-modal-window-bulkpublish-subsection, .wrapper-modal-window-bulkpublish-unit, .course-outline-modal { - + .exam-time-list-fields { + margin-bottom: ($baseline/2); + } .list-fields { - + .field-message { + color: $gray; + font-size: ($baseline/2); + } .field { display: inline-block; vertical-align: top; @@ -625,7 +630,42 @@ } // UI: staff lock section - .edit-staff-lock { + .edit-staff-lock, .edit-settings-timed-examination { + + .checkbox-cosmetic .input-checkbox { + @extend %cont-text-sr; + + // CASE: unchecked + ~ .tip-warning { + display: block; + } + + // CASE: checked + &:checked { + + ~ .tip-warning { + display: none; + } + } + } + + // needed to override poorly scoped margin-bottom on any label element in a view (from _forms.scss) + .checkbox-cosmetic .label { + margin-bottom: 0; + } + } + + // UI: timed and proctored exam section + .edit-settings-timed-examination { + + // give a little space between the sections + padding-bottom: 10px; + + // indent this group a bit to make it seem like + // it is one group, under a header + .modal-section-content { + margin-left: 25px; + } .checkbox-cosmetic .input-checkbox { @extend %cont-text-sr; diff --git a/cms/static/sass/elements/_modules.scss b/cms/static/sass/elements/_modules.scss index 92ad1f86b6..4af90d4d68 100644 --- a/cms/static/sass/elements/_modules.scss +++ b/cms/static/sass/elements/_modules.scss @@ -249,7 +249,7 @@ opacity: 1.0; } - // reset to remove jquery-ui float + // reset to remove jquery-ui float a.link-tab { float: none; } @@ -546,20 +546,24 @@ $outline-indent-width: $baseline; > .subsection-status .status-grading { opacity: 1.0; } + + > .subsection-status .status-timed-proctored-exam { + opacity: 1.0; + } } // status - grading - .status-grading { + .status-grading, .status-timed-proctored-exam { @include transition(opacity $tmg-f2 ease-in-out 0s); opacity: 0.65; } - .status-grading-value { + .status-grading-value, .status-proctored-exam-value { display: inline-block; vertical-align: middle; } - .status-grading-date { + .status-grading-date, .status-due-date { display: inline-block; vertical-align: middle; margin-left: ($baseline/4); diff --git a/cms/templates/course_outline.html b/cms/templates/course_outline.html index 595e083cff..bc5b6a92d7 100644 --- a/cms/templates/course_outline.html +++ b/cms/templates/course_outline.html @@ -21,7 +21,7 @@ from microsite_configuration import microsite <%block name="header_extras"> -% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor']: +% for template_name in ['course-outline', 'xblock-string-field-editor', 'basic-modal', 'modal-button', 'course-outline-modal', 'due-date-editor', 'release-date-editor', 'grading-editor', 'publish-editor', 'staff-lock-editor', 'timed-examination-preference-editor']: diff --git a/cms/templates/js/course-outline.underscore b/cms/templates/js/course-outline.underscore index 97fe255516..94da74b5f0 100644 --- a/cms/templates/js/course-outline.underscore +++ b/cms/templates/js/course-outline.underscore @@ -142,8 +142,19 @@ if (xblockInfo.get('graded')) {

<% } %> + <% if (xblockInfo.get('is_time_limited')) { %> +
+

+ <%= gettext('Proctored Exam') %> + + <%= gettext('Timed and Proctored Exam') %> + <% if (xblockInfo.get('due_date')) { %> + <%= gettext('Due Date') %> <%= xblockInfo.get('due_date') %> + <% } %> +

+
+ <% } %> <% } %> - <% if (statusMessage) { %>
diff --git a/cms/templates/js/timed-examination-preference-editor.underscore b/cms/templates/js/timed-examination-preference-editor.underscore new file mode 100644 index 0000000000..027156cb0f --- /dev/null +++ b/cms/templates/js/timed-examination-preference-editor.underscore @@ -0,0 +1,39 @@ +
+ + +
diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index e5bf35b476..694ab758a0 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -907,6 +907,15 @@ class CourseFields(object): scope=Scope.settings ) + enable_proctored_exams = Boolean( + display_name=_("Enable Proctored Exams"), + help=_( + "Enter true or false. If this value is true, timed and proctored exams are enabled in your course." + ), + default=False, + scope=Scope.settings + ) + minimum_grade_credit = Float( display_name=_("Minimum Grade for Credit"), help=_( diff --git a/common/lib/xmodule/xmodule/seq_module.py b/common/lib/xmodule/xmodule/seq_module.py index e3fe56c646..416db39933 100644 --- a/common/lib/xmodule/xmodule/seq_module.py +++ b/common/lib/xmodule/xmodule/seq_module.py @@ -49,7 +49,49 @@ class SequenceFields(object): ) -class SequenceModule(SequenceFields, XModule): +class ProctoringFields(object): + """ + Fields that are specific to Proctored or Timed Exams + """ + is_time_limited = Boolean( + display_name=_("Is Time Limited"), + help=_( + "This setting indicates whether students have a limited time" + " to view or interact with this courseware component." + ), + default=False, + scope=Scope.settings, + ) + + default_time_limit_minutes = Integer( + display_name=_("Time Limit in Minutes"), + help=_( + "The number of minutes available to students for viewing or interacting with this courseware component." + ), + default=None, + scope=Scope.settings, + ) + + is_proctored_enabled = Boolean( + display_name=_("Is Proctoring Enabled"), + help=_( + "This setting indicates whether this exam is a proctored exam." + ), + default=False, + scope=Scope.settings, + ) + + is_practice_exam = Boolean( + display_name=_("Is Practice Exam"), + help=_( + "This setting indicates whether this exam is for testing purposes only. Practice exams are not verified." + ), + default=False, + scope=Scope.settings, + ) + + +class SequenceModule(SequenceFields, ProctoringFields, XModule): # pylint: disable=abstract-method ''' Layout module which lays out content in a temporal sequence ''' js = { @@ -153,7 +195,10 @@ class SequenceModule(SequenceFields, XModule): return new_class -class SequenceDescriptor(SequenceFields, MakoModuleDescriptor, XmlDescriptor): +class SequenceDescriptor(SequenceFields, ProctoringFields, MakoModuleDescriptor, XmlDescriptor): + """ + A Sequences Descriptor object + """ mako_template = 'widgets/sequence-edit.html' module_class = SequenceModule diff --git a/common/test/acceptance/pages/studio/settings_advanced.py b/common/test/acceptance/pages/studio/settings_advanced.py index d4a02d23d5..3046abe8bc 100644 --- a/common/test/acceptance/pages/studio/settings_advanced.py +++ b/common/test/acceptance/pages/studio/settings_advanced.py @@ -202,4 +202,5 @@ class AdvancedSettingsPage(CoursePage): 'teams_configuration', 'video_bumper', 'cert_html_view_enabled', + 'enable_proctored_exams', ] diff --git a/lms/envs/common.py b/lms/envs/common.py index 1700df4dbc..200533ac55 100644 --- a/lms/envs/common.py +++ b/lms/envs/common.py @@ -2385,6 +2385,9 @@ OPTIONAL_APPS = ( # milestones 'milestones', + + # edX Proctoring + 'edx_proctoring', ) for app_name in OPTIONAL_APPS: diff --git a/openedx/core/djangoapps/credit/signals.py b/openedx/core/djangoapps/credit/signals.py index 4480f10bed..aed3ef8032 100644 --- a/openedx/core/djangoapps/credit/signals.py +++ b/openedx/core/djangoapps/credit/signals.py @@ -15,10 +15,13 @@ from openedx.core.djangoapps.signals.signals import GRADES_UPDATED log = logging.getLogger(__name__) -@receiver(SignalHandler.course_published) -def listen_for_course_publish(sender, course_key, **kwargs): # pylint: disable=unused-argument - """Receive 'course_published' signal and kick off a celery task to update - the credit course requirements. +def on_course_publish(course_key): # pylint: disable=unused-argument + """ + Will receive a delegated 'course_published' signal from cms/djangoapps/contentstore/signals.py + and kick off a celery task to update the credit course requirements. + + IMPORTANT: It is assumed that the edx-proctoring subsystem has been appropriate refreshed + with any on_publish event workflow *BEFORE* this method is called. """ # Import here, because signal is registered at startup, but items in tasks diff --git a/openedx/core/djangoapps/credit/tasks.py b/openedx/core/djangoapps/credit/tasks.py index 82efad96b0..97f36a2b69 100644 --- a/openedx/core/djangoapps/credit/tasks.py +++ b/openedx/core/djangoapps/credit/tasks.py @@ -57,6 +57,9 @@ def _get_course_credit_requirements(course_key): """ Returns the list of credit requirements for the given course. + This will also call into the edx-proctoring subsystem to also + produce proctored exam requirements for credit bearing courses + It returns the minimum_grade_credit and also the ICRV checkpoints if any were added in the course @@ -69,7 +72,10 @@ def _get_course_credit_requirements(course_key): """ credit_xblock_requirements = _get_credit_course_requirement_xblocks(course_key) min_grade_requirement = _get_min_grade_requirement(course_key) - credit_requirements = min_grade_requirement + credit_xblock_requirements + proctored_exams_requirements = _get_proctoring_requirements(course_key) + credit_requirements = ( + min_grade_requirement + credit_xblock_requirements + proctored_exams_requirements + ) return credit_requirements @@ -161,3 +167,44 @@ def _is_credit_requirement(xblock): return False return True + + +def _get_proctoring_requirements(course_key): + """ + Will return list of requirements regarding any exams that have been + marked as proctored exams. For credit-bearing courses, all + proctored exams must be validated and confirmed from a proctoring + standpoint. The passing grade on an exam is not enough. + + Args: + course_key: The key of the course in question + + Returns: + list of requirements dictionary, one per active proctored exam + + """ + + # Note: Need to import here as there appears to be + # a circular reference happening when launching Studio + # process + from edx_proctoring.api import get_all_exams_for_course + + requirements = [ + { + 'namespace': 'proctored_exam', + 'name': 'proctored_exam_id:{id}'.format(id=exam['id']), + 'display_name': exam['exam_name'], + 'criteria': {}, + } + for exam in get_all_exams_for_course(unicode(course_key)) + if exam['is_proctored'] and exam['is_active'] + ] + + log_msg = ( + 'Registering the following as \'proctored_exam\' credit requirements: {log_msg}'.format( + log_msg=requirements + ) + ) + LOGGER.info(log_msg) + + return requirements diff --git a/openedx/core/djangoapps/credit/tests/test_tasks.py b/openedx/core/djangoapps/credit/tests/test_tasks.py index 2ddcabfbd8..216a410ca7 100644 --- a/openedx/core/djangoapps/credit/tests/test_tasks.py +++ b/openedx/core/djangoapps/credit/tests/test_tasks.py @@ -8,11 +8,12 @@ from datetime import datetime from openedx.core.djangoapps.credit.api import get_credit_requirements from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements from openedx.core.djangoapps.credit.models import CreditCourse -from openedx.core.djangoapps.credit.signals import listen_for_course_publish -from xmodule.modulestore.django import SignalHandler +from openedx.core.djangoapps.credit.signals import on_course_publish from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls +from edx_proctoring.api import create_exam + class TestTaskExecution(ModuleStoreTestCase): """Set of tests to ensure that the task code will do the right thing when @@ -44,7 +45,6 @@ class TestTaskExecution(ModuleStoreTestCase): def setUp(self): super(TestTaskExecution, self).setUp() - SignalHandler.course_published.disconnect(listen_for_course_publish) self.course = CourseFactory.create(start=datetime(2015, 3, 1)) def test_task_adding_requirements_invalid_course(self): @@ -53,7 +53,7 @@ class TestTaskExecution(ModuleStoreTestCase): """ requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 0) - listen_for_course_publish(self, self.course.id) + on_course_publish(self.course.id) requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 0) @@ -67,7 +67,7 @@ class TestTaskExecution(ModuleStoreTestCase): self.add_credit_course(self.course.id) requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 0) - listen_for_course_publish(self, self.course.id) + on_course_publish(self.course.id) requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 1) @@ -80,17 +80,100 @@ class TestTaskExecution(ModuleStoreTestCase): self.add_icrv_xblock() requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 0) - listen_for_course_publish(self, self.course.id) + on_course_publish(self.course.id) requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 2) + def test_proctored_exam_requirements(self): + """ + Make sure that proctored exams are being registered as requirements + """ + + self.add_credit_course(self.course.id) + create_exam( + course_id=unicode(self.course.id), + content_id='foo', + exam_name='A Proctored Exam', + time_limit_mins=10, + is_proctored=True, + is_active=True + ) + + requirements = get_credit_requirements(self.course.id) + self.assertEqual(len(requirements), 0) + on_course_publish(self.course.id) + + # just inspect the proctored exam requirement + requirements = [ + requirement + for requirement in get_credit_requirements(self.course.id) + if requirement['namespace'] == 'proctored_exam' + ] + + self.assertEqual(len(requirements), 1) + self.assertEqual(requirements[0]['namespace'], 'proctored_exam') + self.assertEqual(requirements[0]['name'], 'proctored_exam_id:1') + self.assertEqual(requirements[0]['display_name'], 'A Proctored Exam') + self.assertEqual(requirements[0]['criteria'], {}) + + def test_proctored_exam_filtering(self): + """ + Make sure that timed or inactive exams do not end up in the requirements table + """ + + self.add_credit_course(self.course.id) + create_exam( + course_id=unicode(self.course.id), + content_id='foo', + exam_name='A Proctored Exam', + time_limit_mins=10, + is_proctored=False, + is_active=True + ) + + requirements = get_credit_requirements(self.course.id) + self.assertEqual(len(requirements), 0) + + on_course_publish(self.course.id) + + requirements = get_credit_requirements(self.course.id) + self.assertEqual(len(requirements), 1) + + # make sure we don't have a proctoring requirement + self.assertFalse([ + requirement + for requirement in requirements + if requirement['namespace'] == 'proctored_exam' + ]) + + create_exam( + course_id=unicode(self.course.id), + content_id='foo2', + exam_name='A Proctored Exam', + time_limit_mins=10, + is_proctored=True, + is_active=False + ) + + on_course_publish(self.course.id) + + requirements = get_credit_requirements(self.course.id) + self.assertEqual(len(requirements), 1) + + # make sure we don't have a proctoring requirement + self.assertFalse([ + requirement + for requirement in requirements + if requirement['namespace'] == 'proctored_exam' + ]) + def test_query_counts(self): self.add_credit_course(self.course.id) self.add_icrv_xblock() with check_mongo_calls(3): - listen_for_course_publish(self, self.course.id) + on_course_publish(self.course.id) @mock.patch( 'openedx.core.djangoapps.credit.tasks.set_credit_requirements', @@ -108,7 +191,7 @@ class TestTaskExecution(ModuleStoreTestCase): self.add_credit_course(self.course.id) requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 0) - listen_for_course_publish(self, self.course.id) + on_course_publish(self.course.id) requirements = get_credit_requirements(self.course.id) self.assertEqual(len(requirements), 0) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 0000d56255..009fdfbe13 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -56,6 +56,9 @@ git+https://github.com/edx/edx-lint.git@ed8c8d2a0267d4d42f43642d193e25f8bd575d9b git+https://github.com/edx/ecommerce-api-client.git@1.1.0#egg=ecommerce-api-client==1.1.0 -e git+https://github.com/edx/edx-user-state-client.git@64a8b603f42669bb7fdca03d364d4e8d3d6ad67d#egg=edx-user-state-client +-e git+https://github.com/edx/edx-proctoring.git@6e7b4dba5b6d7a13c7dc111ae64e0579a1301ff9#egg=edx-proctoring + + # Third Party XBlocks -e git+https://github.com/mitodl/edx-sga@172a90fd2738f8142c10478356b2d9ed3e55334a#egg=edx-sga -e git+https://github.com/open-craft/xblock-poll@v1.0#egg=xblock-poll From c2fdf8cb2a8529cbe488224e400b3091081b5787 Mon Sep 17 00:00:00 2001 From: Clinton Blackburn Date: Sun, 26 Jul 2015 20:33:16 -0400 Subject: [PATCH 02/10] Corrected CourseMode Display Name The Commerce API now sets the correct display name for known course modes, but continues to default to the mode slug for unknown course modes. --- lms/djangoapps/commerce/api/v1/models.py | 19 ++++++++++- .../commerce/api/v1/tests/test_models.py | 33 +++++++++++++++++++ .../commerce/api/v1/tests/test_views.py | 5 +++ 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 lms/djangoapps/commerce/api/v1/tests/test_models.py diff --git a/lms/djangoapps/commerce/api/v1/models.py b/lms/djangoapps/commerce/api/v1/models.py index 235b1385d7..495dd0b93e 100644 --- a/lms/djangoapps/commerce/api/v1/models.py +++ b/lms/djangoapps/commerce/api/v1/models.py @@ -22,12 +22,29 @@ class Course(object): self.modes = list(modes) self._deleted_modes = [] + def get_mode_display_name(self, mode): + """ Returns display name for the given mode. """ + slug = mode.mode_slug.strip().lower() + + if slug == 'credit': + return 'Credit' + if 'professional' in slug: + return 'Professional Education' + elif slug == 'verified': + return 'Verified Certificate' + elif slug == 'honor': + return 'Honor Certificate' + elif slug == 'audit': + return 'Audit' + + return mode.mode_slug + @transaction.commit_on_success def save(self, *args, **kwargs): # pylint: disable=unused-argument """ Save the CourseMode objects to the database. """ for mode in self.modes: mode.course_id = self.id - mode.mode_display_name = mode.mode_slug + mode.mode_display_name = self.get_mode_display_name(mode) mode.save() deleted_mode_ids = [mode.id for mode in self._deleted_modes] diff --git a/lms/djangoapps/commerce/api/v1/tests/test_models.py b/lms/djangoapps/commerce/api/v1/tests/test_models.py new file mode 100644 index 0000000000..16c40acaaf --- /dev/null +++ b/lms/djangoapps/commerce/api/v1/tests/test_models.py @@ -0,0 +1,33 @@ +""" Tests for models. """ +import ddt +from django.test import TestCase + +from commerce.api.v1.models import Course +from course_modes.models import CourseMode + + +@ddt.ddt +class CourseTests(TestCase): + """ Tests for Course model. """ + def setUp(self): + super(CourseTests, self).setUp() + self.course = Course('a/b/c', []) + + @ddt.unpack + @ddt.data( + ('credit', 'Credit'), + ('professional', 'Professional Education'), + ('no-id-professional', 'Professional Education'), + ('verified', 'Verified Certificate'), + ('honor', 'Honor Certificate'), + ('audit', 'Audit'), + ) + def test_get_mode_display_name(self, slug, expected_display_name): + """ Verify the method properly maps mode slugs to display names. """ + mode = CourseMode(mode_slug=slug) + self.assertEqual(self.course.get_mode_display_name(mode), expected_display_name) + + def test_get_mode_display_name_unknown_slug(self): + """ Verify the method returns the slug if it has no known mapping. """ + mode = CourseMode(mode_slug='Blah!') + self.assertEqual(self.course.get_mode_display_name(mode), mode.mode_slug) diff --git a/lms/djangoapps/commerce/api/v1/tests/test_views.py b/lms/djangoapps/commerce/api/v1/tests/test_views.py index 36b264894c..bdf842af73 100644 --- a/lms/djangoapps/commerce/api/v1/tests/test_views.py +++ b/lms/djangoapps/commerce/api/v1/tests/test_views.py @@ -163,6 +163,11 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase) actual = json.loads(response.content) self.assertEqual(actual, expected) + # Verify the display names are correct + course_modes = CourseMode.objects.filter(course_id=course.id) + actual = [course_mode.mode_display_name for course_mode in course_modes] + self.assertListEqual(actual, ['Verified Certificate', 'Honor Certificate']) + def test_create_with_permissions(self): """ Verify the view supports creating a course as a user with the appropriate permissions. """ permissions = Permission.objects.filter(name__in=('Can add course mode', 'Can change course mode')) From 7d8fb739341c3a6b225f071d4eb2e700b9d3f69a Mon Sep 17 00:00:00 2001 From: jsa Date: Mon, 27 Jul 2015 15:46:15 -0400 Subject: [PATCH 03/10] commerce/api: pass expiration_datetime when updating modes --- lms/djangoapps/commerce/api/v1/models.py | 1 + .../commerce/api/v1/tests/test_views.py | 18 ++++++++++++++++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/commerce/api/v1/models.py b/lms/djangoapps/commerce/api/v1/models.py index 495dd0b93e..c8acf13168 100644 --- a/lms/djangoapps/commerce/api/v1/models.py +++ b/lms/djangoapps/commerce/api/v1/models.py @@ -66,6 +66,7 @@ class Course(object): merged_mode.min_price = posted_mode.min_price merged_mode.currency = posted_mode.currency merged_mode.sku = posted_mode.sku + merged_mode.expiration_datetime = posted_mode.expiration_datetime merged_modes.add(merged_mode) merged_mode_keys.add(merged_mode.mode_slug) diff --git a/lms/djangoapps/commerce/api/v1/tests/test_views.py b/lms/djangoapps/commerce/api/v1/tests/test_views.py index bdf842af73..f3eeabdc66 100644 --- a/lms/djangoapps/commerce/api/v1/tests/test_views.py +++ b/lms/djangoapps/commerce/api/v1/tests/test_views.py @@ -1,4 +1,5 @@ """ Commerce API v1 view tests. """ +from datetime import datetime import json import ddt @@ -6,6 +7,7 @@ from django.conf import settings from django.contrib.auth.models import Permission from django.core.urlresolvers import reverse from django.test.utils import override_settings +from rest_framework.utils.encoders import JSONEncoder from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory @@ -31,12 +33,17 @@ class CourseApiViewTestMixin(object): @staticmethod def _serialize_course_mode(course_mode): """ Serialize a CourseMode to a dict. """ + # encode the datetime (if nonempty) using DRF's encoder, simplifying + # equality assertions. + expires = course_mode.expiration_datetime + if expires is not None: + expires = JSONEncoder().default(expires) return { u'name': course_mode.mode_slug, u'currency': course_mode.currency.lower(), u'price': course_mode.min_price, u'sku': course_mode.sku, - u'expires': course_mode.expiration_datetime, + u'expires': expires, } @@ -112,7 +119,14 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase) """ Verify the view supports updating a course. """ permission = Permission.objects.get(name='Can change course mode') self.user.user_permissions.add(permission) - expected_course_mode = CourseMode(mode_slug=u'verified', min_price=200, currency=u'USD', sku=u'ABC123') + expiration_datetime = datetime.now() + expected_course_mode = CourseMode( + mode_slug=u'verified', + min_price=200, + currency=u'USD', + sku=u'ABC123', + expiration_datetime=expiration_datetime + ) expected = { u'id': unicode(self.course.id), u'modes': [self._serialize_course_mode(expected_course_mode)] From 849ad8df94bdc3d937a4dfbcb9d15a5f6cae3b7f Mon Sep 17 00:00:00 2001 From: jsa Date: Mon, 27 Jul 2015 12:15:43 -0400 Subject: [PATCH 04/10] =?UTF-8?q?commerce/api:=20Don=E2=80=99t=20allow=20s?= =?UTF-8?q?etting=20expiration=20of=20prof=20modes.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit XCOM-497 --- common/djangoapps/course_modes/models.py | 12 ++++++ .../course_modes/tests/test_models.py | 43 +++++++++++++++++-- .../commerce/api/v1/tests/test_views.py | 29 +++++++++++++ 3 files changed, 80 insertions(+), 4 deletions(-) diff --git a/common/djangoapps/course_modes/models.py b/common/djangoapps/course_modes/models.py index e0026329c0..db6ff974f3 100644 --- a/common/djangoapps/course_modes/models.py +++ b/common/djangoapps/course_modes/models.py @@ -4,6 +4,7 @@ Add and create new modes for running courses on this particular LMS import pytz from datetime import datetime +from django.core.exceptions import ValidationError from django.db import models from collections import namedtuple, defaultdict from django.utils.translation import ugettext_lazy as _ @@ -106,8 +107,19 @@ class CourseMode(models.Model): """ meta attributes of this model """ unique_together = ('course_id', 'mode_slug', 'currency') + def clean(self): + """ + Object-level validation - implemented in this method so DRF serializers + catch errors in advance of a save() attempt. + """ + if self.is_professional_slug(self.mode_slug) and self.expiration_datetime is not None: + raise ValidationError( + _(u"Professional education modes are not allowed to have expiration_datetime set.") + ) + def save(self, force_insert=False, force_update=False, using=None): # Ensure currency is always lowercase. + self.clean() # ensure object-level validation is performed before we save. self.currency = self.currency.lower() super(CourseMode, self).save(force_insert, force_update, using) diff --git a/common/djangoapps/course_modes/tests/test_models.py b/common/djangoapps/course_modes/tests/test_models.py index 814291c711..69de9de4a8 100644 --- a/common/djangoapps/course_modes/tests/test_models.py +++ b/common/djangoapps/course_modes/tests/test_models.py @@ -6,12 +6,15 @@ Replace this with more appropriate tests for your application. """ from datetime import datetime, timedelta -import pytz -import ddt +import itertools +import ddt +from django.core.exceptions import ValidationError +from django.test import TestCase from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locator import CourseLocator -from django.test import TestCase +import pytz + from course_modes.models import CourseMode, Mode @@ -26,7 +29,15 @@ class CourseModeModelTest(TestCase): self.course_key = SlashSeparatedCourseKey('Test', 'TestCourse', 'TestCourseRun') CourseMode.objects.all().delete() - def create_mode(self, mode_slug, mode_name, min_price=0, suggested_prices='', currency='usd'): + def create_mode( + self, + mode_slug, + mode_name, + min_price=0, + suggested_prices='', + currency='usd', + expiration_datetime=None, + ): """ Create a new course mode """ @@ -37,6 +48,7 @@ class CourseModeModelTest(TestCase): min_price=min_price, suggested_prices=suggested_prices, currency=currency, + expiration_datetime=expiration_datetime, ) def test_save(self): @@ -264,6 +276,29 @@ class CourseModeModelTest(TestCase): else: self.assertFalse(CourseMode.is_verified_slug(mode_slug)) + @ddt.data(*itertools.product( + ( + CourseMode.HONOR, + CourseMode.AUDIT, + CourseMode.VERIFIED, + CourseMode.PROFESSIONAL, + CourseMode.NO_ID_PROFESSIONAL_MODE + ), + (datetime.now(), None), + )) + @ddt.unpack + def test_invalid_mode_expiration(self, mode_slug, exp_dt): + is_error_expected = CourseMode.is_professional_slug(mode_slug) and exp_dt is not None + try: + self.create_mode(mode_slug=mode_slug, mode_name=mode_slug.title(), expiration_datetime=exp_dt) + self.assertFalse(is_error_expected, "Expected a ValidationError to be thrown.") + except ValidationError, exc: + self.assertTrue(is_error_expected, "Did not expect a ValidationError to be thrown.") + self.assertEqual( + exc.messages, + [u"Professional education modes are not allowed to have expiration_datetime set."], + ) + @ddt.data( ("verified", "verify_need_to_verify"), ("verified", "verify_submitted"), diff --git a/lms/djangoapps/commerce/api/v1/tests/test_views.py b/lms/djangoapps/commerce/api/v1/tests/test_views.py index f3eeabdc66..7419fe8362 100644 --- a/lms/djangoapps/commerce/api/v1/tests/test_views.py +++ b/lms/djangoapps/commerce/api/v1/tests/test_views.py @@ -1,5 +1,6 @@ """ Commerce API v1 view tests. """ from datetime import datetime +import itertools import json import ddt @@ -158,6 +159,34 @@ class CourseRetrieveUpdateViewTests(CourseApiViewTestMixin, ModuleStoreTestCase) # The existing CourseMode should have been removed. self.assertFalse(CourseMode.objects.filter(id=self.course_mode.id).exists()) + @ddt.data(*itertools.product( + ('honor', 'audit', 'verified', 'professional', 'no-id-professional'), + (datetime.now(), None), + )) + @ddt.unpack + def test_update_professional_expiration(self, mode_slug, expiration_datetime): + """ Verify that pushing a mode with a professional certificate and an expiration datetime + will be rejected (this is not allowed). """ + permission = Permission.objects.get(name='Can change course mode') + self.user.user_permissions.add(permission) + + mode = self._serialize_course_mode( + CourseMode( + mode_slug=mode_slug, + min_price=500, + currency=u'USD', + sku=u'ABC123', + expiration_datetime=expiration_datetime + ) + ) + course_id = unicode(self.course.id) + payload = {u'id': course_id, u'modes': [mode]} + path = reverse('commerce_api:v1:courses:retrieve_update', args=[course_id]) + + expected_status = 400 if CourseMode.is_professional_slug(mode_slug) and expiration_datetime is not None else 200 + response = self.client.put(path, json.dumps(payload), content_type=JSON_CONTENT_TYPE) + self.assertEqual(response.status_code, expected_status) + def assert_can_create_course(self, **request_kwargs): """ Verify a course can be created by the view. """ course = CourseFactory.create() From 9e00e2b39775efc04a2e97b8a1108f8623e4f42f Mon Sep 17 00:00:00 2001 From: Braden MacDonald Date: Tue, 21 Jul 2015 15:56:33 -0700 Subject: [PATCH 05/10] Allow previewing cohorted content when masquerading --- .../test/acceptance/pages/lms/staff_view.py | 13 ++++++ .../tests/lms/test_lms_user_preview.py | 40 +++++++++++++++++++ lms/djangoapps/courseware/masquerade.py | 2 +- .../course_groups/partition_scheme.py | 21 ++++++---- 4 files changed, 68 insertions(+), 8 deletions(-) diff --git a/common/test/acceptance/pages/lms/staff_view.py b/common/test/acceptance/pages/lms/staff_view.py index b4eeb5d505..b083fe76ca 100644 --- a/common/test/acceptance/pages/lms/staff_view.py +++ b/common/test/acceptance/pages/lms/staff_view.py @@ -33,6 +33,19 @@ class StaffPage(CoursewarePage): self.q(css=self.VIEW_MODE_OPTIONS_CSS).filter(lambda el: el.text == view_mode).first.click() self.wait_for_ajax() + def set_staff_view_mode_specific_student(self, username_or_email): + """ + Set the current preview mode to "Specific Student" with the given username or email + """ + required_mode = "Specific student" + if self.staff_view_mode != required_mode: + self.q(css=self.VIEW_MODE_OPTIONS_CSS).filter(lambda el: el.text == required_mode).first.click() + # Use a script here because .clear() + .send_keys() triggers unwanted behavior if a username is already set + self.browser.execute_script( + '$(".action-preview-username").val("{}").blur().change();'.format(username_or_email) + ) + self.wait_for_ajax() + def open_staff_debug_info(self): """ Open the staff debug window diff --git a/common/test/acceptance/tests/lms/test_lms_user_preview.py b/common/test/acceptance/tests/lms/test_lms_user_preview.py index 292bbf8dcf..06b42f11a3 100644 --- a/common/test/acceptance/tests/lms/test_lms_user_preview.py +++ b/common/test/acceptance/tests/lms/test_lms_user_preview.py @@ -6,8 +6,10 @@ Tests the "preview" selector in the LMS that allows changing between Staff, Stud from ..helpers import UniqueCourseTest, create_user_partition_json from ...pages.studio.auto_auth import AutoAuthPage from ...pages.lms.courseware import CoursewarePage +from ...pages.lms.instructor_dashboard import InstructorDashboardPage from ...pages.lms.staff_view import StaffPage from ...fixtures.course import CourseFixture, XBlockFixtureDesc +from bok_choy.promise import EmptyPromise from xmodule.partitions.partitions import Group from textwrap import dedent @@ -335,6 +337,44 @@ class CourseWithContentGroupsTest(StaffViewTest): course_page.set_staff_view_mode('Student in beta') verify_expected_problem_visibility(self, course_page, [self.beta_text, self.everyone_text]) + def create_cohorts_and_assign_students(self, student_a_username, student_b_username): + """ + Adds 2 manual cohorts, linked to content groups, to the course. + Each cohort is assigned one student. + """ + instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id) + instructor_dashboard_page.visit() + cohort_management_page = instructor_dashboard_page.select_cohort_management() + cohort_management_page.is_cohorted = True + + def add_cohort_with_student(cohort_name, content_group, student): + """ Create cohort and assign student to it. """ + cohort_management_page.add_cohort(cohort_name, content_group=content_group) + # After adding the cohort, it should automatically be selected + EmptyPromise( + lambda: cohort_name == cohort_management_page.get_selected_cohort(), "Waiting for new cohort" + ).fulfill() + cohort_management_page.add_students_to_selected_cohort([student]) + add_cohort_with_student("Cohort Alpha", "alpha", student_a_username) + add_cohort_with_student("Cohort Beta", "beta", student_b_username) + cohort_management_page.wait_for_ajax() + + def test_as_specific_student(self): + student_a_username = 'tass_student_a' + student_b_username = 'tass_student_b' + AutoAuthPage(self.browser, username=student_a_username, course_id=self.course_id, no_login=True).visit() + AutoAuthPage(self.browser, username=student_b_username, course_id=self.course_id, no_login=True).visit() + self.create_cohorts_and_assign_students(student_a_username, student_b_username) + + # Masquerade as student in alpha cohort: + course_page = self._goto_staff_page() + course_page.set_staff_view_mode_specific_student(student_a_username) + verify_expected_problem_visibility(self, course_page, [self.alpha_text, self.everyone_text]) + + # Masquerade as student in beta cohort: + course_page.set_staff_view_mode_specific_student(student_b_username) + verify_expected_problem_visibility(self, course_page, [self.beta_text, self.everyone_text]) + def verify_expected_problem_visibility(test, courseware_page, expected_problems): """ diff --git a/lms/djangoapps/courseware/masquerade.py b/lms/djangoapps/courseware/masquerade.py index b0bbba9ebe..5c244f18f7 100644 --- a/lms/djangoapps/courseware/masquerade.py +++ b/lms/djangoapps/courseware/masquerade.py @@ -54,8 +54,8 @@ def handle_ajax(request, course_key_string): masquerade_settings = request.session.get(MASQUERADE_SETTINGS_KEY, {}) request_json = request.json role = request_json.get('role', 'student') - user_partition_id = request_json.get('user_partition_id', None) group_id = request_json.get('group_id', None) + user_partition_id = request_json.get('user_partition_id', None) if group_id is not None else None user_name = request_json.get('user_name', None) if user_name: users_in_course = CourseEnrollment.objects.users_enrolled_in(course_key) diff --git a/openedx/core/djangoapps/course_groups/partition_scheme.py b/openedx/core/djangoapps/course_groups/partition_scheme.py index cb352ba993..346528e825 100644 --- a/openedx/core/djangoapps/course_groups/partition_scheme.py +++ b/openedx/core/djangoapps/course_groups/partition_scheme.py @@ -4,7 +4,11 @@ Provides a UserPartition driver for cohorts. import logging from courseware import courses -from courseware.masquerade import get_masquerading_group_info +from courseware.masquerade import ( # pylint: disable=import-error + get_course_masquerade, + get_masquerading_group_info, + is_masquerading_as_specific_student, +) from xmodule.partitions.partitions import NoSuchUserPartitionGroupError from .cohorts import get_cohort, get_group_info_for_cohort @@ -36,16 +40,19 @@ class CohortPartitionScheme(object): If the user has no cohort mapping, or there is no (valid) cohort -> partition group mapping found, the function returns None. """ - # If the current user is masquerading as being in a group - # belonging to the specified user partition, return the - # masquerading group or None if the group can't be found. - group_id, user_partition_id = get_masquerading_group_info(user, course_key) - if user_partition_id == user_partition.id: - if group_id is not None: + # First, check if we have to deal with masquerading. + # If the current user is masquerading as a specific student, use the + # same logic as normal to return that student's group. If the current + # user is masquerading as a generic student in a specific group, then + # return that group. + if get_course_masquerade(user, course_key) and not is_masquerading_as_specific_student(user, course_key): + group_id, user_partition_id = get_masquerading_group_info(user, course_key) + if user_partition_id == user_partition.id and group_id is not None: try: return user_partition.get_group(group_id) except NoSuchUserPartitionGroupError: return None + # The user is masquerading as a generic student. We can't show any particular group. return None cohort = get_cohort(user, course_key, use_cached=use_cached) From 65367a2ded9522a9de849adb138fd9d375ebb779 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Sun, 19 Jul 2015 16:34:15 -0700 Subject: [PATCH 06/10] ECOM-1911: Fixes to credit requirements in Studio. * Exclude orphaned XBlocks from requirements because they have been deleted. * Sort ICRV requirements by start date, then by display name. --- openedx/core/djangoapps/credit/models.py | 3 +- openedx/core/djangoapps/credit/tasks.py | 85 +++++++++++++----- .../djangoapps/credit/tests/test_tasks.py | 90 ++++++++++++++++--- 3 files changed, 143 insertions(+), 35 deletions(-) diff --git a/openedx/core/djangoapps/credit/models.py b/openedx/core/djangoapps/credit/models.py index 11c82d6e13..c1c653ac2e 100644 --- a/openedx/core/djangoapps/credit/models.py +++ b/openedx/core/djangoapps/credit/models.py @@ -286,6 +286,7 @@ class CreditRequirement(TimeStampedModel): Model metadata. """ unique_together = ('namespace', 'name', 'course') + ordering = ["order"] @classmethod def add_or_update_course_requirement(cls, credit_course, requirement, order): @@ -337,7 +338,7 @@ class CreditRequirement(TimeStampedModel): """ # order credit requirements according to their appearance in courseware - requirements = CreditRequirement.objects.filter(course__course_key=course_key, active=True).order_by("-order") + requirements = CreditRequirement.objects.filter(course__course_key=course_key, active=True) if namespace is not None: requirements = requirements.filter(namespace=namespace) diff --git a/openedx/core/djangoapps/credit/tasks.py b/openedx/core/djangoapps/credit/tasks.py index 97f36a2b69..136dd8a8f0 100644 --- a/openedx/core/djangoapps/credit/tasks.py +++ b/openedx/core/djangoapps/credit/tasks.py @@ -2,6 +2,9 @@ This file contains celery tasks for credit course views. """ +import datetime +from pytz import UTC + from django.conf import settings from celery import task @@ -13,17 +16,15 @@ from .api import set_credit_requirements from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements from openedx.core.djangoapps.credit.models import CreditCourse from xmodule.modulestore.django import modulestore +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.exceptions import ItemNotFoundError LOGGER = get_task_logger(__name__) # XBlocks that can be added as credit requirements -CREDIT_REQUIREMENT_XBLOCKS = [ - { - "category": "edx-reverification-block", - "requirement-namespace": "reverification" - } +CREDIT_REQUIREMENT_XBLOCK_CATEGORIES = [ + "edx-reverification-block", ] @@ -123,24 +124,62 @@ def _get_credit_course_requirement_xblocks(course_key): # pylint: disable=inval # Retrieve all XBlocks from the course that we know to be credit requirements. # For performance reasons, we look these up by their "category" to avoid # loading and searching the entire course tree. - for desc in CREDIT_REQUIREMENT_XBLOCKS: + for category in CREDIT_REQUIREMENT_XBLOCK_CATEGORIES: requirements.extend([ { - "namespace": desc["requirement-namespace"], + "namespace": block.get_credit_requirement_namespace(), "name": block.get_credit_requirement_name(), "display_name": block.get_credit_requirement_display_name(), "criteria": {}, } - for block in modulestore().get_items( - course_key, - qualifiers={"category": desc["category"]} - ) + for block in _get_xblocks(course_key, category) if _is_credit_requirement(block) ]) return requirements +def _is_in_course_tree(block): + """ + Check that the XBlock is in the course tree. + + It's possible that the XBlock is not in the course tree + if its parent has been deleted and is now an orphan. + """ + ancestor = block.get_parent() + while ancestor is not None and ancestor.location.category != "course": + ancestor = ancestor.get_parent() + + return ancestor is not None + + +def _get_xblocks(course_key, category): + """ + Retrieve all XBlocks in the course for a particular category. + + Returns only XBlocks that are published and haven't been deleted. + """ + xblocks = [ + block for block in modulestore().get_items( + course_key, + qualifiers={"category": category}, + revision=ModuleStoreEnum.RevisionOption.published_only, + ) + if _is_in_course_tree(block) + ] + + # Secondary sort on credit requirement name + xblocks = sorted(xblocks, key=lambda block: block.get_credit_requirement_display_name()) + + # Primary sort on start date + xblocks = sorted(xblocks, key=lambda block: ( + block.start if block.start is not None + else datetime.datetime(datetime.MINYEAR, 1, 1).replace(tzinfo=UTC) + )) + + return xblocks + + def _is_credit_requirement(xblock): """ Check if the given XBlock is a credit requirement. @@ -152,19 +191,19 @@ def _is_credit_requirement(xblock): True if XBlock is a credit requirement else False """ - if not callable(getattr(xblock, "get_credit_requirement_namespace", None)): - LOGGER.error( - "XBlock %s is marked as a credit requirement but does not " - "implement get_credit_requirement_namespace()", unicode(xblock) - ) - return False + required_methods = [ + "get_credit_requirement_namespace", + "get_credit_requirement_name", + "get_credit_requirement_display_name" + ] - if not callable(getattr(xblock, "get_credit_requirement_name", None)): - LOGGER.error( - "XBlock %s is marked as a credit requirement but does not " - "implement get_credit_requirement_name()", unicode(xblock) - ) - return False + for method_name in required_methods: + if not callable(getattr(xblock, method_name, None)): + LOGGER.error( + "XBlock %s is marked as a credit requirement but does not " + "implement %s", unicode(xblock), method_name + ) + return False return True diff --git a/openedx/core/djangoapps/credit/tests/test_tasks.py b/openedx/core/djangoapps/credit/tests/test_tasks.py index 216a410ca7..3959168b7b 100644 --- a/openedx/core/djangoapps/credit/tests/test_tasks.py +++ b/openedx/core/djangoapps/credit/tests/test_tasks.py @@ -3,14 +3,16 @@ Tests for credit course tasks. """ import mock -from datetime import datetime +from datetime import datetime, timedelta +from pytz import UTC from openedx.core.djangoapps.credit.api import get_credit_requirements from openedx.core.djangoapps.credit.exceptions import InvalidCreditRequirements from openedx.core.djangoapps.credit.models import CreditCourse from openedx.core.djangoapps.credit.signals import on_course_publish +from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls_range from edx_proctoring.api import create_exam @@ -30,22 +32,32 @@ class TestTaskExecution(ModuleStoreTestCase): """ raise InvalidCreditRequirements - def add_icrv_xblock(self): + def add_icrv_xblock(self, related_assessment_name=None, start_date=None): """ Create the 'edx-reverification-block' in course tree """ - - section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') - subsection = ItemFactory.create(parent=section, category='sequential', display_name='Test Subsection') - vertical = ItemFactory.create(parent=subsection, category='vertical', display_name='Test Unit') - ItemFactory.create( - parent=vertical, + block = ItemFactory.create( + parent=self.vertical, category='edx-reverification-block', - display_name='Test Verification Block' ) + if related_assessment_name is not None: + block.related_assessment = related_assessment_name + + block.start = start_date + + self.store.update_item(block, ModuleStoreEnum.UserID.test) + + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): + self.store.publish(block.location, ModuleStoreEnum.UserID.test) + + return block + def setUp(self): super(TestTaskExecution, self).setUp() self.course = CourseFactory.create(start=datetime(2015, 3, 1)) + self.section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section') + self.subsection = ItemFactory.create(parent=self.section, category='sequential', display_name='Test Subsection') + self.vertical = ItemFactory.create(parent=self.subsection, category='vertical', display_name='Test Unit') def test_task_adding_requirements_invalid_course(self): """ @@ -172,9 +184,65 @@ class TestTaskExecution(ModuleStoreTestCase): self.add_credit_course(self.course.id) self.add_icrv_xblock() - with check_mongo_calls(3): + with check_mongo_calls_range(max_finds=7): on_course_publish(self.course.id) + def test_remove_icrv_requirement(self): + self.add_credit_course(self.course.id) + self.add_icrv_xblock() + on_course_publish(self.course.id) + + # There should be one ICRV requirement + requirements = get_credit_requirements(self.course.id, namespace="reverification") + self.assertEqual(len(requirements), 1) + + # Delete the parent section containing the ICRV block + with self.store.branch_setting(ModuleStoreEnum.Branch.draft_preferred, self.course.id): + self.store.delete_item(self.subsection.location, ModuleStoreEnum.UserID.test) + + # Check that the ICRV block is no longer visible in the requirements + on_course_publish(self.course.id) + requirements = get_credit_requirements(self.course.id, namespace="reverification") + self.assertEqual(len(requirements), 0) + + def test_icrv_requirement_ordering(self): + self.add_credit_course(self.course.id) + + # Create multiple ICRV blocks + start = datetime.now(UTC) + self.add_icrv_xblock(related_assessment_name="Midterm A", start_date=start) + + start = start - timedelta(days=1) + self.add_icrv_xblock(related_assessment_name="Midterm B", start_date=start) + + # Primary sort is based on start date + on_course_publish(self.course.id) + requirements = get_credit_requirements(self.course.id, namespace="reverification") + self.assertEqual(len(requirements), 2) + self.assertEqual(requirements[0]["display_name"], "Midterm B") + self.assertEqual(requirements[1]["display_name"], "Midterm A") + + # Add two additional ICRV blocks that have no start date + # and the same name. + start = datetime.now(UTC) + first_block = self.add_icrv_xblock(related_assessment_name="Midterm Start Date") + + start = start + timedelta(days=1) + second_block = self.add_icrv_xblock(related_assessment_name="Midterm Start Date") + + on_course_publish(self.course.id) + requirements = get_credit_requirements(self.course.id, namespace="reverification") + self.assertEqual(len(requirements), 4) + self.assertEqual(requirements[0]["display_name"], "Midterm Start Date") + self.assertEqual(requirements[1]["display_name"], "Midterm Start Date") + self.assertEqual(requirements[2]["display_name"], "Midterm B") + self.assertEqual(requirements[3]["display_name"], "Midterm A") + + # Since the first two requirements have the same display name, + # we need to also check that their internal names (locations) are the same. + self.assertEqual(requirements[0]["name"], first_block.get_credit_requirement_name()) + self.assertEqual(requirements[1]["name"], second_block.get_credit_requirement_name()) + @mock.patch( 'openedx.core.djangoapps.credit.tasks.set_credit_requirements', mock.Mock( From a1f79a3c03dbf85a26ae03a3ccc44766cd939201 Mon Sep 17 00:00:00 2001 From: Will Daly Date: Thu, 23 Jul 2015 14:02:48 -0700 Subject: [PATCH 07/10] Add in-course reverification to advanced components. --- cms/envs/common.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/cms/envs/common.py b/cms/envs/common.py index 04c70e3132..cde1b202b6 100644 --- a/cms/envs/common.py +++ b/cms/envs/common.py @@ -955,6 +955,9 @@ ADVANCED_COMPONENT_TYPES = [ # embed public google drive documents and calendars within edX units 'google-document', 'google-calendar', + + # In-course reverification checkpoint + 'edx-reverification-block', ] # Adding components in this list will disable the creation of new problem for those From 4b9792a149bb2276feeaa4857a96f62194c8d714 Mon Sep 17 00:00:00 2001 From: Carson Gee Date: Mon, 27 Jul 2015 11:15:14 -0400 Subject: [PATCH 08/10] Resolved LogHandler bug in rf6266 --- requirements/edx/base.txt | 1 - requirements/edx/github.txt | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/base.txt b/requirements/edx/base.txt index f3f4239b68..b32a12e0c7 100644 --- a/requirements/edx/base.txt +++ b/requirements/edx/base.txt @@ -75,7 +75,6 @@ pysrt==0.4.7 PyYAML==3.10 requests==2.3.0 requests-oauthlib==0.4.1 -rfc6266==0.0.4 scipy==0.14.0 Shapely==1.2.16 singledispatch==3.4.0.2 diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 009fdfbe13..3b3a9f6689 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -31,6 +31,7 @@ git+https://github.com/pmitros/pyfs.git@96e1922348bfe6d99201b9512a9ed946c87b7e0b git+https://github.com/hmarr/django-debug-toolbar-mongo.git@b0686a76f1ce3532088c4aee6e76b9abe61cc808 # custom opaque-key implementations for ccx -e git+https://github.com/jazkarta/ccx-keys.git@e6b03704b1bb97c1d2f31301ecb4e3a687c536ea#egg=ccx-keys +git+https://github.com/edx/rfc6266.git@v0.0.5-edx#egg=rfc6266 # Our libraries: -e git+https://github.com/edx/XBlock.git@017b46b80e712b1318379912257cf1cc1b68eb0e#egg=XBlock From 6c3a9024826f266915cbd59cb95dbf61f9ce48a0 Mon Sep 17 00:00:00 2001 From: Chris Dodge Date: Wed, 29 Jul 2015 10:49:34 -0400 Subject: [PATCH 09/10] switch to using tags --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index 3b3a9f6689..b6dfc33600 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -57,7 +57,7 @@ git+https://github.com/edx/edx-lint.git@ed8c8d2a0267d4d42f43642d193e25f8bd575d9b git+https://github.com/edx/ecommerce-api-client.git@1.1.0#egg=ecommerce-api-client==1.1.0 -e git+https://github.com/edx/edx-user-state-client.git@64a8b603f42669bb7fdca03d364d4e8d3d6ad67d#egg=edx-user-state-client --e git+https://github.com/edx/edx-proctoring.git@6e7b4dba5b6d7a13c7dc111ae64e0579a1301ff9#egg=edx-proctoring +-e git+https://github.com/edx/edx-proctoring.git@release-2015-07-29#egg=edx-proctoring # Third Party XBlocks From 898a43c49f98fec9c8af0a07a101ed2305bf2682 Mon Sep 17 00:00:00 2001 From: Kevin Falcone Date: Wed, 29 Jul 2015 15:41:33 -0400 Subject: [PATCH 10/10] Pin the rc6266 fork release If we don't pin to the version, pip will see that we have a version of rfc6266 installed and be fine with it. Yes, you have to say it twice. --- requirements/edx/github.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements/edx/github.txt b/requirements/edx/github.txt index b6dfc33600..0ba64275c5 100644 --- a/requirements/edx/github.txt +++ b/requirements/edx/github.txt @@ -31,7 +31,7 @@ git+https://github.com/pmitros/pyfs.git@96e1922348bfe6d99201b9512a9ed946c87b7e0b git+https://github.com/hmarr/django-debug-toolbar-mongo.git@b0686a76f1ce3532088c4aee6e76b9abe61cc808 # custom opaque-key implementations for ccx -e git+https://github.com/jazkarta/ccx-keys.git@e6b03704b1bb97c1d2f31301ecb4e3a687c536ea#egg=ccx-keys -git+https://github.com/edx/rfc6266.git@v0.0.5-edx#egg=rfc6266 +git+https://github.com/edx/rfc6266.git@v0.0.5-edx#egg=rfc6266==0.0.5-edx # Our libraries: -e git+https://github.com/edx/XBlock.git@017b46b80e712b1318379912257cf1cc1b68eb0e#egg=XBlock