From 8fbfa2398eaa54788acbffd5b1a266f5e482e343 Mon Sep 17 00:00:00 2001 From: Greg Price Date: Wed, 27 May 2015 15:28:57 -0400 Subject: [PATCH] Add comment creation to discussion API --- lms/djangoapps/discussion_api/api.py | 47 ++++++ lms/djangoapps/discussion_api/serializers.py | 46 +++++- .../discussion_api/tests/test_api.py | 149 +++++++++++++++++- .../discussion_api/tests/test_serializers.py | 103 ++++++++++++ .../discussion_api/tests/test_views.py | 84 ++++++++++ lms/djangoapps/discussion_api/tests/utils.py | 50 ++++++ lms/djangoapps/discussion_api/views.py | 113 ++++++++----- .../django_comment_client/base/views.py | 37 +++-- lms/lib/comment_client/comment.py | 6 +- openedx/core/lib/api/view_utils.py | 6 +- 10 files changed, 581 insertions(+), 60 deletions(-) diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 43dcbbd4ba..6c072176f8 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -15,6 +15,8 @@ from discussion_api.pagination import get_paginated_data from discussion_api.serializers import CommentSerializer, ThreadSerializer, get_context from django_comment_client.base.views import ( THREAD_CREATED_EVENT_NAME, + get_comment_created_event_data, + get_comment_created_event_name, get_thread_created_event_data, track_forum_event, ) @@ -264,4 +266,49 @@ def create_thread(request, thread_data): get_thread_created_event_data(thread, followed=following) ) + return ret + + +def create_comment(request, comment_data): + """ + Create a comment. + + Parameters: + + request: The django request object used for build_absolute_uri and + determining the requesting user. + + comment_data: The data for the created comment. + + Returns: + + The created comment; see discussion_api.views.CommentViewSet for more + detail. + """ + thread_id = comment_data.get("thread_id") + if not thread_id: + raise ValidationError({"thread_id": ["This field is required."]}) + try: + thread = Thread(id=thread_id).retrieve(mark_as_read=False) + course_key = CourseLocator.from_string(thread["course_id"]) + course = _get_course_or_404(course_key, request.user) + except (Http404, CommentClientRequestError): + raise ValidationError({"thread_id": ["Invalid value."]}) + + parent_id = comment_data.get("parent_id") + context = get_context(course, request, thread, parent_id) + serializer = CommentSerializer(data=comment_data, context=context) + if not serializer.is_valid(): + raise ValidationError(serializer.errors) + serializer.save() + + comment = serializer.object + track_forum_event( + request, + get_comment_created_event_name(comment), + course, + comment, + get_comment_created_event_data(comment, thread["commentable_id"], followed=False) + ) + return serializer.data diff --git a/lms/djangoapps/discussion_api/serializers.py b/lms/djangoapps/discussion_api/serializers.py index 4371401c3d..d524d44fd3 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/lms/djangoapps/discussion_api/serializers.py @@ -5,6 +5,7 @@ from urllib import urlencode from urlparse import urlunparse from django.contrib.auth.models import User as DjangoUser +from django.core.exceptions import ValidationError from django.core.urlresolvers import reverse from rest_framework import serializers @@ -15,12 +16,14 @@ from django_comment_common.models import ( FORUM_ROLE_MODERATOR, Role, ) +from lms.lib.comment_client.comment import Comment from lms.lib.comment_client.thread import Thread from lms.lib.comment_client.user import User as CommentClientUser +from lms.lib.comment_client.utils import CommentClientRequestError from openedx.core.djangoapps.course_groups.cohorts import get_cohort_names -def get_context(course, request, thread=None): +def get_context(course, request, thread=None, parent_id=None): """ Returns a context appropriate for use with ThreadSerializer or (if thread is provided) CommentSerializer. @@ -49,6 +52,7 @@ def get_context(course, request, thread=None): "ta_user_ids": ta_user_ids, "cc_requester": CommentClientUser.from_django_user(requester).retrieve(), "thread": thread, + "parent_id": parent_id, } @@ -204,13 +208,16 @@ class CommentSerializer(_ContentSerializer): """ A serializer for comment data. + Because it is not a field in the underlying data, parent_id must be provided + in the context for both serialization and deserialization. + N.B. This should not be used with a comment_client Comment object that has not had retrieve() called, because of the interaction between DRF's attempts at introspection and Comment's __getattr__. """ thread_id = serializers.CharField() parent_id = serializers.SerializerMethodField("get_parent_id") - endorsed = serializers.BooleanField() + endorsed = serializers.BooleanField(read_only=True) endorsed_by = serializers.SerializerMethodField("get_endorsed_by") endorsed_by_label = serializers.SerializerMethodField("get_endorsed_by_label") endorsed_at = serializers.SerializerMethodField("get_endorsed_at") @@ -218,7 +225,7 @@ class CommentSerializer(_ContentSerializer): def get_parent_id(self, _obj): """Returns the comment's parent's id (taken from the context).""" - return self.context.get("parent_id") + return self.context["parent_id"] def get_endorsed_by(self, obj): """ @@ -257,4 +264,35 @@ class CommentSerializer(_ContentSerializer): """Returns the list of the comment's children, serialized.""" child_context = dict(self.context) child_context["parent_id"] = obj["id"] - return [CommentSerializer(child, context=child_context).data for child in obj["children"]] + return [ + CommentSerializer(child, context=child_context).data + for child in obj.get("children", []) + ] + + def validate(self, attrs): + """ + Ensure that parent_id identifies a comment that is actually in the + thread identified by thread_id. + """ + parent_id = self.context["parent_id"] + if parent_id: + parent = None + try: + parent = Comment(id=parent_id).retrieve() + except CommentClientRequestError: + pass + if not (parent and parent["thread_id"] == attrs["thread_id"]): + raise ValidationError( + "parent_id does not identify a comment in the thread identified by thread_id." + ) + return attrs + + def restore_object(self, attrs, instance=None): + if instance: # pragma: no cover + raise ValueError("CommentSerializer cannot be used for updates.") + return Comment( + course_id=self.context["thread"]["course_id"], + parent_id=self.context["parent_id"], + user_id=self.context["cc_requester"]["id"], + **attrs + ) diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index 7d1e97cfa0..601b6a8cea 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -17,7 +17,13 @@ from django.test.client import RequestFactory from opaque_keys.edx.locator import CourseLocator from courseware.tests.factories import BetaTesterFactory, StaffFactory -from discussion_api.api import create_thread, get_comment_list, get_course_topics, get_thread_list +from discussion_api.api import ( + create_comment, + create_thread, + get_comment_list, + get_course_topics, + get_thread_list, +) from discussion_api.tests.utils import ( CommentsServiceMockMixin, make_minimal_cs_comment, @@ -1116,3 +1122,144 @@ class CreateThreadTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestC data["type"] = "invalid_type" with self.assertRaises(ValidationError): create_thread(self.request, data) + + +@ddt.ddt +class CreateCommentTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTestCase): + """Tests for create_comment""" + @mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def setUp(self): + super(CreateCommentTest, self).setUp() + httpretty.reset() + httpretty.enable() + self.addCleanup(httpretty.disable) + self.user = UserFactory.create() + self.register_get_user_response(self.user) + self.request = RequestFactory().get("/test_path") + self.request.user = self.user + self.course = CourseFactory.create() + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) + self.register_get_thread_response( + make_minimal_cs_thread({ + "id": "test_thread", + "course_id": unicode(self.course.id), + "commentable_id": "test_topic", + }) + ) + self.minimal_data = { + "thread_id": "test_thread", + "raw_body": "Test body", + } + + @ddt.data(None, "test_parent") + @mock.patch("eventtracking.tracker.emit") + def test_success(self, parent_id, mock_emit): + if parent_id: + self.register_get_comment_response({"id": parent_id, "thread_id": "test_thread"}) + self.register_post_comment_response( + { + "id": "test_comment", + "thread_id": "test_thread", + "username": self.user.username, + "created_at": "2015-05-27T00:00:00Z", + "updated_at": "2015-05-27T00:00:00Z", + }, + thread_id=(None if parent_id else "test_thread"), + parent_id=parent_id + ) + data = self.minimal_data.copy() + if parent_id: + data["parent_id"] = parent_id + actual = create_comment(self.request, data) + expected = { + "id": "test_comment", + "thread_id": "test_thread", + "parent_id": parent_id, + "author": self.user.username, + "author_label": None, + "created_at": "2015-05-27T00:00:00Z", + "updated_at": "2015-05-27T00:00:00Z", + "raw_body": "Test body", + "endorsed": False, + "endorsed_by": None, + "endorsed_by_label": None, + "endorsed_at": None, + "abuse_flagged": False, + "voted": False, + "vote_count": 0, + "children": [], + } + self.assertEqual(actual, expected) + expected_url = ( + "/api/v1/comments/{}".format(parent_id) if parent_id else + "/api/v1/threads/test_thread/comments" + ) + self.assertEqual( + urlparse(httpretty.last_request().path).path, + expected_url + ) + self.assertEqual( + httpretty.last_request().parsed_body, + { + "course_id": [unicode(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" + ) + expected_event_data = { + "discussion": {"id": "test_thread"}, + "commentable_id": "test_topic", + "options": {"followed": False}, + "id": "test_comment", + "truncated": False, + "body": "Test body", + "url": "", + "user_forums_roles": [FORUM_ROLE_STUDENT], + "user_course_roles": [], + } + if parent_id: + expected_event_data["response"] = {"id": parent_id} + actual_event_name, actual_event_data = mock_emit.call_args[0] + self.assertEqual(actual_event_name, expected_event_name) + self.assertEqual(actual_event_data, expected_event_data) + + def test_thread_id_missing(self): + with self.assertRaises(ValidationError) as assertion: + create_comment(self.request, {}) + self.assertEqual(assertion.exception.message_dict, {"thread_id": ["This field is required."]}) + + def test_thread_id_not_found(self): + self.register_get_thread_error_response("test_thread", 404) + with self.assertRaises(ValidationError) as assertion: + create_comment(self.request, self.minimal_data) + self.assertEqual(assertion.exception.message_dict, {"thread_id": ["Invalid value."]}) + + def test_nonexistent_course(self): + self.register_get_thread_response( + make_minimal_cs_thread({"id": "test_thread", "course_id": "non/existent/course"}) + ) + with self.assertRaises(ValidationError) as assertion: + create_comment(self.request, self.minimal_data) + self.assertEqual(assertion.exception.message_dict, {"thread_id": ["Invalid value."]}) + + def test_not_enrolled(self): + self.request.user = UserFactory.create() + with self.assertRaises(ValidationError) as assertion: + create_comment(self.request, self.minimal_data) + self.assertEqual(assertion.exception.message_dict, {"thread_id": ["Invalid value."]}) + + def test_discussions_disabled(self): + _remove_discussion_tab(self.course, self.user.id) + with self.assertRaises(ValidationError) as assertion: + create_comment(self.request, self.minimal_data) + self.assertEqual(assertion.exception.message_dict, {"thread_id": ["Invalid value."]}) + + def test_invalid_field(self): + data = self.minimal_data.copy() + del data["raw_body"] + with self.assertRaises(ValidationError): + create_comment(self.request, data) diff --git a/lms/djangoapps/discussion_api/tests/test_serializers.py b/lms/djangoapps/discussion_api/tests/test_serializers.py index e7a8a2a7da..106273b3bd 100644 --- a/lms/djangoapps/discussion_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion_api/tests/test_serializers.py @@ -440,3 +440,106 @@ class ThreadSerializerDeserializationTest(CommentsServiceMockMixin, UrlResetMixi data["type"] = "invalid_type" serializer = ThreadSerializer(data=data) self.assertFalse(serializer.is_valid()) + + +@ddt.ddt +class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStoreTestCase): + """Tests for ThreadSerializer deserialization.""" + def setUp(self): + super(CommentSerializerDeserializationTest, self).setUp() + httpretty.reset() + httpretty.enable() + self.addCleanup(httpretty.disable) + self.course = CourseFactory.create() + self.user = UserFactory.create() + self.register_get_user_response(self.user) + self.request = RequestFactory().get("/dummy") + self.request.user = self.user + self.minimal_data = { + "thread_id": "test_thread", + "raw_body": "Test body", + } + + def save_and_reserialize(self, data, parent_id=None): + """ + Create a serializer with the given data, ensure that it is valid, save + the result, and return the full comment data from the serializer. + """ + context = get_context( + self.course, + self.request, + make_minimal_cs_thread({"course_id": unicode(self.course.id)}), + parent_id + ) + serializer = CommentSerializer(data=data, context=context) + self.assertTrue(serializer.is_valid()) + serializer.save() + return serializer.data + + @ddt.data(None, "test_parent") + def test_success(self, parent_id): + if parent_id: + self.register_get_comment_response({"thread_id": "test_thread", "id": parent_id}) + self.register_post_comment_response( + {"id": "test_comment"}, + thread_id=(None if parent_id else "test_thread"), + parent_id=parent_id + ) + saved = self.save_and_reserialize(self.minimal_data, parent_id=parent_id) + expected_url = ( + "/api/v1/comments/{}".format(parent_id) if parent_id else + "/api/v1/threads/test_thread/comments" + ) + self.assertEqual(urlparse(httpretty.last_request().path).path, expected_url) + self.assertEqual( + httpretty.last_request().parsed_body, + { + "course_id": [unicode(self.course.id)], + "body": ["Test body"], + "user_id": [str(self.user.id)], + } + ) + self.assertEqual(saved["id"], "test_comment") + self.assertEqual(saved["parent_id"], parent_id) + + def test_parent_id_nonexistent(self): + self.register_get_comment_error_response("bad_parent", 404) + context = get_context(self.course, self.request, make_minimal_cs_thread(), "bad_parent") + serializer = CommentSerializer(data=self.minimal_data, context=context) + self.assertFalse(serializer.is_valid()) + self.assertEqual( + serializer.errors, + { + "non_field_errors": [ + "parent_id does not identify a comment in the thread identified by thread_id." + ] + } + ) + + def test_parent_id_wrong_thread(self): + self.register_get_comment_response({"thread_id": "different_thread", "id": "test_parent"}) + context = get_context(self.course, self.request, make_minimal_cs_thread(), "test_parent") + serializer = CommentSerializer(data=self.minimal_data, context=context) + self.assertFalse(serializer.is_valid()) + self.assertEqual( + serializer.errors, + { + "non_field_errors": [ + "parent_id does not identify a comment in the thread identified by thread_id." + ] + } + ) + + def test_missing_field(self): + for field in self.minimal_data: + data = self.minimal_data.copy() + data.pop(field) + serializer = CommentSerializer( + data=data, + context=get_context(self.course, self.request, make_minimal_cs_thread()) + ) + self.assertFalse(serializer.is_valid()) + self.assertEqual( + serializer.errors, + {field: ["This field is required."]} + ) diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index dfe6eb386f..545f8165dd 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -3,6 +3,7 @@ Tests for Discussion API views """ from datetime import datetime import json +from urlparse import urlparse import httpretty import mock @@ -416,3 +417,86 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "mark_as_read": ["True"], } ) + + +@httpretty.activate +class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): + """Tests for CommentViewSet create""" + def setUp(self): + super(CommentViewSetCreateTest, self).setUp() + self.url = reverse("comment-list") + + def test_basic(self): + self.register_get_user_response(self.user) + self.register_get_thread_response( + make_minimal_cs_thread({ + "id": "test_thread", + "course_id": unicode(self.course.id), + "commentable_id": "test_topic", + }) + ) + self.register_post_comment_response( + { + "id": "test_comment", + "thread_id": "test_thread", + "username": self.user.username, + "created_at": "2015-05-27T00:00:00Z", + "updated_at": "2015-05-27T00:00:00Z", + }, + thread_id="test_thread" + ) + request_data = { + "thread_id": "test_thread", + "raw_body": "Test body", + } + expected_response_data = { + "id": "test_comment", + "thread_id": "test_thread", + "parent_id": None, + "author": self.user.username, + "author_label": None, + "created_at": "2015-05-27T00:00:00Z", + "updated_at": "2015-05-27T00:00:00Z", + "raw_body": "Test body", + "endorsed": False, + "endorsed_by": None, + "endorsed_by_label": None, + "endorsed_at": None, + "abuse_flagged": False, + "voted": False, + "vote_count": 0, + "children": [], + } + response = self.client.post( + self.url, + json.dumps(request_data), + content_type="application/json" + ) + self.assertEqual(response.status_code, 200) + response_data = json.loads(response.content) + self.assertEqual(response_data, expected_response_data) + self.assertEqual( + urlparse(httpretty.last_request().path).path, + "/api/v1/threads/test_thread/comments" + ) + self.assertEqual( + httpretty.last_request().parsed_body, + { + "course_id": [unicode(self.course.id)], + "body": ["Test body"], + "user_id": [str(self.user.id)], + } + ) + + def test_error(self): + response = self.client.post( + self.url, + json.dumps({}), + content_type="application/json" + ) + expected_response_data = { + "field_errors": {"thread_id": {"developer_message": "This field is required."}} + } + self.assertEqual(response.status_code, 400) + response_data = json.loads(response.content) + self.assertEqual(response_data, expected_response_data) diff --git a/lms/djangoapps/discussion_api/tests/utils.py b/lms/djangoapps/discussion_api/tests/utils.py index 9ecfc71eb5..726eaf20d1 100644 --- a/lms/djangoapps/discussion_api/tests/utils.py +++ b/lms/djangoapps/discussion_api/tests/utils.py @@ -61,6 +61,56 @@ class CommentsServiceMockMixin(object): status=200 ) + def register_post_comment_response(self, response_overrides, thread_id=None, parent_id=None): + """ + Register a mock response for POST on the CS comments endpoint for the + given thread or parent; exactly one of thread_id and parent_id must be + specified. + """ + def callback(request, _uri, headers): + """ + Simulate the comment creation endpoint by returning the provided data + along with the data from response_overrides. + """ + response_data = make_minimal_cs_comment( + {key: val[0] for key, val in request.parsed_body.items()} + ) + response_data.update(response_overrides or {}) + return (200, headers, json.dumps(response_data)) + + if thread_id and not parent_id: + url = "http://localhost:4567/api/v1/threads/{}/comments".format(thread_id) + elif parent_id and not thread_id: + url = "http://localhost:4567/api/v1/comments/{}".format(parent_id) + else: # pragma: no cover + raise ValueError("Exactly one of thread_id and parent_id must be provided.") + + httpretty.register_uri(httpretty.POST, url, body=callback) + + def register_get_comment_error_response(self, comment_id, status_code): + """ + Register a mock error response for GET on the CS comment instance + endpoint. + """ + httpretty.register_uri( + httpretty.GET, + "http://localhost:4567/api/v1/comments/{id}".format(id=comment_id), + body="", + status=status_code + ) + + def register_get_comment_response(self, response_overrides): + """ + Register a mock response for GET on the CS comment instance endpoint. + """ + comment = make_minimal_cs_comment(response_overrides) + httpretty.register_uri( + httpretty.GET, + "http://localhost:4567/api/v1/comments/{id}".format(id=comment["id"]), + body=json.dumps(comment), + status=200 + ) + def register_get_user_response(self, user, subscribed_thread_ids=None, upvoted_ids=None): """Register a mock response for GET on the CS user instance endpoint""" httpretty.register_uri( diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 2e82641246..96ce59ec06 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -11,7 +11,13 @@ from rest_framework.viewsets import ViewSet from opaque_keys.edx.locator import CourseLocator -from discussion_api.api import create_thread, get_comment_list, get_course_topics, get_thread_list +from discussion_api.api import ( + create_comment, + create_thread, + get_comment_list, + get_course_topics, + get_thread_list, +) from discussion_api.forms import CommentListGetForm, ThreadListGetForm from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin @@ -173,6 +179,12 @@ class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): GET /api/discussion/v1/comments/?thread_id=0123456789abcdef01234567 + POST /api/discussion/v1/comments/ + { + "thread_id": "0123456789abcdef01234567", + "raw_body": "Body text" + } + **GET Parameters**: * thread_id (required): The thread to retrieve comments for @@ -185,55 +197,67 @@ class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): * page_size: The number of items per page (default is 10, max is 100) - **Response Values**: + **POST Parameters**: - * results: The list of comments. Each item in the list includes: + * thread_id (required): The thread to post the comment in - * id: The id of the comment + * parent_id: The parent comment of the new comment. Can be null or + omitted for a comment that should be directly under the thread - * thread_id: The id of the comment's thread + * raw_body: The comment's raw body text - * parent_id: The id of the comment's parent + **GET Response Values**: - * author: The username of the comment's author, or None if the - comment is anonymous - - * author_label: A label indicating whether the author has a special - role in the course, either "staff" for moderators and - administrators or "community_ta" for community TAs - - * created_at: The ISO 8601 timestamp for the creation of the comment - - * updated_at: The ISO 8601 timestamp for the last modification of - the comment, which may not have been an update of the body - - * raw_body: The comment's raw body text without any rendering applied - - * endorsed: Boolean indicating whether the comment has been endorsed - (by a privileged user or, for a question thread, the thread - author) - - * endorsed_by: The username of the endorsing user, if available - - * endorsed_by_label: A label indicating whether the endorsing user - has a special role in the course (see author_label) - - * endorsed_at: The ISO 8601 timestamp for the endorsement, if - available - - * abuse_flagged: Boolean indicating whether the requesting user has - flagged the comment for abuse - - * voted: Boolean indicating whether the requesting user has voted - for the comment - - * vote_count: The number of votes for the comment - - * children: The list of child comments (with the same format) + * results: The list of comments; each item in the list has the same + fields as the POST response below * next: The URL of the next page (or null if first page) * previous: The URL of the previous page (or null if last page) + + **POST Response Values**: + + * id: The id of the comment + + * thread_id: The id of the comment's thread + + * parent_id: The id of the comment's parent + + * author: The username of the comment's author, or None if the + comment is anonymous + + * author_label: A label indicating whether the author has a special + role in the course, either "staff" for moderators and + administrators or "community_ta" for community TAs + + * created_at: The ISO 8601 timestamp for the creation of the comment + + * updated_at: The ISO 8601 timestamp for the last modification of + the comment, which may not have been an update of the body + + * raw_body: The comment's raw body text without any rendering applied + + * endorsed: Boolean indicating whether the comment has been endorsed + (by a privileged user or, for a question thread, the thread + author) + + * endorsed_by: The username of the endorsing user, if available + + * endorsed_by_label: A label indicating whether the endorsing user + has a special role in the course (see author_label) + + * endorsed_at: The ISO 8601 timestamp for the endorsement, if + available + + * abuse_flagged: Boolean indicating whether the requesting user has + flagged the comment for abuse + + * voted: Boolean indicating whether the requesting user has voted + for the comment + + * vote_count: The number of votes for the comment + + * children: The list of child comments (with the same format) """ def list(self, request): """ @@ -252,3 +276,10 @@ class CommentViewSet(_ViewMixin, DeveloperErrorViewMixin, ViewSet): form.cleaned_data["page_size"] ) ) + + def create(self, request): + """ + Implements the POST method for the list endpoint as described in the + class docstring. + """ + return Response(create_comment(request, request.DATA)) diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index 805526fb6b..fddefa89c0 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/lms/djangoapps/django_comment_client/base/views.py @@ -38,6 +38,8 @@ log = logging.getLogger(__name__) TRACKING_MAX_FORUM_BODY = 2000 THREAD_CREATED_EVENT_NAME = "edx.forum.thread.created" +RESPONSE_CREATED_EVENT_NAME = 'edx.forum.response.created' +COMMENT_CREATED_EVENT_NAME = 'edx.forum.comment.created' def permitted(fn): @@ -119,6 +121,29 @@ def get_thread_created_event_data(thread, followed): } +def get_comment_created_event_name(comment): + """Get the appropriate event name for creating a response/comment""" + return COMMENT_CREATED_EVENT_NAME if comment.get("parent_id") else RESPONSE_CREATED_EVENT_NAME + + +def get_comment_created_event_data(comment, commentable_id, followed): + """ + Get the event data payload for comment creation (excluding fields populated + by track_forum_event) + """ + event_data = { + 'discussion': {'id': comment.thread_id}, + 'commentable_id': commentable_id, + 'options': {'followed': followed}, + } + + parent_id = comment.get("parent_id") + if parent_id: + event_data['response'] = {'id': parent_id} + + return event_data + + @require_POST @login_required @permitted @@ -270,16 +295,8 @@ def _create_comment(request, course_key, thread_id=None, parent_id=None): user = cc.User.from_django_user(request.user) user.follow(comment.thread) - event_data = {'discussion': {'id': comment.thread_id}, 'options': {'followed': followed}} - - if parent_id: - event_data['response'] = {'id': comment.parent_id} - event_name = 'edx.forum.comment.created' - else: - event_name = 'edx.forum.response.created' - - event_data['commentable_id'] = comment.thread.commentable_id - + event_name = get_comment_created_event_name(comment) + event_data = get_comment_created_event_data(comment, comment.thread.commentable_id, followed) track_forum_event(request, event_name, course, comment, event_data) if request.is_ajax(): diff --git a/lms/lib/comment_client/comment.py b/lms/lib/comment_client/comment.py index fce94e784c..d2c8dcbcf7 100644 --- a/lms/lib/comment_client/comment.py +++ b/lms/lib/comment_client/comment.py @@ -32,10 +32,10 @@ class Comment(models.Model): @classmethod def url_for_comments(cls, params={}): - if params.get('thread_id'): - return _url_for_thread_comments(params['thread_id']) - else: + if params.get('parent_id'): return _url_for_comment(params['parent_id']) + else: + return _url_for_thread_comments(params['thread_id']) @classmethod def url(cls, action, params={}): diff --git a/openedx/core/lib/api/view_utils.py b/openedx/core/lib/api/view_utils.py index 4286d33c48..674dd5995d 100644 --- a/openedx/core/lib/api/view_utils.py +++ b/openedx/core/lib/api/view_utils.py @@ -27,7 +27,11 @@ class DeveloperErrorViewMixin(object): if hasattr(validation_error, "message_dict"): response_obj = {} message_dict = dict(validation_error.message_dict) - non_field_error_list = message_dict.pop(NON_FIELD_ERRORS, None) + # Extract both Django form and DRF serializer non-field errors + non_field_error_list = ( + message_dict.pop(NON_FIELD_ERRORS, []) + + message_dict.pop("non_field_errors", []) + ) if non_field_error_list: response_obj["developer_message"] = non_field_error_list[0] if message_dict: