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

feat: updated api to get all question type reponses (#34215)

Browse Source 
* feat: updated api to get all question type reponses

* test: fixed and added new test cases
This commit is contained in:
ayesha waris
2024-02-29 11:58:07 +05:00
committed by GitHub
parent c13c7d3bc0
commit c913a55b17
8 changed files with 119 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

13
lms/djangoapps/discussion/django_comment_client/base/tests.py
View File

@@ -399,6 +399,7 @@ class ViewsQueryCountTestCase(
Decorates test methods to count mongo and SQL calls for a
particular modulestore.
"""
def inner(self, default_store, block_count, mongo_calls, sql_queries, *args, **kwargs):
with modulestore().default_store(default_store):
self.set_up_course(block_count=block_count)
@@ -794,7 +795,8 @@ class ViewsTestCase(
('get', f'{CS_PREFIX}/threads/518d4237b023791dca00000d'),
{
'data': None,
'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False, 'reverse_order': False},
'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False, 'reverse_order': False,
'merge_question_type_responses': False},
'headers': ANY,
'timeout': 5
}
@@ -812,7 +814,8 @@ class ViewsTestCase(
('get', f'{CS_PREFIX}/threads/518d4237b023791dca00000d'),
{
'data': None,
'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False, 'reverse_order': False},
'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False, 'reverse_order': False,
'merge_question_type_responses': False},
'headers': ANY,
'timeout': 5
}
@@ -871,7 +874,8 @@ class ViewsTestCase(
('get', f'{CS_PREFIX}/threads/518d4237b023791dca00000d'),
{
'data': None,
'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False, 'reverse_order': False},
'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False, 'reverse_order': False,
'merge_question_type_responses': False},
'headers': ANY,
'timeout': 5
}
@@ -889,7 +893,8 @@ class ViewsTestCase(
('get', f'{CS_PREFIX}/threads/518d4237b023791dca00000d'),
{
'data': None,
'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False, 'reverse_order': False},
'params': {'mark_as_read': True, 'request_id': ANY, 'with_responses': False, 'reverse_order': False,
'merge_question_type_responses': False},
'headers': ANY,
'timeout': 5
}

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

@@ -1168,7 +1168,8 @@ def get_learner_active_thread_list(request, course_key, query_params):
})
def get_comment_list(request, thread_id, endorsed, page, page_size, flagged=False, requested_fields=None):
def get_comment_list(request, thread_id, endorsed, page, page_size, flagged=False, requested_fields=None,
merge_question_type_responses=False):
"""
Return the list of comments in the given thread.
@@ -1211,13 +1212,13 @@ def get_comment_list(request, thread_id, endorsed, page, page_size, flagged=Fals
"response_skip": response_skip,
"response_limit": page_size,
"reverse_order": reverse_order,
"merge_question_type_responses": merge_question_type_responses
}
)
# Responses to discussion threads cannot be separated by endorsed, but
# responses to question threads must be separated by endorsed due to the
# existing comments service interface
if cc_thread["thread_type"] == "question":
if cc_thread["thread_type"] == "question" and not merge_question_type_responses:
if endorsed is None: # lint-amnesty, pylint: disable=no-else-raise
raise ValidationError({"endorsed": ["This field is required for question threads."]})
elif endorsed:
@@ -1229,10 +1230,11 @@ def get_comment_list(request, thread_id, endorsed, page, page_size, flagged=Fals
responses = cc_thread["non_endorsed_responses"]
resp_total = cc_thread["non_endorsed_resp_total"]
else:
if endorsed is not None:
raise ValidationError(
{"endorsed": ["This field may not be specified for discussion threads."]}
)
if not merge_question_type_responses:
if endorsed is not None:
raise ValidationError(
{"endorsed": ["This field may not be specified for discussion threads."]}
)
responses = cc_thread["children"]
resp_total = cc_thread["resp_total"]

1
lms/djangoapps/discussion/rest_api/forms.py
View File

@@ -130,6 +130,7 @@ class CommentListGetForm(_PaginationForm):
flagged = BooleanField(required=False)
endorsed = ExtendedNullBooleanField(required=False)
requested_fields = MultiValueField(required=False)
merge_question_type_responses = BooleanField(required=False)
class UserCommentListGetForm(_PaginationForm):

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

@@ -1260,13 +1260,15 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
overrides.setdefault("course_id", str(self.course.id))
return make_minimal_cs_thread(overrides)
def get_comment_list(self, thread, endorsed=None, page=1, page_size=1):
def get_comment_list(self, thread, endorsed=None, page=1, page_size=1,
merge_question_type_responses=False):
"""
Register the appropriate comments service response, then call
get_comment_list and return the result.
"""
self.register_get_thread_response(thread)
return get_comment_list(self.request, thread["id"], endorsed, page, page_size)
return get_comment_list(self.request, thread["id"], endorsed, page, page_size,
merge_question_type_responses=merge_question_type_responses)
def test_nonexistent_thread(self):
thread_id = "nonexistent_thread"
@@ -1398,10 +1400,14 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
"resp_limit": ["14"],
"with_responses": ["True"],
"reverse_order": ["False"],
"merge_question_type_responses": ["False"],
}
)
def test_discussion_content(self):
def get_source_and_expected_comments(self):
"""
Returns the source comments and expected comments for testing purposes.
"""
source_comments = [
{
"type": "comment",
@@ -1414,7 +1420,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
"created_at": "2015-05-11T00:00:00Z",
"updated_at": "2015-05-11T11:11:11Z",
"body": "Test body",
"endorsed": False,
"endorsed": True,
"abuse_flaggers": [],
"votes": {"up_count": 4},
"child_count": 0,
@@ -1449,7 +1455,7 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
"updated_at": "2015-05-11T11:11:11Z",
"raw_body": "Test body",
"rendered_body": "<p>Test body</p>",
"endorsed": False,
"endorsed": True,
"endorsed_by": None,
"endorsed_by_label": None,
"endorsed_at": None,
@@ -1508,12 +1514,26 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
},
},
]
return source_comments, expected_comments
def test_discussion_content(self):
source_comments, expected_comments = self.get_source_and_expected_comments()
actual_comments = self.get_comment_list(
self.make_minimal_cs_thread({"children": source_comments})
).data["results"]
assert actual_comments == expected_comments
def test_question_content(self):
def test_question_content_with_merge_question_type_responses(self):
source_comments, expected_comments = self.get_source_and_expected_comments()
actual_comments = self.get_comment_list(
self.make_minimal_cs_thread({
"thread_type": "question",
"children": source_comments,
"resp_total": len(source_comments)
}), merge_question_type_responses=True).data["results"]
assert actual_comments == expected_comments
def test_question_content_(self):
thread = self.make_minimal_cs_thread({
"thread_type": "question",
"endorsed_responses": [make_minimal_cs_comment({"id": "endorsed_comment", "username": self.user.username})],
@@ -1547,11 +1567,13 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
assert actual_comments[0]['endorsed_by'] is None
@ddt.data(
("discussion", None, "children", "resp_total"),
("question", False, "non_endorsed_responses", "non_endorsed_resp_total"),
("discussion", None, "children", "resp_total", False),
("question", False, "non_endorsed_responses", "non_endorsed_resp_total", False),
("question", None, "children", "resp_total", True),
)
@ddt.unpack
def test_cs_pagination(self, thread_type, endorsed_arg, response_field, response_total_field):
def test_cs_pagination(self, thread_type, endorsed_arg, response_field,
response_total_field, merge_question_type_responses):
"""
Test cases in which pagination is done by the comments service.
@@ -1571,22 +1593,26 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
})
# Only page
actual = self.get_comment_list(thread, endorsed=endorsed_arg, page=1, page_size=5).data
actual = self.get_comment_list(thread, endorsed=endorsed_arg, page=1, page_size=5,
merge_question_type_responses=merge_question_type_responses).data
assert actual['pagination']['next'] is None
assert actual['pagination']['previous'] is None
# First page of many
actual = self.get_comment_list(thread, endorsed=endorsed_arg, page=1, page_size=2).data
actual = self.get_comment_list(thread, endorsed=endorsed_arg, page=1, page_size=2,
merge_question_type_responses=merge_question_type_responses).data
assert actual['pagination']['next'] == 'http://testserver/test_path?page=2'
assert actual['pagination']['previous'] is None
# Middle page of many
actual = self.get_comment_list(thread, endorsed=endorsed_arg, page=2, page_size=2).data
actual = self.get_comment_list(thread, endorsed=endorsed_arg, page=2, page_size=2,
merge_question_type_responses=merge_question_type_responses).data
assert actual['pagination']['next'] == 'http://testserver/test_path?page=3'
assert actual['pagination']['previous'] == 'http://testserver/test_path?page=1'
# Last page of many
actual = self.get_comment_list(thread, endorsed=endorsed_arg, page=3, page_size=2).data
actual = self.get_comment_list(thread, endorsed=endorsed_arg, page=3, page_size=2,
merge_question_type_responses=merge_question_type_responses).data
assert actual['pagination']['next'] is None
assert actual['pagination']['previous'] == 'http://testserver/test_path?page=2'
@@ -1597,7 +1623,8 @@ class GetCommentListTest(ForumsEnableMixin, CommentsServiceMockMixin, SharedModu
response_total_field: 5
})
with pytest.raises(PageNotFoundError):
self.get_comment_list(thread, endorsed=endorsed_arg, page=2, page_size=5)
self.get_comment_list(thread, endorsed=endorsed_arg, page=2, page_size=5,
merge_question_type_responses=merge_question_type_responses)
def test_question_endorsed_pagination(self):
thread = self.make_minimal_cs_thread({
@@ -4078,6 +4105,7 @@ class CourseTopicsV2Test(ModuleStoreTestCase):
"""
Tests for discussions topic API v2 code.
"""
def setUp(self) -> None:
super().setUp()
self.course = CourseFactory.create(

3
lms/djangoapps/discussion/rest_api/tests/test_forms.py
View File

@@ -207,7 +207,8 @@ class CommentListGetFormTest(FormTestMixin, PaginationTestMixin, TestCase):
'page': 2,
'page_size': 13,
'flagged': False,
'requested_fields': set()
'requested_fields': set(),
'merge_question_type_responses': False
}
def test_missing_thread_id(self):

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

@@ -496,6 +496,7 @@ class CommentViewSetListByUserTest(
@override_settings(DISCUSSION_MODERATION_CLOSE_REASON_CODES={"test-close-reason": "Test Close Reason"})
class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""Tests for CourseView"""
def setUp(self):
super().setUp()
self.url = reverse("discussion_course", kwargs={"course_id": str(self.course.id)})
@@ -545,6 +546,7 @@ class CourseViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class RetireViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""Tests for CourseView"""
def setUp(self):
super().setUp()
RetirementState.objects.create(state_name='PENDING', state_execution_order=1)
@@ -620,6 +622,7 @@ class RetireViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class ReplaceUsernamesViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""Tests for ReplaceUsernamesView"""
def setUp(self):
super().setUp()
self.worker = UserFactory()
@@ -715,6 +718,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, CommentsServiceMockMixin,
"""
Tests for CourseTopicsView
"""
def setUp(self):
httpretty.reset()
httpretty.enable()
@@ -925,6 +929,7 @@ class CourseTopicsViewV3Test(DiscussionAPIViewTestMixin, CommentsServiceMockMixi
"""
Tests for CourseTopicsViewV3
"""
def setUp(self) -> None:
super().setUp()
self.password = self.TEST_PASSWORD
@@ -1013,6 +1018,7 @@ class CourseTopicsViewV3Test(DiscussionAPIViewTestMixin, CommentsServiceMockMixi
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, ProfileImageTestMixin):
"""Tests for ThreadViewSet list"""
def setUp(self):
super().setUp()
self.author = UserFactory.create()
@@ -1354,6 +1360,7 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pro
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""Tests for ThreadViewSet create"""
def setUp(self):
super().setUp()
self.url = reverse("thread-list")
@@ -1424,6 +1431,7 @@ class ThreadViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin):
"""Tests for ThreadViewSet partial_update"""
def setUp(self):
self.unsupported_media_type = JSONParser.media_type
super().setUp()
@@ -1567,6 +1575,7 @@ class ThreadViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTest
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class ThreadViewSetDeleteTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""Tests for ThreadViewSet delete"""
def setUp(self):
super().setUp()
self.url = reverse("thread-detail", kwargs={"thread_id": "test_thread"})
@@ -1906,6 +1915,7 @@ class LearnerThreadViewAPITest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, ProfileImageTestMixin):
"""Tests for CommentViewSet list"""
def setUp(self):
super().setUp()
self.author = UserFactory.create()
@@ -2040,6 +2050,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
"recursive": ["False"],
"with_responses": ["True"],
"reverse_order": ["False"],
"merge_question_type_responses": ["False"],
}
)
@@ -2075,9 +2086,37 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
"recursive": ["False"],
"with_responses": ["True"],
"reverse_order": ["False"],
"merge_question_type_responses": ["False"],
}
)
def test_question_content_with_merge_question_type_responses(self):
self.register_get_user_response(self.user)
thread = self.make_minimal_cs_thread({
"thread_type": "question",
"children": [make_minimal_cs_comment({
"id": "endorsed_comment",
"user_id": self.user.id,
"username": self.user.username,
"endorsed": True,
}),
make_minimal_cs_comment({
"id": "non_endorsed_comment",
"user_id": self.user.id,
"username": self.user.username,
"endorsed": False,
})],
"resp_total": 2,
})
self.register_get_thread_response(thread)
response = self.client.get(self.url, {
"thread_id": thread["id"],
"merge_question_type_responses": True
})
parsed_content = json.loads(response.content.decode('utf-8'))
assert parsed_content['results'][0]['id'] == "endorsed_comment"
assert parsed_content['results'][1]['id'] == "non_endorsed_comment"
@ddt.data(
(True, "endorsed_comment"),
("true", "endorsed_comment"),
@@ -2144,7 +2183,12 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
}}
)
def test_child_comments_count(self):
@ddt.data(
("discussion", False),
("question", True)
)
@ddt.unpack
def test_child_comments_count(self, thread_type, merge_question_type_responses):
self.register_get_user_response(self.user)
response_1 = make_minimal_cs_comment({
"id": "test_response_1",
@@ -2163,15 +2207,16 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
thread = self.make_minimal_cs_thread({
"id": self.thread_id,
"course_id": str(self.course.id),
"thread_type": "discussion",
"thread_type": thread_type,
"children": [response_1, response_2],
"resp_total": 2,
"comments_count": 8,
"unread_comments_count": 0,
})
self.register_get_thread_response(thread)
response = self.client.get(self.url, {"thread_id": self.thread_id})
response = self.client.get(self.url, {
"thread_id": self.thread_id,
"merge_question_type_responses": merge_question_type_responses})
expected_comments = [
self.expected_response_comment(overrides={"id": "test_response_1", "child_count": 2, "can_delete": False}),
self.expected_response_comment(overrides={"id": "test_response_2", "child_count": 3, "can_delete": False}),
@@ -2316,6 +2361,7 @@ class CommentViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, Pr
"recursive": ["False"],
"with_responses": ["True"],
"reverse_order": ["True"],
"merge_question_type_responses": ["False"],
}
)
@@ -2365,6 +2411,7 @@ class CommentViewSetDeleteTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
@mock.patch("lms.djangoapps.discussion.signals.handlers.send_response_notifications", new=mock.Mock())
class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""Tests for CommentViewSet create"""
def setUp(self):
super().setUp()
self.url = reverse("comment-list")
@@ -2462,6 +2509,7 @@ class CommentViewSetCreateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, PatchMediaTypeMixin):
"""Tests for CommentViewSet partial_update"""
def setUp(self):
self.unsupported_media_type = JSONParser.media_type
super().setUp()
@@ -2586,6 +2634,7 @@ class CommentViewSetPartialUpdateTest(DiscussionAPIViewTestMixin, ModuleStoreTes
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class ThreadViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, ProfileImageTestMixin):
"""Tests for ThreadViewSet Retrieve"""
def setUp(self):
super().setUp()
self.url = reverse("thread-detail", kwargs={"thread_id": "test_thread"})
@@ -2637,6 +2686,7 @@ class ThreadViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase,
@mock.patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
class CommentViewSetRetrieveTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase, ProfileImageTestMixin):
"""Tests for CommentViewSet Retrieve"""
def setUp(self):
super().setUp()
self.url = reverse("comment-detail", kwargs={"comment_id": "test_comment"})

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

@@ -948,6 +948,7 @@ class CommentViewSet(DeveloperErrorViewMixin, ViewSet):
form.cleaned_data["page_size"],
form.cleaned_data["flagged"],
form.cleaned_data["requested_fields"],
form.cleaned_data["merge_question_type_responses"]
)
def list_by_user(self, request):

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

@@ -145,6 +145,7 @@ class Thread(models.Model):
'resp_skip': kwargs.get('response_skip'),
'resp_limit': kwargs.get('response_limit'),
'reverse_order': kwargs.get('reverse_order', False),
'merge_question_type_responses': kwargs.get('merge_question_type_responses', False)
}
request_params = utils.strip_none(request_params)