Merge branch 'release'

cms/djangoapps/contentstore/views/tests/test_videos.py

@@ -97,6 +97,20 @@ class VideoUploadTestMixin(object):
                 ]
             },
         ]
+        # Ensure every status string is tested
+        self.previous_uploads += [
+            {
+                "edx_video_id": "status_test_{}".format(status),
+                "client_video_id": "status_test.mp4",
+                "duration": 3.14,
+                "status": status,
+                "encoded_videos": [],
+            }
+            for status in (
+                StatusDisplayStrings._STATUS_MAP.keys() +  # pylint:disable=protected-access
+                ["non_existent_status"]
+            )
+        ]
         for profile in self.profiles:
             create_profile(profile)
         for video in self.previous_uploads:

cms/djangoapps/contentstore/views/videos.py

@@ -48,10 +48,10 @@ class StatusDisplayStrings(object):
     # Translators: This is the status for a video that the servers have successfully processed
     _COMPLETE = ugettext_noop("Complete")
     # Translators: This is the status for a video that the servers have failed to process
-    _FAILED = ugettext_noop("Failed"),
+    _FAILED = ugettext_noop("Failed")
     # Translators: This is the status for a video for which an invalid
     # processing token was provided in the course settings
-    _INVALID_TOKEN = ugettext_noop("Invalid Token"),
+    _INVALID_TOKEN = ugettext_noop("Invalid Token")
     # Translators: This is the status for a video that is in an unknown state
     _UNKNOWN = ugettext_noop("Unknown")
 

common/test/acceptance/pages/lms/discussion.py

@@ -311,15 +311,13 @@ class DiscussionSortPreferencePage(CoursePage):
 
 
 class DiscussionTabSingleThreadPage(CoursePage):
-    def __init__(self, browser, course_id, discussion_id, thread_id):
+    def __init__(self, browser, course_id, thread_id):
         super(DiscussionTabSingleThreadPage, self).__init__(browser, course_id)
         self.thread_page = DiscussionThreadPage(
             browser,
             "body.discussion .discussion-article[data-id='{thread_id}']".format(thread_id=thread_id)
         )
-        self.url_path = "discussion/forum/{discussion_id}/threads/{thread_id}".format(
-            discussion_id=discussion_id, thread_id=thread_id
-        )
+        self.url_path = "discussion/forum/dummy/threads/" + thread_id
 
     def is_browser_on_page(self):
         return self.thread_page.is_browser_on_page()

common/test/acceptance/tests/discussion/helpers.py

@@ -5,15 +5,12 @@ Helper functions and classes for discussion tests.
 from uuid import uuid4
 import json
 
-from ...fixtures import LMS_BASE_URL
-from ...fixtures.course import CourseFixture
 from ...fixtures.discussion import (
     SingleThreadViewFixture,
     Thread,
     Response,
 )
-from ...pages.lms.discussion import DiscussionTabSingleThreadPage
-from ...tests.helpers import UniqueCourseTest
+from ...fixtures import LMS_BASE_URL
 
 
 class BaseDiscussionMixin(object):
@@ -86,22 +83,3 @@ class CohortTestMixin(object):
         data = {"users": username}
         response = course_fixture.session.post(url, data=data, headers=course_fixture.headers)
         self.assertTrue(response.ok, "Failed to add user to cohort")
-
-
-class BaseDiscussionTestCase(UniqueCourseTest):
-    def setUp(self):
-        super(BaseDiscussionTestCase, self).setUp()
-
-        self.discussion_id = "test_discussion_{}".format(uuid4().hex)
-        self.course_fixture = CourseFixture(**self.course_info)
-        self.course_fixture.add_advanced_settings(
-            {'discussion_topics': {'value': {'Test Discussion Topic': {'id': self.discussion_id}}}}
-        )
-        self.course_fixture.install()
-
-    def create_single_thread_page(self, thread_id):
-        """
-        Sets up a `DiscussionTabSingleThreadPage` for a given
-        `thread_id`.
-        """
-        return DiscussionTabSingleThreadPage(self.browser, self.course_id, self.discussion_id, thread_id)

common/test/acceptance/tests/discussion/test_cohorts.py

@@ -3,7 +3,7 @@ Tests related to the cohorting feature.
 """
 from uuid import uuid4
 
-from .helpers import BaseDiscussionMixin, BaseDiscussionTestCase
+from .helpers import BaseDiscussionMixin
 from .helpers import CohortTestMixin
 from ..helpers import UniqueCourseTest
 from ...pages.lms.auto_auth import AutoAuthPage
@@ -57,17 +57,20 @@ class CohortedDiscussionTestMixin(BaseDiscussionMixin, CohortTestMixin):
         self.assertEquals(self.thread_page.get_group_visibility_label(), "This post is visible to everyone.")
 
 
-class DiscussionTabSingleThreadTest(BaseDiscussionTestCase):
+class DiscussionTabSingleThreadTest(UniqueCourseTest):
     """
     Tests for the discussion page displaying a single thread.
     """
     def setUp(self):
         super(DiscussionTabSingleThreadTest, self).setUp()
+        self.discussion_id = "test_discussion_{}".format(uuid4().hex)
+        # Create a course to register for
+        self.course_fixture = CourseFixture(**self.course_info).install()
         self.setup_cohorts()
         AutoAuthPage(self.browser, course_id=self.course_id).visit()
 
     def setup_thread_page(self, thread_id):
-        self.thread_page = DiscussionTabSingleThreadPage(self.browser, self.course_id, self.discussion_id, thread_id)  # pylint: disable=attribute-defined-outside-init
+        self.thread_page = DiscussionTabSingleThreadPage(self.browser, self.course_id, thread_id)  # pylint: disable=attribute-defined-outside-init
         self.thread_page.visit()
 
     # pylint: disable=unused-argument

common/test/acceptance/tests/discussion/test_discussion.py

@@ -7,7 +7,6 @@ from pytz import UTC
 from uuid import uuid4
 from nose.plugins.attrib import attr
 
-from .helpers import BaseDiscussionTestCase
 from ..helpers import UniqueCourseTest
 from ...pages.lms.auto_auth import AutoAuthPage
 from ...pages.lms.courseware import CoursewarePage
@@ -140,17 +139,22 @@ class DiscussionHomePageTest(UniqueCourseTest):
 
 
 @attr('shard_1')
-class DiscussionTabSingleThreadTest(BaseDiscussionTestCase, DiscussionResponsePaginationTestMixin):
+class DiscussionTabSingleThreadTest(UniqueCourseTest, DiscussionResponsePaginationTestMixin):
     """
     Tests for the discussion page displaying a single thread
     """
 
     def setUp(self):
         super(DiscussionTabSingleThreadTest, self).setUp()
+        self.discussion_id = "test_discussion_{}".format(uuid4().hex)
+
+        # Create a course to register for
+        CourseFixture(**self.course_info).install()
+
         AutoAuthPage(self.browser, course_id=self.course_id).visit()
 
     def setup_thread_page(self, thread_id):
-        self.thread_page = self.create_single_thread_page(thread_id)  # pylint: disable=attribute-defined-outside-init
+        self.thread_page = DiscussionTabSingleThreadPage(self.browser, self.course_id, thread_id)  # pylint: disable=attribute-defined-outside-init
         self.thread_page.visit()
 
     def test_marked_answer_comments(self):
@@ -176,7 +180,7 @@ class DiscussionTabSingleThreadTest(BaseDiscussionTestCase, DiscussionResponsePa
 
 
 @attr('shard_1')
-class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase):
+class DiscussionOpenClosedThreadTest(UniqueCourseTest):
     """
     Tests for checking the display of attributes on open and closed threads
     """
@@ -184,6 +188,8 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase):
     def setUp(self):
         super(DiscussionOpenClosedThreadTest, self).setUp()
 
+        # Create a course to register for
+        CourseFixture(**self.course_info).install()
         self.thread_id = "test_thread_{}".format(uuid4().hex)
 
     def setup_user(self, roles=[]):
@@ -191,7 +197,6 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase):
         self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id()
 
     def setup_view(self, **thread_kwargs):
-        thread_kwargs.update({'commentable_id': self.discussion_id})
         view = SingleThreadViewFixture(
             Thread(id=self.thread_id, **thread_kwargs)
         )
@@ -204,7 +209,7 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase):
             self.setup_view(closed=True)
         else:
             self.setup_view()
-        page = self.create_single_thread_page(self.thread_id)
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, self.thread_id)
         page.visit()
         page.close_open_thread()
         return page
@@ -225,16 +230,23 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase):
 
 
 @attr('shard_1')
-class DiscussionCommentDeletionTest(BaseDiscussionTestCase):
+class DiscussionCommentDeletionTest(UniqueCourseTest):
     """
     Tests for deleting comments displayed beneath responses in the single thread view.
     """
+
+    def setUp(self):
+        super(DiscussionCommentDeletionTest, self).setUp()
+
+        # Create a course to register for
+        CourseFixture(**self.course_info).install()
+
     def setup_user(self, roles=[]):
         roles_str = ','.join(roles)
         self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id()
 
     def setup_view(self):
-        view = SingleThreadViewFixture(Thread(id="comment_deletion_test_thread", commentable_id=self.discussion_id))
+        view = SingleThreadViewFixture(Thread(id="comment_deletion_test_thread"))
         view.addResponse(
             Response(id="response1"),
             [Comment(id="comment_other_author", user_id="other"), Comment(id="comment_self_author", user_id=self.user_id)])
@@ -243,7 +255,7 @@ class DiscussionCommentDeletionTest(BaseDiscussionTestCase):
     def test_comment_deletion_as_student(self):
         self.setup_user()
         self.setup_view()
-        page = self.create_single_thread_page("comment_deletion_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_deletion_test_thread")
         page.visit()
         self.assertTrue(page.is_comment_deletable("comment_self_author"))
         self.assertTrue(page.is_comment_visible("comment_other_author"))
@@ -253,7 +265,7 @@ class DiscussionCommentDeletionTest(BaseDiscussionTestCase):
     def test_comment_deletion_as_moderator(self):
         self.setup_user(roles=['Moderator'])
         self.setup_view()
-        page = self.create_single_thread_page("comment_deletion_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_deletion_test_thread")
         page.visit()
         self.assertTrue(page.is_comment_deletable("comment_self_author"))
         self.assertTrue(page.is_comment_deletable("comment_other_author"))
@@ -262,16 +274,23 @@ class DiscussionCommentDeletionTest(BaseDiscussionTestCase):
 
 
 @attr('shard_1')
-class DiscussionResponseEditTest(BaseDiscussionTestCase):
+class DiscussionResponseEditTest(UniqueCourseTest):
     """
     Tests for editing responses displayed beneath thread in the single thread view.
     """
+
+    def setUp(self):
+        super(DiscussionResponseEditTest, self).setUp()
+
+        # Create a course to register for
+        CourseFixture(**self.course_info).install()
+
     def setup_user(self, roles=[]):
         roles_str = ','.join(roles)
         self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id()
 
     def setup_view(self):
-        view = SingleThreadViewFixture(Thread(id="response_edit_test_thread", commentable_id=self.discussion_id))
+        view = SingleThreadViewFixture(Thread(id="response_edit_test_thread"))
         view.addResponse(
             Response(id="response_other_author", user_id="other", thread_id="response_edit_test_thread"),
         )
@@ -298,7 +317,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
         """
         self.setup_user()
         self.setup_view()
