Revert "Revert "feat: Add support for returning thread counts for all topics in a course [BD-38] [TNL-8724] [BB-4927] (#29062)" (#29087)" (#29152)

This reverts commit ed74db1daf.

lms/djangoapps/course_goals/tests/test_user_activity.py

@@ -3,7 +3,7 @@ Unit tests for user activity methods.
 """
 
 from datetime import datetime, timedelta
-
+from unittest.mock import Mock
 
 import ddt
 from django.contrib.auth import get_user_model
@@ -173,6 +173,7 @@ class UserActivityTests(UrlResetMixin, ModuleStoreTestCase):
         '/api/discussion/v1/courses/{COURSE_ID}/',
         '/api/discussion/v1/course_topics/{COURSE_ID}',
     )
+    @patch('lms.djangoapps.discussion.rest_api.api.get_course_commentable_counts', Mock(return_value={}))
     def test_mobile_app_user_activity_calls(self, url):
         url = url.replace('{COURSE_ID}', str(self.course.id))
         with patch.object(UserActivity, 'record_user_activity') as record_user_activity_mock:

lms/djangoapps/discussion/rest_api/api.py

@@ -1,10 +1,11 @@
 """
 Discussion API internal interface
 """
+from __future__ import annotations
 
 import itertools
 from collections import defaultdict
-from typing import List, Literal, Optional
+from typing import Dict, List, Literal, Optional, Set, Tuple
 from urllib.parse import urlencode, urlunparse
 
 from django.contrib.auth import get_user_model
@@ -21,6 +22,7 @@ from lms.djangoapps.courseware.courses import get_course_with_access
 from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
 from openedx.core.djangoapps.discussions.utils import get_accessible_discussion_xblocks
 from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment
+from openedx.core.djangoapps.django_comment_common.comment_client.course import get_course_commentable_counts
 from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread
 from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError
 from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings
@@ -36,6 +38,7 @@ from openedx.core.djangoapps.django_comment_common.signals import (
 )
 from openedx.core.djangoapps.user_api.accounts.api import get_account_settings
 from openedx.core.lib.exceptions import CourseNotFoundError, DiscussionNotFoundError, PageNotFoundError
+from xmodule.course_module import CourseBlock
 from .exceptions import (
     CommentNotFoundError,
     DiscussionBlackOutException,
@@ -80,11 +83,21 @@ class DiscussionTopic:
     Class for discussion topic structure
     """
 
-    def __init__(self, topic_id, name, thread_list_url, children=None):
+    def __init__(
+        self,
+        topic_id: Optional[str],
+        name: str,
+        thread_list_url: str,
+        children: Optional[List[DiscussionTopic]] = None,
+        thread_counts: Dict[str, int] = None,
+    ):
         self.id = topic_id  # pylint: disable=invalid-name
         self.name = name
         self.thread_list_url = thread_list_url
-        self.children: List[DiscussionTopic] = children or []  # children are of same type i.e. DiscussionTopic
+        self.children = children or []  # children are of same type i.e. DiscussionTopic
+        if not children and not thread_counts:
+            thread_counts = {"discussion": 0, "question": 0}
+        self.thread_counts = thread_counts
 
 
 class DiscussionEntity(Enum):
@@ -251,7 +264,13 @@ def get_course(request, course_key):
     }
 
 
