From ffe3ee3869832ace59a9a6ab8918422bb4ccdc33 Mon Sep 17 00:00:00 2001 From: Aayush Agrawal Date: Fri, 11 Sep 2020 17:02:48 +0530 Subject: [PATCH] feat: Adds additional data and filters to discussions API This change adds three new filters to the threads API. They are: * Filtering only threads that are flagged for abuse * Filtering by the thread type (discussion or question) * Filtering by the thread author In addition it also adds a new ``abuse_flagged_count`` field for threads. It returns a count of the number of comments in a thread that are flagged for abuse. This is only visible to users that have moderator privileges or higher. Finally it also adds a ``abuse_flagged_any_user`` field that is set if any user has flagged a thread. This field too, is only visible to moderators or above. Co-authored-by: Kshitij Sobti --- cms/djangoapps/contentstore/views/item.py | 1 + lms/djangoapps/discussion/rest_api/api.py | 123 +++++++----- lms/djangoapps/discussion/rest_api/forms.py | 6 + .../discussion/rest_api/serializers.py | 38 +++- .../discussion/rest_api/tests/test_api.py | 184 +++++++++++++++--- .../discussion/rest_api/tests/test_forms.py | 28 ++- .../rest_api/tests/test_serializers.py | 18 +- .../discussion/rest_api/tests/test_views.py | 20 +- .../discussion/rest_api/tests/utils.py | 4 +- lms/djangoapps/discussion/rest_api/views.py | 58 ++++-- 10 files changed, 366 insertions(+), 114 deletions(-) diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 3dcc3e4cec..a0f6ef1f6d 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -30,6 +30,7 @@ from xblock.fields import Scope from cms.djangoapps.contentstore.config.waffle import SHOW_REVIEW_RULES_FLAG from cms.djangoapps.models.settings.course_grading import CourseGradingModel +from cms.djangoapps.xblock_config.models import CourseEditLTIFieldsEnabledFlag from cms.lib.xblock.authoring_mixin import VISIBILITY_VIEW from common.djangoapps.edxmako.shortcuts import render_to_string from common.djangoapps.static_replace import replace_static_urls diff --git a/lms/djangoapps/discussion/rest_api/api.py b/lms/djangoapps/discussion/rest_api/api.py index 66fa5367ca..50b1ed12d2 100644 --- a/lms/djangoapps/discussion/rest_api/api.py +++ b/lms/djangoapps/discussion/rest_api/api.py @@ -2,52 +2,23 @@ Discussion API internal interface """ - import itertools from collections import defaultdict from enum import Enum +from typing import List, Literal, Optional +from urllib.parse import urlencode, urlunparse +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 opaque_keys import InvalidKeyError from opaque_keys.edx.locator import CourseKey from rest_framework.exceptions import PermissionDenied -from six.moves.urllib.parse import urlencode, urlunparse +from rest_framework.request import Request from lms.djangoapps.courseware.courses import get_course_with_access from lms.djangoapps.courseware.exceptions import CourseAccessRedirect -from lms.djangoapps.discussion.django_comment_client.base.views import ( - track_comment_created_event, - track_thread_created_event, - track_voted_event -) -from lms.djangoapps.discussion.django_comment_client.utils import ( - get_accessible_discussion_xblocks, - get_group_id_for_user, - is_commentable_divided, -) -from lms.djangoapps.discussion.rest_api.exceptions import ( - CommentNotFoundError, - ThreadNotFoundError, - DiscussionDisabledError, - DiscussionBlackOutException -) -from lms.djangoapps.discussion.rest_api.forms import CommentActionsForm, ThreadActionsForm -from lms.djangoapps.discussion.rest_api.pagination import DiscussionAPIPagination -from lms.djangoapps.discussion.rest_api.permissions import ( - can_delete, - get_editable_fields, - get_initializable_comment_fields, - get_initializable_thread_fields -) -from lms.djangoapps.discussion.rest_api.serializers import ( - CommentSerializer, - DiscussionTopicSerializer, - ThreadSerializer, - get_context -) -from lms.djangoapps.discussion.rest_api.utils import discussion_open_for_user from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment from openedx.core.djangoapps.django_comment_common.comment_client.thread import Thread from openedx.core.djangoapps.django_comment_common.comment_client.utils import CommentClientRequestError @@ -60,10 +31,48 @@ from openedx.core.djangoapps.django_comment_common.signals import ( thread_created, thread_deleted, thread_edited, - thread_voted + thread_voted, ) from openedx.core.djangoapps.user_api.accounts.api import get_account_settings from openedx.core.lib.exceptions import CourseNotFoundError, DiscussionNotFoundError, PageNotFoundError +from .exceptions import ( + CommentNotFoundError, + DiscussionBlackOutException, + DiscussionDisabledError, + ThreadNotFoundError, +) +from .forms import CommentActionsForm, ThreadActionsForm +from .pagination import DiscussionAPIPagination +from .permissions import ( + can_delete, + get_editable_fields, + get_initializable_comment_fields, + get_initializable_thread_fields, +) +from .serializers import ( + CommentSerializer, + DiscussionTopicSerializer, + ThreadSerializer, + 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_accessible_discussion_xblocks, + get_group_id_for_user, + is_commentable_divided, +) + + +User = get_user_model() + +ThreadType = Literal["discussion", "question"] +ViewType = Literal["unread", "unanswered"] +ThreadOrderingType = Literal["last_activity_at", "comment_count", "vote_count"] class DiscussionTopic: @@ -75,7 +84,7 @@ class DiscussionTopic: 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 + self.children: List[DiscussionTopic] = children or [] # children are of same type i.e. DiscussionTopic class DiscussionEntity(Enum): @@ -517,17 +526,20 @@ def _serialize_discussion_entities(request, context, discussion_entities, reques def get_thread_list( - request, - course_key, - page, - page_size, - topic_id_list=None, - text_search=None, - following=False, - view=None, - order_by="last_activity_at", - order_direction="desc", - requested_fields=None, + request: Request, + course_key: CourseKey, + page: int, + page_size: int, + topic_id_list: List[str] = None, + text_search: Optional[str] = None, + following: Optional[bool] = False, + author: Optional[str] = None, + thread_type: Optional[ThreadType] = None, + flagged: Optional[bool] = None, + view: Optional[ViewType] = None, + order_by: ThreadOrderingType = "last_activity_at", + order_direction: Literal["desc"] = "desc", + requested_fields: Optional[List[Literal["profile_image"]]] = None, ): """ Return the list of all discussion threads pertaining to the given course @@ -541,6 +553,9 @@ def get_thread_list( topic_id_list: The list of topic_ids to get the discussion threads for text_search A text search query string to match following: If true, retrieve only threads the requester is following + author: If provided, retrieve only threads by this author + thread_type: filter for "discussion" or "question threads + flagged: filter for only threads that are flagged view: filters for either "unread" or "unanswered" threads order_by: The key in which to sort the threads by. The only values are "last_activity_at", "comment_count", and "vote_count". The default is @@ -584,6 +599,18 @@ def get_thread_list( course = _get_course(course_key, request.user) context = get_context(course, request) + author_id = None + if author: + try: + author_id = User.objects.get(username=author).id + except User.DoesNotExist: + # Raising an error for a missing user leaks the presence of a username, + # so just return an empty response. + return DiscussionAPIPagination(request, 0, 1).get_paginated_response({ + "results": [], + "text_search_rewrite": None, + }) + query_params = { "user_id": str(request.user.id), "group_id": ( @@ -594,6 +621,10 @@ def get_thread_list( "per_page": page_size, "text": text_search, "sort_key": cc_map.get(order_by), + "author_id": author_id, + "flagged": flagged, + "thread_type": thread_type, + "count_flagged": context["is_requester_privileged"] or None, } if view: diff --git a/lms/djangoapps/discussion/rest_api/forms.py b/lms/djangoapps/discussion/rest_api/forms.py index 861862192e..d69fff5061 100644 --- a/lms/djangoapps/discussion/rest_api/forms.py +++ b/lms/djangoapps/discussion/rest_api/forms.py @@ -42,6 +42,12 @@ class ThreadListGetForm(_PaginationForm): topic_id = MultiValueField(required=False) text_search = CharField(required=False) following = ExtendedNullBooleanField(required=False) + author = CharField(required=False) + thread_type = ChoiceField( + choices=[(choice, choice) for choice in ["discussion", "question"]], + required=False, + ) + flagged = ExtendedNullBooleanField(required=False) view = ChoiceField( choices=[(choice, choice) for choice in ["unread", "unanswered"]], required=False, diff --git a/lms/djangoapps/discussion/rest_api/serializers.py b/lms/djangoapps/discussion/rest_api/serializers.py index b1936f55c1..be6eca5218 100644 --- a/lms/djangoapps/discussion/rest_api/serializers.py +++ b/lms/djangoapps/discussion/rest_api/serializers.py @@ -2,12 +2,12 @@ Discussion API serializers """ +from urllib.parse import urlencode, urlunparse -from django.contrib.auth.models import User as DjangoUser # lint-amnesty, pylint: disable=imported-auth-user +from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from django.urls import reverse from rest_framework import serializers -from six.moves.urllib.parse import urlencode, urlunparse from common.djangoapps.student.models import get_user_by_username_or_email from lms.djangoapps.discussion.django_comment_client.utils import ( @@ -16,12 +16,12 @@ from lms.djangoapps.discussion.django_comment_client.utils import ( get_group_id_for_user, get_group_name, get_group_names_by_id, - is_comment_too_deep + is_comment_too_deep, ) from lms.djangoapps.discussion.rest_api.permissions import ( NON_UPDATABLE_COMMENT_FIELDS, NON_UPDATABLE_THREAD_FIELDS, - get_editable_fields + get_editable_fields, ) from lms.djangoapps.discussion.rest_api.render import render_body from lms.djangoapps.discussion.views import get_divided_discussions @@ -34,9 +34,11 @@ from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_ADMINISTRATOR, FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR, - Role + Role, ) +User = get_user_model() + def get_context(course, request, thread=None): """ @@ -208,6 +210,7 @@ class ThreadSerializer(_ContentSerializer): source="thread_type", choices=[(val, val) for val in ["discussion", "question"]] ) + abuse_flagged_count = serializers.SerializerMethodField(required=False) title = serializers.CharField(validators=[validate_not_blank]) pinned = serializers.SerializerMethodField(read_only=True) closed = serializers.BooleanField(read_only=True) @@ -230,6 +233,15 @@ class ThreadSerializer(_ContentSerializer): if self.instance and self.instance.get("pinned") is None: self.instance["pinned"] = False + def get_abuse_flagged_count(self, obj): + """ + Returns the number of users that flagged content as abusive only if user has staff permissions + """ + course = self.context.get('course', None) + is_requester_privileged = self.context.get('is_requester_privileged') + if course and is_requester_privileged: + return obj.get("abuse_flagged_count") + def get_pinned(self, obj): """ Compensate for the fact that some threads in the comments service do @@ -325,6 +337,7 @@ class CommentSerializer(_ContentSerializer): endorsed_at = serializers.SerializerMethodField() child_count = serializers.IntegerField(read_only=True) children = serializers.SerializerMethodField(required=False) + abuse_flagged_any_user = serializers.SerializerMethodField(required=False) non_updatable_fields = NON_UPDATABLE_COMMENT_FIELDS @@ -351,7 +364,7 @@ class CommentSerializer(_ContentSerializer): self._is_anonymous(self.context["thread"]) and not self._is_user_privileged(endorser_id) ): - return DjangoUser.objects.get(id=endorser_id).username + return User.objects.get(id=endorser_id).username return None def get_endorsed_by_label(self, obj): @@ -390,6 +403,17 @@ class CommentSerializer(_ContentSerializer): return data + def get_abuse_flagged_any_user(self, obj): + """ + Returns a boolean indicating whether any user has flagged the + content as abusive. + """ + course = self.context.get('course', None) + is_requester_privileged = self.context.get('is_requester_privileged') + + if course and is_requester_privileged: + return len(obj.get("abuse_flaggers", [])) > 0 + def validate(self, attrs): """ Ensure that parent_id identifies a comment that is actually in the @@ -566,7 +590,7 @@ class DiscussionRolesSerializer(serializers.Serializer): try: self.user = get_user_by_username_or_email(user_id) return user_id - except DjangoUser.DoesNotExist: + except User.DoesNotExist: raise ValidationError(f"'{user_id}' is not a valid student identifier") # lint-amnesty, pylint: disable=raise-missing-from def validate(self, attrs): diff --git a/lms/djangoapps/discussion/rest_api/tests/test_api.py b/lms/djangoapps/discussion/rest_api/tests/test_api.py index 7d16765c2e..13dce7c7df 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_api.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_api.py @@ -16,10 +16,18 @@ from django.test.client import RequestFactory from opaque_keys.edx.locator import CourseLocator from pytz import UTC from rest_framework.exceptions import PermissionDenied +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +from xmodule.partitions.partitions import Group, UserPartition -from common.djangoapps.student.tests.factories import BetaTesterFactory -from common.djangoapps.student.tests.factories import CourseEnrollmentFactory, UserFactory -from common.djangoapps.student.tests.factories import StaffFactory +from common.djangoapps.student.tests.factories import ( + BetaTesterFactory, + CourseEnrollmentFactory, + StaffFactory, + UserFactory, +) from common.djangoapps.util.testing import UrlResetMixin from common.test.utils import MockSignalHandlerMixin, disable_signal from lms.djangoapps.discussion.django_comment_client.tests.utils import ForumsEnableMixin @@ -35,19 +43,19 @@ from lms.djangoapps.discussion.rest_api.api import ( get_thread, get_thread_list, update_comment, - update_thread + update_thread, ) from lms.djangoapps.discussion.rest_api.exceptions import ( CommentNotFoundError, + DiscussionBlackOutException, DiscussionDisabledError, ThreadNotFoundError, - DiscussionBlackOutException ) from lms.djangoapps.discussion.rest_api.tests.utils import ( CommentsServiceMockMixin, make_minimal_cs_comment, make_minimal_cs_thread, - make_paginated_api_response + make_paginated_api_response, ) from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory @@ -56,14 +64,9 @@ from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR, FORUM_ROLE_STUDENT, - Role + Role, ) from openedx.core.lib.exceptions import CourseNotFoundError, PageNotFoundError -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase, SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory -from xmodule.partitions.partitions import Group, UserPartition def _remove_discussion_tab(course, user_id): @@ -730,6 +733,7 @@ class GetThreadListTest(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMix "read": True, "created_at": "2015-04-28T00:00:00Z", "updated_at": "2015-04-28T11:11:11Z", + "abuse_flagged_count": None, }), self.expected_thread_data({ "id": "test_thread_id_1", @@ -753,6 +757,7 @@ class GetThreadListTest(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMix "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_1&endorsed=False" ), "editable_fields": ["abuse_flagged", "following", "read", "voted"], + "abuse_flagged_count": None, }), ] @@ -836,6 +841,135 @@ class GetThreadListTest(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMix "text": ["test search string"], }) + def test_filter_threads_by_author(self): + thread = make_minimal_cs_thread() + self.register_get_threads_response([thread], page=1, num_pages=10) + thread_results = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + author=self.user.username, + ).data.get('results') + assert len(thread_results) == 1 + + expected_last_query_params = { + "user_id": [str(self.user.id)], + "course_id": [str(self.course.id)], + "sort_key": ["activity"], + "page": ["1"], + "per_page": ["10"], + "author_id": [str(self.user.id)], + } + + self.assert_last_query_params(expected_last_query_params) + + def test_filter_threads_by_missing_author(self): + self.register_get_threads_response([make_minimal_cs_thread()], page=1, num_pages=10) + results = get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + author="a fake and missing username", + ).data.get('results') + assert len(results) == 0 + + @ddt.data('question', 'discussion', None) + def test_thread_type(self, thread_type): + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + + self.register_get_threads_response([], page=1, num_pages=0) + assert get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + thread_type=thread_type, + ).data == expected_result + + expected_last_query_params = { + "user_id": [str(self.user.id)], + "course_id": [str(self.course.id)], + "sort_key": ["activity"], + "page": ["1"], + "per_page": ["10"], + "thread_type": [thread_type], + } + + if thread_type is None: + del expected_last_query_params["thread_type"] + + self.assert_last_query_params(expected_last_query_params) + + @ddt.data(True, False, None) + def test_flagged(self, flagged_boolean): + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + + self.register_get_threads_response([], page=1, num_pages=0) + assert get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + flagged=flagged_boolean, + ).data == expected_result + + expected_last_query_params = { + "user_id": [str(self.user.id)], + "course_id": [str(self.course.id)], + "sort_key": ["activity"], + "page": ["1"], + "per_page": ["10"], + "flagged": [str(flagged_boolean)], + } + + if flagged_boolean is None: + del expected_last_query_params["flagged"] + + self.assert_last_query_params(expected_last_query_params) + + @ddt.data( + (FORUM_ROLE_ADMINISTRATOR, True), + (FORUM_ROLE_MODERATOR, True), + (FORUM_ROLE_COMMUNITY_TA, True), + (FORUM_ROLE_STUDENT, False), + ) + @ddt.unpack + def test_flagged_count(self, role, is_count_flagged_flag_set): + expected_result = make_paginated_api_response( + results=[], count=0, num_pages=0, next_link=None, previous_link=None + ) + expected_result.update({"text_search_rewrite": None}) + + _assign_role_to_user(self.user, self.course.id, role=role) + + self.register_get_threads_response([], page=1, num_pages=0) + get_thread_list( + self.request, + self.course.id, + page=1, + page_size=10, + ) + + expected_last_query_params = { + "user_id": [str(self.user.id)], + "course_id": [str(self.course.id)], + "sort_key": ["activity"], + "page": ["1"], + "per_page": ["10"], + } + if is_count_flagged_flag_set: + expected_last_query_params["count_flagged"] = ["True"] + + self.assert_last_query_params(expected_last_query_params) + def test_following(self): self.register_subscribed_threads_response(self.user, [], page=1, num_pages=0) result = get_thread_list( @@ -1196,6 +1330,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": False, + "abuse_flagged_any_user": None, "voted": False, "vote_count": 4, "editable_fields": ["abuse_flagged", "voted"], @@ -1217,6 +1352,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": True, + "abuse_flagged_any_user": None, "voted": False, "vote_count": 7, "editable_fields": ["abuse_flagged", "voted"], @@ -1811,6 +1947,7 @@ class CreateCommentTest( "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": False, + "abuse_flagged_any_user": None, "voted": False, "vote_count": 0, "children": [], @@ -1891,29 +2028,25 @@ class CreateCommentTest( "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": False, + "abuse_flagged_any_user": False, "voted": False, "vote_count": 0, "children": [], "editable_fields": ["abuse_flagged", "endorsed", "raw_body", "voted"], "child_count": 0, } - self.assertEqual(actual, expected) + assert actual == expected expected_url = ( f"/api/v1/comments/{parent_id}" if parent_id else "/api/v1/threads/test_thread/comments" ) - self.assertEqual( - urlparse(httpretty.last_request().path).path, # lint-amnesty, pylint: disable=no-member - expected_url - ) - self.assertEqual( - httpretty.last_request().parsed_body, # lint-amnesty, pylint: disable=no-member - { - "course_id": [str(self.course.id)], - "body": ["Test body"], - "user_id": [str(self.user.id)] - } - ) + assert urlparse(httpretty.last_request().path).path == expected_url # pylint: disable=no-member + assert httpretty.last_request().parsed_body == { # pylint: disable=no-member + "course_id": [str(self.course.id)], + "body": ["Test body"], + "user_id": [str(self.user.id)] + } + expected_event_name = ( "edx.forum.comment.created" if parent_id else "edx.forum.response.created" @@ -2528,6 +2661,7 @@ class UpdateCommentTest( "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": False, + "abuse_flagged_any_user": None, "voted": False, "vote_count": 0, "children": [], diff --git a/lms/djangoapps/discussion/rest_api/tests/test_forms.py b/lms/djangoapps/discussion/rest_api/tests/test_forms.py index 0443a19c3c..fc2c41b81c 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_forms.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_forms.py @@ -5,11 +5,11 @@ Tests for Discussion API forms import itertools from unittest import TestCase +from urllib.parse import urlencode import ddt from django.http import QueryDict from opaque_keys.edx.locator import CourseLocator -from six.moves.urllib.parse import urlencode from lms.djangoapps.discussion.rest_api.forms import CommentListGetForm, ThreadListGetForm from openedx.core.djangoapps.util.test_forms import FormTestMixin @@ -66,6 +66,9 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase): 'topic_id': set(), 'text_search': '', 'following': None, + 'author': '', + 'thread_type': '', + 'flagged': None, 'view': '', 'order_by': 'last_activity_at', 'order_direction': 'desc', @@ -94,6 +97,29 @@ class ThreadListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase): self.form_data.setlist("topic_id", ["", "not empty"]) self.assert_error("topic_id", "This field cannot be empty.") + @ddt.data("discussion", "question") + def test_thread_type(self, value): + self.form_data["thread_type"] = value + self.assert_field_value("thread_type", value) + + def test_thread_type_invalid(self): + self.form_data["thread_type"] = "invalid-option" + self.assert_error("thread_type", "Select a valid choice. invalid-option is not one of the available choices.") + + @ddt.data("True", "true", 1, True) + def test_flagged_true(self, value): + self.form_data["flagged"] = value + self.assert_field_value("flagged", True) + + @ddt.data("False", "false", 0, False) + def test_flagged_false(self, value): + self.form_data["flagged"] = value + self.assert_field_value("flagged", False) + + def test_invalid_flagged(self): + self.form_data["flagged"] = "invalid-boolean" + self.assert_error("flagged", "Invalid Boolean Value.") + @ddt.data("True", "true", 1, True) def test_following_true(self, value): self.form_data["following"] = value diff --git a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py index 6e1d3e7165..353aee0e5f 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_serializers.py @@ -2,14 +2,17 @@ Tests for Discussion API serializers """ - import itertools from unittest import mock +from urllib.parse import urlparse import ddt import httpretty from django.test.client import RequestFactory -from six.moves.urllib.parse import urlparse +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory from common.djangoapps.student.tests.factories import UserFactory from common.djangoapps.util.testing import UrlResetMixin @@ -18,7 +21,7 @@ from lms.djangoapps.discussion.rest_api.serializers import CommentSerializer, Th from lms.djangoapps.discussion.rest_api.tests.utils import ( CommentsServiceMockMixin, make_minimal_cs_comment, - make_minimal_cs_thread + make_minimal_cs_thread, ) from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory from openedx.core.djangoapps.django_comment_common.comment_client.comment import Comment @@ -28,12 +31,8 @@ from openedx.core.djangoapps.django_comment_common.models import ( FORUM_ROLE_COMMUNITY_TA, FORUM_ROLE_MODERATOR, FORUM_ROLE_STUDENT, - Role + Role, ) -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory @ddt.ddt @@ -186,6 +185,7 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe "unread_comment_count": 3, "pinned": True, "editable_fields": ["abuse_flagged", "following", "read", "voted"], + "abuse_flagged_count": None, }) assert self.serialize(thread) == expected @@ -306,12 +306,14 @@ class CommentSerializerTest(SerializerTestMixin, SharedModuleStoreTestCase): "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": False, + "abuse_flagged_any_user": None, "voted": False, "vote_count": 4, "children": [], "editable_fields": ["abuse_flagged", "voted"], "child_count": 0, } + assert self.serialize(comment) == expected @ddt.data( diff --git a/lms/djangoapps/discussion/rest_api/tests/test_views.py b/lms/djangoapps/discussion/rest_api/tests/test_views.py index 704a9b5ad9..e4a46a1a4d 100644 --- a/lms/djangoapps/discussion/rest_api/tests/test_views.py +++ b/lms/djangoapps/discussion/rest_api/tests/test_views.py @@ -6,6 +6,7 @@ Tests for Discussion API views import json from datetime import datetime from unittest import mock +from urllib.parse import urlparse import ddt import httpretty @@ -14,7 +15,10 @@ from opaque_keys.edx.keys import CourseKey from pytz import UTC from rest_framework.parsers import JSONParser from rest_framework.test import APIClient, APITestCase -from six.moves.urllib.parse import urlparse +from xmodule.modulestore import ModuleStoreEnum +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls from common.djangoapps.course_modes.models import CourseMode from common.djangoapps.course_modes.tests.factories import CourseModeFactory @@ -25,7 +29,7 @@ from common.test.utils import disable_signal from lms.djangoapps.discussion.django_comment_client.tests.utils import ( ForumsEnableMixin, config_course_discussions, - topic_name_to_id + topic_name_to_id, ) from lms.djangoapps.discussion.rest_api import api from lms.djangoapps.discussion.rest_api.tests.utils import ( @@ -33,7 +37,7 @@ from lms.djangoapps.discussion.rest_api.tests.utils import ( ProfileImageTestMixin, make_minimal_cs_comment, make_minimal_cs_thread, - make_paginated_api_response + make_paginated_api_response, ) from openedx.core.djangoapps.course_groups.tests.helpers import config_course_cohorts from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role @@ -42,10 +46,6 @@ from openedx.core.djangoapps.oauth_dispatch.jwt import create_jwt_for_user from openedx.core.djangoapps.oauth_dispatch.tests.factories import AccessTokenFactory, ApplicationFactory from openedx.core.djangoapps.user_api.accounts.image_helpers import get_profile_image_storage from openedx.core.djangoapps.user_api.models import RetirementState, UserRetirementStatus -from xmodule.modulestore import ModuleStoreEnum -from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory, check_mongo_calls class DiscussionAPIViewTestMixin(ForumsEnableMixin, CommentsServiceMockMixin, UrlResetMixin): @@ -549,6 +549,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro "voted": True, "author": self.author.username, "editable_fields": ["abuse_flagged", "following", "read", "voted"], + "abuse_flagged_count": None, })] self.register_get_threads_response(source_threads, page=1, num_pages=2) response = self.client.get(self.url, {"course_id": str(self.course.id), "following": ""}) @@ -1112,6 +1113,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": False, + "abuse_flagged_any_user": None, "voted": False, "vote_count": 0, "children": [], @@ -1499,6 +1501,7 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": False, + "abuse_flagged_any_user": None, "voted": False, "vote_count": 0, "children": [], @@ -1583,6 +1586,7 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes "endorsed_by_label": None, "endorsed_at": None, "abuse_flagged": False, + "abuse_flagged_any_user": None, "voted": False, "vote_count": 0, "children": [], @@ -1642,6 +1646,7 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes response_data = json.loads(response.content.decode('utf-8')) assert response_data == self.expected_response_data({ 'abuse_flagged': value, + "abuse_flagged_any_user": None, 'editable_fields': ['abuse_flagged'] }) @@ -1768,6 +1773,7 @@ class CommentViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase "voted": False, "vote_count": 0, "abuse_flagged": False, + "abuse_flagged_any_user": None, "editable_fields": ["abuse_flagged", "raw_body", "voted"], "child_count": 0, } diff --git a/lms/djangoapps/discussion/rest_api/tests/utils.py b/lms/djangoapps/discussion/rest_api/tests/utils.py index 45c145139f..509555738b 100644 --- a/lms/djangoapps/discussion/rest_api/tests/utils.py +++ b/lms/djangoapps/discussion/rest_api/tests/utils.py @@ -358,7 +358,7 @@ class CommentsServiceMockMixin: """ actual_params = dict(httpretty_request.querystring) actual_params.pop("request_id") # request_id is random - assert actual_params == expected_params + assert actual_params == expected_params, f"""[\n\t{actual_params} \n\t{expected_params}\n]""" def assert_last_query_params(self, expected_params): """ @@ -388,6 +388,7 @@ class CommentsServiceMockMixin: "raw_body": "Test body", "rendered_body": "

Test body

", "abuse_flagged": False, + "abuse_flagged_count": None, "voted": False, "vote_count": 0, "editable_fields": ["abuse_flagged", "following", "raw_body", "read", "title", "topic_id", "type", "voted"], @@ -438,6 +439,7 @@ def make_minimal_cs_thread(overrides=None): "pinned": False, "closed": False, "abuse_flaggers": [], + "abuse_flagged_count": None, "votes": {"up_count": 0}, "comments_count": 0, "unread_comments_count": 0, diff --git a/lms/djangoapps/discussion/rest_api/views.py b/lms/djangoapps/discussion/rest_api/views.py index 418df91bf1..26abbf561d 100644 --- a/lms/djangoapps/discussion/rest_api/views.py +++ b/lms/djangoapps/discussion/rest_api/views.py @@ -4,7 +4,8 @@ Discussion API views import logging -from django.contrib.auth.models import User # lint-amnesty, pylint: disable=imported-auth-user + +from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser @@ -15,9 +16,17 @@ from rest_framework.parsers import JSONParser from rest_framework.response import Response from rest_framework.views import APIView from rest_framework.viewsets import ViewSet +from xmodule.modulestore.django import modulestore -from lms.djangoapps.discussion.django_comment_client.utils import available_division_schemes # lint-amnesty, pylint: disable=unused-import -from lms.djangoapps.discussion.rest_api.api import ( +from lms.djangoapps.instructor.access import update_forum_role +from openedx.core.djangoapps.django_comment_common import comment_client +from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings, Role +from openedx.core.djangoapps.user_api.accounts.permissions import CanReplaceUsername, CanRetireUser +from openedx.core.djangoapps.user_api.models import UserRetirementStatus +from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser +from openedx.core.lib.api.parsers import MergePatchParser +from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes +from ..rest_api.api import ( create_comment, create_thread, delete_comment, @@ -29,34 +38,25 @@ from lms.djangoapps.discussion.rest_api.api import ( get_thread, get_thread_list, update_comment, - update_thread + update_thread, ) -from lms.djangoapps.discussion.rest_api.forms import ( +from ..rest_api.forms import ( CommentGetForm, CommentListGetForm, CourseDiscussionRolesForm, CourseDiscussionSettingsForm, - ThreadListGetForm + ThreadListGetForm, ) -from lms.djangoapps.discussion.rest_api.serializers import ( +from ..rest_api.serializers import ( DiscussionRolesListSerializer, DiscussionRolesSerializer, - DiscussionSettingsSerializer + DiscussionSettingsSerializer, ) -from lms.djangoapps.discussion.views import get_divided_discussions # lint-amnesty, pylint: disable=unused-import -from lms.djangoapps.instructor.access import update_forum_role -from openedx.core.djangoapps.django_comment_common import comment_client -from openedx.core.djangoapps.django_comment_common.models import Role -from openedx.core.djangoapps.django_comment_common.models import CourseDiscussionSettings -from openedx.core.djangoapps.user_api.accounts.permissions import CanReplaceUsername, CanRetireUser -from openedx.core.djangoapps.user_api.models import UserRetirementStatus -from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser -from openedx.core.lib.api.parsers import MergePatchParser -from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin, view_auth_classes -from xmodule.modulestore.django import modulestore log = logging.getLogger(__name__) +User = get_user_model() + @view_auth_classes() class CourseView(DeveloperErrorViewMixin, APIView): @@ -83,6 +83,8 @@ class CourseView(DeveloperErrorViewMixin, APIView): * thread_list_url: The URL of the list of all threads in the course. + * following_thread_list_url: thread_list_url with parameter following=True + * topics_url: The URL of the topic listing for the course. """ def get(self, request, course_id): @@ -174,6 +176,14 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): multiple topic_id queries to retrieve threads from multiple topics at once. + * author: The username of an author. If provided, only threads by this + author will be returned. + + * thread_type: Can be 'discussion' or 'question', only return threads of + the selected thread type. + + * flagged: If True, only return threads that have been flagged (reported) + * text_search: A search string to match. Any thread whose content (including the bodies of comments in the thread) matches the search string will be returned. @@ -290,6 +300,9 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): * response_count: The number of direct responses for a thread + * abuse_flagged_count: The number of flags(reports) on and within the + thread. Returns null if requesting user is not a moderator + **DELETE response values: No content is returned for a DELETE request @@ -314,6 +327,9 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): form.cleaned_data["topic_id"], form.cleaned_data["text_search"], form.cleaned_data["following"], + form.cleaned_data["author"], + form.cleaned_data["thread_type"], + form.cleaned_data["flagged"], form.cleaned_data["view"], form.cleaned_data["order_by"], form.cleaned_data["order_direction"], @@ -468,6 +484,10 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet): * abuse_flagged: Boolean indicating whether the requesting user has flagged the comment for abuse + * abuse_flagged_any_user: Boolean indicating whether any user has + flagged the comment for abuse. Returns null if requesting user + is not a moderator. + * voted: Boolean indicating whether the requesting user has voted for the comment