REVE-37: Prevent modifications to feature based enrollment user partitions

This commit is contained in:
Gabe Mulley
2018-11-19 08:37:48 -05:00
parent 4826b69035
commit 5177e994c8
10 changed files with 93 additions and 26 deletions

View File

@@ -10,7 +10,7 @@ from contentstore.utils import reverse_usage_url
from lms.lib.utils import get_parent_unit
from openedx.core.djangoapps.course_groups.partition_scheme import get_cohorted_user_partition
from util.db import MYSQL_MAX_INT, generate_int_id
from xmodule.partitions.partitions import MINIMUM_STATIC_PARTITION_ID, UserPartition
from xmodule.partitions.partitions import MINIMUM_STATIC_PARTITION_ID, ReadOnlyUserPartitionError, UserPartition
from xmodule.partitions.partitions_service import get_all_partitions_for_course
from xmodule.split_test_module import get_split_user_partitions
@@ -105,7 +105,10 @@ class GroupConfiguration(object):
"""
Get user partition for saving in course.
"""
return UserPartition.from_json(self.configuration)
try:
return UserPartition.from_json(self.configuration)
except ReadOnlyUserPartitionError:
raise GroupConfigurationsValidationError(_("unable to load this type of group configuration"))
@staticmethod
def _get_usage_info(course, unit, item, usage_info, group_id, scheme_name=None):

View File

@@ -88,6 +88,7 @@ from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore import EdxJSONEncoder
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.exceptions import DuplicateCourseError, ItemNotFoundError
from xmodule.partitions.partitions import UserPartition
from xmodule.tabs import CourseTab, CourseTabList, InvalidTabsException
from .component import ADVANCED_COMPONENT_TYPES
@@ -1602,6 +1603,8 @@ def group_configurations_list_handler(request, course_key_string):
has_content_groups = False
displayable_partitions = []
for partition in all_partitions:
partition['read_only'] = getattr(UserPartition.get_scheme(partition['scheme']), 'read_only', False)
if partition['scheme'] == COHORT_SCHEME:
has_content_groups = True
displayable_partitions.append(partition)

View File

@@ -11,7 +11,8 @@ from operator import itemgetter
from contentstore.utils import reverse_course_url, reverse_usage_url
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 openedx.features.content_type_gating.partitions import CONTENT_GATING_PARTITION_ID
from xmodule.partitions.partitions import Group, UserPartition, ENROLLMENT_TRACK_PARTITION_ID
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.validation import StudioValidation, StudioValidationMessage
from xmodule.modulestore.django import modulestore
@@ -238,6 +239,7 @@ class GroupConfigurationsBaseTestCase(object):
self.assertIn("error", content)
@ddt.ddt
class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
"""
Test cases for group_configurations_list_handler.
@@ -329,7 +331,22 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
self.reload_course()
self.assertEqual(len(self.course.user_partitions), 0)
@ddt.data('content_type_gate', 'enrollment_track')
def test_cannot_create_restricted_group_configuration(self, scheme_id):
"""
Test that you cannot create a restricted group configuration.
"""
group_config = dict(GROUP_CONFIGURATION_JSON)
group_config['scheme'] = scheme_id
group_config.setdefault('parameters', {})['course_id'] = unicode(self.course.id)
response = self.client.ajax_post(
self._url(),
data=group_config
)
self.assertEqual(response.status_code, 400)
@ddt.ddt
class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfigurationsBaseTestCase, HelperMethods):
"""
Test cases for group_configurations_detail_handler.
@@ -633,6 +650,41 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
self.assertEqual(len(user_partititons), 2)
self.assertEqual(user_partititons[0].name, 'Name 0')
@ddt.data('content_type_gate', 'enrollment_track')
def test_cannot_create_restricted_group_configuration(self, scheme_id):
"""
Test that you cannot create a restricted group configuration.
"""
group_config = dict(GROUP_CONFIGURATION_JSON)
group_config['scheme'] = scheme_id
group_config.setdefault('parameters', {})['course_id'] = unicode(self.course.id)
response = self.client.ajax_post(
self._url(),
data=group_config
)
self.assertEqual(response.status_code, 400)
@ddt.data(
('content_type_gate', CONTENT_GATING_PARTITION_ID),
('enrollment_track', ENROLLMENT_TRACK_PARTITION_ID),
)
@ddt.unpack
def test_cannot_edit_restricted_group_configuration(self, scheme_id, partition_id):
"""
Test that you cannot edit a restricted group configuration.
"""
group_config = dict(GROUP_CONFIGURATION_JSON)
group_config['scheme'] = scheme_id
group_config.setdefault('parameters', {})['course_id'] = unicode(self.course.id)
response = self.client.put(
self._url(cid=partition_id),
data=json.dumps(group_config),
content_type="application/json",
HTTP_ACCEPT="application/json",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
self.assertEqual(response.status_code, 400)
@ddt.ddt
class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):

View File

@@ -23,7 +23,8 @@ function(Backbone, _, gettext, GroupModel, GroupCollection) {
]),
showGroups: false,
editing: false,
usage: []
usage: [],
read_only: false
};
},
@@ -78,7 +79,8 @@ function(Backbone, _, gettext, GroupModel, GroupCollection) {
scheme: this.get('scheme'),
description: this.get('description'),
version: this.get('version'),
groups: this.get('groups').toJSON()
groups: this.get('groups').toJSON(),
read_only: this.get('read_only')
};
},

