diff --git a/cms/djangoapps/contentstore/views/tests/test_videos.py b/cms/djangoapps/contentstore/views/tests/test_videos.py index 9d7ff44878..e3d8c49688 100644 --- a/cms/djangoapps/contentstore/views/tests/test_videos.py +++ b/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: diff --git a/cms/djangoapps/contentstore/views/videos.py b/cms/djangoapps/contentstore/views/videos.py index e1232953e5..6cc3233054 100644 --- a/cms/djangoapps/contentstore/views/videos.py +++ b/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") diff --git a/common/test/acceptance/pages/lms/discussion.py b/common/test/acceptance/pages/lms/discussion.py index 5fe900079e..b026a9bdb8 100644 --- a/common/test/acceptance/pages/lms/discussion.py +++ b/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() diff --git a/common/test/acceptance/tests/discussion/helpers.py b/common/test/acceptance/tests/discussion/helpers.py index f0722cd236..46cdba1155 100644 --- a/common/test/acceptance/tests/discussion/helpers.py +++ b/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) diff --git a/common/test/acceptance/tests/discussion/test_cohorts.py b/common/test/acceptance/tests/discussion/test_cohorts.py index 9bc1a3f52f..3c567a15b9 100644 --- a/common/test/acceptance/tests/discussion/test_cohorts.py +++ b/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 diff --git a/common/test/acceptance/tests/discussion/test_discussion.py b/common/test/acceptance/tests/discussion/test_discussion.py index 8aebf75efc..9efcadb629 100644 --- a/common/test/acceptance/tests/discussion/test_discussion.py +++ b/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")) diff --git a/lms/djangoapps/certificates/queue.py b/lms/djangoapps/certificates/queue.py index e0bef5a49b..0c0d77d99e 100644 --- a/lms/djangoapps/certificates/queue.py +++ b/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: diff --git a/lms/djangoapps/certificates/tests/test_queue.py b/lms/djangoapps/certificates/tests/test_queue.py index f91469e720..ee1843e739 100644 --- a/lms/djangoapps/certificates/tests/test_queue.py +++ b/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): diff --git a/lms/djangoapps/django_comment_client/base/tests.py b/lms/djangoapps/django_comment_client/base/tests.py index f949f434f9..da31e2e7bb 100644 --- a/lms/djangoapps/django_comment_client/base/tests.py +++ b/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( diff --git a/lms/djangoapps/django_comment_client/base/views.py b/lms/djangoapps/django_comment_client/base/views.py index dc41404e28..133f5af161 100644 --- a/lms/djangoapps/django_comment_client/base/views.py +++ b/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: diff --git a/lms/djangoapps/django_comment_client/forum/tests.py b/lms/djangoapps/django_comment_client/forum/tests.py index 48921dde77..8e2ec25ed4 100644 --- a/lms/djangoapps/django_comment_client/forum/tests.py +++ b/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) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index fb4b988b4c..d1cb4e83da 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/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 diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index e3fb9b6ef7..99dd020cbd 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/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) diff --git a/lms/djangoapps/django_comment_client/tests/utils.py b/lms/djangoapps/django_comment_client/tests/utils.py index 7cef278717..557e36c202 100644 --- a/lms/djangoapps/django_comment_client/tests/utils.py +++ b/lms/djangoapps/django_comment_client/tests/utils.py @@ -1,28 +1,20 @@ -""" -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) diff --git a/lms/djangoapps/django_comment_client/utils.py b/lms/djangoapps/django_comment_client/utils.py index 800545bc3e..966e335eed 100644 --- a/lms/djangoapps/django_comment_client/utils.py +++ b/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'] diff --git a/lms/envs/common.py b/lms/envs/common.py index 38c7f3ea1a..9a72d221be 100644 --- a/lms/envs/common.py +++ b/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, diff --git a/lms/envs/test.py b/lms/envs/test.py index 9de7b2071a..8b22784449 100644 --- a/lms/envs/test.py +++ b/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 diff --git a/lms/urls.py b/lms/urls.py index a7948e3212..f53481a5c7 100644 --- a/lms/urls.py +++ b/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 += (