-        page = self.create_single_thread_page("response_edit_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "response_edit_test_thread")
         page.visit()
         self.assertTrue(page.is_response_visible("response_other_author"))
         self.assertFalse(page.is_response_editable("response_other_author"))
@@ -315,7 +334,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
         """
         self.setup_user(roles=["Moderator"])
         self.setup_view()
-        page = self.create_single_thread_page("response_edit_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "response_edit_test_thread")
         page.visit()
         self.edit_response(page, "response_self_author")
         self.edit_response(page, "response_other_author")
@@ -343,7 +362,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
         """
         self.setup_user(roles=["Moderator"])
         self.setup_view()
-        page = self.create_single_thread_page("response_edit_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "response_edit_test_thread")
         page.visit()
         self.edit_response(page, "response_self_author")
         self.edit_response(page, "response_other_author")
@@ -356,16 +375,23 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
 
 
 @attr('shard_1')
-class DiscussionCommentEditTest(BaseDiscussionTestCase):
+class DiscussionCommentEditTest(UniqueCourseTest):
     """
     Tests for editing comments displayed beneath responses in the single thread view.
     """
+
+    def setUp(self):
+        super(DiscussionCommentEditTest, self).setUp()
+
+        # Create a course to register for
+        CourseFixture(**self.course_info).install()
+
     def setup_user(self, roles=[]):
         roles_str = ','.join(roles)
         self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id()
 
     def setup_view(self):
-        view = SingleThreadViewFixture(Thread(id="comment_edit_test_thread", commentable_id=self.discussion_id))
+        view = SingleThreadViewFixture(Thread(id="comment_edit_test_thread"))
         view.addResponse(
             Response(id="response1"),
             [Comment(id="comment_other_author", user_id="other"), Comment(id="comment_self_author", user_id=self.user_id)])
@@ -380,7 +406,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase):
     def test_edit_comment_as_student(self):
         self.setup_user()
         self.setup_view()
-        page = self.create_single_thread_page("comment_edit_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread")
         page.visit()
         self.assertTrue(page.is_comment_editable("comment_self_author"))
         self.assertTrue(page.is_comment_visible("comment_other_author"))
@@ -390,7 +416,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase):
     def test_edit_comment_as_moderator(self):
         self.setup_user(roles=["Moderator"])
         self.setup_view()
-        page = self.create_single_thread_page("comment_edit_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread")
         page.visit()
         self.assertTrue(page.is_comment_editable("comment_self_author"))
         self.assertTrue(page.is_comment_editable("comment_other_author"))
@@ -400,7 +426,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase):
     def test_cancel_comment_edit(self):
         self.setup_user()
         self.setup_view()
-        page = self.create_single_thread_page("comment_edit_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread")
         page.visit()
         self.assertTrue(page.is_comment_editable("comment_self_author"))
         original_body = page.get_comment_body("comment_self_author")
@@ -412,7 +438,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase):
         """Only one editor should be visible at a time within a single response"""
         self.setup_user(roles=["Moderator"])
         self.setup_view()
-        page = self.create_single_thread_page("comment_edit_test_thread")
+        page = DiscussionTabSingleThreadPage(self.browser, self.course_id, "comment_edit_test_thread")
         page.visit()
         self.assertTrue(page.is_comment_editable("comment_self_author"))
         self.assertTrue(page.is_comment_editable("comment_other_author"))

lms/djangoapps/certificates/queue.py

@@ -416,7 +416,7 @@ class XQueueCertInterface(object):
                 error_reason=unicode(exc)
             )
 
