Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Remove most references to old teams config scheme (#22238)

Browse Source 
This is a follow up from MST-16, which was commited
in 3858036a4e.

Changes:
* Enrich course teams_configuration from a plain Dict
  to a custom XBlock field that uses the new TeamsConfig
  wrapper class.
* Remove teams_conf property from course, as the previous
  change made it redundant.
* Update teams_enabled implementation.
* Remove teams_max_size field from course, which is
  no longer semantically correct, as max team size
  is now defined on a teamset level.
* Remove teams_topics in order to discourage use of raw
  teams config dict.
* Add convenience properties teamsets and teamsets_by_id
  to course.
* Allow periods and spaces in teamset IDs to avoid breaking
  existing course teams.

Some parts of the code still use the old raw config data
(identifiable by searching "cleaned_data_old_format"),
which we expect to be slowly factored away as we build
new teams features. MST-40 has been created to remove any
remaining references if necessary.

MST-18

* fix: bokchoy test

* fix: remove pdb break
This commit is contained in:
Kyle McCormick
2019-11-06 20:43:32 -05:00
committed by GitHub
parent d6099e08fd
commit 4f3262a40b
12 changed files with 203 additions and 168 deletions
Download Patch File Download Diff File Expand all files Collapse all files

86
common/lib/xmodule/xmodule/course_module.py
View File

@@ -271,6 +271,33 @@ def get_available_providers():
return available_providers
class TeamsConfigField(Dict):
"""
XBlock field for teams configuration, including definitions for teamsets.
Serializes to JSON dictionary.
"""
_default = TeamsConfig({})
def from_json(self, value):
"""
Return a TeamsConfig instance from a dict.
"""
return TeamsConfig(value)
def to_json(self, value):
"""
Convert a TeamsConfig instance back to a dict.
If we have the data that was used to build the TeamsConfig instance,
return that instead of `value.cleaned_data`, thus preserving the
data in the form that the user entered it.
"""
if value.source_data is not None:
return value.source_data
return value.cleaned_data
class CourseFields(object):
lti_passports = List(
display_name=_("LTI Passports"),
@@ -797,7 +824,7 @@ class CourseFields(object):
scope=Scope.settings
)
teams_configuration = Dict(
teams_configuration = TeamsConfigField(
display_name=_("Teams Configuration"),
# Translators: please don't translate "id".
help=_(
@@ -808,6 +835,8 @@ class CourseFields(object):
'For example, to specify that teams should have a maximum of 5 participants and provide a list of '
'2 topics, enter the configuration in this format: {example_format}. '
'In "id" values, the only supported special characters are underscore, hyphen, and period.'
# Note that we also support space (" "), which may have been an accident, but it's in
# our DB now. Let's not advertise the fact, though.
),
help_format_args=dict(
# Put the sample JSON into a format variable so that translators
@@ -1471,65 +1500,32 @@ class CourseDescriptor(CourseFields, SequenceDescriptor, LicenseMixin):
"""
return course_metadata_utils.clean_course_key(self.location.course_key, padding_char)
@property
def teams_conf(self):
"""
Returns a TeamsConfig object wrapping the dict `teams_configuration`.
TODO: In MST-18, `teams_configuration` will be a custom field that
parses its input into a TeamsConfig object.
All references to this property will become references to
`.teams_configuration`.
"""
# If the `teams_configuration` dict hasn't changed, return a cached
# `TeamsConfig` to avoid re-parsing things.
# If it has changed, recompute the `TeamsConfig`.
try:
cached_teams_conf, cached_teams_conf_source = (
self._teams_conf, self._teams_conf_source
)
except AttributeError:
pass
else:
if cached_teams_conf_source == self.teams_configuration:
return cached_teams_conf
self._teams_conf_source = self.teams_configuration
self._teams_conf = TeamsConfig(self._teams_conf_source)
return self._teams_conf
@property
def teams_enabled(self):
"""
Returns whether or not teams has been enabled for this course.
Alias to `self.teams_configuration.is_enabled`, for convenience.
Currently, teams are considered enabled when at least one topic has been configured for the course.
Deprecated; please use `self.teams_conf.is_enabled` instead.
Will be removed (TODO MST-18).
Returns bool.
"""
if self.teams_configuration:
return len(self.teams_configuration.get('topics', [])) > 0
return False
return self.teams_configuration.is_enabled # pylint: disable=no-member
@property
def teams_max_size(self):
def teamsets(self):
"""
Returns the max size for teams if teams has been configured, else None.
Alias to `self.teams_configuration.teamsets`, for convenience.
Deprecated; please use `self.teams_conf.calc_max_team_size(...)` instead.
Will be removed (TODO MST-18).
Returns list[TeamsetConfig].
"""
return self.teams_configuration.get('max_team_size', None)
return self.teams_configuration.teamsets # pylint: disable=no-member
@property
def teams_topics(self):
def teamsets_by_id(self):
"""
Returns the topics that have been configured for teams for this course, else None.
Alias to `self.teams_configuration.teamsets_by_id`, for convenience.
Deprecated; please use `self.teams_conf.teamsets` instead.
Will be removed (TODO MST-18).
Returns dict[str: TeamsetConfig].
"""
return self.teams_configuration.get('topics', None)
return self.teams_configuration.teamsets_by_id
def set_user_partitions_for_scheme(self, partitions, scheme):
"""

51
common/lib/xmodule/xmodule/tests/test_course_module.py
View File

@@ -15,6 +15,7 @@ from opaque_keys.edx.keys import CourseKey
from pytz import utc
from xblock.runtime import DictKeyValueStore, KvsFieldData
from openedx.core.lib.teams_config import TeamsConfig
import xmodule.course_module
from xmodule.modulestore.xml import ImportSystem, XMLModuleStore
@@ -272,25 +273,21 @@ class DiscussionTopicsTestCase(unittest.TestCase):
class TeamsConfigurationTestCase(unittest.TestCase):
"""
Tests for the configuration of teams and the helper methods for accessing them.
Also tests new configuration wrapper, `.teams_conf`.
Eventually the tests for the old dict-based `.teams_configuration` field
wil be removed (TODO MST-18).
"""
def setUp(self):
super(TeamsConfigurationTestCase, self).setUp()
self.course = get_dummy_course('2012-12-02T12:00')
self.course.teams_configuration = dict()
self.course.teams_configuration = TeamsConfig(None)
self.count = itertools.count()
def add_team_configuration(self, max_team_size=3, topics=None):
""" Add a team configuration to the course. """
teams_configuration = {}
teams_configuration["topics"] = [] if topics is None else topics
teams_config_data = {}
teams_config_data["topics"] = [] if topics is None else topics
if max_team_size is not None:
teams_configuration["max_team_size"] = max_team_size
self.course.teams_configuration = teams_configuration
teams_config_data["max_team_size"] = max_team_size
self.course.teams_configuration = TeamsConfig(teams_config_data)
def make_topic(self):
""" Make a sample topic dictionary. """
@@ -303,66 +300,56 @@ class TeamsConfigurationTestCase(unittest.TestCase):
def test_teams_enabled_new_course(self):
# Make sure we can detect when no teams exist.
self.assertFalse(self.course.teams_enabled)
self.assertFalse(self.course.teams_conf.is_enabled)
# add topics
self.add_team_configuration(max_team_size=4, topics=[self.make_topic()])
self.assertTrue(self.course.teams_enabled)
self.assertTrue(self.course.teams_conf.is_enabled)
# remove them again
self.add_team_configuration(max_team_size=4, topics=[])
self.assertFalse(self.course.teams_enabled)
self.assertFalse(self.course.teams_conf.is_enabled)
def test_teams_enabled_max_size_only(self):
self.add_team_configuration(max_team_size=4)
self.assertFalse(self.course.teams_enabled)
self.assertFalse(self.course.teams_conf.is_enabled)
def test_teams_enabled_no_max_size(self):
self.add_team_configuration(max_team_size=None, topics=[self.make_topic()])
self.assertTrue(self.course.teams_enabled)
self.assertTrue(self.course.teams_conf.is_enabled)
def test_teams_max_size_no_teams_configuration(self):
self.assertIsNone(self.course.teams_max_size)
self.assertIsNone(self.course.teams_conf.max_team_size)
self.assertIsNone(self.course.teams_configuration.default_max_team_size)
def test_teams_max_size_with_teams_configured(self):
size = 4
self.add_team_configuration(max_team_size=size, topics=[self.make_topic(), self.make_topic()])
self.assertTrue(self.course.teams_enabled)
self.assertEqual(size, self.course.teams_max_size)
self.assertEqual(size, self.course.teams_conf.max_team_size)
self.assertEqual(size, self.course.teams_configuration.default_max_team_size)
def test_teams_topics_no_teams(self):
self.assertIsNone(self.course.teams_topics)
self.assertEqual(self.course.teams_conf.teamsets, [])
def test_teamsets_no_config(self):
self.assertEqual(self.course.teamsets, [])
def test_teams_topics_no_topics(self):
def test_teamsets_empty(self):
self.add_team_configuration(max_team_size=4)
self.assertEqual(self.course.teams_topics, [])
self.assertEqual(self.course.teams_conf.teamsets, [])
self.assertEqual(self.course.teamsets, [])
def test_teams_topics_with_topics(self):
def test_teamsets_present(self):
topics = [self.make_topic(), self.make_topic()]
self.add_team_configuration(max_team_size=4, topics=topics)
self.assertTrue(self.course.teams_enabled)
self.assertEqual(self.course.teams_topics, topics)
expected_teamsets_data = [
teamset.cleaned_data_old_format
for teamset in self.course.teams_conf.teamsets
for teamset in self.course.teamsets
]
self.assertEqual(expected_teamsets_data, topics)
def test_teams_conf_caching(self):
def test_teams_conf_cached_by_xblock_field(self):
self.add_team_configuration(max_team_size=5, topics=[self.make_topic()])
cold_cache_conf = self.course.teams_conf
warm_cache_conf = self.course.teams_conf
cold_cache_conf = self.course.teams_configuration
warm_cache_conf = self.course.teams_configuration
self.add_team_configuration(max_team_size=5, topics=[self.make_topic(), self.make_topic()])
new_cold_cache_conf = self.course.teams_conf
new_warm_cache_conf = self.course.teams_conf
new_cold_cache_conf = self.course.teams_configuration
new_warm_cache_conf = self.course.teams_configuration
self.assertIs(cold_cache_conf, warm_cache_conf)
self.assertIs(new_cold_cache_conf, new_warm_cache_conf)
self.assertIsNot(cold_cache_conf, new_cold_cache_conf)

13
common/test/acceptance/tests/lms/test_teams.py
View File

@@ -103,15 +103,15 @@ class TeamsTabBase(EventsTestMixin, ForumsConfigMixin, UniqueCourseTest):
)
return json.loads(response.text)
def set_team_configuration(self, configuration, enroll_in_course=True, global_staff=False):
def set_team_configuration(self, configuration_data, enroll_in_course=True, global_staff=False):
"""
Sets team configuration on the course and calls auto-auth on the user.
"""
#pylint: disable=attribute-defined-outside-init
self.course_fixture = CourseFixture(**self.course_info)
if configuration:
if configuration_data:
self.course_fixture.add_advanced_settings(
{u"teams_configuration": {u"value": configuration}}
{u"teams_configuration": {u"value": configuration_data}}
)
self.course_fixture.install()
@@ -534,7 +534,12 @@ class BrowseTopicsTest(TeamsTabBase):
"""
initial_description = "A" + " really" * 50 + " long description"
self.set_team_configuration(
{u"max_team_size": 1, u"topics": [{"name": "", "id": "", "description": initial_description}]}
{
u"max_team_size": 1,
u"topics": [
{"id": "long-description-topic", "description": initial_description},
]
}
)
self.topics_page.visit()
truncated_description = self.topics_page.topic_descriptions[0]

5
lms/djangoapps/discussion/django_comment_client/base/tests.py
View File

@@ -47,6 +47,7 @@ from openedx.core.djangoapps.django_comment_common.utils import (
set_course_discussion_settings
)
from openedx.core.djangoapps.waffle_utils.testutils import WAFFLE_TABLES
from openedx.core.lib.teams_config import TeamsConfig
from student.roles import CourseStaffRole, UserBasedRole
from student.tests.factories import CourseAccessRoleFactory, CourseEnrollmentFactory, UserFactory
from track.middleware import TrackMiddleware
@@ -1440,10 +1441,10 @@ class TeamsPermissionsTestCase(ForumsEnableMixin, UrlResetMixin, SharedModuleSto
def setUpClass(cls):
# pylint: disable=super-method-not-called
with super(TeamsPermissionsTestCase, cls).setUpClassAndTestData():
teams_configuration = {
teams_config_data = {
'topics': [{'id': "topic_id", 'name': 'Solar Power', 'description': 'Solar power is hot'}]
}
cls.course = CourseFactory.create(teams_configuration=teams_configuration)
cls.course = CourseFactory.create(teams_configuration=TeamsConfig(teams_config_data))
@classmethod
def setUpTestData(cls):

7
lms/djangoapps/instructor/tests/test_api.py
View File

@@ -67,6 +67,7 @@ from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_COMM
from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles
from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
from openedx.core.djangoapps.site_configuration.tests.mixins import SiteMixin
from openedx.core.lib.teams_config import TeamsConfig
from openedx.core.lib.xblock_utils import grade_histogram
from shoppingcart.models import (
Coupon,
@@ -3033,9 +3034,9 @@ class TestInstructorAPILevelsDataDump(SharedModuleStoreTestCase, LoginEnrollment
has teams enabled, and does not when the course does not have teams enabled
"""
if has_teams:
self.course = CourseFactory.create(teams_configuration={
'max_size': 2, 'topics': [{'topic-id': 'topic', 'name': 'Topic', 'description': 'A Topic'}]
})
self.course = CourseFactory.create(teams_configuration=TeamsConfig({
'max_size': 2, 'topics': [{'id': 'topic', 'name': 'Topic', 'description': 'A Topic'}]
}))
course_instructor = InstructorFactory(course_key=self.course.id)
self.client.login(username=course_instructor.username, password='test')

19
lms/djangoapps/instructor_task/tests/test_tasks_helper.py
View File

@@ -75,6 +75,7 @@ from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from openedx.core.djangoapps.credit.tests.factories import CreditCourseFactory
from openedx.core.djangoapps.user_api.partition_schemes import RandomUserPartitionScheme
from openedx.core.djangoapps.util.testing import ContentGroupTestCase, TestConditionalContent
from openedx.core.lib.teams_config import TeamsConfig
from shoppingcart.models import (
Coupon,
CourseRegistrationCode,
@@ -96,6 +97,12 @@ from ..models import ReportStore
from ..tasks_helper.utils import UPDATE_STATUS_FAILED, UPDATE_STATUS_SUCCEEDED
_TEAMS_CONFIG = TeamsConfig({
'max_size': 2,
'topics': [{'id': 'topic', 'name': 'Topic', 'description': 'A Topic'}],
})
class InstructorGradeReportTestCase(TestReportMixin, InstructorTaskCourseTestCase):
""" Base class for grade report tests. """
@@ -403,9 +410,7 @@ class TestInstructorGradeReport(InstructorGradeReportTestCase):
course = CourseFactory.create(
cohort_config={'cohorted': True, 'auto_cohort': True, 'auto_cohort_groups': ['cohort 1', 'cohort 2']},
user_partitions=[experiment_partition],
teams_configuration={
'max_size': 2, 'topics': [{'topic-id': 'topic', 'name': 'Topic', 'description': 'A Topic'}]
},
teams_configuration=_TEAMS_CONFIG,
)
_ = CreditCourseFactory(course_key=course.id)
@@ -451,9 +456,7 @@ class TestTeamGradeReport(InstructorGradeReportTestCase):
def setUp(self):
super(TestTeamGradeReport, self).setUp()
self.course = CourseFactory.create(teams_configuration={
'max_size': 2, 'topics': [{'topic-id': 'topic', 'name': 'Topic', 'description': 'A Topic'}]
})
self.course = CourseFactory.create(teams_configuration=_TEAMS_CONFIG)
self.student1 = UserFactory.create()
CourseEnrollment.enroll(self.student1, self.course.id)
self.student2 = UserFactory.create()
@@ -1547,9 +1550,7 @@ class TestTeamStudentReport(TestReportMixin, InstructorTaskCourseTestCase):
def setUp(self):
super(TestTeamStudentReport, self).setUp()
self.course = CourseFactory.create(teams_configuration={
'max_size': 2, 'topics': [{'topic-id': 'topic', 'name': 'Topic', 'description': 'A Topic'}]
})
self.course = CourseFactory.create(teams_configuration=_TEAMS_CONFIG)
self.student1 = UserFactory.create()
CourseEnrollment.enroll(self.student1, self.course.id)
self.student2 = UserFactory.create()

2
lms/djangoapps/teams/templates/teams/teams.html
View File

@@ -51,7 +51,7 @@ from openedx.core.djangolib.js_utils import (
teamMembershipsUrl: '${team_memberships_url | n, js_escaped_string}',
teamMembershipDetailUrl: '${team_membership_detail_url | n, js_escaped_string}',
myTeamsUrl: '${my_teams_url | n, js_escaped_string}',
maxTeamSize: ${course.teams_max_size | n, dump_js_escaped_json},
maxTeamSize: ${course.teams_configuration.default_max_team_size | n, dump_js_escaped_json},
languages: ${languages | n, dump_js_escaped_json},
countries: ${countries | n, dump_js_escaped_json},
teamsBaseUrl: '${teams_base_url | n, js_escaped_string}'

73
lms/djangoapps/teams/tests/test_serializers.py
View File

@@ -10,6 +10,7 @@ from django.test.client import RequestFactory
from lms.djangoapps.teams.serializers import BulkTeamCountTopicSerializer, MembershipSerializer, TopicSerializer
from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory
from openedx.core.lib.teams_config import TeamsConfig
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
@@ -25,10 +26,10 @@ class SerializerTestCase(SharedModuleStoreTestCase):
"""
super(SerializerTestCase, self).setUp()
self.course = CourseFactory.create(
teams_configuration={
teams_configuration=TeamsConfig({
"max_team_size": 10,
"topics": [{u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0'}]
}
}),
)
@@ -41,7 +42,7 @@ class MembershipSerializerTestCase(SerializerTestCase):
super(MembershipSerializerTestCase, self).setUp()
self.team = CourseTeamFactory.create(
course_id=self.course.id,
topic_id=self.course.teams_topics[0]['id']
topic_id=self.course.teamsets[0].teamset_id,
)
self.user = UserFactory.create()
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
@@ -81,7 +82,10 @@ class TopicSerializerTestCase(SerializerTestCase):
team count of 0, and that it only takes one SQL query.
"""
with self.assertNumQueries(1):
serializer = TopicSerializer(self.course.teams_topics[0], context={'course_id': self.course.id})
serializer = TopicSerializer(
self.course.teamsets[0].cleaned_data_old_format,
context={'course_id': self.course.id},
)
self.assertEqual(
serializer.data,
{u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 0}
@@ -92,9 +96,14 @@ class TopicSerializerTestCase(SerializerTestCase):
Verifies that the `TopicSerializer` correctly displays a topic with a
positive team count, and that it only takes one SQL query.
"""
CourseTeamFactory.create(course_id=self.course.id, topic_id=self.course.teams_topics[0]['id'])
CourseTeamFactory.create(
course_id=self.course.id, topic_id=self.course.teamsets[0].teamset_id
)
with self.assertNumQueries(1):
serializer = TopicSerializer(self.course.teams_topics[0], context={'course_id': self.course.id})
serializer = TopicSerializer(
self.course.teamsets[0].cleaned_data_old_format,
context={'course_id': self.course.id},
)
self.assertEqual(
serializer.data,
{u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 1}
@@ -102,17 +111,20 @@ class TopicSerializerTestCase(SerializerTestCase):
def test_scoped_within_course(self):
"""Verify that team count is scoped within a course."""
duplicate_topic = self.course.teams_topics[0]
duplicate_topic = self.course.teamsets[0].cleaned_data_old_format
second_course = CourseFactory.create(
teams_configuration={
teams_configuration=TeamsConfig({
"max_team_size": 10,
"topics": [duplicate_topic]
}
}),
)
CourseTeamFactory.create(course_id=self.course.id, topic_id=duplicate_topic[u'id'])
CourseTeamFactory.create(course_id=second_course.id, topic_id=duplicate_topic[u'id'])
with self.assertNumQueries(1):
serializer = TopicSerializer(self.course.teams_topics[0], context={'course_id': self.course.id})
serializer = TopicSerializer(
self.course.teamsets[0].cleaned_data_old_format,
context={'course_id': self.course.id},
)
self.assertEqual(
serializer.data,
{u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 1}
@@ -140,16 +152,17 @@ class BaseTopicSerializerTestCase(SerializerTestCase):
Helper method to set up topics on the course. Returns a list of
created topics.
"""
self.course.teams_configuration['topics'] = []
topics = [
{u'name': u'Tøpic {}'.format(i), u'description': u'The bést topic! {}'.format(i), u'id': six.text_type(i)}
for i in six.moves.range(num_topics)
]
for i in six.moves.range(num_topics):
topic_id = six.text_type(i)
self.course.teams_configuration['topics'].append(topics[i])
for topic in topics:
for _ in six.moves.range(teams_per_topic):
CourseTeamFactory.create(course_id=self.course.id, topic_id=topic_id)
CourseTeamFactory.create(course_id=self.course.id, topic_id=topic['id'])
self.course.teams_configuration = TeamsConfig({
'max_team_size': self.course.teams_configuration.default_max_team_size,
'topics': topics,
})
return topics
def assert_serializer_output(self, topics, num_teams_per_topic, num_queries):
@@ -157,7 +170,10 @@ class BaseTopicSerializerTestCase(SerializerTestCase):
Verify that the serializer produced the expected topics.
"""
with self.assertNumQueries(num_queries):
page = Paginator(self.course.teams_topics, self.PAGE_SIZE).page(1)
page = Paginator(
self.course.teams_configuration.cleaned_data_old_format['topics'],
self.PAGE_SIZE,
).page(1)
# pylint: disable=not-callable
serializer = self.serializer(instance=page, context={'course_id': self.course.id})
self.assertEqual(
@@ -170,7 +186,7 @@ class BaseTopicSerializerTestCase(SerializerTestCase):
Verify that we return no results and make no SQL queries for a page
with no topics.
"""
self.course.teams_configuration['topics'] = []
self.course.teams_configuration = TeamsConfig({'topics': []})
self.assert_serializer_output([], num_teams_per_topic=0, num_queries=0)
@@ -216,10 +232,10 @@ class BulkTeamCountTopicSerializerTestCase(BaseTopicSerializerTestCase):
first_course_topics = self.setup_topics(num_topics=self.NUM_TOPICS, teams_per_topic=teams_per_topic)
duplicate_topic = first_course_topics[0]
second_course = CourseFactory.create(
teams_configuration={
teams_configuration=TeamsConfig({
"max_team_size": 10,
"topics": [duplicate_topic]
}
}),
)
CourseTeamFactory.create(course_id=second_course.id, topic_id=duplicate_topic[u'id'])
self.assert_serializer_output(first_course_topics, num_teams_per_topic=teams_per_topic, num_queries=1)
@@ -230,23 +246,6 @@ class BulkTeamCountTopicSerializerTestCase(BaseTopicSerializerTestCase):
result.update(second)
return result
def setup_topics(self, num_topics=5, teams_per_topic=0):
"""
Helper method to set up topics on the course. Returns a list of
created topics.
"""
self.course.teams_configuration['topics'] = []
topics = [
{u'name': u'Tøpic {}'.format(i), u'description': u'The bést topic! {}'.format(i), u'id': six.text_type(i)}
for i in six.moves.range(num_topics)
]
for i in six.moves.range(num_topics):
topic_id = six.text_type(i)
self.course.teams_configuration['topics'].append(topics[i])
for _ in six.moves.range(teams_per_topic):
CourseTeamFactory.create(course_id=self.course.id, topic_id=topic_id)
return topics
def assert_serializer_output(self, topics, num_teams_per_topic, num_queries):
"""
Verify that the serializer produced the expected topics.
@@ -263,5 +262,5 @@ class BulkTeamCountTopicSerializerTestCase(BaseTopicSerializerTestCase):
Verify that we return no results and make no SQL queries for a page
with no topics.
"""
self.course.teams_configuration['topics'] = []
self.course.teams_configuration = TeamsConfig({'topics': []})
self.assert_serializer_output([], num_teams_per_topic=0, num_queries=0)

17
lms/djangoapps/teams/tests/test_views.py
View File

@@ -25,6 +25,7 @@ from common.test.utils import skip_signal
from lms.djangoapps.courseware.tests.factories import StaffFactory
from openedx.core.djangoapps.django_comment_common.models import FORUM_ROLE_COMMUNITY_TA, Role
from openedx.core.djangoapps.django_comment_common.utils import seed_permissions_roles
from openedx.core.lib.teams_config import TeamsConfig
from student.models import CourseEnrollment
from student.tests.factories import AdminFactory, CourseEnrollmentFactory, UserFactory
from util.testing import EventTestMixin
@@ -46,7 +47,7 @@ class TestDashboard(SharedModuleStoreTestCase):
def setUpClass(cls):
super(TestDashboard, cls).setUpClass()
cls.course = CourseFactory.create(
teams_configuration={
teams_configuration=TeamsConfig({
"max_team_size": 10,
"topics": [
{
@@ -56,7 +57,7 @@ class TestDashboard(SharedModuleStoreTestCase):
}
for topic_id in range(cls.NUM_TOPICS)
]
}
})
)
def setUp(self):
@@ -158,7 +159,7 @@ class TestDashboard(SharedModuleStoreTestCase):
# Create a course two
course_two = CourseFactory.create(
teams_configuration={
teams_configuration=TeamsConfig({
"max_team_size": 1,
"topics": [
{
@@ -167,7 +168,7 @@ class TestDashboard(SharedModuleStoreTestCase):
"description": "Description for test topic for course two."
}
]
}
})
)
# Login and enroll user in both course course
@@ -204,7 +205,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
def setUpClass(cls):
# pylint: disable=super-method-not-called
with super(TeamAPITestCase, cls).setUpClassAndTestData():
teams_configuration_1 = {
teams_configuration_1 = TeamsConfig({
'topics':
[
{
@@ -213,7 +214,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
'description': u'Description for topic {}.'.format(i)
} for i, name in enumerate([u'Sólar power', 'Wind Power', 'Nuclear Power', 'Coal Power'])
]
}
})
cls.test_course_1 = CourseFactory.create(
org='TestX',
course='TS101',
@@ -221,7 +222,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
teams_configuration=teams_configuration_1
)
teams_configuration_2 = {
teams_configuration_2 = TeamsConfig({
'topics':
[
{
@@ -241,7 +242,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
},
],
'max_team_size': 1
}
})
cls.test_course_2 = CourseFactory.create(
org='MIT',
course='6.002x',

21
lms/djangoapps/teams/views.py
View File

@@ -404,7 +404,7 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
result_filter.update({'membership__user__username': username})
topic_id = request.query_params.get('topic_id', None)
if topic_id is not None:
if topic_id not in [topic['id'] for topic in course_module.teams_configuration['topics']]:
if topic_id not in course_module.teamsets_by_id:
error = build_api_error(
ugettext_noop(u'The supplied topic id {topic_id} is not valid'),
topic_id=topic_id
@@ -824,7 +824,10 @@ def get_alphabetical_topics(course_module):
Returns:
list: a list of sorted team topics
"""
return sorted(course_module.teams_topics, key=lambda t: t['name'].lower())
return sorted(
course_module.teams_configuration.cleaned_data_old_format['topics'],
key=lambda t: t['name'].lower(),
)
class TopicDetailView(APIView):
@@ -884,12 +887,14 @@ class TopicDetailView(APIView):
if not has_team_api_access(request.user, course_id):
return Response(status=status.HTTP_403_FORBIDDEN)
topics = [t for t in course_module.teams_topics if t['id'] == topic_id]
if not topics:
try:
topic = course_module.teamsets_by_id[topic_id]
except KeyError:
return Response(status=status.HTTP_404_NOT_FOUND)
serializer = TopicSerializer(topics[0], context={'course_id': course_id})
serializer = TopicSerializer(
topic.cleaned_data_old_format, context={'course_id': course_id}
)
return Response(serializer.data)
@@ -1112,7 +1117,9 @@ class MembershipListView(ExpandableFieldViewMixin, GenericAPIView):
return Response(status=status.HTTP_404_NOT_FOUND)
course_module = modulestore().get_course(team.course_id)
if course_module.teams_max_size is not None and team.users.count() >= course_module.teams_max_size:
# This should use `calc_max_team_size` instead of `default_max_team_size` (TODO MST-32).
max_team_size = course_module.teams_configuration.default_max_team_size
if max_team_size is not None and team.users.count() >= max_team_size:
return Response(
build_api_error(ugettext_noop("This team is already full.")),
status=status.HTTP_400_BAD_REQUEST

57
openedx/core/lib/teams_config.py
View File

@@ -41,12 +41,25 @@ class TeamsConfig(object):
"""
Return developer-helpful string.
"""
return "<{} max_team_size={} teamsets=[{}]>".format(
return "<{} default_max_team_size={} teamsets=[{}]>".format(
self.__class__.__name__,
self.max_team_size,
self.default_max_team_size,
", ".join(repr(teamset) for teamset in self.teamsets),
)
def __eq__(self, other):
"""
Define equality based on cleaned data.
"""
return isinstance(other, self.__class__) and self.cleaned_data == other.cleaned_data
def __ne__(self, other):
"""
Overrides default inequality to be the inverse of our custom equality.
Safe to remove once we're in Python 3 -- Py3 does this for us.
"""
return not self.__eq__(other)
@property
def source_data(self):
"""
@@ -60,7 +73,7 @@ class TeamsConfig(object):
JSON-friendly dictionary containing cleaned data from this TeamsConfig.
"""
return {
'max_team_size': self.max_team_size,
'max_team_size': self.default_max_team_size,
'team_sets': [
teamset.cleaned_data for teamset in self.teamsets
]
@@ -72,10 +85,10 @@ class TeamsConfig(object):
JSON-friendly dictionary containing cleaned data from this TeamsConfig,
excluding newly added fields.
Here for backwards compatibility; to be removed (TODO MST-18).
Here for backwards compatibility; to be removed (TODO MST-40).
"""
return {
'max_team_size': self.max_team_size,
'max_team_size': self.default_max_team_size,
'topics': [
teamset.cleaned_data_old_format for teamset in self.teamsets
]
@@ -123,9 +136,11 @@ class TeamsConfig(object):
return {teamset.teamset_id: teamset for teamset in self.teamsets}
@cached_property
def max_team_size(self):
def default_max_team_size(self):
"""
The default maximum size for teams in this course.
Can be overriden by individual team sets; see `calc_max_team_size`.
"""
return _clean_max_team_size(self._data.get('max_team_size'))
@@ -147,7 +162,7 @@ class TeamsConfig(object):
return None
if teamset.max_team_size:
return teamset.max_team_size
return self.max_team_size
return self.default_max_team_size
class TeamsetConfig(object):
@@ -157,7 +172,7 @@ class TeamsetConfig(object):
Takes in a configuration from a JSON-friendly dictionary,
and exposes cleaned data from it.
"""
teamset_id_regex = re.compile(r'^[A-Za-z0-9_-]+$')
teamset_id_regex = re.compile(r'^[A-Za-z0-9_. -]+$')
def __init__(self, data):
"""
@@ -192,6 +207,19 @@ class TeamsetConfig(object):
),
)
def __eq__(self, other):
"""
Define equality based on cleaned data.
"""
return isinstance(other, self.__class__) and self.cleaned_data == other.cleaned_data
def __ne__(self, other):
"""
Overrides default inequality to be the inverse of our custom equality.
Safe to remove once we're in Python 3 -- Py3 does this for us.
"""
return not self.__eq__(other)
@property
def source_data(self):
"""
@@ -218,7 +246,7 @@ class TeamsetConfig(object):
JSON-friendly dictionary containing cleaned data from this TeamsConfig,
excluding newly added fields.
Here for backwards compatibility; to be removed (TODO MST-18).
Here for backwards compatibility; to be removed (TODO MST-40).
"""
return {
'id': self.teamset_id,
@@ -231,7 +259,8 @@ class TeamsetConfig(object):
"""
An identifier for this team-set.
Should be a URL-slug friendly string.
Should be a URL-slug friendly string,
but for a historical reasons may contain periods and spaces.
"""
teamset_id = _clean_string(self._data.get('id'))
if not self.teamset_id_regex.match(teamset_id):
@@ -301,11 +330,11 @@ class TeamsetType(Enum):
def _clean_string(value):
"""
Return `value` if it's a string, otherwise "".
Return `str(value)` if it's a string or int, otherwise "".
"""
if not isinstance(value, six.string_types):
return ""
return value
if isinstance(value, six.integer_types + six.string_types):
return six.text_type(value)
return ""
def _clean_max_team_size(value):

20
openedx/core/lib/tests/test_teams_config.py
View File

@@ -91,14 +91,14 @@ class TeamsConfigTests(TestCase):
},
{
# Team-set should be dropped due to invalid ID.
"id": "Not a slug.",
"name": "Assignment about slugs",
"id": "the character & cannot be in an ID",
"name": "Assignment about ampersands",
},
{
# All fields invalid except ID;
# Team-set will exist but have all fallbacks.
"id": "horses",
"name": {"assignment", "about", "horses"},
"id": "_1. How quickly daft-jumping zebras vex",
"name": {"assignment", "about", "zebras"},
"description": object(),
"max_team_size": -1000,
"type": "matrix",
@@ -115,8 +115,8 @@ class TeamsConfigTests(TestCase):
"max_team_size": None,
"team_sets": [
{
"id": "horses",
"name": "horses",
"id": "_1. How quickly daft-jumping zebras vex",
"name": "_1. How quickly daft-jumping zebras vex",
"description": "",
"max_team_size": None,
"type": "open",
@@ -202,3 +202,11 @@ class TeamsConfigTests(TestCase):
assert '987' in config_repr
assert 'open' in config_repr
assert 'hedgehogs' in config_repr
def test_teamset_int_id(self):
"""
Assert integer teaset IDs are treated as strings,
for backwards compatibility.
"""
teamset = TeamsetConfig({"id": 5})
assert teamset.teamset_id == "5"