feat: Add a new user API for discussions (#29287)
Adds a new user API for discussion that returns the discussion stats across the course.
This commit is contained in:
@@ -13,6 +13,7 @@ from django.contrib.auth import get_user_model
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.http import Http404
|
||||
from django.urls import reverse
|
||||
from edx_django_utils.monitoring import function_trace
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.locator import CourseKey
|
||||
from rest_framework.exceptions import PermissionDenied
|
||||
@@ -27,7 +28,10 @@ from lms.djangoapps.courseware.exceptions import CourseAccessRedirect
|
||||
from openedx.core.djangoapps.discussions.models import DiscussionsConfiguration, DiscussionTopicLink, Provider
|
||||
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.course import (
|
||||
get_course_commentable_counts,
|
||||
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.models import (
|
||||
@@ -49,14 +53,8 @@ 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 ..django_comment_client.base.views import (
|
||||
track_comment_created_event,
|
||||
track_thread_created_event,
|
||||
track_voted_event,
|
||||
)
|
||||
from ..django_comment_client.utils import get_group_id_for_user, get_user_role_names, is_commentable_divided
|
||||
from .exceptions import CommentNotFoundError, DiscussionBlackOutException, DiscussionDisabledError, ThreadNotFoundError
|
||||
from .forms import CommentActionsForm, ThreadActionsForm
|
||||
from .forms import CommentActionsForm, ThreadActionsForm, UserOrdering
|
||||
from .pagination import DiscussionAPIPagination
|
||||
from .permissions import (
|
||||
can_delete,
|
||||
@@ -70,9 +68,21 @@ from .serializers import (
|
||||
DiscussionTopicSerializerV2,
|
||||
ThreadSerializer,
|
||||
TopicOrdering,
|
||||
UserStatsSerializer,
|
||||
get_context,
|
||||
)
|
||||
from .utils import discussion_open_for_user
|
||||
from ..django_comment_client.base.views import (
|
||||
track_comment_created_event,
|
||||
track_thread_created_event,
|
||||
track_voted_event,
|
||||
)
|
||||
from ..django_comment_client.utils import (
|
||||
get_group_id_for_user,
|
||||
get_user_role_names,
|
||||
has_discussion_privileges,
|
||||
is_commentable_divided,
|
||||
)
|
||||
|
||||
User = get_user_model()
|
||||
|
||||
@@ -244,6 +254,7 @@ def get_course(request, course_key):
|
||||
CourseNotFoundError: if the course does not exist or is not accessible
|
||||
to the requesting user
|
||||
"""
|
||||
|
||||
def _format_datetime(dt):
|
||||
"""
|
||||
Provide backwards compatible datetime formatting.
|
||||
@@ -576,7 +587,7 @@ def _get_users(discussion_entity_type, discussion_entity, username_profile_dict)
|
||||
|
||||
|
||||
def _add_additional_response_fields(
|
||||
request, serialized_discussion_entities, usernames, discussion_entity_type, include_profile_image
|
||||
request, serialized_discussion_entities, usernames, discussion_entity_type, include_profile_image
|
||||
):
|
||||
"""
|
||||
Adds additional data to serialized discussion thread/comment.
|
||||
@@ -998,7 +1009,7 @@ def _handle_voted_field(form_value, cc_content, api_content, request, context):
|
||||
context["cc_requester"].unvote(cc_content)
|
||||
api_content["vote_count"] -= 1
|
||||
track_voted_event(
|
||||
request, context["course"], cc_content, vote_value="up", undo_vote=False if form_value else True # lint-amnesty, pylint: disable=simplifiable-if-expression
|
||||
request, context["course"], cc_content, vote_value="up", undo_vote=not form_value
|
||||
)
|
||||
|
||||
|
||||
@@ -1411,3 +1422,55 @@ def delete_comment(request, comment_id):
|
||||
comment_deleted.send(sender=None, user=request.user, post=cc_comment)
|
||||
else:
|
||||
raise PermissionDenied
|
||||
|
||||
|
||||
@function_trace("get_course_discussion_user_stats")
|
||||
def get_course_discussion_user_stats(
|
||||
request,
|
||||
course_key_str: str,
|
||||
page: int,
|
||||
page_size: int,
|
||||
order_by: UserOrdering = None,
|
||||
) -> Dict:
|
||||
"""
|
||||
Get paginated course discussion stats for users in the course.
|
||||
|
||||
Args:
|
||||
request (Request): DRF request
|
||||
course_key_str (str): course key string
|
||||
page (int): Page number to fetch
|
||||
page_size (int): Number of items in each page
|
||||
order_by (UserOrdering): The ordering to use for the user stats
|
||||
|
||||
Returns:
|
||||
Paginated data of a user's discussion stats sorted based on the specified ordering.
|
||||
|
||||
"""
|
||||
course_key = CourseKey.from_string(course_key_str)
|
||||
is_privileged = has_discussion_privileges(user=request.user, course_id=course_key)
|
||||
if is_privileged:
|
||||
order_by = order_by or UserOrdering.BY_FLAGS
|
||||
else:
|
||||
order_by = order_by or UserOrdering.BY_ACTIVITY
|
||||
if order_by != UserOrdering.BY_ACTIVITY:
|
||||
raise ValidationError({"order_by": "Invalid value"})
|
||||
course_stats_response = get_course_user_stats(course_key, {
|
||||
'sort_key': str(order_by),
|
||||
'page': page,
|
||||
'per_page': page_size,
|
||||
})
|
||||
serializer = UserStatsSerializer(
|
||||
course_stats_response["user_stats"],
|
||||
context={"is_privileged": is_privileged},
|
||||
many=True,
|
||||
)
|
||||
|
||||
paginator = DiscussionAPIPagination(
|
||||
request,
|
||||
course_stats_response["page"],
|
||||
course_stats_response["num_pages"],
|
||||
course_stats_response["count"],
|
||||
)
|
||||
return paginator.get_paginated_response({
|
||||
"results": serializer.data,
|
||||
})
|
||||
|
||||
@@ -4,6 +4,7 @@ Discussion API forms
|
||||
import urllib.parse
|
||||
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.db.models import TextChoices
|
||||
from django.forms import BooleanField, CharField, ChoiceField, Form, IntegerField
|
||||
from opaque_keys import InvalidKeyError
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
@@ -15,11 +16,16 @@ from openedx.core.djangoapps.django_comment_common.models import (
|
||||
FORUM_ROLE_COMMUNITY_TA,
|
||||
FORUM_ROLE_GROUP_MODERATOR,
|
||||
FORUM_ROLE_MODERATOR,
|
||||
Role
|
||||
Role,
|
||||
)
|
||||
from openedx.core.djangoapps.util.forms import ExtendedNullBooleanField, MultiValueField
|
||||
|
||||
|
||||
class UserOrdering(TextChoices):
|
||||
BY_ACTIVITY = 'activity'
|
||||
BY_FLAGS = 'flagged'
|
||||
|
||||
|
||||
class _PaginationForm(Form):
|
||||
"""A form that includes pagination fields"""
|
||||
page = IntegerField(required=False, min_value=1)
|
||||
@@ -201,8 +207,8 @@ class CourseDiscussionRolesForm(CourseDiscussionSettingsForm):
|
||||
if course_id and rolename:
|
||||
try:
|
||||
role = Role.objects.get(name=rolename, course_id=course_id)
|
||||
except Role.DoesNotExist:
|
||||
raise ValidationError(f"Role '{rolename}' does not exist") # lint-amnesty, pylint: disable=raise-missing-from
|
||||
except Role.DoesNotExist as err:
|
||||
raise ValidationError(f"Role '{rolename}' does not exist") from err
|
||||
|
||||
self.cleaned_data['role'] = role
|
||||
return rolename
|
||||
@@ -218,3 +224,8 @@ class TopicListGetForm(Form):
|
||||
def clean_topic_id(self):
|
||||
topic_ids = self.cleaned_data.get("topic_id", None)
|
||||
return set(topic_ids.strip(',').split(',')) if topic_ids else None
|
||||
|
||||
|
||||
class CourseActivityStatsForm(_PaginationForm):
|
||||
"""Form for validating course activity stats API query parameters"""
|
||||
order_by = ChoiceField(choices=UserOrdering.choices, required=False)
|
||||
|
||||
@@ -682,15 +682,35 @@ class DiscussionRolesListSerializer(serializers.Serializer):
|
||||
|
||||
def create(self, validated_data):
|
||||
"""
|
||||
Overriden create abstract method
|
||||
Overridden create abstract method
|
||||
"""
|
||||
|
||||
def update(self, instance, validated_data):
|
||||
"""
|
||||
Overriden update abstract method
|
||||
Overridden update abstract method
|
||||
"""
|
||||
|
||||
|
||||
class UserStatsSerializer(serializers.Serializer):
|
||||
"""
|
||||
Serializer for course user stats.
|
||||
"""
|
||||
threads = serializers.IntegerField()
|
||||
replies = serializers.IntegerField()
|
||||
responses = serializers.IntegerField()
|
||||
active_flags = serializers.IntegerField()
|
||||
inactive_flags = serializers.IntegerField()
|
||||
username = serializers.CharField()
|
||||
|
||||
def to_representation(self, instance):
|
||||
"""Remove flag counts if user is not privileged."""
|
||||
data = super().to_representation(instance)
|
||||
if not self.context.get("is_privileged", False):
|
||||
data["active_flags"] = None
|
||||
data["inactive_flags"] = None
|
||||
return data
|
||||
|
||||
|
||||
class BlackoutDateSerializer(serializers.Serializer):
|
||||
"""
|
||||
Serializer for blackout dates.
|
||||
|
||||
@@ -4,9 +4,10 @@ Tests for Discussion API views
|
||||
|
||||
|
||||
import json
|
||||
import random
|
||||
from datetime import datetime
|
||||
from unittest import mock
|
||||
from urllib.parse import urlparse, urlencode
|
||||
from urllib.parse import parse_qs, urlparse, urlencode
|
||||
|
||||
import ddt
|
||||
import httpretty
|
||||
@@ -26,7 +27,11 @@ from common.djangoapps.course_modes.models import CourseMode
|
||||
from common.djangoapps.course_modes.tests.factories import CourseModeFactory
|
||||
from common.djangoapps.student.models import get_retired_username_by_username
|
||||
from common.djangoapps.student.roles import CourseInstructorRole, CourseStaffRole, GlobalStaff
|
||||
from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, SuperuserFactory, UserFactory
|
||||
from common.djangoapps.student.tests.factories import (
|
||||
CourseEnrollmentFactory,
|
||||
SuperuserFactory,
|
||||
UserFactory,
|
||||
)
|
||||
from common.djangoapps.util.testing import PatchMediaTypeMixin, UrlResetMixin
|
||||
from common.test.utils import disable_signal
|
||||
from lms.djangoapps.discussion.django_comment_client.tests.utils import (
|
||||
@@ -2664,3 +2669,90 @@ class CourseDiscussionRolesAPIViewTest(APITestCase, UrlResetMixin, ModuleStoreTe
|
||||
content = json.loads(response.content.decode('utf-8'))
|
||||
assertion = self.assertTrue if action == 'allow' else self.assertFalse
|
||||
assertion(any(user.username in x['username'] for x in content['results']))
|
||||
|
||||
|
||||
@ddt.ddt
|
||||
@httpretty.activate
|
||||
class CourseActivityStatsTest(ForumsEnableMixin, UrlResetMixin, CommentsServiceMockMixin, APITestCase):
|
||||
"""
|
||||
Tests for the course stats endpoint
|
||||
"""
|
||||
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
def setUp(self) -> None:
|
||||
super().setUp()
|
||||
self.course_key = 'course-v1:test+test+test'
|
||||
seed_permissions_roles(self.course_key)
|
||||
self.user = UserFactory(username='user')
|
||||
self.moderator = UserFactory(username='moderator')
|
||||
moderator_role = Role.objects.get(name="Moderator", course_id=self.course_key)
|
||||
moderator_role.users.add(self.moderator)
|
||||
self.stats = [
|
||||
{
|
||||
"active_flags": random.randint(0, 3),
|
||||
"inactive_flags": random.randint(0, 2),
|
||||
"replies": random.randint(0, 30),
|
||||
"responses": random.randint(0, 100),
|
||||
"threads": random.randint(0, 10),
|
||||
"username": f"user-{idx}"
|
||||
}
|
||||
for idx in range(10)
|
||||
]
|
||||
self.stats_without_flags = [{**stat, "active_flags": None, "inactive_flags": None} for stat in self.stats]
|
||||
self.register_course_stats_response(self.course_key, self.stats, 1, 3)
|
||||
self.url = reverse("discussion_course_activity_stats", kwargs={"course_key_string": self.course_key})
|
||||
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
def test_regular_user(self):
|
||||
"""
|
||||
Tests that for a regular user stats are returned without flag counts
|
||||
"""
|
||||
self.client.login(username=self.user.username, password='test')
|
||||
response = self.client.get(self.url)
|
||||
data = response.json()
|
||||
assert data["results"] == self.stats_without_flags
|
||||
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
def test_moderator_user(self):
|
||||
"""
|
||||
Tests that for a moderator user stats are returned with flag counts
|
||||
"""
|
||||
self.client.login(username=self.moderator.username, password='test')
|
||||
response = self.client.get(self.url)
|
||||
data = response.json()
|
||||
assert data["results"] == self.stats
|
||||
|
||||
@ddt.data(
|
||||
("moderator", "flagged", "flagged"),
|
||||
("moderator", "activity", "activity"),
|
||||
("moderator", None, "flagged"),
|
||||
("user", None, "activity"),
|
||||
("user", "activity", "activity"),
|
||||
)
|
||||
@ddt.unpack
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
def test_sorting(self, username, ordering_requested, ordering_performed):
|
||||
"""
|
||||
Test valid sorting options and defaults
|
||||
"""
|
||||
self.client.login(username=username, password='test')
|
||||
params = {}
|
||||
if ordering_requested:
|
||||
params = {"order_by": ordering_requested}
|
||||
self.client.get(self.url, params)
|
||||
assert urlparse(
|
||||
httpretty.last_request().path # lint-amnesty, pylint: disable=no-member
|
||||
).path == f"/api/v1/users/{self.course_key}/stats"
|
||||
assert parse_qs(
|
||||
urlparse(httpretty.last_request().path).query # lint-amnesty, pylint: disable=no-member
|
||||
).get("sort_key", None) == [ordering_performed]
|
||||
|
||||
@ddt.data("flagged", "xyz")
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
def test_sorting_error_regular_user(self, order_by):
|
||||
"""
|
||||
Test for invalid sorting options for regular users.
|
||||
"""
|
||||
self.client.login(username=self.user.username, password='test')
|
||||
response = self.client.get(self.url, {"order_by": order_by})
|
||||
assert "order_by" in response.json()["field_errors"]
|
||||
|
||||
@@ -277,6 +277,21 @@ class CommentsServiceMockMixin:
|
||||
status=200
|
||||
)
|
||||
|
||||
def register_course_stats_response(self, course_key, stats, page, num_pages):
|
||||
"""Register a mock response for GET on the CS user course stats instance endpoint"""
|
||||
assert httpretty.is_enabled(), 'httpretty must be enabled to mock calls.'
|
||||
httpretty.register_uri(
|
||||
httpretty.GET,
|
||||
f"http://localhost:4567/api/v1/users/{course_key}/stats",
|
||||
body=json.dumps({
|
||||
"user_stats": stats,
|
||||
"page": page,
|
||||
"num_pages": num_pages,
|
||||
"count": len(stats),
|
||||
}),
|
||||
status=200
|
||||
)
|
||||
|
||||
def register_subscription_response(self, user):
|
||||
"""
|
||||
Register a mock response for POST and DELETE on the CS user subscription
|
||||
|
||||
@@ -3,13 +3,13 @@
|
||||
Discussion API URLs
|
||||
"""
|
||||
|
||||
|
||||
from django.conf import settings
|
||||
from django.urls import include, path, re_path
|
||||
from rest_framework.routers import SimpleRouter
|
||||
|
||||
from lms.djangoapps.discussion.rest_api.views import (
|
||||
CommentViewSet,
|
||||
CourseActivityStatsView,
|
||||
CourseDiscussionRolesAPIView,
|
||||
CourseDiscussionSettingsAPIView,
|
||||
CourseTopicsView,
|
||||
@@ -33,6 +33,11 @@ urlpatterns = [
|
||||
CourseDiscussionSettingsAPIView.as_view(),
|
||||
name="discussion_course_settings",
|
||||
),
|
||||
re_path(
|
||||
fr"^v1/courses/{settings.COURSE_KEY_PATTERN}/activity_stats",
|
||||
CourseActivityStatsView.as_view(),
|
||||
name="discussion_course_activity_stats",
|
||||
),
|
||||
re_path(
|
||||
fr"^v1/courses/{settings.COURSE_ID_PATTERN}/upload$",
|
||||
UploadFileView.as_view(),
|
||||
|
||||
@@ -42,6 +42,7 @@ from ..rest_api.api import (
|
||||
delete_thread,
|
||||
get_comment_list,
|
||||
get_course,
|
||||
get_course_discussion_user_stats,
|
||||
get_course_topics,
|
||||
get_course_topics_v2,
|
||||
get_response_comments,
|
||||
@@ -54,11 +55,13 @@ from ..rest_api.api import (
|
||||
from ..rest_api.forms import (
|
||||
CommentGetForm,
|
||||
CommentListGetForm,
|
||||
CourseActivityStatsForm,
|
||||
CourseDiscussionRolesForm,
|
||||
CourseDiscussionSettingsForm,
|
||||
ThreadListGetForm,
|
||||
TopicListGetForm,
|
||||
UserCommentListGetForm,
|
||||
UserOrdering,
|
||||
)
|
||||
from ..rest_api.permissions import IsStaffOrCourseTeamOrEnrolled
|
||||
from ..rest_api.serializers import (
|
||||
@@ -105,6 +108,79 @@ class CourseView(DeveloperErrorViewMixin, APIView):
|
||||
return Response(get_course(request, course_key))
|
||||
|
||||
|
||||
@view_auth_classes()
|
||||
class CourseActivityStatsView(DeveloperErrorViewMixin, APIView):
|
||||
"""
|
||||
**Use Cases**
|
||||
|
||||
Fetch statistics about a user's activity in a course.
|
||||
|
||||
**Example Requests**:
|
||||
|
||||
GET /api/discussion/v1/courses/course-v1:ExampleX+Subject101+2015/activity_stats?order_by=activity
|
||||
|
||||
**Response Values**:
|
||||
|
||||
|
||||
**Example Response**
|
||||
```json
|
||||
{
|
||||
"pagination": {
|
||||
"count": 3,
|
||||
"next": null,
|
||||
"num_pages": 1,
|
||||
"previous": null
|
||||
},
|
||||
"results": [
|
||||
{
|
||||
"active_flags": 3,
|
||||
"inactive_flags": 0,
|
||||
"replies": 13,
|
||||
"responses": 21,
|
||||
"threads": 32,
|
||||
"username": "edx"
|
||||
},
|
||||
{
|
||||
"active_flags": 1,
|
||||
"inactive_flags": 0,
|
||||
"replies": 6,
|
||||
"responses": 8,
|
||||
"threads": 13,
|
||||
"username": "honor"
|
||||
},
|
||||
...
|
||||
]
|
||||
}
|
||||
```
|
||||
"""
|
||||
|
||||
authentication_classes = (
|
||||
JwtAuthentication,
|
||||
BearerAuthenticationAllowInactiveUser,
|
||||
SessionAuthenticationAllowInactiveUser,
|
||||
)
|
||||
permission_classes = (
|
||||
permissions.IsAuthenticated,
|
||||
IsStaffOrCourseTeamOrEnrolled,
|
||||
)
|
||||
|
||||
def get(self, request, course_key_string):
|
||||
"""Implements the GET method as described in the class docstring."""
|
||||
form_query_string = CourseActivityStatsForm(request.query_params)
|
||||
if not form_query_string.is_valid():
|
||||
raise ValidationError(form_query_string.errors)
|
||||
order_by = form_query_string.cleaned_data.get('order_by', None)
|
||||
order_by = UserOrdering(order_by) if order_by else None
|
||||
data = get_course_discussion_user_stats(
|
||||
request,
|
||||
course_key_string,
|
||||
form_query_string.cleaned_data['page'],
|
||||
form_query_string.cleaned_data['page_size'],
|
||||
order_by,
|
||||
)
|
||||
return data
|
||||
|
||||
|
||||
@view_auth_classes()
|
||||
class CourseTopicsView(DeveloperErrorViewMixin, APIView):
|
||||
"""
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
"""Provides base Commentable model class"""
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import Dict
|
||||
from typing import Dict, Optional
|
||||
|
||||
from edx_django_utils.monitoring import function_trace
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
|
||||
from openedx.core.djangoapps.django_comment_common.comment_client import settings
|
||||
@@ -39,3 +40,50 @@ def get_course_commentable_counts(course_key: CourseKey) -> Dict[str, Dict[str,
|
||||
metric_action='commentable_stats.retrieve',
|
||||
)
|
||||
return response
|
||||
|
||||
|
||||
@function_trace("get_course_user_stats")
|
||||
def get_course_user_stats(course_key: CourseKey, params: Optional[Dict] = None) -> Dict[str, Dict[str, int]]:
|
||||
"""
|
||||
Get stats about a user's participation in a course.
|
||||
|
||||
Args:
|
||||
course_key (str|CourseKey): course key for which stats are needed.
|
||||
params (Optional[Dict]): pagination and sorting query parameters.
|
||||
|
||||
Returns:
|
||||
A mapping of user ids to stats about the user.
|
||||
|
||||
e.g.
|
||||
{
|
||||
"user_stats" [
|
||||
{
|
||||
"active_flags": 2,
|
||||
"inactive_flags": 0,
|
||||
"replies": 3,
|
||||
"responses": 2,
|
||||
"threads": 7,
|
||||
"username": "edx"
|
||||
},
|
||||
...
|
||||
],
|
||||
"num_pages": 12,
|
||||
"page": 3,
|
||||
"count": 124
|
||||
...
|
||||
}
|
||||
|
||||
"""
|
||||
if params is None:
|
||||
params = {}
|
||||
url = f"{settings.PREFIX}/users/{course_key}/stats"
|
||||
return perform_request(
|
||||
'get',
|
||||
url,
|
||||
params,
|
||||
metric_action='user.course_stats',
|
||||
metric_tags=[
|
||||
f"course_key:{course_key}",
|
||||
"function:get_course_user_stats",
|
||||
],
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user