Merge pull request #7087 from edx/rc/2015-02-26

Rc/2015 02 26

AUTHORS

@@ -196,3 +196,4 @@ Davorin Šego <dsego@edx.org>
 Marko Jevtić <mjevtic@edx.org>
 Ahsan Ulhaq <ahsan@edx.org>
 Mat Moore <mat@mooresoftware.co.uk>
+Muzaffar Yousaf <muzaffar@edx.org>

cms/djangoapps/contentstore/views/component.py

@@ -331,7 +331,6 @@ def get_component_templates(courselike, library=False):
                         "Advanced component %s does not exist. It will not be added to the Studio new component menu.",
                         category
                     )
-                    pass
     else:
         log.error(
             "Improper format for course advanced keys! %s",

cms/djangoapps/contentstore/views/course.py

@@ -85,6 +85,10 @@ from util.milestones_helpers import (
 
 MINIMUM_GROUP_ID = 100
 
+RANDOM_SCHEME = "random"
+COHORT_SCHEME = "cohort"
+
+
 # Note: the following content group configuration strings are not
 # translated since they are not visible to users.
 CONTENT_GROUP_CONFIGURATION_DESCRIPTION = 'The groups in this configuration can be mapped to cohort groups in the LMS.'
@@ -1396,19 +1400,38 @@ class GroupConfiguration(object):
         return UserPartition.from_json(self.configuration)
 
     @staticmethod
-    def get_usage_info(course, store):
+    def _get_usage_info(course, unit, item, usage_info, group_id, scheme_name=None):
+        """
+        Get usage info for unit/module.
+        """
+        unit_url = reverse_usage_url(
+            'container_handler',
+            course.location.course_key.make_usage_key(unit.location.block_type, unit.location.name)
+        )
+
+        usage_dict = {'label': u"{} / {}".format(unit.display_name, item.display_name), 'url': unit_url}
+        if scheme_name == RANDOM_SCHEME:
+            validation_summary = item.general_validation_message()
+            usage_dict.update({'validation': validation_summary.to_json() if validation_summary else None})
+
+        usage_info[group_id].append(usage_dict)
+
+        return usage_info
+
+    @staticmethod
+    def get_content_experiment_usage_info(store, course):
         """
         Get usage information for all Group Configurations currently referenced by a split_test instance.
         """
         split_tests = store.get_items(course.id, qualifiers={'category': 'split_test'})
-        return GroupConfiguration._get_usage_info(store, course, split_tests)
+        return GroupConfiguration._get_content_experiment_usage_info(store, course, split_tests)
 
     @staticmethod
-    def get_split_test_partitions_with_usage(course, store):
+    def get_split_test_partitions_with_usage(store, course):
         """
         Returns json split_test group configurations updated with usage information.
         """
-        usage_info = GroupConfiguration.get_usage_info(course, store)
+        usage_info = GroupConfiguration.get_content_experiment_usage_info(store, course)
         configurations = []
         for partition in get_split_user_partitions(course.user_partitions):
             configuration = partition.to_json()
@@ -1417,7 +1440,7 @@ class GroupConfiguration(object):
         return configurations
 
     @staticmethod
-    def _get_usage_info(store, course, split_tests):
+    def _get_content_experiment_usage_info(store, course, split_tests):
         """
         Returns all units names, their urls and validation messages.
 
@@ -1442,28 +1465,70 @@ class GroupConfiguration(object):
             if split_test.user_partition_id not in usage_info:
                 usage_info[split_test.user_partition_id] = []
 
-            unit_location = store.get_parent_location(split_test.location)
-            if not unit_location:
-                log.warning("Parent location of split_test module not found: %s", split_test.location)
+            unit = split_test.get_parent()
+            if not unit:
+                log.warning("Unable to find parent for split_test %s", split_test.location)
                 continue
 
-            try:
-                unit = store.get_item(unit_location)
-            except ItemNotFoundError:
-                log.warning("Unit not found: %s", unit_location)
-                continue
-
-            unit_url = reverse_usage_url(
-                'container_handler',
-                course.location.course_key.make_usage_key(unit.location.block_type, unit.location.name)
+            usage_info = GroupConfiguration._get_usage_info(
+                course=course,
+                unit=unit,
+                item=split_test,
+                usage_info=usage_info,
+                group_id=split_test.user_partition_id,
+                scheme_name=RANDOM_SCHEME
             )
+        return usage_info
+
+    @staticmethod
+    def get_content_groups_usage_info(store, course):
+        """
+        Get usage information for content groups.
+        """
+        items = store.get_items(course.id, settings={'group_access': {'$exists': True}})
+
+        return GroupConfiguration._get_content_groups_usage_info(course, items)
+
+    @staticmethod
+    def _get_content_groups_usage_info(course, items):
+        """
+        Returns all units names and their urls.
+
+        Returns:
+        {'group_id':
+            [
+                {
+                    'label': 'Unit 1 / Problem 1',
+                    'url': 'url_to_unit_1'
+                },
+                {
+                    'label': 'Unit 2 / Problem 2',
+                    'url': 'url_to_unit_2'
+                }
+            ],
+        }
+        """
+        usage_info = {}
+        for item in items:
+            if hasattr(item, 'group_access') and item.group_access:
+                (__, group_ids), = item.group_access.items()
+                for group_id in group_ids:
+                    if group_id not in usage_info:
+                        usage_info[group_id] = []
+
+                    unit = item.get_parent()
+                    if not unit:
+                        log.warning("Unable to find parent for component %s", item.location)
+                        continue
+
+                    usage_info = GroupConfiguration._get_usage_info(
+                        course,
+                        unit=unit,
+                        item=item,
+                        usage_info=usage_info,
+                        group_id=group_id
+                    )
 
-            validation_summary = split_test.general_validation_message()
-            usage_info[split_test.user_partition_id].append({
-                'label': u"{} / {}".format(unit.display_name, split_test.display_name),
-                'url': unit_url,
-                'validation': validation_summary.to_json() if validation_summary else None,
-            })
         return usage_info
 
     @staticmethod
@@ -1473,19 +1538,39 @@ class GroupConfiguration(object):
 
         Returns json of particular group configuration updated with usage information.
         """
-        # Get all Experiments that use particular Group Configuration in course.
-        split_tests = store.get_items(
-            course.id,
-            category='split_test',
-            content={'user_partition_id': configuration.id}
-        )
-        configuration_json = configuration.to_json()
-        usage_information = GroupConfiguration._get_usage_info(store, course, split_tests)
-        configuration_json['usage'] = usage_information.get(configuration.id, [])
+        configuration_json = None
+        # Get all Experiments that use particular  Group Configuration in course.
+        if configuration.scheme.name == RANDOM_SCHEME:
+            split_tests = store.get_items(
+                course.id,
+                category='split_test',
+                content={'user_partition_id': configuration.id}
+            )
+            configuration_json = configuration.to_json()
+            usage_information = GroupConfiguration._get_content_experiment_usage_info(store, course, split_tests)
+            configuration_json['usage'] = usage_information.get(configuration.id, [])
+        elif configuration.scheme.name == COHORT_SCHEME:
+            # In case if scheme is "cohort"
+            configuration_json = GroupConfiguration.update_content_group_usage_info(store, course, configuration)
         return configuration_json
 
     @staticmethod
-    def get_or_create_content_group_configuration(course):
+    def update_content_group_usage_info(store, course, configuration):
+        """
+        Update usage information for particular Content Group Configuration.
+
+        Returns json of particular content group configuration updated with usage information.
+        """
+        usage_info = GroupConfiguration.get_content_groups_usage_info(store, course)
+        content_group_configuration = configuration.to_json()
+
+        for group in content_group_configuration['groups']:
+            group['usage'] = usage_info.get(group['id'], [])
+
+        return content_group_configuration
+
+    @staticmethod
+    def get_or_create_content_group(store, course):
         """
         Returns the first user partition from the course which uses the
         CohortPartitionScheme, or generates one if no such partition is
@@ -1500,11 +1585,60 @@ class GroupConfiguration(object):
                 name=CONTENT_GROUP_CONFIGURATION_NAME,
                 description=CONTENT_GROUP_CONFIGURATION_DESCRIPTION,
                 groups=[],
-                scheme_id='cohort'
+                scheme_id=COHORT_SCHEME
             )
+            return content_group_configuration.to_json()
+
+        content_group_configuration = GroupConfiguration.update_content_group_usage_info(
+            store,
+            course,
+            content_group_configuration
+        )
         return content_group_configuration
 
 
+def remove_content_or_experiment_group(request, store, course, configuration, group_configuration_id, group_id=None):
+    """
+    Remove content group or experiment group configuration only if it's not in use.
+    """
+    configuration_index = course.user_partitions.index(configuration)
+    if configuration.scheme.name == RANDOM_SCHEME:
+        usages = GroupConfiguration.get_content_experiment_usage_info(store, course)
+        used = int(group_configuration_id) in usages
+
+        if used:
+            return JsonResponse(
+                {"error": _("This group configuration is in use and cannot be deleted.")},
+                status=400
+            )
+        course.user_partitions.pop(configuration_index)
+    elif configuration.scheme.name == COHORT_SCHEME:
+        if not group_id:
+            return JsonResponse(status=404)
+
+        group_id = int(group_id)
+        usages = GroupConfiguration.get_content_groups_usage_info(store, course)
+        used = group_id in usages
+
+        if used:
+            return JsonResponse(
+                {"error": _("This content group is in use and cannot be deleted.")},
+                status=400
+            )
+
+        matching_groups = [group for group in configuration.groups if group.id == group_id]
+        if matching_groups:
+            group_index = configuration.groups.index(matching_groups[0])
+            configuration.groups.pop(group_index)
+        else:
+            return JsonResponse(status=404)
+
+        course.user_partitions[configuration_index] = configuration
+
+    store.update_item(course, request.user.id)
+    return JsonResponse(status=204)
+
+
 @require_http_methods(("GET", "POST"))
 @login_required
 @ensure_csrf_cookie
@@ -1527,12 +1661,12 @@ def group_configurations_list_handler(request, course_key_string):
             course_outline_url = reverse_course_url('course_handler', course_key)
             should_show_experiment_groups = are_content_experiments_enabled(course)
             if should_show_experiment_groups:
-                experiment_group_configurations = GroupConfiguration.get_split_test_partitions_with_usage(course, store)
+                experiment_group_configurations = GroupConfiguration.get_split_test_partitions_with_usage(store, course)
             else:
                 experiment_group_configurations = None
-            content_group_configuration = GroupConfiguration.get_or_create_content_group_configuration(
-                course
-            ).to_json()
+
+            content_group_configuration = GroupConfiguration.get_or_create_content_group(store, course)
+
             return render_to_response('group_configurations.html', {
                 'context_course': course,
                 'group_configuration_url': group_configuration_url,
@@ -1566,7 +1700,7 @@ def group_configurations_list_handler(request, course_key_string):
 @login_required
 @ensure_csrf_cookie
 @require_http_methods(("POST", "PUT", "DELETE"))
-def group_configurations_detail_handler(request, course_key_string, group_configuration_id):
+def group_configurations_detail_handler(request, course_key_string, group_configuration_id, group_id=None):
     """
     JSON API endpoint for manipulating a group configuration via its internal ID.
     Used by the Backbone application.
@@ -1600,22 +1734,19 @@ def group_configurations_detail_handler(request, course_key_string, group_config
             store.update_item(course, request.user.id)
             configuration = GroupConfiguration.update_usage_info(store, course, new_configuration)
             return JsonResponse(configuration, status=201)
+
         elif request.method == "DELETE":
             if not configuration:
                 return JsonResponse(status=404)
 
-            # Verify that group configuration is not already in use.
-            usages = GroupConfiguration.get_usage_info(course, store)
-            if usages.get(int(group_configuration_id)):
-                return JsonResponse(
-                    {"error": _("This Group Configuration is already in use and cannot be removed.")},
-                    status=400
-                )
-
-            index = course.user_partitions.index(configuration)
-            course.user_partitions.pop(index)
-            store.update_item(course, request.user.id)
-            return JsonResponse(status=204)
+            return remove_content_or_experiment_group(
+                request=request,
+                store=store,
+                course=course,
+                configuration=configuration,
+                group_configuration_id=group_configuration_id,
+                group_id=group_id
+            )
 
 
 def are_content_experiments_enabled(course):

cms/djangoapps/contentstore/views/tests/test_group_configurations.py

@@ -1,4 +1,5 @@
 #-*- coding: utf-8 -*-
+
 """
 Group Configuration Tests.
 """
@@ -86,16 +87,45 @@ class HelperMethods(object):
         self.save_course()
         return (vertical, split_test)
 
-    def _add_user_partitions(self, count=1):
+    def _create_problem_with_content_group(self, cid, group_id, name_suffix='', special_characters=''):
+        """
+        Create a problem
+        Assign content group to the problem.
+        """
+        vertical = ItemFactory.create(
+            category='vertical',
+            parent_location=self.course.location,
+            display_name="Test Unit {}".format(name_suffix)
+        )
+
+        problem = ItemFactory.create(
+            category='problem',
+            parent_location=vertical.location,
+            display_name=u"Test Problem {}{}".format(name_suffix, special_characters)
+        )
+
+        group_access_content = {'group_access': {cid: [group_id]}}
+
+        self.client.ajax_post(
+            reverse_usage_url("xblock_handler", problem.location),
+            data={'metadata': group_access_content}
+        )
+
+        self.save_course()
+
+        return vertical, problem
+
+    def _add_user_partitions(self, count=1, scheme_id="random"):
         """
         Create user partitions for the course.
         """
         partitions = [
             UserPartition(
-                i, 'Name ' + str(i), 'Description ' + str(i), [Group(0, 'Group A'), Group(1, 'Group B'), Group(2, 'Group C')]
+                i, 'Name ' + str(i), 'Description ' + str(i),
+                [Group(0, 'Group A'), Group(1, 'Group B'), Group(2, 'Group C')],
+                scheme=None, scheme_id=scheme_id
             ) for i in xrange(0, count)
         ]
-
         self.course.user_partitions = partitions
         self.save_course()
 
@@ -285,6 +315,144 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
             kwargs={'group_configuration_id': cid},
         )
 
+    def test_can_create_new_content_group_if_it_does_not_exist(self):
+        """
+        PUT new content group.
+        """
+        expected = {
+            u'id': 666,
+            u'name': u'Test name',
+            u'scheme': u'cohort',
+            u'description': u'Test description',
+            u'version': UserPartition.VERSION,
+            u'groups': [
+                {u'id': 0, u'name': u'Group A', u'version': 1, u'usage': []},
+                {u'id': 1, u'name': u'Group B', u'version': 1, u'usage': []},
+            ],
+        }
+        response = self.client.put(
+            self._url(cid=666),
+            data=json.dumps(expected),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+        )
+        content = json.loads(response.content)
+
+        self.assertEqual(content, expected)
+        self.reload_course()
+        # Verify that user_partitions in the course contains the new group configuration.
+        user_partitions = self.course.user_partitions
+        self.assertEqual(len(user_partitions), 1)
+        self.assertEqual(user_partitions[0].name, u'Test name')
+        self.assertEqual(len(user_partitions[0].groups), 2)
+        self.assertEqual(user_partitions[0].groups[0].name, u'Group A')
+        self.assertEqual(user_partitions[0].groups[1].name, u'Group B')
+
+    def test_can_edit_content_group(self):
+        """
+        Edit content group and check its id and modified fields.
+        """
+        self._add_user_partitions(scheme_id='cohort')
+        self.save_course()
+
+        expected = {
+            u'id': self.ID,
+            u'name': u'New Test name',
+            u'scheme': u'cohort',
+            u'description': u'New Test description',
+            u'version': UserPartition.VERSION,
+            u'groups': [
+                {u'id': 0, u'name': u'New Group Name', u'version': 1, u'usage': []},
+                {u'id': 2, u'name': u'Group C', u'version': 1, u'usage': []},
+            ],
+        }
+
+        response = self.client.put(
+            self._url(),
+            data=json.dumps(expected),
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+        )
+        content = json.loads(response.content)
+        self.assertEqual(content, expected)
+        self.reload_course()
+
+        # Verify that user_partitions is properly updated in the course.
+        user_partititons = self.course.user_partitions
+
+        self.assertEqual(len(user_partititons), 1)
+        self.assertEqual(user_partititons[0].name, u'New Test name')
+        self.assertEqual(len(user_partititons[0].groups), 2)
+        self.assertEqual(user_partititons[0].groups[0].name, u'New Group Name')
+        self.assertEqual(user_partititons[0].groups[1].name, u'Group C')
+
+    def test_can_delete_content_group(self):
+        """
+        Delete content group and check user partitions.
+        """
+        self._add_user_partitions(count=1, scheme_id='cohort')
+        self.save_course()
+
+        details_url_with_group_id = self._url(cid=0) + '/1'
+        response = self.client.delete(
+            details_url_with_group_id,
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+        )
+        self.assertEqual(response.status_code, 204)
+        self.reload_course()
+        # Verify that group and partition is properly updated in the course.
+        user_partititons = self.course.user_partitions
+        self.assertEqual(len(user_partititons), 1)
+        self.assertEqual(user_partititons[0].name, 'Name 0')
+        self.assertEqual(len(user_partititons[0].groups), 2)
+        self.assertEqual(user_partititons[0].groups[1].name, 'Group C')
+
+    def test_cannot_delete_used_content_group(self):
+        """
+        Cannot delete content group if it is in use.
+        """
+        self._add_user_partitions(count=1, scheme_id='cohort')
+        self._create_problem_with_content_group(cid=0, group_id=1)
+
+        details_url_with_group_id = self._url(cid=0) + '/1'
+        response = self.client.delete(
+            details_url_with_group_id,
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+        )
+        self.assertEqual(response.status_code, 400)
+        content = json.loads(response.content)
+        self.assertTrue(content['error'])
+        self.reload_course()
+        # Verify that user_partitions and groups are still the same.
+        user_partititons = self.course.user_partitions
+        self.assertEqual(len(user_partititons), 1)
+        self.assertEqual(len(user_partititons[0].groups), 3)
+        self.assertEqual(user_partititons[0].groups[1].name, 'Group B')
+
+    def test_cannot_delete_non_existent_content_group(self):
+        """
+        Cannot delete content group if it is doesn't exist.
+        """
+        self._add_user_partitions(count=1, scheme_id='cohort')
+        details_url_with_group_id = self._url(cid=0) + '/90'
+        response = self.client.delete(
+            details_url_with_group_id,
+            content_type="application/json",
+            HTTP_ACCEPT="application/json",
+            HTTP_X_REQUESTED_WITH="XMLHttpRequest",
+        )
+        self.assertEqual(response.status_code, 404)
+        # Verify that user_partitions is still the same.
+        user_partititons = self.course.user_partitions
+        self.assertEqual(len(user_partititons), 1)
+        self.assertEqual(len(user_partititons[0].groups), 3)
+
     def test_can_create_new_group_configuration_if_it_does_not_exist(self):
         """
         PUT new group configuration when no configurations exist in the course.
@@ -423,18 +591,107 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
 # pylint: disable=no-member
 class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
     """
-    Tests for usage information of configurations.
+    Tests for usage information of configurations and content groups.
     """
 
     def setUp(self):
         super(GroupConfigurationsUsageInfoTestCase, self).setUp()
 
+    def _get_expected_content_group(self, usage_for_group):
+        """
+        Returns the expected configuration with particular usage.
+        """
+        return {
+            'id': 0,
+            'name': 'Name 0',
+            'scheme': 'cohort',
+            'description': 'Description 0',
+            'version': UserPartition.VERSION,
+            'groups': [
+                {'id': 0, 'name': 'Group A', 'version': 1, 'usage': []},
+                {'id': 1, 'name': 'Group B', 'version': 1, 'usage': usage_for_group},
+                {'id': 2, 'name': 'Group C', 'version': 1, 'usage': []},
+            ],
+        }
+
+    def test_content_group_not_used(self):
+        """
+        Test that right data structure will be created if content group is not used.
+        """
+        self._add_user_partitions(scheme_id='cohort')
+        actual = GroupConfiguration.get_or_create_content_group(self.store, self.course)
+        expected = self._get_expected_content_group(usage_for_group=[])
+        self.assertEqual(actual, expected)
+
+    def test_can_get_correct_usage_info_when_special_characters_are_in_content(self):
+        """
+        Test if content group json updated successfully with usage information.
+        """
+        self._add_user_partitions(count=1, scheme_id='cohort')
+        vertical, __ = self._create_problem_with_content_group(
+            cid=0, group_id=1, name_suffix='0', special_characters=u"JOSÉ ANDRÉS"
+        )
+
+        actual = GroupConfiguration.get_or_create_content_group(self.store, self.course)
+        expected = self._get_expected_content_group(
+            usage_for_group=[
+                {
+                    'url': u"/container/{}".format(vertical.location),
+                    'label': u"Test Unit 0 / Test Problem 0JOSÉ ANDRÉS"
+                }
+            ]
+        )
+
+        self.assertEqual(actual, expected)
+
+    def test_can_get_correct_usage_info_for_content_groups(self):
+        """
+        Test if content group json updated successfully with usage information.
+        """
+        self._add_user_partitions(count=1, scheme_id='cohort')
+        vertical, __ = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='0')
+
+        actual = GroupConfiguration.get_or_create_content_group(self.store, self.course)
+
+        expected = self._get_expected_content_group(usage_for_group=[
+            {
+                'url': '/container/{}'.format(vertical.location),
+                'label': 'Test Unit 0 / Test Problem 0'
+            }
+        ])
+
+        self.assertEqual(actual, expected)
+
+    def test_can_use_one_content_group_in_multiple_problems(self):
+        """
+        Test if multiple problems are present in usage info when they use same
+        content group.
+        """
+        self._add_user_partitions(scheme_id='cohort')
+        vertical, __ = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='0')
+        vertical1, __ = self._create_problem_with_content_group(cid=0, group_id=1, name_suffix='1')
+
+        actual = GroupConfiguration.get_or_create_content_group(self.store, self.course)
+
+        expected = self._get_expected_content_group(usage_for_group=[
+            {
+                'url': '/container/{}'.format(vertical.location),
+                'label': 'Test Unit 0 / Test Problem 0'
+            },
+            {
+                'url': '/container/{}'.format(vertical1.location),
+                'label': 'Test Unit 1 / Test Problem 1'
+            }
+        ])
+
+        self.assertEqual(actual, expected)
+
     def test_group_configuration_not_used(self):
         """
         Test that right data structure will be created if group configuration is not used.
         """
         self._add_user_partitions()
-        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)
+        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course)
         expected = [{
             'id': 0,
             'name': 'Name 0',
@@ -458,7 +715,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
         vertical, __ = self._create_content_experiment(cid=0, name_suffix='0')
         self._create_content_experiment(name_suffix='1')
 
-        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)
+        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course)
 
         expected = [{
             'id': 0,
@@ -500,7 +757,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
         self._add_user_partitions(count=1)
         vertical, __ = self._create_content_experiment(cid=0, name_suffix='0', special_characters=u"JOSÉ ANDRÉS")
 
-        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)
+        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course, )
 
         expected = [{
             'id': 0,
@@ -531,7 +788,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
         vertical, __ = self._create_content_experiment(cid=0, name_suffix='0')
         vertical1, __ = self._create_content_experiment(cid=0, name_suffix='1')
 
-        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)
+        actual = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course)
 
         expected = [{
             'id': 0,
@@ -572,7 +829,7 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
             modulestore().update_item(orphan, ModuleStoreEnum.UserID.test)
 
         self.save_course()
-        actual = GroupConfiguration.get_usage_info(self.course, self.store)
+        actual = GroupConfiguration.get_content_experiment_usage_info(self.store, self.course)
         self.assertEqual(actual, {0: []})
 
 
@@ -595,7 +852,7 @@ class GroupConfigurationsValidationTestCase(CourseTestCase, HelperMethods):
         validation.add(mocked_message)
         mocked_validation_messages.return_value = validation
 
-        group_configuration = GroupConfiguration.get_split_test_partitions_with_usage(self.course, self.store)[0]
+        group_configuration = GroupConfiguration.get_split_test_partitions_with_usage(self.store, self.course)[0]
         self.assertEqual(expected_result.to_json(), group_configuration['usage'][0]['validation'])
 
     def test_error_message_present(self):

cms/djangoapps/contentstore/views/xblock.py

@@ -25,7 +25,7 @@ def xblock_resource(request, block_type, uri):  # pylint: disable=unused-argumen
     except IOError:
         log.info('Failed to load xblock resource', exc_info=True)
         raise Http404
-    except Exception:  # pylint: disable-msg=broad-except
+    except Exception:  # pylint: disable=broad-except
         log.error('Failed to load xblock resource', exc_info=True)
         raise Http404
 

cms/envs/aws.py

@@ -144,6 +144,7 @@ if 'loc_cache' not in CACHES:
     }
 
 SESSION_COOKIE_DOMAIN = ENV_TOKENS.get('SESSION_COOKIE_DOMAIN')
+SESSION_COOKIE_HTTPONLY = ENV_TOKENS.get('SESSION_COOKIE_HTTPONLY', True)
 SESSION_ENGINE = ENV_TOKENS.get('SESSION_ENGINE', SESSION_ENGINE)
 SESSION_COOKIE_SECURE = ENV_TOKENS.get('SESSION_COOKIE_SECURE', SESSION_COOKIE_SECURE)
 

cms/static/js/factories/group_configurations.js

@@ -14,6 +14,7 @@ define([
         experimentGroupConfigurations.url = groupConfigurationUrl;
         experimentGroupConfigurations.outlineUrl = courseOutlineUrl;
         contentGroupConfiguration.urlRoot = groupConfigurationUrl;
+        contentGroupConfiguration.outlineUrl = courseOutlineUrl;
         new GroupConfigurationsPage({
             el: $('#content'),
             experimentsEnabled: experimentsEnabled,

cms/static/js/models/group.js

@@ -8,8 +8,17 @@ define([
             return {
                 name: '',
                 version: 1,
-                order: null
-             };
+                order: null,
+                usage: []
+            };
+        },
+        url : function() {
+            var parentModel = this.collection.parents[0];
+            return parentModel.urlRoot + '/' + encodeURIComponent(parentModel.id) + '/' + encodeURIComponent(this.id);
+        },
+
+        reset: function() {
+            this.set(this._originalAttributes, { parse: true });
         },
 
         isEmpty: function() {
@@ -20,7 +29,8 @@ define([
             return {
                 id: this.get('id'),
                 name: this.get('name'),
-                version: this.get('version')
+                version: this.get('version'),
+                usage: this.get('usage')
              };
         },
 

cms/static/js/spec/models/group_configuration_spec.js

@@ -106,10 +106,12 @@ define([
                         'groups': [
                             {
                                 'version': 1,
-                                'name': 'Group 1'
+                                'name': 'Group 1',
+                                'usage': []
                             }, {
                                 'version': 1,
-                                'name': 'Group 2'
+                                'name': 'Group 2',
+                                'usage': []
                             }
                         ]
                     },
@@ -125,11 +127,13 @@ define([
                             {
                                 'version': 1,
                                 'order': 0,
-                                'name': 'Group 1'
+                                'name': 'Group 1',
+                                'usage': []
                             }, {
                                 'version': 1,
                                 'order': 1,
-                                'name': 'Group 2'
+                                'name': 'Group 2',
+                                'usage': []
                             }
                         ],
                         'usage': []

cms/static/js/spec/video/transcripts/file_uploader_spec.js

@@ -5,7 +5,8 @@ define(
         "xmodule", "jquery.form", "jasmine-jquery"
     ],
 function ($, _, Utils, FileUploader) {
-    describe('Transcripts.FileUploader', function () {
+    // TODO: fix TNL-559 Intermittent failures of Transcript FileUploader JS tests
+    xdescribe('Transcripts.FileUploader', function () {
         var videoListEntryTemplate = readFixtures(
                 'video/transcripts/metadata-videolist-entry.underscore'
             ),

cms/static/js/spec/video/transcripts/message_manager_spec.js

@@ -7,7 +7,8 @@ define(
     ],
 function ($, _, Utils, MessageManager, FileUploader, sinon) {
 
-    describe('Transcripts.MessageManager', function () {
+    // TODO: fix TNL-559 Intermittent failures of Transcript FileUploader JS tests
+    xdescribe('Transcripts.MessageManager', function () {
         var videoListEntryTemplate = readFixtures(
                 'video/transcripts/metadata-videolist-entry.underscore'
             ),

cms/static/js/spec/views/group_configuration_spec.js

@@ -3,13 +3,14 @@ define([
     'js/collections/group_configuration', 'js/collections/group',
     'js/views/group_configuration_details', 'js/views/group_configurations_list', 'js/views/group_configuration_editor',
     'js/views/group_configuration_item', 'js/views/experiment_group_edit', 'js/views/content_group_list',
+    'js/views/content_group_details', 'js/views/content_group_editor', 'js/views/content_group_item',
     'js/views/feedback_notification', 'js/common_helpers/ajax_helpers', 'js/common_helpers/template_helpers',
     'js/spec_helpers/view_helpers', 'jasmine-stealth'
 ], function(
     _, Course, GroupConfigurationModel, GroupModel, GroupConfigurationCollection, GroupCollection,
     GroupConfigurationDetailsView, GroupConfigurationsListView, GroupConfigurationEditorView,
-    GroupConfigurationItemView, ExperimentGroupEditView, GroupList, Notification, AjaxHelpers, TemplateHelpers,
-    ViewHelpers
+    GroupConfigurationItemView, ExperimentGroupEditView, GroupList, ContentGroupDetailsView,
+    ContentGroupEditorView, ContentGroupItemView, Notification, AjaxHelpers, TemplateHelpers, ViewHelpers
 ) {
     'use strict';
     var SELECTORS = {
@@ -40,6 +41,134 @@ define([
         note: '.wrapper-delete-button'
     };
 
+    var assertTheDetailsView = function (view, text) {
+        expect(view.$el).toContainText(text);
+        expect(view.$el).toContainText('ID: 0');
+        expect(view.$('.delete')).toExist();
+    };
+    var assertShowEmptyUsages = function (view, usageText) {
+        expect(view.$(SELECTORS.usageCount)).not.toExist();
+        expect(view.$(SELECTORS.usageText)).toContainText(usageText);
+        expect(view.$(SELECTORS.usageTextAnchor)).toExist();
+        expect(view.$(SELECTORS.usageUnit)).not.toExist();
+    };
+    var assertHideEmptyUsages = function (view) {
+        expect(view.$(SELECTORS.usageText)).not.toExist();
+        expect(view.$(SELECTORS.usageUnit)).not.toExist();
+        expect(view.$(SELECTORS.usageCount)).toContainText('Not in Use');
+    };
+    var assertShowNonEmptyUsages = function (view, usageText, toolTipText) {
+        var usageUnitAnchors = view.$(SELECTORS.usageUnitAnchor);
+
+        expect(view.$(SELECTORS.note)).toHaveAttr(
+            'data-tooltip', toolTipText
+        );
+        expect(view.$('.delete')).toHaveClass('is-disabled');
+        expect(view.$(SELECTORS.usageCount)).not.toExist();
+        expect(view.$(SELECTORS.usageText)).toContainText(usageText);
+        expect(view.$(SELECTORS.usageUnit).length).toBe(2);
+        expect(usageUnitAnchors.length).toBe(2);
+        expect(usageUnitAnchors.eq(0)).toContainText('label1');
+        expect(usageUnitAnchors.eq(0).attr('href')).toBe('url1');
+        expect(usageUnitAnchors.eq(1)).toContainText('label2');
+        expect(usageUnitAnchors.eq(1).attr('href')).toBe('url2');
+    };
+    var assertHideNonEmptyUsages = function (view) {
+        expect(view.$('.delete')).toHaveClass('is-disabled');
+        expect(view.$(SELECTORS.usageText)).not.toExist();
+        expect(view.$(SELECTORS.usageUnit)).not.toExist();
+        expect(view.$(SELECTORS.usageCount)).toContainText('Used in 2 units');
+    };
+    var setUsageInfo = function (model) {
+        model.set('usage', [
+            {'label': 'label1', 'url': 'url1'},
+            {'label': 'label2', 'url': 'url2'}
+        ]);
+    };
+    var assertHideValidationContent = function (view) {
+        expect(view.$(SELECTORS.usageUnitMessage)).not.toExist();
+        expect(view.$(SELECTORS.usageUnitWarningIcon)).not.toExist();
+        expect(view.$(SELECTORS.usageUnitErrorIcon)).not.toExist();
+    };
+    var assertControllerView = function (view, detailsView, editView) {
+        // Details view by default
+        expect(view.$(detailsView)).toExist();
+        view.$('.action-edit .edit').click();
+        expect(view.$(editView)).toExist();
+        expect(view.$(detailsView)).not.toExist();
+        view.$('.action-cancel').click();
+        expect(view.$(detailsView)).toExist();
+        expect(view.$(editView)).not.toExist();
+    };
+    var clickDeleteItem = function (that, promptSpy, promptText) {
+        that.view.$('.delete').click();
+        ViewHelpers.verifyPromptShowing(promptSpy, promptText);
+        ViewHelpers.confirmPrompt(promptSpy);
+        ViewHelpers.verifyPromptHidden(promptSpy);
+    };
+    var patchAndVerifyRequest = function (requests, url, notificationSpy) {
+        // Backbone.emulateHTTP is enabled in our system, so setting this
+        // option  will fake PUT, PATCH and DELETE requests with a HTTP POST,
+        // setting the X-HTTP-Method-Override header with the true method.
+        AjaxHelpers.expectJsonRequest(requests, 'POST', url);
+        expect(_.last(requests).requestHeaders['X-HTTP-Method-Override']).toBe('DELETE');
+        ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/);
+    };
+    var assertAndDeleteItemError = function (that, url, promptText) {
+        var requests = AjaxHelpers.requests(that),
+            promptSpy = ViewHelpers.createPromptSpy(),
+            notificationSpy = ViewHelpers.createNotificationSpy();
+
+        clickDeleteItem(that, promptSpy, promptText);
+
+        patchAndVerifyRequest(requests, url, notificationSpy);
+
+        AjaxHelpers.respondToDelete(requests);
+        ViewHelpers.verifyNotificationHidden(notificationSpy);
+        expect($(SELECTORS.itemView)).not.toExist();
+    };
+    var assertAndDeleteItemWithError = function (that, url, listItemView, promptText) {
+        var requests = AjaxHelpers.requests(that),
+            promptSpy = ViewHelpers.createPromptSpy(),
+            notificationSpy = ViewHelpers.createNotificationSpy();
+
+        clickDeleteItem(that, promptSpy, promptText);
+        patchAndVerifyRequest(requests, url, notificationSpy);
+
+        AjaxHelpers.respondWithError(requests);
+        ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/);
+        expect($(listItemView)).toExist();
+    };
+    var submitAndVerifyFormSuccess = function (view, requests, notificationSpy) {
+        view.$('form').submit();
+        ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/);
+        requests[0].respond(200);
+        ViewHelpers.verifyNotificationHidden(notificationSpy);
+    };
+    var submitAndVerifyFormError = function (view, requests, notificationSpy) {
+        view.$('form').submit();
+        ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/);
+        AjaxHelpers.respondWithError(requests);
+        ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/);
+    };
+    var assertCannotDeleteUsed = function (that, toolTipText, warningText){
+        setUsageInfo(that.model);
+        that.view.render();
+        expect(that.view.$(SELECTORS.note)).toHaveAttr(
+            'data-tooltip', toolTipText
+        );
+        expect(that.view.$(SELECTORS.warningMessage)).toContainText(warningText);
+        expect(that.view.$(SELECTORS.warningIcon)).toExist();
+        expect(that.view.$('.delete')).toHaveClass('is-disabled');
+    };
+    var assertUnusedOptions = function (that) {
+        that.model.set('usage', []);
+        that.view.render();
+        expect(that.view.$(SELECTORS.warningMessage)).not.toExist();
+        expect(that.view.$(SELECTORS.warningIcon)).not.toExist();
+    };
+
+
     beforeEach(function() {
         window.course = new Course({
             id: '5',
@@ -107,9 +236,7 @@ define([
         });
 
         it('should render properly', function() {
-            expect(this.view.$el).toContainText('Configuration');
-            expect(this.view.$el).toContainText('ID: 0');
-            expect(this.view.$('.delete')).toExist();
+            assertTheDetailsView(this.view, 'Configuration');
         });
 
         it('should show groups appropriately', function() {
@@ -142,69 +269,40 @@ define([
         it('should show empty usage appropriately', function() {
             this.model.set('showGroups', false);
             this.view.$('.show-groups').click();
-
-            expect(this.view.$(SELECTORS.usageCount)).not.toExist();
-            expect(this.view.$(SELECTORS.usageText))
-                .toContainText('This Group Configuration is not in use. ' +
-                               'Start by adding a content experiment to any ' +
-                               'Unit via the');
-            expect(this.view.$(SELECTORS.usageTextAnchor)).toExist();
-            expect(this.view.$(SELECTORS.usageUnit)).not.toExist();
+            assertShowEmptyUsages(
+                this.view,
+                'This Group Configuration is not in use. ' +
+                'Start by adding a content experiment to any Unit via the'
+            );
         });
 
         it('should hide empty usage appropriately', function() {
             this.model.set('showGroups', true);
             this.view.$('.hide-groups').click();
-
-            expect(this.view.$(SELECTORS.usageText)).not.toExist();
-            expect(this.view.$(SELECTORS.usageUnit)).not.toExist();
-            expect(this.view.$(SELECTORS.usageCount))
-                .toContainText('Not in Use');
+            assertHideEmptyUsages(this.view)
         });
 
         it('should show non-empty usage appropriately', function() {
-            var usageUnitAnchors;
-
-            this.model.set('usage', [
-                {'label': 'label1', 'url': 'url1'},
-                {'label': 'label2', 'url': 'url2'}
-            ]);
+            setUsageInfo(this.model);
             this.model.set('showGroups', false);
             this.view.$('.show-groups').click();
 
-            usageUnitAnchors = this.view.$(SELECTORS.usageUnitAnchor);
-
-            expect(this.view.$(SELECTORS.note)).toHaveAttr(
-                'data-tooltip', 'Cannot delete when in use by an experiment'
-            );
-            expect(this.view.$('.delete')).toHaveClass('is-disabled');
-            expect(this.view.$(SELECTORS.usageCount)).not.toExist();
-            expect(this.view.$(SELECTORS.usageText))
-                .toContainText('This Group Configuration is used in:');
-            expect(this.view.$(SELECTORS.usageUnit).length).toBe(2);
-            expect(usageUnitAnchors.length).toBe(2);
-            expect(usageUnitAnchors.eq(0)).toContainText('label1');
-            expect(usageUnitAnchors.eq(0).attr('href')).toBe('url1');
-            expect(usageUnitAnchors.eq(1)).toContainText('label2');
-            expect(usageUnitAnchors.eq(1).attr('href')).toBe('url2');
+            assertShowNonEmptyUsages(
+                this.view,
+                'This Group Configuration is used in:',
+                'Cannot delete when in use by an experiment'
+            )
         });
 
         it('should hide non-empty usage appropriately', function() {
-            this.model.set('usage', [
-                {'label': 'label1', 'url': 'url1'},
-                {'label': 'label2', 'url': 'url2'}
-            ]);
+            setUsageInfo(this.model);
             this.model.set('showGroups', true);
             this.view.$('.hide-groups').click();
 
             expect(this.view.$(SELECTORS.note)).toHaveAttr(
                 'data-tooltip', 'Cannot delete when in use by an experiment'
             );
-            expect(this.view.$('.delete')).toHaveClass('is-disabled');
-            expect(this.view.$(SELECTORS.usageText)).not.toExist();
-            expect(this.view.$(SELECTORS.usageUnit)).not.toExist();
-            expect(this.view.$(SELECTORS.usageCount))
-                .toContainText('Used in 2 units');
+            assertHideNonEmptyUsages(this.view);
         });
 
         it('should show validation warning icon and message appropriately', function() {
@@ -244,16 +342,11 @@ define([
         });
 
         it('should hide validation icons and messages appropriately', function() {
-            this.model.set('usage', [
-                {'label': 'label1', 'url': 'url1'},
-                {'label': 'label2', 'url': 'url2'}
-            ]);
+            setUsageInfo(this.model);
             this.model.set('showGroups', true);
             this.view.$('.hide-groups').click();
 
-            expect(this.view.$(SELECTORS.usageUnitMessage)).not.toExist();
-            expect(this.view.$(SELECTORS.usageUnitWarningIcon)).not.toExist();
-            expect(this.view.$(SELECTORS.usageUnitErrorIcon)).not.toExist();
+            assertHideValidationContent(this.view);
         });
     });
 
@@ -312,10 +405,7 @@ define([
                 inputDescription: 'New Description'
             });
 
-            this.view.$('form').submit();
-            ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/);
-            requests[0].respond(200);
-            ViewHelpers.verifyNotificationHidden(notificationSpy);
+            submitAndVerifyFormSuccess(this.view, requests, notificationSpy);
 
             expect(this.model).toBeCorrectValuesInModel({
                 name: 'New Configuration',
@@ -333,10 +423,7 @@ define([
                 notificationSpy = ViewHelpers.createNotificationSpy();
 
             setValuesToInputs(this.view, { inputName: 'New Configuration' });
-            this.view.$('form').submit();
-            ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/);
-            AjaxHelpers.respondWithError(requests);
-            ViewHelpers.verifyNotificationShowing(notificationSpy, /Saving/);
+            submitAndVerifyFormError(this.view, requests, notificationSpy);
         });
 
         it('does not save on cancel', function() {
@@ -379,7 +466,7 @@ define([
             this.view.$('form').submit();
             // See error message
             expect(this.view.$(SELECTORS.errorMessage)).toContainText(
-                'Group Configuration name is required.'
+                'Group Configuration name is required'
             );
             // No request
             expect(requests.length).toBe(0);
@@ -461,30 +548,17 @@ define([
         });
 
         it('cannot be deleted if it is in use', function () {
-            this.model.set('usage', [ {'label': 'label1', 'url': 'url1'} ]);
-            this.view.render();
-            expect(this.view.$(SELECTORS.note)).toHaveAttr(
-                'data-tooltip', 'Cannot delete when in use by an experiment'
-            );
-            expect(this.view.$('.delete')).toHaveClass('is-disabled');
-        });
-
-        it('contains warning message if it is in use', function () {
-            this.model.set('usage', [ {'label': 'label1', 'url': 'url1'} ]);
-            this.view.render();
-            expect(this.view.$(SELECTORS.warningMessage)).toContainText(
+            assertCannotDeleteUsed(
+                this,
+                'Cannot delete when in use by an experiment',
                 'This configuration is currently used in content ' +
                 'experiments. If you make changes to the groups, you may ' +
                 'need to edit those experiments.'
             );
-            expect(this.view.$(SELECTORS.warningIcon)).toExist();
         });
 
         it('does not contain warning message if it is not in use', function () {
-            this.model.set('usage', []);
-            this.view.render();
-            expect(this.view.$(SELECTORS.warningMessage)).not.toExist();
-            expect(this.view.$(SELECTORS.warningIcon)).not.toExist();
+           assertUnusedOptions(this);
         });
     });
 
@@ -535,7 +609,6 @@ define([
     });
 
     describe('Experiment group configurations controller view', function() {
-        var clickDeleteItem;
 
         beforeEach(function() {
             TemplateHelpers.installTemplates([
@@ -550,56 +623,21 @@ define([
             appendSetFixtures(this.view.render().el);
         });
 
-        clickDeleteItem = function (view, promptSpy) {
-            view.$('.delete').click();
-            ViewHelpers.verifyPromptShowing(promptSpy, /Delete this group configuration/);
-            ViewHelpers.confirmPrompt(promptSpy);
-            ViewHelpers.verifyPromptHidden(promptSpy);
-        };
-
         it('should render properly', function() {
-            // Details view by default
-            expect(this.view.$(SELECTORS.detailsView)).toExist();
-            this.view.$('.action-edit .edit').click();
-            expect(this.view.$(SELECTORS.editView)).toExist();
-            expect(this.view.$(SELECTORS.detailsView)).not.toExist();
-            this.view.$('.action-cancel').click();
-            expect(this.view.$(SELECTORS.detailsView)).toExist();
-            expect(this.view.$(SELECTORS.editView)).not.toExist();
+            assertControllerView(this.view, SELECTORS.detailsView, SELECTORS.editView);
         });
 
         it('should destroy itself on confirmation of deleting', function () {
-            var requests = AjaxHelpers.requests(this),
-                promptSpy = ViewHelpers.createPromptSpy(),
-                notificationSpy = ViewHelpers.createNotificationSpy();
-
-            clickDeleteItem(this.view, promptSpy);
-            // Backbone.emulateHTTP is enabled in our system, so setting this
-            // option  will fake PUT, PATCH and DELETE requests with a HTTP POST,
-            // setting the X-HTTP-Method-Override header with the true method.
-            AjaxHelpers.expectJsonRequest(requests, 'POST', '/group_configurations/0');
-            expect(_.last(requests).requestHeaders['X-HTTP-Method-Override']).toBe('DELETE');
-            ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/);
-            AjaxHelpers.respondToDelete(requests);
-            ViewHelpers.verifyNotificationHidden(notificationSpy);
-            expect($(SELECTORS.itemView)).not.toExist();
+            assertAndDeleteItemError(this, '/group_configurations/0', 'Delete this group configuration?');
         });
 
         it('does not hide deleting message if failure', function() {
-            var requests = AjaxHelpers.requests(this),
-                promptSpy = ViewHelpers.createPromptSpy(),
-                notificationSpy = ViewHelpers.createNotificationSpy();
-
-            clickDeleteItem(this.view, promptSpy);
-            // Backbone.emulateHTTP is enabled in our system, so setting this
-            // option  will fake PUT, PATCH and DELETE requests with a HTTP POST,
-            // setting the X-HTTP-Method-Override header with the true method.
-            AjaxHelpers.expectJsonRequest(requests, 'POST', '/group_configurations/0');
-            expect(_.last(requests).requestHeaders['X-HTTP-Method-Override']).toBe('DELETE');
-            ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/);
-            AjaxHelpers.respondWithError(requests);
-            ViewHelpers.verifyNotificationShowing(notificationSpy, /Deleting/);
-            expect($(SELECTORS.itemView)).toExist();
+            assertAndDeleteItemWithError(
+                this,
+                '/group_configurations/0',
+                SELECTORS.itemView,
+                'Delete this group configuration?'
+            );
         });
     });
 
@@ -651,9 +689,9 @@ define([
             }
         };
 
-        createGroups = function (groupNames) {
-            var groups = new GroupCollection(_.map(groupNames, function (groupName) {
-                    return {name: groupName};
+        createGroups = function (groupNamesWithId) {
+            var groups = new GroupCollection(_.map(groupNamesWithId, function (groupName, id) {
+                    return {id: id, name: groupName};
                 })),
                 groupConfiguration = new GroupConfigurationModel({
                     id: 0,
@@ -661,11 +699,12 @@ define([
                     groups: groups
                 }, {canBeEmpty: true});
             groupConfiguration.urlRoot = '/mock_url';
+            groupConfiguration.outlineUrl = '/mock_url';
             return groups;
         };
 
-        renderView = function(groupNames) {
-            var view = new GroupList({collection: createGroups(groupNames || [])}).render();
+        renderView = function(groupNamesWithId) {
+            var view = new GroupList({collection: createGroups(groupNamesWithId || {})}).render();
             appendSetFixtures(view.el);
             return view;
         };
@@ -778,7 +817,7 @@ define([
             var requests = AjaxHelpers.requests(this),
                 oldGroupName = 'Old Group Name',
                 newGroupName = 'New Group Name',
-                view = renderView([oldGroupName]);
+                view = renderView({1: oldGroupName});
             editNewGroup(view, {newName: newGroupName, save: true});
             respondToSave(requests, view);
             verifyEditingGroup(view, false, 1);
@@ -837,5 +876,189 @@ define([
             view.collection.add({name: 'Editing Group', editing: true});
             verifyEditingGroup(view, true);
         });
+
+    });
+
+    describe('Content groups details view', function() {
+
+        beforeEach(function() {
+            TemplateHelpers.installTemplate('content-group-details', true);
+            this.model = new GroupModel({name: 'Content Group', id: 0});
+
+            var saveableModel = new GroupConfigurationModel({
+                name: 'Content Group Configuration',
+                id: 0,
+                scheme:'cohort',
+                groups: new GroupCollection([this.model]),
+            }, {canBeEmpty: true});
+
+            saveableModel.urlRoot = '/mock_url';
+
+            this.collection = new GroupConfigurationCollection([ saveableModel ]);
+            this.collection.outlineUrl = '/outline';
+
+            this.view = new ContentGroupDetailsView({
+                model: this.model
+            });
+            appendSetFixtures(this.view.render().el);
+        });
+
+        it('should render properly', function() {
+            assertTheDetailsView(this.view, 'Content Group');
+        });
+
+        it('should show empty usage appropriately', function() {
+            this.view.$('.show-groups').click();
+            assertShowEmptyUsages(this.view, 'This content group is not in use. ');
+        });
+
+        it('should hide empty usage appropriately', function() {
+            this.view.$('.hide-groups').click();
+            assertHideEmptyUsages(this.view)
+        });
+
+        it('should show non-empty usage appropriately', function() {
+            setUsageInfo(this.model);
+            this.view.$('.show-groups').click();
+
+            assertShowNonEmptyUsages(
+                this.view,
+                'This content group is used in:',
+                'Cannot delete when in use by a unit'
+            )
+        });
+
+        it('should hide non-empty usage appropriately', function() {
+            setUsageInfo(this.model);
+            this.view.$('.hide-groups').click();
+
+            expect(this.view.$('li.action-delete')).toHaveAttr(
+                'data-tooltip', 'Cannot delete when in use by a unit'
+            );
+            assertHideNonEmptyUsages(this.view);
+        });
+
+        it('should hide validation icons and messages appropriately', function() {
+            setUsageInfo(this.model);
+            this.view.$('.hide-groups').click();
+            assertHideValidationContent(this.view);
+        });
+    });
+
+    describe('Content groups editor view', function() {
+
+        beforeEach(function() {
+            ViewHelpers.installViewTemplates();
+            TemplateHelpers.installTemplates(['content-group-editor']);
+
+            this.model = new GroupModel({name: 'Content Group', id: 0});
+
+            this.saveableModel = new GroupConfigurationModel({
+                name: 'Content Group Configuration',
+                id: 0,
+                scheme:'cohort',
+                groups: new GroupCollection([this.model]),
+                editing:true
+            });
+
+            this.collection = new GroupConfigurationCollection([ this.saveableModel ]);
+            this.collection.outlineUrl = '/outline';
+            this.collection.url = '/group_configurations';
+
+            this.view = new ContentGroupEditorView({
+                model: this.model
+            });
+            appendSetFixtures(this.view.render().el);
+        });
+
+        it('should save properly', function() {
+            var requests = AjaxHelpers.requests(this),
+                notificationSpy = ViewHelpers.createNotificationSpy();
+
+            this.view.$('.action-add').click();
+            this.view.$(SELECTORS.inputName).val('New Content Group');
+
+            submitAndVerifyFormSuccess(this.view, requests, notificationSpy);
+
+            expect(this.model).toBeCorrectValuesInModel({
+                name: 'New Content Group'
+            });
+            expect(this.view.$el).not.toExist();
+        });
+
+        it('does not hide saving message if failure', function() {
+            var requests = AjaxHelpers.requests(this),
+                notificationSpy = ViewHelpers.createNotificationSpy();
+            this.view.$(SELECTORS.inputName).val('New Content Group')
+
+            submitAndVerifyFormError(this.view, requests, notificationSpy)
+        });
+
+        it('does not save on cancel', function() {
+            expect(this.view.$('.action-add'));
+            this.view.$('.action-add').click();
+            this.view.$(SELECTORS.inputName).val('New Content Group');
+
+            this.view.$('.action-cancel').click();
+            expect(this.model).toBeCorrectValuesInModel({
+                name: 'Content Group',
+            });
+            // Model is still exist in the collection
+            expect(this.collection.indexOf(this.saveableModel)).toBeGreaterThan(-1);
+            expect(this.collection.length).toBe(1);
+        });
+
+        it('cannot be deleted if it is in use', function () {
+            assertCannotDeleteUsed(
+                this,
+                'Cannot delete when in use by a unit',
+                'This content group is used in one or more units.'
+            );
+        });
+
+        it('does not contain warning message if it is not in use', function () {
+            assertUnusedOptions(this);
+        });
+    });
+
+    describe('Content group controller view', function() {
+        beforeEach(function() {
+            TemplateHelpers.installTemplates([
+                'content-group-editor', 'content-group-details'
+            ], true);
+
+            this.model = new GroupModel({name: 'Content Group', id: 0});
+
+            this.saveableModel = new GroupConfigurationModel({
+                name: 'Content Group Configuration',
+                id: 0,
+                scheme:'cohort',
+                groups: new GroupCollection([this.model])
+            });
+            this.saveableModel.urlRoot = '/group_configurations';
+            this.collection = new GroupConfigurationCollection([ this.saveableModel ]);
+            this.collection.url = '/group_configurations';
+            this.view = new ContentGroupItemView({
+                model: this.model
+            });
+            appendSetFixtures(this.view.render().el);
+        });
+
+        it('should render properly', function() {
+            assertControllerView(this.view, '.content-group-details', '.content-group-edit');
+        });
+
+        it('should destroy itself on confirmation of deleting', function () {
+            assertAndDeleteItemError(this, '/group_configurations/0/0', 'Delete this content group');
+        });
+
+        it('does not hide deleting message if failure', function() {
+            assertAndDeleteItemWithError(
+                this,
+                '/group_configurations/0/0',
+                '.content-groups-list-item',
+                'Delete this content group'
+            );
+        });
     });
 });

cms/static/js/spec/views/pages/group_configurations_spec.js

@@ -108,7 +108,7 @@ define([
             });
 
             it('should show a notification message if a content group is changed', function () {
-                this.view.contentGroupConfiguration.get('groups').add({name: 'Content Group'});
+                this.view.contentGroupConfiguration.get('groups').add({id: 0, name: 'Content Group'});
                 expect(this.view.onBeforeUnload())
                     .toBe('You have unsaved changes. Do you really want to leave this page?');
             });

cms/static/js/views/content_group_details.js

@@ -3,16 +3,26 @@
  * It is expected to be backed by a Group model.
  */
 define([
-    'js/views/baseview'
-], function(BaseView) {
+    'js/views/baseview', 'underscore', 'gettext', 'underscore.string'
+], function(BaseView, _, gettext, str) {
     'use strict';
 
     var ContentGroupDetailsView = BaseView.extend({
         tagName: 'div',
-        className: 'content-group-details collection',
-
         events: {
-            'click .edit': 'editGroup'
+            'click .edit': 'editGroup',
+            'click .show-groups': 'showContentGroupUsages',
+            'click .hide-groups': 'hideContentGroupUsages'
+        },
+
+        className: function () {
+            var index = this.model.collection.indexOf(this.model);
+
+            return [
+                'collection',
+                'content-group-details',
+                'content-group-details-' + index
+            ].join(' ');
         },
 
         editGroup: function() {
@@ -21,10 +31,66 @@ define([
 
         initialize: function() {
             this.template = this.loadTemplate('content-group-details');
+            this.listenTo(this.model, 'change', this.render);
         },
 
-        render: function() {
-            this.$el.html(this.template(this.model.toJSON()));
+        render: function(showContentGroupUsages) {
+           var attrs = $.extend({}, this.model.attributes, {
+                usageCountMessage: this.getUsageCountTitle(),
+                outlineAnchorMessage: this.getOutlineAnchorMessage(),
+                index: this.model.collection.indexOf(this.model),
+                showContentGroupUsages: showContentGroupUsages || false
+            });
+            this.$el.html(this.template(attrs));
+            return this;
+        },
+
+        showContentGroupUsages: function(event) {
+            if (event && event.preventDefault) { event.preventDefault(); }
+            this.render(true);
+        },
+
+        hideContentGroupUsages: function(event) {
+            if (event && event.preventDefault) { event.preventDefault(); }
+            this.render(false);
+        },
+
+        getUsageCountTitle: function () {
+            var count = this.model.get('usage').length, message;
+
+            if (count === 0) {
+                message = gettext('Not in Use');
+            } else {
+                message = ngettext(
+                    /*
+                        Translators: 'count' is number of units that the group
+                        configuration is used in.
+                    */
+                    'Used in %(count)s unit', 'Used in %(count)s units',
+                    count
+                );
+            }
+
+            return interpolate(message, { count: count }, true);
+        },
+
+        getOutlineAnchorMessage: function () {
+            var message = gettext(
+                    /*
+                        Translators: 'outlineAnchor' is an anchor pointing to
+                        the course outline page.
+                    */
+                    'This content group is not in use. Add a content group to any unit from the %(outlineAnchor)s.'
+                ),
+                anchor = str.sprintf(
+                    '<a href="%(url)s" title="%(text)s">%(text)s</a>',
+                    {
+                            url: this.model.collection.parents[0].outlineUrl,
+                            text: gettext('Course Outline')
+                    }
+                );
+
+            return str.sprintf(message, {outlineAnchor: anchor});
         }
     });
 

cms/static/js/views/content_group_editor.js

@@ -23,9 +23,11 @@ function(ListItemEditorView, _) {
 
         getTemplateOptions: function() {
             return {
+                id: this.model.escape('id'),
                 name: this.model.escape('name'),
                 index: this.model.collection.indexOf(this.model),
                 isNew: this.model.isNew(),
+                usage: this.model.get('usage'),
                 uniqueId: _.uniqueId()
             };
         },

cms/static/js/views/content_group_item.js

@@ -5,15 +5,30 @@
  * It is expected to be backed by a Group model.
  */
 define([
-    'js/views/list_item', 'js/views/content_group_editor', 'js/views/content_group_details'
-], function(ListItemView, ContentGroupEditorView, ContentGroupDetailsView) {
+    'js/views/list_item', 'js/views/content_group_editor', 'js/views/content_group_details', 'gettext', "js/views/utils/view_utils"
+], function(ListItemView, ContentGroupEditorView, ContentGroupDetailsView, gettext) {
     'use strict';
 
     var ContentGroupItemView = ListItemView.extend({
+        events: {
+            'click .delete': 'deleteItem'
+        },
+
         tagName: 'section',
 
         baseClassName: 'content-group',
 
+        canDelete: true,
+
+        itemDisplayName: gettext('content group'),
+
+        attributes: function () {
+            return {
+                'id': this.model.get('id'),
+                'tabindex': -1
+            };
+        },
+
         createEditView: function() {
             return new ContentGroupEditorView({model: this.model});
         },

cms/static/sass/views/_group-configuration.scss

@@ -100,6 +100,28 @@
         color: $gray-l1;
         margin-left: $baseline;
 
+        &.group-configuration-info-inline {
+          display: table;
+          width: 70%;
+          margin: ($baseline/4) 0 ($baseline/2) $baseline;
+
+          li {
+            @include box-sizing(border-box);
+            display: table-cell;
+            margin-right: 1%;
+
+            &.group-configuration-usage-count {
+              font-style: italic;
+            }
+          }
+        }
+
+        &.group-configuration-info-block {
+          li {
+            padding: ($baseline/4) 0;
+          }
+        }
+
         &.collection-info-inline {
           display: table;
           width: 70%;
@@ -355,12 +377,31 @@
           }
         }
 
-        .field.add-collection-name label {
-          @extend %t-title5;
-          display: inline-block;
-          vertical-align: bottom;
+        .field.add-collection-name {
+
+          label {
+            width: 50%;
+            @extend %t-title5;
+            display: inline-block;
+            vertical-align: bottom;
+          }
+
+          .group-configuration-id {
+            display: inline-block;
+            width: 45%;
+            text-align: right;
+            vertical-align: top;
+            color: $gray-l1;
+
+            .group-configuration-value {
+              @extend %t-strong;
+              white-space: nowrap;
+              margin-left: ($baseline*0.5);
+            }
+          }
         }
 
+
         .actions {
           box-shadow: inset 0 1px 2px $shadow;
           border-top: 1px solid $gray-l1;
@@ -443,7 +484,7 @@
       .collection-header{
 
          .title {
-          margin-bottom: 0;
+            margin-bottom: 0;
         }
       }
     }
@@ -457,28 +498,6 @@
           color: $gray-l1;
           margin-left: $baseline;
 
-          &.group-configuration-info-inline {
-            display: table;
-            width: 70%;
-            margin: ($baseline/4) 0 ($baseline/2) $baseline;
-
-            li {
-              @include box-sizing(border-box);
-              display: table-cell;
-              margin-right: 1%;
-
-              &.group-configuration-usage-count {
-                font-style: italic;
-              }
-            }
-          }
-
-          &.group-configuration-info-block {
-            li {
-              padding: ($baseline/4) 0;
-            }
-          }
-
           .group-configuration-label {
             text-transform: uppercase;
           }
@@ -526,27 +545,12 @@
       .group-configuration-edit {
 
         .add-collection-name label {
-          width: 50%;
           padding-right: 5%;
           overflow: hidden;
           text-overflow: ellipsis;
           vertical-align: bottom;
         }
 
-        .group-configuration-id {
-          display: inline-block;
-          width: 45%;
-          text-align: right;
-          vertical-align: top;
-          color: $gray-l1;
-
-          .group-configuration-value {
-            @extend %t-strong;
-            white-space: nowrap;
-            margin-left: ($baseline*0.5);
-          }
-        }
-
         .field-group {
           @include clearfix();
           margin: 0 0 ($baseline/2) 0;

cms/templates/group_configurations.html

@@ -67,7 +67,8 @@
           <div class="content-groups-doc">
               <h3 class="title-3">${_("Content Groups")}</h3>
               <p>${_("Use content groups to give groups of students access to a specific set of course content. In addition to course content that is intended for all students, each content group sees content that you specifically designate as visible to it. By associating a content group with one or more cohorts, you can customize the content that a particular cohort or cohorts sees in your course.")}</p>
-              <p>${_("Click {em_start}New content group{em_end} to add a new content group. To edit the name of a content group, hover over its box and click {em_start}Edit{em_end}. Content groups cannot be deleted.").format(em_start="<strong>", em_end="</strong>")}</p>
+              <p>${_("Click {em_start}New content group{em_end} to add a new content group. To edit the name of a content group, hover over its box and click {em_start}Edit{em_end}.").format(em_start="<strong>", em_end="</strong>")}</p>
+              <p>${_("You can delete a content group only if it is not in use by a unit. To delete a content group, hover over its box and click the delete icon.")}</p>
               <p><a href="${get_online_help_info(content_groups_help_token())['doc_url']}" target="_blank" class="button external-help-button">${_("Learn More")}</a></p>
           </div>
         </div>

cms/templates/js/content-group-details.underscore

@@ -1,13 +1,58 @@
-<div class="collection-details">
+<div class="collection-details wrapper-group-configuration">
     <header class="collection-header">
         <h3 class="title">
-            <%- name %>
+            <a href="#" class="toggle group-toggle <% if (showContentGroupUsages){ print('hide'); } else { print('show'); } %>-groups">
+                <i class="ui-toggle-expansion icon fa fa-caret-<% if (showContentGroupUsages){ print('down'); } else { print('right'); } %>"></i>
+                <%= name %>
+            </a>
         </h3>
     </header>
 
-    <ul class="actions">
+   <ol class="collection-info group-configuration-info group-configuration-info-<% if(showContentGroupUsages){ print('block'); } else { print('inline'); } %>">
+     <% if (!_.isUndefined(id)) { %>
+         <li class="group-configuration-id"
+             ><span class="group-configuration-label"><%= gettext('ID') %>: </span
+             ><span class="group-configuration-value"><%= id %></span
+         ></li>
+     <% } %>
+     <% if (!showContentGroupUsages) { %>
+          <li class="group-configuration-usage-count">
+             <%= usageCountMessage %>
+         </li>
+     <% } %>
+   </ol>
+
+    <ul class="actions group-configuration-actions">
         <li class="action action-edit">
             <button class="edit"><i class="icon fa fa-pencil"></i> <%= gettext("Edit") %></button>
         </li>
+        <% if (_.isEmpty(usage)) { %>
+            <li class="action action-delete wrapper-delete-button" data-tooltip="<%= gettext('Delete') %>">
+                <button class="delete action-icon"><i class="icon fa fa-trash-o"></i><span><%= gettext("Delete") %></span></button>
+            </li>
+        <% } else { %>
+            <li class="action action-delete wrapper-delete-button" data-tooltip="<%= gettext('Cannot delete when in use by a unit') %>">
+                <button class="delete action-icon is-disabled" aria-disabled="true"><i class="icon fa fa-trash-o"></i><span><%= gettext("Delete") %></span></button>
+            </li>
+        <% } %>
     </ul>
 </div>
+
+<% if (showContentGroupUsages) { %>
+  <div class="collection-references wrapper-group-configuration-usages">
+    <% if (!_.isEmpty(usage)) { %>
+      <h4 class="intro group-configuration-usage-text"><%= gettext('This content group is used in:') %></h4>
+      <ol class="usage group-configuration-usage">
+        <% _.each(usage, function(unit) { %>
+          <li class="usage-unit group-configuration-usage-unit">
+            <p><a href=<%= unit.url %> ><%= unit.label %></a></p>
+          </li>
+        <% }) %>
+      </ol>
+    <% } else { %>
+        <p class="group-configuration-usage-text">
+        <%= outlineAnchorMessage %>
+        </p>
+    <% } %>
+  </div>
+<% } %>

cms/templates/js/content-group-editor.underscore

@@ -7,13 +7,40 @@
     <div class="wrapper-form">
         <fieldset class="collection-fields">
             <div class="input-wrap field text required add-collection-name <% if(error && error.attributes && error.attributes.name) { print('error'); } %>">
-                <label for="group-cohort-name-<%= uniqueId %>"><%= gettext("Content Group Name") %></label>
+                <label for="group-cohort-name-<%= uniqueId %>"><%= gettext("Content Group Name") %></label><%
+                if (!_.isUndefined(id) && !_.isEmpty(id)) {
+                    %><span class="group-configuration-id">
+                        <span class="group-configuration-label"><%= gettext('Content Group ID') %></span>
+                        <span class="group-configuration-value"><%= id %></span>
+                    </span><%
+                }
+                %>
                 <input name="group-cohort-name" id="group-cohort-name-<%= uniqueId %>" class="collection-name-input input-text" value="<%- name %>" type="text" placeholder="<%= gettext("This is the name of the group") %>">
             </div>
         </fieldset>
+        <% if (!_.isEmpty(usage)) { %>
+            <div class="wrapper-group-configuration-validation usage-validation">
+                <i class="icon fa fa-warning"></i>
+                <p class="group-configuration-validation-text">
+                    <%= gettext('This content group is used in one or more units.') %>
+                </p>
+            </div>
+        <% } %>
     </div>
     <div class="actions">
         <button class="action action-primary" type="submit"><% if (isNew) { print(gettext("Create")) } else { print(gettext("Save")) } %></button>
         <button class="action action-secondary action-cancel"><%= gettext("Cancel") %></button>
+
+         <% if (!isNew) { %>
+            <% if (_.isEmpty(usage)) { %>
+                <span class="wrapper-delete-button" data-tooltip="<%= gettext("Delete") %>">
+                    <a class="button action-delete delete" href="#"><%= gettext("Delete") %></a>
+                </span>
+            <% } else { %>
+                <span class="wrapper-delete-button" data-tooltip="<%= gettext('Cannot delete when in use by a unit') %>">
+                    <a class="button action-delete delete is-disabled" href="#" aria-disabled="true" ><%= gettext("Delete") %></a>
+                </span>
+            <% } %>
+        <% } %>
     </div>
 </form>

cms/urls.py

@@ -108,8 +108,8 @@ urlpatterns += patterns(
     url(r'^videos/{}$'.format(settings.COURSE_KEY_PATTERN), 'videos_handler'),
     url(r'^video_encodings_download/{}$'.format(settings.COURSE_KEY_PATTERN), 'video_encodings_download'),
     url(r'^group_configurations/{}$'.format(settings.COURSE_KEY_PATTERN), 'group_configurations_list_handler'),
-    url(r'^group_configurations/{}/(?P<group_configuration_id>\d+)/?$'.format(settings.COURSE_KEY_PATTERN),
-        'group_configurations_detail_handler'),
+    url(r'^group_configurations/{}/(?P<group_configuration_id>\d+)(/)?(?P<group_id>\d+)?$'.format(
+        settings.COURSE_KEY_PATTERN), 'group_configurations_detail_handler'),
 
     url(r'^api/val/v0/', include('edxval.urls')),
 )

common/djangoapps/cors_csrf/middleware.py

@@ -0,0 +1,67 @@
+"""
+Middleware for handling CSRF checks with CORS requests
+
+When processing HTTPS requests, the default CSRF middleware checks that the referer
+domain and protocol is the same as the request's domain and protocol. This is meant
+to avoid a type of attack for sites which serve their content with both HTTP and HTTPS,
+with a man in the middle on the HTTP requests.
+
+https://github.com/django/django/blob/b91c385e324f1cb94d20e2ad146372c259d51d3b/django/middleware/csrf.py#L117
+
+This doesn't work well with CORS requests, which aren't vulnerable to this attack when
+the server from which the request is coming uses HTTPS too, as it prevents the man in the
+middle attack vector.
+
+We thus do the CSRF check of requests coming from an authorized CORS host separately
+in this middleware, applying the same protections as the default CSRF middleware, but
+without the referrer check, when both the request and the referer use HTTPS.
+"""
+
+import logging
+import urlparse
+
+from django.conf import settings
+from django.middleware.csrf import CsrfViewMiddleware
+
+log = logging.getLogger(__name__)
+
+
+class CorsCSRFMiddleware(CsrfViewMiddleware):
+    """
+    Middleware for handling CSRF checks with CORS requests
+    """
+    def is_enabled(self, request):
+        """
+        Override the `is_enabled()` method to allow cross-domain HTTPS requests
+        """
+        if not settings.FEATURES.get('ENABLE_CORS_HEADERS'):
+            return False
+
+        referer = request.META.get('HTTP_REFERER')
+        if not referer:
+            return False
+        referer_parts = urlparse.urlparse(referer)
+
+        if referer_parts.hostname not in getattr(settings, 'CORS_ORIGIN_WHITELIST', []):
+            return False
+        if not request.is_secure() or referer_parts.scheme != 'https':
+            return False
+
+        return True
+
+    def process_view(self, request, callback, callback_args, callback_kwargs):
+        if not self.is_enabled(request):
+            return
+
+        is_secure_default = request.is_secure
+
+        def is_secure_patched():
+            """
+            Avoid triggering the additional CSRF middleware checks on the referrer
+            """
+            return False
+        request.is_secure = is_secure_patched
+
+        res = super(CorsCSRFMiddleware, self).process_view(request, callback, callback_args, callback_kwargs)
+        request.is_secure = is_secure_default
+        return res

common/djangoapps/cors_csrf/tests.py

@@ -0,0 +1,101 @@
+"""
+Tests for the CORS CSRF middleware
+"""
+
+from mock import patch, Mock
+
+from django.test import TestCase
+from django.test.utils import override_settings
+from django.middleware.csrf import CsrfViewMiddleware
+
+from cors_csrf.middleware import CorsCSRFMiddleware
+
+
+SENTINEL = object()
+
+
+class TestCorsMiddlewareProcessRequest(TestCase):
+    """
+    Test processing a request through the middleware
+    """
+    def get_request(self, is_secure, http_referer):
+        """
+        Build a test request
+        """
+        request = Mock()
+        request.META = {'HTTP_REFERER': http_referer}
+        request.is_secure = lambda: is_secure
+        return request
+
+    def setUp(self):
+        self.middleware = CorsCSRFMiddleware()
+
+    def check_not_enabled(self, request):
+        """
+        Check that the middleware does NOT process the provided request
+        """
+        with patch.object(CsrfViewMiddleware, 'process_view') as mock_method:
+            res = self.middleware.process_view(request, None, None, None)
+
+        self.assertIsNone(res)
+        self.assertFalse(mock_method.called)
+
+    def check_enabled(self, request):
+        """
+        Check that the middleware does process the provided request
+        """
+        def cb_check_req_is_secure_false(request, callback, args, kwargs):
+            """
+            Check that the request doesn't pass (yet) the `is_secure()` test
+            """
+            self.assertFalse(request.is_secure())
+            return SENTINEL
+
+        with patch.object(CsrfViewMiddleware, 'process_view') as mock_method:
+            mock_method.side_effect = cb_check_req_is_secure_false
+            res = self.middleware.process_view(request, None, None, None)
+
+        self.assertIs(res, SENTINEL)
+        self.assertTrue(request.is_secure())
+
+    @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True},
+                       CORS_ORIGIN_WHITELIST=['foo.com'])
+    def test_enabled(self):
+        request = self.get_request(is_secure=True,
+                                   http_referer='https://foo.com/bar')
+        self.check_enabled(request)
+
+    @override_settings(FEATURES={'ENABLE_CORS_HEADERS': False},
+                       CORS_ORIGIN_WHITELIST=['foo.com'])
+    def test_disabled_no_cors_headers(self):
+        request = self.get_request(is_secure=True,
+                                   http_referer='https://foo.com/bar')
+        self.check_not_enabled(request)
+
+    @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True},
+                       CORS_ORIGIN_WHITELIST=['bar.com'])
+    def test_disabled_wrong_cors_domain(self):
+        request = self.get_request(is_secure=True,
+                                   http_referer='https://foo.com/bar')
+        self.check_not_enabled(request)
+
+    @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True},
+                       CORS_ORIGIN_WHITELIST=['foo.com'])
+    def test_disabled_wrong_cors_domain_reversed(self):
+        request = self.get_request(is_secure=True,
+                                   http_referer='https://bar.com/bar')
+        self.check_not_enabled(request)
+
+    @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True},
+                       CORS_ORIGIN_WHITELIST=['foo.com'])
+    def test_disabled_http_request(self):
+        request = self.get_request(is_secure=False,
+                                   http_referer='https://foo.com/bar')
+        self.check_not_enabled(request)
+
+    @override_settings(FEATURES={'ENABLE_CORS_HEADERS': True},
+                       CORS_ORIGIN_WHITELIST=['foo.com'])
+    def test_disabled_http_referer(self):
+        request = self.get_request(is_secure=True,
+                                   http_referer='http://foo.com/bar')
+        self.check_not_enabled(request)

common/djangoapps/embargo/models.py

@@ -156,7 +156,7 @@ class RestrictedCourse(models.Model):
         Cache all restricted courses and returns the list of course_keys that are restricted
         """
         restricted_courses = cache.get(cls.COURSE_LIST_CACHE_KEY)
-        if not restricted_courses:
+        if restricted_courses is None:
             restricted_courses = list(RestrictedCourse.objects.values_list('course_key', flat=True))
             cache.set(cls.COURSE_LIST_CACHE_KEY, restricted_courses)
         return restricted_courses
@@ -413,7 +413,7 @@ class CountryAccessRule(models.Model):
         """
         cache_key = cls.CACHE_KEY.format(course_key=course_id)
         allowed_countries = cache.get(cache_key)
-        if not allowed_countries:
+        if allowed_countries is None:
             allowed_countries = cls._get_country_access_list(course_id)
             cache.set(cache_key, allowed_countries)
 

common/djangoapps/embargo/tests/test_api.py

@@ -166,6 +166,16 @@ class EmbargoCheckAccessApiTests(ModuleStoreTestCase):
         with self.assertNumQueries(0):
             embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0')
 
+    def test_caching_no_restricted_courses(self):
+        RestrictedCourse.objects.all().delete()
+        cache.clear()
+
+        with self.assertNumQueries(1):
+            embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0')
+
+        with self.assertNumQueries(0):
+            embargo_api.check_course_access(self.course.id, user=self.user, ip_address='0.0.0.0')
+
 
 @ddt.ddt
 @override_settings(MODULESTORE=MODULESTORE_CONFIG)

common/djangoapps/enrollment/data.py

@@ -97,7 +97,8 @@ def create_course_enrollment(username, course_id, mode, is_active):
     except CourseFullError as err:
         raise CourseEnrollmentFullError(err.message)
     except AlreadyEnrolledError as err:
-        raise CourseEnrollmentExistsError(err.message)
+        enrollment = get_course_enrollment(username, course_id)
+        raise CourseEnrollmentExistsError(err.message, enrollment)
 
 
 def update_course_enrollment(username, course_id, mode=None, is_active=None):

common/djangoapps/enrollment/errors.py

@@ -30,7 +30,11 @@ class CourseEnrollmentFullError(CourseEnrollmentError):
 
 
 class CourseEnrollmentExistsError(CourseEnrollmentError):
-    pass
+    enrollment = None
+
+    def __init__(self, message, enrollment):
+        super(CourseEnrollmentExistsError, self).__init__(message)
+        self.enrollment = enrollment
 
 
 class CourseModeNotFoundError(CourseEnrollmentError):

common/djangoapps/enrollment/tests/test_views.py

@@ -147,7 +147,7 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase):
         self.client.logout()
 
         # Try to enroll, this should fail.
-        self._create_enrollment(expected_status=status.HTTP_403_FORBIDDEN)
+        self._create_enrollment(expected_status=status.HTTP_401_UNAUTHORIZED)
 
     def test_user_not_activated(self):
         # Log out the default user, Bob.
@@ -255,6 +255,11 @@ class EnrollmentTest(ModuleStoreTestCase, APITestCase):
             self.assertTrue(data['is_active'])
         return resp
 
+    def test_enrollment_already_enrolled(self):
+        response = self._create_enrollment()
+        repeat_response = self._create_enrollment()
+        self.assertEqual(json.loads(response.content), json.loads(repeat_response.content))
+
     def test_get_enrollment_with_invalid_key(self):
         resp = self.client.post(
             reverse('courseenrollments'),

common/djangoapps/enrollment/views.py

@@ -9,7 +9,6 @@ from opaque_keys import InvalidKeyError
 from opaque_keys.edx.locator import CourseLocator
 from openedx.core.djangoapps.user_api import api as user_api
 from rest_framework import status
-from rest_framework.authentication import OAuth2Authentication
 from rest_framework import permissions
 from rest_framework.response import Response
 from rest_framework.throttling import UserRateThrottle
@@ -17,9 +16,11 @@ from rest_framework.views import APIView
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys import InvalidKeyError
 from enrollment import api
-from enrollment.errors import CourseNotFoundError, CourseEnrollmentError, CourseModeNotFoundError
+from enrollment.errors import (
+    CourseNotFoundError, CourseEnrollmentError, CourseModeNotFoundError, CourseEnrollmentExistsError
+)
 from embargo import api as embargo_api
-from util.authentication import SessionAuthenticationAllowInactiveUser
+from util.authentication import SessionAuthenticationAllowInactiveUser, OAuth2AuthenticationAllowInactiveUser
 from util.disable_rate_limit import can_disable_rate_limit
 
 
@@ -71,7 +72,7 @@ class EnrollmentView(APIView):
             * user: The ID of the user.
     """
 
-    authentication_classes = OAuth2Authentication, SessionAuthenticationAllowInactiveUser
+    authentication_classes = OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser
     permission_classes = permissions.IsAuthenticated,
     throttle_classes = EnrollmentUserThrottle,
 
@@ -243,7 +244,7 @@ class EnrollmentListView(APIView):
                 * user: The ID of the user.
     """
 
-    authentication_classes = OAuth2Authentication, SessionAuthenticationAllowInactiveUser
+    authentication_classes = OAuth2AuthenticationAllowInactiveUser, SessionAuthenticationAllowInactiveUser
     permission_classes = permissions.IsAuthenticated,
     throttle_classes = EnrollmentUserThrottle,
 
@@ -339,6 +340,8 @@ class EnrollmentListView(APIView):
                     "message": u"No course '{course_id}' found for enrollment".format(course_id=course_id)
                 }
             )
+        except CourseEnrollmentExistsError as error:
+            return Response(data=error.enrollment)
         except CourseEnrollmentError:
             return Response(
                 status=status.HTTP_400_BAD_REQUEST,

common/djangoapps/student/admin.py

@@ -29,6 +29,17 @@ class CourseAccessRoleAdmin(admin.ModelAdmin):
         'id', 'user', 'org', 'course_id', 'role'
     )
 
+
+class LinkedInAddToProfileConfigurationAdmin(admin.ModelAdmin):
+    """Admin interface for the LinkedIn Add to Profile configuration. """
+
+    class Meta:
+        model = LinkedInAddToProfileConfiguration
+
+    # Exclude deprecated fields
+    exclude = ('dashboard_tracking_code',)
+
+
 admin.site.register(UserProfile)
 
 admin.site.register(UserTestGroup)
@@ -45,4 +56,4 @@ admin.site.register(CourseAccessRole, CourseAccessRoleAdmin)
 
 admin.site.register(DashboardConfiguration, ConfigurationModelAdmin)
 
-admin.site.register(LinkedInAddToProfileConfiguration)
+admin.site.register(LinkedInAddToProfileConfiguration, LinkedInAddToProfileConfigurationAdmin)

common/djangoapps/student/forms.py

@@ -2,16 +2,23 @@
 Utility functions for validating forms
 """
 from django import forms
+from django.core.exceptions import ValidationError
 from django.contrib.auth.models import User
 from django.contrib.auth.forms import PasswordResetForm
 from django.contrib.auth.hashers import UNUSABLE_PASSWORD
 from django.contrib.auth.tokens import default_token_generator
 
 from django.utils.http import int_to_base36
+from django.utils.translation import ugettext_lazy as _
 from django.template import loader
 
 from django.conf import settings
 from microsite_configuration import microsite
+from util.password_policy_validators import (
+    validate_password_length,
+    validate_password_complexity,
+    validate_password_dictionary,
+)
 
 
 class PasswordResetFormNoActive(PasswordResetForm):
@@ -70,3 +77,160 @@ class PasswordResetFormNoActive(PasswordResetForm):
             subject = subject.replace('\n', '')
             email = loader.render_to_string(email_template_name, context)
             send_mail(subject, email, from_email, [user.email])
+
+
+class TrueField(forms.BooleanField):
+    """
+    A boolean field that only accepts "true" (case-insensitive) as true
+    """
+    def to_python(self, value):
+        # CheckboxInput converts string to bool by case-insensitive match to "true" or "false"
+        if value is True:
+            return value
+        else:
+            return None
+
+
+_USERNAME_TOO_SHORT_MSG = _("Username must be minimum of two characters long")
+_EMAIL_INVALID_MSG = _("A properly formatted e-mail is required")
+_PASSWORD_INVALID_MSG = _("A valid password is required")
+_NAME_TOO_SHORT_MSG = _("Your legal name must be a minimum of two characters long")
+
+
+class AccountCreationForm(forms.Form):
+    """
+    A form to for account creation data. It is currently only used for
+    validation, not rendering.
+    """
+    # TODO: Resolve repetition
+    username = forms.SlugField(
+        min_length=2,
+        max_length=30,
+        error_messages={
+            "required": _USERNAME_TOO_SHORT_MSG,
+            "invalid": _("Username should only consist of A-Z and 0-9, with no spaces."),
+            "min_length": _USERNAME_TOO_SHORT_MSG,
+            "max_length": _("Username cannot be more than %(limit_value)s characters long"),
+        }
+    )
+    email = forms.EmailField(
+        max_length=75,  # Limit per RFCs is 254, but User's email field in django 1.4 only takes 75
+        error_messages={
+            "required": _EMAIL_INVALID_MSG,
+            "invalid": _EMAIL_INVALID_MSG,
+            "max_length": _("Email cannot be more than %(limit_value)s characters long"),
+        }
+    )
+    password = forms.CharField(
+        min_length=2,
+        error_messages={
+            "required": _PASSWORD_INVALID_MSG,
+            "min_length": _PASSWORD_INVALID_MSG,
+        }
+    )
+    name = forms.CharField(
+        min_length=2,
+        error_messages={
+            "required": _NAME_TOO_SHORT_MSG,
+            "min_length": _NAME_TOO_SHORT_MSG,
+        }
+    )
+
+    def __init__(
+            self,
+            data=None,
+            extra_fields=None,
+            extended_profile_fields=None,
+            enforce_username_neq_password=False,
+            enforce_password_policy=False,
+            tos_required=True
+    ):
+        super(AccountCreationForm, self).__init__(data)
+
+        extra_fields = extra_fields or {}
+        self.extended_profile_fields = extended_profile_fields or {}
+        self.enforce_username_neq_password = enforce_username_neq_password
+        self.enforce_password_policy = enforce_password_policy
+        if tos_required:
+            self.fields["terms_of_service"] = TrueField(
+                error_messages={"required": _("You must accept the terms of service.")}
+            )
+
+        # TODO: These messages don't say anything about minimum length
+        error_message_dict = {
+            "level_of_education": _("A level of education is required"),
+            "gender": _("Your gender is required"),
+            "year_of_birth": _("Your year of birth is required"),
+            "mailing_address": _("Your mailing address is required"),
+            "goals": _("A description of your goals is required"),
+            "city": _("A city is required"),
+            "country": _("A country is required")
+        }
+        for field_name, field_value in extra_fields.items():
+            if field_name not in self.fields:
+                if field_name == "honor_code":
+                    if field_value == "required":
+                        self.fields[field_name] = TrueField(
+                            error_messages={
+                                "required": _("To enroll, you must follow the honor code.")
+                            }
+                        )
+                else:
+                    required = field_value == "required"
+                    min_length = 1 if field_name in ("gender", "level_of_education") else 2
+                    error_message = error_message_dict.get(
+                        field_name,
+                        _("You are missing one or more required fields")
+                    )
+                    self.fields[field_name] = forms.CharField(
+                        required=required,
+                        min_length=min_length,
+                        error_messages={
+                            "required": error_message,
+                            "min_length": error_message,
+                        }
+                    )
+
+        for field in self.extended_profile_fields:
+            if field not in self.fields:
+                self.fields[field] = forms.CharField(required=False)
+
+    def clean_password(self):
+        """Enforce password policies (if applicable)"""
+        password = self.cleaned_data["password"]
+        if (
+                self.enforce_username_neq_password and
+                "username" in self.cleaned_data and
+                self.cleaned_data["username"] == password
+        ):
+            raise ValidationError(_("Username and password fields cannot match"))
+        if self.enforce_password_policy:
+            try:
+                validate_password_length(password)
+                validate_password_complexity(password)
+                validate_password_dictionary(password)
+            except ValidationError, err:
+                raise ValidationError(_("Password: ") + "; ".join(err.messages))
+        return password
+
+    def clean_year_of_birth(self):
+        """
+        Parse year_of_birth to an integer, but just use None instead of raising
+        an error if it is malformed
+        """
+        try:
+            year_str = self.cleaned_data["year_of_birth"]
+            return int(year_str) if year_str is not None else None
+        except ValueError:
+            return None
+
+    @property
+    def cleaned_extended_profile(self):
+        """
+        Return a dictionary containing the extended_profile_fields and values
+        """
+        return {
+            key: value
+            for key, value in self.cleaned_data.items()
+            if key in self.extended_profile_fields and value is not None
+        }

common/djangoapps/student/helpers.py

@@ -153,7 +153,21 @@ def check_verify_status_by_course(user, course_enrollment_pairs, all_course_mode
                     else:
                         status = VERIFY_STATUS_NEED_TO_VERIFY
                 else:
-                    status = VERIFY_STATUS_MISSED_DEADLINE
+                    # If a user currently has an active or pending verification,
+                    # then they may have submitted an additional attempt after
+                    # the verification deadline passed.  This can occur,
+                    # for example, when the support team asks a student
+                    # to reverify after the deadline so they can receive
+                    # a verified certificate.
+                    # In this case, we still want to show them as "verified"
+                    # on the dashboard.
+                    if has_active_or_pending:
+                        status = VERIFY_STATUS_APPROVED
+
+                    # Otherwise, the student missed the deadline, so show
+                    # them as "honor" (the kind of certificate they will receive).
+                    else:
+                        status = VERIFY_STATUS_MISSED_DEADLINE
 
             # Set the status for the course only if we're displaying some kind of message
             # Otherwise, leave the course out of the dictionary.

common/djangoapps/student/management/commands/create_random_users.py

@@ -8,26 +8,30 @@ from student.models import CourseEnrollment
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from student.forms import AccountCreationForm
 from student.views import _do_create_account
 
 
-def get_random_post_override():
+def make_random_form():
     """
     Generate unique user data for dummy users.
     """
     identification = uuid.uuid4().hex[:8]
-    return {
-        'username': 'user_{id}'.format(id=identification),
-        'email': 'email_{id}@example.com'.format(id=identification),
-        'password': '12345',
-        'name': 'User {id}'.format(id=identification),
-    }
+    return AccountCreationForm(
+        data={
+            'username': 'user_{id}'.format(id=identification),
+            'email': 'email_{id}@example.com'.format(id=identification),
+            'password': '12345',
+            'name': 'User {id}'.format(id=identification),
+        },
+        tos_required=False
+    )
 
 
 def create(num, course_key):
     """Create num users, enrolling them in course_key if it's not None"""
     for idx in range(num):
-        (user, user_profile, __) = _do_create_account(get_random_post_override())
+        (user, _, _) = _do_create_account(make_random_form())
         if course_key is not None:
             CourseEnrollment.enroll(user, course_key)
 

common/djangoapps/student/management/commands/create_user.py

@@ -8,6 +8,7 @@ from django.utils import translation
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
+from student.forms import AccountCreationForm
 from student.models import CourseEnrollment, Registration, create_comments_service_user
 from student.views import _do_create_account, AccountValidationError
 from track.management.tracked_command import TrackedCommand
@@ -80,21 +81,22 @@ class Command(TrackedCommand):
             except InvalidKeyError:
                 course = SlashSeparatedCourseKey.from_deprecated_string(options['course'])
 
-        post_data = {
-            'username': username,
-            'email': options['email'],
-            'password': options['password'],
-            'name': name,
-            'honor_code': u'true',
-            'terms_of_service': u'true',
-        }
+        form = AccountCreationForm(
+            data={
+                'username': username,
+                'email': options['email'],
+                'password': options['password'],
+                'name': name,
+            },
+            tos_required=False
+        )
         # django.utils.translation.get_language() will be used to set the new
         # user's preferred language.  This line ensures that the result will
         # match this installation's default locale.  Otherwise, inside a
         # management command, it will always return "en-us".
         translation.activate(settings.LANGUAGE_CODE)
         try:
-            user, profile, reg = _do_create_account(post_data)
+            user, _, reg = _do_create_account(form)
             if options['staff']:
                 user.is_staff = True
                 user.save()

common/djangoapps/student/migrations/0045_add_trk_partner_to_linkedin_config.py

@@ -0,0 +1,184 @@
+# -*- coding: utf-8 -*-
+import datetime
+from south.db import db
+from south.v2 import SchemaMigration
+from django.db import models
+
+
+class Migration(SchemaMigration):
+
+    def forwards(self, orm):
+        # Adding field 'LinkedInAddToProfileConfiguration.trk_partner_name'
+        db.add_column('student_linkedinaddtoprofileconfiguration', 'trk_partner_name',
+                      self.gf('django.db.models.fields.CharField')(default='', max_length=10, blank=True),
+                      keep_default=False)
+
+
+    def backwards(self, orm):
+        # Deleting field 'LinkedInAddToProfileConfiguration.trk_partner_name'
+        db.delete_column('student_linkedinaddtoprofileconfiguration', 'trk_partner_name')
+
+
+    models = {
+        'auth.group': {
+            'Meta': {'object_name': 'Group'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
+            'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
+        },
+        'auth.permission': {
+            'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
+            'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
+        },
+        'auth.user': {
+            'Meta': {'object_name': 'User'},
+            'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
+            'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
+            'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
+        },
+        'contenttypes.contenttype': {
+            'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
+            'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
+        },
+        'student.anonymoususerid': {
+            'Meta': {'object_name': 'AnonymousUserId'},
+            'anonymous_user_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32'}),
+            'course_id': ('xmodule_django.models.CourseKeyField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        },
+        'student.courseaccessrole': {
+            'Meta': {'unique_together': "(('user', 'org', 'course_id', 'role'),)", 'object_name': 'CourseAccessRole'},
+            'course_id': ('xmodule_django.models.CourseKeyField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'org': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '64', 'blank': 'True'}),
+            'role': ('django.db.models.fields.CharField', [], {'max_length': '64', 'db_index': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        },
+        'student.courseenrollment': {
+            'Meta': {'ordering': "('user', 'course_id')", 'unique_together': "(('user', 'course_id'),)", 'object_name': 'CourseEnrollment'},
+            'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'mode': ('django.db.models.fields.CharField', [], {'default': "'honor'", 'max_length': '100'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        },
+        'student.courseenrollmentallowed': {
+            'Meta': {'unique_together': "(('email', 'course_id'),)", 'object_name': 'CourseEnrollmentAllowed'},
+            'auto_enroll': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
+            'created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'null': 'True', 'db_index': 'True', 'blank': 'True'}),
+            'email': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'})
+        },
+        'student.dashboardconfiguration': {
+            'Meta': {'object_name': 'DashboardConfiguration'},
+            'change_date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'on_delete': 'models.PROTECT'}),
+            'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'recent_enrollment_time_delta': ('django.db.models.fields.PositiveIntegerField', [], {'default': '0'})
+        },
+        'student.linkedinaddtoprofileconfiguration': {
+            'Meta': {'object_name': 'LinkedInAddToProfileConfiguration'},
+            'change_date': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
+            'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'null': 'True', 'on_delete': 'models.PROTECT'}),
+            'company_identifier': ('django.db.models.fields.TextField', [], {}),
+            'dashboard_tracking_code': ('django.db.models.fields.TextField', [], {'default': "''", 'blank': 'True'}),
+            'enabled': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'trk_partner_name': ('django.db.models.fields.CharField', [], {'default': "''", 'max_length': '10', 'blank': 'True'})
+        },
+        'student.loginfailures': {
+            'Meta': {'object_name': 'LoginFailures'},
+            'failure_count': ('django.db.models.fields.IntegerField', [], {'default': '0'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'lockout_until': ('django.db.models.fields.DateTimeField', [], {'null': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        },
+        'student.passwordhistory': {
+            'Meta': {'object_name': 'PasswordHistory'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
+            'time_set': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        },
+        'student.pendingemailchange': {
+            'Meta': {'object_name': 'PendingEmailChange'},
+            'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'new_email': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+            'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'})
+        },
+        'student.pendingnamechange': {
+            'Meta': {'object_name': 'PendingNameChange'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'new_name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'blank': 'True'}),
+            'rationale': ('django.db.models.fields.CharField', [], {'max_length': '1024', 'blank': 'True'}),
+            'user': ('django.db.models.fields.related.OneToOneField', [], {'to': "orm['auth.User']", 'unique': 'True'})
+        },
+        'student.registration': {
+            'Meta': {'object_name': 'Registration', 'db_table': "'auth_registration'"},
+            'activation_key': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '32', 'db_index': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'unique': 'True'})
+        },
+        'student.userprofile': {
+            'Meta': {'object_name': 'UserProfile', 'db_table': "'auth_userprofile'"},
+            'allow_certificate': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
+            'city': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'null': 'True', 'blank': 'True'}),
+            'courseware': ('django.db.models.fields.CharField', [], {'default': "'course.xml'", 'max_length': '255', 'blank': 'True'}),
+            'gender': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}),
+            'goals': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'language': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+            'level_of_education': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '6', 'null': 'True', 'blank': 'True'}),
+            'location': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+            'mailing_address': ('django.db.models.fields.TextField', [], {'null': 'True', 'blank': 'True'}),
+            'meta': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
+            'user': ('django.db.models.fields.related.OneToOneField', [], {'related_name': "'profile'", 'unique': 'True', 'to': "orm['auth.User']"}),
+            'year_of_birth': ('django.db.models.fields.IntegerField', [], {'db_index': 'True', 'null': 'True', 'blank': 'True'})
+        },
+        'student.usersignupsource': {
+            'Meta': {'object_name': 'UserSignupSource'},
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'site': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
+        },
+        'student.userstanding': {
+            'Meta': {'object_name': 'UserStanding'},
+            'account_status': ('django.db.models.fields.CharField', [], {'max_length': '31', 'blank': 'True'}),
+            'changed_by': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']", 'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'standing_last_changed_at': ('django.db.models.fields.DateTimeField', [], {'auto_now': 'True', 'blank': 'True'}),
+            'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'standing'", 'unique': 'True', 'to': "orm['auth.User']"})
+        },
+        'student.usertestgroup': {
+            'Meta': {'object_name': 'UserTestGroup'},
+            'description': ('django.db.models.fields.TextField', [], {'blank': 'True'}),
+            'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
+            'name': ('django.db.models.fields.CharField', [], {'max_length': '32', 'db_index': 'True'}),
+            'users': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.User']", 'db_index': 'True', 'symmetrical': 'False'})
+        }
+    }
+
+    complete_apps = ['student']

common/djangoapps/student/models.py

@@ -1467,34 +1467,88 @@ class LinkedInAddToProfileConfiguration(ConfigurationModel):
     # Deprecated
     dashboard_tracking_code = models.TextField(default="", blank=True)
 
-    def add_to_profile_url(self, course_name, enrollment_mode, cert_url, source="o"):
+    trk_partner_name = models.CharField(
+        max_length=10,
+        default="",
+        blank=True,
+        help_text=ugettext_lazy(
+            u"Short identifier for the LinkedIn partner used in the tracking code.  "
+            u"(Example: 'edx')  "
+            u"If no value is provided, tracking codes will not be sent to LinkedIn."
+        )
+    )
+
+    def add_to_profile_url(self, course_key, course_name, cert_mode, cert_url, source="o", target="dashboard"):
         """Construct the URL for the "add to profile" button.
 
         Arguments:
+            course_key (CourseKey): The identifier for the course.
             course_name (unicode): The display name of the course.
-            enrollment_mode (str): The enrollment mode of the user (e.g. "verified", "honor", "professional")
+            cert_mode (str): The course mode of the user's certificate (e.g. "verified", "honor", "professional")
             cert_url (str): The download URL for the certificate.
 
         Keyword Arguments:
             source (str): Either "o" (for onsite/UI), "e" (for emails), or "m" (for mobile)
+            target (str): An identifier for the occurrance of the button.
 
         """
         params = OrderedDict([
             ('_ed', self.company_identifier),
-            ('pfCertificationName', self._cert_name(course_name, enrollment_mode).encode('utf-8')),
+            ('pfCertificationName', self._cert_name(course_name, cert_mode).encode('utf-8')),
             ('pfCertificationUrl', cert_url),
             ('source', source)
         ])
+
+        tracking_code = self._tracking_code(course_key, cert_mode, target)
+        if tracking_code is not None:
+            params['trk'] = tracking_code
+
         return u'http://www.linkedin.com/profile/add?{params}'.format(
             params=urlencode(params)
         )
 
-    def _cert_name(self, course_name, enrollment_mode):
+    def _cert_name(self, course_name, cert_mode):
         """Name of the certification, for display on LinkedIn. """
         return self.MODE_TO_CERT_NAME.get(
-            enrollment_mode,
+            cert_mode,
             _(u"{platform_name} Certificate for {course_name}")
         ).format(
             platform_name=settings.PLATFORM_NAME,
             course_name=course_name
         )
+
+    def _tracking_code(self, course_key, cert_mode, target):
+        """Create a tracking code for the button.
+
+        Tracking codes are used by LinkedIn to collect
+        analytics about certifications users are adding
+        to their profiles.
+
+        The tracking code format is:
+            &trk=[partner name]-[certificate type]-[date]-[target field]
+
+        In our case, we're sending:
+            &trk=edx-{COURSE ID}_{COURSE MODE}-{TARGET}
+
+        If no partner code is configured, then this will
+        return None, indicating that tracking codes are disabled.
+
+        Arguments:
+
+            course_key (CourseKey): The identifier for the course.
+            cert_mode (str): The enrollment mode for the course.
+            target (str): Identifier for where the button is located.
+
+        Returns:
+            unicode or None
+
+        """
+        return (
+            u"{partner}-{course_key}_{cert_mode}-{target}".format(
+                partner=self.trk_partner_name,
+                course_key=unicode(course_key),
+                cert_mode=cert_mode,
+                target=target
+            )
+            if self.trk_partner_name else None
+        )

common/djangoapps/student/tests/test_create_account.py

@@ -1,4 +1,5 @@
 "Tests for account creation"
+import json
 
 import ddt
 import unittest
@@ -9,11 +10,13 @@ from django.core.urlresolvers import reverse
 from django.contrib.auth.models import AnonymousUser
 from django.utils.importlib import import_module
 from django.test import TestCase, TransactionTestCase
+from django.test.utils import override_settings
 
 import mock
 
 from openedx.core.djangoapps.user_api.models import UserPreference
 from lang_pref import LANGUAGE_KEY
+from notification_prefs import NOTIFICATION_PREF_KEY
 
 from edxmako.tests import mako_middleware_process_request
 from external_auth.models import ExternalAuthMap
@@ -23,6 +26,21 @@ TEST_CS_URL = 'https://comments.service.test:123/'
 
 
 @ddt.ddt
+@override_settings(
+    MICROSITE_CONFIGURATION={
+        "microsite": {
+            "domain_prefix": "microsite",
+            "extended_profile_fields": ["extra1", "extra2"],
+        }
+    },
+    REGISTRATION_EXTRA_FIELDS={
+        key: "optional"
+        for key in [
+            "level_of_education", "gender", "mailing_address", "city", "country", "goals",
+            "year_of_birth"
+        ]
+    }
+)
 class TestCreateAccount(TestCase):
     "Tests for account creation"
 
@@ -50,9 +68,112 @@ class TestCreateAccount(TestCase):
     @ddt.data("en", "eo")
     def test_header_lang_pref_saved(self, lang):
         response = self.client.post(self.url, self.params, HTTP_ACCEPT_LANGUAGE=lang)
+        user = User.objects.get(username=self.username)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(UserPreference.get_preference(user, LANGUAGE_KEY), lang)
+
+    def create_account_and_fetch_profile(self):
+        """
+        Create an account with self.params, assert that the response indicates
+        success, and return the UserProfile object for the newly created user
+        """
+        response = self.client.post(self.url, self.params, HTTP_HOST="microsite.example.com")
         self.assertEqual(response.status_code, 200)
         user = User.objects.get(username=self.username)
-        self.assertEqual(UserPreference.get_preference(user, LANGUAGE_KEY), lang)
+        return user.profile
+
+    def test_marketing_cookie(self):
+        response = self.client.post(self.url, self.params)
+        self.assertEqual(response.status_code, 200)
+        self.assertIn(settings.EDXMKTG_COOKIE_NAME, self.client.cookies)
+
+    @unittest.skipUnless(
+        "microsite_configuration.middleware.MicrositeMiddleware" in settings.MIDDLEWARE_CLASSES,
+        "Microsites not implemented in this environment"
+    )
+    def test_profile_saved_no_optional_fields(self):
+        profile = self.create_account_and_fetch_profile()
+        self.assertEqual(profile.name, self.params["name"])
+        self.assertEqual(profile.level_of_education, "")
+        self.assertEqual(profile.gender, "")
+        self.assertEqual(profile.mailing_address, "")
+        self.assertEqual(profile.city, "")
+        self.assertEqual(profile.country, "")
+        self.assertEqual(profile.goals, "")
+        self.assertEqual(
+            profile.get_meta(),
+            {
+                "extra1": "",
+                "extra2": "",
+            }
+        )
+        self.assertIsNone(profile.year_of_birth)
+
+    @unittest.skipUnless(
+        "microsite_configuration.middleware.MicrositeMiddleware" in settings.MIDDLEWARE_CLASSES,
+        "Microsites not implemented in this environment"
+    )
+    def test_profile_saved_all_optional_fields(self):
+        self.params.update({
+            "level_of_education": "a",
+            "gender": "o",
+            "mailing_address": "123 Example Rd",
+            "city": "Exampleton",
+            "country": "US",
+            "goals": "To test this feature",
+            "year_of_birth": "2015",
+            "extra1": "extra_value1",
+            "extra2": "extra_value2",
+        })
+        profile = self.create_account_and_fetch_profile()
+        self.assertEqual(profile.level_of_education, "a")
+        self.assertEqual(profile.gender, "o")
+        self.assertEqual(profile.mailing_address, "123 Example Rd")
+        self.assertEqual(profile.city, "Exampleton")
+        self.assertEqual(profile.country, "US")
+        self.assertEqual(profile.goals, "To test this feature")
+        self.assertEqual(
+            profile.get_meta(),
+            {
+                "extra1": "extra_value1",
+                "extra2": "extra_value2",
+            }
+        )
+        self.assertEqual(profile.year_of_birth, 2015)
+
+    @unittest.skipUnless(
+        "microsite_configuration.middleware.MicrositeMiddleware" in settings.MIDDLEWARE_CLASSES,
+        "Microsites not implemented in this environment"
+    )
+    def test_profile_saved_empty_optional_fields(self):
+        self.params.update({
+            "level_of_education": "",
+            "gender": "",
+            "mailing_address": "",
+            "city": "",
+            "country": "",
+            "goals": "",
+            "year_of_birth": "",
+            "extra1": "",
+            "extra2": "",
+        })
+        profile = self.create_account_and_fetch_profile()
+        self.assertEqual(profile.level_of_education, "")
+        self.assertEqual(profile.gender, "")
+        self.assertEqual(profile.mailing_address, "")
+        self.assertEqual(profile.city, "")
+        self.assertEqual(profile.country, "")
+        self.assertEqual(profile.goals, "")
+        self.assertEqual(
+            profile.get_meta(),
+            {"extra1": "", "extra2": ""}
+        )
+        self.assertEqual(profile.year_of_birth, None)
+
+    def test_profile_year_of_birth_non_integer(self):
+        self.params["year_of_birth"] = "not_an_integer"
+        profile = self.create_account_and_fetch_profile()
+        self.assertIsNone(profile.year_of_birth)
 
     def base_extauth_bypass_sending_activation_email(self, bypass_activation_email_for_extauth_setting):
         """
@@ -98,6 +219,248 @@ class TestCreateAccount(TestCase):
         """
         self.base_extauth_bypass_sending_activation_email(False)
 
+    @ddt.data(True, False)
+    def test_discussions_email_digest_pref(self, digest_enabled):
+        with mock.patch.dict("student.models.settings.FEATURES", {"ENABLE_DISCUSSION_EMAIL_DIGEST": digest_enabled}):
+            response = self.client.post(self.url, self.params)
+            self.assertEqual(response.status_code, 200)
+            user = User.objects.get(username=self.username)
+            preference = UserPreference.get_preference(user, NOTIFICATION_PREF_KEY)
+            if digest_enabled:
+                self.assertIsNotNone(preference)
+            else:
+                self.assertIsNone(preference)
+
+
+@ddt.ddt
+class TestCreateAccountValidation(TestCase):
+    """
+    Test validation of various parameters in the create_account view
+    """
+    def setUp(self):
+        super(TestCreateAccountValidation, self).setUp()
+        self.url = reverse("create_account")
+        self.minimal_params = {
+            "username": "test_username",
+            "email": "test_email@example.com",
+            "password": "test_password",
+            "name": "Test Name",
+            "honor_code": "true",
+            "terms_of_service": "true",
+        }
+
+    def assert_success(self, params):
+        """
+        Request account creation with the given params and assert that the
+        response properly indicates success
+        """
+        response = self.client.post(self.url, params)
+        self.assertEqual(response.status_code, 200)
+        response_data = json.loads(response.content)
+        self.assertTrue(response_data["success"])
+
+    def assert_error(self, params, expected_field, expected_value):
+        """
+        Request account creation with the given params and assert that the
+        response properly indicates an error with the given field and value
+        """
+        response = self.client.post(self.url, params)
+        self.assertEqual(response.status_code, 400)
+        response_data = json.loads(response.content)
+        self.assertFalse(response_data["success"])
+        self.assertEqual(response_data["field"], expected_field)
+        self.assertEqual(response_data["value"], expected_value)
+
+    def test_minimal_success(self):
+        self.assert_success(self.minimal_params)
+
+    def test_username(self):
+        params = dict(self.minimal_params)
+
+        def assert_username_error(expected_error):
+            """
+            Assert that requesting account creation results in the expected
+            error
+            """
+            self.assert_error(params, "username", expected_error)
+
+        # Missing
+        del params["username"]
+        assert_username_error("Username must be minimum of two characters long")
+
+        # Empty, too short
+        for username in ["", "a"]:
+            params["username"] = username
+            assert_username_error("Username must be minimum of two characters long")
+
+        # Too long
+        params["username"] = "this_username_has_31_characters"
+        assert_username_error("Username cannot be more than 30 characters long")
+
+        # Invalid
+        params["username"] = "invalid username"
+        assert_username_error("Username should only consist of A-Z and 0-9, with no spaces.")
+
+    def test_email(self):
+        params = dict(self.minimal_params)
+
+        def assert_email_error(expected_error):
+            """
+            Assert that requesting account creation results in the expected
+            error
+            """
+            self.assert_error(params, "email", expected_error)
+
+        # Missing
+        del params["email"]
+        assert_email_error("A properly formatted e-mail is required")
+
+        # Empty, too short
+        for email in ["", "a"]:
+            params["email"] = email
+            assert_email_error("A properly formatted e-mail is required")
+
+        # Too long
+        params["email"] = "this_email_address_has_76_characters_in_it_so_it_is_unacceptable@example.com"
+        assert_email_error("Email cannot be more than 75 characters long")
+
+        # Invalid
+        params["email"] = "not_an_email_address"
+        assert_email_error("A properly formatted e-mail is required")
+
+    def test_password(self):
+        params = dict(self.minimal_params)
+
+        def assert_password_error(expected_error):
+            """
+            Assert that requesting account creation results in the expected
+            error
+            """
+            self.assert_error(params, "password", expected_error)
+
+        # Missing
+        del params["password"]
+        assert_password_error("A valid password is required")
+
+        # Empty, too short
+        for password in ["", "a"]:
+            params["password"] = password
+            assert_password_error("A valid password is required")
+
+        # Password policy is tested elsewhere
+
+        # Matching username
+        params["username"] = params["password"] = "test_username_and_password"
+        assert_password_error("Username and password fields cannot match")
+
+    def test_name(self):
+        params = dict(self.minimal_params)
+
+        def assert_name_error(expected_error):
+            """
+            Assert that requesting account creation results in the expected
+            error
+            """
+            self.assert_error(params, "name", expected_error)
+
+        # Missing
+        del params["name"]
+        assert_name_error("Your legal name must be a minimum of two characters long")
+
+        # Empty, too short
+        for name in ["", "a"]:
+            params["name"] = name
+            assert_name_error("Your legal name must be a minimum of two characters long")
+
+    def test_honor_code(self):
+        params = dict(self.minimal_params)
+
+        def assert_honor_code_error(expected_error):
+            """
+            Assert that requesting account creation results in the expected
+            error
+            """
+            self.assert_error(params, "honor_code", expected_error)
+
+        with override_settings(REGISTRATION_EXTRA_FIELDS={"honor_code": "required"}):
+            # Missing
+            del params["honor_code"]
+            assert_honor_code_error("To enroll, you must follow the honor code.")
+
+            # Empty, invalid
+            for honor_code in ["", "false", "not_boolean"]:
+                params["honor_code"] = honor_code
+                assert_honor_code_error("To enroll, you must follow the honor code.")
+
+            # True
+            params["honor_code"] = "tRUe"
+            self.assert_success(params)
+
+        with override_settings(REGISTRATION_EXTRA_FIELDS={"honor_code": "optional"}):
+            # Missing
+            del params["honor_code"]
+            # Need to change username/email because user was created above
+            params["username"] = "another_test_username"
+            params["email"] = "another_test_email@example.com"
+            self.assert_success(params)
+
+    def test_terms_of_service(self):
+        params = dict(self.minimal_params)
+
+        def assert_terms_of_service_error(expected_error):
+            """
+            Assert that requesting account creation results in the expected
+            error
+            """
+            self.assert_error(params, "terms_of_service", expected_error)
+
+        # Missing
+        del params["terms_of_service"]
+        assert_terms_of_service_error("You must accept the terms of service.")
+
+        # Empty, invalid
+        for terms_of_service in ["", "false", "not_boolean"]:
+            params["terms_of_service"] = terms_of_service
+            assert_terms_of_service_error("You must accept the terms of service.")
+
+        # True
+        params["terms_of_service"] = "tRUe"
+        self.assert_success(params)
+
+    @ddt.data(
+        ("level_of_education", 1, "A level of education is required"),
+        ("gender", 1, "Your gender is required"),
+        ("year_of_birth", 2, "Your year of birth is required"),
+        ("mailing_address", 2, "Your mailing address is required"),
+        ("goals", 2, "A description of your goals is required"),
+        ("city", 2, "A city is required"),
+        ("country", 2, "A country is required"),
+        ("custom_field", 2, "You are missing one or more required fields")
+    )
+    @ddt.unpack
+    def test_extra_fields(self, field, min_length, expected_error):
+        params = dict(self.minimal_params)
+
+        def assert_extra_field_error():
+            """
+            Assert that requesting account creation results in the expected
+            error
+            """
+            self.assert_error(params, field, expected_error)
+
+        with override_settings(REGISTRATION_EXTRA_FIELDS={field: "required"}):
+            # Missing
+            assert_extra_field_error()
+
+            # Empty
+            params[field] = ""
+            assert_extra_field_error()
+
+            # Too short
+            if min_length > 1:
+                params[field] = "a"
+                assert_extra_field_error()
+
 
 @mock.patch.dict("student.models.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
 @mock.patch("lms.lib.comment_client.User.base_url", TEST_CS_URL)

common/djangoapps/student/tests/test_linkedin.py

@@ -0,0 +1,70 @@
+# -*- coding: utf-8 -*-
+"""Tests for LinkedIn Add to Profile configuration. """
+
+import ddt
+from urllib import urlencode
+
+from django.test import TestCase
+from opaque_keys.edx.locator import CourseLocator
+from student.models import LinkedInAddToProfileConfiguration
+
+
+@ddt.ddt
+class LinkedInAddToProfileUrlTests(TestCase):
+    """Tests for URL generation of LinkedInAddToProfileConfig. """
+
+    COURSE_KEY = CourseLocator(org="edx", course="DemoX", run="Demo_Course")
+    COURSE_NAME = u"Test Course ☃"
+    CERT_URL = u"http://s3.edx/cert"
+
+    @ddt.data(
+        ('honor', u'edX+Honor+Code+Certificate+for+Test+Course+%E2%98%83'),
+        ('verified', u'edX+Verified+Certificate+for+Test+Course+%E2%98%83'),
+        ('professional', u'edX+Professional+Certificate+for+Test+Course+%E2%98%83'),
+        ('default_mode', u'edX+Certificate+for+Test+Course+%E2%98%83')
+    )
+    @ddt.unpack
+    def test_linked_in_url(self, cert_mode, expected_cert_name):
+        config = LinkedInAddToProfileConfiguration(
+            company_identifier='0_mC_o2MizqdtZEmkVXjH4eYwMj4DnkCWrZP_D9',
+            enabled=True
+        )
+
+        expected_url = (
+            'http://www.linkedin.com/profile/add'
+            '?_ed=0_mC_o2MizqdtZEmkVXjH4eYwMj4DnkCWrZP_D9&'
+            'pfCertificationName={expected_cert_name}&'
+            'pfCertificationUrl=http%3A%2F%2Fs3.edx%2Fcert&'
+            'source=o'
+        ).format(expected_cert_name=expected_cert_name)
+
+        actual_url = config.add_to_profile_url(
+            self.COURSE_KEY,
+            self.COURSE_NAME,
+            cert_mode,
+            self.CERT_URL
+        )
+
+        self.assertEqual(actual_url, expected_url)
+
+    def test_linked_in_url_tracking_code(self):
+        config = LinkedInAddToProfileConfiguration(
+            company_identifier="abcd123",
+            trk_partner_name="edx",
+            enabled=True
+        )
+
+        expected_param = urlencode({
+            'trk': u'edx-{course_key}_honor-dashboard'.format(
+                course_key=self.COURSE_KEY
+            )
+        })
+
+        actual_url = config.add_to_profile_url(
+            self.COURSE_KEY,
+            self.COURSE_NAME,
+            'honor',
+            self.CERT_URL
+        )
+
+        self.assertIn(expected_param, actual_url)

common/djangoapps/student/tests/test_verification_status.py

@@ -212,6 +212,19 @@ class TestCourseVerificationStatus(UrlResetMixin, ModuleStoreTestCase):
         # a verification is active).
         self._assert_course_verification_status(VERIFY_STATUS_NEED_TO_REVERIFY)
 
+    def test_verification_occurred_after_deadline(self):
+        # Expiration date in the past
+        self._setup_mode_and_enrollment(self.PAST, "verified")
+
+        # The deadline has passed, and we've asked the student
+        # to reverify (through the support team).
+        attempt = SoftwareSecurePhotoVerification.objects.create(user=self.user)
+        attempt.mark_ready()
+        attempt.submit()
+
+        # Expect that the user's displayed enrollment mode is verified.
+        self._assert_course_verification_status(VERIFY_STATUS_APPROVED)
+
     def _setup_mode_and_enrollment(self, deadline, enrollment_mode):
         """Create a course mode and enrollment.
 

common/djangoapps/student/tests/tests.py

@@ -17,7 +17,6 @@ from django.contrib.sessions.middleware import SessionMiddleware
 from django.core.urlresolvers import reverse
 from django.test import TestCase
 from django.test.client import RequestFactory, Client
-from django.test.utils import override_settings
 from mock import Mock, patch
 from opaque_keys.edx.locations import SlashSeparatedCourseKey
 
@@ -49,13 +48,6 @@ log = logging.getLogger(__name__)
 class CourseEndingTest(TestCase):
     """Test things related to course endings: certificates, surveys, etc"""
 
-    def setUp(self):
-        super(CourseEndingTest, self).setUp()
-
-        # Clear the model-based config cache to avoid
-        # interference between tests.
-        cache.clear()
-
     def test_process_survey_link(self):
         username = "fred"
         user = Mock(username=username)
@@ -198,118 +190,6 @@ class CourseEndingTest(TestCase):
         }
         self.assertIsNone(_cert_info(user, course2, cert_status, course_mode))
 
-    def test_linked_in_url_with_unicode_course_display_name(self):
-        """Test with unicode display name values."""
-
-        user = Mock(username="fred")
-        survey_url = "http://a_survey.com"
-        course = Mock(
-            end_of_course_survey_url=survey_url,
-            certificates_display_behavior='end',
-            display_name=u'edx/abc/courseregisters®'
-        )
-        download_url = 'http://s3.edx/cert'
-
-        cert_status = {
-            'status': 'downloadable', 'grade': '67',
-            'download_url': download_url,
-            'mode': 'honor'
-        }
-        LinkedInAddToProfileConfiguration(
-            company_identifier='0_mC_o2MizqdtZEmkVXjH4eYwMj4DnkCWrZP_D9',
-            enabled=True
-        ).save()
-
-        status_dict = _cert_info(user, course, cert_status, 'honor')
-        expected_url = (
-            'http://www.linkedin.com/profile/add'
-            '?_ed=0_mC_o2MizqdtZEmkVXjH4eYwMj4DnkCWrZP_D9&'
-            'pfCertificationName=edX+Honor+Code+Certificate+for+edx%2Fabc%2Fcourseregisters%C2%AE&'
-            'pfCertificationUrl=http%3A%2F%2Fs3.edx%2Fcert&'
-            'source=o'
-        )
-        self.assertEqual(expected_url, status_dict['linked_in_url'])
-
-    def test_linked_in_url_not_exists_without_config(self):
-        user = Mock(username="fred")
-        survey_url = "http://a_survey.com"
-        course = Mock(
-            display_name="Demo Course",
-            end_of_course_survey_url=survey_url,
-            certificates_display_behavior='end'
-        )
-
-        download_url = 'http://s3.edx/cert'
-        cert_status = {
-            'status': 'downloadable', 'grade': '67',
-            'download_url': download_url,
-            'mode': 'verified'
-        }
-
-        self.assertEqual(
-            _cert_info(user, course, cert_status, 'verified'),
-            {
-                'status': 'ready',
-                'show_disabled_download_button': False,
-                'show_download_url': True,
-                'download_url': download_url,
-                'show_survey_button': True,
-                'survey_url': survey_url,
-                'grade': '67',
-                'mode': 'verified',
-                'linked_in_url': None
-            }
-        )
-
-        # Enabling the configuration will cause the LinkedIn
-        # "add to profile" button to appear.
-        # We need to clear the cache again to make sure we
-        # pick up the modified configuration.
-        cache.clear()
-        LinkedInAddToProfileConfiguration(
-            company_identifier='0_mC_o2MizqdtZEmkVXjH4eYwMj4DnkCWrZP_D9',
-            enabled=True
-        ).save()
-
-        status_dict = _cert_info(user, course, cert_status, 'honor')
-        expected_url = (
-            'http://www.linkedin.com/profile/add'
-            '?_ed=0_mC_o2MizqdtZEmkVXjH4eYwMj4DnkCWrZP_D9&'
-            'pfCertificationName=edX+Verified+Certificate+for+Demo+Course&'
-            'pfCertificationUrl=http%3A%2F%2Fs3.edx%2Fcert&'
-            'source=o'
-        )
-        self.assertEqual(expected_url, status_dict['linked_in_url'])
-
-    @ddt.data(
-        ('honor', 'edX Honor Code Certificate for DemoX'),
-        ('verified', 'edX Verified Certificate for DemoX'),
-        ('professional', 'edX Professional Certificate for DemoX'),
-        ('default_mode', 'edX Certificate for DemoX')
-    )
-    @ddt.unpack
-    def test_linked_in_url_certificate_types(self, cert_mode, cert_name):
-        user = Mock(username="fred")
-        course = Mock(
-            display_name='DemoX',
-            end_of_course_survey_url='http://example.com',
-            certificates_display_behavior='end'
-        )
-        cert_status = {
-            'status': 'downloadable',
-            'grade': '67',
-            'download_url': 'http://edx.org',
-            'mode': cert_mode
-        }
-
-        LinkedInAddToProfileConfiguration(
-            company_identifier="abcd123",
-            enabled=True
-        ).save()
-
-        status_dict = _cert_info(user, course, cert_status, cert_mode)
-        self.assertIn(cert_name.replace(' ', '+'), status_dict['linked_in_url'])
-
 
 class DashboardTest(ModuleStoreTestCase):
     """
@@ -518,10 +398,7 @@ class DashboardTest(ModuleStoreTestCase):
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
     def test_linked_in_add_to_profile_btn_not_appearing_without_config(self):
-
-        """
-         without linked-in config don't show Add Certificate to LinkedIn button
-        """
+        # Without linked-in config don't show Add Certificate to LinkedIn button
         self.client.login(username="jack", password="test")
 
         CourseModeFactory.create(
@@ -557,12 +434,8 @@ class DashboardTest(ModuleStoreTestCase):
 
     @unittest.skipUnless(settings.ROOT_URLCONF == 'lms.urls', 'Test only valid in lms')
     def test_linked_in_add_to_profile_btn_with_certificate(self):
-
-        """
-        If user has a certificate with valid linked-in config then Add Certificate to LinkedIn button
-        should be visible. and it has URL value with valid parameters.
-        """
-
+        # If user has a certificate with valid linked-in config then Add Certificate to LinkedIn button
+        # should be visible. and it has URL value with valid parameters.
         self.client.login(username="jack", password="test")
         LinkedInAddToProfileConfiguration(
             company_identifier='0_mC_o2MizqdtZEmkVXjH4eYwMj4DnkCWrZP_D9',

common/djangoapps/student/views.py

@@ -56,7 +56,7 @@ from student.models import (
     CourseEnrollmentAllowed, UserStanding, LoginFailures,
     create_comments_service_user, PasswordHistory, UserSignupSource,
     DashboardConfiguration, LinkedInAddToProfileConfiguration)
-from student.forms import PasswordResetFormNoActive
+from student.forms import AccountCreationForm, PasswordResetFormNoActive
 
 from verify_student.models import SoftwareSecurePhotoVerification, MidcourseReverificationWindow
 from certificates.models import CertificateStatuses, certificate_status_for_student
@@ -86,6 +86,7 @@ from bulk_email.models import Optout, CourseAuthorization
 import shoppingcart
 from openedx.core.djangoapps.user_api.models import UserPreference
 from lang_pref import LANGUAGE_KEY
+from notification_prefs.views import enable_notifications
 
 import track.views
 
@@ -360,6 +361,7 @@ def _cert_info(user, course, cert_status, course_mode):
             linkedin_config = LinkedInAddToProfileConfiguration.current()
             if linkedin_config.enabled:
                 status_dict['linked_in_url'] = linkedin_config.add_to_profile_url(
+                    course.id,
                     course.display_name,
                     cert_status.get('mode'),
                     cert_status['download_url']
@@ -1336,7 +1338,7 @@ def user_signup_handler(sender, **kwargs):  # pylint: disable=unused-argument
             log.info(u'user {} originated from a white labeled "Microsite"'.format(kwargs['instance'].id))
 
 
-def _do_create_account(post_vars, extended_profile=None):
+def _do_create_account(form):
     """
     Given cleaned post variables, create the User and UserProfile objects, as well as the
     registration for this user.
@@ -1345,10 +1347,15 @@ def _do_create_account(post_vars, extended_profile=None):
 
     Note: this function is also used for creating test users.
     """
-    user = User(username=post_vars['username'],
-                email=post_vars['email'],
-                is_active=False)
-    user.set_password(post_vars['password'])
+    if not form.is_valid():
+        raise ValidationError(form.errors)
+
+    user = User(
+        username=form.cleaned_data["username"],
+        email=form.cleaned_data["email"],
+        is_active=False
+    )
+    user.set_password(form.cleaned_data["password"])
     registration = Registration()
 
     # TODO: Rearrange so that if part of the process fails, the whole process fails.
@@ -1357,14 +1364,14 @@ def _do_create_account(post_vars, extended_profile=None):
         user.save()
     except IntegrityError:
         # Figure out the cause of the integrity error
-        if len(User.objects.filter(username=post_vars['username'])) > 0:
+        if len(User.objects.filter(username=user.username)) > 0:
             raise AccountValidationError(
-                _("An account with the Public Username '{username}' already exists.").format(username=post_vars['username']),
+                _("An account with the Public Username '{username}' already exists.").format(username=user.username),
                 field="username"
             )
-        elif len(User.objects.filter(email=post_vars['email'])) > 0:
+        elif len(User.objects.filter(email=user.email)) > 0:
             raise AccountValidationError(
-                _("An account with the Email '{email}' already exists.").format(email=post_vars['email']),
+                _("An account with the Email '{email}' already exists.").format(email=user.email),
                 field="email"
             )
         else:
@@ -1377,25 +1384,17 @@ def _do_create_account(post_vars, extended_profile=None):
 
     registration.register(user)
 
-    profile = UserProfile(user=user)
-    profile.name = post_vars['name']
-    profile.level_of_education = post_vars.get('level_of_education')
-    profile.gender = post_vars.get('gender')
-    profile.mailing_address = post_vars.get('mailing_address')
-    profile.city = post_vars.get('city')
-    profile.country = post_vars.get('country')
-    profile.goals = post_vars.get('goals')
-
-    # add any extended profile information in the denormalized 'meta' field in the profile
+    profile_fields = [
+        "name", "level_of_education", "gender", "mailing_address", "city", "country", "goals",
+        "year_of_birth"
+    ]
+    profile = UserProfile(
+        user=user,
+        **{key: form.cleaned_data.get(key) for key in profile_fields}
+    )
+    extended_profile = form.cleaned_extended_profile
     if extended_profile:
         profile.meta = json.dumps(extended_profile)
-
-    try:
-        profile.year_of_birth = int(post_vars['year_of_birth'])
-    except (ValueError, KeyError):
-        # If they give us garbage, just ignore it instead
-        # of asking them to put an integer.
-        profile.year_of_birth = None
     try:
         profile.save()
     except Exception:  # pylint: disable=broad-except
@@ -1407,15 +1406,23 @@ def _do_create_account(post_vars, extended_profile=None):
     return (user, profile, registration)
 
 
-@csrf_exempt
-def create_account(request, post_override=None):  # pylint: disable-msg=too-many-statements
+def create_account_with_params(request, params):
     """
-    JSON call to create new edX account.
-    Used by form in signup_modal.html, which is included into navigation.html
-    """
-    js = {'success': False}  # pylint: disable-msg=invalid-name
+    Given a request and a dict of parameters (which may or may not have come
+    from the request), create an account for the requesting user, including
+    creating a comments service user object and sending an activation email.
+    This also takes external/third-party auth into account, updates that as
+    necessary, and authenticates the user for the request's session.
 
-    post_vars = post_override if post_override else request.POST
+    Does not return anything.
+
+    Raises AccountValidationError if an account with the username or email
+    specified by params already exists, or ValidationError if any of the given
+    parameters is invalid for any other reason.
+    """
+    # Copy params so we can modify it; we can't just do dict(params) because if
+    # params is request.POST, that results in a dict containing lists of values
+    params = dict(params.items())
 
     # allow for microsites to define their own set of required/optional/hidden fields
     extra_fields = microsite.get_value(
@@ -1424,42 +1431,30 @@ def create_account(request, post_override=None):  # pylint: disable-msg=too-many
     )
 
     if third_party_auth.is_enabled() and pipeline.running(request):
-        post_vars = dict(post_vars.items())
-        post_vars.update({'password': pipeline.make_random_password()})
+        params["password"] = pipeline.make_random_password()
 
     # if doing signup for an external authorization, then get email, password, name from the eamap
     # don't use the ones from the form, since the user could have hacked those
     # unless originally we didn't get a valid email or name from the external auth
+    # TODO: We do not check whether these values meet all necessary criteria, such as email length
     do_external_auth = 'ExternalAuthMap' in request.session
     if do_external_auth:
         eamap = request.session['ExternalAuthMap']
         try:
             validate_email(eamap.external_email)
-            email = eamap.external_email
+            params["email"] = eamap.external_email
         except ValidationError:
-            email = post_vars.get('email', '')
-        if eamap.external_name.strip() == '':
-            name = post_vars.get('name', '')
-        else:
-            name = eamap.external_name
-        password = eamap.internal_password
-        post_vars = dict(post_vars.items())
-        post_vars.update(dict(email=email, name=name, password=password))
-        log.debug(u'In create_account with external_auth: user = %s, email=%s', name, email)
-
-    # Confirm we have a properly formed request
-    for req_field in ['username', 'email', 'password', 'name']:
-        if req_field not in post_vars:
-            js['value'] = _("Error (401 {field}). E-mail us.").format(field=req_field)
-            js['field'] = req_field
-            return JsonResponse(js, status=400)
-
-    if extra_fields.get('honor_code', 'required') == 'required' and \
-            post_vars.get('honor_code', 'false') != u'true':
-        js['value'] = _("To enroll, you must follow the honor code.")
-        js['field'] = 'honor_code'
-        return JsonResponse(js, status=400)
+            pass
+        if eamap.external_name.strip() != '':
+            params["name"] = eamap.external_name
+        params["password"] = eamap.internal_password
+        log.debug(u'In create_account with external_auth: user = %s, email=%s', params["name"], params["email"])
 
+    extended_profile_fields = microsite.get_value('extended_profile_fields', [])
+    enforce_password_policy = (
+        settings.FEATURES.get("ENFORCE_PASSWORD_POLICY", False) and
+        not do_external_auth
+    )
     # Can't have terms of service for certain SHIB users, like at Stanford
     tos_required = (
         not settings.FEATURES.get("AUTH_USE_SHIB") or
@@ -1470,135 +1465,34 @@ def create_account(request, post_override=None):  # pylint: disable-msg=too-many
         )
     )
 
-    if tos_required:
-        if post_vars.get('terms_of_service', 'false') != u'true':
-            js['value'] = _("You must accept the terms of service.")
-            js['field'] = 'terms_of_service'
-            return JsonResponse(js, status=400)
+    form = AccountCreationForm(
+        data=params,
+        extra_fields=extra_fields,
+        extended_profile_fields=extended_profile_fields,
+        enforce_username_neq_password=True,
+        enforce_password_policy=enforce_password_policy,
+        tos_required=tos_required
+    )
 
-    # Confirm appropriate fields are there.
-    # TODO: Check e-mail format is correct.
-    # TODO: Confirm e-mail is not from a generic domain (mailinator, etc.)? Not sure if
-    # this is a good idea
-    # TODO: Check password is sane
-
-    required_post_vars = ['username', 'email', 'name', 'password']
-    required_post_vars += [fieldname for fieldname, val in extra_fields.items()
-                           if val == 'required']
-    if tos_required:
-        required_post_vars.append('terms_of_service')
-
-    for field_name in required_post_vars:
-        if field_name in ('gender', 'level_of_education'):
-            min_length = 1
-        else:
-            min_length = 2
-
-        if field_name not in post_vars or len(post_vars[field_name]) < min_length:
-            error_str = {
-                'username': _('Username must be minimum of two characters long'),
-                'email': _('A properly formatted e-mail is required'),
-                'name': _('Your legal name must be a minimum of two characters long'),
-                'password': _('A valid password is required'),
-                'terms_of_service': _('Accepting Terms of Service is required'),
-                'honor_code': _('Agreeing to the Honor Code is required'),
-                'level_of_education': _('A level of education is required'),
-                'gender': _('Your gender is required'),
-                'year_of_birth': _('Your year of birth is required'),
-                'mailing_address': _('Your mailing address is required'),
-                'goals': _('A description of your goals is required'),
-                'city': _('A city is required'),
-                'country': _('A country is required')
-            }
-
-            if field_name in error_str:
-                js['value'] = error_str[field_name]
-            else:
-                js['value'] = _('You are missing one or more required fields')
-
-            js['field'] = field_name
-            return JsonResponse(js, status=400)
-
-        max_length = 75
-        if field_name == 'username':
-            max_length = 30
-
-        if field_name in ('email', 'username') and len(post_vars[field_name]) > max_length:
-            error_str = {
-                'username': _('Username cannot be more than {num} characters long').format(num=max_length),
-                'email': _('Email cannot be more than {num} characters long').format(num=max_length)
-            }
-            js['value'] = error_str[field_name]
-            js['field'] = field_name
-            return JsonResponse(js, status=400)
-
-    try:
-        validate_email(post_vars['email'])
-    except ValidationError:
-        js['value'] = _("Valid e-mail is required.")
-        js['field'] = 'email'
-        return JsonResponse(js, status=400)
-
-    try:
-        validate_slug(post_vars['username'])
-    except ValidationError:
-        js['value'] = _("Username should only consist of A-Z and 0-9, with no spaces.")
-        js['field'] = 'username'
-        return JsonResponse(js, status=400)
-
-    # enforce password complexity as an optional feature
-    # but not if we're doing ext auth b/c those pws never get used and are auto-generated so might not pass validation
-    if settings.FEATURES.get('ENFORCE_PASSWORD_POLICY', False) and not do_external_auth:
-        try:
-            password = post_vars['password']
-
-            validate_password_length(password)
-            validate_password_complexity(password)
-            validate_password_dictionary(password)
-        except ValidationError, err:
-            js['value'] = _('Password: ') + '; '.join(err.messages)
-            js['field'] = 'password'
-            return JsonResponse(js, status=400)
-
-    # allow microsites to define 'extended profile fields' which are
-    # captured on user signup (for example via an overriden registration.html)
-    # and then stored in the UserProfile
-    extended_profile_fields = microsite.get_value('extended_profile_fields', [])
-    extended_profile = None
-
-    for field in extended_profile_fields:
-        if field in post_vars:
-            if not extended_profile:
-                extended_profile = {}
-            extended_profile[field] = post_vars[field]
-
-    # Make sure that password and username fields do not match
-    username = post_vars['username']
-    password = post_vars['password']
-    if username == password:
-        js['value'] = _("Username and password fields cannot match")
-        js['field'] = 'username'
-        return JsonResponse(js, status=400)
-
-    # Ok, looks like everything is legit.  Create the account.
-    try:
-        with transaction.commit_on_success():
-            ret = _do_create_account(post_vars, extended_profile)
-    except AccountValidationError as exc:
-        return JsonResponse({'success': False, 'value': exc.message, 'field': exc.field}, status=400)
+    with transaction.commit_on_success():
+        ret = _do_create_account(form)
 
     (user, profile, registration) = ret
 
-    dog_stats_api.increment("common.student.account_created")
+    if settings.FEATURES.get('ENABLE_DISCUSSION_EMAIL_DIGEST'):
+        try:
+            enable_notifications(user)
+        except Exception:
+            log.exception("Enable discussion notifications failed for user {id}.".format(id=user.id))
 
-    email = post_vars['email']
+    dog_stats_api.increment("common.student.account_created")
 
     # Track the user's registration
     if settings.FEATURES.get('SEGMENT_IO_LMS') and hasattr(settings, 'SEGMENT_IO_LMS_KEY'):
         tracking_context = tracker.get_tracker().resolve_context()
         analytics.identify(user.id, {
-            'email': email,
-            'username': username,
+            'email': user.email,
+            'username': user.username,
         })
 
         # If the user is registering via 3rd party auth, track which provider they use
@@ -1613,7 +1507,7 @@ def create_account(request, post_override=None):  # pylint: disable-msg=too-many
             "edx.bi.user.account.registered",
             {
                 'category': 'conversion',
-                'label': request.POST.get('course_id'),
+                'label': params.get('course_id'),
                 'provider': provider_name
             },
             context={
@@ -1626,7 +1520,7 @@ def create_account(request, post_override=None):  # pylint: disable-msg=too-many
     create_comments_service_user(user)
 
     context = {
-        'name': post_vars['name'],
+        'name': profile.name,
         'key': registration.activation_key,
     }
 
@@ -1657,16 +1551,11 @@ def create_account(request, post_override=None):  # pylint: disable-msg=too-many
                 user.email_user(subject, message, from_address)
         except Exception:  # pylint: disable=broad-except
             log.error(u'Unable to send activation email to user from "%s"', from_address, exc_info=True)
-            js['value'] = _('Could not send activation e-mail.')
-            # What is the correct status code to use here? I think it's 500, because
-            # the problem is on the server's end -- but also, the account was created.
-            # Seems like the core part of the request was successful.
-            return JsonResponse(js, status=500)
 
     # Immediately after a user creates an account, we log them in. They are only
     # logged in until they close the browser. They can't log in again until they click
     # the activation link from the email.
-    new_user = authenticate(username=post_vars['username'], password=post_vars['password'])
+    new_user = authenticate(username=user.username, password=params['password'])
     login(request, new_user)
     request.session.set_expiry(0)
 
@@ -1679,8 +1568,8 @@ def create_account(request, post_override=None):  # pylint: disable-msg=too-many
         eamap.user = new_user
         eamap.dtsignup = datetime.datetime.now(UTC)
         eamap.save()
-        AUDIT_LOG.info(u"User registered with external_auth %s", post_vars['username'])
-        AUDIT_LOG.info(u'Updated ExternalAuthMap for %s to be %s', post_vars['username'], eamap)
+        AUDIT_LOG.info(u"User registered with external_auth %s", new_user.username)
+        AUDIT_LOG.info(u'Updated ExternalAuthMap for %s to be %s', new_user.username, eamap)
 
         if settings.FEATURES.get('BYPASS_ACTIVATION_EMAIL_FOR_EXTAUTH'):
             log.info('bypassing activation email')
@@ -1688,7 +1577,55 @@ def create_account(request, post_override=None):  # pylint: disable-msg=too-many
             new_user.save()
             AUDIT_LOG.info(u"Login activated on extauth account - {0} ({1})".format(new_user.username, new_user.email))
 
-    dog_stats_api.increment("common.student.account_created")
+
+def set_marketing_cookie(request, response):
+    """
+    Set the login cookie for the edx marketing site on the given response. Its
+    expiration will match that of the given request's session.
+    """
+    if request.session.get_expire_at_browser_close():
+        max_age = None
+        expires = None
+    else:
+        max_age = request.session.get_expiry_age()
+        expires_time = time.time() + max_age
+        expires = cookie_date(expires_time)
+
+    # we want this cookie to be accessed via javascript
+    # so httponly is set to None
+    response.set_cookie(
+        settings.EDXMKTG_COOKIE_NAME,
+        'true',
+        max_age=max_age,
+        expires=expires,
+        domain=settings.SESSION_COOKIE_DOMAIN,
+        path='/',
+        secure=None,
+        httponly=None
+    )
+
+
+@csrf_exempt
+def create_account(request, post_override=None):
+    """
+    JSON call to create new edX account.
+    Used by form in signup_modal.html, which is included into navigation.html
+    """
+    try:
+        create_account_with_params(request, post_override or request.POST)
+    except AccountValidationError as exc:
+        return JsonResponse({'success': False, 'value': exc.message, 'field': exc.field}, status=400)
+    except ValidationError as exc:
+        field, error_list = next(exc.message_dict.iteritems())
+        return JsonResponse(
+            {
+                "success": False,
+                "field": field,
+                "value": error_list[0],
+            },
+            status=400
+        )
+
     redirect_url = try_change_enrollment(request)
 
     # Resume the third-party-auth pipeline if necessary.
@@ -1700,25 +1637,7 @@ def create_account(request, post_override=None):  # pylint: disable-msg=too-many
         'success': True,
         'redirect_url': redirect_url,
     })
-
-    # set the login cookie for the edx marketing site
-    # we want this cookie to be accessed via javascript
-    # so httponly is set to None
-
-    if request.session.get_expire_at_browser_close():
-        max_age = None
-        expires = None
-    else:
-        max_age = request.session.get_expiry_age()
-        expires_time = time.time() + max_age
-        expires = cookie_date(expires_time)
-
-    response.set_cookie(settings.EDXMKTG_COOKIE_NAME,
-                        'true', max_age=max_age,
-                        expires=expires, domain=settings.SESSION_COOKIE_DOMAIN,
-                        path='/',
-                        secure=None,
-                        httponly=None)
+    set_marketing_cookie(request, response)
     return response
 
 
@@ -1757,21 +1676,21 @@ def auto_auth(request):
     role_names = [v.strip() for v in request.GET.get('roles', '').split(',') if v.strip()]
     login_when_done = 'no_login' not in request.GET
 
-    # Get or create the user object
-    post_data = {
-        'username': username,
-        'email': email,
-        'password': password,
-        'name': full_name,
-        'honor_code': u'true',
-        'terms_of_service': u'true',
-    }
+    form = AccountCreationForm(
+        data={
+            'username': username,
+            'email': email,
+            'password': password,
+            'name': full_name,
+        },
+        tos_required=False
+    )
 
     # Attempt to create the account.
     # If successful, this will return a tuple containing
     # the new user object.
     try:
-        user, _profile, reg = _do_create_account(post_data)
+        user, _profile, reg = _do_create_account(form)
     except AccountValidationError:
         # Attempt to retrieve the existing user.
         user = User.objects.get(username=username)

common/djangoapps/util/authentication.py

@@ -1,5 +1,7 @@
 """ Common Authentication Handlers used across projects. """
 from rest_framework import authentication
+from rest_framework.exceptions import AuthenticationFailed
+from rest_framework.compat import oauth2_provider, provider_now
 
 
 class SessionAuthenticationAllowInactiveUser(authentication.SessionAuthentication):
@@ -39,10 +41,42 @@ class SessionAuthenticationAllowInactiveUser(authentication.SessionAuthenticatio
         # Unauthenticated, CSRF validation not required
         # This is where regular `SessionAuthentication` checks that the user is active.
         # We have removed that check in this implementation.
-        if not user:
+        # But we added a check to prevent anonymous users since we require a logged-in account.
+        if not user or user.is_anonymous():
             return None
 
         self.enforce_csrf(request)
 
         # CSRF passed with authenticated user
         return (user, None)
+
+
+class OAuth2AuthenticationAllowInactiveUser(authentication.OAuth2Authentication):
+    """
+    This is a temporary workaround while the is_active field on the user is coupled
+    with whether or not the user has verified ownership of their claimed email address.
+    Once is_active is decoupled from verified_email, we will no longer need this
+    class override.
+
+    But until then, this authentication class ensures that the user is logged in,
+    but does not require that their account "is_active".
+
+    This class can be used for an OAuth2-accessible endpoint that allows users to access
+    that endpoint without having their email verified.  For example, this is used
+    for mobile endpoints.
+
+    """
+    def authenticate_credentials(self, request, access_token):
+        """
+        Authenticate the request, given the access token.
+        Override base class implementation to discard failure if user is inactive.
+        """
+        try:
+            token = oauth2_provider.oauth2.models.AccessToken.objects.select_related('user')
+            # provider_now switches to timezone aware datetime when
+            # the oauth2_provider version supports to it.
+            token = token.get(token=access_token, expires__gt=provider_now())
+        except oauth2_provider.oauth2.models.AccessToken.DoesNotExist:
+            raise AuthenticationFailed('Invalid token')
+
+        return token.user, token

common/djangoapps/util/tests/test_authentication.py

@@ -0,0 +1,63 @@
+"""Tests for util.authentication module."""
+
+from mock import patch
+from django.conf import settings
+from rest_framework import permissions
+from rest_framework.compat import patterns, url
+from rest_framework.tests import test_authentication
+from provider import scope, constants
+from unittest import skipUnless
+
+from ..authentication import OAuth2AuthenticationAllowInactiveUser
+
+
+class OAuth2AuthAllowInactiveUserDebug(OAuth2AuthenticationAllowInactiveUser):
+    """
+    A debug class analogous to the OAuth2AuthenticationDebug class that tests
+    the OAuth2 flow with the access token sent in a query param."""
+    allow_query_params_token = True
+
+
+# The following patch overrides the URL patterns for the MockView class used in
+# rest_framework.tests.test_authentication so that the corresponding AllowInactiveUser
+# classes are tested instead.
+@skipUnless(settings.FEATURES.get('ENABLE_OAUTH2_PROVIDER'), 'OAuth2 not enabled')
+@patch.object(
+    test_authentication,
+    'urlpatterns',
+    patterns(
+        '',
+        url(
+            r'^oauth2-test/$',
+            test_authentication.MockView.as_view(authentication_classes=[OAuth2AuthenticationAllowInactiveUser])
+        ),
+        url(
+            r'^oauth2-test-debug/$',
+            test_authentication.MockView.as_view(authentication_classes=[OAuth2AuthAllowInactiveUserDebug])
+        ),
+        url(
+            r'^oauth2-with-scope-test/$',
+            test_authentication.MockView.as_view(
+                authentication_classes=[OAuth2AuthenticationAllowInactiveUser],
+                permission_classes=[permissions.TokenHasReadWriteScope]
+            )
+        )
+    )
+)
+class OAuth2AuthenticationAllowInactiveUserTestCase(test_authentication.OAuth2Tests):
+    """
+    Tests the OAuth2AuthenticationAllowInactiveUser class by running all the existing tests in
+    OAuth2Tests but with the is_active flag on the user set to False.
+    """
+    def setUp(self):
+        super(OAuth2AuthenticationAllowInactiveUserTestCase, self).setUp()
+
+        # set the user's is_active flag to False.
+        self.user.is_active = False
+        self.user.save()
+
+        # Override the SCOPE_NAME_DICT setting for tests for oauth2-with-scope-test.  This is
+        # needed to support READ and WRITE scopes as they currently aren't supported by the
+        # edx-auth2-provider, and their scope values collide with other scopes defined in the
+        # edx-auth2-provider.
+        scope.SCOPE_NAME_DICT = {'read': constants.READ, 'write': constants.WRITE}

common/lib/capa/capa/capa_problem.py

@@ -710,12 +710,14 @@ class LoncapaProblem(object):
             hint = ''
             hintmode = None
             input_id = problemtree.get('id')
+            answervariable = None
             if problemid in self.correct_map:
                 pid = input_id
                 status = self.correct_map.get_correctness(pid)
                 msg = self.correct_map.get_msg(pid)
                 hint = self.correct_map.get_hint(pid)
                 hintmode = self.correct_map.get_hintmode(pid)
+                answervariable = self.correct_map.get_property(pid, 'answervariable')
 
             value = ""
             if self.student_answers and problemid in self.student_answers:
@@ -730,6 +732,7 @@ class LoncapaProblem(object):
                 'status': status,
                 'id': input_id,
                 'input_state': self.input_state[input_id],
+                'answervariable': answervariable,
                 'feedback': {
                     'message': msg,
                     'hint': hint,

common/lib/capa/capa/correctmap.py

@@ -46,6 +46,7 @@ class CorrectMap(object):
         hint='',
         hintmode=None,
         queuestate=None,
+        answervariable=None,    # pylint: disable=C0330
         **kwargs
     ):
 
@@ -57,6 +58,7 @@ class CorrectMap(object):
                 'hint': hint,
                 'hintmode': hintmode,
                 'queuestate': queuestate,
+                'answervariable': answervariable,
             }
 
     def __repr__(self):

common/lib/capa/capa/customrender.py

@@ -6,8 +6,6 @@ These tags do not have state, so they just get passed the system (for access to
 and the xml element.
 """
 
-from .registry import TagRegistry
-
 import logging
 import re
 
@@ -137,3 +135,35 @@ class TargetedFeedbackRenderer(object):
         return xhtml
 
 registry.register(TargetedFeedbackRenderer)
+
+#-----------------------------------------------------------------------------
+
+
+class ClarificationRenderer(object):
+    """
+    A clarification appears as an inline icon which reveals more information when the user
+    hovers over it.
+
+    e.g. <p>Enter the ROA <clarification>Return on Assets</clarification> for 2015:</p>
+    """
+    tags = ['clarification']
+
+    def __init__(self, system, xml):
+        self.system = system
+        # Get any text content found inside this tag prior to the first child tag. It may be a string or None type.
+        initial_text = xml.text if xml.text else ''
+        self.inner_html = initial_text + ''.join(etree.tostring(element) for element in xml)  # pylint: disable=no-member
+        self.tail = xml.tail
+
+    def get_html(self):
+        """
+        Return the contents of this tag, rendered to html, as an etree element.
+        """
+        context = {'clarification': self.inner_html}
+        html = self.system.render_template("clarification.html", context)
+        xml = etree.XML(html)  # pylint: disable=no-member
+        # We must include any text that was following our original <clarification>...</clarification> XML node.:
+        xml.tail = self.tail
+        return xml
+
+registry.register(ClarificationRenderer)

common/lib/capa/capa/inputtypes.py

@@ -213,6 +213,7 @@ class InputTypeBase(object):
         self.hint = feedback.get('hint', '')
         self.hintmode = feedback.get('hintmode', None)
         self.input_state = state.get('input_state', {})
+        self.answervariable = state.get("answervariable", None)
 
         # put hint above msg if it should be displayed
         if self.hintmode == 'always':
@@ -310,6 +311,8 @@ class InputTypeBase(object):
             (a, v) for (a, v) in self.loaded_attributes.iteritems() if a in self.to_render
         )
         context.update(self._extra_context())
+        if self.answervariable:
+            context.update({'answervariable': self.answervariable})
         return context
 
     def _extra_context(self):
@@ -1013,8 +1016,6 @@ class Schematic(InputTypeBase):
         ]
 
     def _extra_context(self):
-        """
-        """
         context = {
             'setup_script': '{static_url}js/capa/schematicinput.js'.format(
                 static_url=self.capa_system.STATIC_URL),
@@ -1407,8 +1408,6 @@ class EditAMoleculeInput(InputTypeBase):
                 Attribute('missing', None)]
 
     def _extra_context(self):
-        """
-        """
         context = {
             'applet_loader': '{static_url}js/capa/editamolecule.js'.format(
                 static_url=self.capa_system.STATIC_URL),
@@ -1443,8 +1442,6 @@ class DesignProtein2dInput(InputTypeBase):
                 ]
 
     def _extra_context(self):
-        """
-        """
         context = {
             'applet_loader': '{static_url}js/capa/design-protein-2d.js'.format(
                 static_url=self.capa_system.STATIC_URL),
@@ -1479,8 +1476,6 @@ class EditAGeneInput(InputTypeBase):
                 ]
 
     def _extra_context(self):
-        """
-            """
         context = {
             'applet_loader': '{static_url}js/capa/edit-a-gene.js'.format(
                 static_url=self.capa_system.STATIC_URL),

common/lib/capa/capa/responsetypes.py

@@ -1097,6 +1097,9 @@ class OptionResponse(LoncapaResponse):
                 cmap.set(aid, 'correct')
             else:
                 cmap.set(aid, 'incorrect')
+            answer_variable = self.get_student_answer_variable_name(student_answers, aid)
+            if answer_variable:
+                cmap.set_property(aid, 'answervariable', answer_variable)
         return cmap
 
     def get_answers(self):
@@ -1105,6 +1108,18 @@ class OptionResponse(LoncapaResponse):
         # log.debug('%s: expected answers=%s' % (unicode(self),amap))
         return amap
 
+    def get_student_answer_variable_name(self, student_answers, aid):
+        """
+        Return student answers variable name if exist in context else None.
+        """
+        if aid in student_answers:
+            for key, val in self.context.iteritems():
+                # convert val into unicode because student answer always be a unicode string
+                # even it is a list, dict etc.
+                if unicode(val) == student_answers[aid]:
+                    return '$' + key
+        return None
+
 #-----------------------------------------------------------------------------
 
 

common/lib/capa/capa/templates/clarification.html

@@ -0,0 +1,5 @@
+<span class="clarification" tabindex="0" role="note" aria-label="Clarification">
+    <i data-tooltip="${clarification | h}" data-tooltip-show-on-click="true"
+       class="fa fa-info-circle" aria-hidden="true"></i>
+    <span class="sr">(${clarification})</span>
+</span>

common/lib/capa/capa/templates/formulaequationinput.html

@@ -25,7 +25,7 @@
 
       <div id="input_${id}_preview" class="equation">
         \[\]
-        <img src="${STATIC_URL}images/spinner.gif" class="loading"/>
+        <img src="${STATIC_URL}images/spinner.gif" class="loading" alt="Loading"/>
       </div>
     <p id="answer_${id}" class="answer"></p>
   </div>

common/lib/capa/capa/templates/imageinput.html

@@ -22,6 +22,7 @@
         src="${STATIC_URL}images/green-pointer.png"
         id="cross_${id}"
         style="position: absolute; top: ${gy}px; left: ${gx}px;"
+        alt="Selection indicator"
       />
     </div>
     <div

common/lib/capa/capa/templates/matlabinput.html

@@ -104,6 +104,7 @@
                   matlab_result_task = new PendingMatlabResult(get_callback);
                   matlab_result_task.task_poller.start();
               } else {
+                  // Used response.message because input_ajax is returning "message"
                   gentle_alert(problem_elt, response.message);
               }
           };
@@ -115,7 +116,8 @@
                       {'submission': submission}, plot_callback);
               }
               else {
-                  gentle_alert(problem_elt, response.message);
+                  // Used response.msg because problem_save is returning "msg" instead of "message"
+                  gentle_alert(problem_elt, response.msg);
               }
           };
 

common/lib/capa/capa/templates/optioninput.html

@@ -5,7 +5,7 @@
       <option value="option_${id}_dummy_default">  </option>
       % for option_id, option_description in options:
           <option value="${option_id}"
-          % if (option_id==value):
+          % if (option_id==value or option_id==answervariable):
         	selected="true"
           % endif
           > ${option_description}</option>

common/lib/capa/capa/tests/response_xml_factory.py

@@ -254,27 +254,6 @@ class CustomResponseXMLFactory(ResponseXMLFactory):
         return ResponseXMLFactory.textline_input_xml(**kwargs)
 
 
-class SymbolicResponseXMLFactory(ResponseXMLFactory):
-    """ Factory for creating <symbolicresponse> XML trees """
-
-    def create_response_element(self, **kwargs):
-        cfn = kwargs.get('cfn', None)
-        answer = kwargs.get('answer', None)
-        options = kwargs.get('options', None)
-
-        response_element = etree.Element("symbolicresponse")
-        if cfn:
-            response_element.set('cfn', str(cfn))
-        if answer:
-            response_element.set('answer', str(answer))
-        if options:
-            response_element.set('options', str(options))
-        return response_element
-
-    def create_input_element(self, **kwargs):
-        return ResponseXMLFactory.textline_input_xml(**kwargs)
-
-
 class SchematicResponseXMLFactory(ResponseXMLFactory):
     """ Factory for creating <schematicresponse> XML trees """
 

common/lib/capa/capa/tests/test_html_render.py

@@ -27,8 +27,7 @@ class CapaHtmlRenderTest(unittest.TestCase):
 
         # Render the HTML
         etree.XML(problem.get_html())
-        # expect that we made it here without blowing up
-        self.assertTrue(True)
+        # TODO: This test should inspect the rendered html and assert one or more things about it
 
     def test_include_html(self):
         # Create a test file to include

common/lib/capa/capa/tests/test_responsetypes.py

@@ -23,6 +23,23 @@ import calc
 from capa.responsetypes import LoncapaProblemError, \
     StudentInputError, ResponseError
 from capa.correctmap import CorrectMap
+from capa.tests.response_xml_factory import (
+    AnnotationResponseXMLFactory,
+    ChoiceResponseXMLFactory,
+    CodeResponseXMLFactory,
+    ChoiceTextResponseXMLFactory,
+    CustomResponseXMLFactory,
+    FormulaResponseXMLFactory,
+    ImageResponseXMLFactory,
+    JavascriptResponseXMLFactory,
+    MultipleChoiceResponseXMLFactory,
+    NumericalResponseXMLFactory,
+    OptionResponseXMLFactory,
+    SchematicResponseXMLFactory,
+    StringResponseXMLFactory,
+    SymbolicResponseXMLFactory,
+    TrueFalseResponseXMLFactory,
+)
 from capa.util import convert_files_to_filenames
 from capa.util import compare_with_tolerance
 from capa.xqueue_interface import dateformat
@@ -77,7 +94,6 @@ class ResponseTest(unittest.TestCase):
 
 
 class MultiChoiceResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import MultipleChoiceResponseXMLFactory
     xml_factory_class = MultipleChoiceResponseXMLFactory
 
     def test_multiple_choice_grade(self):
@@ -99,7 +115,6 @@ class MultiChoiceResponseTest(ResponseTest):
 
 
 class TrueFalseResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import TrueFalseResponseXMLFactory
     xml_factory_class = TrueFalseResponseXMLFactory
 
     def test_true_false_grade(self):
@@ -139,7 +154,6 @@ class TrueFalseResponseTest(ResponseTest):
 
 
 class ImageResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import ImageResponseXMLFactory
     xml_factory_class = ImageResponseXMLFactory
 
     def test_rectangle_grade(self):
@@ -203,7 +217,6 @@ class ImageResponseTest(ResponseTest):
 
 
 class SymbolicResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import SymbolicResponseXMLFactory
     xml_factory_class = SymbolicResponseXMLFactory
 
     def test_grade_single_input_correct(self):
@@ -321,7 +334,6 @@ class SymbolicResponseTest(ResponseTest):
 
 
 class OptionResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import OptionResponseXMLFactory
     xml_factory_class = OptionResponseXMLFactory
 
     def test_grade(self):
@@ -347,12 +359,31 @@ class OptionResponseTest(ResponseTest):
         self.assert_grade(problem, "hasn\'t", "correct")
         self.assert_grade(problem, "has'nt", "incorrect")
 
+    def test_variable_options(self):
+        """
+        Test that if variable are given in option response then correct map must contain answervariable value.
+        """
+        script = textwrap.dedent("""\
+        a = 1000
+        b = a*2
+        c = a*3
+        """)
+        problem = self.build_problem(
+            options=['$a', '$b', '$c'],
+            correct_option='$a',
+            script=script
+        )
+
+        input_dict = {'1_2_1': '1000'}
+        correct_map = problem.grade_answers(input_dict)
+        self.assertEqual(correct_map.get_correctness('1_2_1'), 'correct')
+        self.assertEqual(correct_map.get_property('1_2_1', 'answervariable'), '$a')
+
 
 class FormulaResponseTest(ResponseTest):
     """
     Test the FormulaResponse class
     """
-    from capa.tests.response_xml_factory import FormulaResponseXMLFactory
     xml_factory_class = FormulaResponseXMLFactory
 
     def test_grade(self):
@@ -501,7 +532,6 @@ class FormulaResponseTest(ResponseTest):
 
 
 class StringResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import StringResponseXMLFactory
     xml_factory_class = StringResponseXMLFactory
 
     def test_backward_compatibility_for_multiple_answers(self):
@@ -851,7 +881,6 @@ class StringResponseTest(ResponseTest):
 
 
 class CodeResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import CodeResponseXMLFactory
     xml_factory_class = CodeResponseXMLFactory
 
     def setUp(self):
@@ -1043,7 +1072,6 @@ class CodeResponseTest(ResponseTest):
 
 
 class ChoiceResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import ChoiceResponseXMLFactory
     xml_factory_class = ChoiceResponseXMLFactory
 
     def test_radio_group_grade(self):
@@ -1086,7 +1114,6 @@ class ChoiceResponseTest(ResponseTest):
 
 
 class JavascriptResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import JavascriptResponseXMLFactory
     xml_factory_class = JavascriptResponseXMLFactory
 
     def test_grade(self):
@@ -1127,7 +1154,6 @@ class JavascriptResponseTest(ResponseTest):
 
 
 class NumericalResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import NumericalResponseXMLFactory
     xml_factory_class = NumericalResponseXMLFactory
 
     # We blend the line between integration (using evaluator) and exclusively
@@ -1352,7 +1378,6 @@ class NumericalResponseTest(ResponseTest):
 
 
 class CustomResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import CustomResponseXMLFactory
     xml_factory_class = CustomResponseXMLFactory
 
     def test_inline_code(self):
@@ -1904,7 +1929,6 @@ class SchematicResponseTest(ResponseTest):
     """
     Class containing setup and tests for Schematic responsetype.
     """
-    from capa.tests.response_xml_factory import SchematicResponseXMLFactory
     xml_factory_class = SchematicResponseXMLFactory
 
     def test_grade(self):
@@ -1955,7 +1979,6 @@ class SchematicResponseTest(ResponseTest):
 
 
 class AnnotationResponseTest(ResponseTest):
-    from capa.tests.response_xml_factory import AnnotationResponseXMLFactory
     xml_factory_class = AnnotationResponseXMLFactory
 
     def test_grade(self):
@@ -1997,7 +2020,6 @@ class ChoiceTextResponseTest(ResponseTest):
     Class containing setup and tests for ChoiceText responsetype.
     """
 
-    from response_xml_factory import ChoiceTextResponseXMLFactory
     xml_factory_class = ChoiceTextResponseXMLFactory
 
     # `TEST_INPUTS` is a dictionary mapping from
@@ -2168,13 +2190,6 @@ class ChoiceTextResponseTest(ResponseTest):
         with self.assertRaises(Exception):
             self.build_problem(type="invalidtextgroup")
 
-    def test_valid_xml(self):
-        """
-        Test that `build_problem` builds valid xml
-        """
-        self.build_problem()
-        self.assertTrue(True)
-
     def test_unchecked_input_not_validated(self):
         """
         Test that a student can have a non numeric answer in an unselected

common/lib/xmodule/xmodule/css/capa/display.scss

@@ -162,6 +162,12 @@ div.problem {
         white-space: nowrap;
         overflow: hidden;
       }
+      span.clarification i {
+        font-style: normal;
+        &:hover {
+          color: $blue;
+        }
+      }
     }
 
     &.unanswered {

common/lib/xmodule/xmodule/exceptions.py

@@ -53,4 +53,4 @@ class HeartbeatFailure(Exception):
         In addition to a msg, provide the name of the service.
         """
         self.service = service
-        return super(HeartbeatFailure, self).__init__(msg)
+        super(HeartbeatFailure, self).__init__(msg)

common/lib/xmodule/xmodule/js/spec/video/video_context_menu_spec.js

@@ -217,43 +217,30 @@
                 expect(menuSubmenuItem).not.toHaveClass('is-opened');
             });
 
-            // Flaky-test resulting in timeout errors. Disabled 09/18/2014
-            // See TNL-439
-            xit('mouse left/right-clicking behaves as expected on play/pause menu item', function () {
+            it('mouse left/right-clicking behaves as expected on play/pause menu item', function () {
                 var menuItem = menuItems.first();
-                runs(function () {
-                    // Left-click on play
-                    menuItem.click();
+                spyOn(state.videoPlayer, 'play').andCallFake(function () {
+                    state.videoControl.isPlaying = true;
+                    state.el.trigger('play');
                 });
-
-                waitsFor(function () {
-                    return state.videoPlayer.isPlaying();
-                }, 'video to start playing', 200);
-
-                runs(function () {
-                    expect(menuItem).toHaveText('Pause');
-                    openMenu();
-                    // Left-click on pause
-                    menuItem.click();
-                });
-
-                waitsFor(function () {
-                    return !state.videoPlayer.isPlaying();
-                }, 'video to start playing', 200);
-
-                runs(function () {
-                    expect(menuItem).toHaveText('Play');
-                    // Right-click on play
-                    menuItem.trigger('contextmenu');
-                });
-
-                waitsFor(function () {
-                    return state.videoPlayer.isPlaying();
-                }, 'video to start playing', 200);
-
-                runs(function () {
-                    expect(menuItem).toHaveText('Pause');
+                spyOn(state.videoPlayer, 'pause').andCallFake(function () {
+                    state.videoControl.isPlaying = false;
+                    state.el.trigger('pause');
                 });
+                // Left-click on play
+                menuItem.click();
+                expect(state.videoPlayer.play).toHaveBeenCalled();
+                expect(menuItem).toHaveText('Pause');
+                openMenu();
+                // Left-click on pause
+                menuItem.click();
+                expect(state.videoPlayer.pause).toHaveBeenCalled();
+                expect(menuItem).toHaveText('Play');
+                state.videoPlayer.play.reset();
+                // Right-click on play
+                menuItem.trigger('contextmenu');
+                expect(state.videoPlayer.play).toHaveBeenCalled();
+                expect(menuItem).toHaveText('Pause');
             });
 
             it('mouse left/right-clicking behaves as expected on mute/unmute menu item', function () {

common/lib/xmodule/xmodule/js/spec/video/video_player_spec.js

@@ -367,6 +367,8 @@ function (VideoPlayer) {
             beforeEach(function () {
                 state = jasmine.initializePlayer();
                 state.videoEl = $('video, iframe');
+                jasmine.Clock.useMock();
+                spyOn(state.videoPlayer, 'duration').andReturn(120);
             });
 
             describe('when the video is playing', function () {
@@ -376,9 +378,7 @@ function (VideoPlayer) {
                     });
 
                     waitsFor(function () {
-                        var duration = state.videoPlayer.duration();
-
-                        return duration > 0 && state.videoPlayer.isPlaying();
+                        return state.videoPlayer.isPlaying();
                     }, 'video didn\'t start playing', WAIT_TIMEOUT);
                 });
 
@@ -388,125 +388,13 @@ function (VideoPlayer) {
                         spyOn(state.videoPlayer, 'stopTimer');
                         spyOn(state.videoPlayer, 'runTimer');
                         state.videoPlayer.seekTo(10);
+                        // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                        // That's why we have to do this tick(300).
+                        jasmine.Clock.tick(300);
+                        expect(state.videoPlayer.currentTime).toBe(10);
+                        expect(state.videoPlayer.stopTimer).toHaveBeenCalled();
+                        expect(state.videoPlayer.runTimer).toHaveBeenCalled();
                     });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 10;
-                    }, 'currentTime is less than 10 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        expect(state.videoPlayer.stopTimer)
-                            .toHaveBeenCalled();
-                        expect(state.videoPlayer.runTimer)
-                            .toHaveBeenCalled();
-                    });
-                });
-
-                // as per TNL-439 this test is deemed flaky and needs to be fixed.
-                // disabled 09/18/2014
-                xit('slider event causes log update', function () {
-                    runs(function () {
-                        spyOn(state.videoPlayer, 'log');
-                        state.videoProgressSlider.onSlide(
-                            jQuery.Event('slide'), { value: 2 }
-                        );
-                    });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 2;
-                    }, 'currentTime is less than 2 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        // Depending on the browser, the object of arrays may list the
-                        // arrays in a different order. Find the array that is relevent
-                        // to onSeek. Fail if that is not found.
-                        var seekVideoArgIndex
-                        for(var i = 0; i < state.videoPlayer.log.calls.length; i++){
-                            if (state.videoPlayer.log.calls[i].args[0] == 'seek_video') {
-                                seekVideoArgIndex = i
-                                break;
-                            }
-                        }
-
-                        expect(seekVideoArgIndex).toBeDefined;
-
-                        var args = state.videoPlayer.log.calls[seekVideoArgIndex].args;
-
-                        expect(args[1].old_time).toBeLessThan(2);
-                        expect(args[1].new_time).toBe(2);
-                        expect(args[1].type).toBe('onSlideSeek');
-                    });
-                });
-
-                // as per TNL-439 this test is deemed flaky and needs to be fixed.
-                // disabled 09/18/2014
-                xit('seek the player', function () {
-                    runs(function () {
-                        spyOn(state.videoPlayer.player, 'seekTo')
-                            .andCallThrough();
-                        state.videoProgressSlider.onSlide(
-                            jQuery.Event('slide'), { value: 30 }
-                        );
-                    });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 30;
-                    }, 'currentTime is less than 30 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        expect(state.videoPlayer.player.seekTo)
-                            .toHaveBeenCalledWith(30, true);
-                    });
-                });
-
-                // as per TNL-439 this test is deemed flaky and needs to be fixed.
-                // disabled 09/18/2014
-                xit('call updatePlayTime on player', function () {
-                    runs(function () {
-                        spyOn(state.videoPlayer, 'updatePlayTime')
-                            .andCallThrough();
-                        state.videoProgressSlider.onSlide(
-                            jQuery.Event('slide'), { value: 30 }
-                        );
-                    });
-
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 30;
-                    }, 'currentTime is less than 30 seconds', WAIT_TIMEOUT);
-
-                    runs(function () {
-                        expect(state.videoPlayer.updatePlayTime)
-                            .toHaveBeenCalledWith(30, true);
-                    });
-                });
-            });
-
-            // Disabled 10/25/13 due to flakiness in master
-            xit(
-                'when the player is not playing: set the current time',
-                function ()
-            {
-                runs(function () {
-                    state.videoProgressSlider.onSlide(
-                        jQuery.Event('slide'), { value: 20 }
-                    );
-                    state.videoPlayer.pause();
-                    state.videoProgressSlider.onSlide(
-                        jQuery.Event('slide'), { value: 10 }
-                    );
-
-                    waitsFor(function () {
-                        return Math.round(state.videoPlayer.currentTime) === 10;
-                    }, 'currentTime got updated', 10000);
-                });
-            });
-
-            // as per TNL-439 these tests are deemed flaky and needs to be fixed.
-            // disabled 09/18/2014
-            xdescribe('when the video is not playing', function () {
-                beforeEach(function () {
-                    spyOn(state.videoPlayer, 'setPlaybackRate')
-                        .andCallThrough();
                 });
 
                 it('slider event causes log update', function () {
@@ -515,23 +403,90 @@ function (VideoPlayer) {
                         state.videoProgressSlider.onSlide(
                             jQuery.Event('slide'), { value: 2 }
                         );
+                        // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                        // That's why we have to do this tick(300).
+                        jasmine.Clock.tick(300);
+                        expect(state.videoPlayer.currentTime).toBe(2);
+
+                        expect(state.videoPlayer.log).toHaveBeenCalledWith('seek_video', {
+                            old_time: jasmine.any(Number),
+                            new_time: 2,
+                            type: 'onSlideSeek'
+                        });
                     });
+                });
 
-                    waitsFor(function () {
-                        return state.videoPlayer.currentTime >= 2;
-                    }, 'currentTime is less than 2 seconds', WAIT_TIMEOUT);
-
+                it('seek the player', function () {
                     runs(function () {
-                        expect(state.videoPlayer.log).toHaveBeenCalledWith(
-                            'seek_video', {
-                                old_time: 0,
-                                new_time: 2,
-                                type: 'onSlideSeek'
-                            }
+                        spyOn(state.videoPlayer.player, 'seekTo').andCallThrough();
+                        state.videoProgressSlider.onSlide(
+                            jQuery.Event('slide'), { value: 30 }
                         );
+                        // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                        // That's why we have to do this tick(300).
+                        jasmine.Clock.tick(300);
+                        expect(state.videoPlayer.currentTime).toBe(30);
+                        expect(state.videoPlayer.player.seekTo).toHaveBeenCalledWith(30, true);
                     });
                 });
 
+                it('call updatePlayTime on player', function () {
+                    runs(function () {
+                        spyOn(state.videoPlayer, 'updatePlayTime').andCallThrough();
+                        state.videoProgressSlider.onSlide(
+                            jQuery.Event('slide'), { value: 30 }
+                        );
+                        // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                        // That's why we have to do this tick(300).
+                        jasmine.Clock.tick(300);
+                        expect(state.videoPlayer.currentTime).toBe(30);
+                        expect(state.videoPlayer.updatePlayTime).toHaveBeenCalledWith(30, true);
+                    });
+                });
+            });
+
+            it('when the player is not playing: set the current time', function () {
+                state.videoProgressSlider.onSlide(
+                    jQuery.Event('slide'), { value: 20 }
+                );
+                // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                // That's why we have to do this tick(300).
+                jasmine.Clock.tick(300);
+                state.videoPlayer.pause();
+                expect(state.videoPlayer.currentTime).toBe(20);
+                state.videoProgressSlider.onSlide(
+                    jQuery.Event('slide'), { value: 10 }
+                );
+                // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                // That's why we have to do this tick(300).
+                jasmine.Clock.tick(300);
+                expect(state.videoPlayer.currentTime).toBe(10);
+            });
+
+            describe('when the video is not playing', function () {
+                beforeEach(function () {
+                    spyOn(state.videoPlayer, 'setPlaybackRate')
+                        .andCallThrough();
+                });
+
+                it('slider event causes log update', function () {
+                    spyOn(state.videoPlayer, 'log');
+                    state.videoProgressSlider.onSlide(
+                        jQuery.Event('slide'), { value: 2 }
+                    );
+                    // Video player uses _.debounce (with a wait time in 300 ms) for seeking.
+                    // That's why we have to do this tick(300).
+                    jasmine.Clock.tick(300);
+                    expect(state.videoPlayer.currentTime).toBe(2);
+                    expect(state.videoPlayer.log).toHaveBeenCalledWith(
+                        'seek_video', {
+                            old_time: 0,
+                            new_time: 2,
+                            type: 'onSlideSeek'
+                        }
+                    );
+                });
+
                 it('video has a correct speed', function () {
                     state.speed = '2.0';
                     state.videoPlayer.onPlay();

common/lib/xmodule/xmodule/js/spec/video/video_quality_control_spec.js

@@ -92,6 +92,19 @@
                 qualityControl.el.click();
                 expect(player.setPlaybackQuality).toHaveBeenCalledWith('large');
             });
+
+            it('quality control is active if HD is available',
+                function () {
+                 player.getAvailableQualityLevels.andReturn(
+                     ['highres', 'hd1080', 'hd720']
+                 );
+
+                 qualityControl.quality = 'highres';
+
+                 videoPlayer.onPlay();
+                 expect(qualityControl.el).toHaveClass('active');
+            });
+
         });
 
         describe('constructor, HTML5 mode', function () {

common/lib/xmodule/xmodule/js/src/capa/display.coffee

@@ -34,6 +34,12 @@ class @Problem
     @$('div.action input.reset').click @reset
     @$('div.action button.show').click @show
     @$('div.action input.save').click @save
+    # Accessibility helper for sighted keyboard users to show <clarification> tooltips on focus:
+    @$('.clarification').focus (ev) =>
+      icon = $(ev.target).children "i"
+      window.globalTooltipManager.openTooltip icon
+    @$('.clarification').blur (ev) =>
+      window.globalTooltipManager.hide()
 
     @bindResetCorrectness()
 

common/lib/xmodule/xmodule/js/src/video/05_video_quality_control.js

@@ -103,6 +103,7 @@ function () {
         // HD qualities are available, show video quality control.
         if (this.config.availableHDQualities.length > 0) {
             this.trigger('videoQualityControl.showQualityControl');
+            this.trigger('videoQualityControl.onQualityChange', this.videoQualityControl.quality);
         }
         // On initialization, force the video quality to be 'large' instead of
         // 'default'. Otherwise, the player will sometimes switch to HD
@@ -115,7 +116,6 @@ function () {
     function onQualityChange(value) {
         var controlStateStr;
         this.videoQualityControl.quality = value;
-
         if (_.contains(this.config.availableHDQualities, value)) {
             controlStateStr = gettext('HD on');
             this.videoQualityControl.el
@@ -141,6 +141,7 @@ function () {
         event.preventDefault();
 
         newQuality = isHD ? 'large' : 'highres';
+        
         this.trigger('videoPlayer.handlePlaybackQualityChange', newQuality);
     }
 

common/lib/xmodule/xmodule/modulestore/__init__.py

@@ -777,10 +777,8 @@ class ModuleStoreRead(ModuleStoreAssetBase):
 
         for key, criteria in qualifiers.iteritems():
             is_set, value = _is_set_on(key)
-
             if isinstance(criteria, dict) and '$exists' in criteria and criteria['$exists'] == is_set:
                 continue
-
             if not is_set:
                 return False
             if not self._value_matches(value, criteria):

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -423,11 +423,12 @@ class SplitBulkWriteMixin(BulkOperationsMixin):
                     ids.remove(definition_id)
                     definitions.append(definition)
 
-        # Query the db for the definitions.
-        defs_from_db = self.db_connection.get_definitions(list(ids))
-        # Add the retrieved definitions to the cache.
-        bulk_write_record.definitions.update({d.get('_id'): d for d in defs_from_db})
-        definitions.extend(defs_from_db)
+        if len(ids):
+            # Query the db for the definitions.
+            defs_from_db = self.db_connection.get_definitions(list(ids))
+            # Add the retrieved definitions to the cache.
+            bulk_write_record.definitions.update({d.get('_id'): d for d in defs_from_db})
+            definitions.extend(defs_from_db)
         return definitions
 
     def update_definition(self, course_key, definition):
@@ -683,7 +684,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         connection.close()
 
     def cache_items(self, system, base_block_ids, course_key, depth=0, lazy=True):
-        '''
+        """
         Handles caching of items once inheritance and any other one time
         per course per fetch operations are done.
 
@@ -692,8 +693,8 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
             base_block_ids: list of BlockIds to fetch
             course_key: the destination course providing the context
             depth: how deep below these to prefetch
-            lazy: whether to fetch definitions or use placeholders
-        '''
+            lazy: whether to load definitions now or later
+        """
         with self.bulk_operations(course_key, emit_signals=False):
             new_module_data = {}
             for block_id in base_block_ids:
@@ -704,13 +705,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
                     new_module_data
                 )
 
-            # This code supports lazy loading, where the descendent definitions aren't loaded
+            # This method supports lazy loading, where the descendent definitions aren't loaded
             # until they're actually needed.
-            # However, assume that depth == 0 means no depth is specified and depth != 0 means
-            # a depth *is* specified. If a non-zero depth is specified, force non-lazy definition
-            # loading in order to populate the definition cache for later access.
-            load_definitions_now = depth != 0 or not lazy
-            if load_definitions_now:
+            if not lazy:
                 # Non-lazy loading: Load all descendants by id.
                 descendent_definitions = self.get_definitions(
                     course_key,
@@ -734,15 +731,16 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
             return system.module_data
 
     @contract(course_entry=CourseEnvelope, block_keys="list(BlockKey)", depth="int | None")
-    def _load_items(self, course_entry, block_keys, depth=0, lazy=True, **kwargs):
-        '''
+    def _load_items(self, course_entry, block_keys, depth=0, **kwargs):
+        """
         Load & cache the given blocks from the course. May return the blocks in any order.
 
-        Load the definitions into each block if lazy is False;
-        otherwise, use the lazy definition placeholder.
-        '''
+        Load the definitions into each block if lazy is in kwargs and is False;
+        otherwise, do not load the definitions - they'll be loaded later when needed.
+        """
         runtime = self._get_cache(course_entry.structure['_id'])
         if runtime is None:
+            lazy = kwargs.pop('lazy', True)
             runtime = self.create_runtime(course_entry, lazy)
             self._add_cache(course_entry.structure['_id'], runtime)
             self.cache_items(runtime, block_keys, course_entry.course_key, depth, lazy)
@@ -786,7 +784,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
             self.request_cache.data['course_cache'] = {}
 
     def _lookup_course(self, course_key):
-        '''
+        """
         Decode the locator into the right series of db access. Does not
         return the CourseDescriptor! It returns the actual db json from
         structures.
@@ -797,7 +795,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         reference)
 
         :param course_key: any subclass of CourseLocator
-        '''
+        """
         if course_key.org and course_key.course and course_key.run:
             if course_key.branch is None:
                 raise InsufficientSpecificationError(course_key)
@@ -864,7 +862,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
             locator = locator_factory(structure_info, branch)
             envelope = CourseEnvelope(locator, entry)
             root = entry['root']
-            structures_list = self._load_items(envelope, [root], 0, lazy=True, **kwargs)
+            structures_list = self._load_items(envelope, [root], depth=0, **kwargs)
             if not isinstance(structures_list[0], ErrorDescriptor):
                 result.append(structures_list[0])
         return result
@@ -929,13 +927,13 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         """
         structure_entry = self._lookup_course(structure_id)
         root = structure_entry.structure['root']
-        result = self._load_items(structure_entry, [root], depth, lazy=True, **kwargs)
+        result = self._load_items(structure_entry, [root], depth, **kwargs)
         return result[0]
 
     def get_course(self, course_id, depth=0, **kwargs):
-        '''
+        """
         Gets the course descriptor for the course identified by the locator
-        '''
+        """
         if not isinstance(course_id, CourseLocator) or course_id.deprecated:
             # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore.
             raise ItemNotFoundError(course_id)
@@ -951,14 +949,14 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         return self._get_structure(library_id, depth, **kwargs)
 
     def has_course(self, course_id, ignore_case=False, **kwargs):
-        '''
+        """
         Does this course exist in this modulestore. This method does not verify that the branch &/or
         version in the course_id exists. Use get_course_index_info to check that.
 
         Returns the course_id of the course if it was found, else None
         Note: we return the course_id instead of a boolean here since the found course may have
            a different id than the given course_id when ignore_case is True.
-        '''
+        """
         if not isinstance(course_id, CourseLocator) or course_id.deprecated:
             # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore.
             return False
@@ -967,12 +965,12 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         return CourseLocator(course_index['org'], course_index['course'], course_index['run'], course_id.branch) if course_index else None
 
     def has_library(self, library_id, ignore_case=False, **kwargs):
-        '''
+        """
         Does this library exist in this modulestore. This method does not verify that the branch &/or
         version in the library_id exists.
 
         Returns the library_id of the course if it was found, else None.
-        '''
+        """
         if not isinstance(library_id, LibraryLocator):
             return None
 
@@ -1017,7 +1015,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
 
         with self.bulk_operations(usage_key.course_key):
             course = self._lookup_course(usage_key.course_key)
-            items = self._load_items(course, [BlockKey.from_usage_key(usage_key)], depth, lazy=True, **kwargs)
+            items = self._load_items(course, [BlockKey.from_usage_key(usage_key)], depth, **kwargs)
             if len(items) == 0:
                 raise ItemNotFoundError(usage_key)
             elif len(items) > 1:
@@ -1078,7 +1076,7 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
                 if block_name == block_id.id and _block_matches_all(block):
                     block_ids.append(block_id)
 
-            return self._load_items(course, block_ids, lazy=True, **kwargs)
+            return self._load_items(course, block_ids, **kwargs)
 
         if 'category' in qualifiers:
             qualifiers['block_type'] = qualifiers.pop('category')
@@ -1091,18 +1089,18 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
                 items.append(block_id)
 
         if len(items) > 0:
-            return self._load_items(course, items, 0, lazy=True, **kwargs)
+            return self._load_items(course, items, depth=0, **kwargs)
         else:
             return []
 
     def get_parent_location(self, locator, **kwargs):
-        '''
+        """
         Return the location (Locators w/ block_ids) for the parent of this location in this
         course. Could use get_items(location, {'children': block_id}) but this is slightly faster.
         NOTE: the locator must contain the block_id, and this code does not actually ensure block_id exists
 
         :param locator: BlockUsageLocator restricting search scope
-        '''
+        """
         if not isinstance(locator, BlockUsageLocator) or locator.deprecated:
             # The supplied locator is of the wrong type, so it can't possibly be stored in this modulestore.
             raise ItemNotFoundError(locator)
@@ -1208,12 +1206,12 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         return definition['edit_info']
 
     def get_course_successors(self, course_locator, version_history_depth=1):
-        '''
+        """
         Find the version_history_depth next versions of this course. Return as a VersionTree
         Mostly makes sense when course_locator uses a version_guid, but because it finds all relevant
         next versions, these do include those created for other courses.
         :param course_locator:
-        '''
+        """
         if not isinstance(course_locator, CourseLocator) or course_locator.deprecated:
             # The supplied CourseKey is of the wrong type, so it can't possibly be stored in this modulestore.
             raise ItemNotFoundError(course_locator)
@@ -1244,14 +1242,14 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         return VersionTree(course_locator, result)
 
     def get_block_generations(self, block_locator):
-        '''
+        """
         Find the history of this block. Return as a VersionTree of each place the block changed (except
         deletion).
 
         The block's history tracks its explicit changes but not the changes in its children starting
         from when the block was created.
 
-        '''
+        """
         # course_agnostic means we don't care if the head and version don't align, trust the version
         course_struct = self._lookup_course(block_locator.course_key.course_agnostic()).structure
         block_key = BlockKey.from_usage_key(block_locator)
@@ -1296,9 +1294,9 @@ class SplitMongoModuleStore(SplitBulkWriteMixin, ModuleStoreWriteBase):
         )
 
     def get_definition_successors(self, definition_locator, version_history_depth=1):
-        '''
+        """
         Find the version_history_depth next versions of this definition. Return as a VersionTree
-        '''
+        """
         # TODO implement
         pass
 

common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py

@@ -8,6 +8,7 @@ import logging
 import ddt
 import itertools
 import mimetypes
+from unittest import skip
 from uuid import uuid4
 
 # Mixed modulestore depends on django, so we'll manually configure some django settings
@@ -61,7 +62,7 @@ class TestMixedModuleStore(CourseComparisonTest):
     ASSET_COLLECTION = 'assetstore'
     FS_ROOT = DATA_DIR
     DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
-    RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+    RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': ''
 
     MONGO_COURSEID = 'MITx/999/2013_Spring'
     XML_COURSEID1 = 'edX/toy/2012_Fall'
@@ -1963,6 +1964,7 @@ class TestMixedModuleStore(CourseComparisonTest):
             dest_store = self.store._get_modulestore_by_type(ModuleStoreEnum.Type.split)
             self.assertCoursesEqual(source_store, source_course_key, dest_store, dest_course_id)
 
+    @skip("PLAT-449 XModule TestMixedModuleStore intermittent test failure")
     @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
     def test_course_publish_signal_firing(self, default):
         with MongoContentstoreBuilder().build() as contentstore:

common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py

@@ -54,7 +54,7 @@ COLLECTION = 'modulestore'
 ASSET_COLLECTION = 'assetstore'
 FS_ROOT = DATA_DIR  # TODO (vshnayder): will need a real fs_root for testing load_item
 DEFAULT_CLASS = 'xmodule.raw_module.RawDescriptor'
-RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': ''
 
 
 class ReferenceTestXBlock(XBlock, XModuleMixin):

common/lib/xmodule/xmodule/modulestore/tests/test_mongo_call_count.py

@@ -87,14 +87,39 @@ class CountMongoCallsCourseTraversal(TestCase):
     to the leaf nodes.
     """
 
+    # Suppose you want to traverse a course - maybe accessing the fields of each XBlock in the course,
+    # maybe not. What parameters should one use for get_course() in order to minimize the number of
+    # mongo calls? The tests below both ensure that code changes don't increase the number of mongo calls
+    # during traversal -and- demonstrate how to minimize the number of calls.
     @ddt.data(
-        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, None, 189),  # The way this traversal *should* be done.
-        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, 0, 387),     # The pathological case - do *not* query a course this way!
-        (MIXED_SPLIT_MODULESTORE_BUILDER, None, 7),  # The way this traversal *should* be done.
-        (MIXED_SPLIT_MODULESTORE_BUILDER, 0, 145)    # The pathological case - do *not* query a course this way!
+        # These two lines show the way this traversal *should* be done
+        # (if you'll eventually access all the fields and load all the definitions anyway).
+        # 'lazy' does not matter in old Mongo.
+        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, None, False, True, 189),
+        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, None, True, True, 189),
+        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, 0, False, True, 387),
+        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, 0, True, True, 387),
+        # As shown in these two lines: whether or not the XBlock fields are accessed,
+        # the same number of mongo calls are made in old Mongo for depth=None.
+        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, None, False, False, 189),
+        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, None, True, False, 189),
+        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, 0, False, False, 387),
+        (MIXED_OLD_MONGO_MODULESTORE_BUILDER, 0, True, False, 387),
+        # The line below shows the way this traversal *should* be done
+        # (if you'll eventually access all the fields and load all the definitions anyway).
+        (MIXED_SPLIT_MODULESTORE_BUILDER, None, False, True, 4),
+        (MIXED_SPLIT_MODULESTORE_BUILDER, None, True, True, 143),
+        (MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, True, 143),
+        (MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, True, 143),
+        (MIXED_SPLIT_MODULESTORE_BUILDER, None, False, False, 4),
+        (MIXED_SPLIT_MODULESTORE_BUILDER, None, True, False, 4),
+        # TODO: The call count below seems like a bug - should be 4?
+        # Seems to be related to using self.lazy in CachingDescriptorSystem.get_module_data().
+        (MIXED_SPLIT_MODULESTORE_BUILDER, 0, False, False, 143),
+        (MIXED_SPLIT_MODULESTORE_BUILDER, 0, True, False, 4)
     )
     @ddt.unpack