View File

@@ -101,7 +101,8 @@ define([
name: 'Group 2',
usage: []
}
]
],
read_only: true
},
clientModelSpec = {
id: 10,
@@ -124,7 +125,8 @@ define([
usage: []
}
],
usage: []
usage: [],
read_only: true
},
model = new GroupConfigurationModel(
serverModelSpec, {parse: true}

View File

@@ -7,8 +7,7 @@ function($, _, gettext, BasePage, GroupConfigurationsListView, PartitionGroupLis
var GroupConfigurationsPage = BasePage.extend({
initialize: function(options) {
var currentScheme,
i,
enrollmentScheme = 'enrollment_track';
i;
BasePage.prototype.initialize.call(this);
this.experimentsEnabled = options.experimentsEnabled;
@@ -26,7 +25,7 @@ function($, _, gettext, BasePage, GroupConfigurationsListView, PartitionGroupLis
this.allGroupViewList.push(
new PartitionGroupListView({
collection: this.allGroupConfigurations[i].get('groups'),
restrictEditing: currentScheme === enrollmentScheme,
restrictEditing: this.allGroupConfigurations[i].get('read_only'),
scheme: currentScheme
})
);

View File

@@ -40,6 +40,13 @@ class NoSuchUserPartitionGroupError(UserPartitionError):
pass
class ReadOnlyUserPartitionError(UserPartitionError):
"""
Exception to be raised when attempting to modify a read only partition.
"""
pass
class Group(namedtuple("Group", "id name")):
"""
An id and name for a group of students. The id should be unique
@@ -199,6 +206,9 @@ class UserPartition(namedtuple("UserPartition", "id name description groups sche
if not scheme:
raise TypeError("UserPartition dict {0} has unrecognized scheme {1}".format(value, scheme_id))
if getattr(scheme, 'read_only', False):
raise ReadOnlyUserPartitionError("UserPartition dict {0} uses scheme {1} which is read only".format(value, scheme_id))
if hasattr(scheme, "create_user_partition"):
return scheme.create_user_partition(
value["id"],

View File

@@ -49,22 +49,14 @@ class EnrollmentTrackUserPartition(UserPartition):
for mode in CourseMode.modes_for_course(course_key, include_expired=True)
]
def from_json(self):
"""
Because this partition is dynamic, `from_json` is not supported.
`to_json` is supported, but shouldn't be used to persist this partition
within the course itself (used by Studio for sending data to front-end code)
Calling this method will raise a TypeError.
"""
raise TypeError("Because EnrollmentTrackUserPartition is a dynamic partition, 'from_json' is not supported.")
class EnrollmentTrackPartitionScheme(object):
"""
This scheme uses learner enrollment tracks to map learners into partition groups.
"""
read_only = True
@classmethod
def get_group_for_user(cls, course_key, user, user_partition, **kwargs): # pylint: disable=unused-argument
"""

View File

@@ -2,17 +2,18 @@
Tests for verified_track_content/partition_scheme.py.
"""
from datetime import datetime, timedelta
import pytz
from ..partition_scheme import EnrollmentTrackPartitionScheme, EnrollmentTrackUserPartition, ENROLLMENT_GROUP_IDS
from ..models import VerifiedTrackCohortedCourse
from course_modes.models import CourseMode
from student.models import CourseEnrollment
from student.tests.factories import UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from xmodule.partitions.partitions import UserPartition, MINIMUM_STATIC_PARTITION_ID
from xmodule.partitions.partitions import MINIMUM_STATIC_PARTITION_ID, UserPartition, ReadOnlyUserPartitionError
from ..models import VerifiedTrackCohortedCourse
from ..partition_scheme import ENROLLMENT_GROUP_IDS, EnrollmentTrackPartitionScheme, EnrollmentTrackUserPartition
class EnrollmentTrackUserPartitionTest(SharedModuleStoreTestCase):
@@ -60,8 +61,9 @@ class EnrollmentTrackUserPartitionTest(SharedModuleStoreTestCase):
self.assertEqual('Test partition for segmenting users by enrollment track', user_partition_json['description'])
def test_from_json_not_supported(self):
with self.assertRaises(TypeError):
EnrollmentTrackUserPartition.from_json()
user_partition_json = create_enrollment_track_partition(self.course).to_json()
with self.assertRaises(ReadOnlyUserPartitionError):
UserPartition.from_json(user_partition_json)
def test_group_ids(self):
"""

View File

@@ -127,6 +127,8 @@ class ContentTypeGatingPartitionScheme(object):
LIMITED_ACCESS = Group(CONTENT_TYPE_GATE_GROUP_IDS['limited_access'], 'Limited-access Users')
FULL_ACCESS = Group(CONTENT_TYPE_GATE_GROUP_IDS['full_access'], 'Full-access Users')
read_only = True
@classmethod
def get_group_for_user(cls, course_key, user, user_partition, **kwargs): # pylint: disable=unused-argument
"""