From db6add1c59c5051af7e5574eccd4053c77955ec1 Mon Sep 17 00:00:00 2001 From: Daniel Friedman Date: Tue, 28 Jul 2015 10:42:57 -0400 Subject: [PATCH] Fix team count in topic serializers TNL-2892 --- lms/djangoapps/teams/serializers.py | 13 +++++-- .../teams/tests/test_serializers.py | 38 +++++++++++++++++-- lms/djangoapps/teams/views.py | 9 +++-- 3 files changed, 51 insertions(+), 9 deletions(-) diff --git a/lms/djangoapps/teams/serializers.py b/lms/djangoapps/teams/serializers.py index 486d6914c7..b0544fdb70 100644 --- a/lms/djangoapps/teams/serializers.py +++ b/lms/djangoapps/teams/serializers.py @@ -123,17 +123,23 @@ class TopicSerializer(BaseTopicSerializer): """ Adds team_count to the basic topic serializer. Use only when serializing a single topic. When serializing many topics, use - `PaginatedTopicSerializer` to avoid O(N) SQL queries. + `PaginatedTopicSerializer` to avoid O(N) SQL queries. Requires + that `context` is provided with a valid course_id in order to + filter teams within the course. """ team_count = serializers.SerializerMethodField('get_team_count') def get_team_count(self, topic): """Get the number of teams associated with this topic""" - return CourseTeam.objects.filter(topic_id=topic['id']).count() + return CourseTeam.objects.filter(course_id=self.context['course_id'], topic_id=topic['id']).count() class PaginatedTopicSerializer(PaginationSerializer): - """Serializes a set of topics. Adds team_count field to each topic.""" + """ + Serializes a set of topics, adding team_count field to each topic. + Requires that `context` is provided with a valid course_id in + order to filter teams within the course. + """ class Meta(object): """Defines meta information for the PaginatedTopicSerializer.""" object_serializer_class = BaseTopicSerializer @@ -146,6 +152,7 @@ class PaginatedTopicSerializer(PaginationSerializer): # and outputs the result as a list of dicts (one per topic). topic_ids = [topic['id'] for topic in self.data['results']] teams_per_topic = CourseTeam.objects.filter( + course_id=self.context['course_id'], topic_id__in=topic_ids ).values('topic_id').annotate(team_count=Count('topic_id')) diff --git a/lms/djangoapps/teams/tests/test_serializers.py b/lms/djangoapps/teams/tests/test_serializers.py index 56c3c82cb3..69fa07b14a 100644 --- a/lms/djangoapps/teams/tests/test_serializers.py +++ b/lms/djangoapps/teams/tests/test_serializers.py @@ -54,7 +54,7 @@ class TopicSerializerTestCase(TopicTestCase): team count of 0, and that it only takes one SQL query. """ with self.assertNumQueries(1): - serializer = TopicSerializer(self.course.teams_topics[0]) + serializer = TopicSerializer(self.course.teams_topics[0], 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} @@ -67,7 +67,25 @@ class TopicSerializerTestCase(TopicTestCase): """ CourseTeamFactory.create(course_id=self.course.id, topic_id=self.course.teams_topics[0]['id']) with self.assertNumQueries(1): - serializer = TopicSerializer(self.course.teams_topics[0]) + serializer = TopicSerializer(self.course.teams_topics[0], 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} + ) + + def test_scoped_within_course(self): + """Verify that team count is scoped within a course.""" + duplicate_topic = self.course.teams_topics[0] + second_course = CourseFactory.create( + teams_configuration={ + "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}) self.assertEqual( serializer.data, {u'name': u'Tøpic', u'description': u'The bést topic!', u'id': u'0', u'team_count': 1} @@ -110,7 +128,7 @@ class PaginatedTopicSerializerTestCase(TopicTestCase): """ with self.assertNumQueries(num_queries): page = Paginator(self.course.teams_topics, self.PAGE_SIZE).page(1) - serializer = PaginatedTopicSerializer(instance=page) + serializer = PaginatedTopicSerializer(instance=page, context={'course_id': self.course.id}) self.assertEqual( serializer.data['results'], [self._merge_dicts(topic, {u'team_count': num_teams_per_topic}) for topic in topics] @@ -149,3 +167,17 @@ class PaginatedTopicSerializerTestCase(TopicTestCase): teams_per_topic = 10 topics = self.setup_topics(num_topics=self.PAGE_SIZE + 1, teams_per_topic=teams_per_topic) self.assert_serializer_output(topics[:self.PAGE_SIZE], num_teams_per_topic=teams_per_topic, num_queries=1) + + def test_scoped_within_course(self): + """Verify that team counts are scoped within a course.""" + teams_per_topic = 10 + first_course_topics = self.setup_topics(num_topics=self.PAGE_SIZE, teams_per_topic=teams_per_topic) + duplicate_topic = first_course_topics[0] + second_course = CourseFactory.create( + teams_configuration={ + "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) diff --git a/lms/djangoapps/teams/views.py b/lms/djangoapps/teams/views.py index de4974c527..fe443da852 100644 --- a/lms/djangoapps/teams/views.py +++ b/lms/djangoapps/teams/views.py @@ -82,7 +82,10 @@ class TeamsDashboardView(View): sort_order = 'name' topics = get_ordered_topics(course, sort_order) topics_page = Paginator(topics, TOPICS_PER_PAGE).page(1) - topics_serializer = PaginatedTopicSerializer(instance=topics_page, context={'sort_order': sort_order}) + topics_serializer = PaginatedTopicSerializer( + instance=topics_page, + context={'course_id': course.id, 'sort_order': sort_order} + ) context = { "course": course, "topics": topics_serializer.data, @@ -570,7 +573,7 @@ class TopicListView(GenericAPIView): }, status=status.HTTP_400_BAD_REQUEST) page = self.paginate_queryset(topics) - serializer = self.pagination_serializer_class(page, context={'sort_order': ordering}) + serializer = self.pagination_serializer_class(page, context={'course_id': course_id, 'sort_order': ordering}) return Response(serializer.data) # pylint: disable=maybe-no-member @@ -649,7 +652,7 @@ class TopicDetailView(APIView): if len(topics) == 0: return Response(status=status.HTTP_404_NOT_FOUND) - serializer = TopicSerializer(topics[0]) + serializer = TopicSerializer(topics[0], context={'course_id': course_id}) return Response(serializer.data)