feat: add support for specifying and retrieving moderation reason codes from the LMS settings (#30015)
Reason codes will be used by the frontend to list and validate the reasons for specifying moderation actions. Co-authored-by: Kshitij Sobti <kshitij@opencraft.com> Co-authored-by: Felipe Trzaskowski <felipe@opencraft.com>
This commit is contained in:
@@ -9,6 +9,7 @@ from enum import Enum
|
||||
from typing import Dict, Iterable, List, Literal, Optional, Set, Tuple
|
||||
from urllib.parse import urlencode, urlunparse
|
||||
|
||||
from django.conf import settings
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.http import Http404
|
||||
@@ -54,6 +55,19 @@ 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_thread_viewed_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,
|
||||
)
|
||||
from ..toggles import ENABLE_DISCUSSION_MODERATION_REASON_CODES
|
||||
from .exceptions import CommentNotFoundError, DiscussionBlackOutException, DiscussionDisabledError, ThreadNotFoundError
|
||||
from .forms import CommentActionsForm, ThreadActionsForm, UserOrdering
|
||||
from .pagination import DiscussionAPIPagination
|
||||
@@ -73,18 +87,6 @@ from .serializers import (
|
||||
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_thread_viewed_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()
|
||||
|
||||
@@ -281,6 +283,8 @@ def get_course(request, course_key):
|
||||
course = _get_course(course_key, request.user)
|
||||
user_roles = get_user_role_names(request.user, course_key)
|
||||
course_config = DiscussionsConfiguration.get(course_key)
|
||||
EDIT_REASON_CODES = getattr(settings, "DISCUSSION_MODERATION_EDIT_REASON_CODES", {})
|
||||
CLOSE_REASON_CODES = getattr(settings, "DISCUSSION_MODERATION_CLOSE_REASON_CODES", {})
|
||||
|
||||
return {
|
||||
"id": str(course_key),
|
||||
@@ -308,6 +312,16 @@ def get_course(request, course_key):
|
||||
"enable_in_context": course_config.enable_in_context,
|
||||
"group_at_subsection": course_config.plugin_configuration.get("group_at_subsection", False),
|
||||
'learners_tab_enabled': ENABLE_LEARNERS_TAB_IN_DISCUSSIONS_MFE.is_enabled(course_key),
|
||||
"reason_codes_enabled": ENABLE_DISCUSSION_MODERATION_REASON_CODES.is_enabled(course_key),
|
||||
"edit_reasons": [
|
||||
{"code": reason_code, "label": label}
|
||||
for (reason_code, label) in EDIT_REASON_CODES.items()
|
||||
],
|
||||
"post_close_reasons": [
|
||||
{"code": reason_code, "label": label}
|
||||
for (reason_code, label) in CLOSE_REASON_CODES.items()
|
||||
],
|
||||
|
||||
}
|
||||
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ Discussion API serializers
|
||||
from typing import Dict
|
||||
from urllib.parse import urlencode, urlunparse
|
||||
|
||||
from django.conf import settings
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core.exceptions import ValidationError
|
||||
from django.db.models import TextChoices
|
||||
@@ -43,6 +44,9 @@ from openedx.core.lib.api.serializers import CourseKeyField
|
||||
|
||||
User = get_user_model()
|
||||
|
||||
CLOSE_REASON_CODES = getattr(settings, "DISCUSSION_MODERATION_CLOSE_REASON_CODES", {})
|
||||
EDIT_REASON_CODES = getattr(settings, "DISCUSSION_MODERATION_EDIT_REASON_CODES", {})
|
||||
|
||||
|
||||
class TopicOrdering(TextChoices):
|
||||
"""
|
||||
@@ -99,6 +103,26 @@ def validate_not_blank(value):
|
||||
raise ValidationError("This field may not be blank.")
|
||||
|
||||
|
||||
def validate_edit_reason_code(value):
|
||||
"""
|
||||
Validate that the value is a valid edit reason code.
|
||||
|
||||
Raises: ValidationError
|
||||
"""
|
||||
if value not in EDIT_REASON_CODES:
|
||||
raise ValidationError("Invalid edit reason code")
|
||||
|
||||
|
||||
def validate_close_reason_code(value):
|
||||
"""
|
||||
Validate that the value is a valid close reason code.
|
||||
|
||||
Raises: ValidationError
|
||||
"""
|
||||
if value not in CLOSE_REASON_CODES:
|
||||
raise ValidationError("Invalid close reason code")
|
||||
|
||||
|
||||
def _validate_privileged_access(context: Dict) -> bool:
|
||||
"""
|
||||
Return the field specified by ``field_name`` if requesting user is privileged.
|
||||
@@ -137,7 +161,7 @@ class _ContentSerializer(serializers.Serializer):
|
||||
anonymous = serializers.BooleanField(default=False)
|
||||
anonymous_to_peers = serializers.BooleanField(default=False)
|
||||
last_edit = serializers.SerializerMethodField(required=False)
|
||||
edit_reason_code = serializers.CharField(required=False)
|
||||
edit_reason_code = serializers.CharField(required=False, validators=[validate_edit_reason_code])
|
||||
|
||||
non_updatable_fields = set()
|
||||
|
||||
@@ -245,10 +269,16 @@ class _ContentSerializer(serializers.Serializer):
|
||||
Returns information about the last edit for this content for
|
||||
privileged users.
|
||||
"""
|
||||
if _validate_privileged_access(self.context):
|
||||
edit_history = obj.get("edit_history")
|
||||
if edit_history:
|
||||
return edit_history[-1]
|
||||
if not _validate_privileged_access(self.context):
|
||||
return None
|
||||
edit_history = obj.get("edit_history")
|
||||
if not edit_history:
|
||||
return None
|
||||
last_edit = edit_history[-1]
|
||||
reason_code = last_edit.get("reason_code")
|
||||
if reason_code:
|
||||
last_edit["reason"] = EDIT_REASON_CODES.get(reason_code)
|
||||
return last_edit
|
||||
|
||||
|
||||
class ThreadSerializer(_ContentSerializer):
|
||||
@@ -281,8 +311,9 @@ class ThreadSerializer(_ContentSerializer):
|
||||
read = serializers.BooleanField(required=False)
|
||||
has_endorsed = serializers.BooleanField(source="endorsed", read_only=True)
|
||||
response_count = serializers.IntegerField(source="resp_total", read_only=True, required=False)
|
||||
close_reason_code = serializers.SerializerMethodField(required=False)
|
||||
closed_by = serializers.SerializerMethodField(required=False)
|
||||
close_reason_code = serializers.CharField(required=False, validators=[validate_close_reason_code])
|
||||
close_reason = serializers.SerializerMethodField()
|
||||
closed_by = serializers.SerializerMethodField()
|
||||
|
||||
non_updatable_fields = NON_UPDATABLE_THREAD_FIELDS
|
||||
|
||||
@@ -374,12 +405,14 @@ class ThreadSerializer(_ContentSerializer):
|
||||
"""
|
||||
return Truncator(strip_tags(self.get_rendered_body(obj))).chars(35, ).replace('\n', ' ')
|
||||
|
||||
def get_close_reason_code(self, obj):
|
||||
def get_close_reason(self, obj):
|
||||
"""
|
||||
Returns the reason for which the thread was closed.
|
||||
"""
|
||||
if _validate_privileged_access(self.context):
|
||||
return obj.get("close_reason_code")
|
||||
if not _validate_privileged_access(self.context):
|
||||
return None
|
||||
reason_code = obj.get("close_reason_code")
|
||||
return CLOSE_REASON_CODES.get(reason_code)
|
||||
|
||||
def get_closed_by(self, obj):
|
||||
"""
|
||||
@@ -750,6 +783,14 @@ class BlackoutDateSerializer(serializers.Serializer):
|
||||
end = serializers.DateTimeField(help_text="The ISO 8601 timestamp for the end of the blackout period")
|
||||
|
||||
|
||||
class ReasonCodeSeralizer(serializers.Serializer):
|
||||
"""
|
||||
Serializer for reason codes.
|
||||
"""
|
||||
code = serializers.CharField(help_text="A code for the an edit or close reason")
|
||||
label = serializers.CharField(help_text="A user-friendly name text for the close or edit reason")
|
||||
|
||||
|
||||
class CourseMetadataSerailizer(serializers.Serializer):
|
||||
"""
|
||||
Serializer for course metadata.
|
||||
@@ -789,3 +830,11 @@ class CourseMetadataSerailizer(serializers.Serializer):
|
||||
group_at_subsection = serializers.BooleanField(
|
||||
help_text="A boolean indicating whether discussions should be grouped at subsection",
|
||||
)
|
||||
post_close_reasons = serializers.ListField(
|
||||
child=ReasonCodeSeralizer(),
|
||||
help_text="A list of reasons that can be specified by moderators for closing a post",
|
||||
)
|
||||
edit_reasons = serializers.ListField(
|
||||
child=ReasonCodeSeralizer(),
|
||||
help_text="A list of reasons that can be specified by moderators for editing a post, response, or comment",
|
||||
)
|
||||
|
||||
@@ -12,6 +12,7 @@ from urllib.parse import parse_qs, urlencode, urlparse, urlunparse
|
||||
import ddt
|
||||
import httpretty
|
||||
import pytest
|
||||
from django.test import override_settings
|
||||
from edx_toggles.toggles.testutils import override_waffle_flag
|
||||
from django.contrib.auth import get_user_model
|
||||
from django.core.exceptions import ValidationError
|
||||
@@ -153,6 +154,8 @@ def _set_course_discussion_blackout(course, user_id):
|
||||
|
||||
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
@override_settings(DISCUSSION_MODERATION_EDIT_REASON_CODES={"test-edit-reason": "Test Edit Reason"})
|
||||
@override_settings(DISCUSSION_MODERATION_CLOSE_REASON_CODES={"test-close-reason": "Test Close Reason"})
|
||||
@ddt.ddt
|
||||
class GetCourseTest(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase):
|
||||
"""Test for get_course"""
|
||||
@@ -199,6 +202,9 @@ class GetCourseTest(ForumsEnableMixin, UrlResetMixin, SharedModuleStoreTestCase)
|
||||
'user_is_privileged': False,
|
||||
'user_roles': {'Student'},
|
||||
'learners_tab_enabled': False,
|
||||
'reason_codes_enabled': False,
|
||||
'edit_reasons': [{'code': 'test-edit-reason', 'label': 'Test Edit Reason'}],
|
||||
'post_close_reasons': [{'code': 'test-close-reason', 'label': 'Test Close Reason'}],
|
||||
}
|
||||
|
||||
@ddt.data(
|
||||
@@ -2825,6 +2831,9 @@ class UpdateThreadTest(
|
||||
FORUM_ROLE_COMMUNITY_TA,
|
||||
FORUM_ROLE_STUDENT,
|
||||
)
|
||||
@mock.patch("lms.djangoapps.discussion.rest_api.serializers.EDIT_REASON_CODES", {
|
||||
"test-edit-reason": "Test Edit Reason",
|
||||
})
|
||||
def test_update_thread_with_edit_reason_code(self, role_name):
|
||||
"""
|
||||
Test editing comments, specifying and retrieving edit reason codes.
|
||||
@@ -2834,16 +2843,17 @@ class UpdateThreadTest(
|
||||
try:
|
||||
result = update_thread(self.request, "test_thread", {
|
||||
"raw_body": "Edited body",
|
||||
"edit_reason_code": "someReason",
|
||||
"edit_reason_code": "test-edit-reason",
|
||||
})
|
||||
assert role_name != FORUM_ROLE_STUDENT
|
||||
assert result["last_edit"] == {
|
||||
"original_body": "Original body",
|
||||
"reason_code": "someReason",
|
||||
"reason": "Test Edit Reason",
|
||||
"reason_code": "test-edit-reason",
|
||||
"author": self.user.username,
|
||||
}
|
||||
request_body = httpretty.last_request().parsed_body # pylint: disable=no-member
|
||||
assert request_body["edit_reason_code"] == ["someReason"]
|
||||
assert request_body["edit_reason_code"] == ["test-edit-reason"]
|
||||
except ValidationError as error:
|
||||
assert role_name == FORUM_ROLE_STUDENT
|
||||
assert error.message_dict == {"edit_reason_code": ["This field is not editable."]}
|
||||
@@ -3237,6 +3247,9 @@ class UpdateCommentTest(
|
||||
FORUM_ROLE_COMMUNITY_TA,
|
||||
FORUM_ROLE_STUDENT,
|
||||
)
|
||||
@mock.patch("lms.djangoapps.discussion.rest_api.serializers.EDIT_REASON_CODES", {
|
||||
"test-edit-reason": "Test Edit Reason",
|
||||
})
|
||||
def test_update_comment_with_edit_reason_code(self, role_name):
|
||||
"""
|
||||
Test editing comments, specifying and retrieving edit reason codes.
|
||||
@@ -3246,16 +3259,17 @@ class UpdateCommentTest(
|
||||
try:
|
||||
result = update_comment(self.request, "test_comment", {
|
||||
"raw_body": "Edited body",
|
||||
"edit_reason_code": "someReason",
|
||||
"edit_reason_code": "test-edit-reason",
|
||||
})
|
||||
assert role_name != FORUM_ROLE_STUDENT
|
||||
assert result["last_edit"] == {
|
||||
"original_body": "Original body",
|
||||
"reason_code": "someReason",
|
||||
"reason": "Test Edit Reason",
|
||||
"reason_code": "test-edit-reason",
|
||||
"author": self.user.username,
|
||||
}
|
||||
request_body = httpretty.last_request().parsed_body # pylint: disable=no-member
|
||||
assert request_body["edit_reason_code"] == ["someReason"]
|
||||
assert request_body["edit_reason_code"] == ["test-edit-reason"]
|
||||
except ValidationError:
|
||||
assert role_name == FORUM_ROLE_STUDENT
|
||||
|
||||
|
||||
@@ -7,17 +7,18 @@ import json
|
||||
import random
|
||||
from datetime import datetime
|
||||
from unittest import mock
|
||||
from urllib.parse import parse_qs, urlparse, urlencode
|
||||
from urllib.parse import parse_qs, urlencode, urlparse
|
||||
|
||||
import ddt
|
||||
import httpretty
|
||||
from django.urls import reverse
|
||||
from django.core.files.uploadedfile import SimpleUploadedFile
|
||||
from django.test import override_settings
|
||||
from django.urls import reverse
|
||||
from opaque_keys.edx.keys import CourseKey
|
||||
from pytz import UTC
|
||||
from rest_framework import status
|
||||
from rest_framework.parsers import JSONParser
|
||||
from rest_framework.test import APIClient, APITestCase
|
||||
from rest_framework import status
|
||||
from xmodule.modulestore import ModuleStoreEnum
|
||||
from xmodule.modulestore.django import modulestore
|
||||
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
|
||||
@@ -27,11 +28,7 @@ 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 (
|
||||
@@ -483,6 +480,8 @@ class CommentViewSetListByUserTest(
|
||||
|
||||
|
||||
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
|
||||
@override_settings(DISCUSSION_MODERATION_EDIT_REASON_CODES={"test-edit-reason": "Test Edit Reason"})
|
||||
@override_settings(DISCUSSION_MODERATION_CLOSE_REASON_CODES={"test-close-reason": "Test Close Reason"})
|
||||
class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
|
||||
"""Tests for CourseView"""
|
||||
def setUp(self):
|
||||
@@ -512,14 +511,17 @@ class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
|
||||
"http://testserver/api/discussion/v1/threads/?course_id=course-v1%3Ax%2By%2Bz&following=True"
|
||||
),
|
||||
"topics_url": "http://testserver/api/discussion/v1/course_topics/course-v1:x+y+z",
|
||||
'enable_in_context': True,
|
||||
'group_at_subsection': False,
|
||||
'provider': 'legacy',
|
||||
"enable_in_context": True,
|
||||
"group_at_subsection": False,
|
||||
"provider": "legacy",
|
||||
"allow_anonymous": True,
|
||||
"allow_anonymous_to_peers": False,
|
||||
'user_is_privileged': False,
|
||||
'user_roles': ['Student'],
|
||||
"user_is_privileged": False,
|
||||
"user_roles": ["Student"],
|
||||
'learners_tab_enabled': False,
|
||||
"reason_codes_enabled": False,
|
||||
"edit_reasons": [{"code": "test-edit-reason", "label": "Test Edit Reason"}],
|
||||
"post_close_reasons": [{"code": "test-close-reason", "label": "Test Close Reason"}],
|
||||
}
|
||||
)
|
||||
|
||||
|
||||
@@ -487,6 +487,7 @@ class CommentsServiceMockMixin:
|
||||
"response_count": 0,
|
||||
"last_edit": None,
|
||||
"closed_by": None,
|
||||
"close_reason": None,
|
||||
"close_reason_code": None,
|
||||
}
|
||||
response_data.update(overrides or {})
|
||||
|
||||
@@ -50,7 +50,7 @@ from ..rest_api.api import (
|
||||
get_thread_list,
|
||||
get_user_comments,
|
||||
update_comment,
|
||||
update_thread
|
||||
update_thread,
|
||||
)
|
||||
from ..rest_api.forms import (
|
||||
CommentGetForm,
|
||||
|
||||
@@ -32,3 +32,16 @@ ENABLE_NEW_STRUCTURE_DISCUSSIONS = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'enab
|
||||
# .. toggle_target_removal_date: 2022-05-21
|
||||
# lint-amnesty, pylint: disable=line-too-long
|
||||
ENABLE_LEARNERS_TAB_IN_DISCUSSIONS_MFE = CourseWaffleFlag(WAFFLE_FLAG_NAMESPACE, 'enable_learners_tab_in_discussions_mfe', __name__)
|
||||
|
||||
# .. toggle_name: discussions.enable_moderation_reason_codes
|
||||
# .. toggle_implementation: CourseWaffleFlag
|
||||
# .. toggle_default: False
|
||||
# .. toggle_description: Waffle flag to toggle support for the new edit and post close reason codes
|
||||
# .. toggle_use_cases: temporary, open_edx
|
||||
# .. toggle_creation_date: 2022-02-22
|
||||
# .. toggle_target_removal_date: 2022-09-22
|
||||
ENABLE_DISCUSSION_MODERATION_REASON_CODES = CourseWaffleFlag(
|
||||
WAFFLE_FLAG_NAMESPACE,
|
||||
'enable_moderation_reason_codes',
|
||||
__name__,
|
||||
)
|
||||
|
||||
@@ -4788,8 +4788,8 @@ LEARNING_MICROFRONTEND_URL = None
|
||||
# waffle flag.
|
||||
ORA_GRADING_MICROFRONTEND_URL = None
|
||||
# .. setting_name: DISCUSSIONS_MICROFRONTEND_URL
|
||||
# .. setting_default: None
|
||||
# .. setting_description: Base URL of the micro-frontend-based discussions page.
|
||||
# .. setting_default: None
|
||||
# .. setting_warning: Also set site's courseware.discussions_mfe waffle flag.
|
||||
DISCUSSIONS_MICROFRONTEND_URL = None
|
||||
# .. setting_name: DISCUSSIONS_MFE_FEEDBACK_URL = None
|
||||
@@ -4969,3 +4969,24 @@ CUSTOM_PAGES_HELP_URL = "https://edx.readthedocs.io/projects/open-edx-building-a
|
||||
# The expected value is an Integer representing the cutoff point (in months) for inclusion to the message. Example:
|
||||
# a value of `3` would include learners who have logged in within the past 3 months.
|
||||
BULK_COURSE_EMAIL_LAST_LOGIN_ELIGIBILITY_PERIOD = None
|
||||
|
||||
################ Settings for the Discussion Service #########
|
||||
# Provide a list of reason codes for moderators editing posts and
|
||||
# comments, as a mapping from the internal reason code representation,
|
||||
# to an internationalizable label to be shown to moderators in the form UI.
|
||||
DISCUSSION_MODERATION_EDIT_REASON_CODES = {
|
||||
"grammar-spelling": _("Has grammar / spelling issues"),
|
||||
"needs-clarity": _("Content needs clarity"),
|
||||
"academic-integrity": _("Has academic integrity concern"),
|
||||
"inappropriate-language": _("Has inappropriate language"),
|
||||
"contains-pii": _("Contains personally identifiable information"),
|
||||
}
|
||||
# Provide a list of reason codes for moderators to close posts, as a mapping
|
||||
# from the internal reason code representation, to an internationalizable label
|
||||
# to be shown to moderators in the form UI.
|
||||
DISCUSSION_MODERATION_CLOSE_REASON_CODES = {
|
||||
"academic-integrity": _("Post violates honour code or academic integrity"),
|
||||
"read-only": _("Post should be read-only"),
|
||||
"duplicate": _("Post is a duplicate"),
|
||||
"off-topic": _("Post is off-topic"),
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user