-    def _send_to_xqueue(self, contents, key, task_identifier=None, callback_url_path='update_certificate'):
+    def _send_to_xqueue(self, contents, key, task_identifier=None, callback_url_path='/update_certificate'):
         """Create a new task on the XQueue.
 
         Arguments:

lms/djangoapps/certificates/tests/test_queue.py

@@ -2,12 +2,16 @@
 """Tests for the XQueue certificates interface. """
 from contextlib import contextmanager
 import json
-from mock import patch
+from mock import patch, Mock
 
 from django.test import TestCase
 from django.test.utils import override_settings
 
 from opaque_keys.edx.locator import CourseLocator
+from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
+from student.tests.factories import UserFactory, CourseEnrollmentFactory
+from xmodule.modulestore.tests.factories import CourseFactory
+
 
 # It is really unfortunate that we are using the XQueue client
 # code from the capa library.  In the future, we should move this
@@ -21,7 +25,36 @@ from certificates.models import ExampleCertificateSet, ExampleCertificate
 
 
 @override_settings(CERT_QUEUE='certificates')
-class XQueueCertInterfaceTest(TestCase):
+class XQueueCertInterfaceAddCertificateTest(ModuleStoreTestCase):
+    """Test the "add to queue" operation of the XQueue interface. """
+
+    def setUp(self):
+        super(XQueueCertInterfaceAddCertificateTest, self).setUp()
+        self.user = UserFactory.create()
+        self.course = CourseFactory.create()
+        self.enrollment = CourseEnrollmentFactory(
+            user=self.user,
+            course_id=self.course.id,
+            is_active=True,
+            mode="honor",
+        )
+        self.xqueue = XQueueCertInterface()
+
+    def test_add_cert_callback_url(self):
+        with patch('courseware.grades.grade', Mock(return_value={'grade': 'Pass', 'percent': 0.75})):
+            with patch.object(XQueueInterface, 'send_to_queue') as mock_send:
+                mock_send.return_value = (0, None)
+                self.xqueue.add_cert(self.user, self.course.id)
+
+        # Verify that the task was sent to the queue with the correct callback URL
+        self.assertTrue(mock_send.called)
+        __, kwargs = mock_send.call_args_list[0]
+        actual_header = json.loads(kwargs['header'])
+        self.assertIn('https://edx.org/update_certificate?key=', actual_header['lms_callback_url'])
+
+
+@override_settings(CERT_QUEUE='certificates')
+class XQueueCertInterfaceExampleCertificateTest(TestCase):
     """Tests for the XQueue interface for certificate generation. """
 
     COURSE_KEY = CourseLocator(org='test', course='test', run='test')
@@ -31,7 +64,7 @@ class XQueueCertInterfaceTest(TestCase):
     ERROR_MSG = 'Kaboom!'
 
     def setUp(self):
-        super(XQueueCertInterfaceTest, self).setUp()
+        super(XQueueCertInterfaceExampleCertificateTest, self).setUp()
         self.xqueue = XQueueCertInterface()
 
     def test_add_example_cert(self):

lms/djangoapps/django_comment_client/base/tests.py

@@ -14,7 +14,7 @@ from lms.lib.comment_client import Thread
 from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE
 from django_comment_client.base import views
 from django_comment_client.tests.group_id import CohortedTopicGroupIdTestMixin, NonCohortedTopicGroupIdTestMixin, GroupIdAssertionMixin
-from django_comment_client.tests.utils import CohortedTestCase
+from django_comment_client.tests.utils import CohortedContentTestCase
 from django_comment_client.tests.unicode import UnicodeTestMixin
 from django_comment_common.models import Role
 from django_comment_common.utils import seed_permissions_roles
@@ -42,7 +42,7 @@ class MockRequestSetupMixin(object):
 @patch('lms.lib.comment_client.utils.requests.request')
 class CreateThreadGroupIdTestCase(
         MockRequestSetupMixin,
-        CohortedTestCase,
+        CohortedContentTestCase,
         CohortedTopicGroupIdTestMixin,
         NonCohortedTopicGroupIdTestMixin
 ):
@@ -77,7 +77,7 @@ class CreateThreadGroupIdTestCase(
 @patch('lms.lib.comment_client.utils.requests.request')
 class ThreadActionGroupIdTestCase(
         MockRequestSetupMixin,
-        CohortedTestCase,
+        CohortedContentTestCase,
         GroupIdAssertionMixin
 ):
     def call_view(

lms/djangoapps/django_comment_client/base/views.py

@@ -74,7 +74,7 @@ def track_forum_event(request, event_name, course, obj, data, id_map=None):
     user = request.user
     data['id'] = obj.id
     if id_map is None:
-        id_map = get_discussion_id_map(course, user)
+        id_map = get_discussion_id_map(course)
 
     commentable_id = data['commentable_id']
     if commentable_id in id_map:
@@ -173,9 +173,9 @@ def create_thread(request, course_id, commentable_id):
 
     # Calls to id map are expensive, but we need this more than once.
     # Prefetch it.
-    id_map = get_discussion_id_map(course, request.user)
+    id_map = get_discussion_id_map(course)
 
-    add_courseware_context([data], course, request.user, id_map=id_map)
+    add_courseware_context([data], course, id_map=id_map)
 
     track_forum_event(request, 'edx.forum.thread.created',
                       course, thread, event_data, id_map=id_map)
@@ -208,7 +208,7 @@ def update_thread(request, course_id, thread_id):
         thread.thread_type = request.POST["thread_type"]
     if "commentable_id" in request.POST:
         course = get_course_with_access(request.user, 'load', course_key)
-        commentable_ids = get_discussion_categories_ids(course, request.user)
+        commentable_ids = get_discussion_categories_ids(course)
         if request.POST.get("commentable_id") in commentable_ids:
             thread.commentable_id = request.POST["commentable_id"]
         else:

lms/djangoapps/django_comment_client/forum/tests.py

@@ -2,7 +2,6 @@ import json
 import logging
 
 import ddt
-from django.core import cache
 from django.core.urlresolvers import reverse
 from django.http import Http404
 from django.test.client import Client, RequestFactory
@@ -15,7 +14,7 @@ from django_comment_client.tests.group_id import (
     NonCohortedTopicGroupIdTestMixin
 )
 from django_comment_client.tests.unicode import UnicodeTestMixin
-from django_comment_client.tests.utils import CohortedTestCase, ContentGroupTestCase
+from django_comment_client.tests.utils import CohortedContentTestCase
 from django_comment_client.utils import strip_none
 from student.tests.factories import UserFactory, CourseEnrollmentFactory
 from util.testing import UrlResetMixin
@@ -187,7 +186,7 @@ class SingleThreadTestCase(ModuleStoreTestCase):
     def setUp(self):
         super(SingleThreadTestCase, self).setUp(create_user=False)
 
-        self.course = CourseFactory.create(discussion_topics={'dummy discussion': {'id': 'dummy_discussion_id'}})
+        self.course = CourseFactory.create()
         self.student = UserFactory.create()
         CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
 
@@ -302,24 +301,18 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
     MODULESTORE = TEST_DATA_MONGO_MODULESTORE
 
     @ddt.data(
-        # old mongo with cache: number of responses plus 17.  TODO: O(n)!
-        (ModuleStoreEnum.Type.mongo, 1, 23, 18),
-        (ModuleStoreEnum.Type.mongo, 50, 366, 67),
+        # old mongo: number of responses plus 16.  TODO: O(n)!
+        (ModuleStoreEnum.Type.mongo, 1, 17),
+        (ModuleStoreEnum.Type.mongo, 50, 66),
         # split mongo: 3 queries, regardless of thread response size.
-        (ModuleStoreEnum.Type.split, 1, 3, 3),
-        (ModuleStoreEnum.Type.split, 50, 3, 3),
+        (ModuleStoreEnum.Type.split, 1, 3),
+        (ModuleStoreEnum.Type.split, 50, 3),
     )
     @ddt.unpack
-    def test_number_of_mongo_queries(
-            self,
-            default_store,
-            num_thread_responses,
-            num_uncached_mongo_calls,
-            num_cached_mongo_calls,
-            mock_request
-    ):
+    def test_number_of_mongo_queries(self, default_store, num_thread_responses, num_mongo_calls, mock_request):
+
         with modulestore().default_store(default_store):
-            course = CourseFactory.create(discussion_topics={'dummy discussion': {'id': 'dummy_discussion_id'}})
+            course = CourseFactory.create()
 
         student = UserFactory.create()
         CourseEnrollmentFactory.create(user=student, course_id=course.id)
@@ -335,11 +328,7 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
             HTTP_X_REQUESTED_WITH="XMLHttpRequest"
         )
         request.user = student
-
-        def call_single_thread():
-            """
-            Call single_thread and assert that it returns what we expect.
-            """
+        with check_mongo_calls(num_mongo_calls):
             response = views.single_thread(
                 request,
                 course.id.to_deprecated_string(),
@@ -349,30 +338,9 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
             self.assertEquals(response.status_code, 200)
             self.assertEquals(len(json.loads(response.content)["content"]["children"]), num_thread_responses)
 
-        # TODO: update this once django cache is disabled in tests
-        # Test with and without cache, clearing before and after use.
-        single_thread_local_cache = cache.get_cache(
-            backend='default',
-            LOCATION='single_thread_local_cache'
-        )
-        single_thread_dummy_cache = cache.get_cache(
-            backend='django.core.cache.backends.dummy.DummyCache',
-            LOCATION='single_thread_local_cache'
-        )
-        cached_calls = {
-            single_thread_dummy_cache: num_uncached_mongo_calls,
-            single_thread_local_cache: num_cached_mongo_calls
-        }
-        for single_thread_cache, expected_calls in cached_calls.items():
-            single_thread_cache.clear()
-            with patch("django_comment_client.permissions.CACHE", single_thread_cache):
-                with check_mongo_calls(expected_calls):
-                    call_single_thread()
-            single_thread_cache.clear()
-
 
 @patch('requests.request')
-class SingleCohortedThreadTestCase(CohortedTestCase):
+class SingleCohortedThreadTestCase(CohortedContentTestCase):
     def _create_mock_cohorted_thread(self, mock_request):
         self.mock_text = "dummy content"
         self.mock_thread_id = "test_thread_id"
@@ -392,7 +360,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase):
         response = views.single_thread(
             request,
             self.course.id.to_deprecated_string(),
-            "cohorted_topic",
+            "dummy_discussion_id",
             self.mock_thread_id
         )
 
@@ -416,7 +384,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase):
         response = views.single_thread(
             request,
             self.course.id.to_deprecated_string(),
-            "cohorted_topic",
+            "dummy_discussion_id",
             self.mock_thread_id
         )
 
@@ -429,7 +397,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase):
 
 
 @patch('lms.lib.comment_client.utils.requests.request')
-class SingleThreadAccessTestCase(CohortedTestCase):
+class SingleThreadAccessTestCase(CohortedContentTestCase):
     def call_view(self, mock_request, commentable_id, user, group_id, thread_group_id=None, pass_group_id=True):
         thread_id = "test_thread_id"
         mock_request.side_effect = make_mock_request_impl("dummy context", thread_id, group_id=thread_group_id)
@@ -514,7 +482,7 @@ class SingleThreadAccessTestCase(CohortedTestCase):
 
 
 @patch('lms.lib.comment_client.utils.requests.request')
-class SingleThreadGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin):
+class SingleThreadGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
     cs_endpoint = "/threads"
 
     def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True, is_ajax=False):
@@ -536,7 +504,7 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixi
         return views.single_thread(
             request,
             self.course.id.to_deprecated_string(),
-            commentable_id,
+            "dummy_discussion_id",
             "dummy_thread_id"
         )
 
@@ -563,133 +531,9 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixi
         )
 
 
-@patch('requests.request')
-class SingleThreadContentGroupTestCase(ContentGroupTestCase):
-    def assert_can_access(self, user, discussion_id, thread_id, should_have_access):
-        """
-        Verify that a user has access to a thread within a given
-        discussion_id when should_have_access is True, otherwise
-        verify that the user does not have access to that thread.
-        """
-        request = RequestFactory().get("dummy_url")
-        request.user = user
-        mako_middleware_process_request(request)
-
-        def call_single_thread():
-            return views.single_thread(
-                request,
-                unicode(self.course.id),
-                discussion_id,
-                thread_id
-            )
-
-        if should_have_access:
-            self.assertEqual(call_single_thread().status_code, 200)
-        else:
-            with self.assertRaises(Http404):
-                call_single_thread()
-
-    def assert_searched_with_discussion_ids(self, mock_request, expected_commentable_ids):
-        """
-        Verify that the comments service was searched for threads with
-        the expected discussion ids (passed to the comments service as
-        'commentable_ids').
-        """
-        mock_request.assert_called_with(
-            'get',
-            StringEndsWithMatcher('threads'),
-            headers=ANY,
-            timeout=ANY,
-            data=None,
-            params=PartialDictMatcher({
-                'course_id': unicode(self.course.id),
-                'commentable_ids': ','.join(self.course.top_level_discussion_topic_ids + expected_commentable_ids)
-            })
-        )
-
-    def test_staff_user(self, mock_request):
-        """
-        Verify that the staff user can access threads in the alpha,
-        beta, and global discussion modules.
-        """
-        def assert_searched_correct_modules():
-            self.assert_searched_with_discussion_ids(
-                mock_request,
-                [self.beta_module.discussion_id, self.global_module.discussion_id, self.alpha_module.discussion_id]
-            )
-
-        thread_id = "test_thread_id"
-        mock_request.side_effect = make_mock_request_impl("dummy content", thread_id)
-
-        for discussion_module in [self.alpha_module, self.beta_module, self.global_module]:
-            self.assert_can_access(self.staff_user, discussion_module.discussion_id, thread_id, True)
-            assert_searched_correct_modules()
-
-    def test_alpha_user(self, mock_request):
-        """
-        Verify that the alpha user can access threads in the alpha and
-        global discussion modules.
-        """
-        def assert_searched_correct_modules():
-            self.assert_searched_with_discussion_ids(
-                mock_request,
-                [self.global_module.discussion_id, self.alpha_module.discussion_id]
-            )
-
-        thread_id = "test_thread_id"
-        mock_request.side_effect = make_mock_request_impl("dummy content", thread_id)
-
-        for discussion_module in [self.alpha_module, self.global_module]:
-            self.assert_can_access(self.alpha_user, discussion_module.discussion_id, thread_id, True)
-            assert_searched_correct_modules()
-
-        self.assert_can_access(self.alpha_user, self.beta_module.discussion_id, thread_id, False)
-
-    def test_beta_user(self, mock_request):
-        """
-        Verify that the beta user can access threads in the beta and
-        global discussion modules.
-        """
-        def assert_searched_correct_modules():
-            self.assert_searched_with_discussion_ids(
-                mock_request,
-                [self.beta_module.discussion_id, self.global_module.discussion_id]
-            )
-
-        thread_id = "test_thread_id"
-        mock_request.side_effect = make_mock_request_impl("dummy content", thread_id)
-
-        for discussion_module in [self.beta_module, self.global_module]:
-            self.assert_can_access(self.beta_user, discussion_module.discussion_id, thread_id, True)
-            assert_searched_correct_modules()
-
-        self.assert_can_access(self.beta_user, self.alpha_module.discussion_id, thread_id, False)
-
-    def test_non_cohorted_user(self, mock_request):
-        """
-        Verify that the non-cohorted user can access threads in just the
-        global discussion module.
-        """
-        def assert_searched_correct_modules():
-            self.assert_searched_with_discussion_ids(
-                mock_request,
-                [self.global_module.discussion_id]
-            )
-
-        thread_id = "test_thread_id"
-        mock_request.side_effect = make_mock_request_impl("dummy content", thread_id)
-
-        self.assert_can_access(self.non_cohorted_user, self.global_module.discussion_id, thread_id, True)
-        assert_searched_correct_modules()
-
-        self.assert_can_access(self.non_cohorted_user, self.alpha_module.discussion_id, thread_id, False)
-
-        self.assert_can_access(self.non_cohorted_user, self.beta_module.discussion_id, thread_id, False)
-
-
 @patch('lms.lib.comment_client.utils.requests.request')
 class InlineDiscussionGroupIdTestCase(
-        CohortedTestCase,
+        CohortedContentTestCase,
         CohortedTopicGroupIdTestMixin,
         NonCohortedTopicGroupIdTestMixin
 ):
@@ -735,7 +579,7 @@ class InlineDiscussionGroupIdTestCase(
 
 
 @patch('lms.lib.comment_client.utils.requests.request')
-class ForumFormDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin):
+class ForumFormDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
     cs_endpoint = "/threads"
 
     def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True, is_ajax=False):
@@ -785,7 +629,7 @@ class ForumFormDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdT
 
 
 @patch('lms.lib.comment_client.utils.requests.request')
-class UserProfileDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin):
+class UserProfileDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
     cs_endpoint = "/active_threads"
 
     def call_view_for_profiled_user(
@@ -951,7 +795,7 @@ class UserProfileDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupI
 
 
 @patch('lms.lib.comment_client.utils.requests.request')
-class FollowedThreadsDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin):
+class FollowedThreadsDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
     cs_endpoint = "/subscribed_threads"
 
     def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
@@ -1142,7 +986,7 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase):
 
         # Invoke UrlResetMixin
         super(CommentsServiceRequestHeadersTestCase, self).setUp(create_user=False)
-        self.course = CourseFactory.create(discussion_topics={'dummy discussion': {'id': 'dummy_discussion_id'}})
+        self.course = CourseFactory.create()
         self.student = UserFactory.create(username=username, password=password)
         CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
         self.assertTrue(
@@ -1172,7 +1016,7 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase):
                 "django_comment_client.forum.views.single_thread",
                 kwargs={
                     "course_id": self.course.id.to_deprecated_string(),
-                    "discussion_id": "dummy_discussion_id",
+                    "discussion_id": "dummy",
                     "thread_id": thread_id,
                 }
             ),
@@ -1266,7 +1110,7 @@ class SingleThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
     def setUp(self):
         super(SingleThreadUnicodeTestCase, self).setUp()
 
-        self.course = CourseFactory.create(discussion_topics={'dummy_discussion_id': {'id': 'dummy_discussion_id'}})
+        self.course = CourseFactory.create()
         self.student = UserFactory.create()
         CourseEnrollmentFactory(user=self.student, course_id=self.course.id)
 

lms/djangoapps/django_comment_client/forum/views.py

@@ -52,7 +52,7 @@ def _attr_safe_json(obj):
 
 
 @newrelic.agent.function_trace()
-def make_course_settings(course, user):
+def make_course_settings(course):
     """
     Generate a JSON-serializable model for course settings, which will be used to initialize a
     DiscussionCourseSettings object on the client.