-    def test_number_mongo_calls(self, store, depth, num_mongo_calls):
+    def test_number_mongo_calls(self, store, depth, lazy, access_all_block_fields, num_mongo_calls):
         with store.build() as (source_content, source_store):
 
             source_course_key = source_store.make_course_key('a', 'course', 'course')
@@ -116,10 +141,20 @@ class CountMongoCallsCourseTraversal(TestCase):
             # Starting at the root course block, do a breadth-first traversal using
             # get_children() to retrieve each block's children.
             with check_mongo_calls(num_mongo_calls):
-                start_block = source_store.get_course(source_course_key, depth=depth)
-                stack = [start_block]
-                while stack:
-                    curr_block = stack.pop()
-                    if curr_block.has_children:
-                        for block in reversed(curr_block.get_children()):
-                            stack.append(block)
+                with source_store.bulk_operations(source_course_key):
+                    start_block = source_store.get_course(source_course_key, depth=depth, lazy=lazy)
+                    all_blocks = []
+                    stack = [start_block]
+                    while stack:
+                        curr_block = stack.pop()
+                        all_blocks.append(curr_block)
+                        if curr_block.has_children:
+                            for block in reversed(curr_block.get_children()):
+                                stack.append(block)
+
+                    if access_all_block_fields:
+                        # Read the fields on each block in order to ensure each block and its definition is loaded.
+                        for xblock in all_blocks:
+                            for __, field in xblock.fields.iteritems():
+                                if field.is_set_on(xblock):
+                                    __ = field.read_from(xblock)

