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

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

Browse Source 
This reverts commit a2f04fb78b.
This commit is contained in:
Awais Jibran
2021-10-22 17:21:44 +05:00
committed by GitHub
parent 04deb4db62
commit ed74db1daf
7 changed files with 35 additions and 165 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
lms/djangoapps/course_goals/tests/test_user_activity.py
View File

@@ -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,7 +173,6 @@ 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:

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

@@ -1,11 +1,10 @@
"""
Discussion API internal interface
"""
from __future__ import annotations
import itertools
from collections import defaultdict
from typing import Dict, List, Literal, Optional, Set, Tuple
from typing import List, Literal, Optional
from urllib.parse import urlencode, urlunparse
from django.contrib.auth import get_user_model
@@ -21,7 +20,6 @@ from rest_framework.request import Request
from lms.djangoapps.courseware.courses import get_course_with_access
from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
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
@@ -37,7 +35,6 @@ 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,
@@ -83,21 +80,11 @@ class DiscussionTopic:
Class for discussion topic structure
"""
def __init__(
self,
topic_id: Optional[str],
name: str,
thread_list_url: str,
children: Optional[List[DiscussionTopic]] = None,
thread_counts: Dict[str, int] = None,
):
def __init__(self, topic_id, name, thread_list_url, children=None):
self.id = topic_id # pylint: disable=invalid-name
self.name = name
self.thread_list_url = thread_list_url
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
self.children: List[DiscussionTopic] = children or [] # children are of same type i.e. DiscussionTopic
class DiscussionEntity(Enum):
@@ -261,13 +248,7 @@ def get_course(request, course_key):
}
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]]:
def get_courseware_topics(request, course_key, course, topic_ids):
"""
Returns a list of topic trees for courseware-linked topics.
@@ -278,8 +259,6 @@ def get_courseware_topics(
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
@@ -313,8 +292,6 @@ def get_courseware_topics(
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)
@@ -327,20 +304,13 @@ def get_courseware_topics(
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: Request,
course_key: CourseKey,
course: CourseBlock,
topic_ids: Optional[List[str]],
thread_counts: Dict[str, Dict[str, int]]
) -> Tuple[List[Dict], Set[str]]:
def get_non_courseware_topics(request, course_key, course, topic_ids):
"""
Returns a list of topic trees that are not linked to courseware.
@@ -351,8 +321,6 @@ def get_non_courseware_topics(
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
@@ -365,9 +333,7 @@ def get_non_courseware_topics(
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"]]),
None,
thread_counts.get(entry["id"])
entry["id"], name, get_thread_list_url(request, course_key, [entry["id"]])
)
non_courseware_topics.append(DiscussionTopicSerializer(discussion_topic).data)
@@ -377,7 +343,7 @@ def get_non_courseware_topics(
return non_courseware_topics, existing_topic_ids
def get_course_topics(request: Request, course_key: CourseKey, topic_ids: Optional[Set[str]] = None):
def get_course_topics(request, course_key, topic_ids=None):
"""
Returns the course topic listing for the given course and user; filtered
by 'topic_ids' list if given.
@@ -397,13 +363,10 @@ def get_course_topics(request: Request, course_key: CourseKey, topic_ids: Option
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, thread_counts
)
courseware_topics, existing_courseware_topic_ids = get_courseware_topics(request, course_key, course, topic_ids)
non_courseware_topics, existing_non_courseware_topic_ids = get_non_courseware_topics(
request, course_key, course, topic_ids, thread_counts,
request, course_key, course, topic_ids
)
if topic_ids:

1
lms/djangoapps/discussion/rest_api/serializers.py
View File

@@ -499,7 +499,6 @@ 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):
"""

58
lms/djangoapps/discussion/rest_api/tests/test_api.py
View File

@@ -222,14 +222,10 @@ 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(CommentsServiceMockMixin, ForumsEnableMixin, UrlResetMixin, ModuleStoreTestCase):
class GetCourseTopicsTest(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(
@@ -253,12 +249,6 @@ class GetCourseTopicsTest(CommentsServiceMockMixin, ForumsEnableMixin, UrlResetM
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):
"""
@@ -296,13 +286,11 @@ class GetCourseTopicsTest(CommentsServiceMockMixin, ForumsEnableMixin, UrlResetM
"""
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_counts": thread_counts if not children else None
"thread_list_url": self.get_thread_list_url(topic_id_list)
}
return node
@@ -572,39 +560,29 @@ class GetCourseTopicsTest(CommentsServiceMockMixin, ForumsEnableMixin, UrlResetM
assert actual == {
'non_courseware_topics': [],
'courseware_topics': [
{
'children': [
{
'children': [],
'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',
'thread_counts': {'discussion': 0, 'question': 0},
},
],
{'children': [
{'children': [],
'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'}
],
'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',
'thread_counts': None,
'name': 'test_category_1'
},
{
'children': [
{
'children': [],
'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',
'thread_counts': {'discussion': 0, 'question': 0},
}
],
{'children': [
{'children': [],
'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'}
],
'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',
'thread_counts': None,
'name': 'test_category_2'
}
]
}

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

@@ -335,24 +335,13 @@ class ReplaceUsernamesViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
@ddt.ddt
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class CourseTopicsViewTest(DiscussionAPIViewTestMixin, CommentsServiceMockMixin, ModuleStoreTestCase):
class CourseTopicsViewTest(DiscussionAPIViewTestMixin, 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):
"""
@@ -378,7 +367,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, CommentsServiceMockMixin,
discussion_target=f'Discussion {i}',
publish_item=False,
)
return course_url, course.id
return course_url
def make_discussion_xblock(self, topic_id, category, subcategory, **kwargs):
"""
@@ -416,7 +405,6 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, CommentsServiceMockMixin,
"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},
}],
}
)
@@ -432,8 +420,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, CommentsServiceMockMixin,
)
@ddt.unpack
def test_bulk_response(self, modules_count, module_store, mongo_calls, topics):
course_url, course_id = self.create_course(modules_count, module_store, topics)
self.register_get_course_commentable_counts_response(course_id, {})
course_url = self.create_course(modules_count, module_store, topics)
with check_mongo_calls(mongo_calls):
with modulestore().default_store(module_store):
self.client.get(course_url)
@@ -474,14 +461,12 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, CommentsServiceMockMixin,
"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",
"thread_counts": {"discussion": 0, "question": 0},
"name": "test_target_1"
}],
"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",
"thread_counts": None,
"name": "test_category_1"
},
{
"children":
@@ -490,14 +475,12 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, CommentsServiceMockMixin,
"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",
"thread_counts": {"discussion": 0, "question": 0},
"name": "test_target_2"
}],
"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",
"thread_counts": None,
"name": "test_category_2"
}
]
}

11
lms/djangoapps/discussion/rest_api/tests/utils.py
View File

@@ -85,17 +85,6 @@ 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.'

41
openedx/core/djangoapps/django_comment_common/comment_client/course.py
View File

@@ -1,41 +0,0 @@
# 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