diff --git a/cms/djangoapps/contentstore/course_group_config.py b/cms/djangoapps/contentstore/course_group_config.py index e4d32e80cd..aaae31bf42 100644 --- a/cms/djangoapps/contentstore/course_group_config.py +++ b/cms/djangoapps/contentstore/course_group_config.py @@ -8,8 +8,8 @@ from util.db import generate_int_id, MYSQL_MAX_INT from django.utils.translation import ugettext as _ from contentstore.utils import reverse_usage_url -from xmodule.partitions.partitions import UserPartition -from xmodule.partitions.partitions_service import get_all_partitions_for_course, MINIMUM_STATIC_PARTITION_ID +from xmodule.partitions.partitions import UserPartition, MINIMUM_STATIC_PARTITION_ID +from xmodule.partitions.partitions_service import get_all_partitions_for_course from xmodule.split_test_module import get_split_user_partitions from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition @@ -18,11 +18,11 @@ MINIMUM_GROUP_ID = MINIMUM_STATIC_PARTITION_ID 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.' +CONTENT_GROUP_CONFIGURATION_DESCRIPTION = _( + 'The groups in this configuration can be mapped to cohorts in the Instructor Dashboard.' +) -CONTENT_GROUP_CONFIGURATION_NAME = 'Content Group Configuration' +CONTENT_GROUP_CONFIGURATION_NAME = _('Content Groups') log = logging.getLogger(__name__) diff --git a/cms/djangoapps/contentstore/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index a70193474f..f9d58d5047 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -510,7 +510,7 @@ class GetUserPartitionInfoTest(ModuleStoreTestCase): self.assertEqual(len(groups), 3) self.assertEqual(groups[2], { "id": 3, - "name": "Deleted group", + "name": "Deleted Group", "selected": True, "deleted": True }) diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 1154e18174..4a97d056d4 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -401,7 +401,7 @@ def get_user_partition_info(xblock, schemes=None, course=None): for gid in missing_group_ids: groups.append({ "id": gid, - "name": _("Deleted group"), + "name": _("Deleted Group"), "selected": True, "deleted": True, }) @@ -429,30 +429,45 @@ def get_visibility_partition_info(xblock): Returns: dict """ - user_partitions = get_user_partition_info(xblock, schemes=["verification", "cohort"]) - cohort_partitions = [] - verification_partitions = [] - has_selected_groups = False - selected_verified_partition_id = None + selectable_partitions = [] + # We wish to display enrollment partitions before cohort partitions. + enrollment_user_partitions = get_user_partition_info(xblock, schemes=["enrollment_track"]) - # Pre-process the partitions to make it easier to display the UI - for p in user_partitions: - has_selected = any(g["selected"] for g in p["groups"]) - has_selected_groups = has_selected_groups or has_selected + # For enrollment partitions, we only show them if there is a selected group or + # or if the number of groups > 1. + for partition in enrollment_user_partitions: + if len(partition["groups"]) > 1 or any(group["selected"] for group in partition["groups"]): + selectable_partitions.append(partition) - if p["scheme"] == "cohort": - cohort_partitions.append(p) - elif p["scheme"] == "verification": - verification_partitions.append(p) - if has_selected: - selected_verified_partition_id = p["id"] + # Now add the cohort user partitions. + selectable_partitions = selectable_partitions + get_user_partition_info(xblock, schemes=["cohort"]) + + # Find the first partition with a selected group. That will be the one initially enabled in the dialog + # (if the course has only been added in Studio, only one partition should have a selected group). + selected_partition_index = -1 + + # At the same time, build up all the selected groups as they are displayed in the dialog title. + selected_groups_label = '' + + for index, partition in enumerate(selectable_partitions): + for group in partition["groups"]: + if group["selected"]: + if len(selected_groups_label) == 0: + selected_groups_label = group['name'] + else: + # Translators: This is building up a list of groups. It is marked for translation because of the + # comma, which is used as a separator between each group. + selected_groups_label = _('{previous_groups}, {current_group}').format( + previous_groups=selected_groups_label, + current_group=group['name'] + ) + if selected_partition_index == -1: + selected_partition_index = index return { - "user_partitions": user_partitions, - "cohort_partitions": cohort_partitions, - "verification_partitions": verification_partitions, - "has_selected_groups": has_selected_groups, - "selected_verified_partition_id": selected_verified_partition_id, + "selectable_partitions": selectable_partitions, + "selected_partition_index": selected_partition_index, + "selected_groups_label": selected_groups_label, } diff --git a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py index 5e04336d60..8aafc88a8e 100644 --- a/cms/djangoapps/contentstore/views/tests/test_group_configurations.py +++ b/cms/djangoapps/contentstore/views/tests/test_group_configurations.py @@ -8,7 +8,7 @@ import ddt from mock import patch from contentstore.utils import reverse_course_url, reverse_usage_url -from contentstore.course_group_config import GroupConfiguration +from contentstore.course_group_config import GroupConfiguration, CONTENT_GROUP_CONFIGURATION_NAME from contentstore.tests.utils import CourseTestCase from xmodule.partitions.partitions import Group, UserPartition from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory @@ -240,7 +240,7 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations self.assertEqual(response.status_code, 200) self.assertContains(response, 'First name') self.assertContains(response, 'Group C') - self.assertContains(response, 'Content Group Configuration') + self.assertContains(response, CONTENT_GROUP_CONFIGURATION_NAME) def test_unsupported_http_accept_header(self): """ diff --git a/cms/djangoapps/contentstore/views/tests/test_item.py b/cms/djangoapps/contentstore/views/tests/test_item.py index 1bf35d1d73..7c65ac1d19 100644 --- a/cms/djangoapps/contentstore/views/tests/test_item.py +++ b/cms/djangoapps/contentstore/views/tests/test_item.py @@ -44,8 +44,9 @@ from xblock.exceptions import NoSuchHandlerError from xblock_django.user_service import DjangoXBlockUserService from opaque_keys.edx.keys import UsageKey, CourseKey from opaque_keys.edx.locations import Location -from xmodule.partitions.partitions import Group, UserPartition -from xmodule.partitions.partitions_service import ENROLLMENT_TRACK_PARTITION_ID, MINIMUM_STATIC_PARTITION_ID +from xmodule.partitions.partitions import ( + Group, UserPartition, ENROLLMENT_TRACK_PARTITION_ID, MINIMUM_STATIC_PARTITION_ID +) class AsideTest(XBlockAside): @@ -348,9 +349,9 @@ class GetItemTest(ItemTest): self.course.user_partitions = [ UserPartition( id=MINIMUM_STATIC_PARTITION_ID, - name="Verification user partition", - scheme=UserPartition.get_scheme("verification"), - description="Verification user partition", + name="Random user partition", + scheme=UserPartition.get_scheme("random"), + description="Random user partition", groups=[ Group(id=MINIMUM_STATIC_PARTITION_ID + 1, name="Group A"), # See note above. Group(id=MINIMUM_STATIC_PARTITION_ID + 2, name="Group B"), # See note above. @@ -370,7 +371,7 @@ class GetItemTest(ItemTest): self.assertEqual(result["user_partitions"], [ { "id": ENROLLMENT_TRACK_PARTITION_ID, - "name": "Enrollment Track Partition", + "name": "Enrollment Tracks", "scheme": "enrollment_track", "groups": [ { @@ -383,8 +384,8 @@ class GetItemTest(ItemTest): }, { "id": MINIMUM_STATIC_PARTITION_ID, - "name": "Verification user partition", - "scheme": "verification", + "name": "Random user partition", + "scheme": "random", "groups": [ { "id": MINIMUM_STATIC_PARTITION_ID + 1, diff --git a/cms/lib/xblock/test/test_authoring_mixin.py b/cms/lib/xblock/test/test_authoring_mixin.py index f9eb62ff5c..b16eae9092 100644 --- a/cms/lib/xblock/test/test_authoring_mixin.py +++ b/cms/lib/xblock/test/test_authoring_mixin.py @@ -1,16 +1,33 @@ """ Tests for the Studio authoring XBlock mixin. """ +from django.conf import settings +from django.test.utils import override_settings + +from course_modes.tests.factories import CourseModeFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from xmodule.partitions.partitions import Group, UserPartition +from xmodule.partitions.partitions import ( + Group, UserPartition, ENROLLMENT_TRACK_PARTITION_ID, MINIMUM_STATIC_PARTITION_ID +) class AuthoringMixinTestCase(ModuleStoreTestCase): """ Tests the studio authoring XBlock mixin. """ + GROUP_NO_LONGER_EXISTS = "This group no longer exists" + NO_CONTENT_OR_ENROLLMENT_GROUPS = "No visibility settings are defined for this component" + NO_CONTENT_ENROLLMENT_TRACK_ENABLED = "specific groups of learners based either on their enrollment track, or by content groups that you create" + NO_CONTENT_ENROLLMENT_TRACK_DISABLED = "specific groups of learners based on content groups that you create" + CONTENT_GROUPS_TITLE = "Content Groups" + ENROLLMENT_GROUPS_TITLE = "Enrollment Tracks" + STAFF_LOCKED = 'The unit that contains this component is hidden from learners' + + FEATURES_WITH_ENROLLMENT_TRACK_DISABLED = settings.FEATURES.copy() + FEATURES_WITH_ENROLLMENT_TRACK_DISABLED['ENABLE_ENROLLMENT_TRACK_USER_PARTITION'] = False + def setUp(self): """ Create a simple course with a video component. @@ -47,8 +64,8 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): """ # pylint: disable=attribute-defined-outside-init self.content_partition = UserPartition( - 1, - 'Content Groups', + MINIMUM_STATIC_PARTITION_ID, + self.CONTENT_GROUPS_TITLE, 'Contains Groups for Cohorted Courseware', content_groups, scheme_id='cohort' @@ -56,39 +73,19 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): self.course.user_partitions = [self.content_partition] self.store.update_item(self.course, self.user.id) - def create_verification_user_partitions(self, checkpoint_names): - """ - Create user partitions for verification checkpoints. - """ - scheme = UserPartition.get_scheme("verification") - self.course.user_partitions = [ - UserPartition( - id=0, - name=checkpoint_name, - description="Verification checkpoint", - scheme=scheme, - groups=[ - Group(scheme.ALLOW, "Completed verification at {}".format(checkpoint_name)), - Group(scheme.DENY, "Did not complete verification at {}".format(checkpoint_name)), - ], - ) - for checkpoint_name in checkpoint_names - ] - self.store.update_item(self.course, self.user.id) - def set_staff_only(self, item_location): """Make an item visible to staff only.""" item = self.store.get_item(item_location) item.visible_to_staff_only = True self.store.update_item(item, self.user.id) - def set_group_access(self, item_location, group_ids): + def set_group_access(self, item_location, group_ids, partition_id=None): """ Set group_access for the specified item to the specified group ids within the content partition. """ item = self.store.get_item(item_location) - item.group_access[self.content_partition.id] = group_ids + item.group_access[self.content_partition.id if partition_id is None else partition_id] = group_ids self.store.update_item(item, self.user.id) def verify_visibility_view_contains(self, item_location, substrings): @@ -101,39 +98,70 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): for string in substrings: self.assertIn(string, html) + def verify_visibility_view_does_not_contain(self, item_location, substrings): + """ + Verify that an item's visibility view returns an html string + that does NOT contain the provided substrings. + """ + item = self.store.get_item(item_location) + html = item.visibility_view().body_html() + for string in substrings: + self.assertNotIn(string, html) + def test_html_no_partition(self): - self.verify_visibility_view_contains(self.video_location, 'No content groups exist') + self.verify_visibility_view_contains(self.video_location, [self.NO_CONTENT_OR_ENROLLMENT_GROUPS]) def test_html_empty_partition(self): self.create_content_groups([]) - self.verify_visibility_view_contains(self.video_location, 'No content groups exist') + self.verify_visibility_view_contains(self.video_location, [self.NO_CONTENT_OR_ENROLLMENT_GROUPS]) def test_html_populated_partition(self): self.create_content_groups(self.pet_groups) - self.verify_visibility_view_contains(self.video_location, ['Cat Lovers', 'Dog Lovers']) + self.verify_visibility_view_contains( + self.video_location, + [self.CONTENT_GROUPS_TITLE, 'Cat Lovers', 'Dog Lovers'] + ) + + self.verify_visibility_view_does_not_contain( + self.video_location, + [self.NO_CONTENT_OR_ENROLLMENT_GROUPS, self.ENROLLMENT_GROUPS_TITLE] + ) def test_html_no_partition_staff_locked(self): self.set_staff_only(self.vertical_location) - self.verify_visibility_view_contains(self.video_location, ['No content groups exist']) + self.verify_visibility_view_contains(self.video_location, [self.NO_CONTENT_OR_ENROLLMENT_GROUPS]) + self.verify_visibility_view_does_not_contain( + self.video_location, + [self.STAFF_LOCKED, self.CONTENT_GROUPS_TITLE, self.ENROLLMENT_GROUPS_TITLE] + ) def test_html_empty_partition_staff_locked(self): self.create_content_groups([]) self.set_staff_only(self.vertical_location) - self.verify_visibility_view_contains(self.video_location, 'No content groups exist') + self.verify_visibility_view_contains(self.video_location, [self.NO_CONTENT_OR_ENROLLMENT_GROUPS]) + self.verify_visibility_view_does_not_contain( + self.video_location, + [self.STAFF_LOCKED, self.CONTENT_GROUPS_TITLE, self.ENROLLMENT_GROUPS_TITLE] + ) def test_html_populated_partition_staff_locked(self): self.create_content_groups(self.pet_groups) self.set_staff_only(self.vertical_location) self.verify_visibility_view_contains( self.video_location, - ['The Unit this component is contained in is hidden from students.', 'Cat Lovers', 'Dog Lovers'] + [self.STAFF_LOCKED, self.CONTENT_GROUPS_TITLE, 'Cat Lovers', 'Dog Lovers'] ) def test_html_false_content_group(self): self.create_content_groups(self.pet_groups) self.set_group_access(self.video_location, ['false_group_id']) self.verify_visibility_view_contains( - self.video_location, ['Cat Lovers', 'Dog Lovers', 'Content group no longer exists.'] + self.video_location, + [self.CONTENT_GROUPS_TITLE, 'Cat Lovers', 'Dog Lovers', self.GROUP_NO_LONGER_EXISTS] + ) + self.verify_visibility_view_does_not_contain( + self.video_location, + [self.STAFF_LOCKED] ) def test_html_false_content_group_staff_locked(self): @@ -145,18 +173,84 @@ class AuthoringMixinTestCase(ModuleStoreTestCase): [ 'Cat Lovers', 'Dog Lovers', - 'The Unit this component is contained in is hidden from students.', - 'Content group no longer exists.' + self.STAFF_LOCKED, + self.GROUP_NO_LONGER_EXISTS ] ) - def test_html_verification_checkpoints(self): - self.create_verification_user_partitions(["Midterm A", "Midterm B"]) + @override_settings(FEATURES=FEATURES_WITH_ENROLLMENT_TRACK_DISABLED) + def test_enrollment_tracks_disabled(self): + """ + Test that the "no groups" messages doesn't reference enrollment tracks if + they are disabled. + """ + self.verify_visibility_view_contains( + self.video_location, + [self.NO_CONTENT_OR_ENROLLMENT_GROUPS, self.NO_CONTENT_ENROLLMENT_TRACK_DISABLED] + ) + self.verify_visibility_view_does_not_contain(self.video_location, [self.NO_CONTENT_ENROLLMENT_TRACK_ENABLED]) + + def test_enrollment_track_partitions_only(self): + """ + Test what is displayed with no content groups but 2 enrollment modes registered. + In all the cases where no enrollment modes are explicitly added, only the default + enrollment mode exists, and we do not show it as an option (unless the course staff + member has previously selected it). + """ + CourseModeFactory.create(course_id=self.course.id, mode_slug='audit') + CourseModeFactory.create(course_id=self.course.id, mode_slug='verified') + self.verify_visibility_view_contains( + self.video_location, + [self.ENROLLMENT_GROUPS_TITLE, 'audit course', 'verified course'] + ) + self.verify_visibility_view_does_not_contain( + self.video_location, + [self.NO_CONTENT_OR_ENROLLMENT_GROUPS, self.CONTENT_GROUPS_TITLE] + ) + + def test_enrollment_track_partitions_and_content_groups(self): + """ + Test what is displayed with both enrollment groups and content groups. + """ + CourseModeFactory.create(course_id=self.course.id, mode_slug='audit') + CourseModeFactory.create(course_id=self.course.id, mode_slug='verified') + self.create_content_groups(self.pet_groups) self.verify_visibility_view_contains( self.video_location, [ - "Verification Checkpoint", - "Midterm A", - "Midterm B", + self.CONTENT_GROUPS_TITLE, 'Cat Lovers', 'Dog Lovers', + self.ENROLLMENT_GROUPS_TITLE, 'audit course', 'verified course' ] ) + self.verify_visibility_view_does_not_contain( + self.video_location, + [self.NO_CONTENT_OR_ENROLLMENT_GROUPS] + ) + + def test_missing_enrollment_mode(self): + """ + Test that an enrollment mode that is no longer registered is displayed as 'deleted', + regardless of the number of current enrollment modes in the course. + """ + # Only 1 mode (the default) exists, so nothing initially shows in the visibility view. + self.verify_visibility_view_contains( + self.video_location, + [self.NO_CONTENT_OR_ENROLLMENT_GROUPS, self.NO_CONTENT_ENROLLMENT_TRACK_ENABLED] + ) + self.verify_visibility_view_does_not_contain( + self.video_location, [self.ENROLLMENT_GROUPS_TITLE, self.GROUP_NO_LONGER_EXISTS] + ) + + # Set group_access to reference a missing mode. + self.set_group_access(self.video_location, ['10'], ENROLLMENT_TRACK_PARTITION_ID) + self.verify_visibility_view_contains( + self.video_location, [self.ENROLLMENT_GROUPS_TITLE, self.GROUP_NO_LONGER_EXISTS] + ) + + # Add 2 explicit enrollment modes. + CourseModeFactory.create(course_id=self.course.id, mode_slug='audit') + CourseModeFactory.create(course_id=self.course.id, mode_slug='verified') + self.verify_visibility_view_contains( + self.video_location, + [self.ENROLLMENT_GROUPS_TITLE, 'audit course', 'verified course', self.GROUP_NO_LONGER_EXISTS] + ) diff --git a/cms/static/js/models/xblock_info.js b/cms/static/js/models/xblock_info.js index fd7fdd8d90..7c44a07216 100644 --- a/cms/static/js/models/xblock_info.js +++ b/cms/static/js/models/xblock_info.js @@ -256,19 +256,6 @@ function(Backbone, _, str, ModuleUtils) { */ isEditableOnCourseOutline: function() { return this.isSequential() || this.isChapter() || this.isVertical(); - }, - - /* - * Check whether any verification checkpoints are defined in the course. - * Verification checkpoints are defined if there exists a user partition - * that uses the verification partition scheme. - */ - hasVerifiedCheckpoints: function() { - var partitions = this.get('user_partitions') || []; - - return Boolean(_.find(partitions, function(p) { - return p.scheme === 'verification'; - })); } }); return XBlockInfo; diff --git a/cms/static/js/views/modals/course_outline_modals.js b/cms/static/js/views/modals/course_outline_modals.js index 2c48c7e469..36b95b3cec 100644 --- a/cms/static/js/views/modals/course_outline_modals.js +++ b/cms/static/js/views/modals/course_outline_modals.js @@ -15,7 +15,7 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', 'use strict'; var CourseOutlineXBlockModal, SettingsXBlockModal, PublishXBlockModal, AbstractEditor, BaseDateEditor, ReleaseDateEditor, DueDateEditor, GradingEditor, PublishEditor, AbstractVisibilityEditor, StaffLockEditor, - ContentVisibilityEditor, VerificationAccessEditor, TimedExaminationPreferenceEditor, AccessEditor; + ContentVisibilityEditor, TimedExaminationPreferenceEditor, AccessEditor; CourseOutlineXBlockModal = BaseModal.extend({ events: _.extend({}, BaseModal.prototype.events, { @@ -720,109 +720,6 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', } }); - VerificationAccessEditor = AbstractEditor.extend({ - templateName: 'verification-access-editor', - className: 'edit-verification-access', - - // This constant MUST match the group ID - // defined by VerificationPartitionScheme on the backend! - ALLOW_GROUP_ID: 1, - - getSelectedPartition: function() { - var hasRestrictions = $('#verification-access-checkbox').is(':checked'), - selectedPartitionID = null; - - if (hasRestrictions) { - selectedPartitionID = $('#verification-partition-select').val(); - } - - return parseInt(selectedPartitionID, 10); - }, - - getGroupAccess: function() { - var groupAccess = _.clone(this.model.get('group_access')) || [], - userPartitions = this.model.get('user_partitions') || [], - selectedPartition = this.getSelectedPartition(), - that = this; - - // We display a simplified UI to course authors. - // On the backend, each verification checkpoint is associated - // with a user partition that has two groups. For example, - // if two checkpoints were defined, they might look like: - // - // Midterm A: |-- ALLOW --|-- DENY --| - // Midterm B: |-- ALLOW --|-- DENY --| - // - // To make life easier for course authors, we display - // *one* option for each checkpoint: - // - // [X] Must complete verification checkpoint - // Dropdown: - // * Midterm A - // * Midterm B - // - // This is where we map the simplified UI to - // the underlying user partition. If the user checked - // the box, that means there *is* a restriction, - // so only the "ALLOW" group for the selected partition has access. - // Otherwise, all groups in the partition have access. - // - _.each(userPartitions, function(partition) { - if (partition.scheme === 'verification') { - if (selectedPartition === partition.id) { - groupAccess[partition.id] = [that.ALLOW_GROUP_ID]; - } else { - delete groupAccess[partition.id]; - } - } - }); - - return groupAccess; - }, - - getRequestData: function() { - var groupAccess = this.getGroupAccess(), - hasChanges = !_.isEqual(groupAccess, this.model.get('group_access')); - - return hasChanges ? { - publish: 'republish', - metadata: { - group_access: groupAccess - } - } : {}; - }, - - getContext: function() { - var partitions = this.model.get('user_partitions'), - hasRestrictions = false, - verificationPartitions = [], - isSelected = false; - - // Display a simplified version of verified partition schemes. - // Although there are two groups defined (ALLOW and DENY), - // we show only the ALLOW group. - // To avoid searching all the groups, we're assuming that the editor - // either sets the ALLOW group or doesn't set any groups (implicitly allow all). - _.each(partitions, function(item) { - if (item.scheme === 'verification') { - isSelected = _.any(_.pluck(item.groups, 'selected')); - hasRestrictions = hasRestrictions || isSelected; - - verificationPartitions.push({ - 'id': item.id, - 'name': item.name, - 'selected': isSelected - }); - } - }); - - return { - 'hasVerificationRestrictions': hasRestrictions, - 'verificationPartitions': verificationPartitions - }; - } - }); - return { getModal: function(type, xblockInfo, options) { if (type === 'edit') { @@ -837,10 +734,6 @@ define(['jquery', 'backbone', 'underscore', 'gettext', 'js/views/baseview', var editors = []; if (xblockInfo.isVertical()) { editors = [StaffLockEditor]; - - if (xblockInfo.hasVerifiedCheckpoints()) { - editors.push(VerificationAccessEditor); - } } else { tabs = [ { diff --git a/cms/static/js/xblock/authoring.js b/cms/static/js/xblock/authoring.js index ffeed552b9..5cb7bf288c 100644 --- a/cms/static/js/xblock/authoring.js +++ b/cms/static/js/xblock/authoring.js @@ -5,25 +5,26 @@ 'use strict'; function VisibilityEditorView(runtime, element) { + this.getGroupAccess = function() { var groupAccess = {}, - checkboxValues, partitionId, - groupId, + groupId; - // This constant MUST match the group ID - // defined by VerificationPartitionScheme on the backend! - ALLOW_GROUP_ID = 1; + // Get the selected user partition (only allowed to select one). + partitionId = parseInt(element.find('.partition-visibility select').val(), 10); - if (element.find('.visibility-level-all').prop('checked')) { + // "All Learners and Staff" is selected (or "Choose one", which is only shown when + // current visibility is "All Learners and Staff" at the time the dialog is opened). + if (partitionId === -1) { return {}; } - // Cohort partitions (user is allowed to select more than one) - element.find('.field-visibility-content-group input:checked').each(function(index, input) { - checkboxValues = $(input).val().split('-'); - partitionId = parseInt(checkboxValues[0], 10); - groupId = parseInt(checkboxValues[1], 10); + // Otherwise get the checked groups within the selected partition. + element.find( + '.partition-group-visibility-' + partitionId + ' input:checked' + ).each(function(index, input) { + groupId = parseInt($(input).val(), 10); if (groupAccess.hasOwnProperty(partitionId)) { groupAccess[partitionId].push(groupId); @@ -32,38 +33,25 @@ } }); - // Verification partitions (user can select exactly one) - if (element.find('#verification-access-checkbox').prop('checked')) { - partitionId = parseInt($('#verification-access-dropdown').val(), 10); - groupAccess[partitionId] = [ALLOW_GROUP_ID]; - } - return groupAccess; }; - // When selecting "all students and staff", uncheck the specific groups - element.find('.field-visibility-level input').change(function(event) { - if ($(event.target).hasClass('visibility-level-all')) { - element.find('.field-visibility-content-group input, .field-visibility-verification input') - .prop('checked', false); + element.find('.partition-visibility select').change(function(event) { + var partitionId; + + // Hide all the partition group options. + element.find('.partition-group-control').addClass('is-hidden'); + + // If a partition is selected, display its groups. + partitionId = parseInt($(event.target).val(), 10); + if (partitionId >= 0) { + element.find('.partition-group-control-' + partitionId).removeClass('is-hidden'); } }); - - // When selecting a specific group, deselect "all students and staff" and - // select "specific content groups" instead.` - element.find('.field-visibility-content-group input, .field-visibility-verification input') - .change(function() { - element.find('.visibility-level-all').prop('checked', false); - element.find('.visibility-level-specific').prop('checked', true); - }); } VisibilityEditorView.prototype.collectFieldData = function collectFieldData() { - return { - metadata: { - 'group_access': this.getGroupAccess() - } - }; + return {metadata: {group_access: this.getGroupAccess()}}; }; function initializeVisibilityEditor(runtime, element) { diff --git a/cms/static/sass/_build-v1.scss b/cms/static/sass/_build-v1.scss index 94cec810c8..b62a4f855d 100644 --- a/cms/static/sass/_build-v1.scss +++ b/cms/static/sass/_build-v1.scss @@ -91,3 +91,5 @@ // CAPA Problem Feedback @import 'edx-pattern-library-shims/buttons'; +@import 'edx-pattern-library-shims/base/variables'; + diff --git a/cms/static/sass/elements/_modal-window.scss b/cms/static/sass/elements/_modal-window.scss index 472c869a27..7bcb7e9672 100644 --- a/cms/static/sass/elements/_modal-window.scss +++ b/cms/static/sass/elements/_modal-window.scss @@ -1,6 +1,8 @@ // studio - elements - modal-window // ======================== +@import 'edx-pattern-library-shims/base/variables'; + // start with the view/body [class*="view-"] { @@ -482,59 +484,59 @@ // MODAL TYPE: component - visibility modal .xblock-visibility_view { - .visibility-controls-secondary { - max-height: 100%; - overflow-y: auto; - @include margin(($baseline*0.75), 0, 0, $baseline); + // We don't wish the dialog to resize for the common case of 2 groups. + min-height: 190px; + + .visibility-header { + padding-bottom: $baseline; + margin-bottom: 0; + color: $gray-d3; } - .visibility-controls-group { - @extend %wipe-last-child; - margin-bottom: $baseline; + .current-visibility-title { + font-weight: font-weight(semi-bold); + + .icon { + @include margin-right($baseline/8); + } + } + + .group-select-title { + font-weight: font-weight(semi-bold); + font-size: inherit; + } + + .partition-visibility { + padding-top: $baseline; } // UI: form fields - .list-fields { + .partition-group-control { + + padding-top: ($baseline/2); .field { - @extend %wipe-last-child; - margin-bottom: ($baseline/4); - - label { - @extend %t-copy-sub1; - } - } - - // UI: radio and checkbox inputs - .field-radio, .field-checkbox { + margin-top: ($baseline/4); label { @include margin-left($baseline/4); + font-size: inherit; } } } - .field-visibility-verification { - .note { - @extend %t-copy-sub2; - @extend %t-regular; - margin: 14px 0 0 24px; - display: block; - } - } - - // CASE: content group has been removed - .field-visibility-content-group.was-removed { + // CASE: content or enrollment group has been removed + .partition-group-visibility.was-removed { .input-checkbox:checked ~ label { - color: $color-error; + color: $error-color; } .note { @extend %t-copy-sub2; @extend %t-regular; display: block; - color: $color-error; + color: $error-color; } } @@ -698,7 +700,7 @@ } // UI: staff lock section - .edit-staff-lock, .edit-settings-timed-examination, .edit-verification-access { + .edit-staff-lock, .edit-settings-timed-examination { .checkbox-cosmetic .input-checkbox { @extend %cont-text-sr; @@ -730,13 +732,6 @@ } } - .verification-access { - .checkbox-cosmetic .label { - @include float(left); - margin: 2px 6px 0 0; - } - } - // UI: timed and proctored exam section .edit-settings-timed-examination { diff --git a/cms/templates/js/publish-xblock.underscore b/cms/templates/js/publish-xblock.underscore index 44b0f8641e..cab0f43246 100644 --- a/cms/templates/js/publish-xblock.underscore +++ b/cms/templates/js/publish-xblock.underscore @@ -86,7 +86,7 @@ var visibleToStaffOnly = visibilityState === 'staff_only'; <% if (hasContentGroupComponents) { %>

- <%- gettext("Some content in this unit is visible only to particular content groups") %> + <%- gettext("Some content in this unit is visible only to specific groups of learners.") %>

<% } %>