Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 2 commits into from
Feb 29, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5af4645
feat: updated api to get all question type reponses
ayesha-waris Feb 12, 2024
8818fcb
test: fixed and added new test cases
ayesha-waris Feb 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 9 additions & 4 deletions lms/djangoapps/discussion/django_comment_client/base/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -399,6 +399,7 @@ def count_queries(func): # pylint: disable=no-self-argument
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)
Expand Down Expand Up @@ -794,7 +795,8 @@ def flag_thread(self, mock_request, is_closed):
('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
}
Expand All @@ -812,7 +814,8 @@ def flag_thread(self, mock_request, is_closed):
('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
}
Expand Down Expand Up @@ -871,7 +874,8 @@ def un_flag_thread(self, mock_request, is_closed):
('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
}
Expand All @@ -889,7 +893,8 @@ def un_flag_thread(self, mock_request, is_closed):
('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
}
Expand Down
16 changes: 9 additions & 7 deletions lms/djangoapps/discussion/rest_api/api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down Expand Up @@ -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:
Expand All @@ -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"]

Expand Down
1 change: 1 addition & 0 deletions lms/djangoapps/discussion/rest_api/forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
56 changes: 42 additions & 14 deletions lms/djangoapps/discussion/rest_api/tests/test_api.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1260,13 +1260,15 @@ def make_minimal_cs_thread(self, overrides=None):
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"
Expand Down Expand Up @@ -1398,10 +1400,14 @@ def test_basic_query_params(self):
"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",
Expand All @@ -1414,7 +1420,7 @@ def test_discussion_content(self):
"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,
Expand Down Expand Up @@ -1449,7 +1455,7 @@ def test_discussion_content(self):
"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,
Expand Down Expand Up @@ -1508,12 +1514,26 @@ def test_discussion_content(self):
},
},
]
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})],
Expand Down Expand Up @@ -1547,11 +1567,13 @@ def test_endorsed_by_anonymity(self):
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.

Expand All @@ -1571,22 +1593,26 @@ def test_cs_pagination(self, thread_type, endorsed_arg, response_field, response
})

# 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'

Expand All @@ -1597,7 +1623,8 @@ def test_cs_pagination(self, thread_type, endorsed_arg, response_field, response
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({
Expand Down Expand Up @@ -4078,6 +4105,7 @@ class CourseTopicsV2Test(ModuleStoreTestCase):
"""
Tests for discussions topic API v2 code.
"""

def setUp(self) -> None:
super().setUp()
self.course = CourseFactory.create(
Expand Down
3 changes: 2 additions & 1 deletion lms/djangoapps/discussion/rest_api/tests/test_forms.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,8 @@ def test_basic(self):
'page': 2,
'page_size': 13,
'flagged': False,
'requested_fields': set()
'requested_fields': set(),
'merge_question_type_responses': False
}

def test_missing_thread_id(self):
Expand Down
Loading
Loading