Skip to content

E
edx-platform
Commit 2c7590d1 by Greg Price
Options

Use parent_id returned from the comments service 

This depends on cs_comments_service commit 0487891.
parent 77395bdf
Showing with 39 additions and 39 deletions
lms/djangoapps/discussion_api/api.py
......@@ -41,7 +41,7 @@ def _get_course_or_404(course_key, user):
return course
def _get_thread_and_context(request, thread_id, parent_id=None, retrieve_kwargs=None):
def _get_thread_and_context(request, thread_id, retrieve_kwargs=None):
"""
Retrieve the given thread and build a serializer context for it, returning
both. This function also enforces access control for the thread (checking
......@@ -56,7 +56,7 @@ def _get_thread_and_context(request, thread_id, parent_id=None, retrieve_kwargs=
cc_thread = Thread(id=thread_id).retrieve(**retrieve_kwargs)
course_key = CourseKey.from_string(cc_thread["course_id"])
course = _get_course_or_404(course_key, request.user)
context = get_context(course, request, cc_thread, parent_id)
context = get_context(course, request, cc_thread)
if (
not context["is_requester_privileged"] and
cc_thread["group_id"] and
......@@ -350,11 +350,10 @@ def create_comment(request, comment_data):
detail.
"""
thread_id = comment_data.get("thread_id")
parent_id = comment_data.get("parent_id")
if not thread_id:
raise ValidationError({"thread_id": ["This field is required."]})
try:
cc_thread, context = _get_thread_and_context(request, thread_id, parent_id)
cc_thread, context = _get_thread_and_context(request, thread_id)
except Http404:
raise ValidationError({"thread_id": ["Invalid value."]})
......
lms/djangoapps/discussion_api/serializers.py
......@@ -24,7 +24,7 @@ from openedx.core.djangoapps.course_groups.cohorts import get_cohort_names
from openedx.core.lib.api.fields import NonEmptyCharField
def get_context(course, request, thread=None, parent_id=None):
def get_context(course, request, thread=None):
"""
Returns a context appropriate for use with ThreadSerializer or
(if thread is provided) CommentSerializer.
......@@ -48,7 +48,6 @@ def get_context(course, request, thread=None, parent_id=None):
"course": course,
"request": request,
"thread": thread,
"parent_id": parent_id,
# For now, the only groups are cohorts
"group_ids_to_names": get_cohort_names(course),
"is_requester_privileged": requester.id in staff_user_ids or requester.id in ta_user_ids,
......@@ -219,25 +218,18 @@ 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")
parent_id = serializers.CharField(required=False)
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")
children = serializers.SerializerMethodField("get_children")
def get_parent_id(self, _obj):
"""Returns the comment's parent's id (taken from the context)."""
return self.context["parent_id"]
def get_endorsed_by(self, obj):
"""
Returns the username of the endorsing user, if the information is
......@@ -272,11 +264,8 @@ class CommentSerializer(_ContentSerializer):
return endorsement["time"] if endorsement else None
def get_children(self, obj):
"""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
CommentSerializer(child, context=self.context).data
for child in obj.get("children", [])
]
......@@ -285,7 +274,7 @@ class CommentSerializer(_ContentSerializer):
Ensure that parent_id identifies a comment that is actually in the
thread identified by thread_id.
"""
parent_id = self.context["parent_id"]
parent_id = attrs.get("parent_id")
if parent_id:
parent = None
try:
......@@ -303,7 +292,6 @@ class CommentSerializer(_ContentSerializer):
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
)
lms/djangoapps/discussion_api/tests/test_api.py
......@@ -1220,12 +1220,11 @@ class CreateCommentTest(CommentsServiceMockMixin, UrlResetMixin, ModuleStoreTest
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"),
thread_id="test_thread",
parent_id=parent_id
)
data = self.minimal_data.copy()
......
lms/djangoapps/discussion_api/tests/test_serializers.py
......@@ -354,10 +354,17 @@ class CommentSerializerTest(SerializerTestMixin, ModuleStoreTestCase):
"children": [
self.make_cs_content({
"id": "test_child_1",
"parent_id": "test_root",
}),
self.make_cs_content({
"id": "test_child_2",
"children": [self.make_cs_content({"id": "test_grandchild"})],
"parent_id": "test_root",
"children": [
self.make_cs_content({
"id": "test_grandchild",
"parent_id": "test_child_2"
})
],
}),
],
})
......@@ -547,7 +554,7 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
"raw_body": "Test body",
}
def save_and_reserialize(self, data, parent_id=None):
def save_and_reserialize(self, data):
"""
Create a serializer with the given data, ensure that it is valid, save
the result, and return the full comment data from the serializer.
......@@ -555,8 +562,7 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
context = get_context(
self.course,
self.request,
make_minimal_cs_thread({"course_id": unicode(self.course.id)}),
parent_id
make_minimal_cs_thread({"course_id": unicode(self.course.id)})
)
serializer = CommentSerializer(data=data, context=context)
self.assertTrue(serializer.is_valid())
......@@ -565,14 +571,16 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
@ddt.data(None, "test_parent")
def test_success(self, parent_id):
data = self.minimal_data.copy()
if parent_id:
data["parent_id"] = 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"),
thread_id="test_thread",
parent_id=parent_id
)
saved = self.save_and_reserialize(self.minimal_data, parent_id=parent_id)
saved = self.save_and_reserialize(data)
expected_url = (
"/api/v1/comments/{}".format(parent_id) if parent_id else
"/api/v1/threads/test_thread/comments"
......@@ -591,8 +599,10 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
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)
data = self.minimal_data.copy()
data["parent_id"] = "bad_parent"
context = get_context(self.course, self.request, make_minimal_cs_thread())
serializer = CommentSerializer(data=data, context=context)
self.assertFalse(serializer.is_valid())
self.assertEqual(
serializer.errors,
......@@ -605,8 +615,10 @@ class CommentSerializerDeserializationTest(CommentsServiceMockMixin, ModuleStore
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)
data = self.minimal_data.copy()
data["parent_id"] = "test_parent"
context = get_context(self.course, self.request, make_minimal_cs_thread())
serializer = CommentSerializer(data=data, context=context)
self.assertFalse(serializer.is_valid())
self.assertEqual(
serializer.errors,
......
lms/djangoapps/discussion_api/tests/test_views.py
......@@ -539,7 +539,6 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
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",
......
lms/djangoapps/discussion_api/tests/utils.py
......@@ -84,7 +84,7 @@ class CommentsServiceMockMixin(object):
status=200
)
def register_post_comment_response(self, response_overrides, thread_id=None, parent_id=None):
def register_post_comment_response(self, response_overrides, thread_id, 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
......@@ -99,14 +99,16 @@ class CommentsServiceMockMixin(object):
{key: val[0] for key, val in request.parsed_body.items()}
)
response_data.update(response_overrides or {})
# thread_id and parent_id are not included in request payload but
# are returned by the comments service
response_data["thread_id"] = thread_id
response_data["parent_id"] = parent_id
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:
if parent_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.")
else:
url = "http://localhost:4567/api/v1/threads/{}/comments".format(thread_id)
httpretty.register_uri(httpretty.POST, url, body=callback)
......@@ -228,6 +230,7 @@ def make_minimal_cs_comment(overrides=None):
ret = {
"id": "dummy",
"thread_id": "dummy",
"parent_id": None,
"user_id": "0",
"username": "dummy",
"anonymous": False,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment