Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #12582 from open-craft/feature/discussion_xblock

Browse Source 
Convert Discussion Module to XBlock
This commit is contained in:
Jillian Vogel
2016-06-29 20:49:35 +12:00
committed by GitHub
parent 115c2ced4f 33b3dfcc92
commit e19fb06e9d
29 changed files with 806 additions and 434 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
cms/djangoapps/contentstore/features/component_settings_editor_helpers.py
View File

@@ -27,6 +27,8 @@ def create_component_instance(step, category, component_type=None, is_advanced=F
module_css = 'div.xmodule_CapaModule'
elif category == 'advanced':
module_css = 'div.xmodule_{}Module'.format(advanced_component.title())
elif category == 'discussion':
module_css = 'div.xblock-author_view-{}'.format(category.lower())
else:
module_css = 'div.xmodule_{}Module'.format(category.title())
@@ -168,7 +170,9 @@ def verify_setting_entry(setting, display_name, value, explicitly_set):
for the problem, rather than derived from the defaults. This is verified
by the existence of a "Clear" button next to the field value.
"""
assert_equal(display_name, setting.find_by_css('.setting-label')[0].html.strip())
label_element = setting.find_by_css('.setting-label')[0]
assert_equal(display_name, label_element.html.strip())
label_for = label_element['for']
# Check if the web object is a list type
# If so, we use a slightly different mechanism for determining its value
@@ -179,7 +183,7 @@ def verify_setting_entry(setting, display_name, value, explicitly_set):
list_value = ', '.join(ele.find_by_css('input')[0].value for ele in setting.find_by_css('.videolist-settings-item'))
assert_equal(value, list_value)
else:
assert_equal(value, setting.find_by_css('.setting-input')[0].value)
assert_equal(value, setting.find_by_id(label_for).value)
# VideoList doesn't have clear button
if not setting.has_class('metadata-videolist-enum'):
@@ -201,7 +205,7 @@ def verify_all_setting_entries(expected_entries):
@world.absorb
def save_component():
world.css_click("a.action-save")
world.css_click("a.action-save,a.save-button")
world.wait_for_ajax_complete()
@@ -241,7 +245,7 @@ def get_setting_entry(label):
@world.absorb
def get_setting_entry_index(label):
def get_index():
settings = world.css_find('.metadata_edit .wrapper-comp-setting')
settings = world.css_find('.wrapper-comp-setting')
for index, setting in enumerate(settings):
if setting.find_by_css('.setting-label')[0].value == label:
return index
@@ -259,6 +263,6 @@ def set_field_value(index, value):
Instead we will find the element, set its value, then hit the Tab key
to get to the next field.
"""
elem = world.css_find('.metadata_edit div.wrapper-comp-setting input.setting-input')[index]
elem = world.css_find('div.wrapper-comp-setting input')[index]
elem.value = value
elem.type(Keys.TAB)

2
cms/djangoapps/contentstore/features/discussion-editor.feature
View File

@@ -5,7 +5,7 @@ Feature: CMS.Discussion Component Editor
Scenario: User can view discussion component metadata
Given I have created a Discussion Tag
And I edit the component
Then I see three alphabetized settings and their expected values
Then I see three settings and their expected values
# Safari doesn't save the name properly
@skip_safari

4
cms/djangoapps/contentstore/features/discussion-editor.py
View File

@@ -13,12 +13,12 @@ def i_created_discussion_tag(step):
)
@step('I see three alphabetized settings and their expected values$')
@step('I see three settings and their expected values$')
def i_see_only_the_settings_and_values(step):
world.verify_all_setting_entries(
[
['Category', "Week 1", False],
['Display Name', "Discussion", False],
['Category', "Week 1", False],
['Subcategory', "Topic-Level Student-Visible Label", False]
])

1
common/lib/xmodule/setup.py
View File

