Merge pull request #2209 from edx/db/lms_date_agnostic

Detached categories don't access control on date

cms/djangoapps/contentstore/views/item.py

@@ -40,9 +40,6 @@ __all__ = ['orphan_handler', 'xblock_handler']
 
 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})

common/lib/xmodule/xmodule/html_module.py

@@ -3,14 +3,11 @@ from fs.errors import ResourceNotFoundError
 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.x_module import XModule
 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="<ol></ol>",
         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

common/lib/xmodule/xmodule/modulestore/mongo/base.py

@@ -34,6 +34,7 @@ from xmodule.modulestore import ModuleStoreWriteBase, Location, MONGO_MODULESTOR
 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 @@ class MongoModuleStore(ModuleStoreWriteBase):
         """
         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,

common/lib/xmodule/xmodule/modulestore/split_mongo/split.py

@@ -439,12 +439,13 @@ class SplitMongoModuleStore(ModuleStoreWriteBase):
                 )
                 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'])

common/lib/xmodule/xmodule/modulestore/tests/test_orphan.py

@@ -135,7 +135,7 @@ class TestOrphan(unittest.TestCase):
         """
         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 @@ class TestOrphan(unittest.TestCase):
         """
         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)

lms/djangoapps/courseware/access.py

@@ -24,7 +24,6 @@ from student.roles import (
     GlobalStaff, CourseStaffRole, CourseInstructorRole,
     OrgStaffRole, OrgInstructorRole, CourseBetaTesterRole
 )
-
 DEBUG_ACCESS = False
 
 log = logging.getLogger(__name__)
@@ -246,7 +245,7 @@ def _has_access_descriptor(user, descriptor, action, course_context=None):
             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,

lms/djangoapps/courseware/tests/test_about.py

+"""
+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)

lms/djangoapps/courseware/tests/test_course_info.py

+"""
+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)

lms/djangoapps/courseware/tests/test_tabs.py

@@ -8,8 +8,9 @@ from django.test.utils import override_settings
 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 @@ class StaticTabTestCase(TestCase):
         )
         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):