From 05b2e65e0f0ec29a36fd04870e492c0657e83c46 Mon Sep 17 00:00:00 2001 From: wajeeha-khalid Date: Tue, 12 Apr 2016 17:48:57 +0500 Subject: [PATCH] MA-2247 return student view data for discussion module --- .../lib/xmodule/xmodule/discussion_module.py | 6 ++ .../course_api/blocks/tests/test_views.py | 11 ++- .../tests/test_discussion_module.py | 44 ++++++++- .../discussion_api/tests/test_views.py | 98 ++++++++++++------- 4 files changed, 121 insertions(+), 38 deletions(-) diff --git a/common/lib/xmodule/xmodule/discussion_module.py b/common/lib/xmodule/xmodule/discussion_module.py index defe10edd1..3d2a47941f 100644 --- a/common/lib/xmodule/xmodule/discussion_module.py +++ b/common/lib/xmodule/xmodule/discussion_module.py @@ -124,3 +124,9 @@ class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawD # We may choose to enable sort_keys in the future, but while Kevin is investigating.... non_editable_fields.extend([DiscussionDescriptor.discussion_id, DiscussionDescriptor.sort_key]) return non_editable_fields + + def student_view_data(self): + """ + Returns a JSON representation of the student_view of this XModule. + """ + return {'topic_id': self.discussion_id} diff --git a/lms/djangoapps/course_api/blocks/tests/test_views.py b/lms/djangoapps/course_api/blocks/tests/test_views.py index 700077baef..3480437efb 100644 --- a/lms/djangoapps/course_api/blocks/tests/test_views.py +++ b/lms/djangoapps/course_api/blocks/tests/test_views.py @@ -22,6 +22,7 @@ class TestBlocksView(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): Test class for BlocksView """ requested_fields = ['graded', 'format', 'student_view_multi_device', 'children', 'not_a_field'] + BLOCK_TYPES_WITH_STUDENT_VIEW_DATA = ['video', 'discussion'] @classmethod def setUpClass(cls): @@ -203,10 +204,16 @@ class TestBlocksView(EnableTransformerRegistryMixin, SharedModuleStoreTestCase): ) def test_student_view_data_param(self): - response = self.verify_response(params={'student_view_data': ['video', 'chapter']}) + response = self.verify_response(params={ + 'student_view_data': self.BLOCK_TYPES_WITH_STUDENT_VIEW_DATA + ['chapter'] + }) self.verify_response_block_dict(response) for block_data in response.data['blocks'].itervalues(): - self.assert_in_iff('student_view_data', block_data, block_data['type'] == 'video') + self.assert_in_iff( + 'student_view_data', + block_data, + block_data['type'] in self.BLOCK_TYPES_WITH_STUDENT_VIEW_DATA + ) def test_navigation_param(self): response = self.verify_response(params={'nav_depth': 10}) diff --git a/lms/djangoapps/courseware/tests/test_discussion_module.py b/lms/djangoapps/courseware/tests/test_discussion_module.py index d673f76667..ce9c7c38db 100644 --- a/lms/djangoapps/courseware/tests/test_discussion_module.py +++ b/lms/djangoapps/courseware/tests/test_discussion_module.py @@ -1,17 +1,22 @@ # -*- coding: utf-8 -*- """Test for Discussion Xmodule functional logic.""" import ddt +from django.core.urlresolvers import reverse from mock import Mock from . import BaseTestXmodule +from course_api.blocks.tests.helpers import deserialize_usage_key +from course_blocks.tests.helpers import EnableTransformerRegistryMixin from courseware.module_render import get_module_for_descriptor_internal +from student.tests.factories import UserFactory, CourseEnrollmentFactory from xmodule.discussion_module import DiscussionModule from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore -from student.tests.factories import UserFactory +from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase +from xmodule.modulestore.tests.factories import ToyCourseFactory, ItemFactory @ddt.ddt -class DiscussionModuleTest(BaseTestXmodule): +class DiscussionModuleTest(BaseTestXmodule, EnableTransformerRegistryMixin, SharedModuleStoreTestCase): """Logic tests for Discussion Xmodule.""" CATEGORY = "discussion" @@ -90,3 +95,38 @@ class DiscussionModuleTest(BaseTestXmodule): block = block.get_parent() return block + + def test_discussion_student_view_data(self): + """ + Tests that course block api returns student_view_data for discussion module + """ + course_key = ToyCourseFactory.create().id + course_usage_key = self.store.make_course_usage_key(course_key) + user = UserFactory.create() + self.client.login(username=user.username, password='test') + CourseEnrollmentFactory.create(user=user, course_id=course_key) + discussion_id = "test_discussion_module_id" + ItemFactory.create( + parent_location=course_usage_key, + category='discussion', + discussion_id=discussion_id, + discussion_category='Category discussion', + discussion_target='Target Discussion', + ) + + url = reverse('blocks_in_block_tree', kwargs={'usage_key_string': unicode(course_usage_key)}) + query_params = { + 'depth': 'all', + 'username': user.username, + 'block_types_filter': 'discussion', + 'student_view_data': 'discussion' + } + response = self.client.get(url, query_params) + self.assertEquals(response.status_code, 200) + self.assertEquals(response.data['root'], unicode(course_usage_key)) # pylint: disable=no-member + for block_key_string, block_data in response.data['blocks'].iteritems(): # pylint: disable=no-member + block_key = deserialize_usage_key(block_key_string, course_key) + self.assertEquals(block_data['id'], block_key_string) + self.assertEquals(block_data['type'], block_key.block_type) + self.assertEquals(block_data['display_name'], self.store.get_item(block_key).display_name or '') + self.assertEqual(block_data['student_view_data'], {"topic_id": discussion_id}) diff --git a/lms/djangoapps/discussion_api/tests/test_views.py b/lms/djangoapps/discussion_api/tests/test_views.py index c59ac80e8f..9b87c91fb8 100644 --- a/lms/djangoapps/discussion_api/tests/test_views.py +++ b/lms/djangoapps/discussion_api/tests/test_views.py @@ -237,6 +237,43 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): self.author = UserFactory.create() self.url = reverse("thread-list") + def make_expected_thread(self, overrides=None): + """ + Create a sample expected thread for response + """ + thread = { + "id": "test_thread", + "course_id": unicode(self.course.id), + "topic_id": "test_topic", + "group_id": None, + "group_name": None, + "author": "dummy", + "author_label": None, + "created_at": "1970-01-01T00:00:00Z", + "updated_at": "1970-01-01T00:00:00Z", + "type": "discussion", + "title": "dummy", + "raw_body": "dummy", + "rendered_body": "

