Skip to content

E
edx-platform
Commit 84938839 by David Baumgold
Options

Merge pull request #7244 from edx/jsa/hotfix-forums 

Revert "Merge pull request #6771 from edx/dan-f/forums-filter-discussion...
parents 5802e17c 13ba2c4a
Showing with 162 additions and 587 deletions
common/test/acceptance/pages/lms/discussion.py
...@@ -311,15 +311,13 @@ class DiscussionSortPreferencePage(CoursePage): ...@@ -311,15 +311,13 @@ class DiscussionSortPreferencePage(CoursePage):
class DiscussionTabSingleThreadPage(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) super(DiscussionTabSingleThreadPage, self).__init__(browser, course_id)
self.thread_page = DiscussionThreadPage( self.thread_page = DiscussionThreadPage(
browser, browser,
"body.discussion .discussion-article[data-id='{thread_id}']".format(thread_id=thread_id) "body.discussion .discussion-article[data-id='{thread_id}']".format(thread_id=thread_id)
) )
self.url_path = "discussion/forum/{discussion_id}/threads/{thread_id}".format( self.url_path = "discussion/forum/dummy/threads/" + thread_id
discussion_id=discussion_id, thread_id=thread_id
)
def is_browser_on_page(self): def is_browser_on_page(self):
return self.thread_page.is_browser_on_page() 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. ...@@ -5,15 +5,12 @@ Helper functions and classes for discussion tests.
from uuid import uuid4 from uuid import uuid4
import json import json
from ...fixtures import LMS_BASE_URL
from ...fixtures.course import CourseFixture
from ...fixtures.discussion import ( from ...fixtures.discussion import (
SingleThreadViewFixture, SingleThreadViewFixture,
Thread, Thread,
Response, Response,
) )
from ...pages.lms.discussion import DiscussionTabSingleThreadPage from ...fixtures import LMS_BASE_URL
from ...tests.helpers import UniqueCourseTest
class BaseDiscussionMixin(object): class BaseDiscussionMixin(object):
...@@ -86,22 +83,3 @@ class CohortTestMixin(object): ...@@ -86,22 +83,3 @@ class CohortTestMixin(object):
data = {"users": username} data = {"users": username}
response = course_fixture.session.post(url, data=data, headers=course_fixture.headers) response = course_fixture.session.post(url, data=data, headers=course_fixture.headers)
self.assertTrue(response.ok, "Failed to add user to cohort") 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. ...@@ -3,7 +3,7 @@ Tests related to the cohorting feature.
""" """
from uuid import uuid4 from uuid import uuid4
from .helpers import BaseDiscussionMixin, BaseDiscussionTestCase from .helpers import BaseDiscussionMixin
from .helpers import CohortTestMixin from .helpers import CohortTestMixin
from ..helpers import UniqueCourseTest from ..helpers import UniqueCourseTest
from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.auto_auth import AutoAuthPage
...@@ -57,17 +57,20 @@ class CohortedDiscussionTestMixin(BaseDiscussionMixin, CohortTestMixin): ...@@ -57,17 +57,20 @@ class CohortedDiscussionTestMixin(BaseDiscussionMixin, CohortTestMixin):
self.assertEquals(self.thread_page.get_group_visibility_label(), "This post is visible to everyone.") 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. Tests for the discussion page displaying a single thread.
""" """
def setUp(self): def setUp(self):
super(DiscussionTabSingleThreadTest, self).setUp() 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() self.setup_cohorts()
AutoAuthPage(self.browser, course_id=self.course_id).visit() AutoAuthPage(self.browser, course_id=self.course_id).visit()
def setup_thread_page(self, thread_id): 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() self.thread_page.visit()
# pylint: disable=unused-argument # pylint: disable=unused-argument
......
common/test/acceptance/tests/discussion/test_discussion.py
...@@ -7,7 +7,6 @@ from pytz import UTC ...@@ -7,7 +7,6 @@ from pytz import UTC
from uuid import uuid4 from uuid import uuid4
from nose.plugins.attrib import attr from nose.plugins.attrib import attr
from .helpers import BaseDiscussionTestCase
from ..helpers import UniqueCourseTest from ..helpers import UniqueCourseTest
from ...pages.lms.auto_auth import AutoAuthPage from ...pages.lms.auto_auth import AutoAuthPage
from ...pages.lms.courseware import CoursewarePage from ...pages.lms.courseware import CoursewarePage
...@@ -140,17 +139,22 @@ class DiscussionHomePageTest(UniqueCourseTest): ...@@ -140,17 +139,22 @@ class DiscussionHomePageTest(UniqueCourseTest):
@attr('shard_1') @attr('shard_1')
class DiscussionTabSingleThreadTest(BaseDiscussionTestCase, DiscussionResponsePaginationTestMixin): class DiscussionTabSingleThreadTest(UniqueCourseTest, DiscussionResponsePaginationTestMixin):
""" """
Tests for the discussion page displaying a single thread Tests for the discussion page displaying a single thread
""" """
def setUp(self): def setUp(self):
super(DiscussionTabSingleThreadTest, self).setUp() 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() AutoAuthPage(self.browser, course_id=self.course_id).visit()
def setup_thread_page(self, thread_id): 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() self.thread_page.visit()
def test_marked_answer_comments(self): def test_marked_answer_comments(self):
...@@ -176,7 +180,7 @@ class DiscussionTabSingleThreadTest(BaseDiscussionTestCase, DiscussionResponsePa ...@@ -176,7 +180,7 @@ class DiscussionTabSingleThreadTest(BaseDiscussionTestCase, DiscussionResponsePa
@attr('shard_1') @attr('shard_1')
class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase): class DiscussionOpenClosedThreadTest(UniqueCourseTest):
""" """
Tests for checking the display of attributes on open and closed threads Tests for checking the display of attributes on open and closed threads
""" """
...@@ -184,6 +188,8 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase): ...@@ -184,6 +188,8 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase):
def setUp(self): def setUp(self):
super(DiscussionOpenClosedThreadTest, self).setUp() super(DiscussionOpenClosedThreadTest, self).setUp()
# Create a course to register for
CourseFixture(**self.course_info).install()
self.thread_id = "test_thread_{}".format(uuid4().hex) self.thread_id = "test_thread_{}".format(uuid4().hex)
def setup_user(self, roles=[]): def setup_user(self, roles=[]):
...@@ -191,7 +197,6 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase): ...@@ -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() self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id()
def setup_view(self, **thread_kwargs): def setup_view(self, **thread_kwargs):
thread_kwargs.update({'commentable_id': self.discussion_id})
view = SingleThreadViewFixture( view = SingleThreadViewFixture(
Thread(id=self.thread_id, **thread_kwargs) Thread(id=self.thread_id, **thread_kwargs)
) )
...@@ -204,7 +209,7 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase): ...@@ -204,7 +209,7 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase):
self.setup_view(closed=True) self.setup_view(closed=True)
else: else:
self.setup_view() 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.visit()
page.close_open_thread() page.close_open_thread()
return page return page
...@@ -225,16 +230,23 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase): ...@@ -225,16 +230,23 @@ class DiscussionOpenClosedThreadTest(BaseDiscussionTestCase):
@attr('shard_1') @attr('shard_1')
class DiscussionCommentDeletionTest(BaseDiscussionTestCase): class DiscussionCommentDeletionTest(UniqueCourseTest):
""" """
Tests for deleting comments displayed beneath responses in the single thread view. 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=[]): def setup_user(self, roles=[]):
roles_str = ','.join(roles) roles_str = ','.join(roles)
self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id() self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id()
def setup_view(self): 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( view.addResponse(
Response(id="response1"), Response(id="response1"),
[Comment(id="comment_other_author", user_id="other"), Comment(id="comment_self_author", user_id=self.user_id)]) [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): ...@@ -243,7 +255,7 @@ class DiscussionCommentDeletionTest(BaseDiscussionTestCase):
def test_comment_deletion_as_student(self): def test_comment_deletion_as_student(self):
self.setup_user() self.setup_user()
self.setup_view() 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() page.visit()
self.assertTrue(page.is_comment_deletable("comment_self_author")) self.assertTrue(page.is_comment_deletable("comment_self_author"))
self.assertTrue(page.is_comment_visible("comment_other_author")) self.assertTrue(page.is_comment_visible("comment_other_author"))
...@@ -253,7 +265,7 @@ class DiscussionCommentDeletionTest(BaseDiscussionTestCase): ...@@ -253,7 +265,7 @@ class DiscussionCommentDeletionTest(BaseDiscussionTestCase):
def test_comment_deletion_as_moderator(self): def test_comment_deletion_as_moderator(self):
self.setup_user(roles=['Moderator']) self.setup_user(roles=['Moderator'])
self.setup_view() 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() page.visit()
self.assertTrue(page.is_comment_deletable("comment_self_author")) self.assertTrue(page.is_comment_deletable("comment_self_author"))
self.assertTrue(page.is_comment_deletable("comment_other_author")) self.assertTrue(page.is_comment_deletable("comment_other_author"))
...@@ -262,16 +274,23 @@ class DiscussionCommentDeletionTest(BaseDiscussionTestCase): ...@@ -262,16 +274,23 @@ class DiscussionCommentDeletionTest(BaseDiscussionTestCase):
@attr('shard_1') @attr('shard_1')
class DiscussionResponseEditTest(BaseDiscussionTestCase): class DiscussionResponseEditTest(UniqueCourseTest):
""" """
Tests for editing responses displayed beneath thread in the single thread view. 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=[]): def setup_user(self, roles=[]):
roles_str = ','.join(roles) roles_str = ','.join(roles)
self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id() self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id()
def setup_view(self): 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( view.addResponse(
Response(id="response_other_author", user_id="other", thread_id="response_edit_test_thread"), Response(id="response_other_author", user_id="other", thread_id="response_edit_test_thread"),
) )
...@@ -298,7 +317,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase): ...@@ -298,7 +317,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
""" """
self.setup_user() self.setup_user()
self.setup_view() 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() page.visit()
self.assertTrue(page.is_response_visible("response_other_author")) self.assertTrue(page.is_response_visible("response_other_author"))
self.assertFalse(page.is_response_editable("response_other_author")) self.assertFalse(page.is_response_editable("response_other_author"))
...@@ -315,7 +334,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase): ...@@ -315,7 +334,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
""" """
self.setup_user(roles=["Moderator"]) self.setup_user(roles=["Moderator"])
self.setup_view() 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() page.visit()
self.edit_response(page, "response_self_author") self.edit_response(page, "response_self_author")
self.edit_response(page, "response_other_author") self.edit_response(page, "response_other_author")
...@@ -343,7 +362,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase): ...@@ -343,7 +362,7 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
""" """
self.setup_user(roles=["Moderator"]) self.setup_user(roles=["Moderator"])
self.setup_view() 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() page.visit()
self.edit_response(page, "response_self_author") self.edit_response(page, "response_self_author")
self.edit_response(page, "response_other_author") self.edit_response(page, "response_other_author")
...@@ -356,16 +375,23 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase): ...@@ -356,16 +375,23 @@ class DiscussionResponseEditTest(BaseDiscussionTestCase):
@attr('shard_1') @attr('shard_1')
class DiscussionCommentEditTest(BaseDiscussionTestCase): class DiscussionCommentEditTest(UniqueCourseTest):
""" """
Tests for editing comments displayed beneath responses in the single thread view. 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=[]): def setup_user(self, roles=[]):
roles_str = ','.join(roles) roles_str = ','.join(roles)
self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id() self.user_id = AutoAuthPage(self.browser, course_id=self.course_id, roles=roles_str).visit().get_user_id()
def setup_view(self): 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( view.addResponse(
Response(id="response1"), Response(id="response1"),
[Comment(id="comment_other_author", user_id="other"), Comment(id="comment_self_author", user_id=self.user_id)]) [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): ...@@ -380,7 +406,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase):
def test_edit_comment_as_student(self): def test_edit_comment_as_student(self):
self.setup_user() self.setup_user()
self.setup_view() 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() page.visit()
self.assertTrue(page.is_comment_editable("comment_self_author")) self.assertTrue(page.is_comment_editable("comment_self_author"))
self.assertTrue(page.is_comment_visible("comment_other_author")) self.assertTrue(page.is_comment_visible("comment_other_author"))
...@@ -390,7 +416,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase): ...@@ -390,7 +416,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase):
def test_edit_comment_as_moderator(self): def test_edit_comment_as_moderator(self):
self.setup_user(roles=["Moderator"]) self.setup_user(roles=["Moderator"])
self.setup_view() 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() page.visit()
self.assertTrue(page.is_comment_editable("comment_self_author")) self.assertTrue(page.is_comment_editable("comment_self_author"))
self.assertTrue(page.is_comment_editable("comment_other_author")) self.assertTrue(page.is_comment_editable("comment_other_author"))
...@@ -400,7 +426,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase): ...@@ -400,7 +426,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase):
def test_cancel_comment_edit(self): def test_cancel_comment_edit(self):
self.setup_user() self.setup_user()
self.setup_view() 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() page.visit()
self.assertTrue(page.is_comment_editable("comment_self_author")) self.assertTrue(page.is_comment_editable("comment_self_author"))
original_body = page.get_comment_body("comment_self_author") original_body = page.get_comment_body("comment_self_author")
...@@ -412,7 +438,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase): ...@@ -412,7 +438,7 @@ class DiscussionCommentEditTest(BaseDiscussionTestCase):
"""Only one editor should be visible at a time within a single response""" """Only one editor should be visible at a time within a single response"""
self.setup_user(roles=["Moderator"]) self.setup_user(roles=["Moderator"])
self.setup_view() 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() page.visit()
self.assertTrue(page.is_comment_editable("comment_self_author")) self.assertTrue(page.is_comment_editable("comment_self_author"))
self.assertTrue(page.is_comment_editable("comment_other_author")) self.assertTrue(page.is_comment_editable("comment_other_author"))
......
lms/djangoapps/django_comment_client/base/tests.py
...@@ -14,7 +14,7 @@ from lms.lib.comment_client import Thread ...@@ -14,7 +14,7 @@ from lms.lib.comment_client import Thread
from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE from xmodule.modulestore.tests.django_utils import TEST_DATA_MOCK_MODULESTORE
from django_comment_client.base import views from django_comment_client.base import views
from django_comment_client.tests.group_id import CohortedTopicGroupIdTestMixin, NonCohortedTopicGroupIdTestMixin, GroupIdAssertionMixin 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_client.tests.unicode import UnicodeTestMixin
from django_comment_common.models import Role from django_comment_common.models import Role
from django_comment_common.utils import seed_permissions_roles from django_comment_common.utils import seed_permissions_roles
...@@ -42,7 +42,7 @@ class MockRequestSetupMixin(object): ...@@ -42,7 +42,7 @@ class MockRequestSetupMixin(object):
@patch('lms.lib.comment_client.utils.requests.request') @patch('lms.lib.comment_client.utils.requests.request')
class CreateThreadGroupIdTestCase( class CreateThreadGroupIdTestCase(
MockRequestSetupMixin, MockRequestSetupMixin,
CohortedTestCase, CohortedContentTestCase,
CohortedTopicGroupIdTestMixin, CohortedTopicGroupIdTestMixin,
NonCohortedTopicGroupIdTestMixin NonCohortedTopicGroupIdTestMixin
): ):
...@@ -77,7 +77,7 @@ class CreateThreadGroupIdTestCase( ...@@ -77,7 +77,7 @@ class CreateThreadGroupIdTestCase(
@patch('lms.lib.comment_client.utils.requests.request') @patch('lms.lib.comment_client.utils.requests.request')
class ThreadActionGroupIdTestCase( class ThreadActionGroupIdTestCase(
MockRequestSetupMixin, MockRequestSetupMixin,
CohortedTestCase, CohortedContentTestCase,
GroupIdAssertionMixin GroupIdAssertionMixin
): ):
def call_view( 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): ...@@ -74,7 +74,7 @@ def track_forum_event(request, event_name, course, obj, data, id_map=None):
user = request.user user = request.user
data['id'] = obj.id data['id'] = obj.id
if id_map is None: 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'] commentable_id = data['commentable_id']
if commentable_id in id_map: if commentable_id in id_map:
...@@ -173,9 +173,9 @@ def create_thread(request, course_id, commentable_id): ...@@ -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. # Calls to id map are expensive, but we need this more than once.
# Prefetch it. # 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', track_forum_event(request, 'edx.forum.thread.created',
course, thread, event_data, id_map=id_map) course, thread, event_data, id_map=id_map)
...@@ -208,7 +208,7 @@ def update_thread(request, course_id, thread_id): ...@@ -208,7 +208,7 @@ def update_thread(request, course_id, thread_id):
thread.thread_type = request.POST["thread_type"] thread.thread_type = request.POST["thread_type"]
if "commentable_id" in request.POST: if "commentable_id" in request.POST:
course = get_course_with_access(request.user, 'load', course_key) 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: if request.POST.get("commentable_id") in commentable_ids:
thread.commentable_id = request.POST["commentable_id"] thread.commentable_id = request.POST["commentable_id"]
else: else:
......
lms/djangoapps/django_comment_client/forum/tests.py
...@@ -2,7 +2,6 @@ import json ...@@ -2,7 +2,6 @@ import json
import logging import logging
import ddt import ddt
from django.core import cache
from django.core.urlresolvers import reverse from django.core.urlresolvers import reverse
from django.http import Http404 from django.http import Http404
from django.test.client import Client, RequestFactory from django.test.client import Client, RequestFactory
...@@ -15,7 +14,7 @@ from django_comment_client.tests.group_id import ( ...@@ -15,7 +14,7 @@ from django_comment_client.tests.group_id import (
NonCohortedTopicGroupIdTestMixin NonCohortedTopicGroupIdTestMixin
) )
from django_comment_client.tests.unicode import UnicodeTestMixin 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 django_comment_client.utils import strip_none
from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory
from util.testing import UrlResetMixin from util.testing import UrlResetMixin
...@@ -187,7 +186,7 @@ class SingleThreadTestCase(ModuleStoreTestCase): ...@@ -187,7 +186,7 @@ class SingleThreadTestCase(ModuleStoreTestCase):
def setUp(self): def setUp(self):
super(SingleThreadTestCase, self).setUp(create_user=False) 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() self.student = UserFactory.create()
CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
...@@ -302,24 +301,18 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase): ...@@ -302,24 +301,18 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
MODULESTORE = TEST_DATA_MONGO_MODULESTORE MODULESTORE = TEST_DATA_MONGO_MODULESTORE
@ddt.data( @ddt.data(
# old mongo with cache: number of responses plus 17. TODO: O(n)! # old mongo: number of responses plus 16. TODO: O(n)!
(ModuleStoreEnum.Type.mongo, 1, 23, 18), (ModuleStoreEnum.Type.mongo, 1, 17),
(ModuleStoreEnum.Type.mongo, 50, 366, 67), (ModuleStoreEnum.Type.mongo, 50, 66),
# split mongo: 3 queries, regardless of thread response size. # split mongo: 3 queries, regardless of thread response size.
(ModuleStoreEnum.Type.split, 1, 3, 3), (ModuleStoreEnum.Type.split, 1, 3),
(ModuleStoreEnum.Type.split, 50, 3, 3), (ModuleStoreEnum.Type.split, 50, 3),
) )
@ddt.unpack @ddt.unpack
def test_number_of_mongo_queries( def test_number_of_mongo_queries(self, default_store, num_thread_responses, num_mongo_calls, mock_request):
self,
default_store,
num_thread_responses,
num_uncached_mongo_calls,
num_cached_mongo_calls,
mock_request
):
with modulestore().default_store(default_store): with modulestore().default_store(default_store):
course = CourseFactory.create(discussion_topics={'dummy discussion': {'id': 'dummy_discussion_id'}}) course = CourseFactory.create()
student = UserFactory.create() student = UserFactory.create()
CourseEnrollmentFactory.create(user=student, course_id=course.id) CourseEnrollmentFactory.create(user=student, course_id=course.id)
...@@ -335,11 +328,7 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase): ...@@ -335,11 +328,7 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
HTTP_X_REQUESTED_WITH="XMLHttpRequest" HTTP_X_REQUESTED_WITH="XMLHttpRequest"
) )
request.user = student request.user = student
with check_mongo_calls(num_mongo_calls):
def call_single_thread():
"""
Call single_thread and assert that it returns what we expect.
"""
response = views.single_thread( response = views.single_thread(
request, request,
course.id.to_deprecated_string(), course.id.to_deprecated_string(),
...@@ -349,30 +338,9 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase): ...@@ -349,30 +338,9 @@ class SingleThreadQueryCountTestCase(ModuleStoreTestCase):
self.assertEquals(response.status_code, 200) self.assertEquals(response.status_code, 200)
self.assertEquals(len(json.loads(response.content)["content"]["children"]), num_thread_responses) 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') @patch('requests.request')
class SingleCohortedThreadTestCase(CohortedTestCase): class SingleCohortedThreadTestCase(CohortedContentTestCase):
def _create_mock_cohorted_thread(self, mock_request): def _create_mock_cohorted_thread(self, mock_request):
self.mock_text = "dummy content" self.mock_text = "dummy content"
self.mock_thread_id = "test_thread_id" self.mock_thread_id = "test_thread_id"
...@@ -392,7 +360,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase): ...@@ -392,7 +360,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase):
response = views.single_thread( response = views.single_thread(
request, request,
self.course.id.to_deprecated_string(), self.course.id.to_deprecated_string(),
"cohorted_topic", "dummy_discussion_id",
self.mock_thread_id self.mock_thread_id
) )
...@@ -416,7 +384,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase): ...@@ -416,7 +384,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase):
response = views.single_thread( response = views.single_thread(
request, request,
self.course.id.to_deprecated_string(), self.course.id.to_deprecated_string(),
"cohorted_topic", "dummy_discussion_id",
self.mock_thread_id self.mock_thread_id
) )
...@@ -429,7 +397,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase): ...@@ -429,7 +397,7 @@ class SingleCohortedThreadTestCase(CohortedTestCase):
@patch('lms.lib.comment_client.utils.requests.request') @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): def call_view(self, mock_request, commentable_id, user, group_id, thread_group_id=None, pass_group_id=True):
thread_id = "test_thread_id" thread_id = "test_thread_id"
mock_request.side_effect = make_mock_request_impl("dummy context", thread_id, group_id=thread_group_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): ...@@ -514,7 +482,7 @@ class SingleThreadAccessTestCase(CohortedTestCase):
@patch('lms.lib.comment_client.utils.requests.request') @patch('lms.lib.comment_client.utils.requests.request')
class SingleThreadGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin): class SingleThreadGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
cs_endpoint = "/threads" cs_endpoint = "/threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True, is_ajax=False): 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 ...@@ -536,7 +504,7 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixi
return views.single_thread( return views.single_thread(
request, request,
self.course.id.to_deprecated_string(), self.course.id.to_deprecated_string(),
commentable_id, "dummy_discussion_id",
"dummy_thread_id" "dummy_thread_id"
) )
...@@ -563,133 +531,9 @@ class SingleThreadGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixi ...@@ -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') @patch('lms.lib.comment_client.utils.requests.request')
class InlineDiscussionGroupIdTestCase( class InlineDiscussionGroupIdTestCase(
CohortedTestCase, CohortedContentTestCase,
CohortedTopicGroupIdTestMixin, CohortedTopicGroupIdTestMixin,
NonCohortedTopicGroupIdTestMixin NonCohortedTopicGroupIdTestMixin
): ):
...@@ -735,7 +579,7 @@ class InlineDiscussionGroupIdTestCase( ...@@ -735,7 +579,7 @@ class InlineDiscussionGroupIdTestCase(
@patch('lms.lib.comment_client.utils.requests.request') @patch('lms.lib.comment_client.utils.requests.request')
class ForumFormDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin): class ForumFormDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
cs_endpoint = "/threads" cs_endpoint = "/threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True, is_ajax=False): 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 ...@@ -785,7 +629,7 @@ class ForumFormDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdT
@patch('lms.lib.comment_client.utils.requests.request') @patch('lms.lib.comment_client.utils.requests.request')
class UserProfileDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin): class UserProfileDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
cs_endpoint = "/active_threads" cs_endpoint = "/active_threads"
def call_view_for_profiled_user( def call_view_for_profiled_user(
...@@ -951,7 +795,7 @@ class UserProfileDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupI ...@@ -951,7 +795,7 @@ class UserProfileDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupI
@patch('lms.lib.comment_client.utils.requests.request') @patch('lms.lib.comment_client.utils.requests.request')
class FollowedThreadsDiscussionGroupIdTestCase(CohortedTestCase, CohortedTopicGroupIdTestMixin): class FollowedThreadsDiscussionGroupIdTestCase(CohortedContentTestCase, CohortedTopicGroupIdTestMixin):
cs_endpoint = "/subscribed_threads" cs_endpoint = "/subscribed_threads"
def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True): def call_view(self, mock_request, commentable_id, user, group_id, pass_group_id=True):
...@@ -1142,7 +986,7 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase): ...@@ -1142,7 +986,7 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase):
# Invoke UrlResetMixin # Invoke UrlResetMixin
super(CommentsServiceRequestHeadersTestCase, self).setUp(create_user=False) 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) self.student = UserFactory.create(username=username, password=password)
CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id) CourseEnrollmentFactory.create(user=self.student, course_id=self.course.id)
self.assertTrue( self.assertTrue(
...@@ -1172,7 +1016,7 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase): ...@@ -1172,7 +1016,7 @@ class CommentsServiceRequestHeadersTestCase(UrlResetMixin, ModuleStoreTestCase):
"django_comment_client.forum.views.single_thread", "django_comment_client.forum.views.single_thread",
kwargs={ kwargs={
"course_id": self.course.id.to_deprecated_string(), "course_id": self.course.id.to_deprecated_string(),
"discussion_id": "dummy_discussion_id", "discussion_id": "dummy",
"thread_id": thread_id, "thread_id": thread_id,
} }
), ),
...@@ -1266,7 +1110,7 @@ class SingleThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin): ...@@ -1266,7 +1110,7 @@ class SingleThreadUnicodeTestCase(ModuleStoreTestCase, UnicodeTestMixin):
def setUp(self): def setUp(self):
super(SingleThreadUnicodeTestCase, self).setUp() 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() self.student = UserFactory.create()
CourseEnrollmentFactory(user=self.student, course_id=self.course.id) 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): ...@@ -52,7 +52,7 @@ def _attr_safe_json(obj):
@newrelic.agent.function_trace() @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 Generate a JSON-serializable model for course settings, which will be used to initialize a
DiscussionCourseSettings object on the client. DiscussionCourseSettings object on the client.
...@@ -63,14 +63,14 @@ def make_course_settings(course, user): ...@@ -63,14 +63,14 @@ def make_course_settings(course, user):
'allow_anonymous': course.allow_anonymous, 'allow_anonymous': course.allow_anonymous,
'allow_anonymous_to_peers': course.allow_anonymous_to_peers, 'allow_anonymous_to_peers': course.allow_anonymous_to_peers,
'cohorts': [{"id": str(g.id), "name": g.name} for g in get_course_cohorts(course)], '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 return obj
@newrelic.agent.function_trace() @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 This may raise an appropriate subclass of cc.utils.CommentClientError
if something goes wrong, or ValueError if the group_id is invalid. 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): ...@@ -81,19 +81,12 @@ def get_threads(request, course, discussion_id=None, per_page=THREADS_PER_PAGE):
'sort_key': 'date', 'sort_key': 'date',
'sort_order': 'desc', 'sort_order': 'desc',
'text': '', 'text': '',
'course_id': unicode(course.id), 'commentable_id': discussion_id,
'course_id': course_key.to_deprecated_string(),
'user_id': request.user.id, '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 not request.GET.get('sort_key'):
# If the user did not select a sort key, use their last used 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) cc_user = cc.User.from_django_user(request.user)
...@@ -170,7 +163,7 @@ def inline_discussion(request, course_key, discussion_id): ...@@ -170,7 +163,7 @@ def inline_discussion(request, course_key, discussion_id):
user_info = cc_user.to_dict() user_info = cc_user.to_dict()
try: 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: except ValueError:
return HttpResponseBadRequest("Invalid group_id") return HttpResponseBadRequest("Invalid group_id")
...@@ -179,7 +172,7 @@ def inline_discussion(request, course_key, discussion_id): ...@@ -179,7 +172,7 @@ def inline_discussion(request, course_key, discussion_id):
is_staff = cached_has_permission(request.user, 'openclose_thread', course.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] threads = [utils.prepare_content(thread, course_key, is_staff) for thread in threads]
with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
add_courseware_context(threads, course, request.user) add_courseware_context(threads, course)
return utils.JsonResponse({ return utils.JsonResponse({
'is_commentable_cohorted': is_commentable_cohorted(course_key, discussion_id), 'is_commentable_cohorted': is_commentable_cohorted(course_key, discussion_id),
'discussion_data': threads, 'discussion_data': threads,
...@@ -188,7 +181,7 @@ def inline_discussion(request, course_key, discussion_id): ...@@ -188,7 +181,7 @@ def inline_discussion(request, course_key, discussion_id):
'page': query_params['page'], 'page': query_params['page'],
'num_pages': query_params['num_pages'], 'num_pages': query_params['num_pages'],
'roles': utils.get_role_ids(course_key), '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): ...@@ -201,13 +194,13 @@ def forum_form_discussion(request, course_key):
nr_transaction = newrelic.agent.current_transaction() nr_transaction = newrelic.agent.current_transaction()
course = get_course_with_access(request.user, 'load_forum', course_key, check_if_enrolled=True) 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 = cc.User.from_django_user(request.user)
user_info = user.to_dict() user_info = user.to_dict()
try: 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) is_staff = cached_has_permission(request.user, 'openclose_thread', course.id)
threads = [utils.prepare_content(thread, course_key, is_staff) for thread in unsafethreads] threads = [utils.prepare_content(thread, course_key, is_staff) for thread in unsafethreads]
except cc.utils.CommentClientMaintenanceError: except cc.utils.CommentClientMaintenanceError:
...@@ -220,7 +213,7 @@ def forum_form_discussion(request, course_key): ...@@ -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) annotated_content_info = utils.get_metadata_for_threads(course_key, threads, request.user, user_info)
with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): 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(): if request.is_ajax():
return utils.JsonResponse({ return utils.JsonResponse({
...@@ -265,21 +258,15 @@ def single_thread(request, course_key, discussion_id, thread_id): ...@@ -265,21 +258,15 @@ def single_thread(request, course_key, discussion_id, thread_id):
""" """
Renders a response to display a single discussion thread. Renders a response to display a single discussion thread.
""" """
nr_transaction = newrelic.agent.current_transaction() nr_transaction = newrelic.agent.current_transaction()
course = get_course_with_access(request.user, 'load_forum', course_key) 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) cc_user = cc.User.from_django_user(request.user)
user_info = cc_user.to_dict() user_info = cc_user.to_dict()
is_moderator = cached_has_permission(request.user, "see_all_cohorts", course_key) 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 # 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 # page; it would be a nice optimization to avoid that extra round trip to
# the comments service. # the comments service.
...@@ -307,7 +294,7 @@ def single_thread(request, course_key, discussion_id, thread_id): ...@@ -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) 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) content = utils.prepare_content(thread.to_dict(), course_key, is_staff)
with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"):
add_courseware_context([content], course, request.user) add_courseware_context([content], course)
return utils.JsonResponse({ return utils.JsonResponse({
'content': content, 'content': content,
'annotated_content_info': annotated_content_info, 'annotated_content_info': annotated_content_info,
...@@ -315,13 +302,13 @@ def single_thread(request, course_key, discussion_id, thread_id): ...@@ -315,13 +302,13 @@ def single_thread(request, course_key, discussion_id, thread_id):
else: else:
try: try:
threads, query_params = get_threads(request, course) threads, query_params = get_threads(request, course_key)
except ValueError: except ValueError:
return HttpResponseBadRequest("Invalid group_id") return HttpResponseBadRequest("Invalid group_id")
threads.append(thread.to_dict()) threads.append(thread.to_dict())
with newrelic.agent.FunctionTrace(nr_transaction, "add_courseware_context"): 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: for thread in threads:
# patch for backward compatibility with comments service # patch for backward compatibility with comments service
......
lms/djangoapps/django_comment_client/tests/test_utils.py
...@@ -10,7 +10,6 @@ import mock ...@@ -10,7 +10,6 @@ import mock
from django_comment_client.tests.factories import RoleFactory from django_comment_client.tests.factories import RoleFactory
from django_comment_client.tests.unicode import UnicodeTestMixin from django_comment_client.tests.unicode import UnicodeTestMixin
from django_comment_client.tests.utils import ContentGroupTestCase
import django_comment_client.utils as utils import django_comment_client.utils as utils
from student.tests.factories import UserFactory, CourseEnrollmentFactory from student.tests.factories import UserFactory, CourseEnrollmentFactory
from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory
...@@ -89,7 +88,7 @@ class CoursewareContextTestCase(ModuleStoreTestCase): ...@@ -89,7 +88,7 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
comment client service integration comment client service integration
""" """
def setUp(self): 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.course = CourseFactory.create(org="TestX", number="101", display_name="Test Course")
self.discussion1 = ItemFactory.create( self.discussion1 = ItemFactory.create(
...@@ -108,12 +107,12 @@ class CoursewareContextTestCase(ModuleStoreTestCase): ...@@ -108,12 +107,12 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
) )
def test_empty(self): def test_empty(self):
utils.add_courseware_context([], self.course, self.user) utils.add_courseware_context([], self.course)
def test_missing_commentable_id(self): def test_missing_commentable_id(self):
orig = {"commentable_id": "non-inline"} orig = {"commentable_id": "non-inline"}
modified = dict(orig) modified = dict(orig)
utils.add_courseware_context([modified], self.course, self.user) utils.add_courseware_context([modified], self.course)
self.assertEqual(modified, orig) self.assertEqual(modified, orig)
def test_basic(self): def test_basic(self):
...@@ -121,7 +120,7 @@ class CoursewareContextTestCase(ModuleStoreTestCase): ...@@ -121,7 +120,7 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
{"commentable_id": self.discussion1.discussion_id}, {"commentable_id": self.discussion1.discussion_id},
{"commentable_id": self.discussion2.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 def assertThreadCorrect(thread, discussion, expected_title): # pylint: disable=invalid-name
"""Asserts that the given thread has the expected set of properties""" """Asserts that the given thread has the expected set of properties"""
...@@ -145,29 +144,13 @@ class CoursewareContextTestCase(ModuleStoreTestCase): ...@@ -145,29 +144,13 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
assertThreadCorrect(threads[1], self.discussion2, "Subsection / Discussion 2") assertThreadCorrect(threads[1], self.discussion2, "Subsection / Discussion 2")
class CategoryMapTestMixin(object): class CategoryMapTestCase(ModuleStoreTestCase):
"""
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):
""" """
Base testcase class for discussion categories for the Base testcase class for discussion categories for the
comment client service integration comment client service integration
""" """
def setUp(self): def setUp(self):
super(CategoryMapTestCase, self).setUp(create_user=True) super(CategoryMapTestCase, self).setUp()
self.course = CourseFactory.create( self.course = CourseFactory.create(
org="TestX", number="101", display_name="Test Course", org="TestX", number="101", display_name="Test Course",
...@@ -193,8 +176,20 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -193,8 +176,20 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
**kwargs **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): 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): def test_configured_topics(self):
self.course.discussion_topics = { self.course.discussion_topics = {
...@@ -203,8 +198,8 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -203,8 +198,8 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
"Topic C": {"id": "Topic_C"} "Topic C": {"id": "Topic_C"}
} }
def check_cohorted_topics(expected_ids): def check_cohorted_topics(expected_ids): # pylint: disable=missing-docstring
self.assert_category_map_equals( self.assertCategoryMapEquals(
{ {
"entries": { "entries": {
"Topic A": {"id": "Topic_A", "sort_key": "Topic A", "is_cohorted": "Topic_A" in expected_ids}, "Topic A": {"id": "Topic_A", "sort_key": "Topic A", "is_cohorted": "Topic_A" in expected_ids},
...@@ -236,7 +231,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -236,7 +231,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
def test_single_inline(self): def test_single_inline(self):
self.create_discussion("Chapter", "Discussion") self.create_discussion("Chapter", "Discussion")
self.assert_category_map_equals( self.assertCategoryMapEquals(
{ {
"entries": {}, "entries": {},
"subcategories": { "subcategories": {
...@@ -266,7 +261,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -266,7 +261,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
def check_cohorted(is_cohorted): def check_cohorted(is_cohorted):
self.assert_category_map_equals( self.assertCategoryMapEquals(
{ {
"entries": {}, "entries": {},
"subcategories": { "subcategories": {
...@@ -369,7 +364,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -369,7 +364,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion", start=later) self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion", start=later)
self.create_discussion("Chapter 3 / Section 1", "Discussion", start=later) self.create_discussion("Chapter 3 / Section 1", "Discussion", start=later)
self.assert_category_map_equals( self.assertCategoryMapEquals(
{ {
"entries": {}, "entries": {},
"subcategories": { "subcategories": {
...@@ -407,7 +402,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -407,7 +402,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
self.create_discussion("Chapter", "Discussion 4", sort_key="C") self.create_discussion("Chapter", "Discussion 4", sort_key="C")
self.create_discussion("Chapter", "Discussion 5", sort_key="B") self.create_discussion("Chapter", "Discussion 5", sort_key="B")
self.assert_category_map_equals( self.assertCategoryMapEquals(
{ {
"entries": {}, "entries": {},
"subcategories": { "subcategories": {
...@@ -459,7 +454,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -459,7 +454,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
"Topic B": {"id": "Topic_B", "sort_key": "C"}, "Topic B": {"id": "Topic_B", "sort_key": "C"},
"Topic C": {"id": "Topic_C", "sort_key": "A"} "Topic C": {"id": "Topic_C", "sort_key": "A"}
} }
self.assert_category_map_equals( self.assertCategoryMapEquals(
{ {
"entries": { "entries": {
"Topic A": {"id": "Topic_A", "sort_key": "B", "is_cohorted": False}, "Topic A": {"id": "Topic_A", "sort_key": "B", "is_cohorted": False},
...@@ -480,7 +475,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -480,7 +475,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
self.create_discussion("Chapter", "Discussion C") self.create_discussion("Chapter", "Discussion C")
self.create_discussion("Chapter", "Discussion B") self.create_discussion("Chapter", "Discussion B")
self.assert_category_map_equals( self.assertCategoryMapEquals(
{ {
"entries": {}, "entries": {},
"subcategories": { "subcategories": {
...@@ -533,7 +528,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -533,7 +528,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
self.create_discussion("Chapter B", "Discussion 1") self.create_discussion("Chapter B", "Discussion 1")
self.create_discussion("Chapter A", "Discussion 2") self.create_discussion("Chapter A", "Discussion 2")
self.assert_category_map_equals( self.assertCategoryMapEquals(
{ {
"entries": {}, "entries": {},
"subcategories": { "subcategories": {
...@@ -586,7 +581,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -586,7 +581,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
) )
def test_ids_empty(self): 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): def test_ids_configured_topics(self):
self.course.discussion_topics = { self.course.discussion_topics = {
...@@ -595,7 +590,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -595,7 +590,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
"Topic C": {"id": "Topic_C"} "Topic C": {"id": "Topic_C"}
} }
self.assertItemsEqual( self.assertItemsEqual(
utils.get_discussion_categories_ids(self.course, self.user), utils.get_discussion_categories_ids(self.course),
["Topic_A", "Topic_B", "Topic_C"] ["Topic_A", "Topic_B", "Topic_C"]
) )
...@@ -607,7 +602,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -607,7 +602,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion") self.create_discussion("Chapter 2 / Section 1 / Subsection 2", "Discussion")
self.create_discussion("Chapter 3 / Section 1", "Discussion") self.create_discussion("Chapter 3 / Section 1", "Discussion")
self.assertItemsEqual( self.assertItemsEqual(
utils.get_discussion_categories_ids(self.course, self.user), utils.get_discussion_categories_ids(self.course),
["discussion1", "discussion2", "discussion3", "discussion4", "discussion5", "discussion6"] ["discussion1", "discussion2", "discussion3", "discussion4", "discussion5", "discussion6"]
) )
...@@ -621,177 +616,11 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase): ...@@ -621,177 +616,11 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
self.create_discussion("Chapter 2", "Discussion") self.create_discussion("Chapter 2", "Discussion")
self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion") self.create_discussion("Chapter 2 / Section 1 / Subsection 1", "Discussion")
self.assertItemsEqual( 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"] ["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): class JsonResponseTestCase(TestCase, UnicodeTestMixin):
def _test_unicode_data(self, text): def _test_unicode_data(self, text):
response = utils.JsonResponse(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 mock import patch
from pytz import UTC
from openedx.core.djangoapps.course_groups.models import CourseUserGroupPartitionGroup from openedx.core.djangoapps.course_groups.models import CourseUserGroup
from openedx.core.djangoapps.course_groups.tests.helpers import CohortFactory
from django_comment_common.models import Role from django_comment_common.models import Role
from django_comment_common.utils import seed_permissions_roles from django_comment_common.utils import seed_permissions_roles
from student.tests.factories import CourseEnrollmentFactory, UserFactory 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.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. Sets up a course with a student, a moderator and their cohorts.
""" """
@patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True})
def setUp(self): def setUp(self):
super(CohortedTestCase, self).setUp() super(CohortedContentTestCase, self).setUp()
self.course = CourseFactory.create( self.course = CourseFactory.create(
discussion_topics={ discussion_topics={
...@@ -34,13 +26,15 @@ class CohortedTestCase(ModuleStoreTestCase): ...@@ -34,13 +26,15 @@ class CohortedTestCase(ModuleStoreTestCase):
"cohorted_discussions": ["cohorted_topic"] "cohorted_discussions": ["cohorted_topic"]
} }
) )
self.student_cohort = CohortFactory.create( self.student_cohort = CourseUserGroup.objects.create(
name="student_cohort", 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", name="moderator_cohort",
course_id=self.course.id course_id=self.course.id,
group_type=CourseUserGroup.COHORT
) )
seed_permissions_roles(self.course.id) seed_permissions_roles(self.course.id)
...@@ -51,82 +45,3 @@ class CohortedTestCase(ModuleStoreTestCase): ...@@ -51,82 +45,3 @@ class CohortedTestCase(ModuleStoreTestCase):
self.moderator.roles.add(Role.objects.get(name="Moderator", course_id=self.course.id)) self.moderator.roles.add(Role.objects.get(name="Moderator", course_id=self.course.id))
self.student_cohort.users.add(self.student) self.student_cohort.users.add(self.student)
self.moderator_cohort.users.add(self.moderator) 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 ...@@ -17,7 +17,7 @@ from xmodule.modulestore.django import modulestore
from django_comment_common.models import Role, FORUM_ROLE_STUDENT from django_comment_common.models import Role, FORUM_ROLE_STUDENT
from django_comment_client.permissions import check_permissions_by_view, cached_has_permission from django_comment_client.permissions import check_permissions_by_view, cached_has_permission
from edxmako import lookup_template 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, \ from openedx.core.djangoapps.course_groups.cohorts import get_cohort_by_id, get_cohort_id, is_commentable_cohorted, \
is_course_cohorted is_course_cohorted
from openedx.core.djangoapps.course_groups.models import CourseUserGroup from openedx.core.djangoapps.course_groups.models import CourseUserGroup
...@@ -59,11 +59,9 @@ def has_forum_access(uname, course_id, rolename): ...@@ -59,11 +59,9 @@ def has_forum_access(uname, course_id, rolename):
return role.users.filter(username=uname).exists() return role.users.filter(username=uname).exists()
# pylint: disable=invalid-name def _get_discussion_modules(course):
def get_accessible_discussion_modules(course, user):
""" """
Get all discussion modules within a course which are accessible to Return a list of all valid discussion modules in this course.
the user.
""" """
all_modules = modulestore().get_items(course.id, qualifiers={'category': 'discussion'}) all_modules = modulestore().get_items(course.id, qualifiers={'category': 'discussion'})
...@@ -74,23 +72,20 @@ def get_accessible_discussion_modules(course, user): ...@@ -74,23 +72,20 @@ def get_accessible_discussion_modules(course, user):
return False return False
return True return True
return [ return filter(has_required_keys, all_modules)
module for module in all_modules
if has_required_keys(module) and has_access(user, 'load', module, course.id)
]
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 discussion_id = module.discussion_id
title = module.discussion_target title = module.discussion_target
last_category = module.discussion_category.split("/")[-1].strip() last_category = module.discussion_category.split("/")[-1].strip()
return (discussion_id, {"location": module.location, "title": last_category + " / " + title}) 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): def _filter_unstarted_categories(category_map):
...@@ -143,14 +138,14 @@ def _sort_map_entries(category_map, sort_alpha): ...@@ -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"])] 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 Transform the list of this course's discussion modules into a recursive dictionary structure. This is used
the user within a course. to render the discussion category map in the discussion tab sidebar.
""" """
unexpanded_category_map = defaultdict(list) unexpanded_category_map = defaultdict(list)
modules = get_accessible_discussion_modules(course, user) modules = _get_discussion_modules(course)
is_course_cohorted = course.is_cohorted is_course_cohorted = course.is_cohorted
cohorted_discussion_ids = course.cohorted_discussions cohorted_discussion_ids = course.cohorted_discussions
...@@ -216,12 +211,12 @@ def get_discussion_category_map(course, user): ...@@ -216,12 +211,12 @@ def get_discussion_category_map(course, user):
return _filter_unstarted_categories(category_map) 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. Returns a list of available ids of categories for the course.
""" """
ids = [] ids = []
queue = [get_discussion_category_map(course, user)] queue = [get_discussion_category_map(course)]
while queue: while queue:
category_map = queue.pop() category_map = queue.pop()
for child in category_map["children"]: for child in category_map["children"]:
...@@ -380,12 +375,12 @@ def extend_content(content): ...@@ -380,12 +375,12 @@ def extend_content(content):
return merge_dict(content, content_info) 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. Decorates `content_list` with courseware metadata.
""" """
if id_map is None: 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: for content in content_list:
commentable_id = content['commentable_id'] commentable_id = content['commentable_id']
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment