Merge pull request #9103 from edx/dan-f/fix-team-count

Fix team count in topic serializers

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'))
 

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)

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)