dummy

", + "pinned": False, + "closed": False, + "following": False, + "abuse_flagged": False, + "voted": False, + "vote_count": 0, + "comment_count": 1, + "unread_comment_count": 1, + "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", "read", "voted"], + "read": False, + "has_endorsed": False, + "response_count": 0, + } + thread.update(overrides or {}) + return thread + def test_course_id_missing(self): response = self.client.get(self.url) self.assert_response_correct( @@ -255,59 +292,32 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): def test_basic(self): self.register_get_user_response(self.user, upvoted_ids=["test_thread"]) - source_threads = [{ - "type": "thread", + source_threads = [make_minimal_cs_thread({ "id": "test_thread", "course_id": unicode(self.course.id), "commentable_id": "test_topic", - "group_id": None, "user_id": str(self.author.id), "username": self.author.username, - "anonymous": False, - "anonymous_to_peers": False, "created_at": "2015-04-28T00:00:00Z", "updated_at": "2015-04-28T11:11:11Z", - "thread_type": "discussion", "title": "Test Title", "body": "Test body", - "pinned": False, - "closed": False, - "abuse_flaggers": [], "votes": {"up_count": 4}, "comments_count": 5, "unread_comments_count": 3, - "read": False, - "endorsed": False - }] - expected_threads = [{ - "id": "test_thread", - "course_id": unicode(self.course.id), - "topic_id": "test_topic", - "group_id": None, - "group_name": None, - "author": self.author.username, - "author_label": None, + })] + expected_threads = [self.make_expected_thread({ "created_at": "2015-04-28T00:00:00Z", "updated_at": "2015-04-28T11:11:11Z", - "type": "discussion", - "title": "Test Title", "raw_body": "Test body", "rendered_body": "

Test body

", - "pinned": False, - "closed": False, - "following": False, - "abuse_flagged": False, - "voted": True, + "title": "Test Title", "vote_count": 4, "comment_count": 6, "unread_comment_count": 4, - "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", "read", "voted"], - "read": False, - "has_endorsed": False, - }] + "voted": True, + "author": self.author.username + })] self.register_get_threads_response(source_threads, page=1, num_pages=2) response = self.client.get(self.url, {"course_id": unicode(self.course.id), "following": ""}) expected_response = make_paginated_api_response( @@ -521,6 +531,26 @@ class ThreadViewSetListTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase): "sort_order": [query], }) + def test_mutually_exclusive(self): + """ + Tests GET thread_list api does not allow filtering on mutually exclusive parameters + """ + self.register_get_user_response(self.user) + self.register_get_threads_search_response([], None, num_pages=0) + response = self.client.get(self.url, { + "course_id": unicode(self.course.id), + "text_search": "test search string", + "topic_id": "topic1, topic2", + }) + self.assert_response_correct( + response, + 400, + { + "developer_message": "The following query parameters are mutually exclusive: topic_id, " + "text_search, following" + } + ) + @httpretty.activate @disable_signal(api, 'thread_created')