Commit 356b2335 by Andy Armstrong

Add base support for cohorted group configurations

TNL-649
parent 699b3d08
...@@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes, ...@@ -5,6 +5,8 @@ These are notable changes in edx-platform. This is a rolling list of changes,
in roughly chronological order, most recent first. Add your entries at or near in roughly chronological order, most recent first. Add your entries at or near
the top. Include a label indicating the component affected. the top. Include a label indicating the component affected.
Platform: Add base support for cohorted group configurations. TNL-649
Common: Add configurable reset button to units Common: Add configurable reset button to units
Studio: Add support xblock validation messages on Studio unit/container page. TNL-683 Studio: Add support xblock validation messages on Studio unit/container page. TNL-683
......
...@@ -22,7 +22,7 @@ from xmodule.error_module import ErrorDescriptor ...@@ -22,7 +22,7 @@ from xmodule.error_module import ErrorDescriptor
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.contentstore.content import StaticContent from xmodule.contentstore.content import StaticContent
from xmodule.tabs import PDFTextbookTabs from xmodule.tabs import PDFTextbookTabs
from xmodule.partitions.partitions import UserPartition, Group from xmodule.partitions.partitions import UserPartition
from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore import EdxJSONEncoder
from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError
from opaque_keys import InvalidKeyError from opaque_keys import InvalidKeyError
...@@ -1173,7 +1173,7 @@ class GroupConfiguration(object): ...@@ -1173,7 +1173,7 @@ class GroupConfiguration(object):
configuration = json.loads(json_string) configuration = json.loads(json_string)
except ValueError: except ValueError:
raise GroupConfigurationsValidationError(_("invalid JSON")) raise GroupConfigurationsValidationError(_("invalid JSON"))
configuration["version"] = UserPartition.VERSION
return configuration return configuration
def validate(self): def validate(self):
...@@ -1224,14 +1224,7 @@ class GroupConfiguration(object): ...@@ -1224,14 +1224,7 @@ class GroupConfiguration(object):
""" """
Get user partition for saving in course. Get user partition for saving in course.
""" """
groups = [Group(g["id"], g["name"]) for g in self.configuration["groups"]] return UserPartition.from_json(self.configuration)
return UserPartition(
self.configuration["id"],
self.configuration["name"],
self.configuration["description"],
groups
)
@staticmethod @staticmethod
def get_usage_info(course, store): def get_usage_info(course, store):
...@@ -1345,15 +1338,12 @@ def group_configurations_list_handler(request, course_key_string): ...@@ -1345,15 +1338,12 @@ def group_configurations_list_handler(request, course_key_string):
if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'): if 'text/html' in request.META.get('HTTP_ACCEPT', 'text/html'):
group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key) group_configuration_url = reverse_course_url('group_configurations_list_handler', course_key)
course_outline_url = reverse_course_url('course_handler', course_key) course_outline_url = reverse_course_url('course_handler', course_key)
split_test_enabled = SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules
configurations = GroupConfiguration.add_usage_info(course, store) configurations = GroupConfiguration.add_usage_info(course, store)
return render_to_response('group_configurations.html', { return render_to_response('group_configurations.html', {
'context_course': course, 'context_course': course,
'group_configuration_url': group_configuration_url, 'group_configuration_url': group_configuration_url,
'course_outline_url': course_outline_url, 'course_outline_url': course_outline_url,
'configurations': configurations if split_test_enabled else None, 'configurations': configurations if should_show_group_configurations_page(course) else None,
}) })
elif "application/json" in request.META.get('HTTP_ACCEPT'): elif "application/json" in request.META.get('HTTP_ACCEPT'):
if request.method == 'POST': if request.method == 'POST':
...@@ -1432,6 +1422,16 @@ def group_configurations_detail_handler(request, course_key_string, group_config ...@@ -1432,6 +1422,16 @@ def group_configurations_detail_handler(request, course_key_string, group_config
return JsonResponse(status=204) return JsonResponse(status=204)
def should_show_group_configurations_page(course):
"""
Returns true if Studio should show the "Group Configurations" page for the specified course.
"""
return (
SPLIT_TEST_COMPONENT_TYPE in ADVANCED_COMPONENT_TYPES and
SPLIT_TEST_COMPONENT_TYPE in course.advanced_modules
)
def _get_course_creator_status(user): def _get_course_creator_status(user):
""" """
Helper method for returning the course creator status for a particular user, Helper method for returning the course creator status for a particular user,
......
...@@ -15,10 +15,17 @@ from xmodule.modulestore import ModuleStoreEnum ...@@ -15,10 +15,17 @@ from xmodule.modulestore import ModuleStoreEnum
GROUP_CONFIGURATION_JSON = { GROUP_CONFIGURATION_JSON = {
u'name': u'Test name', u'name': u'Test name',
u'scheme': u'random',
u'description': u'Test description', u'description': u'Test description',
u'version': UserPartition.VERSION,
u'groups': [ u'groups': [
{u'name': u'Group A'}, {
{u'name': u'Group B'}, u'name': u'Group A',
u'version': 1,
}, {
u'name': u'Group B',
u'version': 1,
},
], ],
} }
...@@ -229,7 +236,8 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations ...@@ -229,7 +236,8 @@ class GroupConfigurationsListHandlerTestCase(CourseTestCase, GroupConfigurations
expected = { expected = {
u'description': u'Test description', u'description': u'Test description',
u'name': u'Test name', u'name': u'Test name',
u'version': 1, u'scheme': u'random',
u'version': UserPartition.VERSION,
u'groups': [ u'groups': [
{u'name': u'Group A', u'version': 1}, {u'name': u'Group A', u'version': 1},
{u'name': u'Group B', u'version': 1}, {u'name': u'Group B', u'version': 1},
...@@ -279,15 +287,16 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -279,15 +287,16 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
kwargs={'group_configuration_id': cid}, kwargs={'group_configuration_id': cid},
) )
def test_can_create_new_group_configuration_if_it_is_not_exist(self): def test_can_create_new_group_configuration_if_it_does_not_exist(self):
""" """
PUT new group configuration when no configurations exist in the course. PUT new group configuration when no configurations exist in the course.
""" """
expected = { expected = {
u'id': 999, u'id': 999,
u'name': u'Test name', u'name': u'Test name',
u'scheme': u'random',
u'description': u'Test description', u'description': u'Test description',
u'version': 1, u'version': UserPartition.VERSION,
u'groups': [ u'groups': [
{u'id': 0, u'name': u'Group A', u'version': 1}, {u'id': 0, u'name': u'Group A', u'version': 1},
{u'id': 1, u'name': u'Group B', u'version': 1}, {u'id': 1, u'name': u'Group B', u'version': 1},
...@@ -306,12 +315,12 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -306,12 +315,12 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
self.assertEqual(content, expected) self.assertEqual(content, expected)
self.reload_course() self.reload_course()
# Verify that user_partitions in the course contains the new group configuration. # Verify that user_partitions in the course contains the new group configuration.
user_partititons = self.course.user_partitions user_partitions = self.course.user_partitions
self.assertEqual(len(user_partititons), 1) self.assertEqual(len(user_partitions), 1)
self.assertEqual(user_partititons[0].name, u'Test name') self.assertEqual(user_partitions[0].name, u'Test name')
self.assertEqual(len(user_partititons[0].groups), 2) self.assertEqual(len(user_partitions[0].groups), 2)
self.assertEqual(user_partititons[0].groups[0].name, u'Group A') self.assertEqual(user_partitions[0].groups[0].name, u'Group A')
self.assertEqual(user_partititons[0].groups[1].name, u'Group B') self.assertEqual(user_partitions[0].groups[1].name, u'Group B')
def test_can_edit_group_configuration(self): def test_can_edit_group_configuration(self):
""" """
...@@ -323,8 +332,9 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio ...@@ -323,8 +332,9 @@ class GroupConfigurationsDetailHandlerTestCase(CourseTestCase, GroupConfiguratio
expected = { expected = {
u'id': self.ID, u'id': self.ID,
u'name': u'New Test name', u'name': u'New Test name',
u'scheme': u'random',
u'description': u'New Test description', u'description': u'New Test description',
u'version': 1, u'version': UserPartition.VERSION,
u'groups': [ u'groups': [
{u'id': 0, u'name': u'New Group Name', u'version': 1}, {u'id': 0, u'name': u'New Group Name', u'version': 1},
{u'id': 2, u'name': u'Group C', u'version': 1}, {u'id': 2, u'name': u'Group C', u'version': 1},
...@@ -430,8 +440,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): ...@@ -430,8 +440,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
expected = [{ expected = [{
'id': 0, 'id': 0,
'name': 'Name 0', 'name': 'Name 0',
'scheme': 'random',
'description': 'Description 0', 'description': 'Description 0',
'version': 1, 'version': UserPartition.VERSION,
'groups': [ 'groups': [
{'id': 0, 'name': 'Group A', 'version': 1}, {'id': 0, 'name': 'Group A', 'version': 1},
{'id': 1, 'name': 'Group B', 'version': 1}, {'id': 1, 'name': 'Group B', 'version': 1},
...@@ -454,8 +465,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): ...@@ -454,8 +465,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
expected = [{ expected = [{
'id': 0, 'id': 0,
'name': 'Name 0', 'name': 'Name 0',
'scheme': 'random',
'description': 'Description 0', 'description': 'Description 0',
'version': 1, 'version': UserPartition.VERSION,
'groups': [ 'groups': [
{'id': 0, 'name': 'Group A', 'version': 1}, {'id': 0, 'name': 'Group A', 'version': 1},
{'id': 1, 'name': 'Group B', 'version': 1}, {'id': 1, 'name': 'Group B', 'version': 1},
...@@ -469,8 +481,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): ...@@ -469,8 +481,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
}, { }, {
'id': 1, 'id': 1,
'name': 'Name 1', 'name': 'Name 1',
'scheme': 'random',
'description': 'Description 1', 'description': 'Description 1',
'version': 1, 'version': UserPartition.VERSION,
'groups': [ 'groups': [
{'id': 0, 'name': 'Group A', 'version': 1}, {'id': 0, 'name': 'Group A', 'version': 1},
{'id': 1, 'name': 'Group B', 'version': 1}, {'id': 1, 'name': 'Group B', 'version': 1},
...@@ -495,8 +508,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods): ...@@ -495,8 +508,9 @@ class GroupConfigurationsUsageInfoTestCase(CourseTestCase, HelperMethods):
expected = [{ expected = [{
'id': 0, 'id': 0,
'name': 'Name 0', 'name': 'Name 0',
'scheme': 'random',
'description': 'Description 0', 'description': 'Description 0',
'version': 1, 'version': UserPartition.VERSION,
'groups': [ 'groups': [
{'id': 0, 'name': 'Group A', 'version': 1}, {'id': 0, 'name': 'Group A', 'version': 1},
{'id': 1, 'name': 'Group B', 'version': 1}, {'id': 1, 'name': 'Group B', 'version': 1},
......
...@@ -223,8 +223,9 @@ class GetItemTest(ItemTest): ...@@ -223,8 +223,9 @@ class GetItemTest(ItemTest):
GROUP_CONFIGURATION_JSON = { GROUP_CONFIGURATION_JSON = {
u'id': 0, u'id': 0,
u'name': u'first_partition', u'name': u'first_partition',
u'scheme': u'random',
u'description': u'First Partition', u'description': u'First Partition',
u'version': 1, u'version': UserPartition.VERSION,
u'groups': [ u'groups': [
{u'id': 0, u'name': u'New_NAME_A', u'version': 1}, {u'id': 0, u'name': u'New_NAME_A', u'version': 1},
{u'id': 1, u'name': u'New_NAME_B', u'version': 1}, {u'id': 1, u'name': u'New_NAME_B', u'version': 1},
......
...@@ -28,7 +28,6 @@ class CourseMetadata(object): ...@@ -28,7 +28,6 @@ class CourseMetadata(object):
'graded', 'graded',
'hide_from_toc', 'hide_from_toc',
'pdf_textbooks', 'pdf_textbooks',
'user_partitions',
'name', # from xblock 'name', # from xblock
'tags', # from xblock 'tags', # from xblock
'visible_to_staff_only', 'visible_to_staff_only',
......
...@@ -607,7 +607,7 @@ INSTALLED_APPS = ( ...@@ -607,7 +607,7 @@ INSTALLED_APPS = (
'reverification', 'reverification',
# User preferences # User preferences
'user_api', 'openedx.core.djangoapps.user_api',
'django_openid_auth', 'django_openid_auth',
'embargo', 'embargo',
......
...@@ -7,7 +7,7 @@ define([ ...@@ -7,7 +7,7 @@ define([
defaults: function() { defaults: function() {
return { return {
name: '', name: '',
version: null, version: 1,
order: null order: null
}; };
}, },
......
...@@ -9,8 +9,9 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { ...@@ -9,8 +9,9 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) {
defaults: function() { defaults: function() {
return { return {
name: '', name: '',
scheme: 'random',
description: '', description: '',
version: null, version: 2,
groups: new GroupCollection([ groups: new GroupCollection([
{ {
name: gettext('Group A'), name: gettext('Group A'),
...@@ -71,6 +72,7 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) { ...@@ -71,6 +72,7 @@ function(Backbone, _, str, gettext, GroupModel, GroupCollection) {
return { return {
id: this.get('id'), id: this.get('id'),
name: this.get('name'), name: this.get('name'),
scheme: this.get('scheme'),
description: this.get('description'), description: this.get('description'),
version: this.get('version'), version: this.get('version'),
groups: this.get('groups').toJSON() groups: this.get('groups').toJSON()
......
...@@ -99,7 +99,8 @@ define([ ...@@ -99,7 +99,8 @@ define([
'id': 10, 'id': 10,
'name': 'My Group Configuration', 'name': 'My Group Configuration',
'description': 'Some description', 'description': 'Some description',
'version': 1, 'version': 2,
'scheme': 'random',
'groups': [ 'groups': [
{ {
'version': 1, 'version': 1,
...@@ -114,9 +115,10 @@ define([ ...@@ -114,9 +115,10 @@ define([
'id': 10, 'id': 10,
'name': 'My Group Configuration', 'name': 'My Group Configuration',
'description': 'Some description', 'description': 'Some description',
'scheme': 'random',
'showGroups': false, 'showGroups': false,
'editing': false, 'editing': false,
'version': 1, 'version': 2,
'groups': [ 'groups': [
{ {
'version': 1, 'version': 1,
......
...@@ -55,7 +55,7 @@ ...@@ -55,7 +55,7 @@
</div> </div>
% else: % else:
<div class="ui-loading"> <div class="ui-loading">
<p><span class="spin"><i class="icon-refresh"></i></span> <span class="copy">${_("Loading...")}</span></p> <p><span class="spin"><i class="icon-refresh"></i></span> <span class="copy">${_("Loading")}</span></p>
</div> </div>
% endif % endif
</article> </article>
......
...@@ -7,6 +7,7 @@ ...@@ -7,6 +7,7 @@
<%! <%!
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from contentstore import utils from contentstore import utils
from contentstore.views.course import should_show_group_configurations_page
import urllib import urllib
%> %>
...@@ -312,10 +313,10 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}'; ...@@ -312,10 +313,10 @@ CMS.URL.UPLOAD_ASSET = '${upload_asset_url}';
<ul> <ul>
<li class="nav-item"><a href="${grading_config_url}">${_("Grading")}</a></li> <li class="nav-item"><a href="${grading_config_url}">${_("Grading")}</a></li>
<li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li> <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
% if should_show_group_configurations_page(context_course):
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
<li class="nav-item"><a href="${advanced_config_url}">${_("Advanced Settings")}</a></li> <li class="nav-item"><a href="${advanced_config_url}">${_("Advanced Settings")}</a></li>
% if "split_test" in context_course.advanced_modules:
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
</ul> </ul>
</nav> </nav>
% endif % endif
......
...@@ -4,6 +4,7 @@ ...@@ -4,6 +4,7 @@
<%! <%!
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from contentstore import utils from contentstore import utils
from contentstore.views.course import should_show_group_configurations_page
from django.utils.html import escapejs from django.utils.html import escapejs
%> %>
<%block name="title">${_("Advanced Settings")}</%block> <%block name="title">${_("Advanced Settings")}</%block>
...@@ -91,9 +92,9 @@ ...@@ -91,9 +92,9 @@
<li class="nav-item"><a href="${details_url}">${_("Details &amp; Schedule")}</a></li> <li class="nav-item"><a href="${details_url}">${_("Details &amp; Schedule")}</a></li>
<li class="nav-item"><a href="${grading_url}">${_("Grading")}</a></li> <li class="nav-item"><a href="${grading_url}">${_("Grading")}</a></li>
<li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li> <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
% if "split_test" in context_course.advanced_modules: % if should_show_group_configurations_page(context_course):
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li> <li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif % endif
</ul> </ul>
</nav> </nav>
% endif % endif
......
...@@ -6,6 +6,7 @@ ...@@ -6,6 +6,7 @@
<%namespace name='static' file='static_content.html'/> <%namespace name='static' file='static_content.html'/>
<%! <%!
from contentstore import utils from contentstore import utils
from contentstore.views.course import should_show_group_configurations_page
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
%> %>
...@@ -134,10 +135,10 @@ ...@@ -134,10 +135,10 @@
<ul> <ul>
<li class="nav-item"><a href="${detailed_settings_url}">${_("Details &amp; Schedule")}</a></li> <li class="nav-item"><a href="${detailed_settings_url}">${_("Details &amp; Schedule")}</a></li>
<li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li> <li class="nav-item"><a href="${course_team_url}">${_("Course Team")}</a></li>
% if should_show_group_configurations_page(context_course):
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
<li class="nav-item"><a href="${advanced_settings_url}">${_("Advanced Settings")}</a></li> <li class="nav-item"><a href="${advanced_settings_url}">${_("Advanced Settings")}</a></li>
% if "split_test" in context_course.advanced_modules:
<li class="nav-item"><a href="${utils.reverse_course_url('group_configurations_list_handler', context_course.id)}">${_("Group Configurations")}</a></li>
% endif
</ul> </ul>
</nav> </nav>
% endif % endif
......
...@@ -3,6 +3,7 @@ ...@@ -3,6 +3,7 @@
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.utils.translation import ugettext as _ from django.utils.translation import ugettext as _
from contentstore.context_processors import doc_url from contentstore.context_processors import doc_url
from contentstore.views.course import should_show_group_configurations_page
%> %>
<%page args="online_help_token"/> <%page args="online_help_token"/>
...@@ -81,14 +82,14 @@ ...@@ -81,14 +82,14 @@
<li class="nav-item nav-course-settings-team"> <li class="nav-item nav-course-settings-team">
<a href="${course_team_url}">${_("Course Team")}</a> <a href="${course_team_url}">${_("Course Team")}</a>
</li> </li>
% if should_show_group_configurations_page(context_course):
<li class="nav-item nav-course-settings-group-configurations">
<a href="${reverse('contentstore.views.group_configurations_list_handler', kwargs={'course_key_string': unicode(course_key)})}">${_("Group Configurations")}</a>
</li>
% endif
<li class="nav-item nav-course-settings-advanced"> <li class="nav-item nav-course-settings-advanced">
<a href="${advanced_settings_url}">${_("Advanced Settings")}</a> <a href="${advanced_settings_url}">${_("Advanced Settings")}</a>
</li> </li>
% if "split_test" in context_course.advanced_modules:
<li class="nav-item nav-course-settings-group-configurations">
<a href="${reverse('contentstore.views.group_configurations_list_handler', kwargs={'course_key_string': unicode(course_key)})}">${_("Group Configurations")}</a>
</li>
% endif
</ul> </ul>
</div> </div>
</div> </div>
......
...@@ -39,7 +39,7 @@ urlpatterns = patterns('', # nopep8 ...@@ -39,7 +39,7 @@ urlpatterns = patterns('', # nopep8
url(r'^xmodule/', include('pipeline_js.urls')), url(r'^xmodule/', include('pipeline_js.urls')),
url(r'^heartbeat$', include('heartbeat.urls')), url(r'^heartbeat$', include('heartbeat.urls')),
url(r'^user_api/', include('user_api.urls')), url(r'^user_api/', include('openedx.core.djangoapps.user_api.urls')),
url(r'^lang_pref/', include('lang_pref.urls')), url(r'^lang_pref/', include('lang_pref.urls')),
) )
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
Middleware for Language Preferences Middleware for Language Preferences
""" """
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from lang_pref import LANGUAGE_KEY from lang_pref import LANGUAGE_KEY
......
...@@ -3,7 +3,7 @@ from django.test.client import RequestFactory ...@@ -3,7 +3,7 @@ from django.test.client import RequestFactory
from django.contrib.sessions.middleware import SessionMiddleware from django.contrib.sessions.middleware import SessionMiddleware
from lang_pref.middleware import LanguagePreferenceMiddleware from lang_pref.middleware import LanguagePreferenceMiddleware
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from lang_pref import LANGUAGE_KEY from lang_pref import LANGUAGE_KEY
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
......
...@@ -4,7 +4,7 @@ Tests for the language setting view ...@@ -4,7 +4,7 @@ Tests for the language setting view
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.test import TestCase from django.test import TestCase
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from lang_pref import LANGUAGE_KEY from lang_pref import LANGUAGE_KEY
......
...@@ -4,7 +4,7 @@ Views for accessing language preferences ...@@ -4,7 +4,7 @@ Views for accessing language preferences
from django.contrib.auth.decorators import login_required from django.contrib.auth.decorators import login_required
from django.http import HttpResponse, HttpResponseBadRequest from django.http import HttpResponse, HttpResponseBadRequest
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from lang_pref import LANGUAGE_KEY from lang_pref import LANGUAGE_KEY
......
...@@ -12,7 +12,7 @@ from django.test import TestCase, TransactionTestCase ...@@ -12,7 +12,7 @@ from django.test import TestCase, TransactionTestCase
import mock import mock
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from lang_pref import LANGUAGE_KEY from lang_pref import LANGUAGE_KEY
from edxmako.tests import mako_middleware_process_request from edxmako.tests import mako_middleware_process_request
......
...@@ -83,7 +83,7 @@ from external_auth.login_and_register import ( ...@@ -83,7 +83,7 @@ from external_auth.login_and_register import (
from bulk_email.models import Optout, CourseAuthorization from bulk_email.models import Optout, CourseAuthorization
import shoppingcart import shoppingcart
from shoppingcart.models import DonationConfiguration from shoppingcart.models import DonationConfiguration
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from lang_pref import LANGUAGE_KEY from lang_pref import LANGUAGE_KEY
import track.views import track.views
......
...@@ -142,8 +142,8 @@ class InheritanceMixin(XBlockMixin): ...@@ -142,8 +142,8 @@ class InheritanceMixin(XBlockMixin):
# This is should be scoped to content, but since it's defined in the policy # This is should be scoped to content, but since it's defined in the policy
# file, it is currently scoped to settings. # file, it is currently scoped to settings.
user_partitions = UserPartitionList( user_partitions = UserPartitionList(
display_name=_("Experiment Group Configurations"), display_name=_("Group Configurations"),
help=_("Enter the configurations that govern how students are grouped for content experiments."), help=_("Enter the configurations that govern how students are grouped together."),
default=[], default=[],
scope=Scope.settings scope=Scope.settings
) )
......
...@@ -31,6 +31,7 @@ from xmodule.modulestore.xml_exporter import export_to_xml ...@@ -31,6 +31,7 @@ from xmodule.modulestore.xml_exporter import export_to_xml
from xmodule.modulestore.split_mongo.split_draft import DraftVersioningModuleStore from xmodule.modulestore.split_mongo.split_draft import DraftVersioningModuleStore
from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST
from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.modulestore.inheritance import InheritanceMixin
from xmodule.partitions.tests.test_partitions import PartitionTestCase
from xmodule.x_module import XModuleMixin from xmodule.x_module import XModuleMixin
from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore.xml import XMLModuleStore
...@@ -291,7 +292,7 @@ COURSE_DATA_NAMES = ( ...@@ -291,7 +292,7 @@ COURSE_DATA_NAMES = (
@ddt.ddt @ddt.ddt
@attr('mongo') @attr('mongo')
class CrossStoreXMLRoundtrip(CourseComparisonTest): class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase):
""" """
This class exists to test XML import and export between different modulestore This class exists to test XML import and export between different modulestore
classes. classes.
......
"""Defines ``Group`` and ``UserPartition`` models for partitioning""" """Defines ``Group`` and ``UserPartition`` models for partitioning"""
from collections import namedtuple from collections import namedtuple
from stevedore.extension import ExtensionManager
# We use ``id`` in this file as the IDs of our Groups and UserPartitions, # We use ``id`` in this file as the IDs of our Groups and UserPartitions,
# which Pylint disapproves of. # which Pylint disapproves of.
# pylint: disable=invalid-name, redefined-builtin # pylint: disable=invalid-name, redefined-builtin
class UserPartitionError(Exception):
"""
An error was found regarding user partitions.
"""
pass
class Group(namedtuple("Group", "id name")): class Group(namedtuple("Group", "id name")):
""" """
An id and name for a group of students. The id should be unique An id and name for a group of students. The id should be unique
...@@ -45,7 +55,7 @@ class Group(namedtuple("Group", "id name")): ...@@ -45,7 +55,7 @@ class Group(namedtuple("Group", "id name")):
if isinstance(value, Group): if isinstance(value, Group):
return value return value
for key in ('id', 'name', 'version'): for key in ("id", "name", "version"):
if key not in value: if key not in value:
raise TypeError("Group dict {0} missing value key '{1}'".format( raise TypeError("Group dict {0} missing value key '{1}'".format(
value, key)) value, key))
...@@ -57,21 +67,50 @@ class Group(namedtuple("Group", "id name")): ...@@ -57,21 +67,50 @@ class Group(namedtuple("Group", "id name")):
return Group(value["id"], value["name"]) return Group(value["id"], value["name"])
class UserPartition(namedtuple("UserPartition", "id name description groups")): # The Stevedore extension point namespace for user partition scheme plugins.
USER_PARTITION_SCHEME_NAMESPACE = 'openedx.user_partition_scheme'
class UserPartition(namedtuple("UserPartition", "id name description groups scheme")):
""" """
A named way to partition users into groups, primarily intended for running 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 experiments. It is expected that each user will be in at most one group in a
partition. partition.
A Partition has an id, name, description, and a list of groups. A Partition has an id, name, scheme, description, and a list of groups.
The id is intended to be unique within the context where these are used. (e.g. for 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) partitions of users within a course, the ids should be unique per-course).
The scheme is used to assign users into groups.
""" """
VERSION = 1 VERSION = 2
# The collection of user partition scheme extensions.
scheme_extensions = None
def __new__(cls, id, name, description, groups): # The default scheme to be used when upgrading version 1 partitions.
VERSION_1_SCHEME = "random"
def __new__(cls, id, name, description, groups, scheme=None, scheme_id=VERSION_1_SCHEME):
# pylint: disable=super-on-old-class # pylint: disable=super-on-old-class
return super(UserPartition, cls).__new__(cls, int(id), name, description, groups) if not scheme:
scheme = UserPartition.get_scheme(scheme_id)
return super(UserPartition, cls).__new__(cls, int(id), name, description, groups, scheme)
@staticmethod
def get_scheme(name):
"""
Returns the user partition scheme with the given name.
"""
# Note: we're creating the extension manager lazily to ensure that the Python path
# has been correctly set up. Trying to create this statically will fail, unfortunately.
if not UserPartition.scheme_extensions:
UserPartition.scheme_extensions = ExtensionManager(namespace=USER_PARTITION_SCHEME_NAMESPACE)
try:
scheme = UserPartition.scheme_extensions[name].plugin
except KeyError:
raise UserPartitionError("Unrecognized scheme {0}".format(name))
scheme.name = name
return scheme
def to_json(self): def to_json(self):
""" """
...@@ -84,6 +123,7 @@ class UserPartition(namedtuple("UserPartition", "id name description groups")): ...@@ -84,6 +123,7 @@ class UserPartition(namedtuple("UserPartition", "id name description groups")):
return { return {
"id": self.id, "id": self.id,
"name": self.name, "name": self.name,
"scheme": self.scheme.name,
"description": self.description, "description": self.description,
"groups": [g.to_json() for g in self.groups], "groups": [g.to_json() for g in self.groups],
"version": UserPartition.VERSION "version": UserPartition.VERSION
...@@ -102,20 +142,38 @@ class UserPartition(namedtuple("UserPartition", "id name description groups")): ...@@ -102,20 +142,38 @@ class UserPartition(namedtuple("UserPartition", "id name description groups")):
if isinstance(value, UserPartition): if isinstance(value, UserPartition):
return value return value
for key in ('id', 'name', 'description', 'version', 'groups'): for key in ("id", "name", "description", "version", "groups"):
if key not in value: if key not in value:
raise TypeError("UserPartition dict {0} missing value key '{1}'" raise TypeError("UserPartition dict {0} missing value key '{1}'".format(value, key))
.format(value, key))
if value["version"] == 1:
if value["version"] != UserPartition.VERSION: # If no scheme was provided, set it to the default ('random')
raise TypeError("UserPartition dict {0} has unexpected version" scheme_id = UserPartition.VERSION_1_SCHEME
.format(value)) elif value["version"] == UserPartition.VERSION:
if not "scheme" in value:
raise TypeError("UserPartition dict {0} missing value key 'scheme'".format(value))
scheme_id = value["scheme"]
else:
raise TypeError("UserPartition dict {0} has unexpected version".format(value))
groups = [Group.from_json(g) for g in value["groups"]] groups = [Group.from_json(g) for g in value["groups"]]
scheme = UserPartition.get_scheme(scheme_id)
if not scheme:
raise TypeError("UserPartition dict {0} has unrecognized scheme {1}".format(value, scheme_id))
return UserPartition( return UserPartition(
value["id"], value["id"],
value["name"], value["name"],
value["description"], value["description"],
groups groups,
scheme,
) )
def get_group(self, group_id):
"""
Returns the group with the specified id.
"""
for group in self.groups: # pylint: disable=no-member
if group.id == group_id:
return group
return None
...@@ -3,7 +3,6 @@ This is a service-like API that assigns tracks which groups users are in for var ...@@ -3,7 +3,6 @@ This is a service-like API that assigns tracks which groups users are in for var
user partitions. It uses the user_service key/value store provided by the LMS runtime to user partitions. It uses the user_service key/value store provided by the LMS runtime to
persist the assignments. persist the assignments.
""" """
import random
from abc import ABCMeta, abstractproperty from abc import ABCMeta, abstractproperty
...@@ -22,13 +21,11 @@ class PartitionService(object): ...@@ -22,13 +21,11 @@ class PartitionService(object):
""" """
raise NotImplementedError('Subclasses must implement course_partition') raise NotImplementedError('Subclasses must implement course_partition')
def __init__(self, user_tags_service, course_id, track_function): def __init__(self, runtime, track_function):
self.random = random.Random() self.runtime = runtime
self._user_tags_service = user_tags_service
self._course_id = course_id
self._track_function = track_function self._track_function = track_function
def get_user_group_for_partition(self, user_partition_id): def get_user_group_id_for_partition(self, user_partition_id):
""" """
If the user is already assigned to a group in user_partition_id, return the If the user is already assigned to a group in user_partition_id, return the
group_id. group_id.
...@@ -53,17 +50,15 @@ class PartitionService(object): ...@@ -53,17 +50,15 @@ class PartitionService(object):
if user_partition is None: if user_partition is None:
raise ValueError( raise ValueError(
"Configuration problem! No user_partition with id {0} " "Configuration problem! No user_partition with id {0} "
"in course {1}".format(user_partition_id, self._course_id) "in course {1}".format(user_partition_id, self.runtime.course_id)
) )
group_id = self._get_group(user_partition) group = self._get_group(user_partition)
return group.id if group else None
return group_id
def _get_user_partition(self, user_partition_id): def _get_user_partition(self, user_partition_id):
""" """
Look for a user partition with a matching id in Look for a user partition with a matching id in the course's partitions.
in the course's partitions.
Returns: Returns:
A UserPartition, or None if not found. A UserPartition, or None if not found.
...@@ -74,65 +69,13 @@ class PartitionService(object): ...@@ -74,65 +69,13 @@ class PartitionService(object):
return None 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): def _get_group(self, user_partition):
""" """
Return the group of the current user in user_partition. If they don't already have Returns the group from the specified user partition to which the user is assigned.
one assigned, pick one and save it. Uses the runtime's user_service service to look up If the user has not yet been assigned, a group will be chosen for them based upon
and persist the info. the partition's scheme.
""" """
key = self._key_for_partition(user_partition) user = self.runtime.get_real_user(self.runtime.anonymous_student_id)
scope = self._user_tags_service.COURSE_SCOPE return user_partition.scheme.get_group_for_user(
self.runtime.course_id, user, user_partition, track_function=self._track_function
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('xmodule.partitions.assigned_user_to_partition', event_info)
return group.id
...@@ -188,7 +188,7 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule): ...@@ -188,7 +188,7 @@ class SplitTestModule(SplitTestFields, XModule, StudioEditableModule):
partitions_service = self.runtime.service(self, 'partitions') partitions_service = self.runtime.service(self, 'partitions')
if not partitions_service: if not partitions_service:
return None return None
return partitions_service.get_user_group_for_partition(self.user_partition_id) return partitions_service.get_user_group_id_for_partition(self.user_partition_id)
@property @property
def is_configured(self): def is_configured(self):
......
...@@ -79,13 +79,15 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')): ...@@ -79,13 +79,15 @@ def get_test_system(course_id=SlashSeparatedCourseKey('org', 'course', 'run')):
where `my_render_func` is a function of the form my_render_func(template, context). where `my_render_func` is a function of the form my_render_func(template, context).
""" """
user = Mock(is_staff=False)
return TestModuleSystem( return TestModuleSystem(
static_url='/static', static_url='/static',
track_function=Mock(), track_function=Mock(),
get_module=Mock(), get_module=Mock(),
render_template=mock_render_template, render_template=mock_render_template,
replace_urls=str, replace_urls=str,
user=Mock(is_staff=False), user=user,
get_real_user=lambda(__): user,
filestore=Mock(), filestore=Mock(),
debug=True, debug=True,
hostname="edx.org", hostname="edx.org",
......
...@@ -6,6 +6,7 @@ import lxml ...@@ -6,6 +6,7 @@ import lxml
from mock import Mock, patch from mock import Mock, patch
from fs.memoryfs import MemoryFS from fs.memoryfs import MemoryFS
from xmodule.partitions.tests.test_partitions import StaticPartitionService, PartitionTestCase, MockUserPartitionScheme
from xmodule.tests.xml import factories as xml from xmodule.tests.xml import factories as xml
from xmodule.tests.xml import XModuleXmlImportTest from xmodule.tests.xml import XModuleXmlImportTest
from xmodule.tests import get_test_system from xmodule.tests import get_test_system
...@@ -13,7 +14,6 @@ from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW ...@@ -13,7 +14,6 @@ from xmodule.x_module import AUTHOR_VIEW, STUDENT_VIEW
from xmodule.validation import StudioValidationMessage from xmodule.validation import StudioValidationMessage
from xmodule.split_test_module import SplitTestDescriptor, SplitTestFields from xmodule.split_test_module import SplitTestDescriptor, SplitTestFields
from xmodule.partitions.partitions import Group, UserPartition from xmodule.partitions.partitions import Group, UserPartition
from xmodule.partitions.test_partitions import StaticPartitionService, MemoryUserTagsService
class SplitTestModuleFactory(xml.XmlImportFactory): class SplitTestModuleFactory(xml.XmlImportFactory):
...@@ -23,11 +23,12 @@ class SplitTestModuleFactory(xml.XmlImportFactory): ...@@ -23,11 +23,12 @@ class SplitTestModuleFactory(xml.XmlImportFactory):
tag = 'split_test' tag = 'split_test'
class SplitTestModuleTest(XModuleXmlImportTest): class SplitTestModuleTest(XModuleXmlImportTest, PartitionTestCase):
""" """
Base class for all split_module tests. Base class for all split_module tests.
""" """
def setUp(self): def setUp(self):
super(SplitTestModuleTest, self).setUp()
self.course_id = 'test_org/test_course_number/test_run' self.course_id = 'test_org/test_course_number/test_run'
# construct module # construct module
course = xml.CourseFactory.build() course = xml.CourseFactory.build()
...@@ -57,16 +58,16 @@ class SplitTestModuleTest(XModuleXmlImportTest): ...@@ -57,16 +58,16 @@ class SplitTestModuleTest(XModuleXmlImportTest):
self.module_system.descriptor_system = self.course.runtime self.module_system.descriptor_system = self.course.runtime
self.course.runtime.export_fs = MemoryFS() self.course.runtime.export_fs = MemoryFS()
self.tags_service = MemoryUserTagsService()
self.module_system._services['user_tags'] = self.tags_service # pylint: disable=protected-access
self.partitions_service = StaticPartitionService( self.partitions_service = StaticPartitionService(
[ [
UserPartition(0, 'first_partition', 'First Partition', [Group("0", 'alpha'), Group("1", 'beta')]), self.user_partition,
UserPartition(1, 'second_partition', 'Second Partition', [Group("0", 'abel'), Group("1", 'baker'), Group("2", 'charlie')]) UserPartition(
1, 'second_partition', 'Second Partition',
[Group("0", 'abel'), Group("1", 'baker'), Group("2", 'charlie')],
MockUserPartitionScheme()
)
], ],
user_tags_service=self.tags_service, runtime=self.module_system,
course_id=self.course.id,
track_function=Mock(name='track_function'), track_function=Mock(name='track_function'),
) )
self.module_system._services['partitions'] = self.partitions_service # pylint: disable=protected-access self.module_system._services['partitions'] = self.partitions_service # pylint: disable=protected-access
...@@ -81,50 +82,28 @@ class SplitTestModuleLMSTest(SplitTestModuleTest): ...@@ -81,50 +82,28 @@ class SplitTestModuleLMSTest(SplitTestModuleTest):
Test the split test module Test the split test module
""" """
@ddt.data(('0', 'split_test_cond0'), ('1', 'split_test_cond1')) @ddt.data((0, 'split_test_cond0'), (1, 'split_test_cond1'))
@ddt.unpack @ddt.unpack
def test_child(self, user_tag, child_url_name): def test_child(self, user_tag, child_url_name):
self.tags_service.set_tag( self.user_partition.scheme.current_group = self.user_partition.groups[user_tag] # pylint: disable=no-member
self.tags_service.COURSE_SCOPE,
'xblock.partition_service.partition_0',
user_tag
)
self.assertEquals(self.split_test_module.child_descriptor.url_name, child_url_name) self.assertEquals(self.split_test_module.child_descriptor.url_name, child_url_name)
@ddt.data(('0',), ('1',)) @ddt.data((0, 'HTML FOR GROUP 0'), (1, 'HTML FOR GROUP 1'))
@ddt.unpack
def test_child_old_tag_value(self, _user_tag):
# If user_tag has a stale value, we should still get back a valid child url
self.tags_service.set_tag(
self.tags_service.COURSE_SCOPE,
'xblock.partition_service.partition_0',
'2'
)
self.assertIn(self.split_test_module.child_descriptor.url_name, ['split_test_cond0', 'split_test_cond1'])
@ddt.data(('0', 'HTML FOR GROUP 0'), ('1', 'HTML FOR GROUP 1'))
@ddt.unpack @ddt.unpack
def test_get_html(self, user_tag, child_content): def test_get_html(self, user_tag, child_content):
self.tags_service.set_tag( self.user_partition.scheme.current_group = self.user_partition.groups[user_tag] # pylint: disable=no-member
self.tags_service.COURSE_SCOPE,
'xblock.partition_service.partition_0',
user_tag
)
self.assertIn( self.assertIn(
child_content, child_content,
self.module_system.render(self.split_test_module, STUDENT_VIEW).content self.module_system.render(self.split_test_module, STUDENT_VIEW).content
) )
@ddt.data(('0',), ('1',)) @ddt.data((0,), (1,))
@ddt.unpack @ddt.unpack
def test_child_missing_tag_value(self, _user_tag): def test_child_missing_tag_value(self, _user_tag):
# If user_tag has a missing value, we should still get back a valid child url # If user_tag has a missing value, we should still get back a valid child url
self.assertIn(self.split_test_module.child_descriptor.url_name, ['split_test_cond0', 'split_test_cond1']) self.assertIn(self.split_test_module.child_descriptor.url_name, ['split_test_cond0', 'split_test_cond1'])
@ddt.data(('100',), ('200',), ('300',), ('400',), ('500',), ('600',), ('700',), ('800',), ('900',), ('1000',)) @ddt.data((100,), (200,), (300,), (400,), (500,), (600,), (700,), (800,), (900,), (1000,))
@ddt.unpack @ddt.unpack
def test_child_persist_new_tag_value_when_tag_missing(self, _user_tag): def test_child_persist_new_tag_value_when_tag_missing(self, _user_tag):
# If a user_tag has a missing value, a group should be saved/persisted for that user. # If a user_tag has a missing value, a group should be saved/persisted for that user.
......
[run] [run]
data_file = reports/bok_choy/.coverage data_file = reports/bok_choy/.coverage
source = lms, cms, common/djangoapps, common/lib source = lms, cms, common/djangoapps, common/lib
omit = lms/envs/*, cms/envs/*, common/djangoapps/terrain/*, common/djangoapps/*/migrations/*, */test*, */management/*, */urls*, */wsgi* omit = lms/envs/*, cms/envs/*, common/djangoapps/terrain/*, common/djangoapps/*/migrations/*, openedx/core/djangoapps/*/migrations/*, */test*, */management/*, */urls*, */wsgi*
parallel = True parallel = True
[report] [report]
......
...@@ -2,7 +2,7 @@ ...@@ -2,7 +2,7 @@
[run] [run]
data_file = reports/lms/.coverage data_file = reports/lms/.coverage
source = lms,common/djangoapps source = lms,common/djangoapps
omit = lms/envs/*, common/djangoapps/terrain/*, common/djangoapps/*/migrations/* omit = lms/envs/*, common/djangoapps/terrain/*, common/djangoapps/*/migrations/*, openedx/core/djangoapps/*/migrations/*
[report] [report]
ignore_errors = True ignore_errors = True
......
...@@ -12,7 +12,7 @@ from student.tests.factories import UserFactory, CourseEnrollmentFactory ...@@ -12,7 +12,7 @@ from student.tests.factories import UserFactory, CourseEnrollmentFactory
from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory from xmodule.modulestore.tests.factories import ItemFactory, CourseFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.partitions.partitions import Group, UserPartition from xmodule.partitions.partitions import Group, UserPartition
from user_api.tests.factories import UserCourseTagFactory from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory
@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE)
......
...@@ -27,7 +27,7 @@ from student.models import anonymous_id_for_user ...@@ -27,7 +27,7 @@ from student.models import anonymous_id_for_user
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
from xmodule.partitions.partitions import Group, UserPartition from xmodule.partitions.partitions import Group, UserPartition
from user_api.tests.factories import UserCourseTagFactory from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory
@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE)
......
...@@ -62,7 +62,7 @@ import instructor_analytics.basic ...@@ -62,7 +62,7 @@ import instructor_analytics.basic
import instructor_analytics.distributions import instructor_analytics.distributions
import instructor_analytics.csvs import instructor_analytics.csvs
import csv import csv
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from instructor.views import INVOICE_KEY from instructor.views import INVOICE_KEY
from submissions import api as sub_api # installed from the edx-submissions repository from submissions import api as sub_api # installed from the edx-submissions repository
......
...@@ -17,7 +17,7 @@ from django.core.urlresolvers import reverse ...@@ -17,7 +17,7 @@ from django.core.urlresolvers import reverse
from capa.tests.response_xml_factory import (CodeResponseXMLFactory, from capa.tests.response_xml_factory import (CodeResponseXMLFactory,
CustomResponseXMLFactory) CustomResponseXMLFactory)
from user_api.tests.factories import UserCourseTagFactory from openedx.core.djangoapps.user_api.tests.factories import UserCourseTagFactory
from xmodule.modulestore.tests.factories import ItemFactory from xmodule.modulestore.tests.factories import ItemFactory
from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore import ModuleStoreEnum
from xmodule.partitions.partitions import Group, UserPartition from xmodule.partitions.partitions import Group, UserPartition
......
from django.contrib.auth.models import User from django.contrib.auth.models import User
from lettuce import step, world from lettuce import step, world
from notification_prefs import NOTIFICATION_PREF_KEY from notification_prefs import NOTIFICATION_PREF_KEY
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
USERNAME = "robot" USERNAME = "robot"
......
...@@ -12,7 +12,7 @@ from notification_prefs import NOTIFICATION_PREF_KEY ...@@ -12,7 +12,7 @@ from notification_prefs import NOTIFICATION_PREF_KEY
from notification_prefs.views import ajax_enable, ajax_disable, ajax_status, set_subscription, UsernameCipher from notification_prefs.views import ajax_enable, ajax_disable, ajax_status, set_subscription, UsernameCipher
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from edxmako.tests import mako_middleware_process_request from edxmako.tests import mako_middleware_process_request
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from util.testing import UrlResetMixin from util.testing import UrlResetMixin
......
...@@ -12,7 +12,7 @@ from django.views.decorators.http import require_GET, require_POST ...@@ -12,7 +12,7 @@ from django.views.decorators.http import require_GET, require_POST
from edxmako.shortcuts import render_to_response from edxmako.shortcuts import render_to_response
from notification_prefs import NOTIFICATION_PREF_KEY from notification_prefs import NOTIFICATION_PREF_KEY
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
class UsernameDecryptionException(Exception): class UsernameDecryptionException(Exception):
......
...@@ -13,8 +13,8 @@ from notifier_api.views import NotifierUsersViewSet ...@@ -13,8 +13,8 @@ from notifier_api.views import NotifierUsersViewSet
from opaque_keys.edx.locator import CourseLocator from opaque_keys.edx.locator import CourseLocator
from student.models import CourseEnrollment from student.models import CourseEnrollment
from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from user_api.tests.factories import UserPreferenceFactory from openedx.core.djangoapps.user_api.tests.factories import UserPreferenceFactory
from util.testing import UrlResetMixin from util.testing import UrlResetMixin
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
......
...@@ -3,7 +3,7 @@ from rest_framework.viewsets import ReadOnlyModelViewSet ...@@ -3,7 +3,7 @@ from rest_framework.viewsets import ReadOnlyModelViewSet
from notification_prefs import NOTIFICATION_PREF_KEY from notification_prefs import NOTIFICATION_PREF_KEY
from notifier_api.serializers import NotifierUserSerializer from notifier_api.serializers import NotifierUserSerializer
from user_api.views import ApiKeyHeaderPermission from openedx.core.djangoapps.user_api.views import ApiKeyHeaderPermission
class NotifierUsersViewSet(ReadOnlyModelViewSet): class NotifierUsersViewSet(ReadOnlyModelViewSet):
......
...@@ -6,7 +6,7 @@ from django.core.cache import cache ...@@ -6,7 +6,7 @@ from django.core.cache import cache
from courseware.access import has_access from courseware.access import has_access
from student.models import anonymous_id_for_user from student.models import anonymous_id_for_user
from student.models import UserProfile from student.models import UserProfile
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from lang_pref import LANGUAGE_KEY from lang_pref import LANGUAGE_KEY
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.course_module import CourseDescriptor from xmodule.course_module import CourseDescriptor
......
...@@ -9,7 +9,7 @@ from student.models import anonymous_id_for_user ...@@ -9,7 +9,7 @@ from student.models import anonymous_id_for_user
from student.models import UserProfile from student.models import UserProfile
from student.roles import CourseStaffRole, CourseInstructorRole from student.roles import CourseStaffRole, CourseInstructorRole
from student.tests.factories import UserFactory, UserProfileFactory from student.tests.factories import UserFactory, UserProfileFactory
from user_api.models import UserPreference from openedx.core.djangoapps.user_api.models import UserPreference
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
# Will also run default tests for IDTokens and UserInfo # Will also run default tests for IDTokens and UserInfo
......
...@@ -16,9 +16,8 @@ from django.test.utils import override_settings ...@@ -16,9 +16,8 @@ from django.test.utils import override_settings
from util.testing import UrlResetMixin from util.testing import UrlResetMixin
from third_party_auth.tests.testutil import simulate_running_pipeline from third_party_auth.tests.testutil import simulate_running_pipeline
from user_api.api import account as account_api from openedx.core.djangoapps.user_api.api import account as account_api
from user_api.api import profile as profile_api from openedx.core.djangoapps.user_api.api import profile as profile_api
from util.bad_request_rate_limiter import BadRequestRateLimiter
from xmodule.modulestore.tests.django_utils import ( from xmodule.modulestore.tests.django_utils import (
ModuleStoreTestCase, mixed_store_config ModuleStoreTestCase, mixed_store_config
) )
......
...@@ -24,8 +24,8 @@ from student.views import ( ...@@ -24,8 +24,8 @@ from student.views import (
register_user as old_register_view register_user as old_register_view
) )
from user_api.api import account as account_api from openedx.core.djangoapps.user_api.api import account as account_api
from user_api.api import profile as profile_api from openedx.core.djangoapps.user_api.api import profile as profile_api
from util.bad_request_rate_limiter import BadRequestRateLimiter from util.bad_request_rate_limiter import BadRequestRateLimiter
from student_account.helpers import auth_pipeline_urls from student_account.helpers import auth_pipeline_urls
......
...@@ -11,8 +11,8 @@ from django.conf import settings ...@@ -11,8 +11,8 @@ from django.conf import settings
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from util.testing import UrlResetMixin from util.testing import UrlResetMixin
from user_api.api import account as account_api from openedx.core.djangoapps.user_api.api import account as account_api
from user_api.api import profile as profile_api from openedx.core.djangoapps.user_api.api import profile as profile_api
from lang_pref import LANGUAGE_KEY, api as language_api from lang_pref import LANGUAGE_KEY, api as language_api
......
...@@ -11,7 +11,7 @@ from django.views.decorators.http import require_http_methods ...@@ -11,7 +11,7 @@ from django.views.decorators.http import require_http_methods
from django_future.csrf import ensure_csrf_cookie from django_future.csrf import ensure_csrf_cookie
from django.contrib.auth.decorators import login_required from django.contrib.auth.decorators import login_required
from edxmako.shortcuts import render_to_response from edxmako.shortcuts import render_to_response
from user_api.api import profile as profile_api from openedx.core.djangoapps.user_api.api import profile as profile_api
from lang_pref import LANGUAGE_KEY, api as language_api from lang_pref import LANGUAGE_KEY, api as language_api
import third_party_auth import third_party_auth
......
...@@ -931,7 +931,7 @@ MIDDLEWARE_CLASSES = ( ...@@ -931,7 +931,7 @@ MIDDLEWARE_CLASSES = (
# Adds user tags to tracking events # Adds user tags to tracking events
# Must go before TrackMiddleware, to get the context set up # Must go before TrackMiddleware, to get the context set up
'user_api.middleware.UserTagsEventContextMiddleware', 'openedx.core.djangoapps.user_api.middleware.UserTagsEventContextMiddleware',
'django.contrib.messages.middleware.MessageMiddleware', 'django.contrib.messages.middleware.MessageMiddleware',
'track.middleware.TrackMiddleware', 'track.middleware.TrackMiddleware',
...@@ -1482,7 +1482,7 @@ INSTALLED_APPS = ( ...@@ -1482,7 +1482,7 @@ INSTALLED_APPS = (
# User API # User API
'rest_framework', 'rest_framework',
'user_api', 'openedx.core.djangoapps.user_api',
# Shopping cart # Shopping cart
'shoppingcart', 'shoppingcart',
......
...@@ -7,7 +7,7 @@ import xblock.reference.plugins ...@@ -7,7 +7,7 @@ import xblock.reference.plugins
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.conf import settings from django.conf import settings
from user_api.api import course_tag as user_course_tag_api from openedx.core.djangoapps.user_api.api import course_tag as user_course_tag_api
from xmodule.modulestore.django import modulestore from xmodule.modulestore.django import modulestore
from xmodule.x_module import ModuleSystem from xmodule.x_module import ModuleSystem
from xmodule.partitions.partitions_service import PartitionService from xmodule.partitions.partitions_service import PartitionService
...@@ -128,13 +128,13 @@ class LmsPartitionService(PartitionService): ...@@ -128,13 +128,13 @@ class LmsPartitionService(PartitionService):
course. course.
(If and when XBlock directly provides access from one block (e.g. a split_test_module) (If and when XBlock directly provides access from one block (e.g. a split_test_module)
to another (e.g. a course_module), this won't be neccessary, but for now it seems like to another (e.g. a course_module), this won't be necessary, but for now it seems like
the least messy way to hook things through) the least messy way to hook things through)
""" """
@property @property
def course_partitions(self): def course_partitions(self):
course = modulestore().get_course(self._course_id) course = modulestore().get_course(self.runtime.course_id)
return course.user_partitions return course.user_partitions
...@@ -194,8 +194,7 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract ...@@ -194,8 +194,7 @@ class LmsModuleSystem(LmsHandlerUrls, ModuleSystem): # pylint: disable=abstract
services = kwargs.setdefault('services', {}) services = kwargs.setdefault('services', {})
services['user_tags'] = UserTagsService(self) services['user_tags'] = UserTagsService(self)
services['partitions'] = LmsPartitionService( services['partitions'] = LmsPartitionService(
user_tags_service=services['user_tags'], runtime=self,
course_id=kwargs.get('course_id', None),
track_function=kwargs.get('track_function', None), track_function=kwargs.get('track_function', None),
) )
services['fs'] = xblock.reference.plugins.FSService() services['fs'] = xblock.reference.plugins.FSService()
......
...@@ -57,7 +57,7 @@ urlpatterns = ('', # nopep8 ...@@ -57,7 +57,7 @@ urlpatterns = ('', # nopep8
url(r'^heartbeat$', include('heartbeat.urls')), url(r'^heartbeat$', include('heartbeat.urls')),
url(r'^user_api/', include('user_api.urls')), url(r'^user_api/', include('openedx.core.djangoapps.user_api.urls')),
url(r'^notifier_api/', include('notifier_api.urls')), url(r'^notifier_api/', include('notifier_api.urls')),
......
...@@ -11,8 +11,8 @@ from django.db import transaction, IntegrityError ...@@ -11,8 +11,8 @@ from django.db import transaction, IntegrityError
from django.core.validators import validate_email, validate_slug, ValidationError from django.core.validators import validate_email, validate_slug, ValidationError
from django.contrib.auth.forms import PasswordResetForm from django.contrib.auth.forms import PasswordResetForm
from user_api.models import User, UserProfile, Registration, PendingEmailChange from ..models import User, UserProfile, Registration, PendingEmailChange
from user_api.helpers import intercept_errors from ..helpers import intercept_errors
USERNAME_MIN_LENGTH = 2 USERNAME_MIN_LENGTH = 2
......
...@@ -13,9 +13,9 @@ from django.db import IntegrityError ...@@ -13,9 +13,9 @@ from django.db import IntegrityError
from pytz import UTC from pytz import UTC
import analytics import analytics
from user_api.models import User, UserProfile, UserPreference, UserOrgTag
from user_api.helpers import intercept_errors
from eventtracking import tracker from eventtracking import tracker
from ..models import User, UserProfile, UserPreference, UserOrgTag
from ..helpers import intercept_errors
log = logging.getLogger(__name__) log = logging.getLogger(__name__)
...@@ -34,6 +34,7 @@ class ProfileInvalidField(ProfileRequestError): ...@@ -34,6 +34,7 @@ class ProfileInvalidField(ProfileRequestError):
""" The proposed value for a field is not in a valid format. """ """ The proposed value for a field is not in a valid format. """
def __init__(self, field, value): def __init__(self, field, value):
super(ProfileInvalidField, self).__init__()
self.field = field self.field = field
self.value = value self.value = value
......
...@@ -12,7 +12,7 @@ from django.http import HttpResponseBadRequest ...@@ -12,7 +12,7 @@ from django.http import HttpResponseBadRequest
LOGGER = logging.getLogger(__name__) LOGGER = logging.getLogger(__name__)
def intercept_errors(api_error, ignore_errors=[]): def intercept_errors(api_error, ignore_errors=None):
""" """
Function decorator that intercepts exceptions Function decorator that intercepts exceptions
and translates them into API-specific errors (usually an "internal" error). and translates them into API-specific errors (usually an "internal" error).
...@@ -33,13 +33,20 @@ def intercept_errors(api_error, ignore_errors=[]): ...@@ -33,13 +33,20 @@ def intercept_errors(api_error, ignore_errors=[]):
""" """
def _decorator(func): def _decorator(func):
"""
Function decorator that intercepts exceptions and translates them into API-specific errors.
"""
@wraps(func) @wraps(func)
def _wrapped(*args, **kwargs): def _wrapped(*args, **kwargs):
"""
Wrapper that evaluates a function, intercepting exceptions and translating them into
API-specific errors.
"""
try: try:
return func(*args, **kwargs) return func(*args, **kwargs)
except Exception as ex: except Exception as ex:
# Raise the original exception if it's in our list of "ignored" errors # Raise the original exception if it's in our list of "ignored" errors
for ignored in ignore_errors: for ignored in ignore_errors or []:
if isinstance(ex, ignored): if isinstance(ex, ignored):
raise raise
......
"""Generate a list indicating whether users have opted in or out of receiving email from an org.
Email opt-in is stored as an org-level preference.
When reports are generated, we need to handle:
1) Org aliases: some organizations might have multiple course key "org" values.
We choose the most recently set preference among all org aliases.
Since this information isn't stored anywhere in edx-platform,
the caller needs to pass in the list of orgs and aliases.
2) No preference set: Some users may not have an opt-in preference set
if they enrolled before the preference was introduced.
These users are opted in by default.
3) Restricting to a subset of courses in an org: Some orgs have courses
that we don't want to include in the results (e.g. EdX-created test courses).
Allow the caller to explicitly specify the list of courses in the org.
The command will always use the read replica database if one is configured.
"""
import os.path
import csv
import time
import contextlib
import logging
from django.core.management.base import BaseCommand, CommandError
from django.conf import settings
from django.db import connections
from opaque_keys.edx.keys import CourseKey
from xmodule.modulestore.django import modulestore
LOGGER = logging.getLogger(__name__)
class Command(BaseCommand):
"""Generate a list of email opt-in values for user enrollments. """
args = "<OUTPUT_FILENAME> <ORG_ALIASES> --courses=COURSE_ID_LIST"
help = "Generate a list of email opt-in values for user enrollments."
# Fields output in the CSV
OUTPUT_FIELD_NAMES = [
"email",
"full_name",
"course_id",
"is_opted_in_for_email",
"preference_set_date"
]
# Number of records to read at a time when making
# multiple queries over a potentially large dataset.
QUERY_INTERVAL = 1000
def handle(self, *args, **options):
"""Execute the command.
Arguments:
file_path (str): Path to the output file.
*org_list (unicode): List of organization aliases.
Keyword Arguments:
courses (unicode): Comma-separated list of course keys. If provided,
include only these courses in the results.
Raises:
CommandError
"""
file_path, org_list = self._parse_args(args)
# Retrieve all the courses for the org.
# If we were given a specific list of courses to include,
# filter out anything not in that list.
courses = self._get_courses_for_org(org_list)
only_courses = options.get("courses")
if only_courses is not None:
only_courses = [
CourseKey.from_string(course_key.strip())
for course_key in only_courses.split(",")
]
courses = list(set(courses) & set(only_courses))
# Add in organizations from the course keys, to ensure
# we're including orgs with different capitalizations
org_list = list(set(org_list) | set(course.org for course in courses))
# If no courses are found, abort
if not courses:
raise CommandError(
u"No courses found for orgs: {orgs}".format(
orgs=", ".join(org_list)
)
)
# Let the user know what's about to happen
LOGGER.info(
u"Retrieving data for courses: {courses}".format(
courses=", ".join([unicode(course) for course in courses])
)
)
# Open the output file and generate the report.
with open(file_path, "w") as file_handle:
with self._log_execution_time():
self._write_email_opt_in_prefs(file_handle, org_list, courses)
# Remind the user where the output file is
LOGGER.info(u"Output file: {file_path}".format(file_path=file_path))
def _parse_args(self, args):
"""Check and parse arguments.
Validates that the right number of args were provided
and that the output file doesn't already exist.
Arguments:
args (list): List of arguments given at the command line.
Returns:
Tuple of (file_path, org_list)
Raises:
CommandError
"""
if len(args) < 2:
raise CommandError(u"Usage: {args}".format(args=self.args))
file_path = args[0]
org_list = args[1:]
if os.path.exists(file_path):
raise CommandError("File already exists at '{path}'".format(path=file_path))
return file_path, org_list
def _get_courses_for_org(self, org_aliases):
"""Retrieve all course keys for a particular org.
Arguments:
org_aliases (list): List of aliases for the org.
Returns:
List of `CourseKey`s
"""
all_courses = modulestore().get_courses()
orgs_lowercase = [org.lower() for org in org_aliases]
return [
course.id
for course in all_courses
if course.id.org.lower() in orgs_lowercase
]
@contextlib.contextmanager
def _log_execution_time(self):
"""Context manager for measuring execution time. """
start_time = time.time()
yield
execution_time = time.time() - start_time
LOGGER.info(u"Execution time: {time} seconds".format(time=execution_time))
def _write_email_opt_in_prefs(self, file_handle, org_aliases, courses):
"""Write email opt-in preferences to the output file.
This will generate a CSV with one row for each enrollment.
This means that the user's "opt in" preference will be specified
multiple times if the user has enrolled in multiple courses
within the org. However, the values should always be the same:
if the user is listed as "opted out" for course A, she will
also be listed as "opted out" for courses B, C, and D.
Arguments:
file_handle (file): Handle to the output file.
org_aliases (list): List of aliases for the org.
courses (list): List of course keys in the org.
Returns:
None
"""
writer = csv.DictWriter(file_handle, fieldnames=self.OUTPUT_FIELD_NAMES)
cursor = self._db_cursor()
query = (
u"""
SELECT
user.`email` AS `email`,
profile.`name` AS `full_name`,
enrollment.`course_id` AS `course_id`,
(
SELECT value
FROM user_api_userorgtag
WHERE org IN ( {org_list} )
AND `key`=\"email-optin\"
AND `user_id`=user.`id`
ORDER BY modified DESC
LIMIT 1
) AS `is_opted_in_for_email`,
(
SELECT modified
FROM user_api_userorgtag
WHERE org IN ( {org_list} )
AND `key`=\"email-optin\"
AND `user_id`=user.`id`
ORDER BY modified DESC
LIMIT 1
) AS `preference_set_date`
FROM
student_courseenrollment AS enrollment
LEFT JOIN auth_user AS user ON user.id=enrollment.user_id
LEFT JOIN auth_userprofile AS profile ON profile.user_id=user.id
WHERE enrollment.course_id IN ( {course_id_list} )
"""
).format(
course_id_list=self._sql_list(courses),
org_list=self._sql_list(org_aliases)
)
cursor.execute(query)
row_count = 0
for row in self._iterate_results(cursor):
email, full_name, course_id, is_opted_in, pref_set_date = row
writer.writerow({
"email": email.encode('utf-8'),
"full_name": full_name.encode('utf-8'),
"course_id": course_id.encode('utf-8'),
"is_opted_in_for_email": is_opted_in if is_opted_in else "True",
"preference_set_date": pref_set_date,
})
row_count += 1
# Log the number of rows we processed
LOGGER.info(u"Retrieved {num_rows} records.".format(num_rows=row_count))
def _iterate_results(self, cursor):
"""Iterate through the results of a database query, fetching in chunks.
Arguments:
cursor: The database cursor
Yields:
tuple of row values from the query
"""
while True:
rows = cursor.fetchmany(self.QUERY_INTERVAL)
if not rows:
break
for row in rows:
yield row
def _sql_list(self, values):
"""Serialize a list of values for including in a SQL "IN" statement. """
return u",".join([u'"{}"'.format(val) for val in values])
def _db_cursor(self):
"""Return a database cursor to the read replica if one is available. """
# Use the read replica if one has been configured
db_alias = (
'read_replica'
if 'read_replica' in settings.DATABASES
else 'default'
)
return connections[db_alias].cursor()
...@@ -8,7 +8,8 @@ from opaque_keys import InvalidKeyError ...@@ -8,7 +8,8 @@ from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey from opaque_keys.edx.keys import CourseKey
from track.contexts import COURSE_REGEX from track.contexts import COURSE_REGEX
from user_api.models import UserCourseTag
from .models import UserCourseTag
class UserTagsEventContextMiddleware(object): class UserTagsEventContextMiddleware(object):
......
"""
Provides partition support to the user service.
"""
import random
import api.course_tag as course_tag_api
from xmodule.partitions.partitions import UserPartitionError
class RandomUserPartitionScheme(object):
"""
This scheme randomly assigns users into the partition's groups.
"""
RANDOM = random.Random()
@classmethod
def get_group_for_user(cls, course_id, user, user_partition, track_function=None):
"""
Returns the group from the specified user position to which the user is assigned.
If the user has not yet been assigned, a group will be randomly chosen for them.
"""
partition_key = cls._key_for_partition(user_partition)
group_id = course_tag_api.get_course_tag(user, course_id, partition_key)
group = user_partition.get_group(int(group_id)) if not group_id is None else None
if group is None:
if not user_partition.groups:
raise UserPartitionError('Cannot assign user to an empty user partition')
# pylint: disable=fixme
# 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.
group = cls.RANDOM.choice(user_partition.groups)
# persist the value as a course tag
course_tag_api.set_course_tag(user, course_id, partition_key, group.id)
if track_function:
# 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
}
# pylint: disable=fixme
# TODO: Use the XBlock publish api instead
track_function('xmodule.partitions.assigned_user_to_partition', event_info)
return group
@classmethod
def _key_for_partition(cls, user_partition):
"""
Returns the key to use to look up and save the user's group for a given user partition.
"""
return 'xblock.partition_service.partition_{0}'.format(user_partition.id)
from django.contrib.auth.models import User from django.contrib.auth.models import User
from rest_framework import serializers from rest_framework import serializers
from student.models import UserProfile from student.models import UserProfile
from user_api.models import UserPreference
from .models import UserPreference
class UserSerializer(serializers.HyperlinkedModelSerializer): class UserSerializer(serializers.HyperlinkedModelSerializer):
......
...@@ -2,9 +2,10 @@ ...@@ -2,9 +2,10 @@
from factory.django import DjangoModelFactory from factory.django import DjangoModelFactory
from factory import SubFactory from factory import SubFactory
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from user_api.models import UserPreference, UserCourseTag, UserOrgTag
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from ..models import UserPreference, UserCourseTag, UserOrgTag
# Factories are self documenting # Factories are self documenting
# pylint: disable=missing-docstring # pylint: disable=missing-docstring
......
...@@ -12,8 +12,8 @@ from django.core import mail ...@@ -12,8 +12,8 @@ from django.core import mail
from django.test import TestCase from django.test import TestCase
from django.conf import settings from django.conf import settings
from user_api.api import account as account_api from ..api import account as account_api
from user_api.models import UserProfile from ..models import UserProfile
@ddt.ddt @ddt.ddt
......
...@@ -4,11 +4,11 @@ Test the user course tag API. ...@@ -4,11 +4,11 @@ Test the user course tag API.
from django.test import TestCase from django.test import TestCase
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from user_api.api import course_tag as course_tag_api from openedx.core.djangoapps.user_api.api import course_tag as course_tag_api
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
class TestUserService(TestCase): class TestCourseTagAPI(TestCase):
""" """
Test the user service Test the user service
""" """
......
...@@ -4,10 +4,10 @@ Tests for helper functions. ...@@ -4,10 +4,10 @@ Tests for helper functions.
import json import json
import mock import mock
import ddt import ddt
from django.http import HttpRequest, HttpResponse
from django.test import TestCase from django.test import TestCase
from nose.tools import raises from nose.tools import raises
from django.http import HttpRequest, HttpResponse from ..helpers import (
from user_api.helpers import (
intercept_errors, shim_student_view, intercept_errors, shim_student_view,
FormDescription, InvalidFieldError FormDescription, InvalidFieldError
) )
...@@ -49,12 +49,12 @@ class InterceptErrorsTest(TestCase): ...@@ -49,12 +49,12 @@ class InterceptErrorsTest(TestCase):
def test_ignores_expected_errors(self): def test_ignores_expected_errors(self):
intercepted_function(raise_error=ValueError) intercepted_function(raise_error=ValueError)
@mock.patch('user_api.helpers.LOGGER') @mock.patch('openedx.core.djangoapps.user_api.helpers.LOGGER')
def test_logs_errors(self, mock_logger): def test_logs_errors(self, mock_logger):
expected_log_msg = ( expected_log_msg = (
u"An unexpected error occurred when calling 'intercepted_function' " u"An unexpected error occurred when calling 'intercepted_function' "
u"with arguments '()' and " u"with arguments '()' and "
u"keyword arguments '{'raise_error': <class 'user_api.tests.test_helpers.FakeInputException'>}': " u"keyword arguments '{'raise_error': <class 'openedx.core.djangoapps.user_api.tests.test_helpers.FakeInputException'>}': "
u"FakeInputException()" u"FakeInputException()"
) )
......
...@@ -6,8 +6,9 @@ from django.http import HttpResponse ...@@ -6,8 +6,9 @@ from django.http import HttpResponse
from django.test.client import RequestFactory from django.test.client import RequestFactory
from student.tests.factories import UserFactory, AnonymousUserFactory from student.tests.factories import UserFactory, AnonymousUserFactory
from user_api.tests.factories import UserCourseTagFactory
from user_api.middleware import UserTagsEventContextMiddleware from ..tests.factories import UserCourseTagFactory
from ..middleware import UserTagsEventContextMiddleware
class TagsMiddlewareTest(TestCase): class TagsMiddlewareTest(TestCase):
...@@ -29,7 +30,7 @@ class TagsMiddlewareTest(TestCase): ...@@ -29,7 +30,7 @@ class TagsMiddlewareTest(TestCase):
self.response = Mock(spec=HttpResponse) self.response = Mock(spec=HttpResponse)
patcher = patch('user_api.middleware.tracker') patcher = patch('openedx.core.djangoapps.user_api.middleware.tracker')
self.tracker = patcher.start() self.tracker = patcher.start()
self.addCleanup(patcher.stop) self.addCleanup(patcher.stop)
......
...@@ -2,8 +2,9 @@ from django.db import IntegrityError ...@@ -2,8 +2,9 @@ from django.db import IntegrityError
from django.test import TestCase from django.test import TestCase
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from user_api.tests.factories import UserPreferenceFactory, UserCourseTagFactory, UserOrgTagFactory
from user_api.models import UserPreference from ..tests.factories import UserPreferenceFactory, UserCourseTagFactory, UserOrgTagFactory
from ..models import UserPreference
class UserPreferenceModelTest(TestCase): class UserPreferenceModelTest(TestCase):
......
"""
Test the user api's partition extensions.
"""
from collections import defaultdict
from mock import patch
from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme, UserPartitionError
from student.tests.factories import UserFactory
from xmodule.partitions.partitions import Group, UserPartition
from xmodule.partitions.tests.test_partitions import PartitionTestCase
class MemoryCourseTagAPI(object):
"""
An implementation of a user service that uses an in-memory dictionary for storage
"""
def __init__(self):
self._tags = defaultdict(dict)
def get_course_tag(self, __, course_id, key):
"""Sets the value of ``key`` to ``value``"""
return self._tags[course_id].get(key)
def set_course_tag(self, __, course_id, key, value):
"""Gets the value of ``key``"""
self._tags[course_id][key] = value
class TestRandomUserPartitionScheme(PartitionTestCase):
"""
Test getting a user's group out of a partition
"""
MOCK_COURSE_ID = "mock-course-id"
def setUp(self):
super(TestRandomUserPartitionScheme, self).setUp()
# Patch in a memory-based user service instead of using the persistent version
course_tag_api = MemoryCourseTagAPI()
self.user_service_patcher = patch(
'openedx.core.djangoapps.user_api.partition_schemes.course_tag_api', course_tag_api
)
self.user_service_patcher.start()
# Create a test user
self.user = UserFactory.create()
def tearDown(self):
self.user_service_patcher.stop()
def test_get_group_for_user(self):
# get a group assigned to the user
group1_id = RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, self.user_partition)
# make sure we get the same group back out every time
for __ in range(0, 10):
group2_id = RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, self.user_partition)
self.assertEqual(group1_id, group2_id)
def test_empty_partition(self):
empty_partition = UserPartition(
self.TEST_ID,
'Test Partition',
'for testing purposes',
[],
scheme=RandomUserPartitionScheme
)
# get a group assigned to the user
with self.assertRaisesRegexp(UserPartitionError, "Cannot assign user to an empty user partition"):
RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, empty_partition)
def test_user_in_deleted_group(self):
# get a group assigned to the user - should be group 0 or 1
old_group = RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, self.user_partition)
self.assertIn(old_group.id, [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.TEST_ID, 'Test Partition', 'for testing purposes', groups)
# Now, get a new group using the same call - should be 3 or 4
new_group = RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, user_partition)
self.assertIn(new_group.id, [3, 4])
# We should get the same group over multiple calls
new_group_2 = RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, user_partition)
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 = RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, self.user_partition)
self.assertIn(old_group.id, [0, 1])
# Change the group names
groups = [Group(0, 'Group 0'), Group(1, 'Group 1')]
user_partition = UserPartition(
self.TEST_ID,
'Test Partition',
'for testing purposes',
groups,
scheme=RandomUserPartitionScheme
)
# Now, get a new group using the same call
new_group = RandomUserPartitionScheme.get_group_for_user(self.MOCK_COURSE_ID, self.user, user_partition)
self.assertEqual(old_group.id, new_group.id)
...@@ -10,9 +10,9 @@ from dateutil.parser import parse as parse_datetime ...@@ -10,9 +10,9 @@ from dateutil.parser import parse as parse_datetime
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
import datetime import datetime
from user_api.api import account as account_api from ..api import account as account_api
from user_api.api import profile as profile_api from ..api import profile as profile_api
from user_api.models import UserProfile, UserOrgTag from ..models import UserProfile, UserOrgTag
@ddt.ddt @ddt.ddt
......
...@@ -16,16 +16,16 @@ from pytz import UTC ...@@ -16,16 +16,16 @@ from pytz import UTC
import mock import mock
from xmodule.modulestore.tests.factories import CourseFactory from xmodule.modulestore.tests.factories import CourseFactory
from user_api.api import account as account_api, profile as profile_api
from student.tests.factories import UserFactory from student.tests.factories import UserFactory
from user_api.models import UserOrgTag from unittest import SkipTest
from user_api.tests.factories import UserPreferenceFactory
from django_comment_common import models from django_comment_common import models
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from third_party_auth.tests.testutil import simulate_running_pipeline from third_party_auth.tests.testutil import simulate_running_pipeline
from user_api.tests.test_constants import SORTED_COUNTRIES from ..api import account as account_api, profile as profile_api
from ..models import UserOrgTag
from ..tests.factories import UserPreferenceFactory
from ..tests.test_constants import SORTED_COUNTRIES
TEST_API_KEY = "test_api_key" TEST_API_KEY = "test_api_key"
......
# pylint: disable=missing-docstring """
Defines the URL routes for this app.
"""
from django.conf import settings from django.conf import settings
from django.conf.urls import include, patterns, url from django.conf.urls import include, patterns, url
from rest_framework import routers from rest_framework import routers
from user_api import views as user_api_views
from user_api.models import UserPreference from . import views as user_api_views
from .models import UserPreference
user_api_router = routers.DefaultRouter() USER_API_ROUTER = routers.DefaultRouter()
user_api_router.register(r'users', user_api_views.UserViewSet) USER_API_ROUTER.register(r'users', user_api_views.UserViewSet)
user_api_router.register(r'user_prefs', user_api_views.UserPreferenceViewSet) USER_API_ROUTER.register(r'user_prefs', user_api_views.UserPreferenceViewSet)
urlpatterns = patterns( urlpatterns = patterns(
'', '',
url(r'^v1/', include(user_api_router.urls)), url(r'^v1/', include(USER_API_ROUTER.urls)),
url( url(
r'^v1/preferences/(?P<pref_key>{})/users/$'.format(UserPreference.KEY_REGEX), r'^v1/preferences/(?P<pref_key>{})/users/$'.format(UserPreference.KEY_REGEX),
user_api_views.PreferenceUsersListView.as_view() user_api_views.PreferenceUsersListView.as_view()
......
"""HTTP end-points for the User API. """ """HTTP end-points for the User API. """
import copy import copy
import third_party_auth
from django.conf import settings from django.conf import settings
from django.contrib.auth.models import User from django.contrib.auth.models import User
...@@ -19,16 +20,15 @@ from rest_framework import viewsets ...@@ -19,16 +20,15 @@ from rest_framework import viewsets
from rest_framework.views import APIView from rest_framework.views import APIView
from rest_framework.exceptions import ParseError from rest_framework.exceptions import ParseError
from django_countries import countries from django_countries import countries
from user_api.serializers import UserSerializer, UserPreferenceSerializer
from user_api.models import UserPreference, UserProfile
from django_comment_common.models import Role from django_comment_common.models import Role
from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.locations import SlashSeparatedCourseKey
from edxmako.shortcuts import marketing_link from edxmako.shortcuts import marketing_link
import third_party_auth
from util.authentication import SessionAuthenticationAllowInactiveUser from util.authentication import SessionAuthenticationAllowInactiveUser
from user_api.api import account as account_api, profile as profile_api from .api import account as account_api, profile as profile_api
from user_api.helpers import FormDescription, shim_student_view, require_post_params from .helpers import FormDescription, shim_student_view, require_post_params
from .models import UserPreference
from .serializers import UserSerializer, UserPreferenceSerializer
class ApiKeyHeaderPermission(permissions.BasePermission): class ApiKeyHeaderPermission(permissions.BasePermission):
......
...@@ -48,7 +48,7 @@ def test_system(options): ...@@ -48,7 +48,7 @@ def test_system(options):
if test_id: if test_id:
if not system: if not system:
system = test_id.split('/')[0] system = test_id.split('/')[0]
if system == 'common': if system in ['common', 'openedx']:
system = 'lms' system = 'lms'
opts['test_id'] = test_id opts['test_id'] = test_id
......
...@@ -128,7 +128,7 @@ class SystemTestSuite(NoseTestSuite): ...@@ -128,7 +128,7 @@ class SystemTestSuite(NoseTestSuite):
# django-nose will import them early in the test process, # django-nose will import them early in the test process,
# thereby making sure that we load any django models that are # thereby making sure that we load any django models that are
# only defined in test files. # only defined in test files.
default_test_id = "{system}/djangoapps/* common/djangoapps/*".format( default_test_id = "{system}/djangoapps/* common/djangoapps/* openedx/core/djangoapps/*".format(
system=self.root system=self.root
) )
......
from setuptools import setup, find_packages """
Setup script for the Open edX package.
"""
from setuptools import setup
setup( setup(
name="Open edX", name="Open edX",
version="0.1", version="0.2",
install_requires=['distribute'], install_requires=['distribute'],
requires=[], requires=[],
# NOTE: These are not the names we should be installing. This tree should # NOTE: These are not the names we should be installing. This tree should
# be reorgnized to be a more conventional Python tree. # be reorganized to be a more conventional Python tree.
packages=[ packages=[
"openedx.core.djangoapps.user_api",
"lms", "lms",
"cms", "cms",
], ],
entry_points={
'openedx.user_partition_scheme': [
'random = openedx.core.djangoapps.user_api.partition_schemes:RandomUserPartitionScheme',
],
}
) )
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment