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

feat: add edit_reason_code and close_reason_code (#29609)

Browse Source 
Add `edit_reason_code` field to both `Comments` and `Threads`, making it
editable for anyone who can also edit `raw_body`.

Add `close_reason_code` field to `Threads`, and make it editable by
anyone who can also edit `closed`.
This commit is contained in:
Felipe Trzaskowski
2022-03-16 09:02:47 -03:00
committed by GitHub
parent 626a0e112e
commit 6bfb741c4a
11 changed files with 178 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -975,7 +975,7 @@ def _do_extra_actions(api_content, cc_content, request_fields, actions_form, con
update that require a separate comments service request.
"""
for field, form_value in actions_form.cleaned_data.items():
if field in request_fields and form_value != api_content[field]:
if field in request_fields and field in api_content and form_value != api_content[field]:
api_content[field] = form_value
if field == "following":
_handle_following_field(form_value, context["cc_requester"], cc_content)

2
lms/djangoapps/discussion/rest_api/permissions.py
View File

@@ -102,6 +102,7 @@ def get_editable_fields(cc_content: Union[Thread, Comment], context: Dict) -> Se
editable_fields = {
"abuse_flagged": True,
"closed": is_thread and is_privileged,
"close_reason_code": is_thread and is_privileged,
"pinned": is_thread and is_privileged,
"read": is_thread,
}
@@ -114,6 +115,7 @@ def get_editable_fields(cc_content: Union[Thread, Comment], context: Dict) -> Se
editable_fields.update({
"voted": True,
"raw_body": is_privileged or is_author,
"edit_reason_code": is_privileged,
"following": is_thread,
"topic_id": is_thread and (is_author or is_privileged),
"type": is_thread and (is_author or is_privileged),

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

@@ -136,6 +136,8 @@ class _ContentSerializer(serializers.Serializer):
can_delete = serializers.SerializerMethodField()
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)
non_updatable_fields = set()
@@ -238,6 +240,16 @@ class _ContentSerializer(serializers.Serializer):
"""
return can_delete(obj, self.context)
def get_last_edit(self, obj):
"""
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]
class ThreadSerializer(_ContentSerializer):
"""
@@ -269,6 +281,8 @@ 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)
non_updatable_fields = NON_UPDATABLE_THREAD_FIELDS
@@ -360,6 +374,21 @@ class ThreadSerializer(_ContentSerializer):
"""
return Truncator(strip_tags(self.get_rendered_body(obj))).chars(35, ).replace('\n', ' ')
def get_close_reason_code(self, obj):
"""
Returns the reason for which the thread was closed.
"""
if _validate_privileged_access(self.context):
return obj.get("close_reason_code")
def get_closed_by(self, obj):
"""
Returns the username of the moderator who closed this thread,
only to other privileged users.
"""
if _validate_privileged_access(self.context):
return obj.get("closed_by")
def create(self, validated_data):
thread = Thread(user_id=self.context["cc_requester"]["id"], **validated_data)
thread.save()

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

@@ -1425,6 +1425,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
"can_delete": False,
"anonymous": False,
"anonymous_to_peers": False,
"last_edit": None,
},
{
"id": "test_comment_2",
@@ -1450,6 +1451,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
"can_delete": False,
"anonymous": True,
"anonymous_to_peers": False,
"last_edit": None,
},
]
actual_comments = self.get_comment_list(
@@ -1879,8 +1881,19 @@ class CreateThreadTest(
"comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_id",
"read": True,
"editable_fields": [
"abuse_flagged", "anonymous", "closed", "following", "pinned",
"raw_body", "read", "title", "topic_id", "type", "voted"
"abuse_flagged",
"anonymous",
"close_reason_code",
"closed",
"edit_reason_code",
"following",
"pinned",
"raw_body",
"read",
"title",
"topic_id",
"type",
"voted",
],
})
assert actual == expected
@@ -2169,6 +2182,7 @@ class CreateCommentTest(
"can_delete": True,
"anonymous": False,
"anonymous_to_peers": False,
"last_edit": None,
}
assert actual == expected
expected_url = (
@@ -2250,11 +2264,19 @@ class CreateCommentTest(
"voted": False,
"vote_count": 0,
"children": [],
"editable_fields": ["abuse_flagged", "anonymous", "endorsed", "raw_body", "voted"],
"editable_fields": [
"abuse_flagged",
"anonymous",
"edit_reason_code",
"endorsed",
"raw_body",
"voted",
],
"child_count": 0,
"can_delete": True,
"anonymous": False,
"anonymous_to_peers": False,
"last_edit": None,
}
assert actual == expected
expected_url = (
@@ -2455,6 +2477,7 @@ class UpdateThreadTest(
MockSignalHandlerMixin
):
"""Tests for update_thread"""
@classmethod
def setUpClass(cls):
super().setUpClass()
@@ -2463,6 +2486,7 @@ class UpdateThreadTest(
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
def setUp(self):
super().setUp()
httpretty.reset()
httpretty.enable()
self.addCleanup(httpretty.reset)
@@ -2514,7 +2538,7 @@ class UpdateThreadTest(
'preview_body': 'Edited body',
'topic_id': 'original_topic',
'read': True,
'title': 'Original Title'
'title': 'Original Title',
})
assert parsed_body(httpretty.last_request()) == {
'course_id': [str(self.course.id)],
@@ -2795,6 +2819,35 @@ class UpdateThreadTest(
update_thread(self.request, "test_thread", {"raw_body": ""})
assert assertion.value.message_dict == {'raw_body': ['This field may not be blank.']}
@ddt.data(
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_STUDENT,
)
def test_update_thread_with_edit_reason_code(self, role_name):
"""
Test editing comments, specifying and retrieving edit reason codes.
"""
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_thread()
try:
result = update_thread(self.request, "test_thread", {
"raw_body": "Edited body",
"edit_reason_code": "someReason",
})
assert role_name != FORUM_ROLE_STUDENT
assert result["last_edit"] == {
"original_body": "Original body",
"reason_code": "someReason",
"author": self.user.username,
}
request_body = httpretty.last_request().parsed_body # pylint: disable=no-member
assert request_body["edit_reason_code"] == ["someReason"]
except ValidationError as error:
assert role_name == FORUM_ROLE_STUDENT
assert error.message_dict == {"edit_reason_code": ["This field is not editable."]}
@ddt.ddt
@disable_signal(api, 'comment_edited')
@@ -2893,6 +2946,7 @@ class UpdateCommentTest(
"editable_fields": ["abuse_flagged", "anonymous", "raw_body", "voted"],
"child_count": 0,
"can_delete": True,
"last_edit": None,
}
assert actual == expected
assert parsed_body(httpretty.last_request()) == {
@@ -3177,6 +3231,34 @@ class UpdateCommentTest(
assert httpretty.last_request().method == 'PUT'
assert parsed_body(httpretty.last_request()) == {'user_id': [str(self.user.id)]}
@ddt.data(
FORUM_ROLE_ADMINISTRATOR,
FORUM_ROLE_MODERATOR,
FORUM_ROLE_COMMUNITY_TA,
FORUM_ROLE_STUDENT,
)
def test_update_comment_with_edit_reason_code(self, role_name):
"""
Test editing comments, specifying and retrieving edit reason codes.
"""
_assign_role_to_user(user=self.user, course_id=self.course.id, role=role_name)
self.register_comment()
try:
result = update_comment(self.request, "test_comment", {
"raw_body": "Edited body",
"edit_reason_code": "someReason",
})
assert role_name != FORUM_ROLE_STUDENT
assert result["last_edit"] == {
"original_body": "Original body",
"reason_code": "someReason",
"author": self.user.username,
}
request_body = httpretty.last_request().parsed_body # pylint: disable=no-member
assert request_body["edit_reason_code"] == ["someReason"]
except ValidationError:
assert role_name == FORUM_ROLE_STUDENT
@ddt.ddt
@disable_signal(api, 'thread_deleted')

8
lms/djangoapps/discussion/rest_api/tests/test_permissions.py
View File

@@ -66,7 +66,7 @@ class GetInitializableFieldsTest(ModuleStoreTestCase):
"abuse_flagged", "course_id", "following", "raw_body", "read", "title", "topic_id", "type", "voted"
}
if is_privileged:
expected |= {"closed", "pinned"}
expected |= {"closed", "pinned", "close_reason_code", "edit_reason_code"}
if is_privileged and is_cohorted:
expected |= {"group_id"}
if allow_anonymous:
@@ -87,6 +87,8 @@ class GetInitializableFieldsTest(ModuleStoreTestCase):
expected = {
"anonymous", "abuse_flagged", "parent_id", "raw_body", "thread_id", "voted"
}
if is_privileged:
expected |= {"edit_reason_code"}
if (is_thread_author and thread_type == "question") or is_privileged:
expected |= {"endorsed"}
assert actual == expected
@@ -116,7 +118,7 @@ class GetEditableFieldsTest(ModuleStoreTestCase):
actual = get_editable_fields(thread, context)
expected = {"abuse_flagged", "following", "read", "voted"}
if is_privileged:
expected |= {"closed", "pinned"}
expected |= {"closed", "pinned", "close_reason_code", "edit_reason_code"}
if is_author or is_privileged:
expected |= {"topic_id", "type", "title", "raw_body"}
if is_privileged and is_cohorted:
@@ -148,6 +150,8 @@ class GetEditableFieldsTest(ModuleStoreTestCase):
)
actual = get_editable_fields(comment, context)
expected = {"abuse_flagged", "voted"}
if is_privileged:
expected |= {"edit_reason_code"}
if is_author or is_privileged:
expected |= {"raw_body"}
if (is_thread_author and thread_type == "question") or is_privileged:

1
lms/djangoapps/discussion/rest_api/tests/test_serializers.py
View File

@@ -317,6 +317,7 @@ class CommentSerializerTest(SerializerTestMixin, SharedModuleStoreTestCase):
"editable_fields": ["abuse_flagged", "voted"],
"child_count": 0,
"can_delete": False,
"last_edit": None,
}
assert self.serialize(comment) == expected

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

@@ -1510,6 +1510,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
"can_delete": True,
"anonymous": False,
"anonymous_to_peers": False,
"last_edit": None,
}
response_data.update(overrides or {})
return response_data
@@ -1902,6 +1903,7 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"can_delete": True,
"anonymous": False,
"anonymous_to_peers": False,
"last_edit": None,
}
response = self.client.post(
self.url,
@@ -1992,6 +1994,7 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes
"can_delete": True,
"anonymous": False,
"anonymous_to_peers": False,
"last_edit": None,
}
response_data.update(overrides or {})
return response_data
@@ -2179,6 +2182,7 @@ class CommentViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase
"can_delete": True,
"anonymous": False,
"anonymous_to_peers": False,
"last_edit": None,
}
response = self.client.get(self.url)

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

@@ -31,10 +31,19 @@ def _get_thread_callback(thread_data):
additional required fields.
"""
response_data = make_minimal_cs_thread(thread_data)
original_data = response_data.copy()
for key, val_list in parsed_body(request).items():
val = val_list[0]
if key in ["anonymous", "anonymous_to_peers", "closed", "pinned"]:
response_data[key] = val == "True"
elif key == "edit_reason_code":
response_data["edit_history"] = [
{
"original_body": original_data["body"],
"author": thread_data.get('username'),
"reason_code": val,
},
]
else:
response_data[key] = val
return (200, headers, json.dumps(response_data))
@@ -53,6 +62,7 @@ def _get_comment_callback(comment_data, thread_id, parent_id):
Simulate the comment creation or update endpoint as described above.
"""
response_data = make_minimal_cs_comment(comment_data)
original_data = response_data.copy()
# thread_id and parent_id are not included in request payload but
# are returned by the comments service
response_data["thread_id"] = thread_id
@@ -61,6 +71,14 @@ def _get_comment_callback(comment_data, thread_id, parent_id):
val = val_list[0]
if key in ["anonymous", "anonymous_to_peers", "endorsed"]:
response_data[key] = val == "True"
elif key == "edit_reason_code":
response_data["edit_history"] = [
{
"original_body": original_data["body"],
"author": comment_data.get('username'),
"reason_code": val,
},
]
else:
response_data[key] = val
return (200, headers, json.dumps(response_data))
@@ -438,8 +456,15 @@ class CommentsServiceMockMixin:
"voted": False,
"vote_count": 0,
"editable_fields": [
"abuse_flagged", "anonymous", "following", "raw_body", "read",
"title", "topic_id", "type", "voted"
"abuse_flagged",
"anonymous",
"following",
"raw_body",
"read",
"title",
"topic_id",
"type",
"voted",
],
"course_id": str(self.course.id),
"topic_id": "test_topic",
@@ -460,6 +485,9 @@ class CommentsServiceMockMixin:
"id": "test_thread",
"type": "discussion",
"response_count": 0,
"last_edit": None,
"closed_by": None,
"close_reason_code": None,
}
response_data.update(overrides or {})
return response_data
@@ -497,6 +525,8 @@ def make_minimal_cs_thread(overrides=None):
"read": False,
"endorsed": False,
"resp_total": 0,
"closed_by": None,
"close_reason_code": None,
}
ret.update(overrides or {})
return ret

11
lms/djangoapps/discussion/rest_api/views.py
View File

@@ -401,6 +401,14 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet):
* read (optional): A boolean to mark thread as read
* closed (optional, privileged): A boolean to mark thread as closed.
* edit_reason_code (optional, privileged): A string containing a reason
code for editing the thread's body.
* close_reason_code (optional, privileged): A string containing a reason
code for closing the thread.
* topic_id, type, title, raw_body, anonymous, and anonymous_to_peers
are accepted with the same meaning as in a POST request
@@ -630,6 +638,9 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet):
* raw_body, anonymous and anonymous_to_peers are accepted with the same
meaning as in a POST request
* edit_reason_code (optional, privileged): A string containing a reason
code for a moderator to edit the comment.
If "application/merge-patch+json" is not the specified content type,
a 415 error is returned.

4
openedx/core/djangoapps/django_comment_common/comment_client/comment.py
View File

@@ -14,12 +14,12 @@ class Comment(models.Model):
'endorsed', 'parent_id', 'thread_id', 'username', 'votes', 'user_id',
'closed', 'created_at', 'updated_at', 'depth', 'at_position_list',
'type', 'commentable_id', 'abuse_flaggers', 'endorsement',
'child_count',
'child_count', 'edit_history',
]
updatable_fields = [
'body', 'anonymous', 'anonymous_to_peers', 'course_id', 'closed',
'user_id', 'endorsed', 'endorsement_user_id',
'user_id', 'endorsed', 'endorsement_user_id', 'edit_reason_code',
]
initializable_fields = updatable_fields

7
openedx/core/djangoapps/django_comment_common/comment_client/thread.py
View File

@@ -21,19 +21,20 @@ class Thread(models.Model):
'highlighted_body', 'endorsed', 'read', 'group_id', 'group_name', 'pinned',
'abuse_flaggers', 'resp_skip', 'resp_limit', 'resp_total', 'thread_type',
'endorsed_responses', 'non_endorsed_responses', 'non_endorsed_resp_total',
'context', 'last_activity_at',
'context', 'last_activity_at', 'closed_by', 'close_reason_code', 'edit_history',
]
# updateable_fields are sent in PUT requests
updatable_fields = [
'title', 'body', 'anonymous', 'anonymous_to_peers', 'course_id', 'read',
'closed', 'user_id', 'commentable_id', 'group_id', 'group_name', 'pinned', 'thread_type'
'closed', 'user_id', 'commentable_id', 'group_id', 'group_name', 'pinned', 'thread_type',
'close_reason_code', 'edit_reason_code',
]
# metric_tag_fields are used by Datadog to record metrics about the model
metric_tag_fields = [
'course_id', 'group_id', 'pinned', 'closed', 'anonymous', 'anonymous_to_peers',
'endorsed', 'read'
'endorsed', 'read',
]
# initializable_fields are sent in POST requests