diff --git a/common/djangoapps/user_api/migrations/0002_auto__add_usercoursetags__add_unique_usercoursetags_user_course_id_key.py b/common/djangoapps/user_api/migrations/0002_auto__add_usercoursetags__add_unique_usercoursetags_user_course_id_key.py new file mode 100644 index 0000000000..cc7e27cc4d --- /dev/null +++ b/common/djangoapps/user_api/migrations/0002_auto__add_usercoursetags__add_unique_usercoursetags_user_course_id_key.py @@ -0,0 +1,87 @@ +# -*- 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 model 'UserCourseTags' + db.create_table('user_api_usercoursetags', ( + ('id', self.gf('django.db.models.fields.AutoField')(primary_key=True)), + ('user', self.gf('django.db.models.fields.related.ForeignKey')(related_name='+', to=orm['auth.User'])), + ('key', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('course_id', self.gf('django.db.models.fields.CharField')(max_length=255, db_index=True)), + ('value', self.gf('django.db.models.fields.TextField')()), + )) + db.send_create_signal('user_api', ['UserCourseTags']) + + # Adding unique constraint on 'UserCourseTags', fields ['user', 'course_id', 'key'] + db.create_unique('user_api_usercoursetags', ['user_id', 'course_id', 'key']) + + + def backwards(self, orm): + # Removing unique constraint on 'UserCourseTags', fields ['user', 'course_id', 'key'] + db.delete_unique('user_api_usercoursetags', ['user_id', 'course_id', 'key']) + + # Deleting model 'UserCourseTags' + db.delete_table('user_api_usercoursetags') + + + 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'}) + }, + 'user_api.usercoursetags': { + 'Meta': {'unique_together': "(('user', 'course_id', 'key'),)", 'object_name': 'UserCourseTags'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'key': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['auth.User']"}), + 'value': ('django.db.models.fields.TextField', [], {}) + }, + 'user_api.userpreference': { + 'Meta': {'unique_together': "(('user', 'key'),)", 'object_name': 'UserPreference'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'key': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['auth.User']"}), + 'value': ('django.db.models.fields.TextField', [], {}) + } + } + + complete_apps = ['user_api'] \ No newline at end of file diff --git a/common/djangoapps/user_api/migrations/0003_rename_usercoursetags.py b/common/djangoapps/user_api/migrations/0003_rename_usercoursetags.py new file mode 100644 index 0000000000..dc448817e3 --- /dev/null +++ b/common/djangoapps/user_api/migrations/0003_rename_usercoursetags.py @@ -0,0 +1,72 @@ +# -*- 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): + db.rename_table('user_api_usercoursetags', 'user_api_usercoursetag') + + + def backwards(self, orm): + db.rename_table('user_api_usercoursetag', 'user_api_usercoursetags') + + + 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'}) + }, + 'user_api.usercoursetag': { + 'Meta': {'unique_together': "(('user', 'course_id', 'key'),)", 'object_name': 'UserCourseTag'}, + 'course_id': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'key': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['auth.User']"}), + 'value': ('django.db.models.fields.TextField', [], {}) + }, + 'user_api.userpreference': { + 'Meta': {'unique_together': "(('user', 'key'),)", 'object_name': 'UserPreference'}, + 'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}), + 'key': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}), + 'user': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'+'", 'to': "orm['auth.User']"}), + 'value': ('django.db.models.fields.TextField', [], {}) + } + } + + complete_apps = ['user_api'] \ No newline at end of file diff --git a/common/djangoapps/user_api/models.py b/common/djangoapps/user_api/models.py index 77ab4c6c95..48a85c2bc7 100644 --- a/common/djangoapps/user_api/models.py +++ b/common/djangoapps/user_api/models.py @@ -33,3 +33,17 @@ class UserPreference(models.Model): return user_pref.value except cls.DoesNotExist: return default + + +class UserCourseTag(models.Model): + """ + Per-course user tags, to be used by various things that want to store tags about + the user. Added initially to store assignment to experimental groups. + """ + user = models.ForeignKey(User, db_index=True, related_name="+") + key = models.CharField(max_length=255, db_index=True) + course_id = models.CharField(max_length=255, db_index=True) + value = models.TextField() + + class Meta: + unique_together = ("user", "course_id", "key") diff --git a/common/djangoapps/user_api/tests/test_user_service.py b/common/djangoapps/user_api/tests/test_user_service.py new file mode 100644 index 0000000000..f63f702bcb --- /dev/null +++ b/common/djangoapps/user_api/tests/test_user_service.py @@ -0,0 +1,34 @@ +""" +Test the user service +""" +from django.test import TestCase + +from student.tests.factories import UserFactory +from user_api import user_service + + +class TestUserService(TestCase): + """ + Test the user service + """ + def setUp(self): + self.user = UserFactory.create() + self.course_id = 'test_org/test_course_number/test_run' + self.test_key = 'test_key' + + def test_get_set_course_tag(self): + # get a tag that doesn't exist + tag = user_service.get_course_tag(self.user, self.course_id, self.test_key) + self.assertIsNone(tag) + + # test setting a new key + test_value = 'value' + user_service.set_course_tag(self.user, self.course_id, self.test_key, test_value) + tag = user_service.get_course_tag(self.user, self.course_id, self.test_key) + self.assertEqual(tag, test_value) + + #test overwriting an existing key + test_value = 'value2' + user_service.set_course_tag(self.user, self.course_id, self.test_key, test_value) + tag = user_service.get_course_tag(self.user, self.course_id, self.test_key) + self.assertEqual(tag, test_value) diff --git a/common/djangoapps/user_api/user_service.py b/common/djangoapps/user_api/user_service.py new file mode 100644 index 0000000000..a6355cd1ef --- /dev/null +++ b/common/djangoapps/user_api/user_service.py @@ -0,0 +1,61 @@ +""" +A service-like user_info interface. Could be made into an http API later, but for now +just in-process. Exposes global and per-course key-value pairs for users. + +Implementation note: +Stores global metadata using the UserPreference model, and per-course metadata using the +UserCourseTag model. +""" + +from user_api.models import UserCourseTag + + +def get_course_tag(user, course_id, key): + """ + Gets the value of the user's course tag for the specified key in the specified + course_id. + + Args: + user: the User object for the course tag + course_id: course identifier (string) + key: arbitrary (<=255 char string) + + Returns: + string value, or None if there is no value saved + """ + try: + record = UserCourseTag.objects.get( + user=user, + course_id=course_id, + key=key) + + return record.value + except UserCourseTag.DoesNotExist: + return None + + +def set_course_tag(user, course_id, key, value): + """ + Sets the value of the user's course tag for the specified key in the specified + course_id. Overwrites any previous value. + + The intention is that the values are fairly short, as they will be included in all + analytics events about this user. + + Args: + user: the User object + course_id: course identifier (string) + key: arbitrary (<=255 char string) + value: arbitrary string + """ + + record, _ = UserCourseTag.objects.get_or_create( + user=user, + course_id=course_id, + key=key) + + record.value = value + record.save() + + # TODO: There is a risk of IntegrityErrors being thrown here given + # simultaneous calls from many processes. Handle by retrying after a short delay? diff --git a/common/lib/xmodule/setup.py b/common/lib/xmodule/setup.py index f487081f8f..58d8170967 100644 --- a/common/lib/xmodule/setup.py +++ b/common/lib/xmodule/setup.py @@ -17,6 +17,7 @@ XMODULES = [ "problem = xmodule.capa_module:CapaDescriptor", "problemset = xmodule.seq_module:SequenceDescriptor", "randomize = xmodule.randomize_module:RandomizeDescriptor", + "split_test = xmodule.split_test_module:SplitTestDescriptor", "section = xmodule.backcompat_module:SemanticSectionDescriptor", "sequential = xmodule.seq_module:SequenceDescriptor", "slides = xmodule.backcompat_module:TranslateCustomTagDescriptor", diff --git a/common/lib/xmodule/xmodule/course_module.py b/common/lib/xmodule/xmodule/course_module.py index c56569dd3a..c728da1cae 100644 --- a/common/lib/xmodule/xmodule/course_module.py +++ b/common/lib/xmodule/xmodule/course_module.py @@ -9,6 +9,7 @@ import dateutil.parser from lazy import lazy from xmodule.modulestore import Location +from xmodule.partitions.partitions import UserPartition from xmodule.seq_module import SequenceDescriptor, SequenceModule from xmodule.graders import grader_from_conf import json @@ -156,10 +157,27 @@ class TextbookList(List): return json_data +class UserPartitionList(List): + def from_json(self, values): + return [UserPartition.from_json(v) for v in values] + + def to_json(self, values): + return [user_partition.to_json() + for user_partition in values] + + class CourseFields(object): lti_passports = List(help="LTI tools passports as id:client_key:client_secret", scope=Scope.settings) textbooks = TextbookList(help="List of pairs of (title, url) for textbooks used in this course", default=[], scope=Scope.content) + + # This field is intended for Studio to update, not to be exposed directly via + # advanced_settings. + user_partitions = UserPartitionList( + help="List of user partitions of this course into groups, used e.g. for experiments", + default=[], scope=Scope.content) + + wiki_slug = String(help="Slug that points to the wiki for this course", scope=Scope.content) enrollment_start = Date(help="Date that enrollment for this class is opened", scope=Scope.settings) enrollment_end = Date(help="Date that enrollment for this class is closed", scope=Scope.settings) @@ -354,7 +372,7 @@ class CourseFields(object): # Ensure that courses imported from XML keep their image default="images_course_image.jpg" ) - + ## Course level Certificate Name overrides. cert_name_short = String( help="Sitewide name of completion statements given to students (short).", diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 6ee52a8b01..381292064f 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -321,13 +321,13 @@ class MongoModuleStore(ModuleStoreWriteBase): ''' TODO (cdodge) This method can be deleted when the 'split module store' work has been completed ''' - # get all collections in the course, this query should not return any leaf nodes # note this is a bit ugly as when we add new categories of containers, we have to add it here + + block_types_with_children = set(name for name, class_ in XBlock.load_classes() if getattr(class_, 'has_children', False)) query = {'_id.org': location.org, '_id.course': location.course, - '_id.category': {'$in': ['course', 'chapter', 'sequential', 'vertical', 'videosequence', - 'wrapper', 'problemset', 'conditional', 'randomize']} + '_id.category': {'$in': list(block_types_with_children)} } # we just want the Location, children, and inheritable metadata record_filter = {'_id': 1, 'definition.children': 1} diff --git a/common/lib/xmodule/xmodule/partitions/__init__.py b/common/lib/xmodule/xmodule/partitions/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/common/lib/xmodule/xmodule/partitions/partitions.py b/common/lib/xmodule/xmodule/partitions/partitions.py new file mode 100644 index 0000000000..e5d78b79e8 --- /dev/null +++ b/common/lib/xmodule/xmodule/partitions/partitions.py @@ -0,0 +1,116 @@ +"""Defines ``Group`` and ``UserPartition`` models for partitioning""" + +# We use ``id`` in this file as the IDs of our Groups and UserPartitions, +# which Pylint disapproves of. +# pylint: disable=invalid-name, redefined-builtin + + +class Group(object): + """ + An id and name for a group of students. The id should be unique + within the UserPartition this group appears in. + """ + # in case we want to add to this class, a version will be handy + # for deserializing old versions. (This will be serialized in courses) + VERSION = 1 + + def __init__(self, id, name): + self.id = int(id) + self.name = name + + def to_json(self): + """ + 'Serialize' to a json-serializable representation. + + Returns: + a dictionary with keys for the properties of the group. + """ + return { + "id": self.id, + "name": self.name, + "version": Group.VERSION + } + + @staticmethod + def from_json(value): + """ + Deserialize a Group from a json-like representation. + + Args: + value: a dictionary with keys for the properties of the group. + + Raises TypeError if the value doesn't have the right keys. + """ + for key in ('id', 'name', 'version'): + if key not in value: + raise TypeError("Group dict {0} missing value key '{1}'".format( + value, key)) + + if value["version"] != Group.VERSION: + raise TypeError("Group dict {0} has unexpected version".format( + value)) + + return Group(value["id"], value["name"]) + + +class UserPartition(object): + """ + A named way to partition users into groups, primarily intended for running + experiments. It is expected that each user will be in at most one group in a + partition. + + A Partition has an id, name, description, and a list of groups. + The id is intended to be unique within the context where these are used. (e.g. for + partitions of users within a course, the ids should be unique per-course) + """ + VERSION = 1 + + def __init__(self, id, name, description, groups): + + self.id = int(id) + self.name = name + self.description = description + self.groups = groups + + def to_json(self): + """ + 'Serialize' to a json-serializable representation. + + Returns: + a dictionary with keys for the properties of the partition. + """ + return { + "id": self.id, + "name": self.name, + "description": self.description, + "groups": [g.to_json() for g in self.groups], + "version": UserPartition.VERSION + } + + @staticmethod + def from_json(value): + """ + Deserialize a Group from a json-like representation. + + Args: + value: a dictionary with keys for the properties of the group. + + Raises TypeError if the value doesn't have the right keys. + """ + for key in ('id', 'name', 'description', 'version', 'groups'): + if key not in value: + raise TypeError("UserPartition dict {0} missing value key '{1}'" + .format(value, key)) + + if value["version"] != UserPartition.VERSION: + raise TypeError("UserPartition dict {0} has unexpected version" + .format(value)) + + groups = [Group.from_json(g) for g in value["groups"]] + + return UserPartition( + value["id"], + value["name"], + value["description"], + groups + ) diff --git a/common/lib/xmodule/xmodule/partitions/partitions_service.py b/common/lib/xmodule/xmodule/partitions/partitions_service.py new file mode 100644 index 0000000000..3fb5f06e6c --- /dev/null +++ b/common/lib/xmodule/xmodule/partitions/partitions_service.py @@ -0,0 +1,138 @@ +""" +This is a service-like API that assigns tracks which groups users are in for various +user partitions. It uses the user_service key/value store provided by the LMS runtime to +persist the assignments. +""" +import random +from abc import ABCMeta, abstractproperty + + +class PartitionService(object): + """ + This is an XBlock service that assigns tracks which groups users are in for various + user partitions. It uses the provided user_tags service object to + persist the assignments. + """ + __metaclass__ = ABCMeta + + @abstractproperty + def course_partitions(self): + """ + Return the set of partitions assigned to self._course_id + """ + raise NotImplementedError('Subclasses must implement course_partition') + + def __init__(self, user_tags_service, course_id, track_function): + self.random = random.Random() + self._user_tags_service = user_tags_service + self._course_id = course_id + self._track_function = track_function + + def get_user_group_for_partition(self, user_partition_id): + """ + If the user is already assigned to a group in user_partition_id, return the + group_id. + + If not, assign them to one of the groups, persist that decision, and + return the group_id. + + If the group they are assigned to doesn't exist anymore, re-assign to one of + the existing groups and return its id. + + Args: + user_partition_id -- an id of a partition that's hopefully in the + runtime.user_partitions list. + + Returns: + The id of one of the groups in the specified user_partition_id (as a string). + + Raises: + ValueError if the user_partition_id isn't found. + """ + user_partition = self._get_user_partition(user_partition_id) + if user_partition is None: + raise ValueError( + "Configuration problem! No user_partition with id {0} " + "in course {1}".format(user_partition_id, self._course_id) + ) + + group_id = self._get_group(user_partition) + + return group_id + + def _get_user_partition(self, user_partition_id): + """ + Look for a user partition with a matching id in + in the course's partitions. + + Returns: + A UserPartition, or None if not found. + """ + for partition in self.course_partitions: + if partition.id == user_partition_id: + return partition + + return None + + def _key_for_partition(self, user_partition): + """ + Returns the key to use to look up and save the user's group for a particular + condition. Always use this function rather than constructing the key directly. + """ + return 'xblock.partition_service.partition_{0}'.format(user_partition.id) + + def _get_group(self, user_partition): + """ + Return the group of the current user in user_partition. If they don't already have + one assigned, pick one and save it. Uses the runtime's user_service service to look up + and persist the info. + """ + key = self._key_for_partition(user_partition) + scope = self._user_tags_service.COURSE + + group_id = self._user_tags_service.get_tag(scope, key) + if group_id is not None: + group_id = int(group_id) + + partition_group_ids = [group.id for group in user_partition.groups] + + # If a valid group id has been saved already, return it + if group_id is not None and group_id in partition_group_ids: + return group_id + + # TODO: what's the atomicity of the get above and the save here? If it's not in a + # single transaction, we could get a situation where the user sees one state in one + # thread, but then that decision gets overwritten--low probability, but still bad. + + # (If it is truly atomic, we should be fine--if one process is in the + # process of finding no group and making one, the other should block till it + # appears. HOWEVER, if we allow reads by the second one while the first + # process runs the transaction, we have a problem again: could read empty, + # have the first transaction finish, and pick a different group in a + # different process.) + + # If a group id hasn't yet been saved, or the saved group id is invalid, + # we need to pick one, save it, then return it + + # TODO: had a discussion in arch council about making randomization more + # deterministic (e.g. some hash). Could do that, but need to be careful not + # to introduce correlation between users or bias in generation. + + # See note above for explanation of local_random() + group = self.random.choice(user_partition.groups) + self._user_tags_service.set_tag(scope, key, group.id) + + # emit event for analytics + # FYI - context is always user ID that is logged in, NOT the user id that is + # being operated on. If instructor can move user explicitly, then we should + # put in event_info the user id that is being operated on. + event_info = { + 'group_id': group.id, + 'group_name': group.name, + 'partition_id': user_partition.id, + 'partition_name': user_partition.name + } + # TODO: Use the XBlock publish api instead + self._track_function('edx.split_test.assigned_user_to_partition', event_info) + + return group.id diff --git a/common/lib/xmodule/xmodule/partitions/test_partitions.py b/common/lib/xmodule/xmodule/partitions/test_partitions.py new file mode 100644 index 0000000000..d468af307b --- /dev/null +++ b/common/lib/xmodule/xmodule/partitions/test_partitions.py @@ -0,0 +1,155 @@ +""" +Test the partitions and partitions service + +""" + +from unittest import TestCase +from mock import Mock, MagicMock + +from xmodule.partitions.partitions import Group, UserPartition +from xmodule.partitions.partitions_service import PartitionService + + +class TestGroup(TestCase): + """Test constructing groups""" + def test_construct(self): + test_id = 10 + name = "Grendel" + group = Group(test_id, name) + self.assertEqual(group.id, test_id) + self.assertEqual(group.name, name) + + def test_string_id(self): + test_id = "10" + name = "Grendel" + group = Group(test_id, name) + self.assertEqual(group.id, 10) + + def test_to_json(self): + test_id = 10 + name = "Grendel" + group = Group(test_id, name) + jsonified = group.to_json() + act_jsonified = { + "id": test_id, + "name": name, + "version": group.VERSION + } + self.assertEqual(jsonified, act_jsonified) + + def test_from_json(self): + test_id = 5 + name = "Grendel" + jsonified = { + "id": test_id, + "name": name, + "version": Group.VERSION + } + group = Group.from_json(jsonified) + self.assertEqual(group.id, test_id) + self.assertEqual(group.name, name) + + +class StaticPartitionService(PartitionService): + """ + Mock PartitionService for testing. + """ + def __init__(self, partitions, **kwargs): + super(StaticPartitionService, self).__init__(**kwargs) + self._partitions = partitions + + @property + def course_partitions(self): + return self._partitions + + +class TestPartitionsService(TestCase): + """ + Test getting a user's group out of a partition + + """ + + def setUp(self): + groups = [Group(0, 'Group 1'), Group(1, 'Group 2')] + self.partition_id = 0 + + # construct the user_service + self.user_tags = dict() + self.user_tags_service = MagicMock() + + def mock_set_tag(_scope, key, value): + """Sets the value of ``key`` to ``value``""" + self.user_tags[key] = value + + def mock_get_tag(_scope, key): + """Gets the value of ``key``""" + if key in self.user_tags: + return self.user_tags[key] + return None + + self.user_tags_service.set_tag = mock_set_tag + self.user_tags_service.get_tag = mock_get_tag + + user_partition = UserPartition(self.partition_id, 'Test Partition', 'for testing purposes', groups) + self.partitions_service = StaticPartitionService( + [user_partition], + user_tags_service=self.user_tags_service, + course_id=Mock(), + track_function=Mock() + ) + + def test_get_user_group_for_partition(self): + # get a group assigned to the user + group1 = self.partitions_service.get_user_group_for_partition(self.partition_id) + + # make sure we get the same group back out if we try a second time + group2 = self.partitions_service.get_user_group_for_partition(self.partition_id) + + self.assertEqual(group1, group2) + + # test that we error if given an invalid partition id + with self.assertRaises(ValueError): + self.partitions_service.get_user_group_for_partition(3) + + def test_user_in_deleted_group(self): + # get a group assigned to the user - should be group 0 or 1 + old_group = self.partitions_service.get_user_group_for_partition(self.partition_id) + self.assertIn(old_group, [0, 1]) + + # Change the group definitions! No more group 0 or 1 + groups = [Group(3, 'Group 3'), Group(4, 'Group 4')] + user_partition = UserPartition(self.partition_id, 'Test Partition', 'for testing purposes', groups) + self.partitions_service = StaticPartitionService( + [user_partition], + user_tags_service=self.user_tags_service, + course_id=Mock(), + track_function=Mock() + ) + + # Now, get a new group using the same call - should be 3 or 4 + new_group = self.partitions_service.get_user_group_for_partition(self.partition_id) + self.assertIn(new_group, [3, 4]) + + # We should get the same group over multiple calls + new_group_2 = self.partitions_service.get_user_group_for_partition(self.partition_id) + self.assertEqual(new_group, new_group_2) + + def test_change_group_name(self): + # Changing the name of the group shouldn't affect anything + # get a group assigned to the user - should be group 0 or 1 + old_group = self.partitions_service.get_user_group_for_partition(self.partition_id) + self.assertIn(old_group, [0, 1]) + + # Change the group names + groups = [Group(0, 'Group 0'), Group(1, 'Group 1')] + user_partition = UserPartition(self.partition_id, 'Test Partition', 'for testing purposes', groups) + self.partitions_service = StaticPartitionService( + [user_partition], + user_tags_service=self.user_tags_service, + course_id=Mock(), + track_function=Mock() + ) + + # Now, get a new group using the same call + new_group = self.partitions_service.get_user_group_for_partition(self.partition_id) + self.assertEqual(old_group, new_group) diff --git a/common/lib/xmodule/xmodule/public/js/split_test_staff.js b/common/lib/xmodule/xmodule/public/js/split_test_staff.js new file mode 100644 index 0000000000..8dd68bced6 --- /dev/null +++ b/common/lib/xmodule/xmodule/public/js/split_test_staff.js @@ -0,0 +1,43 @@ + +/** + * Creates a new selector for managing toggling which child to show + * @constructor + */ + +function ABTestSelector(elem) { + me = this; + me.elem = $(elem); + + select_child = function(group_id) { + // iterate over all the children and hide all the ones that haven't been selected + // and show the one that was selected + me.elem.find('.split-test-child').each(function() { + // force this id to remain a string, even if it looks like something else + child_group_id = $(this).data('group-id').toString(); + if(child_group_id === group_id) { + $(this).show(); + } + else { + $(this).hide(); + } + + }); + } + + // hide all the children + me.elem.find('.split-test-child').hide(); + + select = me.elem.find('.split-test-select'); + cur_group_id = select.val(); + select_child(cur_group_id); + + // bind the change event to the dropdown + select.change(function() { + group_id = $(this).val() + select_child(group_id); + }); + +} + + + diff --git a/common/lib/xmodule/xmodule/split_test_module.py b/common/lib/xmodule/xmodule/split_test_module.py new file mode 100644 index 0000000000..45440a9edf --- /dev/null +++ b/common/lib/xmodule/xmodule/split_test_module.py @@ -0,0 +1,185 @@ +""" +Module for running content split tests +""" + +import logging + +from xmodule.progress import Progress +from xmodule.seq_module import SequenceDescriptor +from xmodule.x_module import XModule, module_attr + +from lxml import etree + +from xblock.core import XBlock +from xblock.fields import Scope, Integer, Dict +from xblock.fragment import Fragment + +log = logging.getLogger('edx.' + __name__) + + +class SplitTestFields(object): + user_partition_id = Integer(help="Which user partition is used for this test", + scope=Scope.content) + + # group_id is an int + # child is a serialized UsageId (aka Location). This child + # location needs to actually match one of the children of this + # Block. (expected invariant that we'll need to test, and handle + # authoring tools that mess this up) + + # TODO: is there a way to add some validation around this, to + # be run on course load or in studio or .... + + group_id_to_child = Dict(help="Which child module students in a particular " + "group_id should see", + scope=Scope.content) + + +@XBlock.needs('user_tags') +@XBlock.needs('partitions') +class SplitTestModule(SplitTestFields, XModule): + """ + Show the user the appropriate child. Uses the ExperimentState + API to figure out which child to show. + + Course staff still get put in an experimental condition, but have the option + to see the other conditions. The only thing that counts toward their + grade/progress is the condition they are actually in. + + Technical notes: + - There is more dark magic in this code than I'd like. The whole varying-children + + grading interaction is a tangle between super and subclasses of descriptors and + modules. +""" + def __init__(self, *args, **kwargs): + + super(SplitTestModule, self).__init__(*args, **kwargs) + + self.child_descriptor = self.get_child_descriptors()[0] + if self.child_descriptor is not None: + self.child = self.system.get_module(self.child_descriptor) + else: + self.child = None + + def get_child_descriptor_by_location(self, location): + """ + Look through the children and look for one with the given location. + Returns the descriptor. + If none match, return None + """ + # NOTE: calling self.get_children() creates a circular reference-- + # it calls get_child_descriptors() internally, but that doesn't work until + # we've picked a choice. Use self.descriptor.get_children() instead. + + for child in self.descriptor.get_children(): + if child.location.url() == location: + return child + + return None + + def get_child_descriptors(self): + """ + For grading--return just the chosen child. + """ + group_id = self.runtime.service(self, 'partitions').get_user_group_for_partition(self.user_partition_id) + + # group_id_to_child comes from json, so it has to have string keys + str_group_id = str(group_id) + if str_group_id in self.group_id_to_child: + child_location = self.group_id_to_child[str_group_id] + child_descriptor = self.get_child_descriptor_by_location(child_location) + else: + # Oops. Config error. + log.debug("configuration error in split test module: invalid group_id %r (not one of %r). Showing error", str_group_id, self.group_id_to_child.keys()) + + if child_descriptor is None: + # Peak confusion is great. Now that we set child_descriptor, + # get_children() should return a list with one element--the + # xmodule for the child + log.debug("configuration error in split test module: no such child") + return [] + + return [child_descriptor] + + def _staff_view(self, context): + """ + Render the staff view for a split test module. + """ + fragment = Fragment() + contents = [] + + for group_id in self.group_id_to_child: + child_location = self.group_id_to_child[group_id] + child_descriptor = self.get_child_descriptor_by_location(child_location) + child = self.system.get_module(child_descriptor) + rendered_child = child.render('student_view', context) + fragment.add_frag_resources(rendered_child) + + contents.append({ + 'group_id': group_id, + 'id': child.id, + 'content': rendered_child.content + }) + + # Use the new template + fragment.add_content(self.system.render_template('split_test_staff_view.html', { + 'items': contents, + })) + frag_js = """ + $(document).ready(function() {{ + ABTestSelector($('.split-test-view')); + }}); + """ + fragment.add_javascript(frag_js) + fragment.add_javascript_url(self.runtime.local_resource_url(self, 'public/js/split_test_staff.js')) + return fragment + + def student_view(self, context): + """ + Render the contents of the chosen condition for students, and all the + conditions for staff. + """ + if self.child is None: + # raise error instead? In fact, could complain on descriptor load... + return Fragment(content=u"