@@ -63,14 +63,14 @@ def make_course_settings(course, user):
         'allow_anonymous': course.allow_anonymous,
         'allow_anonymous_to_peers': course.allow_anonymous_to_peers,
         'cohorts': [{"id": str(g.id), "name": g.name} for g in get_course_cohorts(course)],
-        'category_map': utils.get_discussion_category_map(course, user)
+        'category_map': utils.get_discussion_category_map(course)
     }
 
     return obj
 
 
 @newrelic.agent.function_trace()
-def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
+def get_threads(request, course_key, discussion_id=None, per_page=THREADS_PER_PAGE):
     """
     This may raise an appropriate subclass of cc.utils.CommentClientError
     if something goes wrong, or ValueError if the group_id is invalid.
@@ -81,19 +81,12 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
         'sort_key': 'date',
         'sort_order': 'desc',
         'text': '',
-        'course_id': unicode(course.id),
+        'commentable_id': discussion_id,
+        'course_id': course_key.to_deprecated_string(),
         'user_id': request.user.id,
-        'group_id': get_group_id_for_comments_service(request, course.id, discussion_id),  # may raise ValueError
+        'group_id': get_group_id_for_comments_service(request, course_key, discussion_id),  # may raise ValueError
     }
 
-    if discussion_id is not None:
-        default_query_params['commentable_id'] = discussion_id
-    else:
-        default_query_params['commentable_ids'] = ','.join(
-            course.top_level_discussion_topic_ids +
-            utils.get_discussion_id_map(course, request.user).keys()
-        )
-
     if not request.GET.get('sort_key'):
         # If the user did not select a sort key, use their last used sort key
         cc_user = cc.User.from_django_user(request.user)
@@ -170,7 +163,7 @@ def inline_discussion(request, course_key, discussion_id):
     user_info = cc_user.to_dict()
 
     try:
-        threads, query_params = get_threads(request, course, discussion_id, per_page=INLINE_THREADS_PER_PAGE)
+        threads, query_params = get_threads(request, course_key, discussion_id, per_page=INLINE_THREADS_PER_PAGE)
     except ValueError:
         return HttpResponseBadRequest("Invalid group_id")
 
@@ -179,7 +172,7 @@ def inline_discussion(request, course_key, discussion_id):
     is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)
     threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads]
     with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
-        add_courseware_context(threads, course, request.user)
+        add_courseware_context(threads, course)
     return utils.JsonResponse({
         'is_commentable_cohorted': is_commentable_cohorted(course_key, discussion_id),
         'discussion_data': threads,
@@ -188,7 +181,7 @@ def inline_discussion(request, course_key, discussion_id):
         'page': query_params['page'],
         'num_pages': query_params['num_pages'],
         'roles': utils.get_role_ids(course_key),
-        'course_settings': make_course_settings(course, request.user)
+        'course_settings': make_course_settings(course)
     })
 
 
@@ -201,13 +194,13 @@ def forum_form_discussion(request, course_key):
     nr_transaction = newrelic.agent.current_transaction()
 
     course = get_course_with_access(request.user, 'load_forum', course_key, check_if_enrolled=True)
-    course_settings = make_course_settings(course, request.user)
+    course_settings = make_course_settings(course)
 
     user = cc.User.from_django_user(request.user)
     user_info = user.to_dict()
 
     try:
-        unsafethreads, query_params = get_threads(request, course)   # This might process a search query
+        unsafethreads, query_params = get_threads(request, course_key)   # This might process a search query
         is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)
         threads = [utils.prepare_content(thread, course_key, is_staff) for thread in unsafethreads]
     except cc.utils.CommentClientMaintenanceError:
@@ -220,7 +213,7 @@ def forum_form_discussion(request, course_key):
         annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info)
 
     with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
-        add_courseware_context(threads, course, request.user)
+        add_courseware_context(threads, course)
 
     if request.is_ajax():
         return utils.JsonResponse({
@@ -265,21 +258,15 @@ def single_thread(request, course_key, discussion_id, thread_id):
     """
     Renders a response to display a single discussion thread.
     """
+
     nr_transaction = newrelic.agent.current_transaction()
 
     course = get_course_with_access(request.user, 'load_forum', course_key)
-    course_settings = make_course_settings(course, request.user)
+    course_settings = make_course_settings(course)
     cc_user = cc.User.from_django_user(request.user)
     user_info = cc_user.to_dict()
     is_moderator = cached_has_permission(request.user, "see_all_cohorts", course_key)
 
-    # Verify that the student has access to this thread if belongs to a discussion module
-    accessible_discussion_ids = [
-        module.discussion_id for module in utils.get_accessible_discussion_modules(course, request.user)
-    ]
-    if discussion_id not in set(course.top_level_discussion_topic_ids + accessible_discussion_ids):
-        raise Http404
-
     # Currently, the front end always loads responses via AJAX, even for this
     # page; it would be a nice optimization to avoid that extra round trip to
     # the comments service.
@@ -307,7 +294,7 @@ def single_thread(request, course_key, discussion_id, thread_id):
             annotated_content_info = utils.get_annotated_content_infos(course_key, thread, request.user, user_info=user_info)
         content = utils.prepare_content(thread.to_dict(), course_key, is_staff)
         with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
-            add_courseware_context([content], course, request.user)
+            add_courseware_context([content], course)
         return utils.JsonResponse({
             'content': content,
             'annotated_content_info': annotated_content_info,
@@ -315,13 +302,13 @@ def single_thread(request, course_key, discussion_id, thread_id):
 
     else:
         try:
-            threads, query_params = get_threads(request, course)
+            threads, query_params = get_threads(request, course_key)
         except ValueError:
             return HttpResponseBadRequest("Invalid group_id")
         threads.append(thread.to_dict())
 
         with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
-            add_courseware_context(threads, course, request.user)
+            add_courseware_context(threads, course)
 
         for thread in threads:
             # patch for backward compatibility with comments service

lms/djangoapps/django_comment_client/tests/test_utils.py

@@ -10,7 +10,6 @@ import mock
 
 from django_comment_client.tests.factories import RoleFactory
 from django_comment_client.tests.unicode import UnicodeTestMixin
-from django_comment_client.tests.utils import ContentGroupTestCase
 import django_comment_client.utils as utils
 from student.tests.factories import UserFactory, CourseEnrollmentFactory
 from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
@@ -89,7 +88,7 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
     comment client service integration
     """
     def setUp(self):
