Commit 48f72612 by Peter Fogg

Prevent over-expansion of team memberships.

TNL-3202
parent 17e64535
......@@ -112,7 +112,8 @@ class MembershipSerializer(serializers.ModelSerializer):
view_name='teams_detail',
read_only=True,
),
expanded_serializer=CourseTeamSerializer(read_only=True)
expanded_serializer=CourseTeamSerializer(read_only=True),
exclude_expand_fields={'user'},
)
class Meta(object):
......
......@@ -68,8 +68,11 @@ define([
return _.map(_.range(startIndex, stopIndex + 1), function (i) {
return {
user: {
'username': testUser,
'url': 'https://openedx.example.com/api/user/v1/accounts/' + testUser
username: testUser,
url: 'https://openedx.example.com/api/user/v1/accounts/' + testUser,
profile_image: {
image_url_small: 'test_profile_image'
}
},
team: teams[i-1]
};
......
......@@ -32,11 +32,13 @@
initialize: function (options) {
this.maxTeamSize = options.maxTeamSize;
this.memberships = options.memberships;
},
render: function () {
var allMemberships = _(this.model.get('membership'))
.sortBy(function (member) {return new Date(member.last_activity_at);}).reverse(),
var allMemberships = _(this.memberships).sortBy(function (member) {
return new Date(member.last_activity_at);
}).reverse(),
displayableMemberships = allMemberships.slice(0, 5),
maxMemberCount = this.maxTeamSize;
this.$el.html(this.template({
......@@ -97,7 +99,7 @@
CardView.prototype.initialize.apply(this, arguments);
// TODO: show last activity detail view
this.detailViews = [
new TeamMembershipView({model: this.teamModel(), maxTeamSize: this.maxTeamSize}),
new TeamMembershipView({memberships: this.getMemberships(), maxTeamSize: this.maxTeamSize}),
new TeamCountryLanguageView({
model: this.teamModel(),
countries: this.countries,
......@@ -105,6 +107,9 @@
}),
new TeamActivityView({date: this.teamModel().get('last_activity_at')})
];
this.model.on('change:membership', function () {
this.detailViews[0].memberships = this.getMemberships();
}, this);
},
teamModel: function () {
......@@ -112,6 +117,15 @@
return this.model;
},
getMemberships: function () {
if (this.model.has('team')) {
return [this.model.attributes];
}
else {
return this.model.get('membership');
}
},
configuration: 'list_card',
cardClass: 'team-card',
title: function () { return this.teamModel().get('name'); },
......
......@@ -3,18 +3,24 @@
Tests for custom Teams Serializers.
"""
from django.core.paginator import Paginator
from django.test.client import RequestFactory
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import CourseFactory
from lms.djangoapps.teams.tests.factories import CourseTeamFactory
from lms.djangoapps.teams.tests.factories import CourseTeamFactory, CourseTeamMembershipFactory
from lms.djangoapps.teams.serializers import (
BaseTopicSerializer, PaginatedTopicSerializer, BulkTeamCountPaginatedTopicSerializer,
TopicSerializer, add_team_count
BaseTopicSerializer,
PaginatedTopicSerializer,
BulkTeamCountPaginatedTopicSerializer,
TopicSerializer,
MembershipSerializer,
add_team_count
)
class TopicTestCase(ModuleStoreTestCase):
class SerializerTestCase(SharedModuleStoreTestCase):
"""
Base test class to set up a course with topics
"""
......@@ -22,7 +28,7 @@ class TopicTestCase(ModuleStoreTestCase):
"""
Set up a course with a teams configuration.
"""
super(TopicTestCase, self).setUp()
super(SerializerTestCase, self).setUp()
self.course = CourseFactory.create(
teams_configuration={
"max_team_size": 10,
......@@ -31,7 +37,46 @@ class TopicTestCase(ModuleStoreTestCase):
)
class BaseTopicSerializerTestCase(TopicTestCase):
class MembershipSerializerTestCase(SerializerTestCase):
"""
Tests for the membership serializer.
"""
def setUp(self):
super(MembershipSerializerTestCase, self).setUp()
self.team = CourseTeamFactory.create(
course_id=self.course.id,
topic_id=self.course.teams_topics[0]['id']
)
self.user = UserFactory.create()
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
self.team_membership = CourseTeamMembershipFactory.create(team=self.team, user=self.user)
def test_membership_serializer_expand_user_and_team(self):
"""Verify that the serializer only expands the user and team one level."""
data = MembershipSerializer(self.team_membership, context={
'expand': [u'team', u'user'],
'request': RequestFactory().get('/api/team/v0/team_membership')
}).data
username = self.user.username
self.assertEqual(data['user'], {
'url': 'http://testserver/api/user/v1/accounts/' + username,
'username': username,
'profile_image': {
'image_url_full': 'http://testserver/static/default_500.png',
'image_url_large': 'http://testserver/static/default_120.png',
'image_url_medium': 'http://testserver/static/default_50.png',
'image_url_small': 'http://testserver/static/default_30.png',
'has_image': False
}
})
self.assertEqual(data['team']['membership'][0]['user'], {
'url': 'http://testserver/api/user/v1/accounts/' + username,
'username': username
})
class BaseTopicSerializerTestCase(SerializerTestCase):
"""
Tests for the `BaseTopicSerializer`, which should not serialize team count
data.
......@@ -46,7 +91,7 @@ class BaseTopicSerializerTestCase(TopicTestCase):
)
class TopicSerializerTestCase(TopicTestCase):
class TopicSerializerTestCase(SerializerTestCase):
"""
Tests for the `TopicSerializer`, which should serialize team count data for
a single topic.
......@@ -95,7 +140,7 @@ class TopicSerializerTestCase(TopicTestCase):
)
class BasePaginatedTopicSerializerTestCase(TopicTestCase):
class BasePaginatedTopicSerializerTestCase(SerializerTestCase):
"""
Base class for testing the two paginated topic serializers.
"""
......
......@@ -5,18 +5,26 @@ from rest_framework.serializers import CharField, Field
class ExpandableField(Field):
"""Field that can dynamically use a more detailed serializer based on a user-provided "expand" parameter."""
"""Field that can dynamically use a more detailed serializer based on a user-provided "expand" parameter.
Kwargs:
collapsed_serializer (Serializer): the serializer to use for a non-expanded representation.
expanded_serializer (Serializer): the serializer to use for an expanded representation.
exclude_expand_fields (set(str)): a set of fields which will not be expanded by sub-serializers.
"""
def __init__(self, **kwargs):
"""Sets up the ExpandableField with the collapsed and expanded versions of the serializer."""
assert 'collapsed_serializer' in kwargs and 'expanded_serializer' in kwargs
self.collapsed = kwargs.pop('collapsed_serializer')
self.expanded = kwargs.pop('expanded_serializer')
self.exclude_expand_fields = kwargs.pop('exclude_expand_fields', set())
super(ExpandableField, self).__init__(**kwargs)
def field_to_native(self, obj, field_name):
"""Converts obj to a native representation, using the expanded serializer if the context requires it."""
if 'expand' in self.context and field_name in self.context['expand']:
self.expanded.initialize(self, field_name)
self.expanded.context['expand'] = list(set(self.expanded.context['expand']) - self.exclude_expand_fields)
return self.expanded.field_to_native(obj, field_name)
else:
self.collapsed.initialize(self, field_name)
......
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