Refactor django_comment_client.utils.prepare_content() to query

course cohort settings only once.

TNL-1258

lms/djangoapps/django_comment_client/forum/tests.py

@@ -309,18 +309,18 @@ class SingleThreadTestCase(ModuleStoreTestCase):
 @patch('requests.request')
 class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
     """
-    Ensures the number of modulestore queries is deterministic based on the
-    number of responses retrieved for a given discussion thread.
+    Ensures the number of modulestore queries and number of sql queries are
+    independent of the number of responses retrieved for a given discussion thread.
     """
     MODULESTORE = TEST_DATA_MONGO_MODULESTORE
 
     @ddt.data(
         # old mongo with cache: 15
-        (ModuleStoreEnum.Type.mongo, 1, 21, 15),
-        (ModuleStoreEnum.Type.mongo, 50, 315, 15),
+        (ModuleStoreEnum.Type.mongo, 1, 21, 15, 30),
+        (ModuleStoreEnum.Type.mongo, 50, 315, 15, 30),
         # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, 1, 3, 3),
-        (ModuleStoreEnum.Type.split, 50, 3, 3),
+        (ModuleStoreEnum.Type.split, 1, 3, 3, 30),
+        (ModuleStoreEnum.Type.split, 50, 3, 3, 30),
     )
     @ddt.unpack
     def test_number_of_mongo_queries(
@@ -329,6 +329,7 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
             num_thread_responses,
             num_uncached_mongo_calls,
             num_cached_mongo_calls,
+            num_sql_queries,
             mock_request
     ):
         with modulestore().default_store(default_store):
@@ -377,8 +378,9 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
         for single_thread_cache, expected_calls in cached_calls.items():
             single_thread_cache.clear()
             with patch("django_comment_client.permissions.CACHE", single_thread_cache):
-                with check_mongo_calls(expected_calls):
-                    call_single_thread()
+                with self.assertNumQueries(num_sql_queries):
+                    with check_mongo_calls(expected_calls):
+                        call_single_thread()
             single_thread_cache.clear()
 
 

lms/djangoapps/django_comment_client/tests/test_utils.py

@@ -18,7 +18,6 @@ import django_comment_client.utils as utils
 
 from openedx.core.djangoapps.course_groups.cohorts import set_course_cohort_settings
 from student.tests.factories import UserFactory, CourseEnrollmentFactory
-from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
 

lms/djangoapps/django_comment_client/utils.py

@@ -405,7 +405,7 @@ def add_courseware_context(content_list, course, user, id_map=None):
             content.update({"courseware_url": url, "courseware_title": title})
 
 
-def prepare_content(content, course_key, is_staff=False):
+def prepare_content(content, course_key, is_staff=False, **kwargs):
     """
     This function is used to pre-process thread and comment models in various
     ways before adding them to the HTTP response.  This includes fixing empty
@@ -414,6 +414,12 @@ def prepare_content(content, course_key, is_staff=False):
 
     @TODO: not all response pre-processing steps are currently integrated into
     this function.
+
+    Arguments:
+        content (dict): A thread or comment.
+        course_key (CourseKey): The course key of the course.
+        is_staff (bool): Whether the user is a staff member.
+        course_is_cohorted (bool): Whether the course is cohorted.
     """
     fields = [
         'id', 'title', 'body', 'course_id', 'anonymous', 'anonymous_to_peers',
@@ -453,14 +459,19 @@ def prepare_content(content, course_key, is_staff=False):
         else:
             del endorsement["user_id"]
 
+    course_is_cohorted = kwargs.get('course_is_cohorted')
+    if course_is_cohorted is None:
+        course_is_cohorted = is_course_cohorted(course_key)
+
     for child_content_key in ["children", "endorsed_responses", "non_endorsed_responses"]:
         if child_content_key in content:
             children = [
-                prepare_content(child, course_key, is_staff) for child in content[child_content_key]
+                prepare_content(child, course_key, is_staff, course_is_cohorted=course_is_cohorted)
+                for child in content[child_content_key]
             ]
             content[child_content_key] = children
 
-    if is_course_cohorted(course_key):
+    if course_is_cohorted:
         # Augment the specified thread info to include the group name if a group id is present.
         if content.get('group_id') is not None:
             content['group_name'] = get_cohort_by_id(course_key, content.get('group_id')).name