common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore_bulk_operations.py

@@ -405,7 +405,12 @@ class TestBulkWriteMixinFindMethods(TestBulkWriteMixin):
 
         self.conn.get_definitions.return_value = db_definitions
         results = self.bulk.get_definitions(self.course_key, search_ids)
-        self.conn.get_definitions.assert_called_once_with(list(set(search_ids) - set(active_ids)))
+        definitions_gotten = list(set(search_ids) - set(active_ids))
+        if len(definitions_gotten) > 0:
+            self.conn.get_definitions.assert_called_once_with(definitions_gotten)
+        else:
+            # If no definitions to get, then get_definitions() should *not* have been called.
+            self.assertEquals(self.conn.get_definitions.call_count, 0)
         for _id in active_ids:
             if _id in search_ids:
                 self.assertIn(active_definition(_id), results)

common/lib/xmodule/xmodule/modulestore/tests/test_xml_importer.py

@@ -28,7 +28,7 @@ class ModuleStoreNoSettings(unittest.TestCase):
     COLLECTION = 'modulestore'
     FS_ROOT = DATA_DIR
     DEFAULT_CLASS = 'xmodule.modulestore.tests.test_xml_importer.StubXBlock'
-    RENDER_TEMPLATE = lambda t_n, d, ctx = None, nsp = 'main': ''
+    RENDER_TEMPLATE = lambda t_n, d, ctx=None, nsp='main': ''
 
     modulestore_options = {
         'default_class': DEFAULT_CLASS,

common/lib/xmodule/xmodule/modulestore/xml.py

@@ -706,7 +706,7 @@ class XMLModuleStore(ModuleStoreReadBase):
         """
         return CourseLocator(org, course, run, deprecated=True)
 
-    def get_courses(self, depth=0, **kwargs):
+    def get_courses(self, **kwargs):
         """
         Returns a list of course descriptors.  If there were errors on loading,
         some of these may be ErrorDescriptors instead.

common/lib/xmodule/xmodule/modulestore/xml_exporter.py

@@ -41,7 +41,12 @@ def export_to_xml(modulestore, contentstore, course_key, root_dir, course_dir):
 
     with modulestore.bulk_operations(course_key):
 
-        course = modulestore.get_course(course_key, depth=None)  # None means infinite
+        # depth = None: Traverses down the entire course structure.
+        # lazy = False: Loads and caches all block definitions during traversal for fast access later
+        #               -and- to eliminate many round-trips to read individual definitions.
+        # Why these parameters? Because a course export needs to access all the course block information
+        # eventually. Accessing it all now at the beginning increases performance of the export.
+        course = modulestore.get_course(course_key, depth=None, lazy=False)
         fsm = OSFS(root_dir)
         export_fs = course.runtime.export_fs = fsm.makeopendir(course_dir)
         root_course_dir = root_dir + '/' + course_dir

common/lib/xmodule/xmodule/open_ended_grading_classes/peer_grading_service.py

@@ -117,13 +117,12 @@ class PeerGradingService(GradingService):
         return result
 
 
-"""
-This is a mock peer grading service that can be used for unit tests
-without making actual service calls to the grading controller
-"""
-
-
 class MockPeerGradingService(object):
+    """
+    This is a mock peer grading service that can be used for unit tests
+    without making actual service calls to the grading controller
+    """
+
     def get_next_submission(self, problem_location, grader_id):
         return {
             'success': True,

common/lib/xmodule/xmodule/tabs.py

@@ -1,12 +1,6 @@
 """
 Implement CourseTab
 """
-# pylint: disable=incomplete-protocol
-# Note: pylint complains that we do not implement __delitem__ and __len__, although we implement __setitem__
-# and __getitem__.  However, the former two do not apply to the CourseTab class so we do not implement them.
-# The reason we implement the latter two is to enable callers to continue to use the CourseTab object with
-# dict-type accessors.
-
 from abc import ABCMeta, abstractmethod
 from xblock.fields import List
 
@@ -15,7 +9,7 @@ from xblock.fields import List
 _ = lambda text: text
 
 
-class CourseTab(object):  # pylint: disable=incomplete-protocol
+class CourseTab(object):
     """
     The Course Tab class is a data abstraction for all tabs (i.e., course navigation links) within a course.
     It is an abstract class - to be inherited by various tab types.

common/lib/xmodule/xmodule/tests/test_error_module.py

@@ -119,8 +119,8 @@ class TestErrorModuleConstruction(unittest.TestCase):
     """
     Test that error module construction happens correctly
     """
-
     def setUp(self):
+        # pylint: disable=abstract-class-instantiated
         super(TestErrorModuleConstruction, self).setUp()
         field_data = Mock(spec=FieldData)
         self.descriptor = BrokenDescriptor(

common/static/js/spec/tooltip_manager_spec.js

@@ -70,6 +70,17 @@ describe('TooltipManager', function () {
         expect($('.tooltip')).toBeHidden();
     });
 
+    it('can be configured to show when user clicks on the element', function () {
+        this.element.attr('data-tooltip-show-on-click', true);
+        this.element.trigger($.Event("click"));
+        expect($('.tooltip')).toBeVisible();
+    });
+
+    it('can be be triggered manually', function () {
+        this.tooltip.openTooltip(this.element);
+        expect($('.tooltip')).toBeVisible();
+    });
+
     it('should moves correctly', function () {
         showTooltip(this.element);
         expect($('.tooltip')).toBeVisible();

common/static/js/src/tooltip_manager.js

@@ -26,7 +26,7 @@
                 'mouseover.TooltipManager': this.showTooltip,
                 'mousemove.TooltipManager': this.moveTooltip,
                 'mouseout.TooltipManager': this.hideTooltip,
-                'click.TooltipManager': this.hideTooltip
+                'click.TooltipManager': this.click
             }, this.SELECTOR);
         },
 
@@ -46,17 +46,31 @@
         },
 
         showTooltip: function(event) {
-            var tooltipText = $(event.currentTarget).attr('data-tooltip');
-            this.tooltip
-                .html(tooltipText)
-                .css(this.getCoords(event.pageX, event.pageY));
-
+            this.prepareTooltip(event.currentTarget, event.pageX, event.pageY);
             if (this.tooltipTimer) {
                 clearTimeout(this.tooltipTimer);
             }
             this.tooltipTimer = setTimeout(this.show, 500);
         },
 
+        prepareTooltip: function(element, pageX, pageY) {
+            pageX = typeof pageX !== 'undefined' ? pageX : element.offset().left + element.width()/2;
+            pageY = typeof pageY !== 'undefined' ? pageY : element.offset().top + element.height()/2;
+            var tooltipText = $(element).attr('data-tooltip');
+            this.tooltip
+                .html(tooltipText)
+                .css(this.getCoords(pageX, pageY));
+        },
+
+        // To manually trigger a tooltip to reveal, other than through user mouse movement:
+        openTooltip: function(element) {
+            this.prepareTooltip(element);
+            this.show();
+            if (this.tooltipTimer) {
+                clearTimeout(this.tooltipTimer);
+            }
+        },
+
         moveTooltip: function(event) {
             this.tooltip.css(this.getCoords(event.pageX, event.pageY));
         },
@@ -68,6 +82,18 @@
             this.tooltipTimer = setTimeout(this.hide, 50);
         },
 
+        click: function(event) {
+            var showOnClick = !!$(event.currentTarget).data('tooltip-show-on-click'); // Default is false
+            if (showOnClick) {
+                this.show();
+                if (this.tooltipTimer) {
+                    clearTimeout(this.tooltipTimer);
+                }
+            } else {
+                this.hideTooltip(event);
+            }
+        },
+
         destroy: function () {
             this.tooltip.remove();
             // Unbind all delegated event handlers in the ".TooltipManager"
@@ -78,6 +104,6 @@
 
     window.TooltipManager = TooltipManager;
     $(document).ready(function () {
-        new TooltipManager(document.body);
+        window.globalTooltipManager = new TooltipManager(document.body);
     });
 }());

common/templates/edxnotes_wrapper.html

@@ -1,4 +1,5 @@
 <%! import json %>
+<%! from django.utils.translation import ugettext as _ %>
 <%! from student.models import anonymous_id_for_user %>
 <%
     if user:

common/test/acceptance/fixtures/course.py

@@ -102,7 +102,7 @@ class CourseFixture(XBlockContainerFixture):
     between tests, you should use unique course identifiers for each fixture.
     """
 
-    def __init__(self, org, number, run, display_name, start_date=None, end_date=None):
+    def __init__(self, org, number, run, display_name, start_date=None, end_date=None, settings=None):
         """
         Configure the course fixture to create a course with
 
@@ -112,6 +112,8 @@ class CourseFixture(XBlockContainerFixture):
         The default is for the course to have started in the distant past, which is generally what
         we want for testing so students can enroll.
 
+        `settings` can be any additional course settings needs to be enabled. for example
+        to enable entrance exam settings would be a dict like this {"entrance_exam_enabled": "true"}
         These have the same meaning as in the Studio restful API /course end-point.
         """
         super(CourseFixture, self).__init__()
@@ -134,6 +136,9 @@ class CourseFixture(XBlockContainerFixture):
         if end_date is not None:
             self._course_details['end_date'] = end_date.isoformat()
 
+        if settings is not None:
+            self._course_details.update(settings)
+
         self._updates = []
         self._handouts = []
         self._assets = []

common/test/acceptance/pages/lms/instructor_dashboard.py

@@ -37,6 +37,15 @@ class InstructorDashboardPage(CoursePage):
         data_download_section.wait_for_page()
         return data_download_section
 
+    def select_student_admin(self):
+        """
+        Selects the student admin tab and returns the MembershipSection
+        """
+        self.q(css='a[data-section=student_admin]').first.click()
+        student_admin_section = StudentAdminPage(self.browser)
+        student_admin_section.wait_for_page()
+        return student_admin_section
+
     @staticmethod
     def get_asset_path(file_name):
         """
@@ -161,6 +170,11 @@ class MembershipPageCohortManagementSection(PageObject):
         self._get_cohort_options().filter(
             lambda el: self._cohort_name(el.text) == cohort_name
         ).first.click()
+        # wait for cohort to render as selected on screen
+        EmptyPromise(
+            lambda: self.q(css='.title-value').text[0] == cohort_name,
+            "Waiting to confirm cohort has been selected"
+        ).fulfill()
 
     def add_cohort(self, cohort_name, content_group=None):
         """
@@ -455,3 +469,126 @@ class DataDownloadPage(PageObject):
         """
         reports = self.q(css="#report-downloads-table .file-download-link>a").map(lambda el: el.text)
         return reports.results
+
+
+class StudentAdminPage(PageObject):
+    """
+    Student admin section of the Instructor dashboard.
+    """
+    url = None
+    EE_CONTAINER = ".entrance-exam-grade-container"
+
+    def is_browser_on_page(self):
+        """
+        Confirms student admin section is present
+        """
+        return self.q(css='a[data-section=student_admin].active-section').present
+
+    @property
+    def student_email_input(self):
+        """
+        Returns email address/username input box.
+        """
+        return self.q(css='{} input[name=entrance-exam-student-select-grade]'.format(self.EE_CONTAINER))
+
+    @property
+    def reset_attempts_button(self):
+        """
+        Returns reset student attempts button.
+        """
+        return self.q(css='{} input[name=reset-entrance-exam-attempts]'.format(self.EE_CONTAINER))
+
+    @property
+    def rescore_submission_button(self):
+        """
+        Returns rescore student submission button.
+        """
+        return self.q(css='{} input[name=rescore-entrance-exam]'.format(self.EE_CONTAINER))
+
+    @property
+    def delete_student_state_button(self):
+        """
+        Returns delete student state button.
+        """
+        return self.q(css='{} input[name=delete-entrance-exam-state]'.format(self.EE_CONTAINER))
+
+    @property
+    def background_task_history_button(self):
+        """
+        Returns show background task history for student button.
+        """
+        return self.q(css='{} input[name=entrance-exam-task-history]'.format(self.EE_CONTAINER))
+
+    @property
+    def top_notification(self):
+        """
+        Returns show background task history for student button.
+        """
+        return self.q(css='{} .request-response-error'.format(self.EE_CONTAINER)).first
+
+    def is_student_email_input_visible(self):
+        """
+        Returns True if student email address/username input box is present.
+        """
+        return self.student_email_input.is_present()
+
+    def is_reset_attempts_button_visible(self):
+        """
+        Returns True if reset student attempts button is present.
+        """
+        return self.reset_attempts_button.is_present()
+
+    def is_rescore_submission_button_visible(self):
+        """
+        Returns True if rescore student submission button is present.
+        """
+        return self.rescore_submission_button.is_present()
+
+    def is_delete_student_state_button_visible(self):
+        """
+        Returns True if delete student state for entrance exam button is present.
+        """
+        return self.delete_student_state_button.is_present()
+
+    def is_background_task_history_button_visible(self):
+        """
+        Returns True if show background task history for student button is present.
+        """
+        return self.background_task_history_button.is_present()
+
+    def is_background_task_history_table_visible(self):
+        """
+        Returns True if background task history table is present.
+        """
+        return self.q(css='{} .entrance-exam-task-history-table'.format(self.EE_CONTAINER)).is_present()
+
+    def click_reset_attempts_button(self):
+        """
+        clicks reset student attempts button.
+        """
+        return self.reset_attempts_button.click()
+
+    def click_rescore_submissions_button(self):
+        """
+        clicks rescore submissions button.
+        """
+        return self.rescore_submission_button.click()
+
+    def click_delete_student_state_button(self):
+        """
+        clicks delete student state button.
+        """
+        return self.delete_student_state_button.click()
+
+    def click_task_history_button(self):
+        """
+        clicks background task history button.
+        """
+        return self.background_task_history_button.click()
+
+    def set_student_email(self, email_addres):
+        """
+        Sets given email address as value of student email address/username input box.
+        """
+        input_box = self.student_email_input.first.results[0]
+        input_box.send_keys(email_addres)

common/test/acceptance/pages/lms/problem.py

@@ -46,3 +46,19 @@ class ProblemPage(PageObject):
         Is there a "correct" status showing?
         """
         return self.q(css="div.problem div.capa_inputtype.textline div.correct p.status").is_present()
+
+    def click_clarification(self, index=0):
+        """
+        Click on an inline icon that can be included in problem text using an HTML <clarification> element:
+
+        Problem <clarification>clarification text hidden by an icon in rendering</clarification> Text
+        """
+        self.q(css='div.problem .clarification:nth-child({index}) i[data-tooltip]'.format(index=index + 1)).click()
+
+    @property
+    def visible_tooltip_text(self):
+        """
+        Get the text seen in any tooltip currently visible on the page.
+        """
+        self.wait_for_element_visibility('body > .tooltip', 'A tooltip is visible.')
+        return self.q(css='body > .tooltip').text[0]

common/test/acceptance/pages/lms/video/video.py

@@ -356,10 +356,6 @@ class VideoPage(PageObject):
 
         self.q(css=button_selector).first.click()
 
-        button_states = {'play': 'playing', 'pause': 'pause'}
-        if button in button_states:
-            self.wait_for_state(button_states[button])
-
         self.wait_for_ajax()
 
     def _get_element_dimensions(self, selector):
@@ -677,7 +673,7 @@ class VideoPage(PageObject):
         elif 'is-ended' in current_state:
             return 'finished'
 
-    def _wait_for(self, check_func, desc, result=False, timeout=200):
+    def _wait_for(self, check_func, desc, result=False, timeout=200, try_interval=0.2):
         """
         Calls the method provided as an argument until the Promise satisfied or BrokenPromise
 
@@ -689,9 +685,9 @@ class VideoPage(PageObject):
 
         """
         if result:
-            return Promise(check_func, desc, timeout=timeout).fulfill()
+            return Promise(check_func, desc, timeout=timeout, try_interval=try_interval).fulfill()
         else:
-            return EmptyPromise(check_func, desc, timeout=timeout).fulfill()
+            return EmptyPromise(check_func, desc, timeout=timeout, try_interval=try_interval).fulfill()
 
     def wait_for_state(self, state):
         """

common/test/acceptance/pages/studio/settings_group_configurations.py

@@ -167,8 +167,11 @@ class GroupConfiguration(object):
         return self.find_css('.actions .delete.is-disabled').present
 
     @property
-    def delete_button_is_absent(self):
-        return not self.find_css('.actions .delete').present
+    def delete_button_is_present(self):
+        """
+        Returns whether or not the delete icon is present.
+        """
+        return self.find_css('.actions .delete').present
 
     def delete(self):
         """

common/test/acceptance/tests/discussion/test_cohort_management.py

@@ -51,9 +51,9 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
         ).visit().get_user_id()
         self.add_user_to_cohort(self.course_fixture, self.student_name, self.manual_cohort_id)
 
-        # create a user with unicode characters in their username
-        self.unicode_student_id = AutoAuthPage(
-            self.browser, username="Ωπ", email="unicode_student_user@example.com",
+        # create a second student user
+        self.other_student_id = AutoAuthPage(
+            self.browser, username="other_student_user", email="other_student_user@example.com",
             course_id=self.course_id, staff=False
         ).visit().get_user_id()
 
@@ -389,12 +389,12 @@ class CohortConfigurationTest(UniqueCourseTest, CohortTestMixin):
             }).count(),
             1
         )
