diff --git a/cms/djangoapps/contentstore/views/item.py b/cms/djangoapps/contentstore/views/item.py index 4a9d0e66fef1..36779d996491 100644 --- a/cms/djangoapps/contentstore/views/item.py +++ b/cms/djangoapps/contentstore/views/item.py @@ -40,9 +40,6 @@ log = logging.getLogger(__name__) -# cdodge: these are categories which should not be parented, they are detached from the hierarchy -DETACHED_CATEGORIES = ['about', 'static_tab', 'course_info'] - CREATE_IF_NOT_FOUND = ['course_info'] @@ -297,10 +294,11 @@ def _create_item(request): dest_location, definition_data=data, metadata=metadata, - system=parent.system, + system=parent.runtime, ) - if category not in DETACHED_CATEGORIES: + # TODO replace w/ nicer accessor + if not 'detached' in parent.runtime.load_block_type(category)._class_tags: get_modulestore(parent.location).update_children(parent_location, parent.children + [dest_location.url()]) course_location = loc_mapper().translate_locator_to_location(parent_locator, get_course=True) @@ -329,7 +327,7 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non dest_location, definition_data=source_item.data if hasattr(source_item, 'data') else None, metadata=duplicate_metadata, - system=source_item.system if hasattr(source_item, 'system') else None, + system=source_item.runtime, ) # Children are not automatically copied over (and not all xblocks have a 'children' attribute). @@ -340,7 +338,7 @@ def _duplicate_item(parent_location, duplicate_source_location, display_name=Non copied_children.append(_duplicate_item(dest_location, Location(child)).url()) get_modulestore(dest_location).update_children(dest_location, copied_children) - if category not in DETACHED_CATEGORIES: + if not 'detached' in source_item.runtime.load_block_type(category)._class_tags: parent = get_modulestore(parent_location).get_item(parent_location) # If source was already a child of the parent, add duplicate immediately afterward. # Otherwise, add child to end. @@ -404,12 +402,12 @@ def orphan_handler(request, tag=None, package_id=None, branch=None, version_guid old_location = loc_mapper().translate_locator_to_location(location) if request.method == 'GET': if has_course_access(request.user, old_location): - return JsonResponse(modulestore().get_orphans(old_location, DETACHED_CATEGORIES, 'draft')) + return JsonResponse(modulestore().get_orphans(old_location, 'draft')) else: raise PermissionDenied() if request.method == 'DELETE': if request.user.is_staff: - items = modulestore().get_orphans(old_location, DETACHED_CATEGORIES, 'draft') + items = modulestore().get_orphans(old_location, 'draft') for item in items: modulestore('draft').delete_item(item, True) return JsonResponse({'deleted': items}) diff --git a/common/lib/xmodule/xmodule/html_module.py b/common/lib/xmodule/xmodule/html_module.py index a821543c0b85..03c3ed337291 100644 --- a/common/lib/xmodule/xmodule/html_module.py +++ b/common/lib/xmodule/xmodule/html_module.py @@ -3,14 +3,11 @@ import logging import os import sys -from datetime import datetime from lxml import etree from path import path -from pytz import UTC from pkg_resources import resource_string from xblock.fields import Scope, String, Boolean -from xmodule.fields import Date from xmodule.editing_module import EditingDescriptor from xmodule.html_checker import check_html from xmodule.stringify import stringify_children @@ -18,6 +15,7 @@ from xmodule.xml_module import XmlDescriptor, name_to_pathname import textwrap from xmodule.contentstore.content import StaticContent +from xblock.core import XBlock log = logging.getLogger("edx.courseware") @@ -230,14 +228,9 @@ class AboutFields(object): default="", scope=Scope.content ) - # this exists purely to override the default start date - start = Date( - help="placeholder to make sure that About is always active", - default=datetime.fromtimestamp(0, UTC), - scope=Scope.settings, - ) +@XBlock.tag("detached") class AboutModule(AboutFields, HtmlModule): """ Overriding defaults but otherwise treated as HtmlModule. @@ -245,6 +238,7 @@ class AboutModule(AboutFields, HtmlModule): pass +@XBlock.tag("detached") class AboutDescriptor(AboutFields, HtmlDescriptor): """ These pieces of course content are treated as HtmlModules but we need to overload where the templates are located @@ -271,14 +265,9 @@ class StaticTabFields(object): scope=Scope.content, help="HTML for the additional pages" ) - # this exists purely to override the default start date - start = Date( - help="placeholder to make sure that Static Tabs are always active", - default=datetime.fromtimestamp(0, UTC), - scope=Scope.settings, - ) +@XBlock.tag("detached") class StaticTabModule(StaticTabFields, HtmlModule): """ Supports the field overrides @@ -286,6 +275,7 @@ class StaticTabModule(StaticTabFields, HtmlModule): pass +@XBlock.tag("detached") class StaticTabDescriptor(StaticTabFields, HtmlDescriptor): """ These pieces of course content are treated as HtmlModules but we need to overload where the templates are located @@ -304,14 +294,9 @@ class CourseInfoFields(object): default="
    ", scope=Scope.content ) - # this exists purely to override the default start date - start = Date( - help="placeholder to make sure that Course Info is always active", - default=datetime.fromtimestamp(0, UTC), - scope=Scope.settings, - ) +@XBlock.tag("detached") class CourseInfoModule(CourseInfoFields, HtmlModule): """ Just to support xblock field overrides @@ -319,6 +304,7 @@ class CourseInfoModule(CourseInfoFields, HtmlModule): pass +@XBlock.tag("detached") class CourseInfoDescriptor(CourseInfoFields, HtmlDescriptor): """ These pieces of course content are treated as HtmlModules but we need to overload where the templates are located diff --git a/common/lib/xmodule/xmodule/modulestore/mongo/base.py b/common/lib/xmodule/xmodule/modulestore/mongo/base.py index 1961247a6f8f..9870a1d3d811 100644 --- a/common/lib/xmodule/xmodule/modulestore/mongo/base.py +++ b/common/lib/xmodule/xmodule/modulestore/mongo/base.py @@ -34,6 +34,7 @@ from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import own_metadata, InheritanceMixin, inherit_metadata, InheritanceKeyValueStore from xmodule.modulestore.xml import LocationReader +from xblock.core import XBlock log = logging.getLogger(__name__) @@ -869,10 +870,11 @@ def get_modulestore_type(self, course_id): """ return MONGO_MODULESTORE_TYPE - def get_orphans(self, course_location, detached_categories, _branch): + def get_orphans(self, course_location, _branch): """ Return an array all of the locations for orphans in the course. """ + detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")] all_items = self.collection.find({ '_id.org': course_location.org, '_id.course': course_location.course, diff --git a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py index 4d86cf14fae3..5f5f047f1bf7 100644 --- a/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py +++ b/common/lib/xmodule/xmodule/modulestore/split_mongo/split.py @@ -439,12 +439,13 @@ def get_parent_locations(self, locator, course_id=None): ) for parent_id in items] - def get_orphans(self, package_id, detached_categories, branch): + def get_orphans(self, package_id, branch): """ Return a dict of all of the orphans in the course. :param package_id: """ + detached_categories = [name for name, __ in XBlock.load_tagged_classes("detached")] course = self._lookup_course(CourseLocator(package_id=package_id, branch=branch)) items = {LocMapperStore.decode_key_from_mongo(block_id) for block_id in course['structure']['blocks'].keys()} items.remove(course['structure']['root']) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py index 7aaf1723c665..16c70ea634f6 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py @@ -135,7 +135,7 @@ def test_mongo_orphan(self): """ Test that old mongo finds the orphans """ - orphans = self.old_mongo.get_orphans(self.course_location, ['static_tab', 'about', 'course_info'], None) + orphans = self.old_mongo.get_orphans(self.course_location, None) self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) location = self.course_location.replace(category='chapter', name='OrphanChapter') self.assertIn(location.url(), orphans) @@ -148,7 +148,7 @@ def test_split_orphan(self): """ Test that old mongo finds the orphans """ - orphans = self.split_mongo.get_orphans(self.split_package_id, ['static_tab', 'about', 'course_info'], 'draft') + orphans = self.split_mongo.get_orphans(self.split_package_id, 'draft') self.assertEqual(len(orphans), 3, "Wrong # {}".format(orphans)) location = BlockUsageLocator(package_id=self.split_package_id, branch='draft', block_id='OrphanChapter') self.assertIn(location, orphans) diff --git a/lms/djangoapps/courseware/access.py b/lms/djangoapps/courseware/access.py index 7cfa2bcb6b98..3a7de60ea72d 100644 --- a/lms/djangoapps/courseware/access.py +++ b/lms/djangoapps/courseware/access.py @@ -24,7 +24,6 @@ GlobalStaff, CourseStaffRole, CourseInstructorRole, OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole ) - DEBUG_ACCESS = False log = logging.getLogger(__name__) @@ -246,7 +245,7 @@ def can_load(): return True # Check start date - if descriptor.start is not None: + if 'detached' not in descriptor._class_tags and descriptor.start is not None: now = datetime.now(UTC()) effective_start = _adjust_start_date_for_beta_testers( user, diff --git a/lms/djangoapps/courseware/tests/test_about.py b/lms/djangoapps/courseware/tests/test_about.py new file mode 100644 index 000000000000..7a4a036a1760 --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_about.py @@ -0,0 +1,33 @@ +""" +Test the about xblock +""" +from django.test.utils import override_settings +from django.core.urlresolvers import reverse + +from .helpers import LoginEnrollmentTestCase +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class AboutTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): + def setUp(self): + self.course = CourseFactory.create() + self.about = ItemFactory.create( + category="about", parent_location=self.course.location, + data="OOGIE BLOOGIE", display_name="overview" + ) + + def test_logged_in(self): + self.setup_user() + url = reverse('about_course', args=[self.course.id]) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertIn("OOGIE BLOOGIE", resp.content) + + def test_anonymous_user(self): + url = reverse('about_course', args=[self.course.id]) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertIn("OOGIE BLOOGIE", resp.content) diff --git a/lms/djangoapps/courseware/tests/test_course_info.py b/lms/djangoapps/courseware/tests/test_course_info.py new file mode 100644 index 000000000000..b47c3a73cabf --- /dev/null +++ b/lms/djangoapps/courseware/tests/test_course_info.py @@ -0,0 +1,33 @@ +""" +Test the course_info xblock +""" +from django.test.utils import override_settings +from django.core.urlresolvers import reverse + +from .helpers import LoginEnrollmentTestCase +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory + + +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class CourseInfoTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): + def setUp(self): + self.course = CourseFactory.create() + self.page = ItemFactory.create( + category="course_info", parent_location=self.course.location, + data="OOGIE BLOOGIE", display_name="updates" + ) + + def test_logged_in(self): + self.setup_user() + url = reverse('info', args=[self.course.id]) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertIn("OOGIE BLOOGIE", resp.content) + + def test_anonymous_user(self): + url = reverse('info', args=[self.course.id]) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertNotIn("OOGIE BLOOGIE", resp.content) diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 94ebe611f353..501185714bee 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -8,8 +8,9 @@ from django.core.urlresolvers import reverse from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.factories import CourseFactory +from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from courseware.tests.modulestore_config import TEST_DATA_MIXED_MODULESTORE +from .helpers import LoginEnrollmentTestCase FAKE_REQUEST = None @@ -146,6 +147,29 @@ def test_static_tab(self): ) self.assertEqual(tab_list[0].is_active, False) +@override_settings(MODULESTORE=TEST_DATA_MIXED_MODULESTORE) +class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): + def setUp(self): + self.course = CourseFactory.create() + self.page = ItemFactory.create( + category="static_tab", parent_location=self.course.location, + data="OOGIE BLOOGIE", display_name="new_tab" + ) + + def test_logged_in(self): + self.setup_user() + url = reverse('static_tab', args=[self.course.id, 'new_tab']) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertIn("OOGIE BLOOGIE", resp.content) + + def test_anonymous_user(self): + url = reverse('static_tab', args=[self.course.id, 'new_tab']) + resp = self.client.get(url) + self.assertEqual(resp.status_code, 200) + self.assertIn("OOGIE BLOOGIE", resp.content) + + class TextbooksTestCase(TestCase): def setUp(self):