Skip to content

E
edx-platform
Commit d3d4d330 by jsa
Options

Ensure group info is present in LMS responses. 

TNL-24
parent b3594b69
Showing with 312 additions and 53 deletions
lms/djangoapps/django_comment_client/base/tests.py
......@@ -12,7 +12,7 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey
from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE
from django_comment_client.base import views
from django_comment_client.tests.group_id import CohortedTopicGroupIdTestMixin, NonCohortedTopicGroupIdTestMixin
from django_comment_client.tests.group_id import CohortedTopicGroupIdTestMixin, NonCohortedTopicGroupIdTestMixin, GroupIdAssertionMixin
from django_comment_client.tests.utils import CohortedContentTestCase
from django_comment_client.tests.unicode import UnicodeTestMixin
from django_comment_common.models import Role, FORUM_ROLE_STUDENT
......@@ -29,15 +29,16 @@ CS_PREFIX = "http://localhost:4567/api/v1"
class MockRequestSetupMixin(object):
def _create_repsonse_mock(self, data):
return Mock(text=json.dumps(data), json=Mock(return_value=data))\
def _create_response_mock(self, data):
return Mock(text=json.dumps(data), json=Mock(return_value=data))
def _set_mock_request_data(self, mock_request, data):
mock_request.return_value = self._create_repsonse_mock(data)
mock_request.return_value = self._create_response_mock(data)
@patch('lms.lib.comment_client.utils.requests.request')
class CreateThreadGroupIdTestCase(
MockRequestSetupMixin,
CohortedContentTestCase,
CohortedTopicGroupIdTestMixin,
NonCohortedTopicGroupIdTestMixin
......@@ -45,6 +46,7 @@ class CreateThreadGroupIdTestCase(
cs_endpoint = "/threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
self._set_mock_request_data(mock_request, {})
mock_request.return_value.status_code = 200
request_data = {"body": "body", "title": "title", "thread_type": "discussion"}
if pass_group_id:
......@@ -59,6 +61,105 @@ class CreateThreadGroupIdTestCase(
commentable_id=commentable_id
)
def test_group_info_in_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
None
)
self._assert_json_response_contains_group_info(response)
@patch('lms.lib.comment_client.utils.requests.request')
class ThreadActionGroupIdTestCase(
MockRequestSetupMixin,
CohortedContentTestCase,
GroupIdAssertionMixin
):
def call_view(
self,
view_name,
mock_request,
user=None,
post_params=None,
view_args=None
):
self._set_mock_request_data(
mock_request,
{
"user_id": str(self.student.id),
"group_id": self.student_cohort.id,
"closed": False,
"type": "thread"
}
)
mock_request.return_value.status_code = 200
request = RequestFactory().post("dummy_url", post_params or {})
request.user = user or self.student
request.view_name = view_name
return getattr(views, view_name)(
request,
course_id=self.course.id.to_deprecated_string(),
thread_id="dummy",
**(view_args or {})
)
def test_update(self, mock_request):
response = self.call_view(
"update_thread",
mock_request,
post_params={"body": "body", "title": "title"}
)
self._assert_json_response_contains_group_info(response)
def test_delete(self, mock_request):
response = self.call_view("delete_thread", mock_request)
self._assert_json_response_contains_group_info(response)
def test_vote(self, mock_request):
response = self.call_view(
"vote_for_thread",
mock_request,
view_args={"value": "up"}
)
self._assert_json_response_contains_group_info(response)
response = self.call_view("undo_vote_for_thread", mock_request)
self._assert_json_response_contains_group_info(response)
def test_flag(self, mock_request):
response = self.call_view("flag_abuse_for_thread", mock_request)
self._assert_json_response_contains_group_info(response)
response = self.call_view("un_flag_abuse_for_thread", mock_request)
self._assert_json_response_contains_group_info(response)
def test_pin(self, mock_request):
response = self.call_view(
"pin_thread",
mock_request,
user=self.moderator
)
self._assert_json_response_contains_group_info(response)
response = self.call_view(
"un_pin_thread",
mock_request,
user=self.moderator
)
self._assert_json_response_contains_group_info(response)
def test_openclose(self, mock_request):
response = self.call_view(
"openclose_thread",
mock_request,
user=self.moderator
)
self._assert_json_response_contains_group_info(
response,
lambda d: d['content']
)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@patch('lms.lib.comment_client.utils.requests.request')
......@@ -678,9 +779,9 @@ class ViewPermissionsTestCase(UrlResetMixin, ModuleStoreTestCase, MockRequestSet
def handle_request(*args, **kwargs):
url = args[1]
if "/threads/" in url:
return self._create_repsonse_mock(thread_data)
return self._create_response_mock(thread_data)
elif "/comments/" in url:
return self._create_repsonse_mock(comment_data)
return self._create_response_mock(comment_data)
else:
raise ArgumentError("Bad url to mock request")
mock_request.side_effect = handle_request
......
lms/djangoapps/django_comment_client/base/views.py
......@@ -27,8 +27,7 @@ from django_comment_client.utils import (
get_ability,
JsonError,
JsonResponse,
safe_content,
add_thread_group_name,
prepare_content,
get_group_id_for_comments_service
)
from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
......@@ -60,7 +59,7 @@ def ajax_content_response(request, course_key, content):
user_info = cc.User.from_django_user(request.user).to_dict()
annotated_content_info = get_annotated_content_info(course_key, content, request.user, user_info)
return JsonResponse({
'content': safe_content(content, course_key),
'content': prepare_content(content, course_key),
'annotated_content_info': annotated_content_info,
})
......@@ -122,12 +121,11 @@ def create_thread(request, course_id, commentable_id):
user = cc.User.from_django_user(request.user)
user.follow(thread)
data = thread.to_dict()
add_thread_group_name(data, course_key)
add_courseware_context([data], course)
if request.is_ajax():
return ajax_content_response(request, course_key, data)
else:
return JsonResponse(safe_content(data, course_key))
return JsonResponse(prepare_content(data, course_key))
@require_POST
......@@ -146,10 +144,11 @@ def update_thread(request, course_id, thread_id):
thread.body = request.POST["body"]
thread.title = request.POST["title"]
thread.save()
if request.is_ajax():
return ajax_content_response(request, course_key, thread.to_dict())
else:
return JsonResponse(safe_content(thread.to_dict(), course_key))
return JsonResponse(prepare_content(thread.to_dict(), course_key))
def _create_comment(request, course_key, thread_id=None, parent_id=None):
......@@ -190,7 +189,7 @@ def _create_comment(request, course_key, thread_id=None, parent_id=None):
if request.is_ajax():
return ajax_content_response(request, course_key, comment.to_dict())
else:
return JsonResponse(safe_content(comment.to_dict(), course.id))
return JsonResponse(prepare_content(comment.to_dict(), course.id))
@require_POST
......@@ -218,7 +217,8 @@ def delete_thread(request, course_id, thread_id):
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
thread = cc.Thread.find(thread_id)
thread.delete()
return JsonResponse(safe_content(thread.to_dict(), course_key))
return JsonResponse(prepare_content(thread.to_dict(), course_key))
@require_POST
......@@ -238,7 +238,7 @@ def update_comment(request, course_id, comment_id):
if request.is_ajax():
return ajax_content_response(request, course_key, comment.to_dict())
else:
return JsonResponse(safe_content(comment.to_dict(), course_key))
return JsonResponse(prepare_content(comment.to_dict(), course_key))
@require_POST
......@@ -254,7 +254,7 @@ def endorse_comment(request, course_id, comment_id):
comment.endorsed = request.POST.get('endorsed', 'false').lower() == 'true'
comment.endorsement_user_id = request.user.id
comment.save()
return JsonResponse(safe_content(comment.to_dict(), course_key))
return JsonResponse(prepare_content(comment.to_dict(), course_key))
@require_POST
......@@ -269,10 +269,10 @@ def openclose_thread(request, course_id, thread_id):
thread = cc.Thread.find(thread_id)
thread.closed = request.POST.get('closed', 'false').lower() == 'true'
thread.save()
thread = thread.to_dict()
return JsonResponse({
'content': safe_content(thread, course_key),
'ability': get_ability(course_key, thread, request.user),
'content': prepare_content(thread.to_dict(), course_key),
'ability': get_ability(course_key, thread.to_dict(), request.user),
})
......@@ -301,7 +301,7 @@ def delete_comment(request, course_id, comment_id):
course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id)
comment = cc.Comment.find(comment_id)
comment.delete()
return JsonResponse(safe_content(comment.to_dict(), course_key))
return JsonResponse(prepare_content(comment.to_dict(), course_key))
@require_POST
......@@ -315,7 +315,7 @@ def vote_for_comment(request, course_id, comment_id, value):
user = cc.User.from_django_user(request.user)
comment = cc.Comment.find(comment_id)
user.vote(comment, value)
return JsonResponse(safe_content(comment.to_dict(), course_key))
return JsonResponse(prepare_content(comment.to_dict(), course_key))
@require_POST
......@@ -330,7 +330,7 @@ def undo_vote_for_comment(request, course_id, comment_id):
user = cc.User.from_django_user(request.user)
comment = cc.Comment.find(comment_id)
user.unvote(comment)
return JsonResponse(safe_content(comment.to_dict(), course_key))
return JsonResponse(prepare_content(comment.to_dict(), course_key))
@require_POST
......@@ -345,7 +345,8 @@ def vote_for_thread(request, course_id, thread_id, value):
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
user.vote(thread, value)
return JsonResponse(safe_content(thread.to_dict(), course_key))
return JsonResponse(prepare_content(thread.to_dict(), course_key))
@require_POST
......@@ -360,7 +361,8 @@ def flag_abuse_for_thread(request, course_id, thread_id):
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
thread.flagAbuse(user, thread)
return JsonResponse(safe_content(thread.to_dict(), course_key))
return JsonResponse(prepare_content(thread.to_dict(), course_key))
@require_POST
......@@ -377,7 +379,8 @@ def un_flag_abuse_for_thread(request, course_id, thread_id):
thread = cc.Thread.find(thread_id)
remove_all = cached_has_permission(request.user, 'openclose_thread', course_key) or has_access(request.user, 'staff', course)
thread.unFlagAbuse(user, thread, remove_all)
return JsonResponse(safe_content(thread.to_dict(), course_key))
return JsonResponse(prepare_content(thread.to_dict(), course_key))
@require_POST
......@@ -392,7 +395,7 @@ def flag_abuse_for_comment(request, course_id, comment_id):
user = cc.User.from_django_user(request.user)
comment = cc.Comment.find(comment_id)
comment.flagAbuse(user, comment)
return JsonResponse(safe_content(comment.to_dict(), course_key))
return JsonResponse(prepare_content(comment.to_dict(), course_key))
@require_POST
......@@ -409,7 +412,7 @@ def un_flag_abuse_for_comment(request, course_id, comment_id):
remove_all = cached_has_permission(request.user, 'openclose_thread', course_key) or has_access(request.user, 'staff', course)
comment = cc.Comment.find(comment_id)
comment.unFlagAbuse(user, comment, remove_all)
return JsonResponse(safe_content(comment.to_dict(), course_key))
return JsonResponse(prepare_content(comment.to_dict(), course_key))
@require_POST
......@@ -424,7 +427,8 @@ def undo_vote_for_thread(request, course_id, thread_id):
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
user.unvote(thread)
return JsonResponse(safe_content(thread.to_dict(), course_key))
return JsonResponse(prepare_content(thread.to_dict(), course_key))
@require_POST
......@@ -439,7 +443,8 @@ def pin_thread(request, course_id, thread_id):
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
thread.pin(user, thread_id)
return JsonResponse(safe_content(thread.to_dict(), course_key))
return JsonResponse(prepare_content(thread.to_dict(), course_key))
@require_POST
......@@ -454,7 +459,8 @@ def un_pin_thread(request, course_id, thread_id):
user = cc.User.from_django_user(request.user)
thread = cc.Thread.find(thread_id)
thread.un_pin(user, thread_id)
return JsonResponse(safe_content(thread.to_dict(), course_key))
return JsonResponse(prepare_content(thread.to_dict(), course_key))
@require_POST
......
lms/djangoapps/django_comment_client/forum/tests.py
......@@ -122,7 +122,7 @@ def make_mock_request_impl(text, thread_id="dummy_thread_id", group_id=None):
def mock_request_impl(*args, **kwargs):
url = args[1]
data = None
if url.endswith("threads"):
if url.endswith("threads") or url.endswith("user_profile"):
data = {
"collection": [make_mock_thread_data(text, thread_id, False, group_id=group_id)]
}
......@@ -406,15 +406,19 @@ class SingleThreadAccessTestCase(CohortedContentTestCase):
class SingleThreadGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
cs_endpoint = "/threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True, is_ajax=False):
mock_request.side_effect = make_mock_request_impl("dummy context", group_id=self.student_cohort.id)
request_data = {}
if pass_group_id:
request_data["group_id"] = group_id
headers = {}
if is_ajax:
headers['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest"
request = RequestFactory().get(
"dummy_url",
data=request_data
data=request_data,
**headers
)
request.user = user
mako_middleware_process_request(request)
......@@ -425,6 +429,28 @@ class SingleThreadGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdT
"dummy_thread_id"
)
def test_group_info_in_html_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
self.student_cohort.id,
is_ajax=False
)
self._assert_html_response_contains_group_info(response)
def test_group_info_in_ajax_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
self.student_cohort.id,
is_ajax=True
)
self._assert_json_response_contains_group_info(
response, lambda d: d['content']
)
@patch('lms.lib.comment_client.utils.requests.request')
class InlineDiscussionGroupIdTestCase(
......@@ -435,7 +461,17 @@ class InlineDiscussionGroupIdTestCase(
cs_endpoint = "/threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
mock_request.side_effect = make_mock_request_impl("dummy content")
kwargs = {}
if group_id:
# avoid causing a server error when the LMS chokes attempting
# to find a group name for the group_id, when we're testing with
# an invalid one.
try:
CourseUserGroup.objects.get(id=group_id)
kwargs['group_id'] = group_id
except CourseUserGroup.DoesNotExist:
pass
mock_request.side_effect = make_mock_request_impl("dummy content", **kwargs)
request_data = {}
if pass_group_id:
......@@ -451,20 +487,38 @@ class InlineDiscussionGroupIdTestCase(
commentable_id
)
def test_group_info_in_ajax_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
self.student_cohort.id
)
self._assert_json_response_contains_group_info(
response, lambda d: d['discussion_data'][0]
)
@patch('lms.lib.comment_client.utils.requests.request')
class ForumFormDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
cs_endpoint = "/threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
mock_request.side_effect = make_mock_request_impl("dummy content")
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True, is_ajax=False):
kwargs = {}
if group_id:
kwargs['group_id'] = group_id
mock_request.side_effect = make_mock_request_impl("dummy content", **kwargs)
request_data = {}
if pass_group_id:
request_data["group_id"] = group_id
headers = {}
if is_ajax:
headers['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest"
request = RequestFactory().get(
"dummy_url",
data=request_data
data=request_data,
**headers
)
request.user = user
mako_middleware_process_request(request)
......@@ -473,20 +527,47 @@ class ForumFormDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicG
self.course.id.to_deprecated_string()
)
def test_group_info_in_html_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
self.student_cohort.id
)
self._assert_html_response_contains_group_info(response)
def test_group_info_in_ajax_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
self.student_cohort.id,
is_ajax=True
)
self._assert_json_response_contains_group_info(
response, lambda d: d['discussion_data'][0]
)
@patch('lms.lib.comment_client.utils.requests.request')
class UserProfileDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
cs_endpoint = "/active_threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
mock_request.side_effect = make_mock_request_impl("dummy content")
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True, is_ajax=False):
kwargs = {}
if group_id:
kwargs['group_id'] = group_id
mock_request.side_effect = make_mock_request_impl("dummy content", **kwargs)
request_data = {}
if pass_group_id:
request_data["group_id"] = group_id
headers = {}
if is_ajax:
headers['HTTP_X_REQUESTED_WITH'] = "XMLHttpRequest"
request = RequestFactory().get(
"dummy_url",
data=request_data
data=request_data,
**headers
)
request.user = user
mako_middleware_process_request(request)
......@@ -496,13 +577,38 @@ class UserProfileDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopi
user.id
)
def test_group_info_in_html_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
self.student_cohort.id,
is_ajax=False
)
self._assert_html_response_contains_group_info(response)
def test_group_info_in_ajax_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
self.student_cohort.id,
is_ajax=True
)
self._assert_json_response_contains_group_info(
response, lambda d: d['discussion_data'][0]
)
@patch('lms.lib.comment_client.utils.requests.request')
class FollowedThreadsDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
cs_endpoint = "/subscribed_threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
mock_request.side_effect = make_mock_request_impl("dummy content")
kwargs = {}
if group_id:
kwargs['group_id'] = group_id
mock_request.side_effect = make_mock_request_impl("dummy content", **kwargs)
request_data = {}
if pass_group_id:
......@@ -519,6 +625,17 @@ class FollowedThreadsDiscussionGroupIdTestCase(CohortedContentTestCase, Cohorted
user.id
)
def test_group_info_in_ajax_response(self, mock_request):
response = self.call_view(
mock_request,
"cohorted_topic",
self.student,
self.student_cohort.id
)
self._assert_json_response_contains_group_info(
response, lambda d: d['discussion_data'][0]
)
@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE)
@patch('requests.request')
......
lms/djangoapps/django_comment_client/tests/group_id.py
import json
import re
from course_groups.models import CourseUserGroup
class GroupIdAssertionMixin(object):
def _data_or_params_cs_request(self, mock_request):
"""
......@@ -17,6 +22,31 @@ class GroupIdAssertionMixin(object):
self.assertTrue(mock_request.called)
self.assertNotIn("group_id", self._data_or_params_cs_request(mock_request))
def _assert_html_response_contains_group_info(self, response):
group_info = {"group_id": None, "group_name": None}
match = re.search(r'"group_id": ([\d]*)', response.content)
if match and match.group(1) != '':
group_info["group_id"] = int(match.group(1))
match = re.search(r'"group_name": "([^&]*)"', response.content)
if match:
group_info["group_name"] = match.group(1)
self._assert_thread_contains_group_info(group_info)
def _assert_json_response_contains_group_info(self, response, extract_thread=None):
"""
:param extract_thread: a function which accepts a dictionary (complete
json response payload) and returns another dictionary (first
occurrence of a thread model within that payload). if None is
passed, the identity function is assumed.
"""
payload = json.loads(response.content)
thread = extract_thread(payload) if extract_thread else payload
self._assert_thread_contains_group_info(thread)
def _assert_thread_contains_group_info(self, thread):
self.assertEqual(thread['group_id'], self.student_cohort.id)
self.assertEqual(thread['group_name'], self.student_cohort.name)
class CohortedTopicGroupIdTestMixin(GroupIdAssertionMixin):
"""
......
lms/djangoapps/django_comment_client/utils.py
......@@ -366,7 +366,16 @@ def add_courseware_context(content_list, course):
content.update({"courseware_url": url, "courseware_title": title})
def safe_content(content, course_id, is_staff=False):
def prepare_content(content, course_key, is_staff=False):
"""
This function is used to pre-process thread and comment models in various
ways before adding them to the HTTP response. This includes fixing empty
attribute fields, enforcing author anonymity, and enriching metadata around
group ownership and response endorsement.
@TODO: not all response pre-processing steps are currently integrated into
this function.
"""
fields = [
'id', 'title', 'body', 'course_id', 'anonymous', 'anonymous_to_peers',
'endorsed', 'parent_id', 'thread_id', 'votes', 'closed', 'created_at',
......@@ -407,20 +416,16 @@ def safe_content(content, course_id, is_staff=False):
for child_content_key in ["children", "endorsed_responses", "non_endorsed_responses"]:
if child_content_key in content:
safe_children = [
safe_content(child, course_id, is_staff) for child in content[child_content_key]
children = [
prepare_content(child, course_key, is_staff) for child in content[child_content_key]
]
content[child_content_key] = safe_children
return content
content[child_content_key] = children
# Augment the specified thread info to include the group name if a group id is present.
if content.get('group_id') is not None:
content['group_name'] = get_cohort_by_id(course_key, content.get('group_id')).name
def add_thread_group_name(thread_info, course_key):
"""
Augment the specified thread info to include the group name if a group id is present.
"""
if thread_info.get('group_id') is not None:
thread_info['group_name'] = get_cohort_by_id(course_key, thread_info.get('group_id')).name
return content
def get_group_id_for_comments_service(request, course_key, commentable_id=None):
......
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