-        # unicode_student_user (previously unassigned) is added to manual cohort
+        # other_student_user (previously unassigned) is added to manual cohort
         self.assertEqual(
             self.event_collection.find({
                 "name": "edx.cohort.user_added",
                 "time": {"$gt": start_time},
-                "event.user_id": {"$in": [int(self.unicode_student_id)]},
+                "event.user_id": {"$in": [int(self.other_student_id)]},
                 "event.cohort_name": self.manual_cohort_name,
             }).count(),
             1

common/test/acceptance/tests/helpers.py

@@ -13,6 +13,8 @@ from opaque_keys.edx.locator import CourseLocator
 from xmodule.partitions.partitions import UserPartition
 from xmodule.partitions.tests.test_partitions import MockUserPartitionScheme
 from selenium.webdriver.support.select import Select
+from selenium.webdriver.support.ui import WebDriverWait
+from selenium.webdriver.support import expected_conditions as EC
 
 
 def skip_if_browser(browser):
@@ -252,6 +254,15 @@ def assert_event_emitted_num_times(event_collection, event_name, event_time, eve
     )
 
 
+def get_modal_alert(browser):
+    """
+    Returns instance of modal alert box shown in browser after waiting
+    for 4 seconds
+    """
+    WebDriverWait(browser, 4).until(EC.alert_is_present())
+    return browser.switch_to.alert
+
+
 class UniqueCourseTest(WebAppTest):
     """
     Test that provides a unique course ID.

common/test/acceptance/tests/lms/test_lms_instructor_dashboard.py

@@ -3,7 +3,8 @@
 End-to-end tests for the LMS Instructor Dashboard.
 """
 
-from ..helpers import UniqueCourseTest
+from ..helpers import UniqueCourseTest, get_modal_alert
+from ...pages.common.logout import LogoutPage
 from ...pages.lms.auto_auth import AutoAuthPage
 from ...pages.lms.instructor_dashboard import InstructorDashboardPage
 from ...fixtures.course import CourseFixture
@@ -84,3 +85,166 @@ class AutoEnrollmentWithCSVTest(UniqueCourseTest):
         self.auto_enroll_section.upload_non_csv_file()
         self.assertTrue(self.auto_enroll_section.is_notification_displayed(section_type=self.auto_enroll_section.NOTIFICATION_ERROR))
         self.assertEqual(self.auto_enroll_section.first_notification_message(section_type=self.auto_enroll_section.NOTIFICATION_ERROR), "Make sure that the file you upload is in CSV format with no extraneous characters or rows.")
+
+
+class EntranceExamGradeTest(UniqueCourseTest):
+    """
+    Tests for Entrance exam specific student grading tasks.
+    """
+
+    def setUp(self):
+        super(EntranceExamGradeTest, self).setUp()
+        self.course_info.update({"settings": {"entrance_exam_enabled": "true"}})
+        CourseFixture(**self.course_info).install()
+        self.student_identifier = "johndoe_saee@example.com"
+        # Create the user (automatically logs us in)
+        AutoAuthPage(
+            self.browser,
+            username="johndoe_saee",
+            email=self.student_identifier,
+            course_id=self.course_id,
+            staff=False
+        ).visit()
+
+        LogoutPage(self.browser).visit()
+
+        # login as an instructor
+        AutoAuthPage(self.browser, course_id=self.course_id, staff=True).visit()
+
+        # go to the student admin page on the instructor dashboard
+        instructor_dashboard_page = InstructorDashboardPage(self.browser, self.course_id)
+        instructor_dashboard_page.visit()
+        self.student_admin_section = instructor_dashboard_page.select_student_admin()
+
+    def test_input_text_and_buttons_are_visible(self):
+        """
+        Scenario: On the Student admin tab of the Instructor Dashboard, Student Email input box,
+        Reset Student Attempt, Rescore Student Submission, Delete Student State for entrance exam
+            and Show Background Task History for Student buttons are visible
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            Then I see Student Email input box, Reset Student Attempt, Rescore Student Submission,
+            Delete Student State for entrance exam and Show Background Task History for Student buttons
+        """
+        self.assertTrue(self.student_admin_section.is_student_email_input_visible())
+        self.assertTrue(self.student_admin_section.is_reset_attempts_button_visible())
+        self.assertTrue(self.student_admin_section.is_rescore_submission_button_visible())
+        self.assertTrue(self.student_admin_section.is_delete_student_state_button_visible())
+        self.assertTrue(self.student_admin_section.is_background_task_history_button_visible())
+
+    def test_clicking_reset_student_attempts_button_without_email_shows_error(self):
+        """
+        Scenario: Clicking on the Reset Student Attempts button without entering student email
+        address or username results in error.
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            When I click the Reset Student Attempts Button  under Entrance Exam Grade
+            Adjustment without enter an email address
+            Then I should be shown an Error Notification
+            And The Notification message should read 'Please enter a student email address or username.'
+        """
+        self.student_admin_section.click_reset_attempts_button()
+        self.assertEqual(
+            'Please enter a student email address or username.',
+            self.student_admin_section.top_notification.text[0]
+        )
+
+    def test_clicking_reset_student_attempts_button_with_success(self):
+        """
+        Scenario: Clicking on the Reset Student Attempts button with valid student email
+        address or username should result in success prompt.
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            When I click the Reset Student Attempts Button under Entrance Exam Grade
+            Adjustment after entering a valid student
+            email address or username
+            Then I should be shown an alert with success message
+        """
+        self.student_admin_section.set_student_email(self.student_identifier)
+        self.student_admin_section.click_reset_attempts_button()
+        alert = get_modal_alert(self.student_admin_section.browser)
+        alert.dismiss()
+
+    def test_clicking_reset_student_attempts_button_with_error(self):
+        """
+        Scenario: Clicking on the Reset Student Attempts button with email address or username
+        of a non existing student should result in error message.
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            When I click the Reset Student Attempts Button  under Entrance Exam Grade
+            Adjustment after non existing student email address or username
+            Then I should be shown an error message
+        """
+        self.student_admin_section.set_student_email('non_existing@example.com')
+        self.student_admin_section.click_reset_attempts_button()
+        self.student_admin_section.wait_for_ajax()
+        self.assertGreater(len(self.student_admin_section.top_notification.text[0]), 0)
+
+    def test_clicking_rescore_submission_button_with_success(self):
+        """
+        Scenario: Clicking on the Rescore Student Submission button with valid student email
+        address or username should result in success prompt.
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            When I click the Rescore Student Submission Button  under Entrance Exam Grade
+            Adjustment after entering a valid student email address or username
+            Then I should be shown an alert with success message
+        """
+        self.student_admin_section.set_student_email(self.student_identifier)
+        self.student_admin_section.click_rescore_submissions_button()
+        alert = get_modal_alert(self.student_admin_section.browser)
+        alert.dismiss()
+
+    def test_clicking_rescore_submission_button_with_error(self):
+        """
+        Scenario: Clicking on the Rescore Student Submission button with email address or username
+        of a non existing student should result in error message.
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            When I click the Rescore Student Submission Button under Entrance Exam Grade
+            Adjustment after non existing student email address or username
+            Then I should be shown an error message
+        """
+        self.student_admin_section.set_student_email('non_existing@example.com')
+        self.student_admin_section.click_rescore_submissions_button()
+        self.student_admin_section.wait_for_ajax()
+        self.assertGreater(len(self.student_admin_section.top_notification.text[0]), 0)
+
+    def test_clicking_delete_student_attempts_button_with_success(self):
+        """
+        Scenario: Clicking on the Delete Student State for entrance exam button
+        with valid student email address or username should result in success prompt.
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            When I click the Delete Student State for entrance exam Button
+            under Entrance Exam Grade Adjustment after entering a valid student
+            email address or username
+            Then I should be shown an alert with success message
+        """
+        self.student_admin_section.set_student_email(self.student_identifier)
+        self.student_admin_section.click_delete_student_state_button()
+        alert = get_modal_alert(self.student_admin_section.browser)
+        alert.dismiss()
+
+    def test_clicking_delete_student_attempts_button_with_error(self):
+        """
+        Scenario: Clicking on the Delete Student State for entrance exam button
+        with email address or username of a non existing student should result
+        in error message.
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            When I click the Delete Student State for entrance exam Button
+            under Entrance Exam Grade Adjustment after non existing student
+            email address or username
+            Then I should be shown an error message
+        """
+        self.student_admin_section.set_student_email('non_existing@example.com')
+        self.student_admin_section.click_delete_student_state_button()
+        self.student_admin_section.wait_for_ajax()
+        self.assertGreater(len(self.student_admin_section.top_notification.text[0]), 0)
+
+    def test_clicking_task_history_button_with_success(self):
+        """
+        Scenario: Clicking on the Show Background Task History for Student
+        with valid student email address or username should result in table of tasks.
+            Given that I am on the Student Admin tab on the Instructor Dashboard
+            When I click the Show Background Task History for Student Button
+            under Entrance Exam Grade Adjustment after entering a valid student
+            email address or username
+            Then I should be shown an table listing all background tasks
+        """
+        self.student_admin_section.set_student_email(self.student_identifier)
+        self.student_admin_section.click_task_history_button()
+        self.assertTrue(self.student_admin_section.is_background_task_history_table_visible())

common/test/acceptance/tests/lms/test_lms_matlab_problem.py

@@ -1,38 +1,24 @@
 # -*- coding: utf-8 -*-
 """
-End-to-end tests for the LMS.
+Test for matlab problems
 """
 import time
 
-from ..helpers import UniqueCourseTest
-from ...pages.studio.auto_auth import AutoAuthPage
-from ...pages.lms.courseware import CoursewarePage
 from ...pages.lms.matlab_problem import MatlabProblemPage
-from ...fixtures.course import CourseFixture, XBlockFixtureDesc
+from ...fixtures.course import XBlockFixtureDesc
 from ...fixtures.xqueue import XQueueResponseFixture
+from .test_lms_problems import ProblemsTest
 from textwrap import dedent
 
 
-class MatlabProblemTest(UniqueCourseTest):
+class MatlabProblemTest(ProblemsTest):
     """
     Tests that verify matlab problem "Run Code".
     """
-    USERNAME = "STAFF_TESTER"
-    EMAIL = "johndoe@example.com"
-
-    def setUp(self):
-        super(MatlabProblemTest, self).setUp()
-
-        self.XQUEUE_GRADE_RESPONSE = None
-
-        self.courseware_page = CoursewarePage(self.browser, self.course_id)
-
-        # Install a course with sections/problems, tabs, updates, and handouts
-        course_fix = CourseFixture(
-            self.course_info['org'], self.course_info['number'],
-            self.course_info['run'], self.course_info['display_name']
-        )
-
+    def get_problem(self):
+        """
+        Create a matlab problem for the test.
+        """
         problem_data = dedent("""
             <problem markdown="null">
                   <text>
@@ -62,18 +48,7 @@ class MatlabProblemTest(UniqueCourseTest):
                   </text>
             </problem>
         """)
-
-        course_fix.add_children(
-            XBlockFixtureDesc('chapter', 'Test Section').add_children(
-                XBlockFixtureDesc('sequential', 'Test Subsection').add_children(
-                    XBlockFixtureDesc('problem', 'Test Matlab Problem', data=problem_data)
-                )
-            )
-        ).install()
-
-        # Auto-auth register for the course.
-        AutoAuthPage(self.browser, username=self.USERNAME, email=self.EMAIL,
-                     course_id=self.course_id, staff=False).visit()
+        return XBlockFixtureDesc('problem', 'Test Matlab Problem', data=problem_data)
 
     def _goto_matlab_problem_page(self):
         """
@@ -92,13 +67,13 @@ class MatlabProblemTest(UniqueCourseTest):
         # Enter a submission, which will trigger a pre-defined response from the XQueue stub.
         self.submission = "a=1" + self.unique_id[0:5]
 
-        self.XQUEUE_GRADE_RESPONSE = {'msg': self.submission}
+        self.xqueue_grade_response = {'msg': self.submission}
 
         matlab_problem_page = self._goto_matlab_problem_page()
 
         # Configure the XQueue stub's response for the text we will submit
-        if self.XQUEUE_GRADE_RESPONSE is not None:
-            XQueueResponseFixture(self.submission, self.XQUEUE_GRADE_RESPONSE).install()
+        if self.xqueue_grade_response is not None:
+            XQueueResponseFixture(self.submission, self.xqueue_grade_response).install()
 
         matlab_problem_page.set_response(self.submission)
         matlab_problem_page.click_run_code()
@@ -113,6 +88,6 @@ class MatlabProblemTest(UniqueCourseTest):
 
         self.assertEqual(u'', matlab_problem_page.get_grader_msg(".external-grader-message")[0])
         self.assertEqual(
-            self.XQUEUE_GRADE_RESPONSE.get("msg"),
+            self.xqueue_grade_response.get("msg"),
             matlab_problem_page.get_grader_msg(".ungraded-matlab-result")[0]
         )

common/test/acceptance/tests/lms/test_lms_problems.py

@@ -0,0 +1,88 @@
+# -*- coding: utf-8 -*-
+"""
+Bok choy acceptance tests for problems in the LMS
+
+See also old lettuce tests in lms/djangoapps/courseware/features/problems.feature
+"""
+from ..helpers import UniqueCourseTest
+from ...pages.studio.auto_auth import AutoAuthPage
+from ...pages.lms.courseware import CoursewarePage
+from ...pages.lms.problem import ProblemPage
+from ...fixtures.course import CourseFixture, XBlockFixtureDesc
+from textwrap import dedent
+
+
+class ProblemsTest(UniqueCourseTest):
+    """
+    Base class for tests of problems in the LMS.
+    """
+    USERNAME = "joe_student"
+    EMAIL = "joe@example.com"
+
+    def setUp(self):
+        super(ProblemsTest, self).setUp()
+
+        self.xqueue_grade_response = None
+
+        self.courseware_page = CoursewarePage(self.browser, self.course_id)
+
+        # Install a course with a hierarchy and problems
+        course_fixture = CourseFixture(
+            self.course_info['org'], self.course_info['number'],
+            self.course_info['run'], self.course_info['display_name']
+        )
+
+        problem = self.get_problem()
+        course_fixture.add_children(
+            XBlockFixtureDesc('chapter', 'Test Section').add_children(
+                XBlockFixtureDesc('sequential', 'Test Subsection').add_children(problem)
+            )
+        ).install()
+
+        # Auto-auth register for the course.
+        AutoAuthPage(self.browser, username=self.USERNAME, email=self.EMAIL,
+                     course_id=self.course_id, staff=False).visit()
+
+    def get_problem(self):
+        """ Subclasses should override this to complete the fixture """
+        raise NotImplementedError()
+
+
+class ProblemClarificationTest(ProblemsTest):
+    """
+    Tests the <clarification> element that can be used in problem XML.
+    """
+    def get_problem(self):
+        """
+        Create a problem with a <clarification>
+        """
+        xml = dedent("""
+            <problem markdown="null">
+                <text>
+                    <p>
+                        Given the data in Table 7 <clarification>Table 7: "Example PV Installation Costs",
+                        Page 171 of Roberts textbook</clarification>, compute the ROI
+                        <clarification>Return on Investment <strong>(per year)</strong></clarification> over 20 years.
+                    </p>
+                    <numericalresponse answer="6.5">
+                        <textline label="Enter the annual ROI" trailing_text="%" />
+                    </numericalresponse>
+                </text>
+            </problem>
+        """)
+        return XBlockFixtureDesc('problem', 'TOOLTIP TEST PROBLEM', data=xml)
+
+    def test_clarification(self):
+        """
+        Test that we can see the <clarification> tooltips.
+        """
+        self.courseware_page.visit()
+        problem_page = ProblemPage(self.browser)
+        self.assertEqual(problem_page.problem_name, 'TOOLTIP TEST PROBLEM')
+        problem_page.click_clarification(0)
+        self.assertIn('"Example PV Installation Costs"', problem_page.visible_tooltip_text)
+        problem_page.click_clarification(1)
+        tooltip_text = problem_page.visible_tooltip_text
+        self.assertIn('Return on Investment', tooltip_text)
+        self.assertIn('per year', tooltip_text)
+        self.assertNotIn('strong', tooltip_text)

common/test/acceptance/tests/studio/test_studio_library_container.py

@@ -3,6 +3,7 @@ Acceptance tests for Library Content in LMS
 """
 import ddt
 import textwrap
+from unittest import skip
 
 from .base_studio_test import StudioLibraryTest
 from ...fixtures.course import CourseFixture
@@ -148,6 +149,7 @@ class StudioLibraryContainerTest(StudioLibraryTest, UniqueCourseTest):
         self.assertTrue(library_container.has_validation_error)
         self.assertIn(expected_text, library_container.validation_error_text)
 
+    @skip("TE-745 StudioLibraryContainerTest test_out_of_date_message fails intemittently")
     def test_out_of_date_message(self):
         """
         Scenario: Given I have a library, a course and library content xblock in a course

common/test/acceptance/tests/studio/test_studio_settings.py

@@ -5,9 +5,15 @@ Acceptance tests for Studio's Setting pages
 from nose.plugins.attrib import attr
 
 from base_studio_test import StudioCourseTest
-
+from bok_choy.promise import EmptyPromise
+from ...fixtures.course import XBlockFixtureDesc
+from ..helpers import create_user_partition_json
+from ...pages.studio.overview import CourseOutlinePage
 from ...pages.studio.settings_advanced import AdvancedSettingsPage
 from ...pages.studio.settings_group_configurations import GroupConfigurationsPage
+from unittest import skip
+from textwrap import dedent
+from xmodule.partitions.partitions import Group
 
 
 @attr('shard_1')
@@ -25,6 +31,26 @@ class ContentGroupConfigurationTest(StudioCourseTest):
             self.course_info['run']
         )
 
+        self.outline_page = CourseOutlinePage(
+            self.browser,
+            self.course_info['org'],
+            self.course_info['number'],
+            self.course_info['run']
+        )
+
+    def populate_course_fixture(self, course_fixture):
+        """
+        Populates test course with chapter, sequential, and 1 problems.
+        The problem is visible only to Group "alpha".
+        """
+        course_fixture.add_children(
+            XBlockFixtureDesc('chapter', 'Test Section').add_children(
+                XBlockFixtureDesc('sequential', 'Test Subsection').add_children(
+                    XBlockFixtureDesc('vertical', 'Test Unit')
+                )
+            )
+        )
+
     def create_and_verify_content_group(self, name, existing_groups):
         """
         Creates a new content group and verifies that it was properly created.
@@ -38,7 +64,7 @@ class ContentGroupConfigurationTest(StudioCourseTest):
         config.name = name
         # Save the content group
         self.assertEqual(config.get_text('.action-primary'), "Create")
-        self.assertTrue(config.delete_button_is_absent)
+        self.assertFalse(config.delete_button_is_present)
         config.save()
         self.assertIn(name, config.name)
         return config
@@ -84,16 +110,68 @@ class ContentGroupConfigurationTest(StudioCourseTest):
 
         self.assertIn("Updated Second Content Group", second_config.name)
 
-    def test_cannot_delete_content_group(self):
+    def test_cannot_delete_used_content_group(self):
         """
-        Scenario: Delete is not currently supported for content groups.
-        Given I have a course without content groups
-        When I create a content group
-        Then there is no delete button
+        Scenario: Ensure that the user cannot delete used content group.
+        Given I have a course with 1 Content Group
+        And I go to the Group Configuration page
+        When I try to delete the Content Group with name "New Content Group"
+        Then I see the delete button is disabled.
+        """
+        self.course_fixture._update_xblock(self.course_fixture._course_location, {
+            "metadata": {
+                u"user_partitions": [
+                    create_user_partition_json(
+                        0,
+                        'Configuration alpha,',
+                        'Content Group Partition',
+                        [Group("0", 'alpha')],
+                        scheme="cohort"
+                    )
+                ],
+            },
+        })
+        problem_data = dedent("""
+            <problem markdown="Simple Problem" max_attempts="" weight="">
+              <p>Choose Yes.</p>
+              <choiceresponse>
+                <checkboxgroup direction="vertical">
+                  <choice correct="true">Yes</choice>
+                </checkboxgroup>
+              </choiceresponse>
+            </problem>
+        """)
+        vertical = self.course_fixture.get_nested_xblocks(category="vertical")[0]
+        self.course_fixture.create_xblock(
+            vertical.locator,
+            XBlockFixtureDesc('problem', "VISIBLE TO ALPHA", data=problem_data, metadata={"group_access": {0: [0]}}),
+        )
+        self.group_configurations_page.visit()
+        config = self.group_configurations_page.content_groups[0]
+        self.assertTrue(config.delete_button_is_disabled)
+
+    def test_can_delete_unused_content_group(self):
+        """
+        Scenario: Ensure that the user can delete unused content group.
+        Given I have a course with 1 Content Group
+        And I go to the Group Configuration page
+        When I delete the Content Group with name "New Content Group"
+        Then I see that there is no Content Group
+        When I refresh the page
+        Then I see that the content group has been deleted
         """
         self.group_configurations_page.visit()
         config = self.create_and_verify_content_group("New Content Group", 0)
-        self.assertTrue(config.delete_button_is_absent)
+        self.assertTrue(config.delete_button_is_present)
+
+        self.assertEqual(len(self.group_configurations_page.content_groups), 1)
+
+        # Delete content group
+        config.delete()
+        self.assertEqual(len(self.group_configurations_page.content_groups), 0)
+
+        self.group_configurations_page.visit()
+        self.assertEqual(len(self.group_configurations_page.content_groups), 0)
 
     def test_must_supply_name(self):
         """
@@ -129,6 +207,26 @@ class ContentGroupConfigurationTest(StudioCourseTest):
         config.cancel()
         self.assertEqual(0, len(self.group_configurations_page.content_groups))
 
+    def test_content_group_empty_usage(self):
+        """
+        Scenario: When content group is not used, ensure that the link to outline page works correctly.
+        Given I have a course without content group
+        And I create new content group
+        Then I see a link to the outline page
+        When I click on the outline link
+        Then I see the outline page
+        """
+        self.group_configurations_page.visit()
+        config = self.create_and_verify_content_group("New Content Group", 0)
+        config.toggle()
+        config.click_outline_anchor()
+
+        # Waiting for the page load and verify that we've landed on course outline page
+        EmptyPromise(
+            lambda: self.outline_page.is_browser_on_page(), "loaded page {!r}".format(self.outline_page),
+            timeout=30
+        ).fulfill()
+
 
 @attr('shard_1')
 class AdvancedSettingsValidationTest(StudioCourseTest):

common/test/acceptance/tests/studio/test_studio_split_test.py

@@ -449,7 +449,7 @@ class GroupConfigurationsTest(ContainerBase, SplitTestMixin):
 
         # Save the configuration
         self.assertEqual(config.get_text('.action-primary'), "Create")
-        self.assertTrue(config.delete_button_is_absent)
+        self.assertFalse(config.delete_button_is_present)
         config.save()
 
         self._assert_fields(

common/test/acceptance/tests/test_cohorted_courseware.py

@@ -47,7 +47,7 @@ class EndToEndCohortedCoursewareTest(ContainerBase):
         ).visit()
 
         # Create a student who will end up in the default cohort group
-        self.cohort_default_student_username = "cohort default student"
+        self.cohort_default_student_username = "cohort_default_student"
         self.cohort_default_student_email = "cohort_default_student@example.com"
         StudioAutoAuthPage(
             self.browser, username=self.cohort_default_student_username,

common/test/acceptance/tests/video/test_studio_video_module.py

@@ -181,6 +181,7 @@ class CMSVideoTest(CMSVideoBaseTest):
 
         self.assertTrue(self.video.is_button_shown('play'))
         self.video.click_player_button('play')
+        self.video.wait_for_state('playing')
         self.assertTrue(self.video.is_button_shown('pause'))
 
     def test_youtube_stub_blocks_youtube_api(self):

common/test/acceptance/tests/video/test_video_times.py

@@ -3,7 +3,6 @@ Acceptance tests for Video Times(Start, End and Finish) functionality.
 """
 
 from .test_video_module import VideoBaseTest
-from unittest import skip
 
 
 class VideoTimesTest(VideoBaseTest):
@@ -33,17 +32,16 @@ class VideoTimesTest(VideoBaseTest):
 
         self.assertGreaterEqual(int(self.video.position.split(':')[1]), 10)
 
-    @skip("Intermittently fails 1 Oct 2014")
     def test_video_end_time_with_default_start_time(self):
         """
         Scenario: End time works for Youtube video if starts playing from beginning.
-        Given we have a video in "Youtube" mode with end time set to 00:00:02
+        Given we have a video in "Youtube" mode with end time set to 00:00:05
         And I click video button "play"
         And I wait until video stop playing
-        Then I see video slider at "0:02" position
+        Then I see video slider at "0:05" position
 
         """
-        data = {'end_time': '00:00:02'}
+        data = {'end_time': '00:00:05'}
         self.metadata = self.metadata_for_mode('youtube', additional_data=data)
 
         # go to video
@@ -54,7 +52,7 @@ class VideoTimesTest(VideoBaseTest):
         # wait until video stop playing
         self.video.wait_for_state('pause')
 
-        self.assertEqual(self.video.position, '0:02')
+        self.assertIn(self.video.position, ('0:05', '0:06'))
 
     def test_video_end_time_wo_default_start_time(self):
         """
@@ -79,20 +77,19 @@ class VideoTimesTest(VideoBaseTest):
         # wait until video stop playing
         self.video.wait_for_state('pause')
 
-        self.assertEqual(self.video.position, '1:00')
+        self.assertIn(self.video.position, ('1:00', '1:01'))
 
-    @skip("Intermittently fails 23 Sept 2014")
     def test_video_start_time_and_end_time(self):
         """
         Scenario: Start time and end time work together for Youtube video.
-        Given we a video in "Youtube" mode with start time set to 00:00:10 and end_time set to 00:00:12
+        Given we a video in "Youtube" mode with start time set to 00:00:10 and end_time set to 00:00:15
         And I see video slider at "0:10" position
         And I click video button "play"
         Then I wait until video stop playing
-        Then I see video slider at "0:12" position
+        Then I see video slider at "0:15" position
 
         """
-        data = {'start_time': '00:00:10', 'end_time': '00:00:12'}
+        data = {'start_time': '00:00:10', 'end_time': '00:00:15'}
         self.metadata = self.metadata_for_mode('youtube', additional_data=data)
 
         # go to video
@@ -105,13 +102,12 @@ class VideoTimesTest(VideoBaseTest):
         # wait until video stop playing
         self.video.wait_for_state('pause')
 
-        self.assertEqual(self.video.position, '0:12')
+        self.assertIn(self.video.position, ('0:15', '0:16'))
 
-    @skip("Intermittently fails 03 June 2014")
     def test_video_end_time_and_finish_time(self):
         """
         Scenario: Youtube video works after pausing at end time and then plays again from End Time to the end.
-        Given we have a video in "Youtube" mode with start time set to 00:02:14 and end_time set to 00:02:15
+        Given we have a video in "Youtube" mode with start time set to 00:02:10 and end_time set to 00:02:15
         And I click video button "play"
         And I wait until video stop playing
         Then I see video slider at "2:15" position
@@ -119,7 +115,7 @@ class VideoTimesTest(VideoBaseTest):
         And I wait until video stop playing
         Then I see video slider at "2:20" position
         """
-        data = {'start_time': '00:02:14', 'end_time': '00:02:15'}
+        data = {'start_time': '00:02:10', 'end_time': '00:02:15'}
         self.metadata = self.metadata_for_mode('youtube', additional_data=data)
 
         # go to video
@@ -130,7 +126,7 @@ class VideoTimesTest(VideoBaseTest):
         # wait until video stop playing
         self.video.wait_for_state('pause')
 
-        self.assertEqual(self.video.position, '2:15')
+        self.assertIn(self.video.position, ('2:15', '2:16'))
 
         self.video.click_player_button('play')
 
@@ -142,14 +138,14 @@ class VideoTimesTest(VideoBaseTest):
     def test_video_end_time_with_seek(self):
         """
         Scenario: End Time works for Youtube Video if starts playing before Start Time.
-        Given we have a video in "Youtube" mode with end-time at 0:32 and start-time at 0:30
+        Given we have a video in "Youtube" mode with end-time at 0:35 and start-time at 0:30
         And I seek video to "0:28" position
         And I click video button "play"
         And I wait until video stop playing
-        Then I see video slider at "0:32" position
+        Then I see video slider at "0:35" position
 
         """
-        data = {'start_time': '00:00:30', 'end_time': '00:00:32'}
+        data = {'start_time': '00:00:30', 'end_time': '00:00:35'}
         self.metadata = self.metadata_for_mode('youtube', additional_data=data)
 
         # go to video
@@ -162,7 +158,7 @@ class VideoTimesTest(VideoBaseTest):
         # wait until video stop playing
         self.video.wait_for_state('pause')
 
-        self.assertEqual(self.video.position, '0:32')
+        self.assertIn(self.video.position, ('0:35', '0:36'))
 
     def test_video_finish_time_with_seek(self):
         """

common/test/data/uploads/cohort_users_both_columns.csv

@@ -1,4 +1,4 @@
 username,email,ignored_column,cohort
 instructor_user,,June,ManualCohort1
 ,student_user@example.com,Spring,AutoCohort1
-Ωπ,,Fall,ManualCohort1
+other_student_user,,Fall,ManualCohort1

common/test/data/uploads/cohort_users_only_email.csv

@@ -1,5 +1,5 @@
 email,cohort
 instructor_user@example.com,ManualCohort1
 student_user@example.com,AutoCohort1
-unicode_student_user@example.com,ManualCohort1
+other_student_user@example.com,ManualCohort1
 

common/test/data/uploads/cohort_users_only_username.csv

@@ -1,4 +1,4 @@
 username,cohort
 instructor_user,ManualCohort1
 student_user,AutoCohort1
-Ωπ,ManualCohort1
+other_student_user,ManualCohort1