-def get_courseware_topics(request, course_key, course, topic_ids):
+def get_courseware_topics(
+    request: Request,
+    course_key: CourseKey,
+    course: CourseBlock,
+    topic_ids: Optional[List[str]],
+    thread_counts: Dict[str, Dict[str, int]],
+) -> Tuple[List[Dict], Set[str]]:
     """
     Returns a list of topic trees for courseware-linked topics.
 
@@ -262,6 +281,8 @@ def get_courseware_topics(request, course_key, course, topic_ids):
         course: The course for which topics are requested.
         topic_ids: A list of topic IDs for which details are requested.
             This is optional. If None then all course topics are returned.
+        thread_counts: A map of the thread ids to the count of each type of thread in them
+           e.g. discussion, question
 
     Returns:
         A list of courseware topics and a set of existing topics among
@@ -295,6 +316,8 @@ def get_courseware_topics(request, course_key, course, topic_ids):
                     xblock.discussion_id,
                     xblock.discussion_target,
                     get_thread_list_url(request, course_key, [xblock.discussion_id]),
+                    None,
+                    thread_counts.get(xblock.discussion_id),
                 )
                 children.append(discussion_topic)
 
@@ -307,13 +330,20 @@ def get_courseware_topics(request, course_key, course, topic_ids):
                 category,
                 get_thread_list_url(request, course_key, [item.discussion_id for item in get_sorted_xblocks(category)]),
                 children,
+                None,
             )
             courseware_topics.append(DiscussionTopicSerializer(discussion_topic).data)
 
     return courseware_topics, existing_topic_ids
 
 
-def get_non_courseware_topics(request, course_key, course, topic_ids):
+def get_non_courseware_topics(
+    request: Request,
+    course_key: CourseKey,
+    course: CourseBlock,
+    topic_ids: Optional[List[str]],
+    thread_counts: Dict[str, Dict[str, int]]
+) -> Tuple[List[Dict], Set[str]]:
     """
     Returns a list of topic trees that are not linked to courseware.
 
@@ -324,6 +354,8 @@ def get_non_courseware_topics(request, course_key, course, topic_ids):
         course: The course for which topics are requested.
         topic_ids: A list of topic IDs for which details are requested.
             This is optional. If None then all course topics are returned.
+        thread_counts: A map of the thread ids to the count of each type of thread in them
+           e.g. discussion, question
 
     Returns:
         A list of non-courseware topics and a set of existing topics among
@@ -336,7 +368,9 @@ def get_non_courseware_topics(request, course_key, course, topic_ids):
     for name, entry in sorted_topics:
         if not topic_ids or entry['id'] in topic_ids:
             discussion_topic = DiscussionTopic(
-                entry["id"], name, get_thread_list_url(request, course_key, [entry["id"]])
+                entry["id"], name, get_thread_list_url(request, course_key, [entry["id"]]),
+                None,
+                thread_counts.get(entry["id"])
             )
             non_courseware_topics.append(DiscussionTopicSerializer(discussion_topic).data)
 
@@ -346,7 +380,7 @@ def get_non_courseware_topics(request, course_key, course, topic_ids):
     return non_courseware_topics, existing_topic_ids
 
 
-def get_course_topics(request, course_key, topic_ids=None):
+def get_course_topics(request: Request, course_key: CourseKey, topic_ids: Optional[Set[str]] = None):
     """
     Returns the course topic listing for the given course and user; filtered
     by 'topic_ids' list if given.
@@ -366,10 +400,13 @@ def get_course_topics(request, course_key, topic_ids=None):
         DiscussionNotFoundError: If topic/s not found for given topic_ids.
     """
     course = _get_course(course_key, request.user)
+    thread_counts = get_course_commentable_counts(course.id)
 
-    courseware_topics, existing_courseware_topic_ids = get_courseware_topics(request, course_key, course, topic_ids)
+    courseware_topics, existing_courseware_topic_ids = get_courseware_topics(
+        request, course_key, course, topic_ids, thread_counts
+    )
     non_courseware_topics, existing_non_courseware_topic_ids = get_non_courseware_topics(
-        request, course_key, course, topic_ids
+        request, course_key, course, topic_ids, thread_counts,
     )
 
     if topic_ids:

lms/djangoapps/discussion/rest_api/serializers.py

@@ -497,6 +497,7 @@ class DiscussionTopicSerializer(serializers.Serializer):
     name = serializers.CharField(read_only=True)
     thread_list_url = serializers.CharField(read_only=True)
     children = serializers.SerializerMethodField()
+    thread_counts = serializers.DictField(read_only=True)
 
     def get_children(self, obj):
         """

lms/djangoapps/discussion/rest_api/tests/test_api.py

@@ -222,10 +222,14 @@ class GetCourseTestBlackouts(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCa
 
 @mock.patch.dict("django.conf.settings.FEATURES", {"DISABLE_START_DATES": False})
 @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
-class GetCourseTopicsTest(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase):
+class GetCourseTopicsTest(CommentsServiceMockMixin, ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase):
     """Test for get_course_topics"""
     @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
     def setUp(self):
+        httpretty.reset()
+        httpretty.enable()
+        self.addCleanup(httpretty.reset)
+        self.addCleanup(httpretty.disable)
         super().setUp()
         self.maxDiff = None  # pylint: disable=invalid-name
         self.partition = UserPartition(
@@ -249,6 +253,12 @@ class GetCourseTopicsTest(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase)
         self.request = RequestFactory().get("/dummy")
         self.request.user = self.user
         CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
+        self.thread_counts_map = {
+            "courseware-1": {"discussion": 2, "question": 3},
+            "courseware-2": {"discussion": 4, "question": 5},
+            "courseware-3": {"discussion": 7, "question": 2},
+        }
+        self.register_get_course_commentable_counts_response(self.course.id, self.thread_counts_map)
 
     def make_discussion_xblock(self, topic_id, category, subcategory, **kwargs):
         """
