Skip to content

E
edx-platform
Commit 9f86f188 by Christina Roberts
Options

Merge pull request #9503 from edx/christina/composite-index 

Team API Performance Improvements
parents af609f10 9992ba66
Showing with 410 additions and 118 deletions
lms/djangoapps/teams/errors.py
......@@ -14,3 +14,8 @@ class NotEnrolledInCourseForTeam(TeamAPIRequestError):
class AlreadyOnTeamInCourse(TeamAPIRequestError):
"""User is already a member of another team in the same course."""
pass
class ImmutableMembershipFieldException(Exception):
"""An attempt was made to change an immutable field on a CourseTeamMembership model"""
pass
lms/djangoapps/teams/migrations/0005_add_course_id_and_topic_id_composite_index.py 0 → 100644
# -*- coding: utf-8 -*-
import pytz
from south.utils import datetime_utils as datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models
class Migration(SchemaMigration):
def forwards(self, orm):
# Create a composite index of course_id and topic_id.
db.create_index('teams_courseteam', ['course_id', 'topic_id'])
def backwards(self, orm):
# Delete the composite index of course_id and topic_id.
db.delete_index('teams_courseteam', ['course_id', 'topic_id'])
models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
},
'auth.permission': {
'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
},
'auth.user': {
'Meta': {'object_name': 'User'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
},
'contenttypes.contenttype': {
'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
},
'teams.courseteam': {
'Meta': {'object_name': 'CourseTeam'},
'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'blank': 'True'}),
'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
'date_created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
'description': ('django.db.models.fields.CharField', [], {'max_length': '300'}),
'discussion_topic_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
'language': ('student.models.LanguageField', [], {'max_length': '16', 'blank': 'True'}),
'last_activity_at': ('django.db.models.fields.DateTimeField', [], {}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
'team_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
'topic_id': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
'users': ('django.db.models.fields.related.ManyToManyField', [], {'db_index': 'True', 'related_name': "'teams'", 'symmetrical': 'False', 'through': "orm['teams.CourseTeamMembership']", 'to': "orm['auth.User']"})
},
'teams.courseteammembership': {
'Meta': {'unique_together': "(('user', 'team'),)", 'object_name': 'CourseTeamMembership'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'last_activity_at': ('django.db.models.fields.DateTimeField', [], {}),
'team': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'membership'", 'to': "orm['teams.CourseTeam']"}),
'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
}
}
complete_apps = ['teams']
lms/djangoapps/teams/migrations/0006_add_team_size.py 0 → 100644
# -*- coding: utf-8 -*-
from south.utils import datetime_utils as datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models
from teams.models import CourseTeamMembership
class Migration(SchemaMigration):
def forwards(self, orm):
# Adding field 'CourseTeam.team_size'
db.add_column('teams_courseteam', 'team_size',
self.gf('django.db.models.fields.IntegerField')(default=0, db_index=True),
keep_default=False)
# Adding index on 'CourseTeam', fields ['last_activity_at']
db.create_index('teams_courseteam', ['last_activity_at'])
if not db.dry_run:
for team in orm.CourseTeam.objects.all():
team.team_size = CourseTeamMembership.objects.filter(team=team).count()
team.save()
def backwards(self, orm):
# Removing index on 'CourseTeam', fields ['last_activity_at']
db.delete_index('teams_courseteam', ['last_activity_at'])
# Deleting field 'CourseTeam.team_size'
db.delete_column('teams_courseteam', 'team_size')
models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
},
'auth.permission': {
'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
},
'auth.user': {
'Meta': {'object_name': 'User'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
},
'contenttypes.contenttype': {
'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
},
'teams.courseteam': {
'Meta': {'object_name': 'CourseTeam'},
'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'blank': 'True'}),
'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
'date_created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
'description': ('django.db.models.fields.CharField', [], {'max_length': '300'}),
'discussion_topic_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
'language': ('student.models.LanguageField', [], {'max_length': '16', 'blank': 'True'}),
'last_activity_at': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
'team_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
'team_size': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True'}),
'topic_id': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
'users': ('django.db.models.fields.related.ManyToManyField', [], {'db_index': 'True', 'related_name': "'teams'", 'symmetrical': 'False', 'through': "orm['teams.CourseTeamMembership']", 'to': "orm['auth.User']"})
},
'teams.courseteammembership': {
'Meta': {'unique_together': "(('user', 'team'),)", 'object_name': 'CourseTeamMembership'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'last_activity_at': ('django.db.models.fields.DateTimeField', [], {}),
'team': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'membership'", 'to': "orm['teams.CourseTeam']"}),
'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
}
}
complete_apps = ['teams']
lms/djangoapps/teams/migrations/0007_auto__del_field_courseteam_is_active.py 0 → 100644
# -*- coding: utf-8 -*-
from south.utils import datetime_utils as datetime
from south.db import db
from south.v2 import SchemaMigration
from django.db import models
class Migration(SchemaMigration):
def forwards(self, orm):
# Deleting field 'CourseTeam.is_active'
db.delete_column('teams_courseteam', 'is_active')
def backwards(self, orm):
# Adding field 'CourseTeam.is_active'
db.add_column('teams_courseteam', 'is_active',
self.gf('django.db.models.fields.BooleanField')(default=True),
keep_default=False)
models = {
'auth.group': {
'Meta': {'object_name': 'Group'},
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '80'}),
'permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'})
},
'auth.permission': {
'Meta': {'ordering': "('content_type__app_label', 'content_type__model', 'codename')", 'unique_together': "(('content_type', 'codename'),)", 'object_name': 'Permission'},
'codename': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'content_type': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['contenttypes.ContentType']"}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '50'})
},
'auth.user': {
'Meta': {'object_name': 'User'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'email': ('django.db.models.fields.EmailField', [], {'max_length': '75', 'blank': 'True'}),
'first_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'groups': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Group']", 'symmetrical': 'False', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'is_active': ('django.db.models.fields.BooleanField', [], {'default': 'True'}),
'is_staff': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'is_superuser': ('django.db.models.fields.BooleanField', [], {'default': 'False'}),
'last_login': ('django.db.models.fields.DateTimeField', [], {'default': 'datetime.datetime.now'}),
'last_name': ('django.db.models.fields.CharField', [], {'max_length': '30', 'blank': 'True'}),
'password': ('django.db.models.fields.CharField', [], {'max_length': '128'}),
'user_permissions': ('django.db.models.fields.related.ManyToManyField', [], {'to': "orm['auth.Permission']", 'symmetrical': 'False', 'blank': 'True'}),
'username': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '30'})
},
'contenttypes.contenttype': {
'Meta': {'ordering': "('name',)", 'unique_together': "(('app_label', 'model'),)", 'object_name': 'ContentType', 'db_table': "'django_content_type'"},
'app_label': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'model': ('django.db.models.fields.CharField', [], {'max_length': '100'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '100'})
},
'teams.courseteam': {
'Meta': {'object_name': 'CourseTeam'},
'country': ('django_countries.fields.CountryField', [], {'max_length': '2', 'blank': 'True'}),
'course_id': ('xmodule_django.models.CourseKeyField', [], {'max_length': '255', 'db_index': 'True'}),
'date_created': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
'description': ('django.db.models.fields.CharField', [], {'max_length': '300'}),
'discussion_topic_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'language': ('student.models.LanguageField', [], {'max_length': '16', 'blank': 'True'}),
'last_activity_at': ('django.db.models.fields.DateTimeField', [], {'db_index': 'True'}),
'name': ('django.db.models.fields.CharField', [], {'max_length': '255', 'db_index': 'True'}),
'team_id': ('django.db.models.fields.CharField', [], {'unique': 'True', 'max_length': '255'}),
'team_size': ('django.db.models.fields.IntegerField', [], {'default': '0', 'db_index': 'True'}),
'topic_id': ('django.db.models.fields.CharField', [], {'db_index': 'True', 'max_length': '255', 'blank': 'True'}),
'users': ('django.db.models.fields.related.ManyToManyField', [], {'db_index': 'True', 'related_name': "'teams'", 'symmetrical': 'False', 'through': "orm['teams.CourseTeamMembership']", 'to': "orm['auth.User']"})
},
'teams.courseteammembership': {
'Meta': {'unique_together': "(('user', 'team'),)", 'object_name': 'CourseTeamMembership'},
'date_joined': ('django.db.models.fields.DateTimeField', [], {'auto_now_add': 'True', 'blank': 'True'}),
'id': ('django.db.models.fields.AutoField', [], {'primary_key': 'True'}),
'last_activity_at': ('django.db.models.fields.DateTimeField', [], {}),
'team': ('django.db.models.fields.related.ForeignKey', [], {'related_name': "'membership'", 'to': "orm['teams.CourseTeam']"}),
'user': ('django.db.models.fields.related.ForeignKey', [], {'to': "orm['auth.User']"})
}
}
complete_apps = ['teams']
\ No newline at end of file
lms/djangoapps/teams/models.py
......@@ -26,7 +26,7 @@ from django_comment_common.signals import (
from xmodule_django.models import CourseKeyField
from util.model_utils import slugify
from student.models import LanguageField, CourseEnrollment
from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam
from .errors import AlreadyOnTeamInCourse, NotEnrolledInCourseForTeam, ImmutableMembershipFieldException
from teams import TEAM_DISCUSSION_CONTEXT
......@@ -76,7 +76,6 @@ class CourseTeam(models.Model):
team_id = models.CharField(max_length=255, unique=True)
discussion_topic_id = models.CharField(max_length=255, unique=True)
name = models.CharField(max_length=255, db_index=True)
is_active = models.BooleanField(default=True)
course_id = CourseKeyField(max_length=255, db_index=True)
topic_id = models.CharField(max_length=255, db_index=True, blank=True)
date_created = models.DateTimeField(auto_now_add=True)
......@@ -86,8 +85,9 @@ class CourseTeam(models.Model):
blank=True,
help_text=ugettext_lazy("Optional language the team uses as ISO 639-1 code."),
)
last_activity_at = models.DateTimeField()
last_activity_at = models.DateTimeField(db_index=True) # indexed for ordering
users = models.ManyToManyField(User, db_index=True, related_name='teams', through='CourseTeamMembership')
team_size = models.IntegerField(default=0, db_index=True) # indexed for ordering
@classmethod
def create(cls, name, course_id, description, topic_id=None, country=None, language=None):
......@@ -135,6 +135,11 @@ class CourseTeam(models.Model):
team=self
)
def reset_team_size(self):
"""Reset team_size to reflect the current membership count."""
self.team_size = CourseTeamMembership.objects.filter(team=self).count()
self.save()
class CourseTeamMembership(models.Model):
"""This model represents the membership of a single user in a single team."""
......@@ -148,12 +153,40 @@ class CourseTeamMembership(models.Model):
date_joined = models.DateTimeField(auto_now_add=True)
last_activity_at = models.DateTimeField()
immutable_fields = ('user', 'team', 'date_joined')
def __setattr__(self, name, value):
"""Memberships are immutable, with the exception of last activity
date.
"""
if name in self.immutable_fields:
# Check the current value -- if it is None, then this
# model is being created from the database and it's fine
# to set the value. Otherwise, we're trying to overwrite
# an immutable field.
current_value = getattr(self, name, None)
if current_value is not None:
raise ImmutableMembershipFieldException
super(CourseTeamMembership, self).__setattr__(name, value)
def save(self, *args, **kwargs):
""" Customize save method to set the last_activity_at if it does not currently exist. """
"""Customize save method to set the last_activity_at if it does not
currently exist. Also resets the team's size if this model is
being created.
"""
should_reset_team_size = False
if self.pk is None:
should_reset_team_size = True
if not self.last_activity_at:
self.last_activity_at = datetime.utcnow().replace(tzinfo=pytz.utc)
super(CourseTeamMembership, self).save(*args, **kwargs)
if should_reset_team_size:
self.team.reset_team_size() # pylint: disable=no-member
def delete(self, *args, **kwargs):
"""Recompute the related team's team_size after deleting a membership"""
super(CourseTeamMembership, self).delete(*args, **kwargs)
self.team.reset_team_size() # pylint: disable=no-member
@classmethod
def get_memberships(cls, username=None, course_ids=None, team_id=None):
......
lms/djangoapps/teams/serializers.py
......@@ -51,7 +51,6 @@ class CourseTeamSerializer(serializers.ModelSerializer):
"id",
"discussion_topic_id",
"name",
"is_active",
"course_id",
"topic_id",
"date_created",
......
lms/djangoapps/teams/static/teams/js/models/team.js
......@@ -8,7 +8,6 @@
defaults: {
id: null,
name: '',
is_active: null,
course_id: '',
topic_id: '',
date_created: '',
......
lms/djangoapps/teams/static/teams/js/spec/views/edit_team_spec.js
......@@ -14,7 +14,6 @@ define([
createTeamData = {
id: null,
name: "TeamName",
is_active: null,
course_id: "a/b/c",
topic_id: "awesomeness",
date_created: "",
......
lms/djangoapps/teams/static/teams/js/spec_helpers/team_spec_helpers.js
......@@ -32,7 +32,6 @@ define([
id: "id " + i,
language: testLanguages[i%4][0],
country: testCountries[i%4][0],
is_active: true,
membership: [],
last_activity_at: ''
};
......
lms/djangoapps/teams/tests/test_models.py
# -*- coding: utf-8 -*-
# pylint: disable=no-member
"""Tests for the teams API at the HTTP request level."""
from contextlib import contextmanager
from datetime import datetime
......@@ -20,7 +21,7 @@ from django_comment_common.signals import (
)
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from opaque_keys.edx.keys import CourseKey
from student.tests.factories import UserFactory
from student.tests.factories import CourseEnrollmentFactory, UserFactory
from .factories import CourseTeamFactory, CourseTeamMembershipFactory
from ..models import CourseTeam, CourseTeamMembership
......@@ -42,16 +43,18 @@ class TeamMembershipTest(SharedModuleStoreTestCase):
self.user1 = UserFactory.create(username='user1')
self.user2 = UserFactory.create(username='user2')
self.user3 = UserFactory.create(username='user3')
for user in (self.user1, self.user2, self.user3):
CourseEnrollmentFactory.create(user=user, course_id=COURSE_KEY1)
CourseEnrollmentFactory.create(user=self.user1, course_id=COURSE_KEY2)
self.team1 = CourseTeamFactory(course_id=COURSE_KEY1, team_id='team1')
self.team2 = CourseTeamFactory(course_id=COURSE_KEY2, team_id='team2')
self.team_membership11 = CourseTeamMembership(user=self.user1, team=self.team1)
self.team_membership11.save()
self.team_membership12 = CourseTeamMembership(user=self.user2, team=self.team1)
self.team_membership12.save()
self.team_membership21 = CourseTeamMembership(user=self.user1, team=self.team2)
self.team_membership21.save()
self.team_membership11 = self.team1.add_user(self.user1)
self.team_membership12 = self.team1.add_user(self.user2)
self.team_membership21 = self.team2.add_user(self.user1)
def test_membership_last_activity_set(self):
current_last_activity = self.team_membership11.last_activity_at
......@@ -64,6 +67,24 @@ class TeamMembershipTest(SharedModuleStoreTestCase):
# already exist.
self.assertEqual(self.team_membership11.last_activity_at, current_last_activity)
def test_team_size_delete_membership(self):
"""Test that the team size field is correctly updated when deleting a
team membership.
"""
self.assertEqual(self.team1.team_size, 2)
self.team_membership11.delete()
team = CourseTeam.objects.get(id=self.team1.id)
self.assertEqual(team.team_size, 1)
def test_team_size_create_membership(self):
"""Test that the team size field is correctly updated when creating a
team membership.
"""
self.assertEqual(self.team1.team_size, 2)
self.team1.add_user(self.user3)
team = CourseTeam.objects.get(id=self.team1.id)
self.assertEqual(team.team_size, 3)
@ddt.data(
(None, None, None, 3),
('user1', None, None, 2),
......
lms/djangoapps/teams/tests/test_views.py
......@@ -122,7 +122,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
'id': 'topic_{}'.format(i),
'name': name,
'description': 'Description for topic {}.'.format(i)
} for i, name in enumerate([u'sólar power', 'Wind Power', 'Nuclear Power', 'Coal Power'])
} for i, name in enumerate([u'Sólar power', 'Wind Power', 'Nuclear Power', 'Coal Power'])
]
}
cls.test_course_1 = CourseFactory.create(
......@@ -201,26 +201,20 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
sender=CourseTeam,
dispatch_uid='teams.signals.course_team_post_save_callback'
):
# 'solar team' is intentionally lower case to test case insensitivity in name ordering
self.test_team_1 = CourseTeamFactory.create(
name=u'sólar team',
self.solar_team = CourseTeamFactory.create(
name=u'Sólar team',
course_id=self.test_course_1.id,
topic_id='topic_0'
)
self.test_team_2 = CourseTeamFactory.create(name='Wind Team', course_id=self.test_course_1.id)
self.test_team_3 = CourseTeamFactory.create(name='Nuclear Team', course_id=self.test_course_1.id)
self.test_team_4 = CourseTeamFactory.create(
name='Coal Team',
course_id=self.test_course_1.id,
is_active=False
)
self.test_team_5 = CourseTeamFactory.create(name='Another Team', course_id=self.test_course_2.id)
self.test_team_6 = CourseTeamFactory.create(
self.wind_team = CourseTeamFactory.create(name='Wind Team', course_id=self.test_course_1.id)
self.nuclear_team = CourseTeamFactory.create(name='Nuclear Team', course_id=self.test_course_1.id)
self.another_team = CourseTeamFactory.create(name='Another Team', course_id=self.test_course_2.id)
self.public_profile_team = CourseTeamFactory.create(
name='Public Profile Team',
course_id=self.test_course_2.id,
topic_id='topic_6'
)
self.test_team_7 = CourseTeamFactory.create(
self.search_team = CourseTeamFactory.create(
name='Search',
description='queryable text',
country='GS',
......@@ -230,13 +224,12 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
)
self.test_team_name_id_map = {team.name: team for team in (
self.test_team_1,
self.test_team_2,
self.test_team_3,
self.test_team_4,
self.test_team_5,
self.test_team_6,
self.test_team_7,
self.solar_team,
self.wind_team,
self.nuclear_team,
self.another_team,
self.public_profile_team,
self.search_team,
)}
for user, course in [('staff', self.test_course_1), ('course_staff', self.test_course_1)]:
......@@ -244,10 +237,10 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
self.users[user], course.id, check_access=True
)
self.test_team_1.add_user(self.users['student_enrolled'])
self.test_team_3.add_user(self.users['student_enrolled_both_courses_other_team'])
self.test_team_5.add_user(self.users['student_enrolled_both_courses_other_team'])
self.test_team_6.add_user(self.users['student_enrolled_public_profile'])
self.solar_team.add_user(self.users['student_enrolled'])
self.nuclear_team.add_user(self.users['student_enrolled_both_courses_other_team'])
self.another_team.add_user(self.users['student_enrolled_both_courses_other_team'])
self.public_profile_team.add_user(self.users['student_enrolled_public_profile'])
def build_membership_data_raw(self, username, team):
"""Assembles a membership creation payload based on the raw values provided."""
......@@ -401,7 +394,7 @@ class TeamAPITestCase(APITestCase, SharedModuleStoreTestCase):
def verify_expanded_team(self, team):
"""Verifies that fields exist on the returned team json indicating that it is expanded."""
for field in ['id', 'name', 'is_active', 'course_id', 'topic_id', 'date_created', 'description']:
for field in ['id', 'name', 'course_id', 'topic_id', 'date_created', 'description']:
self.assertIn(field, team)
......@@ -445,24 +438,21 @@ class TestListTeamsAPI(TeamAPITestCase):
)
def test_filter_topic_id(self):
self.verify_names({'course_id': self.test_course_1.id, 'topic_id': 'topic_0'}, 200, [u'sólar team'])
def test_filter_include_inactive(self):
self.verify_names({'include_inactive': True}, 200, ['Coal Team', 'Nuclear Team', u'sólar team', 'Wind Team'])
self.verify_names({'course_id': self.test_course_1.id, 'topic_id': 'topic_0'}, 200, [u'Sólar team'])
@ddt.data(
(None, 200, ['Nuclear Team', u'sólar team', 'Wind Team']),
('name', 200, ['Nuclear Team', u'sólar team', 'Wind Team']),
# Note that "Nuclear Team" and "solar team" have the same open_slots.
# "solar team" comes first due to secondary sort by last_activity_at.
('open_slots', 200, ['Wind Team', u'sólar team', 'Nuclear Team']),
(None, 200, ['Nuclear Team', u'Sólar team', 'Wind Team']),
('name', 200, ['Nuclear Team', u'Sólar team', 'Wind Team']),
# Note that "Nuclear Team" and "Solar team" have the same open_slots.
# "Solar team" comes first due to secondary sort by last_activity_at.
('open_slots', 200, ['Wind Team', u'Sólar team', 'Nuclear Team']),
# Note that "Wind Team" and "Nuclear Team" have the same last_activity_at.
# "Wind Team" comes first due to secondary sort by open_slots.
('last_activity_at', 200, [u'sólar team', 'Wind Team', 'Nuclear Team']),
('last_activity_at', 200, [u'Sólar team', 'Wind Team', 'Nuclear Team']),
)
@ddt.unpack
def test_order_by(self, field, status, names):
# Make "solar team" the most recently active team.
# Make "Solar team" the most recently active team.
# The CourseTeamFactory sets the last_activity_at to a fixed time (in the past), so all of the
# other teams have the same last_activity_at.
with skip_signal(
......@@ -471,7 +461,7 @@ class TestListTeamsAPI(TeamAPITestCase):
sender=CourseTeam,
dispatch_uid='teams.signals.course_team_post_save_callback'
):
solar_team = self.test_team_name_id_map[u'sólar team']
solar_team = self.test_team_name_id_map[u'Sólar team']
solar_team.last_activity_at = datetime.utcnow().replace(tzinfo=pytz.utc)
solar_team.save()
......@@ -612,7 +602,7 @@ class TestCreateTeamAPI(TeamAPITestCase):
# First add the privileged user to a team.
self.post_create_membership(
200,
self.build_membership_data(user, self.test_team_1),
self.build_membership_data(user, self.solar_team),
user=user
)
......@@ -674,7 +664,6 @@ class TestCreateTeamAPI(TeamAPITestCase):
'name': 'Fully specified team',
'language': 'fr',
'country': 'CA',
'is_active': True,
'topic_id': 'great-topic',
'course_id': str(self.test_course_1.id),
'description': 'Another fantastic team'
......@@ -708,10 +697,10 @@ class TestDetailTeamAPI(TeamAPITestCase):
)
@ddt.unpack
def test_access(self, user, status):
team = self.get_team_detail(self.test_team_1.team_id, status, user=user)
team = self.get_team_detail(self.solar_team.team_id, status, user=user)
if status == 200:
self.assertEqual(team['description'], self.test_team_1.description)
self.assertEqual(team['discussion_topic_id'], self.test_team_1.discussion_topic_id)
self.assertEqual(team['description'], self.solar_team.description)
self.assertEqual(team['discussion_topic_id'], self.solar_team.discussion_topic_id)
self.assertEqual(parser.parse(team['last_activity_at']), LAST_ACTIVITY_AT)
def test_does_not_exist(self):
......@@ -719,12 +708,12 @@ class TestDetailTeamAPI(TeamAPITestCase):
def test_expand_private_user(self):
# Use the default user which is already private because to year_of_birth is set
result = self.get_team_detail(self.test_team_1.team_id, 200, {'expand': 'user'})
result = self.get_team_detail(self.solar_team.team_id, 200, {'expand': 'user'})
self.verify_expanded_private_user(result['membership'][0]['user'])
def test_expand_public_user(self):
result = self.get_team_detail(
self.test_team_6.team_id,
self.public_profile_team.team_id,
200,
{'expand': 'user'},
user='student_enrolled_public_profile'
......@@ -747,7 +736,7 @@ class TestUpdateTeamAPI(TeamAPITestCase):
)
@ddt.unpack
def test_access(self, user, status):
team = self.patch_team_detail(self.test_team_1.team_id, status, {'name': 'foo'}, user=user)
team = self.patch_team_detail(self.solar_team.team_id, status, {'name': 'foo'}, user=user)
if status == 200:
self.assertEquals(team['name'], 'foo')
......@@ -772,12 +761,12 @@ class TestUpdateTeamAPI(TeamAPITestCase):
)
@ddt.unpack
def test_bad_requests(self, key, value):
self.patch_team_detail(self.test_team_1.team_id, 400, {key: value}, user='staff')
self.patch_team_detail(self.solar_team.team_id, 400, {key: value}, user='staff')
@ddt.data(('country', 'US'), ('language', 'en'), ('foo', 'bar'))
@ddt.unpack
def test_good_requests(self, key, value):
self.patch_team_detail(self.test_team_1.team_id, 200, {key: value}, user='staff')
self.patch_team_detail(self.solar_team.team_id, 200, {key: value}, user='staff')
def test_does_not_exist(self):
self.patch_team_detail('no_such_team', 404, user='staff')
......@@ -810,11 +799,11 @@ class TestListTopicsAPI(TeamAPITestCase):
self.get_topics_list(400)
@ddt.data(
(None, 200, ['Coal Power', 'Nuclear Power', u'sólar power', 'Wind Power'], 'name'),
('name', 200, ['Coal Power', 'Nuclear Power', u'sólar power', 'Wind Power'], 'name'),
# Note that "Nuclear Power" and "solar power" both have 2 teams. "Coal Power" and "Window Power"
(None, 200, ['Coal Power', 'Nuclear Power', u'Sólar power', 'Wind Power'], 'name'),
('name', 200, ['Coal Power', 'Nuclear Power', u'Sólar power', 'Wind Power'], 'name'),
# Note that "Nuclear Power" and "Solar power" both have 2 teams. "Coal Power" and "Window Power"
# both have 0 teams. The secondary sort is alphabetical by name.
('team_count', 200, ['Nuclear Power', u'sólar power', 'Coal Power', 'Wind Power'], 'team_count'),
('team_count', 200, ['Nuclear Power', u'Sólar power', 'Coal Power', 'Wind Power'], 'team_count'),
('no_such_field', 400, [], None),
)
@ddt.unpack
......@@ -865,7 +854,7 @@ class TestListTopicsAPI(TeamAPITestCase):
'page': 1,
'order_by': 'team_count'
})
self.assertEqual(["Wind Power", u'sólar power'], [topic['name'] for topic in topics['results']])
self.assertEqual(["Wind Power", u'Sólar power'], [topic['name'] for topic in topics['results']])
topics = self.get_topics_list(data={
'course_id': self.test_course_1.id,
......@@ -953,7 +942,7 @@ class TestListMembershipAPI(TeamAPITestCase):
)
@ddt.unpack
def test_access(self, user, status):
membership = self.get_membership_list(status, {'team_id': self.test_team_1.team_id}, user=user)
membership = self.get_membership_list(status, {'team_id': self.solar_team.team_id}, user=user)
if status == 200:
self.assertEqual(membership['count'], 1)
self.assertEqual(membership['results'][0]['user']['username'], self.users['student_enrolled'].username)
......@@ -974,14 +963,14 @@ class TestListMembershipAPI(TeamAPITestCase):
if status == 200:
if has_content:
self.assertEqual(membership['count'], 1)
self.assertEqual(membership['results'][0]['team']['team_id'], self.test_team_1.team_id)
self.assertEqual(membership['results'][0]['team']['team_id'], self.solar_team.team_id)
else:
self.assertEqual(membership['count'], 0)
@ddt.data(
('student_enrolled_both_courses_other_team', 'TestX/TS101/Test_Course', 200, 'Nuclear Team'),
('student_enrolled_both_courses_other_team', 'MIT/6.002x/Circuits', 200, 'Another Team'),
('student_enrolled', 'TestX/TS101/Test_Course', 200, u'sólar team'),
('student_enrolled', 'TestX/TS101/Test_Course', 200, u'Sólar team'),
('student_enrolled', 'MIT/6.002x/Circuits', 400, ''),
)
@ddt.unpack
......@@ -1004,10 +993,10 @@ class TestListMembershipAPI(TeamAPITestCase):
)
@ddt.unpack
def test_course_filter_with_team_id(self, course_id, status):
membership = self.get_membership_list(status, {'team_id': self.test_team_1.team_id, 'course_id': course_id})
membership = self.get_membership_list(status, {'team_id': self.solar_team.team_id, 'course_id': course_id})
if status == 200:
self.assertEqual(membership['count'], 1)
self.assertEqual(membership['results'][0]['team']['team_id'], self.test_team_1.team_id)
self.assertEqual(membership['results'][0]['team']['team_id'], self.solar_team.team_id)
def test_bad_course_id(self):
self.get_membership_list(404, {'course_id': 'no_such_course'})
......@@ -1020,19 +1009,19 @@ class TestListMembershipAPI(TeamAPITestCase):
def test_expand_private_user(self):
# Use the default user which is already private because to year_of_birth is set
result = self.get_membership_list(200, {'team_id': self.test_team_1.team_id, 'expand': 'user'})
result = self.get_membership_list(200, {'team_id': self.solar_team.team_id, 'expand': 'user'})
self.verify_expanded_private_user(result['results'][0]['user'])
def test_expand_public_user(self):
result = self.get_membership_list(
200,
{'team_id': self.test_team_6.team_id, 'expand': 'user'},
{'team_id': self.public_profile_team.team_id, 'expand': 'user'},
user='student_enrolled_public_profile'
)
self.verify_expanded_public_user(result['results'][0]['user'])
def test_expand_team(self):
result = self.get_membership_list(200, {'team_id': self.test_team_1.team_id, 'expand': 'team'})
result = self.get_membership_list(200, {'team_id': self.solar_team.team_id, 'expand': 'team'})
self.verify_expanded_team(result['results'][0]['team'])
......@@ -1055,17 +1044,17 @@ class TestCreateMembershipAPI(TeamAPITestCase):
def test_access(self, user, status):
membership = self.post_create_membership(
status,
self.build_membership_data('student_enrolled_not_on_team', self.test_team_1),
self.build_membership_data('student_enrolled_not_on_team', self.solar_team),
user=user
)
if status == 200:
self.assertEqual(membership['user']['username'], self.users['student_enrolled_not_on_team'].username)
self.assertEqual(membership['team']['team_id'], self.test_team_1.team_id)
memberships = self.get_membership_list(200, {'team_id': self.test_team_1.team_id})
self.assertEqual(membership['team']['team_id'], self.solar_team.team_id)
memberships = self.get_membership_list(200, {'team_id': self.solar_team.team_id})
self.assertEqual(memberships['count'], 2)
def test_no_username(self):
response = self.post_create_membership(400, {'team_id': self.test_team_1.team_id})
response = self.post_create_membership(400, {'team_id': self.solar_team.team_id})
self.assertIn('username', json.loads(response.content)['field_errors'])
def test_no_team(self):
......@@ -1081,7 +1070,7 @@ class TestCreateMembershipAPI(TeamAPITestCase):
def test_bad_username(self):
self.post_create_membership(
404,
self.build_membership_data_raw('no_such_user', self.test_team_1.team_id),
self.build_membership_data_raw('no_such_user', self.solar_team.team_id),
user='staff'
)
......@@ -1089,7 +1078,7 @@ class TestCreateMembershipAPI(TeamAPITestCase):
def test_join_twice(self, user):
response = self.post_create_membership(
400,
self.build_membership_data('student_enrolled', self.test_team_1),
self.build_membership_data('student_enrolled', self.solar_team),
user=user
)
self.assertIn('already a member', json.loads(response.content)['developer_message'])
......@@ -1097,7 +1086,7 @@ class TestCreateMembershipAPI(TeamAPITestCase):
def test_join_second_team_in_course(self):
response = self.post_create_membership(
400,
self.build_membership_data('student_enrolled_both_courses_other_team', self.test_team_1),
self.build_membership_data('student_enrolled_both_courses_other_team', self.solar_team),
user='student_enrolled_both_courses_other_team'
)
self.assertIn('already a member', json.loads(response.content)['developer_message'])
......@@ -1106,7 +1095,7 @@ class TestCreateMembershipAPI(TeamAPITestCase):
def test_not_enrolled_in_team_course(self, user):
response = self.post_create_membership(
400,
self.build_membership_data('student_unenrolled', self.test_team_1),
self.build_membership_data('student_unenrolled', self.solar_team),
user=user
)
self.assertIn('not enrolled', json.loads(response.content)['developer_message'])
......@@ -1114,7 +1103,7 @@ class TestCreateMembershipAPI(TeamAPITestCase):
def test_over_max_team_size_in_course_2(self):
response = self.post_create_membership(
400,
self.build_membership_data('student_enrolled_other_course_not_on_team', self.test_team_5),
self.build_membership_data('student_enrolled_other_course_not_on_team', self.another_team),
user='student_enrolled_other_course_not_on_team'
)
self.assertIn('full', json.loads(response.content)['developer_message'])
......@@ -1137,7 +1126,7 @@ class TestDetailMembershipAPI(TeamAPITestCase):
@ddt.unpack
def test_access(self, user, status):
self.get_membership_detail(
self.test_team_1.team_id,
self.solar_team.team_id,
self.users['student_enrolled'].username,
status,
user=user
......@@ -1147,11 +1136,11 @@ class TestDetailMembershipAPI(TeamAPITestCase):
self.get_membership_detail('no_such_team', self.users['student_enrolled'].username, 404)
def test_bad_username(self):
self.get_membership_detail(self.test_team_1.team_id, 'no_such_user', 404)
self.get_membership_detail(self.solar_team.team_id, 'no_such_user', 404)
def test_no_membership(self):
self.get_membership_detail(
self.test_team_1.team_id,
self.solar_team.team_id,
self.users['student_enrolled_not_on_team'].username,
404
)
......@@ -1159,7 +1148,7 @@ class TestDetailMembershipAPI(TeamAPITestCase):
def test_expand_private_user(self):
# Use the default user which is already private because to year_of_birth is set
result = self.get_membership_detail(
self.test_team_1.team_id,
self.solar_team.team_id,
self.users['student_enrolled'].username,
200,
{'expand': 'user'}
......@@ -1168,7 +1157,7 @@ class TestDetailMembershipAPI(TeamAPITestCase):
def test_expand_public_user(self):
result = self.get_membership_detail(
self.test_team_6.team_id,
self.public_profile_team.team_id,
self.users['student_enrolled_public_profile'].username,
200,
{'expand': 'user'},
......@@ -1178,7 +1167,7 @@ class TestDetailMembershipAPI(TeamAPITestCase):
def test_expand_team(self):
result = self.get_membership_detail(
self.test_team_1.team_id,
self.solar_team.team_id,
self.users['student_enrolled'].username,
200,
{'expand': 'team'}
......@@ -1203,7 +1192,7 @@ class TestDeleteMembershipAPI(TeamAPITestCase):
@ddt.unpack
def test_access(self, user, status):
self.delete_membership(
self.test_team_1.team_id,
self.solar_team.team_id,
self.users['student_enrolled'].username,
status,
user=user
......@@ -1213,7 +1202,7 @@ class TestDeleteMembershipAPI(TeamAPITestCase):
self.delete_membership('no_such_team', self.users['student_enrolled'].username, 404)
def test_bad_username(self):
self.delete_membership(self.test_team_1.team_id, 'no_such_user', 404)
self.delete_membership(self.solar_team.team_id, 'no_such_user', 404)
def test_missing_membership(self):
self.delete_membership(self.test_team_2.team_id, self.users['student_enrolled'].username, 404)
self.delete_membership(self.wind_team.team_id, self.users['student_enrolled'].username, 404)
lms/djangoapps/teams/views.py
......@@ -191,9 +191,6 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
* page: Page number to retrieve.
* include_inactive: If true, inactive teams will be returned. The
default is to not include inactive teams.
* expand: Comma separated list of types for which to return
expanded representations. Supports "user" and "team".
......@@ -220,10 +217,6 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
* name: The name of the team.
* is_active: True if the team is currently active. If false, the
team is considered "soft deleted" and will not be included by
default in results.
* course_id: The identifier for the course this team belongs to.
* topic_id: Optionally specifies which topic the team is associated
......@@ -266,8 +259,8 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
Any logged in user who has verified their email address can create
a team. The format mirrors that of a GET for an individual team,
but does not include the id, is_active, date_created, or membership
fields. id is automatically computed based on name.
but does not include the id, date_created, or membership fields.
id is automatically computed based on name.
If the user is not logged in, a 401 error is returned.
......@@ -292,9 +285,7 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
def get(self, request):
"""GET /api/team/v0/teams/"""
result_filter = {
'is_active': True
}
result_filter = {}
if 'course_id' in request.QUERY_PARAMS:
course_id_string = request.QUERY_PARAMS['course_id']
......@@ -335,8 +326,6 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
)
return Response(error, status=status.HTTP_400_BAD_REQUEST)
result_filter.update({'topic_id': request.QUERY_PARAMS['topic_id']})
if 'include_inactive' in request.QUERY_PARAMS and request.QUERY_PARAMS['include_inactive'].lower() == 'true':
del result_filter['is_active']
if 'text_search' in request.QUERY_PARAMS and CourseTeamIndexer.search_is_enabled():
search_engine = CourseTeamIndexer.engine()
......@@ -355,19 +344,16 @@ class TeamsListView(ExpandableFieldViewMixin, GenericAPIView):
self.get_paginate_by(),
self.get_page()
)
serializer = self.get_pagination_serializer(paginated_results)
else:
queryset = CourseTeam.objects.filter(**result_filter)
order_by_input = request.QUERY_PARAMS.get('order_by', 'name')
if order_by_input == 'name':
queryset = queryset.extra(select={'lower_name': "lower(name)"})
queryset = queryset.order_by('lower_name')
# MySQL does case-insensitive order_by.
queryset = queryset.order_by('name')
elif order_by_input == 'open_slots':
queryset = queryset.annotate(team_size=Count('users'))
queryset = queryset.order_by('team_size', '-last_activity_at')
elif order_by_input == 'last_activity_at':
queryset = queryset.annotate(team_size=Count('users'))
queryset = queryset.order_by('-last_activity_at', 'team_size')
else:
return Response({
......@@ -496,10 +482,6 @@ class TeamsDetailView(ExpandableFieldViewMixin, RetrievePatchAPIView):
* name: The name of the team.
* is_active: True if the team is currently active. If false, the team
is considered "soft deleted" and will not be included by default in
results.
* course_id: The identifier for the course this team belongs to.
* topic_id: Optionally specifies which topic the team is
......
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