@@ -23,7 +23,6 @@ XMODULES = [
"videoalpha = xmodule.video_module:VideoDescriptor",
"videodev = xmodule.backcompat_module:TranslateCustomTagDescriptor",
"videosequence = xmodule.seq_module:SequenceDescriptor",
"discussion = xmodule.discussion_module:DiscussionDescriptor",
"course_info = xmodule.html_module:CourseInfoDescriptor",
"static_tab = xmodule.html_module:StaticTabDescriptor",
"custom_tag_template = xmodule.raw_module:RawDescriptor",

132
common/lib/xmodule/xmodule/discussion_module.py
View File

@@ -1,132 +0,0 @@
"""
Definition of the Discussion module.
"""
import json
from pkg_resources import resource_string
from xblock.core import XBlock
from xmodule.x_module import XModule
from xmodule.raw_module import RawDescriptor
from xmodule.editing_module import MetadataOnlyEditingDescriptor
from xblock.fields import String, Scope, UNIQUE_ID
# Make '_' a no-op so we can scrape strings. Using lambda instead of
# `django.utils.translation.ugettext_noop` because Django cannot be imported in this file
_ = lambda text: text
class DiscussionFields(object):
discussion_id = String(
display_name=_("Discussion Id"),
help=_("The id is a unique identifier for the discussion. It is non editable."),
scope=Scope.settings,
default=UNIQUE_ID)
display_name = String(
display_name=_("Display Name"),
help=_("Display name for this module"),
default="Discussion",
scope=Scope.settings
)
data = String(
help=_("XML data for the problem"),
scope=Scope.content,
default="<discussion></discussion>"
)
discussion_category = String(
display_name=_("Category"),
default="Week 1",
help=_("A category name for the discussion. This name appears in the left pane of the discussion forum for the course."),
scope=Scope.settings
)
discussion_target = String(
display_name=_("Subcategory"),
default="Topic-Level Student-Visible Label",
help=_("A subcategory name for the discussion. This name appears in the left pane of the discussion forum for the course."),
scope=Scope.settings
)
sort_key = String(scope=Scope.settings)
def has_permission(user, permission, course_id):
"""
Copied from django_comment_client/permissions.py because I can't import
that file from here. It causes the xmodule_assets command to fail.
"""
return any(role.has_permission(permission)
for role in user.roles.filter(course_id=course_id))
@XBlock.wants('user')
class DiscussionModule(DiscussionFields, XModule):
"""
XModule for discussion forums.
"""
js = {
'coffee': [
resource_string(__name__, 'js/src/discussion/display.coffee')
],
'js': [
resource_string(__name__, 'js/src/time.js')
]
}
js_module_name = "InlineDiscussion"
def get_html(self):
course = self.get_course()
user = None
user_service = self.runtime.service(self, 'user')
if user_service:
user = user_service._django_user # pylint: disable=protected-access
if user:
course_key = course.id
can_create_comment = has_permission(user, "create_comment", course_key)
can_create_subcomment = has_permission(user, "create_sub_comment", course_key)
can_create_thread = has_permission(user, "create_thread", course_key)
else:
can_create_comment = False
can_create_subcomment = False
can_create_thread = False
context = {
'discussion_id': self.discussion_id,
'course': course,
'can_create_comment': json.dumps(can_create_comment),
'can_create_subcomment': json.dumps(can_create_subcomment),
'can_create_thread': can_create_thread,
}
if getattr(self.system, 'is_author_mode', False):
template = 'discussion/_discussion_module_studio.html'
else:
template = 'discussion/_discussion_module.html'
return self.system.render_template(template, context)
def get_course(self):
"""
Return CourseDescriptor by course id.
"""
course = self.runtime.modulestore.get_course(self.course_id)
return course
class DiscussionDescriptor(DiscussionFields, MetadataOnlyEditingDescriptor, RawDescriptor):
module_class = DiscussionModule
resources_dir = None
# The discussion XML format uses `id` and `for` attributes,
# but these would overload other module attributes, so we prefix them
# for actual use in the code
metadata_translations = dict(RawDescriptor.metadata_translations)
metadata_translations['id'] = 'discussion_id'
metadata_translations['for'] = 'discussion_target'
@property
def non_editable_metadata_fields(self):
non_editable_fields = super(DiscussionDescriptor, self).non_editable_metadata_fields
# We may choose to enable sort_keys in the future, but while Kevin is investigating....
non_editable_fields.extend([DiscussionDescriptor.discussion_id, DiscussionDescriptor.sort_key])
return non_editable_fields
def student_view_data(self):
"""
Returns a JSON representation of the student_view of this XModule.
"""
return {'topic_id': self.discussion_id}

2
common/lib/xmodule/xmodule/tests/test_xblock_wrappers.py
View File

@@ -31,7 +31,6 @@ from xmodule.x_module import ModuleSystem, XModule, XModuleDescriptor, Descripto
from xmodule.annotatable_module import AnnotatableDescriptor
from xmodule.capa_module import CapaDescriptor
from xmodule.course_module import CourseDescriptor
from xmodule.discussion_module import DiscussionDescriptor
from xmodule.html_module import HtmlDescriptor
from xmodule.poll_module import PollDescriptor
from xmodule.word_cloud_module import WordCloudDescriptor
@@ -50,7 +49,6 @@ from xmodule.tests import get_test_descriptor_system, get_test_system
LEAF_XMODULES = {
AnnotatableDescriptor: [{}],
CapaDescriptor: [{}],
DiscussionDescriptor: [{}],
HtmlDescriptor: [{}],
PollDescriptor: [{'display_name': 'Poll Display Name'}],
WordCloudDescriptor: [{}],

3
common/lib/xmodule/xmodule/video_module/video_module.py
View File

@@ -112,6 +112,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
#TODO: For each of the following, ensure that any generated html is properly escaped.
js = {
'js': [
resource_string(module, 'js/src/time.js'),
resource_string(module, 'js/src/video/00_component.js'),
resource_string(module, 'js/src/video/00_video_storage.js'),
resource_string(module, 'js/src/video/00_resizer.js'),
@@ -350,7 +351,7 @@ class VideoModule(VideoFields, VideoTranscriptsMixin, VideoStudentViewHandlers,
'cdn_eval': cdn_eval,
'cdn_exp_group': cdn_exp_group,
'id': self.location.html_id(),
'display_name': self.display_name_with_default_escaped,
'display_name': self.display_name_with_default,
'handout': self.handout,
'download_video_link': download_video_link,
'track': track_url,

24
common/test/acceptance/tests/discussion/test_discussion.py
View File

@@ -966,22 +966,22 @@ class InlineDiscussionTest(UniqueCourseTest, DiscussionResponsePaginationTestMix
self.assertFalse(self.thread_page.is_comment_deletable("comment1"))
self.assertFalse(self.thread_page.is_comment_deletable("comment2"))
def test_dual_discussion_module(self):
def test_dual_discussion_xblock(self):
"""
Scenario: Two discussion module in one unit shouldn't override their actions
Scenario: Two discussion xblocks in one unit shouldn't override their actions
Given that I'm on courseware page where there are two inline discussion
When I click on one discussion module new post button
Then it should add new post form of that module in DOM
And I should be shown new post form of that module
And I shouldn't be shown second discussion module new post form
And I click on second discussion module new post button
Then it should add new post form of second module in DOM
When I click on one discussion xblock new post button
Then it should add new post form of that xblock in DOM
And I should be shown new post form of that xblock
And I shouldn't be shown second discussion xblock new post form
And I click on second discussion xblock new post button
Then it should add new post form of second xblock in DOM
And I should be shown second discussion new post form
And I shouldn't be shown first discussion module new post form
And I shouldn't be shown first discussion xblock new post form
And I have two new post form in the DOM
When I click back on first module new post button
And I should be shown new post form of that module
And I shouldn't be shown second discussion module new post form
When I click back on first xblock new post button
And I should be shown new post form of that xblock
And I shouldn't be shown second discussion xblock new post form
"""
self.discussion_page.wait_for_page()
self.additional_discussion_page.wait_for_page()

5
lms/djangoapps/courseware/management/commands/dump_course_structure.py
View File

@@ -22,9 +22,10 @@ from textwrap import dedent
from django.core.management.base import BaseCommand, CommandError
from xmodule.discussion_module import DiscussionDescriptor
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.inheritance import own_metadata, compute_inherited_metadata
from xblock_discussion import DiscussionXBlock
from xblock.fields import Scope
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
@@ -97,7 +98,7 @@ def dump_module(module, destination=None, inherited=False, defaults=False):
items = own_metadata(module)
# HACK: add discussion ids to list of items to export (AN-6696)
if isinstance(module, DiscussionDescriptor) and 'discussion_id' not in items:
if isinstance(module, DiscussionXBlock) and 'discussion_id' not in items:
items['discussion_id'] = module.discussion_id
filtered_metadata = {k: v for k, v in items.iteritems() if k not in FILTER_LIST}

131
lms/djangoapps/courseware/tests/test_discussion_module.py
View File

@@ -1,131 +0,0 @@
# -*- coding: utf-8 -*-
"""Test for Discussion Xmodule functional logic."""
import ddt
from django.core.urlresolvers import reverse
from mock import Mock
from . import BaseTestXmodule
from course_api.blocks.tests.helpers import deserialize_usage_key
from courseware.module_render import get_module_for_descriptor_internal
from student.tests.factories import UserFactory, CourseEnrollmentFactory
from xmodule.discussion_module import DiscussionModule
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.django import modulestore
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from xmodule.modulestore.tests.factories import ToyCourseFactory, ItemFactory
@ddt.ddt
class DiscussionModuleTest(BaseTestXmodule, SharedModuleStoreTestCase):
"""Logic tests for Discussion Xmodule."""
CATEGORY = "discussion"
def test_html_with_user(self):
discussion = get_module_for_descriptor_internal(
user=self.users[0],
descriptor=self.item_descriptor,
student_data=Mock(name='student_data'),
course_id=self.course.id,
track_function=Mock(name='track_function'),
xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'),
request_token='request_token',
)
fragment = discussion.render('student_view')
html = fragment.content
self.assertIn('data-user-create-comment="false"', html)
self.assertIn('data-user-create-subcomment="false"', html)
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_discussion_render_successfully_with_orphan_parent(self, default_store):
"""
Test that discussion module render successfully
if discussion module is child of an orphan.
"""
user = UserFactory.create()
store = modulestore()
with store.default_store(default_store):
course = store.create_course('testX', 'orphan', '123X', user.id)
orphan_sequential = store.create_item(self.user.id, course.id, 'sequential')
vertical = store.create_child(
user.id,
orphan_sequential.location,
'vertical',
block_id=course.location.block_id
)
discussion = store.create_child(
user.id,
vertical.location,
'discussion',
block_id=course.location.block_id
)
discussion = store.get_item(discussion.location)
root = self.get_root(discussion)
# Assert that orphan sequential is root of the discussion module.
self.assertEqual(orphan_sequential.location.block_type, root.location.block_type)
self.assertEqual(orphan_sequential.location.block_id, root.location.block_id)
# Get module system bound to a user and a descriptor.
discussion_module = get_module_for_descriptor_internal(
user=user,
descriptor=discussion,
student_data=Mock(name='student_data'),
course_id=course.id,
track_function=Mock(name='track_function'),
xqueue_callback_url_prefix=Mock(name='xqueue_callback_url_prefix'),
request_token='request_token',
)
fragment = discussion_module.render('student_view')
html = fragment.content
self.assertIsInstance(discussion_module._xmodule, DiscussionModule) # pylint: disable=protected-access
self.assertIn('data-user-create-comment="false"', html)
self.assertIn('data-user-create-subcomment="false"', html)
def get_root(self, block):
"""
Return root of the block.
"""
while block.parent:
block = block.get_parent()
return block
def test_discussion_student_view_data(self):
"""
Tests that course block api returns student_view_data for discussion module
"""
course_key = ToyCourseFactory.create().id
course_usage_key = self.store.make_course_usage_key(course_key)
user = UserFactory.create()
self.client.login(username=user.username, password='test')
CourseEnrollmentFactory.create(user=user, course_id=course_key)
discussion_id = "test_discussion_module_id"
ItemFactory.create(
parent_location=course_usage_key,
category='discussion',
discussion_id=discussion_id,
discussion_category='Category discussion',
discussion_target='Target Discussion',
)
url = reverse('blocks_in_block_tree', kwargs={'usage_key_string': unicode(course_usage_key)})
query_params = {
'depth': 'all',
'username': user.username,
'block_types_filter': 'discussion',
'student_view_data': 'discussion'
}
response = self.client.get(url, query_params)
self.assertEquals(response.status_code, 200)
self.assertEquals(response.data['root'], unicode(course_usage_key)) # pylint: disable=no-member
for block_key_string, block_data in response.data['blocks'].iteritems(): # pylint: disable=no-member
block_key = deserialize_usage_key(block_key_string, course_key)
self.assertEquals(block_data['id'], block_key_string)
self.assertEquals(block_data['type'], block_key.block_type)
self.assertEquals(block_data['display_name'], self.store.get_item(block_key).display_name or '')
self.assertEqual(block_data['student_view_data'], {"topic_id": discussion_id})

426
lms/djangoapps/courseware/tests/test_discussion_xblock.py Normal file
View File

@@ -0,0 +1,426 @@
"""
Tests for the discussion xblock.
Most of the tests are in common/xblock/xblock_discussion, here are only
tests for functionalities that require django API, and lms specific
functionalities.
"""
import uuid
import ddt
import json
import mock
from django.core.urlresolvers import reverse
from course_api.blocks.tests.helpers import deserialize_usage_key
from courseware.module_render import get_module_for_descriptor_internal
from student.tests.factories import UserFactory, CourseEnrollmentFactory
from xblock.field_data import DictFieldData
from xblock.fragment import Fragment
from xmodule.modulestore import ModuleStoreEnum
from xmodule.modulestore.tests.factories import ToyCourseFactory, ItemFactory
from xmodule.modulestore.tests.django_utils import SharedModuleStoreTestCase
from lms.djangoapps.courseware.tests import XModuleRenderingTestBase
from xblock_discussion import DiscussionXBlock, loader
@ddt.ddt
class TestDiscussionXBlock(XModuleRenderingTestBase):
"""
Base class for tests
"""
PATCH_DJANGO_USER = True
def setUp(self):
"""
Set up the xblock runtime, test course, discussion, and user.
"""
super(TestDiscussionXBlock, self).setUp()
self.patchers = []
self.course_id = "test_course"
self.runtime = self.new_module_runtime()
self.runtime.modulestore = mock.Mock()
self.discussion_id = str(uuid.uuid4())
self.data = DictFieldData({
'discussion_id': self.discussion_id
})
scope_ids = mock.Mock()
scope_ids.usage_id.course_key = self.course_id
self.block = DiscussionXBlock(
self.runtime,
field_data=self.data,
scope_ids=scope_ids
)
self.block.xmodule_runtime = mock.Mock()
if self.PATCH_DJANGO_USER:
self.django_user_canary = object()
self.django_user_mock = self.add_patcher(
mock.patch.object(DiscussionXBlock, "django_user", new_callable=mock.PropertyMock)
)
self.django_user_mock.return_value = self.django_user_canary
def add_patcher(self, patcher):
"""
Registers a patcher object, and returns mock. This patcher will be disabled after the test.
"""
self.patchers.append(patcher)
return patcher.start()
def tearDown(self):
"""
Tears down any patchers added during tests.
"""
super(TestDiscussionXBlock, self).tearDown()
for patcher in self.patchers:
patcher.stop()
class TestGetDjangoUser(TestDiscussionXBlock):
"""
Tests for the django_user property.
"""
PATCH_DJANGO_USER = False
def setUp(self):
"""
Mock the user service and runtime.
"""
super(TestGetDjangoUser, self).setUp()
self.django_user = object()
self.user_service = mock.Mock()
self.add_patcher(
mock.patch.object(self.runtime, "service", return_value=self.user_service)
)
self.user_service._django_user = self.django_user # pylint: disable=protected-access
def test_django_user(self):
"""
Tests that django_user users returns _django_user attribute
of the user service.
"""
actual_user = self.block.django_user
self.runtime.service.assert_called_once_with(
self.block, 'user')
self.assertEqual(actual_user, self.django_user)
def test_django_user_handles_missing_service(self):
"""
Tests that get_django gracefully handles missing user service.
"""
self.runtime.service.return_value = None
self.assertEqual(self.block.django_user, None)
@ddt.ddt
class TestViews(TestDiscussionXBlock):
"""
Tests for student_view and author_view.
"""
def setUp(self):
"""
Mock the methods needed for these tests.
"""
super(TestViews, self).setUp()
self.template_canary = u'canary'
self.render_template = mock.Mock()
self.render_template.return_value = self.template_canary
self.block.runtime.render_template = self.render_template
self.has_permission_mock = mock.Mock()
self.has_permission_mock.return_value = False
self.block.has_permission = self.has_permission_mock
def get_template_context(self):
"""
Returns context passed to rendering of the django template
(rendered by runtime).
"""
self.assertEqual(self.render_template.call_count, 1)
return self.render_template.call_args_list[0][0][1]
def get_rendered_template(self):
"""
Returns the name of the template rendered by runtime.
"""
self.assertEqual(self.render_template.call_count, 1)
return self.render_template.call_args_list[0][0][0]
def test_studio_view(self):
"""
Test for the studio view.
"""
fragment = self.block.author_view()
self.assertIsInstance(fragment, Fragment)
self.assertEqual(fragment.content, self.template_canary)
self.render_template.assert_called_once_with(
'discussion/_discussion_inline_studio.html',
{'discussion_id': self.discussion_id}
)
@ddt.data(
(False, False, False),
(True, False, False),
(False, True, False),
(False, False, True),
)
def test_student_perms_are_correct(self, permissions):
"""
Test that context will get proper permissions.
"""
permission_dict = {
'create_thread': permissions[0],
'create_comment': permissions[1],
'create_subcomment': permissions[2]
}
expected_permissions = {
'can_create_thread': permission_dict['create_thread'],
'can_create_comment': permission_dict['create_comment'],
'can_create_subcomment': permission_dict['create_subcomment'],
}
self.block.has_permission = lambda perm: permission_dict[perm]
with mock.patch.object(loader, 'render_template', mock.Mock):
self.block.student_view()
context = self.get_template_context()
for permission_name, expected_value in expected_permissions.items():
self.assertEqual(expected_value, context[permission_name])
def test_js_init(self):
"""
Test proper js init function is called.
"""
with mock.patch.object(loader, 'render_template', mock.Mock):
fragment = self.block.student_view()
self.assertEqual(fragment.js_init_fn, 'DiscussionInlineBlock')
@ddt.ddt
class TestTemplates(TestDiscussionXBlock):
"""
Tests rendering of templates.
"""
def test_has_permission(self):
"""
Test for has_permission method.
"""
permission_canary = object()
with mock.patch('django_comment_client.permissions.has_permission', return_value=permission_canary) as has_perm:
actual_permission = self.block.has_permission("test_permission")
self.assertEqual(actual_permission, permission_canary)
has_perm.assert_called_once_with(self.django_user_canary, 'test_permission', 'test_course')
def test_studio_view(self):
"""Test for studio view."""
fragment = self.block.author_view({})
self.assertIn('data-discussion-id="{}"'.format(self.discussion_id), fragment.content)
@ddt.data(
(True, False, False),
(False, True, False),
(False, False, True),
)
def test_student_perms_are_correct(self, permissions):
"""
Test for lms view.
"""
permission_dict = {
'create_thread': permissions[0],
'create_comment': permissions[1],
'create_subcomment': permissions[2]
}
self.block.has_permission = lambda perm: permission_dict[perm]
fragment = self.block.student_view()
self.assertIn('data-discussion-id="{}"'.format(self.discussion_id), fragment.content)
self.assertIn('data-user-create-comment="{}"'.format(json.dumps(permissions[1])), fragment.content)
self.assertIn('data-user-create-subcomment="{}"'.format(json.dumps(permissions[2])), fragment.content)
if permissions[0]:
self.assertIn("Add a Post", fragment.content)
else:
self.assertNotIn("Add a Post", fragment.content)
@ddt.ddt
class TestXBlockInCourse(SharedModuleStoreTestCase):
"""
Test the discussion xblock as rendered in the course and course API.
"""
@classmethod
def setUpClass(cls):
"""
Set up a user, course, and discussion XBlock for use by tests.
"""
super(TestXBlockInCourse, cls).setUpClass()
cls.user = UserFactory.create()
cls.course = ToyCourseFactory.create()
cls.course_key = cls.course.id
cls.course_usage_key = cls.store.make_course_usage_key(cls.course_key)
cls.discussion_id = "test_discussion_xblock_id"
cls.discussion = ItemFactory.create(
parent_location=cls.course_usage_key,
category='discussion',
discussion_id=cls.discussion_id,
discussion_category='Category discussion',
discussion_target='Target Discussion',
)
CourseEnrollmentFactory.create(user=cls.user, course_id=cls.course_key)
def get_root(self, block):
"""
Return root of the block.
"""
while block.parent:
block = block.get_parent()
return block
def test_html_with_user(self):
"""
Test rendered DiscussionXBlock permissions.
"""
discussion_xblock = get_module_for_descriptor_internal(
user=self.user,
descriptor=self.discussion,
student_data=mock.Mock(name='student_data'),
course_id=self.course.id,
track_function=mock.Mock(name='track_function'),
xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'),
request_token='request_token',
)
fragment = discussion_xblock.render('student_view')
html = fragment.content
self.assertIn('data-user-create-comment="false"', html)
self.assertIn('data-user-create-subcomment="false"', html)
@ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split)
def test_discussion_render_successfully_with_orphan_parent(self, default_store):
"""
Test that discussion xblock render successfully
if discussion xblock is child of an orphan.
"""
with self.store.default_store(default_store):
orphan_sequential = self.store.create_item(self.user.id, self.course.id, 'sequential')
vertical = self.store.create_child(
self.user.id,
orphan_sequential.location,
'vertical',
block_id=self.course.location.block_id
)
discussion = self.store.create_child(
self.user.id,
vertical.location,
'discussion',
block_id=self.course.location.block_id
)
discussion = self.store.get_item(discussion.location)
root = self.get_root(discussion)
# Assert that orphan sequential is root of the discussion xblock.
self.assertEqual(orphan_sequential.location.block_type, root.location.block_type)
self.assertEqual(orphan_sequential.location.block_id, root.location.block_id)
# Get xblock bound to a user and a descriptor.
discussion_xblock = get_module_for_descriptor_internal(
user=self.user,
descriptor=discussion,
student_data=mock.Mock(name='student_data'),
course_id=self.course.id,
track_function=mock.Mock(name='track_function'),
xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'),
request_token='request_token',
)
fragment = discussion_xblock.render('student_view')
html = fragment.content
self.assertIsInstance(discussion_xblock, DiscussionXBlock)
self.assertIn('data-user-create-comment="false"', html)
self.assertIn('data-user-create-subcomment="false"', html)
def test_discussion_student_view_data(self):
"""
Tests that course block api returns student_view_data for discussion xblock
"""
self.client.login(username=self.user.username, password='test')
url = reverse('blocks_in_block_tree', kwargs={'usage_key_string': unicode(self.course_usage_key)})
query_params = {
'depth': 'all',
'username': self.user.username,
'block_types_filter': 'discussion',
'student_view_data': 'discussion'
}
response = self.client.get(url, query_params)
self.assertEquals(response.status_code, 200)
self.assertEquals(response.data['root'], unicode(self.course_usage_key)) # pylint: disable=no-member
for block_key_string, block_data in response.data['blocks'].iteritems(): # pylint: disable=no-member
block_key = deserialize_usage_key(block_key_string, self.course_key)
self.assertEquals(block_data['id'], block_key_string)
self.assertEquals(block_data['type'], block_key.block_type)
self.assertEquals(block_data['display_name'], self.store.get_item(block_key).display_name or '')
self.assertEqual(block_data['student_view_data'], {"topic_id": self.discussion_id})
class TestXBlockQueryLoad(SharedModuleStoreTestCase):
"""
Test the number of queries executed when rendering the XBlock.
"""
def test_permissions_query_load(self):
"""
Tests that the permissions queries are cached when rendering numerous discussion XBlocks.
"""
user = UserFactory.create()
course = ToyCourseFactory.create()
course_key = course.id
course_usage_key = self.store.make_course_usage_key(course_key)
discussions = []
for counter in range(5):
discussion_id = 'test_discussion_{}'.format(counter)
discussions.append(ItemFactory.create(
parent_location=course_usage_key,
category='discussion',
discussion_id=discussion_id,
discussion_category='Category discussion',
discussion_target='Target Discussion',
))
# 3 queries are required to do first discussion xblock render:
# * django_comment_client_role
# * django_comment_client_permission
# * lms_xblock_xblockasidesconfig
num_queries = 3
for discussion in discussions:
discussion_xblock = get_module_for_descriptor_internal(
user=user,
descriptor=discussion,
student_data=mock.Mock(name='student_data'),
course_id=course.id,
track_function=mock.Mock(name='track_function'),
xqueue_callback_url_prefix=mock.Mock(name='xqueue_callback_url_prefix'),
request_token='request_token',
)
with self.assertNumQueries(num_queries):
fragment = discussion_xblock.render('student_view')
# Permissions are cached, so no queries required for subsequent renders
num_queries = 0
html = fragment.content
self.assertIn('data-user-create-comment="false"', html)
self.assertIn('data-user-create-subcomment="false"', html)

34
lms/djangoapps/courseware/tests/test_self_paced_overrides.py
View File

@@ -9,7 +9,7 @@ from mock import patch
from courseware.tests.factories import BetaTesterFactory
from courseware.access import has_access
from lms.djangoapps.ccx.tests.test_overrides import inject_field_overrides
from lms.djangoapps.django_comment_client.utils import get_accessible_discussion_modules
from lms.djangoapps.django_comment_client.utils import get_accessible_discussion_xblocks
from lms.djangoapps.courseware.field_overrides import OverrideFieldData, OverrideModulestoreFieldData
from openedx.core.djangoapps.self_paced.models import SelfPacedConfiguration
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase
@@ -59,8 +59,8 @@ class SelfPacedDateOverrideTest(ModuleStoreTestCase):
inject_field_overrides((course, section), course, self.user)
return (course, section)
def create_discussion_modules(self, parent):
# Create a released discussion module
def create_discussion_xblocks(self, parent):
# Create a released discussion xblock
ItemFactory.create(
parent=parent,
category='discussion',
@@ -68,7 +68,7 @@ class SelfPacedDateOverrideTest(ModuleStoreTestCase):
start=self.now,
)
# Create a scheduled discussion module
# Create a scheduled discussion xblock
ItemFactory.create(
parent=parent,
category='discussion',
@@ -118,32 +118,32 @@ class SelfPacedDateOverrideTest(ModuleStoreTestCase):
self.assertTrue(has_access(beta_tester, 'load', self_paced_section, self_paced_course.id))
@patch.dict('courseware.access.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_instructor_paced_discussion_module_visibility(self):
def test_instructor_paced_discussion_xblock_visibility(self):
"""
Verify that discussion modules scheduled for release in the future are
Verify that discussion xblocks scheduled for release in the future are
not visible to students in an instructor-paced course.
"""
course, section = self.setup_course(start=self.now, self_paced=False)
self.create_discussion_modules(section)
self.create_discussion_xblocks(section)
# Only the released module should be visible when the course is instructor-paced.
modules = get_accessible_discussion_modules(course, self.non_staff_user)
# Only the released xblocks should be visible when the course is instructor-paced.
xblocks = get_accessible_discussion_xblocks(course, self.non_staff_user)
self.assertTrue(
all(module.display_name == 'released' for module in modules)
all(xblock.display_name == 'released' for xblock in xblocks)
)
@patch.dict('courseware.access.settings.FEATURES', {'DISABLE_START_DATES': False})
def test_self_paced_discussion_module_visibility(self):
def test_self_paced_discussion_xblock_visibility(self):
"""
Regression test. Verify that discussion modules scheduled for release
Regression test. Verify that discussion xblocks scheduled for release
in the future are visible to students in a self-paced course.
"""
course, section = self.setup_course(start=self.now, self_paced=True)
self.create_discussion_modules(section)
self.create_discussion_xblocks(section)
# The scheduled module should be visible when the course is self-paced.
modules = get_accessible_discussion_modules(course, self.non_staff_user)
self.assertEqual(len(modules), 2)
# The scheduled xblocks should be visible when the course is self-paced.
xblocks = get_accessible_discussion_xblocks(course, self.non_staff_user)
self.assertEqual(len(xblocks), 2)
self.assertTrue(
any(module.display_name == 'scheduled' for module in modules)
any(xblock.display_name == 'scheduled' for xblock in xblocks)
)

40
lms/djangoapps/discussion_api/api.py
View File

@@ -42,7 +42,7 @@ from django_comment_common.signals import (
comment_voted,
comment_deleted,
)
from django_comment_client.utils import get_accessible_discussion_modules, is_commentable_cohorted
from django_comment_client.utils import get_accessible_discussion_xblocks, is_commentable_cohorted
from lms.djangoapps.discussion_api.pagination import DiscussionAPIPagination
from lms.lib.comment_client.comment import Comment
from lms.lib.comment_client.thread import Thread
@@ -215,41 +215,41 @@ def get_courseware_topics(request, course_key, course, topic_ids):
courseware_topics = []
existing_topic_ids = set()
def get_module_sort_key(module):
def get_xblock_sort_key(xblock):
"""
Get the sort key for the module (falling back to the discussion_target
Get the sort key for the xblock (falling back to the discussion_target
setting if absent)
"""
return module.sort_key or module.discussion_target
return xblock.sort_key or xblock.discussion_target
def get_sorted_modules(category):
"""Returns key sorted modules by category"""
return sorted(modules_by_category[category], key=get_module_sort_key)
def get_sorted_xblocks(category):
"""Returns key sorted xblocks by category"""
return sorted(xblocks_by_category[category], key=get_xblock_sort_key)
discussion_modules = get_accessible_discussion_modules(course, request.user)
modules_by_category = defaultdict(list)
for module in discussion_modules:
modules_by_category[module.discussion_category].append(module)
discussion_xblocks = get_accessible_discussion_xblocks(course, request.user)
xblocks_by_category = defaultdict(list)
for xblock in discussion_xblocks:
xblocks_by_category[xblock.discussion_category].append(xblock)
for category in sorted(modules_by_category.keys()):
for category in sorted(xblocks_by_category.keys()):
children = []
for module in get_sorted_modules(category):
if not topic_ids or module.discussion_id in topic_ids:
for xblock in get_sorted_xblocks(category):
if not topic_ids or xblock.discussion_id in topic_ids:
discussion_topic = DiscussionTopic(
module.discussion_id,
module.discussion_target,
get_thread_list_url(request, course_key, [module.discussion_id]),
xblock.discussion_id,
xblock.discussion_target,
get_thread_list_url(request, course_key, [xblock.discussion_id]),
)
children.append(discussion_topic)
if topic_ids and module.discussion_id in topic_ids:
existing_topic_ids.add(module.discussion_id)
if topic_ids and xblock.discussion_id in topic_ids:
existing_topic_ids.add(xblock.discussion_id)
if not topic_ids or children:
discussion_topic = DiscussionTopic(
None,
category,
get_thread_list_url(request, course_key, [item.discussion_id for item in get_sorted_modules(category)]),
get_thread_list_url(request, course_key, [item.discussion_id for item in get_sorted_xblocks(category)]),
children,
)
courseware_topics.append(DiscussionTopicSerializer(discussion_topic).data)

48
lms/djangoapps/discussion_api/tests/test_api.py
View File

@@ -65,7 +65,7 @@ def _remove_discussion_tab(course, user_id):
"""
Remove the discussion tab for the course.
user_id is passed to the modulestore as the editor of the module.
user_id is passed to the modulestore as the editor of the xblock.
"""
course.tabs = [tab for tab in course.tabs if not tab.type == 'discussion']
modulestore().update_item(course, user_id)
@@ -206,8 +206,10 @@ class GetCourseTopicsTest(UrlResetMixin, ModuleStoreTestCase):
self.request.user = self.user
CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id)
def make_discussion_module(self, topic_id, category, subcategory, **kwargs):
"""Build a discussion module in self.course"""
def make_discussion_xblock(self, topic_id, category, subcategory, **kwargs):
"""
Build a discussion xblock in self.course.
"""
ItemFactory.create(
parent_location=self.course.location,
category="discussion",
@@ -274,7 +276,7 @@ class GetCourseTopicsTest(UrlResetMixin, ModuleStoreTestCase):
self.assertEqual(actual, expected)
def test_with_courseware(self):
self.make_discussion_module("courseware-topic-id", "Foo", "Bar")
self.make_discussion_xblock("courseware-topic-id", "Foo", "Bar")
actual = self.get_course_topics()
expected = {
"courseware_topics": [
@@ -297,11 +299,11 @@ class GetCourseTopicsTest(UrlResetMixin, ModuleStoreTestCase):
"B": {"id": "non-courseware-2"},
}
self.store.update_item(self.course, self.user.id)
self.make_discussion_module("courseware-1", "A", "1")
self.make_discussion_module("courseware-2", "A", "2")
self.make_discussion_module("courseware-3", "B", "1")
self.make_discussion_module("courseware-4", "B", "2")
self.make_discussion_module("courseware-5", "C", "1")
self.make_discussion_xblock("courseware-1", "A", "1")
self.make_discussion_xblock("courseware-2", "A", "2")
self.make_discussion_xblock("courseware-3", "B", "1")
self.make_discussion_xblock("courseware-4", "B", "2")
self.make_discussion_xblock("courseware-5", "C", "1")
actual = self.get_course_topics()
expected = {
"courseware_topics": [
@@ -343,13 +345,13 @@ class GetCourseTopicsTest(UrlResetMixin, ModuleStoreTestCase):
"Z": {"id": "non-courseware-4", "sort_key": "W"},
}
self.store.update_item(self.course, self.user.id)
self.make_discussion_module("courseware-1", "First", "A", sort_key="D")
self.make_discussion_module("courseware-2", "First", "B", sort_key="B")
self.make_discussion_module("courseware-3", "First", "C", sort_key="E")
self.make_discussion_module("courseware-4", "Second", "A", sort_key="F")
self.make_discussion_module("courseware-5", "Second", "B", sort_key="G")
self.make_discussion_module("courseware-6", "Second", "C")
self.make_discussion_module("courseware-7", "Second", "D", sort_key="A")
self.make_discussion_xblock("courseware-1", "First", "A", sort_key="D")
self.make_discussion_xblock("courseware-2", "First", "B", sort_key="B")
self.make_discussion_xblock("courseware-3", "First", "C", sort_key="E")
self.make_discussion_xblock("courseware-4", "Second", "A", sort_key="F")
self.make_discussion_xblock("courseware-5", "Second", "B", sort_key="G")
self.make_discussion_xblock("courseware-6", "Second", "C")
self.make_discussion_xblock("courseware-7", "Second", "D", sort_key="A")
actual = self.get_course_topics()
expected = {
@@ -411,21 +413,21 @@ class GetCourseTopicsTest(UrlResetMixin, ModuleStoreTestCase):
)
with self.store.bulk_operations(self.course.id, emit_signals=False):
self.make_discussion_module("courseware-1", "First", "Everybody")
self.make_discussion_module(
self.make_discussion_xblock("courseware-1", "First", "Everybody")
self.make_discussion_xblock(
"courseware-2",
"First",
"Cohort A",
group_access={self.partition.id: [self.partition.groups[0].id]}
)
self.make_discussion_module(
self.make_discussion_xblock(
"courseware-3",
"First",
"Cohort B",
group_access={self.partition.id: [self.partition.groups[1].id]}
)
self.make_discussion_module("courseware-4", "Second", "Staff Only", visible_to_staff_only=True)
self.make_discussion_module(
self.make_discussion_xblock("courseware-4", "Second", "Staff Only", visible_to_staff_only=True)
self.make_discussion_xblock(
"courseware-5",
"Second",
"Future Start Date",
@@ -507,8 +509,8 @@ class GetCourseTopicsTest(UrlResetMixin, ModuleStoreTestCase):
"""
topic_id_1 = "topic_id_1"
topic_id_2 = "topic_id_2"
self.make_discussion_module(topic_id_1, "test_category_1", "test_target_1")
self.make_discussion_module(topic_id_2, "test_category_2", "test_target_2")
self.make_discussion_xblock(topic_id_1, "test_category_1", "test_target_1")
self.make_discussion_xblock(topic_id_2, "test_category_2", "test_target_2")
actual = get_course_topics(self.request, self.course.id, {"topic_id_1", "topic_id_2"})
self.assertEqual(
actual,

14
lms/djangoapps/discussion_api/tests/test_views.py
View File

@@ -164,7 +164,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
def create_course(self, modules_count, module_store, topics):
"""
Create a course in a specified module store with discussion module and topics
Create a course in a specified module store with discussion xblocks and topics
"""
course = CourseFactory.create(
org="a",
@@ -176,7 +176,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
)
CourseEnrollmentFactory.create(user=self.user, course_id=course.id)
course_url = reverse("course_topics", kwargs={"course_id": unicode(course.id)})
# add some discussion modules
# add some discussion xblocks
for i in range(modules_count):
ItemFactory.create(
parent_location=course.location,
@@ -188,9 +188,9 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
)
return course_url
def make_discussion_module(self, topic_id, category, subcategory, **kwargs):
def make_discussion_xblock(self, topic_id, category, subcategory, **kwargs):
"""
Build a discussion module in self.course
Build a discussion xblock in self.course
"""
ItemFactory.create(
parent_location=self.course.location,
@@ -249,7 +249,7 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
Tests discussion topic does not exist for the given topic id.
"""
topic_id = "courseware-topic-id"
self.make_discussion_module(topic_id, "test_category", "test_target")
self.make_discussion_xblock(topic_id, "test_category", "test_target")
url = "{}?topic_id=invalid_topic_id".format(self.url)
response = self.client.get(url)
self.assert_response_correct(
@@ -264,8 +264,8 @@ class CourseTopicsViewTest(DiscussionAPIViewTestMixin, ModuleStoreTestCase):
"""
topic_id_1 = "topic_id_1"
topic_id_2 = "topic_id_2"
self.make_discussion_module(topic_id_1, "test_category_1", "test_target_1")
self.make_discussion_module(topic_id_2, "test_category_2", "test_target_2")
self.make_discussion_xblock(topic_id_1, "test_category_1", "test_target_1")
self.make_discussion_xblock(topic_id_2, "test_category_2", "test_target_2")
url = "{}?topic_id=topic_id_1,topic_id_2".format(self.url)
response = self.client.get(url)
self.assert_response_correct(

12
lms/djangoapps/django_comment_client/forum/tests.py
View File

@@ -623,8 +623,8 @@ class SingleThreadContentGroupTestCase(UrlResetMixin, ContentGroupTestCase):
thread_id = "test_thread_id"
mock_request.side_effect = make_mock_request_impl(course=self.course, text="dummy content", thread_id=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)
for discussion_xblock in [self.alpha_module, self.beta_module, self.global_module]:
self.assert_can_access(self.staff_user, discussion_xblock.discussion_id, thread_id, True)
def test_alpha_user(self, mock_request):
"""
@@ -634,8 +634,8 @@ class SingleThreadContentGroupTestCase(UrlResetMixin, ContentGroupTestCase):
thread_id = "test_thread_id"
mock_request.side_effect = make_mock_request_impl(course=self.course, text="dummy content", thread_id=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)
for discussion_xblock in [self.alpha_module, self.global_module]:
self.assert_can_access(self.alpha_user, discussion_xblock.discussion_id, thread_id, True)
self.assert_can_access(self.alpha_user, self.beta_module.discussion_id, thread_id, False)
@@ -647,8 +647,8 @@ class SingleThreadContentGroupTestCase(UrlResetMixin, ContentGroupTestCase):
thread_id = "test_thread_id"
mock_request.side_effect = make_mock_request_impl(course=self.course, text="dummy content", thread_id=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)
for discussion_xblock in [self.beta_module, self.global_module]:
self.assert_can_access(self.beta_user, discussion_xblock.discussion_id, thread_id, True)
self.assert_can_access(self.beta_user, self.alpha_module.discussion_id, thread_id, False)

12
lms/djangoapps/django_comment_client/forum/views.py
View File

@@ -269,10 +269,8 @@ def forum_form_discussion(request, course_key):
'threads': json.dumps(threads),
'thread_pages': query_params['num_pages'],
'user_info': json.dumps(user_info, default=lambda x: None),
'can_create_comment': json.dumps(
has_permission(request.user, "create_comment", course.id)),
'can_create_subcomment': json.dumps(
has_permission(request.user, "create_sub_comment", course.id)),
'can_create_comment': has_permission(request.user, "create_comment", course.id),
'can_create_subcomment': has_permission(request.user, "create_sub_comment", course.id),
'can_create_thread': has_permission(request.user, "create_thread", course.id),
'flag_moderator': bool(
has_permission(request.user, 'openclose_thread', course.id) or
@@ -381,10 +379,8 @@ def single_thread(request, course_key, discussion_id, thread_id):
'csrf': csrf(request)['csrf_token'],
'init': '', # TODO: What is this?
'user_info': json.dumps(user_info),
'can_create_comment': json.dumps(
has_permission(request.user, "create_comment", course.id)),
'can_create_subcomment': json.dumps(
has_permission(request.user, "create_sub_comment", course.id)),
'can_create_comment': has_permission(request.user, "create_comment", course.id),
'can_create_subcomment': has_permission(request.user, "create_sub_comment", course.id),
'can_create_thread': has_permission(request.user, "create_thread", course.id),
'annotated_content_info': json.dumps(annotated_content_info),
'course': course,

24
lms/djangoapps/django_comment_client/tests/test_utils.py
View File

@@ -177,29 +177,29 @@ class CoursewareContextTestCase(ModuleStoreTestCase):
@ddt.data((ModuleStoreEnum.Type.mongo, 2), (ModuleStoreEnum.Type.split, 1))
@ddt.unpack
def test_get_accessible_discussion_modules(self, modulestore_type, expected_discussion_modules):
def test_get_accessible_discussion_xblocks(self, modulestore_type, expected_discussion_xblocks):
"""
Tests that the accessible discussion modules having no parents do not get fetched for split modulestore.
Tests that the accessible discussion xblocks having no parents do not get fetched for split modulestore.
"""
course = CourseFactory.create(default_store=modulestore_type)
# Create a discussion module.
# Create a discussion xblock.
test_discussion = self.store.create_child(self.user.id, course.location, 'discussion', 'test_discussion')
# Assert that created discussion module is not an orphan.
# Assert that created discussion xblock is not an orphan.
self.assertNotIn(test_discussion.location, self.store.get_orphans(course.id))
# Assert that there is only one discussion module in the course at the moment.
self.assertEqual(len(utils.get_accessible_discussion_modules(course, self.user)), 1)
# Assert that there is only one discussion xblock in the course at the moment.
self.assertEqual(len(utils.get_accessible_discussion_xblocks(course, self.user)), 1)
# Add an orphan discussion module to that course
# Add an orphan discussion xblock to that course
orphan = course.id.make_usage_key('discussion', 'orphan_discussion')
self.store.create_item(self.user.id, orphan.course_key, orphan.block_type, block_id=orphan.block_id)
# Assert that the discussion module is an orphan.
# Assert that the discussion xblock is an orphan.
self.assertIn(orphan, self.store.get_orphans(course.id))
self.assertEqual(len(utils.get_accessible_discussion_modules(course, self.user)), expected_discussion_modules)
self.assertEqual(len(utils.get_accessible_discussion_xblocks(course, self.user)), expected_discussion_xblocks)
@attr('shard_3')
@@ -262,7 +262,7 @@ class CachedDiscussionIdMapTestCase(ModuleStoreTestCase):
with self.assertRaises(utils.DiscussionIdMapIsNotCached):
utils.get_cached_discussion_key(self.course, 'test_discussion_id')
def test_module_does_not_have_required_keys(self):
def test_xblock_does_not_have_required_keys(self):
self.assertTrue(utils.has_required_keys(self.discussion))
self.assertFalse(utils.has_required_keys(self.bad_discussion))
@@ -505,7 +505,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
cohorted_if_in_list=True
)
def test_get_unstarted_discussion_modules(self):
def test_get_unstarted_discussion_xblocks(self):
later = datetime.datetime(datetime.MAXYEAR, 1, 1, tzinfo=django_utc())
self.create_discussion("Chapter 1", "Discussion 1", start=later)
@@ -1026,7 +1026,7 @@ class CategoryMapTestCase(CategoryMapTestMixin, ModuleStoreTestCase):
@attr('shard_1')
class ContentGroupCategoryMapTestCase(CategoryMapTestMixin, ContentGroupTestCase):
"""
Tests `get_discussion_category_map` on discussion modules which are
Tests `get_discussion_category_map` on discussion xblocks which are
only visible to some content groups.
"""
def test_staff_user(self):

73
lms/djangoapps/django_comment_client/utils.py
View File

@@ -108,44 +108,44 @@ def has_forum_access(uname, course_id, rolename):
return role.users.filter(username=uname).exists()
def has_required_keys(module):
def has_required_keys(xblock):
"""
Returns True iff module has the proper attributes for generating metadata
Returns True iff xblock has the proper attributes for generating metadata
with get_discussion_id_map_entry()
"""
for key in ('discussion_id', 'discussion_category', 'discussion_target'):
if getattr(module, key, None) is None:
if getattr(xblock, key, None) is None:
log.debug(
"Required key '%s' not in discussion %s, leaving out of category map",
key,
module.location
xblock.location
)
return False
return True
def get_accessible_discussion_modules(course, user, include_all=False): # pylint: disable=invalid-name
def get_accessible_discussion_xblocks(course, user, include_all=False): # pylint: disable=invalid-name
"""
Return a list of all valid discussion modules in this course that
Return a list of all valid discussion xblocks in this course that
are accessible to the given user.
"""
all_modules = modulestore().get_items(course.id, qualifiers={'category': 'discussion'}, include_orphans=False)
all_xblocks = modulestore().get_items(course.id, qualifiers={'category': 'discussion'}, include_orphans=False)
return [
module for module in all_modules
if has_required_keys(module) and (include_all or has_access(user, 'load', module, course.id))
xblock for xblock in all_xblocks
if has_required_keys(xblock) and (include_all or has_access(user, 'load', xblock, course.id))
]
def get_discussion_id_map_entry(module):
def get_discussion_id_map_entry(xblock):
"""
Returns a tuple of (discussion_id, metadata) suitable for inclusion in the results of get_discussion_id_map().
"""
return (
module.discussion_id,
xblock.discussion_id,
{
"location": module.location,
"title": module.discussion_category.split("/")[-1].strip() + " / " + module.discussion_target
"location": xblock.location,
"title": xblock.discussion_category.split("/")[-1].strip() + " / " + xblock.discussion_target
}
)
@@ -157,7 +157,7 @@ class DiscussionIdMapIsNotCached(Exception):
def get_cached_discussion_key(course, discussion_id):
"""
Returns the usage key of the discussion module associated with discussion_id if it is cached. If the discussion id
Returns the usage key of the discussion xblock associated with discussion_id if it is cached. If the discussion id
map is cached but does not contain discussion_id, returns None. If the discussion id map is not cached for course,
raises a DiscussionIdMapIsNotCached exception.
"""
@@ -172,7 +172,7 @@ def get_cached_discussion_key(course, discussion_id):
def get_cached_discussion_id_map(course, discussion_ids, user):
"""
Returns a dict mapping discussion_ids to respective discussion module metadata if it is cached and visible to the
Returns a dict mapping discussion_ids to respective discussion xblock metadata if it is cached and visible to the
user. If not, returns the result of get_discussion_id_map
"""
try:
@@ -181,10 +181,10 @@ def get_cached_discussion_id_map(course, discussion_ids, user):
key = get_cached_discussion_key(course, discussion_id)
if not key:
continue
module = modulestore().get_item(key)
if not (has_required_keys(module) and has_access(user, 'load', module, course.id)):
xblock = modulestore().get_item(key)
if not (has_required_keys(xblock) and has_access(user, 'load', xblock, course.id)):
continue
entries.append(get_discussion_id_map_entry(module))
entries.append(get_discussion_id_map_entry(xblock))
return dict(entries)
except DiscussionIdMapIsNotCached:
return get_discussion_id_map(course, user)
@@ -192,10 +192,10 @@ def get_cached_discussion_id_map(course, discussion_ids, user):
def get_discussion_id_map(course, user):
"""
Transform the list of this course's discussion modules (visible to a given user) into a dictionary of metadata keyed
Transform the list of this course's discussion xblocks (visible to a given user) into a dictionary of metadata keyed
by discussion_id.
"""
return dict(map(get_discussion_id_map_entry, get_accessible_discussion_modules(course, user)))
return dict(map(get_discussion_id_map_entry, get_accessible_discussion_xblocks(course, user)))
def _filter_unstarted_categories(category_map, course):
@@ -256,7 +256,7 @@ def _sort_map_entries(category_map, sort_alpha):
def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude_unstarted=True):
"""
Transform the list of this course's discussion modules into a recursive dictionary structure. This is used
Transform the list of this course's discussion xblocks into a recursive dictionary structure. This is used
to render the discussion category map in the discussion tab sidebar for a given user.
Args:
@@ -301,18 +301,21 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude
"""
unexpanded_category_map = defaultdict(list)
modules = get_accessible_discussion_modules(course, user)
xblocks = get_accessible_discussion_xblocks(course, user)
course_cohort_settings = get_course_cohort_settings(course.id)
for module in modules:
id = module.discussion_id
title = module.discussion_target
sort_key = module.sort_key
category = " / ".join([x.strip() for x in module.discussion_category.split("/")])
# Handle case where module.start is None
entry_start_date = module.start if module.start else datetime.max.replace(tzinfo=pytz.UTC)
unexpanded_category_map[category].append({"title": title, "id": id, "sort_key": sort_key, "start_date": entry_start_date})
for xblock in xblocks:
discussion_id = xblock.discussion_id
title = xblock.discussion_target
sort_key = xblock.sort_key
category = " / ".join([x.strip() for x in xblock.discussion_category.split("/")])
# Handle case where xblock.start is None
entry_start_date = xblock.start if xblock.start else datetime.max.replace(tzinfo=pytz.UTC)
unexpanded_category_map[category].append({"title": title,
"id": discussion_id,
"sort_key": sort_key,
"start_date": entry_start_date})
category_map = {"entries": defaultdict(dict), "subcategories": defaultdict(dict)}
for category_path, entries in unexpanded_category_map.items():
@@ -385,7 +388,7 @@ def get_discussion_category_map(course, user, cohorted_if_in_list=False, exclude
return _filter_unstarted_categories(category_map, course) if exclude_unstarted else category_map
def discussion_category_id_access(course, user, discussion_id, module=None):
def discussion_category_id_access(course, user, discussion_id, xblock=None):
"""
Returns True iff the given discussion_id is accessible for user in course.
Assumes that the commentable identified by discussion_id has a null or 'course' context.
@@ -395,12 +398,12 @@ def discussion_category_id_access(course, user, discussion_id, module=None):
if discussion_id in course.top_level_discussion_topic_ids:
return True
try:
if not module:
if not xblock:
key = get_cached_discussion_key(course, discussion_id)
if not key:
return False
module = modulestore().get_item(key)
return has_required_keys(module) and has_access(user, 'load', module, course.id)
xblock = modulestore().get_item(key)
return has_required_keys(xblock) and has_access(user, 'load', xblock, course.id)
except DiscussionIdMapIsNotCached:
return discussion_id in get_discussion_categories_ids(course, user)
@@ -417,7 +420,7 @@ def get_discussion_categories_ids(course, user, include_all=False):
"""
accessible_discussion_ids = [
module.discussion_id for module in get_accessible_discussion_modules(course, user, include_all=include_all)
xblock.discussion_id for xblock in get_accessible_discussion_xblocks(course, user, include_all=include_all)
]
return course.top_level_discussion_topic_ids + accessible_discussion_ids

28
lms/templates/discussion/_discussion_inline.html Normal file
View File

@@ -0,0 +1,28 @@
<%page expression_filter="h"/>
<%include file="_underscore_templates.html" />
<%!
from django.utils.translation import ugettext as _
from json import dumps as json_dumps
from openedx.core.djangolib.js_utils import js_escaped_string
%>
<div class="discussion-module" data-discussion-id="${discussion_id}" data-user-create-comment="${json_dumps(can_create_comment)}" data-user-create-subcomment="${json_dumps(can_create_subcomment)}" data-read-only="false">
<a class="discussion-show control-button" href="javascript:void(0)" data-discussion-id="${discussion_id}" role="button">
<span class="show-hide-discussion-icon"></span><span class="button-text">${_("Show Discussion")}</span>
</a>
% if can_create_thread:
<button class="new-post-btn btn-neutral btn-small">${_("Add a Post")}</button>
% endif
</div>
<script type="text/javascript">
/* global DiscussionModuleView */
/* exported DiscussionInlineBlock, $$course_id */
var $$course_id = "${course_id | n, js_escaped_string}";
function DiscussionInlineBlock(runtime, element) {
'use strict';
var el = $(element).find('.discussion-module');
/* jshint nonew:false */
new DiscussionModuleView({ el: el });
}
</script>

3
lms/templates/discussion/_discussion_module_studio.html → lms/templates/discussion/_discussion_inline_studio.html
View File

@@ -1,6 +1,7 @@
<%! from django.utils.translation import ugettext as _ %>
<%page expression_filter="h"/>
<div class="discussion-module" data-discussion-id="${discussion_id | h}">
<div class="discussion-module" data-discussion-id="${discussion_id}">
<p>
<span class="discussion-preview">
<span class="icon fa fa-comment"/>

14
lms/templates/discussion/_discussion_module.html
View File

@@ -1,14 +0,0 @@
<%page expression_filter="h"/>
<%include file="_underscore_templates.html" />
<%!
from django.utils.translation import ugettext as _
%>
<div class="discussion-module" data-discussion-id="${discussion_id}" data-user-create-comment="${can_create_comment}" data-user-create-subcomment="${can_create_subcomment}" data-read-only="false">
<a class="discussion-show control-button" href="javascript:void(0)" data-discussion-id="${discussion_id}" role="button">
<span class="show-hide-discussion-icon"></span><span class="button-text">${_("Show Discussion")}</span>
</a>
% if can_create_thread:
<button class="new-post-btn btn-neutral btn-small">${_("Add a Post")}</button>
% endif
</div>

4
openedx/core/djangoapps/content/course_structures/tests.py
View File

@@ -32,12 +32,12 @@ class CourseStructureTaskTests(ModuleStoreTestCase):
super(CourseStructureTaskTests, self).setUp()
self.course = CourseFactory.create(org='TestX', course='TS101', run='T1')
self.section = ItemFactory.create(parent=self.course, category='chapter', display_name='Test Section')
self.discussion_module_1 = ItemFactory.create(
self.discussion_xblock_1 = ItemFactory.create(
parent=self.course,
category='discussion',
discussion_id='test_discussion_id_1'
)
self.discussion_module_2 = ItemFactory.create(
self.discussion_xblock_2 = ItemFactory.create(
parent=self.course,
category='discussion',
discussion_id='test_discussion_id_2'

17
openedx/core/lib/xblock_builtin/README.rst Normal file
View File

@@ -0,0 +1,17 @@
Open edX: Built-in XBlocks
--------------------------
This area is meant for exceptional and hopefully temporary cases where an
XBlock is integral to the functionality of the Open edX platform.
This is not a pattern we wish for normal XBlocks to follow; they should live in
their own repo.
Discussion XBlock
=================
This XBlock was converted from an XModule, and will hopefully be pulled out of
edx-platform into its own repo at some point. From discussions, it's not too
difficult to move the server-side code , but the client-side code is used by
the discussion board tab and the team discussion, so for now, must remain in
edx-platform.

0
openedx/core/lib/xblock_builtin/__init__.py Normal file
View File

0
openedx/core/lib/xblock_builtin/xblock_discussion/__init__.py Normal file
View File

35
openedx/core/lib/xblock_builtin/xblock_discussion/setup.py Normal file
View File

@@ -0,0 +1,35 @@
"""
Setup for discussion-forum XBlock.
"""
import os
from setuptools import setup
def package_data(pkg, root_list):
"""
Generic function to find package_data for `pkg` under `root`.
"""
data = []
for root in root_list:
for dirname, _, files in os.walk(os.path.join(pkg, root)):
for fname in files:
data.append(os.path.relpath(os.path.join(dirname, fname), pkg))
return {pkg: data}
setup(
name='xblock-discussion',
version='0.1',
description='XBlock - Discussion',
install_requires=[
'XBlock',
],
entry_points={
'xblock.v1': [
'discussion = xblock_discussion:DiscussionXBlock'
]
},
package_data=package_data("xblock_discussion", ["static"]),
)

136
openedx/core/lib/xblock_builtin/xblock_discussion/xblock_discussion.py Normal file
View File

@@ -0,0 +1,136 @@
# -*- coding: utf-8 -*-
"""
Discussion XBlock
"""
import logging
from xblockutils.resources import ResourceLoader
from xblockutils.studio_editable import StudioEditableXBlockMixin
from xblock.core import XBlock
from xblock.fields import Scope, String, UNIQUE_ID
from xblock.fragment import Fragment
log = logging.getLogger(__name__)
loader = ResourceLoader(__name__) # pylint: disable=invalid-name
def _(text):
"""
A noop underscore function that marks strings for extraction.
"""
return text
@XBlock.needs('user')
@XBlock.needs('i18n')
class DiscussionXBlock(XBlock, StudioEditableXBlockMixin):
"""
Provides a discussion forum that is inline with other content in the courseware.
"""
discussion_id = String(scope=Scope.settings, default=UNIQUE_ID, force_export=True)
display_name = String(
display_name=_("Display Name"),
help=_("Display name for this component"),
default="Discussion",
scope=Scope.settings
)
discussion_category = String(
display_name=_("Category"),
default=_("Week 1"),
help=_(
"A category name for the discussion. "
"This name appears in the left pane of the discussion forum for the course."
),
scope=Scope.settings
)
discussion_target = String(
display_name=_("Subcategory"),
default="Topic-Level Student-Visible Label",
help=_(
"A subcategory name for the discussion. "
"This name appears in the left pane of the discussion forum for the course."
),
scope=Scope.settings
)
sort_key = String(scope=Scope.settings)
editable_fields = ["display_name", "discussion_category", "discussion_target"]
has_author_view = True # Tells Studio to use author_view
@property
def course_key(self):
"""
:return: int course id
NB: The goal is to move this XBlock out of edx-platform, and so we use
scope_ids.usage_id instead of runtime.course_id so that the code will
continue to work with workbench-based testing.
"""
return getattr(self.scope_ids.usage_id, 'course_key', None)
@property
def django_user(self):
"""
Returns django user associated with user currently interacting
with the XBlock.
"""
user_service = self.runtime.service(self, 'user')
if not user_service:
return None
return user_service._django_user # pylint: disable=protected-access
def has_permission(self, permission):
"""
Encapsulates lms specific functionality, as `has_permission` is not
importable outside of lms context, namely in tests.
:param user:
:param str permission: Permission
:rtype: bool
"""
# normal import causes the xmodule_assets command to fail due to circular import - hence importing locally
from django_comment_client.permissions import has_permission # pylint: disable=import-error
return has_permission(self.django_user, permission, self.course_key)
def student_view(self, context=None):
"""
Renders student view for LMS.
"""
fragment = Fragment()
course = self.runtime.modulestore.get_course(self.course_key)
context = {
'discussion_id': self.discussion_id,
'user': self.django_user,
'course': course,
'course_id': self.course_key,
'can_create_thread': self.has_permission("create_thread"),
'can_create_comment': self.has_permission("create_comment"),
'can_create_subcomment': self.has_permission("create_subcomment"),
}
fragment.add_content(self.runtime.render_template('discussion/_discussion_inline.html', context))
fragment.initialize_js('DiscussionInlineBlock')
return fragment
def author_view(self, context=None): # pylint: disable=unused-argument
"""
Renders author view for Studio.
"""
fragment = Fragment()
fragment.add_content(self.runtime.render_template(
'discussion/_discussion_inline_studio.html',
{'discussion_id': self.discussion_id}
))
return fragment
def student_view_data(self):
"""
Returns a JSON representation of the student_view of this XBlock.
"""
return {'topic_id': self.discussion_id}

2
requirements/edx/local.txt
View File

@@ -8,3 +8,5 @@
-e common/lib/sandbox-packages
-e common/lib/symmath
-e common/lib/xmodule
-e openedx/core/lib/xblock_builtin/xblock_discussion