@@ -286,11 +296,13 @@ class GetCourseTopicsTest(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase)
         """
         topic_id_list = [topic_id] if topic_id else [child["id"] for child in children]
         children = children or []
+        thread_counts = self.thread_counts_map.get(topic_id, {"discussion": 0, "question": 0})
         node = {
             "id": topic_id,
             "name": name,
             "children": children,
-            "thread_list_url": self.get_thread_list_url(topic_id_list)
+            "thread_list_url": self.get_thread_list_url(topic_id_list),
+            "thread_counts": thread_counts if not children else None
         }
 
         return node
@@ -560,29 +572,39 @@ class GetCourseTopicsTest(ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase)
         assert actual == {
             'non_courseware_topics': [],
             'courseware_topics': [
-                {'children': [
+                {
-                    {'children': [],
+                    'children': [
-                     'id': 'topic_id_1',
+                        {
-                     'thread_list_url':
+                            'children': [],
-                         'http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&topic_id=topic_id_1',
+                            'id': 'topic_id_1',
-                     'name': 'test_target_1'}
+                            'thread_list_url':
-                ],
+                                'http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&topic_id=topic_id_1',
+                            'name': 'test_target_1',
+                            'thread_counts': {'discussion': 0, 'question': 0},
+                        },
+                    ],
                     'id': None,
                     'thread_list_url':
                         'http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&topic_id=topic_id_1',
-                    'name': 'test_category_1'
+                    'name': 'test_category_1',
+                    'thread_counts': None,
                 },
-                {'children': [
+                {
-                    {'children': [],
+                    'children': [
-                     'id': 'topic_id_2',
+                        {
-                     'thread_list_url':
+                            'children': [],
-                         'http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&topic_id=topic_id_2',
+                            'id': 'topic_id_2',
-                     'name': 'test_target_2'}
+                            'thread_list_url':
-                ],
+                                'http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&topic_id=topic_id_2',
+                            'name': 'test_target_2',
+                            'thread_counts': {'discussion': 0, 'question': 0},
+                        }
+                    ],
                     'id': None,
                     'thread_list_url':
                         'http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&topic_id=topic_id_2',
-                    'name': 'test_category_2'
+                    'name': 'test_category_2',
+                    'thread_counts': None,
                 }
             ]
         }

lms/djangoapps/discussion/rest_api/tests/test_views.py

@@ -335,13 +335,24 @@ class ReplaceUsernamesViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
 
 @ddt.ddt
 @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
-class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
+class CourseTopicsViewTest(DiscussionAPIViewTestMixin, CommentsServiceMockMixin, ModuleStoreTestCase):
     """
     Tests for CourseTopicsView
     """
+
     def setUp(self):
+        httpretty.reset()
+        httpretty.enable()
+        self.addCleanup(httpretty.reset)
+        self.addCleanup(httpretty.disable)
         super().setUp()
         self.url = reverse("course_topics", kwargs={"course_id": str(self.course.id)})
+        self.thread_counts_map = {
+            "courseware-1": {"discussion": 2, "question": 3},
+            "courseware-2": {"discussion": 4, "question": 5},
+            "courseware-3": {"discussion": 7, "question": 2},
+        }
+        self.register_get_course_commentable_counts_response(self.course.id, self.thread_counts_map)
 
     def create_course(self, modules_count, module_store, topics):
         """
@@ -367,7 +378,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
                 discussion_target=f'Discussion {i}',
                 publish_item=False,
             )
