Merge pull request #12132 from edx/jia/MA-2247

MA-2247: CourseBlockAPI - return discussion student view data

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}

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})

lms/djangoapps/courseware/tests/test_discussion_module.py

 # -*- 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})

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": "<p>dummy</p>",
+            "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": "<p>Test body</p>",
-            "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')