Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: Temporary fix for learners stats api performance issue (#30847)

Browse Source 
* fix: Temporary fix for learners' stats API performance issue

* fix: resolved linter errors

* fix: learners stats API response is now null

* fix: changed waffle dates

* fix: resolved unit test issue
This commit is contained in:
Ahtisham Shahid
2022-08-12 17:24:22 +05:00
committed by GitHub
parent 067ebd79ce
commit dcb01d107f
4 changed files with 149 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

19
lms/djangoapps/discussion/config/waffle.py Normal file
View File

@@ -0,0 +1,19 @@
"""
This module contains configuration settings via waffle switches for the discussions.
"""
from openedx.core.djangoapps.waffle_utils import CourseWaffleFlag
WAFFLE_NAMESPACE = 'discussions'
# .. toggle_name: discussions.enable_learners_stats
# .. toggle_implementation: CourseWaffleFlag
# .. toggle_default: False
# .. toggle_description: Waffle flag to enable learners stats
# .. toggle_use_cases: temporary, open_edx
# .. toggle_creation_date: 2022-08-12
# .. toggle_target_removal_date: 2022-10-02
# .. toggle_warning: When the flag is ON, API will return learners stats with original values.
# .. This is temporary fix for performance issue in API.
# .. toggle_tickets: INF-444
ENABLE_LEARNERS_STATS = CourseWaffleFlag(f'{WAFFLE_NAMESPACE}.enable_learners_stats', __name__)

117
lms/djangoapps/discussion/rest_api/api.py
View File

@@ -5,6 +5,7 @@ from __future__ import annotations
import itertools
from collections import defaultdict
from enum import Enum
from typing import Dict, Iterable, List, Literal, Optional, Set, Tuple
from urllib.parse import urlencode, urlunparse
@@ -42,7 +43,8 @@ from openedx.core.djangoapps.django_comment_common.comment_client.course import
get_course_user_stats,
)
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.comment_client.utils import CommentClientRequestError, \
CommentClient500Error
from openedx.core.djangoapps.django_comment_common.models import (
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_COMMUNITY_TA,
@@ -64,6 +66,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 ..config.waffle import ENABLE_LEARNERS_STATS
from ..django_comment_client.base.views import (
track_comment_created_event,
@@ -101,7 +104,7 @@ from .utils import (
discussion_open_for_user,
get_usernames_from_search_string,
add_stats_for_users_with_no_discussion_content,
set_attribute,
set_attribute, get_usernames_for_course,
)
User = get_user_model()
@@ -967,20 +970,29 @@ def get_learner_active_thread_list(request, course_key, query_params):
else:
comment_client_user = comment_client.User(id=user_id, course_id=course_key, group_id=group_id)
threads, page, num_pages = comment_client_user.active_threads(query_params)
threads = set_attribute(threads, "pinned", False)
results = _serialize_discussion_entities(
request, context, threads, {'profile_image'}, DiscussionEntity.thread
)
paginator = DiscussionAPIPagination(
request,
page,
num_pages,
len(threads)
)
return paginator.get_paginated_response({
"results": results,
})
try:
threads, page, num_pages = comment_client_user.active_threads(query_params)
threads = set_attribute(threads, "pinned", False)
results = _serialize_discussion_entities(
request, context, threads, {'profile_image'}, DiscussionEntity.thread
)
paginator = DiscussionAPIPagination(
request,
page,
num_pages,
len(threads)
)
return paginator.get_paginated_response({
"results": results,
})
except CommentClient500Error:
return DiscussionAPIPagination(
request,
page_num=1,
num_pages=0,
).get_paginated_response({
"results": [],
})
def get_comment_list(request, thread_id, endorsed, page, page_size, flagged=False, requested_fields=None):
@@ -1639,6 +1651,17 @@ def get_course_discussion_user_stats(
order_by = order_by or UserOrdering.BY_ACTIVITY
if order_by != UserOrdering.BY_ACTIVITY:
raise ValidationError({"order_by": "Invalid value"})
if not ENABLE_LEARNERS_STATS.is_enabled(course_key):
return get_users_without_stats(
username_search_string,
course_key,
page,
page_size,
request,
is_privileged
)
params = {
'sort_key': str(order_by),
'page': page,
@@ -1679,3 +1702,65 @@ def get_course_discussion_user_stats(
return paginator.get_paginated_response({
"results": serializer.data,
})
def get_users_without_stats(
username_search_string,
course_key,
page_number,
page_size,
request,
is_privileged
):
"""
This return users with no user stats.
This function will be deprecated when this ticket DOS-3414 is resolved
"""
if username_search_string:
comma_separated_usernames, matched_users_count, matched_users_pages = get_usernames_from_search_string(
course_key, username_search_string, page_number, page_size
)
if not comma_separated_usernames:
return DiscussionAPIPagination(request, 0, 1).get_paginated_response({
"results": [],
})
else:
comma_separated_usernames, matched_users_count, matched_users_pages = get_usernames_for_course(
course_key, page_number, page_size
)
if comma_separated_usernames:
updated_course_stats = add_stats_for_users_with_null_values([], comma_separated_usernames)
serializer = UserStatsSerializer(updated_course_stats, context={"is_privileged": is_privileged}, many=True)
paginator = DiscussionAPIPagination(
request,
page_number,
matched_users_pages,
matched_users_count,
)
return paginator.get_paginated_response({
"results": serializer.data,
})
def add_stats_for_users_with_null_values(course_stats, users_in_course):
"""
Update users stats for users with no discussion stats available in course
"""
users_returned_from_api = [user['username'] for user in course_stats]
user_list = users_in_course.split(',')
users_with_no_discussion_content = set(user_list) ^ set(users_returned_from_api)
updated_course_stats = course_stats
for user in users_with_no_discussion_content:
updated_course_stats.append({
'username': user,
'threads': None,
'replies': None,
'responses': None,
'active_flags': None,
'inactive_flags': None,
})
updated_course_stats = sorted(updated_course_stats, key=lambda d: len(d['username']))
return updated_course_stats

4
lms/djangoapps/discussion/rest_api/tests/test_views.py
View File

@@ -14,11 +14,14 @@ import httpretty
from django.core.files.uploadedfile import SimpleUploadedFile
from django.test import override_settings
from django.urls import reverse
from edx_toggles.toggles.testutils import override_waffle_flag
from opaque_keys.edx.keys import CourseKey
from pytz import UTC
from rest_framework import status
from rest_framework.parsers import JSONParser
from rest_framework.test import APIClient, APITestCase
from lms.djangoapps.discussion.config.waffle import ENABLE_LEARNERS_STATS
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase
@@ -2890,6 +2893,7 @@ class CourseDiscussionRolesAPIViewTest(APITestCase, UrlResetMixin, ModuleStoreTe
@ddt.ddt
@httpretty.activate
@override_waffle_flag(ENABLE_LEARNERS_STATS, True)
class CourseActivityStatsTest(ForumsEnableMixin, UrlResetMixin, CommentsServiceMockMixin, APITestCase,
SharedModuleStoreTestCase):
"""

25
lms/djangoapps/discussion/rest_api/utils.py
View File

@@ -69,6 +69,31 @@ def get_usernames_from_search_string(course_id, search_string, page_number, page
return ','.join(page_matched_users), matched_users_count, matched_users_pages
def get_usernames_for_course(course_id, page_number, page_size):
"""
Gets usernames for all users in course.
Args:
course_id (CourseKey): Course to check discussions for
page_number (int): Page numbers to fetch
page_size (int): Number of items in each page
Returns:
page_matched_users (str): comma seperated usernames for the page
matched_users_count (int): count of matched users in course
matched_users_pages (int): pages of matched users in course
"""
matched_users_in_course = User.objects.filter(courseenrollment__course_id=course_id,)\
.order_by(Length('username').asc()).values_list('username', flat=True)
if not matched_users_in_course:
return '', 0, 0
matched_users_count = len(matched_users_in_course)
paginator = Paginator(matched_users_in_course, page_size)
page_matched_users = paginator.page(page_number)
matched_users_pages = int(matched_users_count / page_size)
return ','.join(page_matched_users), matched_users_count, matched_users_pages
def add_stats_for_users_with_no_discussion_content(course_stats, users_in_course):
"""
Update users stats for users with no discussion stats available in course