-        super(CoursewareContextTestCase, self).setUp(create_user=True)
+        super(CoursewareContextTestCase, self).setUp()
 
         self.course = CourseFactory.create(org="TestX", number="101", display_name="Test Course")
         self.discussion1 = ItemFactory.create(
@@ -108,12 +107,12 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
         )
 
     def test_empty(self):
-        utils.add_courseware_context([], self.course, self.user)
+        utils.add_courseware_context([], self.course)
 
     def test_missing_commentable_id(self):
         orig = {"commentable_id": "non-inline"}
         modified = dict(orig)
-        utils.add_courseware_context([modified], self.course, self.user)
+        utils.add_courseware_context([modified], self.course)
         self.assertEqual(modified, orig)
 
     def test_basic(self):
@@ -121,7 +120,7 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
             {"commentable_id": self.discussion1.discussion_id},
             {"commentable_id": self.discussion2.discussion_id}
         ]
-        utils.add_courseware_context(threads, self.course, self.user)
+        utils.add_courseware_context(threads, self.course)
 
         def assertThreadCorrect(thread, discussion, expected_title):  # pylint: disable=invalid-name
             """Asserts that the given thread has the expected set of properties"""
@@ -145,29 +144,13 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
         assertThreadCorrect(threads[1], self.discussion2, "Subsection / Discussion 2")
 
 
