Skip to content

Commit 328420c

Browse files
committed
fix: inconsistent loading of discussion thread response
1 parent 2867724 commit 328420c

1 file changed

Lines changed: 30 additions & 104 deletions

File tree

  • lms/djangoapps/discussion/rest_api

lms/djangoapps/discussion/rest_api/api.py

Lines changed: 30 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
from __future__ import annotations
77

8+
import itertools
89
import logging
910
import re
1011
from collections import defaultdict
@@ -1235,58 +1236,6 @@ def _serialize_discussion_entities(
12351236
return results
12361237

12371238

1238-
def _prefetch_author_data_into_context(context, content_items):
1239-
"""
1240-
Bulk-prefetch User objects, Role assignments, and GlobalStaff status
1241-
for all content authors and inject into serializer context.
1242-
1243-
This eliminates N+1 DB queries in CommentSerializer.get_author(),
1244-
_get_user_label(), _get_user_labels_all(), and get_learner_status().
1245-
"""
1246-
author_ids = set()
1247-
for item in content_items:
1248-
uid = item.get("user_id")
1249-
if uid:
1250-
try:
1251-
author_ids.add(int(uid))
1252-
except (ValueError, TypeError):
1253-
pass
1254-
1255-
if not author_ids:
1256-
return
1257-
1258-
# 1. Bulk fetch User objects (1 query instead of N)
1259-
users_by_id = {
1260-
u.id: u
1261-
for u in User.objects.filter(id__in=author_ids).only("id", "username")
1262-
}
1263-
context["_prefetched_users"] = users_by_id
1264-
1265-
# 2. Bulk fetch Role assignments
1266-
roles_by_user = defaultdict(set)
1267-
for uid, name in Role.objects.filter(
1268-
users__id__in=author_ids,
1269-
course_id=context["course"].id,
1270-
name__in=[
1271-
FORUM_ROLE_ADMINISTRATOR,
1272-
FORUM_ROLE_MODERATOR,
1273-
FORUM_ROLE_COMMUNITY_TA,
1274-
FORUM_ROLE_GROUP_MODERATOR,
1275-
],
1276-
).values_list("users__id", "name"):
1277-
roles_by_user[int(uid)].add(name)
1278-
context["_prefetched_roles"] = dict(roles_by_user)
1279-
1280-
# 3. Bulk check GlobalStaff (1 check per user, but from cached User objects)
1281-
from common.djangoapps.student.roles import GlobalStaff
1282-
global_staff_checker = GlobalStaff()
1283-
global_staff_ids = set()
1284-
for uid, user in users_by_id.items():
1285-
if global_staff_checker.has_user(user):
1286-
global_staff_ids.add(uid)
1287-
context["_prefetched_global_staff_ids"] = global_staff_ids
1288-
1289-
12901239
def get_thread_list( # pylint: disable=too-many-statements
12911240
request: Request,
12921241
course_key: CourseKey,
@@ -2318,74 +2267,56 @@ def get_thread(request, thread_id, requested_fields=None, course_id=None):
23182267
)[0]
23192268

23202269

2321-
def _normalize_forum_comment(raw_comment):
2322-
"""
2323-
Normalize a Comment.to_dict() output from the forum MySQL backend
2324-
to match the key format expected by CommentSerializer and permissions.py.
2325-
2326-
The MySQL backend's Comment.to_dict() uses keys like:
2327-
_id, _type, author_id, author_username, comment_thread_id
2328-
But CommentSerializer (via _ContentSerializer) and get_editable_fields expect:
2329-
id, type, user_id, username, thread_id
2270+
def get_response_comments(request, comment_id, page, page_size, requested_fields=None, include_muted=False):
23302271
"""
2331-
if not isinstance(raw_comment, dict):
2332-
return raw_comment
2333-
2334-
normalized = dict(raw_comment)
2272+
Return the list of comments for the given thread response.
23352273
2336-
if "_id" in normalized and "id" not in normalized:
2337-
normalized["id"] = normalized["_id"]
2274+
Arguments:
23382275
2339-
if "_type" in normalized and "type" not in normalized:
2340-
normalized["type"] = normalized["_type"].lower()
2276+
request: The django request object used for build_absolute_uri and
2277+
determining the requesting user.
23412278
2342-
if "author_id" in normalized and "user_id" not in normalized:
2343-
normalized["user_id"] = normalized["author_id"]
2279+
comment_id: The id of the comment/response to get child comments for.
23442280
2345-
if "author_username" in normalized and "username" not in normalized:
2346-
normalized["username"] = normalized["author_username"]
2281+
page: The page number (1-indexed) to retrieve
23472282
2348-
if "comment_thread_id" in normalized and "thread_id" not in normalized:
2349-
normalized["thread_id"] = normalized["comment_thread_id"]
2283+
page_size: The number of comments to retrieve per page
23502284
2351-
if "children" not in normalized:
2352-
normalized["children"] = []
2285+
requested_fields: Indicates which additional fields to return for
2286+
each child comment. (i.e. ['profile_image'])
23532287
2354-
return normalized
2288+
Returns:
23552289
2290+
A paginated result containing a list of comments
23562291
2357-
def get_response_comments(request, comment_id, page, page_size, requested_fields=None, include_muted=False):
2358-
"""
2359-
Return the list of comments for the given thread response.
2360-
Uses direct backend queries for efficient DB-level pagination
2361-
instead of loading the entire thread recursively.
23622292
"""
23632293
try:
23642294
cc_comment = Comment(id=comment_id).retrieve()
2365-
23662295
reverse_order = request.GET.get("reverse_order", False)
2367-
reverse_order = reverse_order in ["true", "True", True]
23682296
show_deleted = request.GET.get("show_deleted", False)
23692297
show_deleted = show_deleted in ["true", "True", True]
23702298

23712299
cc_thread, context = _get_thread_and_context(
23722300
request,
23732301
cc_comment["thread_id"],
23742302
retrieve_kwargs={
2375-
"with_responses": False,
2376-
"recursive": False,
2303+
"with_responses": True,
2304+
"recursive": True,
2305+
"reverse_order": reverse_order,
2306+
"show_deleted": show_deleted,
23772307
},
23782308
)
2379-
2380-
if show_deleted and not context["has_moderation_privilege"]:
2381-
raise PermissionDenied(
2382-
"`show_deleted` can only be set by users with moderation roles."
2309+
if cc_thread["thread_type"] == "question":
2310+
thread_responses = itertools.chain(
2311+
cc_thread["endorsed_responses"], cc_thread["non_endorsed_responses"]
23832312
)
2384-
2385-
from forum.backend import get_backend
2386-
2387-
course_id = cc_thread["course_id"]
2388-
backend = get_backend(course_id)()
2313+
else:
2314+
thread_responses = cc_thread["children"]
2315+
response_comments = []
2316+
for response in thread_responses:
2317+
if response["id"] == comment_id:
2318+
response_comments = response["children"]
2319+
break
23892320

23902321
# Filter deleted content from the FULL list first
23912322
if not show_deleted:
@@ -2428,20 +2359,15 @@ def get_response_comments(request, comment_id, page, page_size, requested_fields
24282359
results = _serialize_discussion_entities(
24292360
request,
24302361
context,
2431-
response_comments,
2362+
paged_response_comments,
24322363
requested_fields,
24332364
DiscussionEntity.comment,
24342365
)
24352366

24362367
paginator = DiscussionAPIPagination(
2437-
request,
2438-
page,
2439-
num_pages,
2440-
total_comments_count,
2368+
request, page, num_pages, total_comments_count
24412369
)
2442-
24432370
return paginator.get_paginated_response(results)
2444-
24452371
except CommentClientRequestError as err:
24462372
raise CommentNotFoundError("Comment not found") from err
24472373

@@ -3266,4 +3192,4 @@ def get_deleted_content_for_course(
32663192

32673193
except Exception as e:
32683194
log.exception("Error getting deleted content for course %s: %s", course_id, e)
3269-
raise
3195+
raise

0 commit comments

Comments
 (0)