-        return course_url
+        return course_url, course.id
 
     def make_discussion_xblock(self, topic_id, category, subcategory, **kwargs):
         """
@@ -405,6 +416,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
                     "children": [],
                     "thread_list_url":
                         "http://testserver/api/discussion/v1/threads/?course_id=x%2Fy%2Fz&topic_id=test_topic",
+                    "thread_counts": {"discussion": 0, "question": 0},
                 }],
             }
         )
@@ -420,7 +432,8 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
     )
     @ddt.unpack
     def test_bulk_response(self, modules_count, module_store, mongo_calls, topics):
-        course_url = self.create_course(modules_count, module_store, topics)
+        course_url, course_id = self.create_course(modules_count, module_store, topics)
+        self.register_get_course_commentable_counts_response(course_id, {})
         with check_mongo_calls(mongo_calls):
             with modulestore().default_store(module_store):
                 self.client.get(course_url)
@@ -461,12 +474,14 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
                             "id": "topic_id_1",
                             "thread_list_url": "http://testserver/api/discussion/v1/threads/?"
                                                "course_id=x%2Fy%2Fz&topic_id=topic_id_1",
-                            "name": "test_target_1"
+                            "name": "test_target_1",
+                            "thread_counts": {"discussion": 0, "question": 0},
                         }],
                         "id": None,
                         "thread_list_url": "http://testserver/api/discussion/v1/threads/?"
                                            "course_id=x%2Fy%2Fz&topic_id=topic_id_1",
-                        "name": "test_category_1"
+                        "name": "test_category_1",
+                        "thread_counts": None,
                     },
                     {
                         "children":
@@ -475,12 +490,14 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
                                 "id": "topic_id_2",
                                 "thread_list_url": "http://testserver/api/discussion/v1/threads/?"
                                                    "course_id=x%2Fy%2Fz&topic_id=topic_id_2",
-                                "name": "test_target_2"
+                                "name": "test_target_2",
+                                "thread_counts": {"discussion": 0, "question": 0},
                             }],
                         "id": None,
                         "thread_list_url": "http://testserver/api/discussion/v1/threads/?"
                                            "course_id=x%2Fy%2Fz&topic_id=topic_id_2",
-                        "name": "test_category_2"
+                        "name": "test_category_2",
+                        "thread_counts": None,
                     }
                 ]
             }

lms/djangoapps/discussion/rest_api/tests/utils.py

@@ -85,6 +85,17 @@ class CommentsServiceMockMixin:
             status=200
         )
 
+    def register_get_course_commentable_counts_response(self, course_id, thread_counts):
+        """Register a mock response for GET on the CS thread list endpoint"""
+        assert httpretty.is_enabled(), 'httpretty must be enabled to mock calls.'
+
+        httpretty.register_uri(
+            httpretty.GET,
+            f"http://localhost:4567/api/v1/commentables/{course_id}/counts",
+            body=json.dumps(thread_counts),
+            status=200
+        )
+
     def register_get_threads_search_response(self, threads, rewrite, num_pages=1):
         """Register a mock response for GET on the CS thread search endpoint"""
         assert httpretty.is_enabled(), 'httpretty must be enabled to mock calls.'

openedx/core/djangoapps/django_comment_common/comment_client/course.py

@@ -0,0 +1,41 @@
+# pylint: disable=missing-docstring
+"""Provides base Commentable model class"""
+from __future__ import annotations
+
+from typing import Dict
+
+from opaque_keys.edx.keys import CourseKey
+
+from openedx.core.djangoapps.django_comment_common.comment_client import settings
+from openedx.core.djangoapps.django_comment_common.comment_client.utils import perform_request
+
+
+def get_course_commentable_counts(course_key: CourseKey) -> Dict[str, Dict[str, int]]:
+    """
+    Get stats about the count of different types of threads for each commentable (topic).
+
+    Args:
+        course_key (str|CourseKey): course key for which stats are needed.
+
+    Returns:
+        A mapping of topic ids to the number of question and discussion type posts in them.
+
+        e.g.
+            {
+                "general": { "discussion": 22, "question": 15 },
+                "topic-1": { "discussion": 2, "question": 1 },
+                ...
+            }
+
+    """
+    url = f"{settings.PREFIX}/commentables/{course_key}/counts"
+    response = perform_request(
+        'get',
+        url,
+        metric_tags=[
+            f"course_key:{course_key}",
+            "function:get_course_commentable_counts",
+        ],
+        metric_action='commentable_stats.retrieve',
+    )
+    return response