-class CategoryMapTestMixin(object):
-    """
-    Provides functionality for classes that test
-    `get_discussion_category_map`.
-    """
-    def assert_category_map_equals(self, expected, requesting_user=None):
-        """
-        Call `get_discussion_category_map`, and verify that it returns
-        what is expected.
-        """
-        self.assertEqual(
-            utils.get_discussion_category_map(self.course, requesting_user or self.user),
-            expected
-        )
-
-
-class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
+class CategoryMapTestCase(ModuleStoreTestCase):
     """
     Base testcase class for discussion categories for the
     comment client service integration
     """
     def setUp(self):
-        super(CategoryMapTestCase, self).setUp(create_user=True)
+        super(CategoryMapTestCase, self).setUp()
 
         self.course = CourseFactory.create(
             org="TestX", number="101", display_name="Test Course",
@@ -193,8 +176,20 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
             **kwargs
         )
 
+    def assertCategoryMapEquals(self, expected):
+        """
+        Compare a manually-constructed category map to the actual result from `utils.get_discussion_category_map`
+        """
+        self.assertEqual(
+            utils.get_discussion_category_map(self.course),
+            expected
+        )
+
     def test_empty(self):
-        self.assert_category_map_equals({"entries": {}, "subcategories": {}, "children": []})
+        self.assertEqual(
+            utils.get_discussion_category_map(self.course),
+            {"entries": {}, "subcategories": {}, "children": []}
+        )
 
     def test_configured_topics(self):
         self.course.discussion_topics = {
@@ -203,8 +198,8 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
             "Topic C": {"id": "Topic_C"}
         }
 
