diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 2b3dc81405..46213b5312 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -342,7 +342,7 @@ def get_thread_list( return ret -def get_comment_list(request, thread_id, endorsed, page, page_size, mark_as_read=False): +def get_comment_list(request, thread_id, endorsed, page, page_size): """ Return the list of comments in the given thread. @@ -361,8 +361,6 @@ def get_comment_list(request, thread_id, endorsed, page, page_size, mark_as_read page_size: The number of comments to retrieve per page - mark_as_read: Marks the thread of the comment list as read. - Returns: A paginated result containing a list of comments; see @@ -375,7 +373,6 @@ def get_comment_list(request, thread_id, endorsed, page, page_size, mark_as_read retrieve_kwargs={ "recursive": False, "user_id": request.user.id, - "mark_as_read": mark_as_read, "response_skip": response_skip, "response_limit": page_size, } diff --git a/lms/djangoapps/discussion_api/forms.py b/lms/djangoapps/discussion_api/forms.py index c986663a3e..2bf779651c 100644 --- a/lms/djangoapps/discussion_api/forms.py +++ b/lms/djangoapps/discussion_api/forms.py @@ -109,7 +109,6 @@ class CommentListGetForm(_PaginationForm): # TODO: should we use something better here? This only accepts "True", # "False", "1", and "0" endorsed = NullBooleanField(required=False) - mark_as_read = BooleanField(required=False) class CommentActionsForm(Form): diff --git a/lms/djangoapps/discussion_api/permissions.py b/lms/djangoapps/discussion_api/permissions.py index 4385c070a3..5d3fa7990e 100644 --- a/lms/djangoapps/discussion_api/permissions.py +++ b/lms/djangoapps/discussion_api/permissions.py @@ -57,11 +57,14 @@ def get_editable_fields(cc_content, context): Return the set of fields that the requester can edit on the given content """ - # no edits, except 'abuse_flagged' are allowed on closed threads + # For closed thread: + # no edits, except 'abuse_flagged' and 'read' are allowed for thread + # no edits, except 'abuse_flagged' is allowed for comment ret = {"abuse_flagged"} - if (cc_content["type"] == "thread" and cc_content["closed"]) or ( - cc_content["type"] == "comment" and context["thread"]["closed"] - ): + if cc_content["type"] == "thread" and cc_content["closed"]: + ret |= {"read"} + return ret + if cc_content["type"] == "comment" and context["thread"]["closed"]: return ret # Shared fields @@ -71,7 +74,7 @@ def get_editable_fields(cc_content, context): # Thread fields if cc_content["type"] == "thread": - ret |= {"following"} + ret |= {"following", "read"} if _is_author_or_privileged(cc_content, context): ret |= {"topic_id", "type", "title"} if context["is_requester_privileged"] and context["course"].is_cohorted: diff --git a/lms/djangoapps/discussion_api/serializers.py b/lms/djangoapps/discussion_api/serializers.py index 8cb2d5b479..7a53d1a713 100644 --- a/lms/djangoapps/discussion_api/serializers.py +++ b/lms/djangoapps/discussion_api/serializers.py @@ -195,7 +195,7 @@ class ThreadSerializer(_ContentSerializer): comment_list_url = serializers.SerializerMethodField() endorsed_comment_list_url = serializers.SerializerMethodField() non_endorsed_comment_list_url = serializers.SerializerMethodField() - read = serializers.BooleanField(read_only=True) + read = serializers.BooleanField(required=False) has_endorsed = serializers.BooleanField(read_only=True, source="endorsed") response_count = serializers.IntegerField(source="resp_total", read_only=True, required=False) diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index 0bd5968657..85b8a43f43 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -649,7 +649,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_0", "endorsed_comment_list_url": None, "non_endorsed_comment_list_url": None, - "editable_fields": ["abuse_flagged", "following", "voted"], + "editable_fields": ["abuse_flagged", "following", "read", "voted"], "has_endorsed": True, "read": True, }, @@ -682,7 +682,7 @@ class GetThreadListTest(CommentsServiceMockMixin, UrlResetMixin, SharedModuleSto "non_endorsed_comment_list_url": ( "http://testserver/api/discussion/v1/comments/?thread_id=test_thread_id_1&endorsed=False" ), - "editable_fields": ["abuse_flagged", "following", "voted"], + "editable_fields": ["abuse_flagged", "following", "read", "voted"], "has_endorsed": False, "read": False, }, @@ -1402,7 +1402,7 @@ class CreateThreadTest( "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_id", "endorsed_comment_list_url": None, "non_endorsed_comment_list_url": None, - "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], + "editable_fields": ["abuse_flagged", "following", "raw_body", "read", "title", "topic_id", "type", "voted"], 'read': False, 'has_endorsed': False, 'response_count': 0, @@ -1948,7 +1948,7 @@ class UpdateThreadTest( "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread", "endorsed_comment_list_url": None, "non_endorsed_comment_list_url": None, - "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], + "editable_fields": ["abuse_flagged", "following", "raw_body", "read", "title", "topic_id", "type", "voted"], 'read': False, 'has_endorsed': False, 'response_count': 0 @@ -1967,6 +1967,7 @@ class UpdateThreadTest( "anonymous_to_peers": ["False"], "closed": ["False"], "pinned": ["False"], + "read": ["False"], } ) @@ -2989,7 +2990,7 @@ class RetrieveThreadTest( "abuse_flagged": False, "voted": False, "vote_count": 0, - "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], + "editable_fields": ["abuse_flagged", "following", "raw_body", "read", "title", "topic_id", "type", "voted"], "course_id": unicode(self.course.id), "topic_id": "test_topic", "group_id": None, @@ -3029,7 +3030,7 @@ class RetrieveThreadTest( "abuse_flagged": False, "voted": False, "vote_count": 0, - "editable_fields": ["abuse_flagged", "following", "voted"], + "editable_fields": ["abuse_flagged", "following", "read", "voted"], "course_id": unicode(self.course.id), "topic_id": "test_topic", "group_id": None, diff --git a/lms/djangoapps/discussion_api/tests/test_forms.py b/lms/djangoapps/discussion_api/tests/test_forms.py index 28db7d9a9a..ab09f0b969 100644 --- a/lms/djangoapps/discussion_api/tests/test_forms.py +++ b/lms/djangoapps/discussion_api/tests/test_forms.py @@ -156,8 +156,7 @@ class CommentListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase): "thread_id": "deadbeef", "endorsed": False, "page": 2, - "page_size": 13, - "mark_as_read": False + "page_size": 13 } ) diff --git a/lms/djangoapps/discussion_api/tests/test_permissions.py b/lms/djangoapps/discussion_api/tests/test_permissions.py index 78f0df9606..c272e76327 100644 --- a/lms/djangoapps/discussion_api/tests/test_permissions.py +++ b/lms/djangoapps/discussion_api/tests/test_permissions.py @@ -41,7 +41,7 @@ class GetInitializableFieldsTest(ModuleStoreTestCase): ) actual = get_initializable_thread_fields(context) expected = { - "abuse_flagged", "course_id", "following", "raw_body", "title", "topic_id", "type", "voted" + "abuse_flagged", "course_id", "following", "raw_body", "read", "title", "topic_id", "type", "voted" } if is_privileged and is_cohorted: expected |= {"group_id"} @@ -77,7 +77,7 @@ class GetEditableFieldsTest(ModuleStoreTestCase): is_cohorted=is_cohorted ) actual = get_editable_fields(thread, context) - expected = {"abuse_flagged", "following", "voted"} + expected = {"abuse_flagged", "following", "read", "voted"} if is_author or is_privileged: expected |= {"topic_id", "type", "title", "raw_body"} if is_privileged and is_cohorted: diff --git a/lms/djangoapps/discussion_api/tests/test_serializers.py b/lms/djangoapps/discussion_api/tests/test_serializers.py index b299d89814..ae2c53e7b4 100644 --- a/lms/djangoapps/discussion_api/tests/test_serializers.py +++ b/lms/djangoapps/discussion_api/tests/test_serializers.py @@ -208,7 +208,7 @@ class ThreadSerializerSerializationTest(SerializerTestMixin, SharedModuleStoreTe "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread", "endorsed_comment_list_url": None, "non_endorsed_comment_list_url": None, - "editable_fields": ["abuse_flagged", "following", "voted"], + "editable_fields": ["abuse_flagged", "following", "read", "voted"], "read": False, "has_endorsed": False, } @@ -561,16 +561,19 @@ class ThreadSerializerDeserializationTest(CommentsServiceMockMixin, UrlResetMixi "closed": ["False"], "pinned": ["False"], "user_id": [str(self.user.id)], + "read": ["False"], } ) - def test_update_all(self): + @ddt.data(True, False) + def test_update_all(self, read): self.register_put_thread_response(self.existing_thread.attributes) data = { "topic_id": "edited_topic", "type": "question", "title": "Edited Title", "raw_body": "Edited body", + "read": read, } saved = self.save_and_reserialize(data, self.existing_thread) self.assertEqual( @@ -586,6 +589,7 @@ class ThreadSerializerDeserializationTest(CommentsServiceMockMixin, UrlResetMixi "closed": ["False"], "pinned": ["False"], "user_id": [str(self.user.id)], + "read": [str(read)], } ) for key in data: diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index 35ab192577..536d86ad2c 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -301,7 +301,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread", "endorsed_comment_list_url": None, "non_endorsed_comment_list_url": None, - "editable_fields": ["abuse_flagged", "following", "voted"], + "editable_fields": ["abuse_flagged", "following", "read", "voted"], "read": False, "has_endorsed": False, }] @@ -524,7 +524,7 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "comment_list_url": "http://testserver/api/discussion/v1/comments/?thread_id=test_thread", "endorsed_comment_list_url": None, "non_endorsed_comment_list_url": None, - "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], + "editable_fields": ["abuse_flagged", "following", "raw_body", "read", "title", "topic_id", "type", "voted"], "read": False, "has_endorsed": False, "response_count": 0, @@ -619,7 +619,7 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest def test_basic(self): self.register_get_user_response(self.user) - self.register_thread({"created_at": "Test Date", "updated_at": "Test Date"}) + self.register_thread({"created_at": "Test Created Date", "updated_at": "Test Updated Date"}) request_data = {"raw_body": "Edited body"} response = self.request_patch(request_data) self.assertEqual(response.status_code, 200) @@ -629,9 +629,11 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest self.expected_response_data({ "raw_body": "Edited body", "rendered_body": "
Edited body
", - "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], - "created_at": "Test Date", - "updated_at": "Test Date", + "editable_fields": [ + "abuse_flagged", "following", "raw_body", "read", "title", "topic_id", "type", "voted" + ], + "created_at": "Test Created Date", + "updated_at": "Test Updated Date", }) ) self.assertEqual( @@ -647,6 +649,7 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest "anonymous_to_peers": ["False"], "closed": ["False"], "pinned": ["False"], + "read": ["False"], } ) @@ -680,7 +683,7 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest self.expected_response_data({ "closed": True, "abuse_flagged": value, - "editable_fields": ["abuse_flagged"], + "editable_fields": ["abuse_flagged", "read"], }) ) @@ -1036,7 +1039,7 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes def test_basic(self): self.register_thread() - self.register_comment({"created_at": "Test Date", "updated_at": "Test Date"}) + self.register_comment({"created_at": "Test Created Date", "updated_at": "Test Updated Date"}) request_data = {"raw_body": "Edited body"} response = self.request_patch(request_data) self.assertEqual(response.status_code, 200) @@ -1047,8 +1050,8 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes "raw_body": "Edited body", "rendered_body": "Edited body
", "editable_fields": ["abuse_flagged", "raw_body", "voted"], - "created_at": "Test Date", - "updated_at": "Test Date", + "created_at": "Test Created Date", + "updated_at": "Test Updated Date", }) ) self.assertEqual( @@ -1142,7 +1145,7 @@ class ThreadViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase) "abuse_flagged": False, "voted": False, "vote_count": 0, - "editable_fields": ["abuse_flagged", "following", "raw_body", "title", "topic_id", "type", "voted"], + "editable_fields": ["abuse_flagged", "following", "raw_body", "read", "title", "topic_id", "type", "voted"], "course_id": unicode(self.course.id), "topic_id": "test_topic", "group_id": None, diff --git a/lms/djangoapps/discussion_api/views.py b/lms/djangoapps/discussion_api/views.py index 516f57686f..83ab5cac0f 100644 --- a/lms/djangoapps/discussion_api/views.py +++ b/lms/djangoapps/discussion_api/views.py @@ -104,13 +104,15 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): """ **Use Cases** - Retrieve the list of threads for a course, post a new thread, or modify - or delete an existing thread. + Retrieve the list of threads for a course, retrieve thread details, + post a new thread, or modify or delete an existing thread. **Example Requests**: GET /api/discussion/v1/threads/?course_id=ExampleX/Demo/2015 + GET /api/discussion/v1/threads/thread_id + POST /api/discussion/v1/threads { "course_id": "foo/bar/baz", @@ -126,7 +128,7 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): DELETE /api/discussion/v1/threads/thread_id - **GET Parameters**: + **GET Thread List Parameters**: * course_id (required): The course to retrieve threads for @@ -176,13 +178,19 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): **PATCH Parameters**: - topic_id, type, title, and raw_body are accepted with the same meaning + * abuse_flagged (optional): A boolean to mark thread as abusive + + * voted (optional): A boolean to vote for thread + + * read (optional): A boolean to mark thread as read + + * topic_id, type, title, and raw_body are accepted with the same meaning as in a POST request If "application/merge-patch+json" is not the specified content type, a 415 error is returned. - **GET Response Values**: + **GET Thread List Response Values**: * results: The list of threads; each item in the list has the same fields as the POST/PATCH response below @@ -195,6 +203,10 @@ class ThreadViewSet(DeveloperErrorViewMixin, ViewSet): parameter was rewritten in order to match threads (e.g. for spelling correction) + **GET Thread Details Response Values**: + + Same response fields as the POST/PATCH response below + **POST/PATCH response values**: * id: The id of the thread @@ -300,7 +312,8 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet): """ **Use Cases** - Retrieve the list of comments in a thread, create a comment, or modify + Retrieve the list of comments in a thread, retrieve the list of + child comments for a response comment, create a comment, or modify or delete an existing comment. **Example Requests**: @@ -333,9 +346,6 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet): * page_size: The number of items per page (default is 10, max is 100) - * mark_as_read: Will mark the thread of the comments as read. (default - is False) - **GET Child Comment List Parameters**: * comment_id (required): The comment to retrieve child comments for @@ -439,8 +449,7 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet): form.cleaned_data["thread_id"], form.cleaned_data["endorsed"], form.cleaned_data["page"], - form.cleaned_data["page_size"], - form.cleaned_data["mark_as_read"] + form.cleaned_data["page_size"] ) ) diff --git a/lms/lib/comment_client/thread.py b/lms/lib/comment_client/thread.py index 2811f8975a..559ba35a3b 100644 --- a/lms/lib/comment_client/thread.py +++ b/lms/lib/comment_client/thread.py @@ -25,7 +25,7 @@ class Thread(models.Model): # updateable_fields are sent in PUT requests updatable_fields = [ - 'title', 'body', 'anonymous', 'anonymous_to_peers', 'course_id', + 'title', 'body', 'anonymous', 'anonymous_to_peers', 'course_id', 'read', 'closed', 'user_id', 'commentable_id', 'group_id', 'group_name', 'pinned', 'thread_type' ]