diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index 60186ce738..f3ecab0eb4 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -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, + }) diff --git a/lms/djangoapps/discussion/rest_api/forms.py b/lms/djangoapps/discussion/rest_api/forms.py index 5e55391158..b4f7008453 100644 --- a/lms/djangoapps/discussion/rest_api/forms.py +++ b/lms/djangoapps/discussion/rest_api/forms.py @@ -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) diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index 538411556d..75da9986b6 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -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. diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 9a24c81c59..f47b5e1d6b 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -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"] diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index 23615e43ac..1e117cb4c1 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -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 diff --git a/lms/djangoapps/discussion/rest_api/urls.py b/lms/djangoapps/discussion/rest_api/urls.py index f780046d79..121ffff1ee 100644 --- a/lms/djangoapps/discussion/rest_api/urls.py +++ b/lms/djangoapps/discussion/rest_api/urls.py @@ -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(), diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 788629b5f5..6501dad470 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -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): """ diff --git a/openedx/core/djangoapps/django_comment_common/comment_client/course.py b/openedx/core/djangoapps/django_comment_common/comment_client/course.py index ff3eaddab7..f1f9afc69d 100644 --- a/openedx/core/djangoapps/django_comment_common/comment_client/course.py +++ b/openedx/core/djangoapps/django_comment_common/comment_client/course.py @@ -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", + ], + )