-        def check_cohorted_topics(expected_ids):
-            self.assert_category_map_equals(
+        def check_cohorted_topics(expected_ids):  # pylint: disable=missing-docstring
+            self.assertCategoryMapEquals(
                 {
                     "entries": {
                         "Topic A": {"id": "Topic_A", "sort_key": "Topic A", "is_cohorted": "Topic_A" in expected_ids},
@@ -236,7 +231,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
 
     def test_single_inline(self):
         self.create_discussion("Chapter", "Discussion")
-        self.assert_category_map_equals(
+        self.assertCategoryMapEquals(
             {
                 "entries": {},
                 "subcategories": {
@@ -266,7 +261,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
 
         def check_cohorted(is_cohorted):
 
-            self.assert_category_map_equals(
+            self.assertCategoryMapEquals(
                 {
                     "entries": {},
                     "subcategories": {
@@ -390,7 +385,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion", start=later)
         self.create_discussion("Chapter 3 / Section 1", "Discussion", start=later)
 
-        self.assert_category_map_equals(
+        self.assertCategoryMapEquals(
             {
                 "entries": {},
                 "subcategories": {
@@ -428,7 +423,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         self.create_discussion("Chapter", "Discussion 4", sort_key="C")
         self.create_discussion("Chapter", "Discussion 5", sort_key="B")
 
-        self.assert_category_map_equals(
+        self.assertCategoryMapEquals(
             {
                 "entries": {},
                 "subcategories": {
@@ -480,7 +475,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
             "Topic B": {"id": "Topic_B", "sort_key": "C"},
             "Topic C": {"id": "Topic_C", "sort_key": "A"}
         }
-        self.assert_category_map_equals(
+        self.assertCategoryMapEquals(
             {
                 "entries": {
                     "Topic A": {"id": "Topic_A", "sort_key": "B", "is_cohorted": False},
@@ -501,7 +496,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         self.create_discussion("Chapter", "Discussion C")
         self.create_discussion("Chapter", "Discussion B")
 
-        self.assert_category_map_equals(
+        self.assertCategoryMapEquals(
             {
                 "entries": {},
                 "subcategories": {
@@ -554,7 +549,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         self.create_discussion("Chapter B", "Discussion 1")
         self.create_discussion("Chapter A", "Discussion 2")
 
-        self.assert_category_map_equals(
+        self.assertCategoryMapEquals(
             {
                 "entries": {},
                 "subcategories": {
@@ -607,7 +602,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         )
 
     def test_ids_empty(self):
-        self.assertEqual(utils.get_discussion_categories_ids(self.course, self.user), [])
+        self.assertEqual(utils.get_discussion_categories_ids(self.course), [])
 
     def test_ids_configured_topics(self):
         self.course.discussion_topics = {
@@ -616,7 +611,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
             "Topic C": {"id": "Topic_C"}
         }
         self.assertItemsEqual(
-            utils.get_discussion_categories_ids(self.course, self.user),
+            utils.get_discussion_categories_ids(self.course),
             ["Topic_A", "Topic_B", "Topic_C"]
         )
 
@@ -628,7 +623,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion")
         self.create_discussion("Chapter 3 / Section 1", "Discussion")
         self.assertItemsEqual(
-            utils.get_discussion_categories_ids(self.course, self.user),
+            utils.get_discussion_categories_ids(self.course),
             ["discussion1", "discussion2", "discussion3", "discussion4", "discussion5", "discussion6"]
         )
 
@@ -642,177 +637,11 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
         self.create_discussion("Chapter 2", "Discussion")
         self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
         self.assertItemsEqual(
-            utils.get_discussion_categories_ids(self.course, self.user),
+            utils.get_discussion_categories_ids(self.course),
             ["Topic_A", "Topic_B", "Topic_C", "discussion1", "discussion2", "discussion3"]
         )
 
 
-class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase):
-    """
-    Tests `get_discussion_category_map` on discussion modules which are
-    only visible to some content groups.
-    """
-    def test_staff_user(self):
-        """
-        Verify that the staff user can access the alpha, beta, and
-        global discussion topics.
-        """
-        self.assert_category_map_equals(
-            {
-                'subcategories': {
-                    'Week 1': {
-                        'subcategories': {},
-                        'children': [
-                            'Visible to Alpha',
-                            'Visible to Beta',
-                            'Visible to Everyone'
-                        ],
-                        'entries': {
-                            'Visible to Alpha': {
-                                'sort_key': None,
-                                'is_cohorted': True,
-                                'id': 'alpha_group_discussion'
-                            },
-                            'Visible to Beta': {
-                                'sort_key': None,
-                                'is_cohorted': True,
-                                'id': 'beta_group_discussion'
-                            },
-                            'Visible to Everyone': {
-                                'sort_key': None,
-                                'is_cohorted': True,
-                                'id': 'global_group_discussion'
-                            }
-                        }
-                    }
-                },
-                'children': ['General', 'Week 1'],
-                'entries': {
-                    'General': {
-                        'sort_key': 'General',
-                        'is_cohorted': False,
-                        'id': 'i4x-org-number-course-run'
-                    }
-                }
-            },
-            requesting_user=self.staff_user
-        )
-
-    def test_alpha_user(self):
-        """
-        Verify that the alpha user can access the alpha and global
-        discussion topics.
-        """
-        self.assert_category_map_equals(
-            {
-                'subcategories': {
-                    'Week 1': {
-                        'subcategories': {},
-                        'children': [
-                            'Visible to Alpha',
-                            'Visible to Everyone'
-                        ],
-                        'entries': {
-                            'Visible to Alpha': {
-                                'sort_key': None,
-                                'is_cohorted': True,
-                                'id': 'alpha_group_discussion'
-                            },
-                            'Visible to Everyone': {
-                                'sort_key': None,
-                                'is_cohorted': True,
-                                'id': 'global_group_discussion'
-                            }
-                        }
-                    }
-                },
-                'children': ['General', 'Week 1'],
-                'entries': {
-                    'General': {
-                        'sort_key': 'General',
-                        'is_cohorted': False,
-                        'id': 'i4x-org-number-course-run'
-                    }
-                }
-            },
-            requesting_user=self.alpha_user
-        )
-
-    def test_beta_user(self):
-        """
-        Verify that the beta user can access the beta and global
-        discussion topics.
-        """
-        self.assert_category_map_equals(
-            {
-                'subcategories': {
-                    'Week 1': {
-                        'subcategories': {},
-                        'children': [
-                            'Visible to Beta',
-                            'Visible to Everyone'
-                        ],
-                        'entries': {
-                            'Visible to Beta': {
-                                'sort_key': None,
-                                'is_cohorted': True,
-                                'id': 'beta_group_discussion'
-                            },
-                            'Visible to Everyone': {
-                                'sort_key': None,
-                                'is_cohorted': True,
-                                'id': 'global_group_discussion'
-                            }
-                        }
-                    }
-                },
-                'children': ['General', 'Week 1'],
-                'entries': {
-                    'General': {
-                        'sort_key': 'General',
-                        'is_cohorted': False,
-                        'id': 'i4x-org-number-course-run'
-                    }
-                }
-            },
-            requesting_user=self.beta_user
-        )
-
-    def test_non_cohorted_user(self):
-        """
-        Verify that the non-cohorted user can access the global
-        discussion topic.
-        """
-        self.assert_category_map_equals(
-            {
-                'subcategories': {
-                    'Week 1': {
-                        'subcategories': {},
-                        'children': [
-                            'Visible to Everyone'
-                        ],
-                        'entries': {
-                            'Visible to Everyone': {
-                                'sort_key': None,
-                                'is_cohorted': True,
-                                'id': 'global_group_discussion'
-                            }
-                        }
-                    }
-                },
-                'children': ['General', 'Week 1'],
-                'entries': {
-                    'General': {
-                        'sort_key': 'General',
-                        'is_cohorted': False,
-                        'id': 'i4x-org-number-course-run'
-                    }
-                }
-            },
-            requesting_user=self.non_cohorted_user
-        )
-
-
 class JsonResponseTestCase(TestCase, UnicodeTestMixin):
     def _test_unicode_data(self, text):
         response = utils.JsonResponse(text)

lms/djangoapps/django_comment_client/tests/utils.py

-"""
-Utilities for tests within the django_comment_client module.
-"""
-from datetime import datetime
-from django.test.utils import override_settings
 from mock import patch
-from pytz import UTC
 
-from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup
-from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
+from openedx.core.djangoapps.course_groups.models import CourseUserGroup
 from django_comment_common.models import Role
 from django_comment_common.utils import seed_permissions_roles
 from student.tests.factories import CourseEnrollmentFactory, UserFactory
-from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
+from xmodule.modulestore.tests.factories import CourseFactory
 from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
-from xmodule.partitions.partitions import UserPartition, Group
 
 
-class CohortedTestCase(ModuleStoreTestCase):
+class CohortedContentTestCase(ModuleStoreTestCase):
     """
     Sets up a course with a student, a moderator and their cohorts.
     """
     @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
     def setUp(self):
-        super(CohortedTestCase, self).setUp()
+        super(CohortedContentTestCase, self).setUp()
 
         self.course = CourseFactory.create(
             discussion_topics={
@@ -34,13 +26,15 @@ class CohortedTestCase(ModuleStoreTestCase):
                 "cohorted_discussions": ["cohorted_topic"]
             }
         )
-        self.student_cohort = CohortFactory.create(
+        self.student_cohort = CourseUserGroup.objects.create(
             name="student_cohort",
-            course_id=self.course.id
+            course_id=self.course.id,
+            group_type=CourseUserGroup.COHORT
         )
-        self.moderator_cohort = CohortFactory.create(
+        self.moderator_cohort = CourseUserGroup.objects.create(
             name="moderator_cohort",
-            course_id=self.course.id
+            course_id=self.course.id,
+            group_type=CourseUserGroup.COHORT
         )
 
         seed_permissions_roles(self.course.id)
@@ -51,82 +45,3 @@ class CohortedTestCase(ModuleStoreTestCase):
         self.moderator.roles.add(Role.objects.get(name="Moderator", course_id=self.course.id))
         self.student_cohort.users.add(self.student)
         self.moderator_cohort.users.add(self.moderator)
-
-
-class ContentGroupTestCase(ModuleStoreTestCase):
-    """
-    Sets up discussion modules visible to content groups 'Alpha' and
-    'Beta', as well as a module visible to all students.  Creates a
-    staff user, users with access to Alpha/Beta (by way of cohorts),
-    and a non-cohorted user with no special access.
-    """
-    def setUp(self):
-        super(ContentGroupTestCase, self).setUp()
-
-        self.course = CourseFactory.create(
-            org='org', number='number', run='run',
-            # This test needs to use a course that has already started --
-            # discussion topics only show up if the course has already started,
-            # and the default start date for courses is Jan 1, 2030.
-            start=datetime(2012, 2, 3, tzinfo=UTC),
-            user_partitions=[
-                UserPartition(
-                    0,
-                    'Content Group Configuration',
-                    '',
-                    [Group(1, 'Alpha'), Group(2, 'Beta')],
-                    scheme_id='cohort'
-                )
-            ],
-            cohort_config={'cohorted': True},
-            discussion_topics={}
-        )
-
-        self.staff_user = UserFactory.create(is_staff=True)
-        self.alpha_user = UserFactory.create()
-        self.beta_user = UserFactory.create()
-        self.non_cohorted_user = UserFactory.create()
-        for user in [self.staff_user, self.alpha_user, self.beta_user, self.non_cohorted_user]:
-            CourseEnrollmentFactory.create(user=user, course_id=self.course.id)
-
-        alpha_cohort = CohortFactory(
-            course_id=self.course.id,
-            name='Cohort Alpha',
-            users=[self.alpha_user]
-        )
-        beta_cohort = CohortFactory(
-            course_id=self.course.id,
-            name='Cohort Beta',
-            users=[self.beta_user]
-        )
-        CourseUserGroupPartitionGroup.objects.create(
-            course_user_group=alpha_cohort,
-            partition_id=self.course.user_partitions[0].id,
-            group_id=self.course.user_partitions[0].groups[0].id
-        )
-        CourseUserGroupPartitionGroup.objects.create(
-            course_user_group=beta_cohort,
-            partition_id=self.course.user_partitions[0].id,
-            group_id=self.course.user_partitions[0].groups[1].id
-        )
-        self.alpha_module = ItemFactory.create(
-            parent_location=self.course.location,
-            category='discussion',
-            discussion_id='alpha_group_discussion',
-            discussion_target='Visible to Alpha',
-            group_access={self.course.user_partitions[0].id: [self.course.user_partitions[0].groups[0].id]}
-        )
-        self.beta_module = ItemFactory.create(
-            parent_location=self.course.location,
-            category='discussion',
-            discussion_id='beta_group_discussion',
-            discussion_target='Visible to Beta',
-            group_access={self.course.user_partitions[0].id: [self.course.user_partitions[0].groups[1].id]}
-        )
-        self.global_module = ItemFactory.create(
-            parent_location=self.course.location,
-            category='discussion',
-            discussion_id='global_group_discussion',
-            discussion_target='Visible to Everyone'
-        )
-        self.course = self.store.get_item(self.course.location)

lms/djangoapps/django_comment_client/utils.py

@@ -17,7 +17,7 @@ from xmodule.modulestore.django import modulestore
 from django_comment_common.models import Role, FORUM_ROLE_STUDENT
 from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
 from edxmako import lookup_template
-from courseware.access import has_access
+
 from openedx.core.djangoapps.course_groups.cohorts import get_cohort_by_id, get_cohort_id, is_commentable_cohorted, \
     is_course_cohorted
 from openedx.core.djangoapps.course_groups.models import CourseUserGroup
@@ -59,11 +59,9 @@ def has_forum_access(uname, course_id, rolename):
     return role.users.filter(username=uname).exists()
 
 
-# pylint: disable=invalid-name
-def get_accessible_discussion_modules(course, user):
+def _get_discussion_modules(course):
     """
-    Get all discussion modules within a course which are accessible to
-    the user.
+    Return a list of all valid discussion modules in this course.
     """
     all_modules = modulestore().get_items(course.id, qualifiers={'category': 'discussion'})
 
@@ -74,23 +72,20 @@ def get_accessible_discussion_modules(course, user):
                 return False
         return True
 
-    return [
-        module for module in all_modules
-        if has_required_keys(module) and has_access(user, 'load', module, course.id)
-    ]
+    return filter(has_required_keys, all_modules)
 
 
-def get_discussion_id_map(course, user):
+def get_discussion_id_map(course):
     """
-    Get metadata about discussion modules visible to the user in a course.
+    Transform the list of this course's discussion modules into a dictionary of metadata keyed by discussion_id.
     """
-    def get_entry(module):
+    def get_entry(module):  # pylint: disable=missing-docstring
         discussion_id = module.discussion_id
         title = module.discussion_target
         last_category = module.discussion_category.split("/")[-1].strip()
         return (discussion_id, {"location": module.location, "title": last_category + " / " + title})
 
-    return dict(map(get_entry, get_accessible_discussion_modules(course, user)))
+    return dict(map(get_entry, _get_discussion_modules(course)))
 
 
 def _filter_unstarted_categories(category_map):
@@ -143,14 +138,14 @@ def _sort_map_entries(category_map, sort_alpha):
     category_map["children"] = [x[0] for x in sorted(things, key=lambda x: x[1]["sort_key"])]
 
 
-def get_discussion_category_map(course, user):
+def get_discussion_category_map(course):
     """
-    Get a mapping of categories and subcategories that are visible to
-    the user within a course.
+    Transform the list of this course's discussion modules into a recursive dictionary structure.  This is used
+    to render the discussion category map in the discussion tab sidebar.
     """
     unexpanded_category_map = defaultdict(list)
 
-    modules = get_accessible_discussion_modules(course, user)
+    modules = _get_discussion_modules(course)
 
     is_course_cohorted = course.is_cohorted
     cohorted_discussion_ids = course.cohorted_discussions
@@ -223,12 +218,12 @@ def get_discussion_category_map(course, user):
     return _filter_unstarted_categories(category_map)
 
 
-def get_discussion_categories_ids(course, user):
+def get_discussion_categories_ids(course):
     """
     Returns a list of available ids of categories for the course.
     """
     ids = []
-    queue = [get_discussion_category_map(course, user)]
+    queue = [get_discussion_category_map(course)]
     while queue:
         category_map = queue.pop()
         for child in category_map["children"]:
@@ -387,12 +382,12 @@ def extend_content(content):
     return merge_dict(content, content_info)
 
 
-def add_courseware_context(content_list, course, user, id_map=None):
+def add_courseware_context(content_list, course, id_map=None):
     """
     Decorates `content_list` with courseware metadata.
     """
     if id_map is None:
-        id_map = get_discussion_id_map(course, user)
+        id_map = get_discussion_id_map(course)
 
     for content in content_list:
         commentable_id = content['commentable_id']

lms/envs/common.py

@@ -303,6 +303,9 @@ FEATURES = {
     # Set to True to change the course sorting behavior by their start dates, latest first.
     'ENABLE_COURSE_SORTING_BY_START_DATE': False,
 
+    # Flag to enable new user account APIs.
+    'ENABLE_USER_REST_API': False,
+
     # Expose Mobile REST API. Note that if you use this, you must also set
     # ENABLE_OAUTH2_PROVIDER to True
     'ENABLE_MOBILE_REST_API': False,

lms/envs/test.py

@@ -265,6 +265,7 @@ FEATURES['ENABLE_OAUTH2_PROVIDER'] = True
 FEATURES['ENABLE_MOBILE_REST_API'] = True
 FEATURES['ENABLE_MOBILE_SOCIAL_FACEBOOK_FEATURES'] = True
 FEATURES['ENABLE_VIDEO_ABSTRACTION_LAYER_API'] = True
+FEATURES['ENABLE_USER_REST_API'] = True
 
 ###################### Payment ##############################3
 # Enable fake payment processing page

lms/urls.py

@@ -60,10 +60,8 @@ urlpatterns = (
 
     url(r'^heartbeat$', include('heartbeat.urls')),
 
-    url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')),
-
     # Note: these are older versions of the User API that will eventually be
-    # subsumed by api/user.
+    # subsumed by api/user listed below.
     url(r'^user_api/', include('openedx.core.djangoapps.user_api.legacy_urls')),
 
     url(r'^notifier_api/', include('notifier_api.urls')),
@@ -88,6 +86,11 @@ urlpatterns = (
     url(r'^api/course_structure/', include('course_structure_api.urls', namespace='course_structure_api')),
 )
 
+if settings.FEATURES["ENABLE_USER_REST_API"]:
+    urlpatterns += (
+        url(r'^api/user/', include('openedx.core.djangoapps.user_api.urls')),
+    )
+
 if settings.FEATURES["ENABLE_COMBINED_LOGIN_REGISTRATION"]:
     # Backwards compatibility with old URL structure, but serve the new views
     urlpatterns += (