diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 06addcbe0e99350056a349bdea71df158ef539ff..524729ba410c7dddc5bd28af02ad93f1d1ca14d6 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -23,7 +23,7 @@ from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent from xmodule.tabs import CourseTab -from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager +from openedx.core.lib.course_tabs import CourseTabPluginManager from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError from opaque_keys import InvalidKeyError @@ -998,7 +998,7 @@ def _refresh_course_tabs(request, course_module): Adds or removes a course tab based upon whether it is enabled. """ tab_panel = { - "type": tab_type.name, + "type": tab_type.type, "name": tab_type.title, } has_tab = tab_panel in tabs @@ -1010,7 +1010,7 @@ def _refresh_course_tabs(request, course_module): course_tabs = copy.copy(course_module.tabs) # Additionally update any tabs that are provided by non-dynamic course views - for tab_type in CourseViewTypeManager.get_course_view_types(): + for tab_type in CourseTabPluginManager.get_tab_types(): if not tab_type.is_dynamic and tab_type.is_default: tab_enabled = tab_type.is_enabled(course_module, user=request.user) update_tab(course_tabs, tab_type, tab_enabled) diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 36def116e4a67cce3b87f63a865b2e5cfc8dd32f..6d5cdf9ba3bb5bf66013b426283e159eedc1bb59 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -12,13 +12,13 @@ from django.http import HttpResponse from django.shortcuts import redirect from django.utils.translation import ugettext as _ -from openedx.core.djangoapps.course_views.course_views import StaticTab from edxmako.shortcuts import render_to_string, render_to_response from opaque_keys.edx.keys import UsageKey from xblock.core import XBlock import dogstats_wrapper as dog_stats_api from xmodule.modulestore.django import modulestore from xmodule.x_module import DEPRECATION_VSCOMPAT_EVENT +from xmodule.tabs import StaticTab from contentstore.utils import reverse_course_url, reverse_library_url, reverse_usage_url from models.settings.course_grading import CourseGradingModel diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index eecb80672bf0eeb5f87ae7e93cf7f2895483904c..6afa749aaed0b68c5f56ea11b4baf13995846fd7 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -14,9 +14,8 @@ from django.views.decorators.http import require_http_methods from edxmako.shortcuts import render_to_response from xmodule.modulestore.django import modulestore from xmodule.modulestore import ModuleStoreEnum -from xmodule.tabs import CourseTabList, CourseTab, InvalidTabsException +from xmodule.tabs import CourseTabList, CourseTab, InvalidTabsException, StaticTab from opaque_keys.edx.keys import CourseKey, UsageKey -from openedx.core.djangoapps.course_views.course_views import StaticTab from ..utils import get_lms_link_for_item diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index b1bec3663cc8d5f32e509ac8c123b768c7622de1..6ad7d6f4d4b8481f9465ac316a2c4e80d22ceffb 100644 --- a/cms/templates/edit-tabs.html +++ b/cms/templates/edit-tabs.html @@ -4,7 +4,7 @@ <%! from django.utils.translation import ugettext as _ from django.core.urlresolvers import reverse - from openedx.core.djangoapps.course_views.course_views import StaticTab + from xmodule.tabs import StaticTab from django.template.defaultfilters import escapejs %> <%block name="title">${_("Pages")}</%block> diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py index fe9056b09a1d7255ea8048868dfeb5adda158fc2..2361f94b9d07df28c4c252bac2938a4a6a4254b0 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_cross_modulestore_import_export.py @@ -21,6 +21,7 @@ from tempfile import mkdtemp import ddt from nose.plugins.attrib import attr +from mock import patch from xmodule.tests import CourseComparisonTest from xmodule.modulestore.mongo.base import ModuleStoreEnum @@ -31,6 +32,7 @@ from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.modulestore.xml_exporter import export_course_to_xml from xmodule.modulestore.split_mongo.split_draft import DraftVersioningModuleStore from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST +from xmodule.modulestore.tests.utils import mock_tab_from_json from xmodule.modulestore.inheritance import InheritanceMixin from xmodule.partitions.tests.test_partitions import PartitionTestCase from xmodule.x_module import XModuleMixin @@ -365,6 +367,7 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase): self.export_dir = mkdtemp() self.addCleanup(rmtree, self.export_dir, ignore_errors=True) + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) @ddt.data(*itertools.product( MODULESTORE_SETUPS, MODULESTORE_SETUPS, @@ -373,7 +376,10 @@ class CrossStoreXMLRoundtrip(CourseComparisonTest, PartitionTestCase): COURSE_DATA_NAMES, )) @ddt.unpack - def test_round_trip(self, source_builder, dest_builder, source_content_builder, dest_content_builder, course_data_name): + def test_round_trip( + self, source_builder, dest_builder, source_content_builder, + dest_content_builder, course_data_name, _mock_tab_from_json + ): # Construct the contentstore for storing the first import with source_content_builder.build() as source_content: # Construct the modulestore for storing the first import (using the previously created contentstore) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py index 548440cca26ad15ebfb0b27c535a652b9d274941..7840548a09c6b8051cce7cb0fa59d39c7f621a63 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mixed_modulestore.py @@ -11,6 +11,7 @@ import mimetypes from unittest import skip from uuid import uuid4 from contextlib import contextmanager +from mock import patch # Mixed modulestore depends on django, so we'll manually configure some django settings # before importing the module @@ -47,7 +48,7 @@ from xmodule.modulestore.mixed import MixedModuleStore from xmodule.modulestore.search import path_to_location, navigation_index from xmodule.modulestore.tests.factories import check_mongo_calls, check_exact_number_of_calls, \ mongo_uses_error_check -from xmodule.modulestore.tests.utils import create_modulestore_instance, LocationMixin +from xmodule.modulestore.tests.utils import create_modulestore_instance, LocationMixin, mock_tab_from_json from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.tests import DATA_DIR, CourseComparisonTest @@ -2057,8 +2058,9 @@ class TestMixedModuleStore(CommonMixedModuleStoreSetup): self.store.clone_course(course_key, dest_course_id, self.user_id) self.assertEqual(receiver.call_count, 1) + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) @ddt.data(ModuleStoreEnum.Type.mongo, ModuleStoreEnum.Type.split) - def test_course_publish_signal_import_firing(self, default): + def test_course_publish_signal_import_firing(self, default, _from_json): with MongoContentstoreBuilder().build() as contentstore: self.store = MixedModuleStore( contentstore=contentstore, diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index 4e29d658034e7abe42df576197f1c50a2c174e2c..4996449c4f116e8222a6d35557046c11aeb15d76 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -17,6 +17,7 @@ from uuid import uuid4 from datetime import datetime from pytz import UTC import unittest +from mock import patch from xblock.core import XBlock from xblock.fields import Scope, Reference, ReferenceList, ReferenceValueDict @@ -41,7 +42,7 @@ from git.test.lib.asserts import assert_not_none from xmodule.x_module import XModuleMixin from xmodule.modulestore.mongo.base import as_draft from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST -from xmodule.modulestore.tests.utils import LocationMixin +from xmodule.modulestore.tests.utils import LocationMixin, mock_tab_from_json from xmodule.modulestore.edit_info import EditInfoMixin from xmodule.modulestore.exceptions import ItemNotFoundError from xmodule.modulestore.inheritance import InheritanceMixin @@ -129,36 +130,38 @@ class TestMongoModuleStoreBase(unittest.TestCase): xblock_mixins=(EditInfoMixin, InheritanceMixin, LocationMixin, XModuleMixin) ) - import_course_from_xml( - draft_store, - 999, - DATA_DIR, - cls.courses, - static_content_store=content_store - ) - # also test a course with no importing of static content - import_course_from_xml( - draft_store, - 999, - DATA_DIR, - ['test_import_course'], - static_content_store=content_store, - do_import_static=False, - verbose=True - ) + with patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json): + import_course_from_xml( + draft_store, + 999, + DATA_DIR, + cls.courses, + static_content_store=content_store + ) - # also import a course under a different course_id (especially ORG) - import_course_from_xml( - draft_store, - 999, - DATA_DIR, - ['test_import_course'], - static_content_store=content_store, - do_import_static=False, - verbose=True, - target_id=SlashSeparatedCourseKey('guestx', 'foo', 'bar') - ) + # also test a course with no importing of static content + import_course_from_xml( + draft_store, + 999, + DATA_DIR, + ['test_import_course'], + static_content_store=content_store, + do_import_static=False, + verbose=True + ) + + # also import a course under a different course_id (especially ORG) + import_course_from_xml( + draft_store, + 999, + DATA_DIR, + ['test_import_course'], + static_content_store=content_store, + do_import_static=False, + verbose=True, + target_id=SlashSeparatedCourseKey('guestx', 'foo', 'bar') + ) return content_store, draft_store @@ -203,7 +206,8 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): ) assert_equals(store.get_modulestore_type(''), ModuleStoreEnum.Type.mongo) - def test_get_courses(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_get_courses(self, _from_json): '''Make sure the course objects loaded properly''' courses = self.draft_store.get_courses() @@ -241,7 +245,8 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): assert_false(self.draft_store.has_course(mix_cased)) assert_true(self.draft_store.has_course(mix_cased, ignore_case=True)) - def test_get_org_courses(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_get_org_courses(self, _from_json): """ Make sure that we can query for a filtered list of courses for a given ORG """ @@ -437,7 +442,8 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): {'displayname': 'hello'} ) - def test_get_courses_for_wiki(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_get_courses_for_wiki(self, _from_json): """ Test the get_courses_for_wiki method """ @@ -552,7 +558,8 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): check_xblock_fields() check_mongo_fields() - def test_export_course_image(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_export_course_image(self, _from_json): """ Test to make sure that we have a course image in the contentstore, then export it to ensure it gets copied to both file locations. @@ -571,7 +578,8 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): finally: shutil.rmtree(root_dir) - def test_export_course_image_nondefault(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_export_course_image_nondefault(self, _from_json): """ Make sure that if a non-default image path is specified that we don't export it to the static default location diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py index 5d2df978ddc4d7c6ecda72365817d99e629b363c..a47655f1a721c1f73d7dbee6c5c87b4179f594ad 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -30,6 +30,7 @@ from xmodule.modulestore.split_mongo.split import SplitMongoModuleStore from xmodule.modulestore.tests.test_modulestore import check_has_course_method from xmodule.modulestore.split_mongo import BlockKey from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST +from xmodule.modulestore.tests.utils import mock_tab_from_json from xmodule.modulestore.edit_info import EditInfoMixin @@ -37,14 +38,6 @@ BRANCH_NAME_DRAFT = ModuleStoreEnum.BranchName.draft BRANCH_NAME_PUBLISHED = ModuleStoreEnum.BranchName.published -def mock_tab_from_json(tab_dict): - """ - Mocks out the CourseTab.from_json to just return the tab_dict itself so that we don't have to deal - with plugin errors. - """ - return tab_dict - - @attr('mongo') class SplitModuleTest(unittest.TestCase): ''' @@ -567,7 +560,8 @@ class SplitModuleTest(unittest.TestCase): class TestHasChildrenAtDepth(SplitModuleTest): """Test the has_children_at_depth method of XModuleMixin. """ - def test_has_children_at_depth(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_has_children_at_depth(self, _from_json): course_locator = CourseLocator( org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_DRAFT ) @@ -628,7 +622,8 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertEqual(course.edited_by, "testassist@edx.org") self.assertDictEqual(course.grade_cutoffs, {"Pass": 0.45}) - def test_get_org_courses(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_get_org_courses(self, _from_json): courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT, org='guestx') # should have gotten 1 draft courses @@ -730,7 +725,8 @@ class SplitModuleCourseTests(SplitModuleTest): with self.assertRaises(ItemNotFoundError): modulestore().get_course(CourseLocator(org='testx', course='GreekHero', run="run", branch=BRANCH_NAME_PUBLISHED)) - def test_cache(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_cache(self, _from_json): """ Test that the mechanics of caching work. """ @@ -742,7 +738,8 @@ class SplitModuleCourseTests(SplitModuleTest): self.assertIn(BlockKey('chapter', 'chapter1'), block_map) self.assertIn(BlockKey('problem', 'problem3_2'), block_map) - def test_course_successors(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_course_successors(self, _from_json): """ get_course_successors(course_locator, version_history_depth=1) """ @@ -779,7 +776,8 @@ class SplitModuleItemTests(SplitModuleTest): Item read tests including inheritance ''' - def test_has_item(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_has_item(self, _from_json): ''' has_item(BlockUsageLocator) ''' @@ -843,7 +841,8 @@ class SplitModuleItemTests(SplitModuleTest): ) self.assertFalse(modulestore().has_item(locator)) - def test_get_item(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_get_item(self, _from_json): ''' get_item(blocklocator) ''' @@ -1001,7 +1000,8 @@ class SplitModuleItemTests(SplitModuleTest): parent = modulestore().get_parent_location(locator) self.assertIsNone(parent) - def test_get_children(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_get_children(self, _from_json): """ Test the existing get_children method on xdescriptors """ @@ -1354,7 +1354,8 @@ class TestItemCrud(SplitModuleTest): other_updated = modulestore().update_item(other_block, self.user_id) self.assertIn(moved_child.version_agnostic(), version_agnostic(other_updated.children)) - def test_update_definition(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_update_definition(self, _from_json): """ test updating an item's definition: ensure it gets versioned as well as the course getting versioned """ @@ -1625,7 +1626,8 @@ class TestCourseCreation(SplitModuleTest): fields['grading_policy']['GRADE_CUTOFFS'] ) - def test_update_course_index(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_update_course_index(self, _from_json): """ Test the versions pointers. NOTE: you can change the org, course, or other things, but it's not clear how you'd find them again or associate them w/ existing student history since @@ -1680,7 +1682,8 @@ class TestCourseCreation(SplitModuleTest): dupe_course_key.org, dupe_course_key.course, dupe_course_key.run, user, BRANCH_NAME_DRAFT ) - def test_bulk_ops_get_courses(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_bulk_ops_get_courses(self, _from_json): """ Test get_courses when some are created, updated, and deleted w/in a bulk operation """ @@ -1719,7 +1722,8 @@ class TestInheritance(SplitModuleTest): """ Test the metadata inheritance mechanism. """ - def test_inheritance(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_inheritance(self, _from_json): """ The actual test """ @@ -1799,7 +1803,8 @@ class TestPublish(SplitModuleTest): def tearDown(self): SplitModuleTest.tearDown(self) - def test_publish_safe(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_publish_safe(self, _from_json): """ Test the standard patterns: publish to new branch, revise and publish """ @@ -1868,7 +1873,8 @@ class TestPublish(SplitModuleTest): with self.assertRaises(ItemNotFoundError): modulestore().copy(self.user_id, source_course, destination_course, [problem1], []) - def test_move_delete(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_move_delete(self, _from_json): """ Test publishing moves and deletes. """ diff --git a/common/lib/xmodule/xmodule/modulestore/tests/utils.py b/common/lib/xmodule/xmodule/modulestore/tests/utils.py index ebf8efd767b79457354c01700d36f27d2d051935..5a62e0671011b8882f4bbfba0a035124bc239694 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/utils.py @@ -54,6 +54,14 @@ def create_modulestore_instance( ) +def mock_tab_from_json(tab_dict): + """ + Mocks out the CourseTab.from_json to just return the tab_dict itself so that we don't have to deal + with plugin errors. + """ + return tab_dict + + class LocationMixin(XBlockMixin): """ Adds a `location` property to an :class:`XBlock` so it is more compatible diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 973c24bdb0c2438bff92a2f77d14b80629d5cb78..f7e861539b2e97c488f2114dfd1e6a7715ecbdf4 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -28,54 +28,59 @@ class CourseTab(object): # subclass, shared by all instances of the subclass. type = '' + # The title of the tab, which should be internationalized + title = None + # Class property that specifies whether the tab can be hidden for a particular course is_hideable = False # Class property that specifies whether the tab is hidden for a particular course is_hidden = False + # The relative priority of this view that affects the ordering (lower numbers shown first) + priority = None + # Class property that specifies whether the tab can be moved within a course's list of tabs is_movable = True # Class property that specifies whether the tab is a collection of other tabs is_collection = False - def __init__(self, name, tab_id, link_func): - """ - Initializes class members with values passed in by subclasses. + # True if this tab is dynamically added to the list of tabs + is_dynamic = False - Args: - name: The name of the tab + # True if this tab is a default for the course (when enabled) + is_default = True + + # True if this tab can be included more than once for a course. + allow_multiple = False - tab_id: Intended to be a unique id for this tab, although it is currently not enforced - within this module. It is used by the UI to determine which page is active. + # If there is a single view associated with this tab, this is the name of it + view_name = None - link_func: A function that computes the link for the tab, - given the course and a reverse-url function as input parameters + def __init__(self, tab_dict): """ + Initializes class members with values passed in by subclasses. - self.name = name + Args: + tab_dict (dict) - a dictionary of parameters used to build the tab. + """ - self.tab_id = tab_id + self.name = tab_dict.get('name', self.title) + self.tab_id = tab_dict.get('tab_id', getattr(self, 'tab_id', self.type)) + self.link_func = tab_dict.get('link_func', link_reverse_func(self.view_name)) - self.link_func = link_func + self.is_hidden = tab_dict.get('is_hidden', False) - def is_enabled(self, course, user=None): # pylint: disable=unused-argument - """ - Determines whether the tab is enabled for the given course and a particular user. - This method is to be overridden by subclasses when applicable. The base class - implementation always returns True. + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + """Returns true if this course tab is enabled in the course. Args: - course: An xModule CourseDescriptor - - user: An optional user for whom the tab will be displayed. If none, - then the code should assume a staff user or an author. - - Returns: - A boolean value to indicate whether this instance of the tab is enabled. + course (CourseDescriptor): the course using the feature + user (User): an optional user interacting with the course (defaults to None) """ - return True + raise NotImplementedError() def get(self, key, default=None): """ @@ -98,6 +103,8 @@ class CourseTab(object): return self.type elif key == 'tab_id': return self.tab_id + elif key == 'is_hidden': + return self.is_hidden else: raise KeyError('Key {0} not present in tab {1}'.format(key, self.to_json())) @@ -112,6 +119,8 @@ class CourseTab(object): self.name = value elif key == 'tab_id': self.tab_id = value + elif key == 'is_hidden': + self.is_hidden = value else: raise KeyError('Key {0} cannot be set in tab {1}'.format(key, self.to_json())) @@ -129,8 +138,10 @@ class CourseTab(object): # allow tabs without names; if a name is required, its presence was checked in the validator. name_is_eq = (other.get('name') is None or self.name == other['name']) + is_hidden_eq = self.is_hidden == other.get('is_hidden', False) + # only compare the persisted/serialized members: 'type' and 'name' - return self.type == other.get('type') and name_is_eq + return self.type == other.get('type') and name_is_eq and is_hidden_eq def __ne__(self, other): """ @@ -170,7 +181,10 @@ class CourseTab(object): Returns: a dictionary with keys for the properties of the CourseTab object. """ - return {'type': self.type, 'name': self.name} + to_json_val = {'type': self.type, 'name': self.name} + if self.is_hidden: + to_json_val.update({'is_hidden': True}) + return to_json_val @staticmethod def from_json(tab_dict): @@ -191,22 +205,88 @@ class CourseTab(object): InvalidTabsException if the given tab doesn't have the right keys. """ # TODO: don't import openedx capabilities from common - from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager + from openedx.core.lib.course_tabs import CourseTabPluginManager tab_type_name = tab_dict.get('type') if tab_type_name is None: log.error('No type included in tab_dict: %r', tab_dict) return None try: - tab_type = CourseViewTypeManager.get_plugin(tab_type_name) + tab_type = CourseTabPluginManager.get_plugin(tab_type_name) except PluginError: log.exception( "Unknown tab type %r Known types: %r.", tab_type_name, - CourseViewTypeManager.get_course_view_types() + CourseTabPluginManager.get_tab_types() ) return None + tab_type.validate(tab_dict) - return tab_type.create_tab(tab_dict=tab_dict) + return tab_type(tab_dict=tab_dict) + + +class StaticTab(CourseTab): + """ + A custom tab. + """ + type = 'static_tab' + is_default = False # A static tab is never added to a course by default + allow_multiple = True + + def __init__(self, tab_dict=None, name=None, url_slug=None): + def link_func(course, reverse_func): + """ Returns a url for a given course and reverse function. """ + return reverse_func(self.type, args=[course.id.to_deprecated_string(), self.url_slug]) + + self.url_slug = tab_dict.get('url_slug') if tab_dict else url_slug + + if tab_dict is None: + tab_dict = dict() + + if name is not None: + tab_dict['name'] = name + + tab_dict['link_func'] = link_func + tab_dict['tab_id'] = 'static_tab_{0}'.format(self.url_slug) + + super(StaticTab, self).__init__(tab_dict) + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + """ + Static tabs are viewable to everyone, even anonymous users. + """ + return True + + @classmethod + def validate(cls, tab_dict, raise_error=True): + """ + Ensures that the specified tab_dict is valid. + """ + return (super(StaticTab, cls).validate(tab_dict, raise_error) + and key_checker(['name', 'url_slug'])(tab_dict, raise_error)) + + def __getitem__(self, key): + if key == 'url_slug': + return self.url_slug + else: + return super(StaticTab, self).__getitem__(key) + + def __setitem__(self, key, value): + if key == 'url_slug': + self.url_slug = value + else: + super(StaticTab, self).__setitem__(key, value) + + def to_json(self): + """ Return a dictionary representation of this tab. """ + to_json_val = super(StaticTab, self).to_json() + to_json_val.update({'url_slug': self.url_slug}) + return to_json_val + + def __eq__(self, other): + if not super(StaticTab, self).__eq__(other): + return False + return self.url_slug == other.get('url_slug') class CourseTabList(List): @@ -338,10 +418,10 @@ class CourseTabList(List): # the following tabs should appear only once # TODO: don't import openedx capabilities from common - from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager - for course_view_type in CourseViewTypeManager.get_course_view_types(): - if not course_view_type.allow_multiple: - cls._validate_num_tabs_of_type(tabs, course_view_type.name, 1) + from openedx.core.lib.course_tabs import CourseTabPluginManager + for tab_type in CourseTabPluginManager.get_tab_types(): + if not tab_type.allow_multiple: + cls._validate_num_tabs_of_type(tabs, tab_type.type, 1) @staticmethod def _validate_num_tabs_of_type(tabs, tab_type, max_num): @@ -411,6 +491,16 @@ def key_checker(expected_keys): return check +def link_reverse_func(reverse_name): + """ + Returns a function that takes in a course and reverse_url_func, + and calls the reverse_url_func with the given reverse_name and course's ID. + + This is used to generate the url for a CourseTab without having access to Django's reverse function. + """ + return lambda course, reverse_url_func: reverse_url_func(reverse_name, args=[course.id.to_deprecated_string()]) + + def need_name(dictionary, raise_error=True): """ Returns whether the 'name' key exists in the given dictionary. diff --git a/lms/djangoapps/ccx/plugins.py b/lms/djangoapps/ccx/plugins.py index 3479fa9a566bd11cd71bd4f3d3a124753b3a3e72..61d01009aacf8b4fe81ef2913ab3e2a4f3ab79e2 100644 --- a/lms/djangoapps/ccx/plugins.py +++ b/lms/djangoapps/ccx/plugins.py @@ -5,16 +5,16 @@ Registers the CCX feature for the edX platform. from django.conf import settings from django.utils.translation import ugettext as _ -from openedx.core.djangoapps.course_views.course_views import CourseViewType +from xmodule.tabs import CourseTab from student.roles import CourseCcxCoachRole -class CcxCourseViewType(CourseViewType): +class CcxCourseTab(CourseTab): """ - The representation of the CCX course view type. + The representation of the CCX course tab """ - name = "ccx_coach" + type = "ccx_coach" title = _("CCX Coach") view_name = "ccx_coach_dashboard" is_dynamic = True # The CCX view is dynamically added to the set of tabs when it is enabled diff --git a/lms/djangoapps/course_wiki/tab.py b/lms/djangoapps/course_wiki/tab.py index e5fbdee949ff9f9e53a22fca35b374b68c7bd220..eecc082dcb1325535987f5641cd0c51264ee04fb 100644 --- a/lms/djangoapps/course_wiki/tab.py +++ b/lms/djangoapps/course_wiki/tab.py @@ -6,15 +6,15 @@ a user has on an article. from django.conf import settings from django.utils.translation import ugettext as _ -from courseware.tabs import EnrolledCourseViewType +from courseware.tabs import EnrolledTab -class WikiCourseViewType(EnrolledCourseViewType): +class WikiTab(EnrolledTab): """ Defines the Wiki view type that is shown as a course tab. """ - name = "wiki" + type = "wiki" title = _('Wiki') view_name = "course_wiki" is_hideable = True @@ -28,4 +28,4 @@ class WikiCourseViewType(EnrolledCourseViewType): return False if course.allow_public_wiki_access: return True - return super(WikiCourseViewType, cls).is_enabled(course, user=user) + return super(WikiTab, cls).is_enabled(course, user=user) diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index 7bb9f15ea54418746eb7d87154e8282b4e4a3b34..94b4b25d8872fd7ef62a591a5616114c24a35008 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -7,12 +7,12 @@ from django.utils.translation import ugettext as _ from courseware.access import has_access from courseware.entrance_exams import user_must_complete_entrance_exam -from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager, CourseViewType, StaticTab +from openedx.core.lib.course_tabs import CourseTabPluginManager from student.models import CourseEnrollment from xmodule.tabs import CourseTab, CourseTabList, key_checker -class EnrolledCourseViewType(CourseViewType): +class EnrolledTab(CourseTab): """ A base class for any view types that require a user to be enrolled. """ @@ -23,22 +23,22 @@ class EnrolledCourseViewType(CourseViewType): return CourseEnrollment.is_enrolled(user, course.id) or has_access(user, 'staff', course, course.id) -class CoursewareViewType(EnrolledCourseViewType): +class CoursewareTab(EnrolledTab): """ The main courseware view. """ - name = 'courseware' + type = 'courseware' title = _('Courseware') priority = 10 view_name = 'courseware' is_movable = False -class CourseInfoViewType(CourseViewType): +class CourseInfoTab(CourseTab): """ The course info view. """ - name = 'course_info' + type = 'course_info' title = _('Course Info') priority = 20 view_name = 'info' @@ -50,27 +50,28 @@ class CourseInfoViewType(CourseViewType): return True -class SyllabusCourseViewType(EnrolledCourseViewType): +class SyllabusTab(EnrolledTab): """ A tab for the course syllabus. """ - name = 'syllabus' + type = 'syllabus' title = _('Syllabus') priority = 30 view_name = 'syllabus' + allow_multiple = True @classmethod def is_enabled(cls, course, user=None): # pylint: disable=unused-argument - if not super(SyllabusCourseViewType, cls).is_enabled(course, user=user): + if not super(SyllabusTab, cls).is_enabled(course, user=user): return False return getattr(course, 'syllabus_present', False) -class ProgressCourseViewType(EnrolledCourseViewType): +class ProgressTab(EnrolledTab): """ The course progress view. """ - name = 'progress' + type = 'progress' title = _('Progress') priority = 40 view_name = 'progress' @@ -78,12 +79,12 @@ class ProgressCourseViewType(EnrolledCourseViewType): @classmethod def is_enabled(cls, course, user=None): # pylint: disable=unused-argument - if not super(ProgressCourseViewType, cls).is_enabled(course, user=user): + if not super(ProgressTab, cls).is_enabled(course, user=user): return False return not course.hide_progress_tab -class TextbookCourseViewsBase(CourseViewType): +class TextbookTabsBase(CourseTab): """ Abstract class for textbook collection tabs classes. """ @@ -104,17 +105,17 @@ class TextbookCourseViewsBase(CourseViewType): raise NotImplementedError() -class TextbookCourseViews(TextbookCourseViewsBase): +class TextbookTabs(TextbookTabsBase): """ A tab representing the collection of all textbook tabs. """ - name = 'textbooks' + type = 'textbooks' priority = None view_name = 'book' @classmethod def is_enabled(cls, course, user=None): # pylint: disable=unused-argument - parent_is_enabled = super(TextbookCourseViews, cls).is_enabled(course, user) + parent_is_enabled = super(TextbookTabs, cls).is_enabled(course, user) return settings.FEATURES.get('ENABLE_TEXTBOOK') and parent_is_enabled @classmethod @@ -128,11 +129,11 @@ class TextbookCourseViews(TextbookCourseViewsBase): ) -class PDFTextbookCourseViews(TextbookCourseViewsBase): +class PDFTextbookTabs(TextbookTabsBase): """ A tab representing the collection of all PDF textbook tabs. """ - name = 'pdf_textbooks' + type = 'pdf_textbooks' priority = None view_name = 'pdf_book' @@ -147,11 +148,11 @@ class PDFTextbookCourseViews(TextbookCourseViewsBase): ) -class HtmlTextbookCourseViews(TextbookCourseViewsBase): +class HtmlTextbookTabs(TextbookTabsBase): """ A tab representing the collection of all Html textbook tabs. """ - name = 'html_textbooks' + type = 'html_textbooks' priority = None view_name = 'html_book' @@ -166,89 +167,6 @@ class HtmlTextbookCourseViews(TextbookCourseViewsBase): ) -class StaticCourseViewType(CourseViewType): - """ - The view type that shows a static tab. - """ - name = 'static_tab' - is_default = False # A static tab is never added to a course by default - allow_multiple = True - - @classmethod - def is_enabled(cls, course, user=None): # pylint: disable=unused-argument - """ - Static tabs are viewable to everyone, even anonymous users. - """ - return True - - @classmethod - def validate(cls, tab_dict, raise_error=True): - """ - Ensures that the specified tab_dict is valid. - """ - return (super(StaticCourseViewType, cls).validate(tab_dict, raise_error) - and key_checker(['name', 'url_slug'])(tab_dict, raise_error)) - - @classmethod - def create_tab(cls, tab_dict): - """ - Returns the tab that will be shown to represent an instance of a view. - """ - return StaticTab(tab_dict) - - -class ExternalDiscussionCourseViewType(EnrolledCourseViewType): - """ - A course view links to an external discussion service. - """ - - name = 'external_discussion' - # Translators: 'Discussion' refers to the tab in the courseware that leads to the discussion forums - title = _('Discussion') - priority = None - - @classmethod - def create_tab(cls, tab_dict): - """ - Returns the tab that will be shown to represent an instance of a view. - """ - return LinkTab(tab_dict, cls.title) - - @classmethod - def validate(cls, tab_dict, raise_error=True): - """ Validate that the tab_dict for this course view has the necessary information to render. """ - return (super(ExternalDiscussionCourseViewType, cls).validate(tab_dict, raise_error) and - key_checker(['link'])(tab_dict, raise_error)) - - @classmethod - def is_enabled(cls, course, user=None): # pylint: disable=unused-argument - if not super(ExternalDiscussionCourseViewType, cls).is_enabled(course, user=user): - return False - return course.discussion_link - - -class ExternalLinkCourseViewType(EnrolledCourseViewType): - """ - A course view containing an external link. - """ - name = 'external_link' - priority = None - is_default = False # An external link tab is not added to a course by default - - @classmethod - def create_tab(cls, tab_dict): - """ - Returns the tab that will be shown to represent an instance of a view. - """ - return LinkTab(tab_dict) - - @classmethod - def validate(cls, tab_dict, raise_error=True): - """ Validate that the tab_dict for this course view has the necessary information to render. """ - return (super(ExternalLinkCourseViewType, cls).validate(tab_dict, raise_error) and - key_checker(['link', 'name'])(tab_dict, raise_error)) - - class LinkTab(CourseTab): """ Abstract class for tabs that contain external links. @@ -264,11 +182,9 @@ class LinkTab(CourseTab): self.type = tab_dict['type'] - super(LinkTab, self).__init__( - name=tab_dict['name'] if tab_dict else name, - tab_id=None, - link_func=link_value_func, - ) + tab_dict['link_func'] = link_value_func + + super(LinkTab, self).__init__(tab_dict) def __getitem__(self, key): if key == 'link': @@ -292,6 +208,49 @@ class LinkTab(CourseTab): return False return self.link_value == other.get('link') + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + return True + + +class ExternalDiscussionCourseTab(LinkTab): + """ + A course tab that links to an external discussion service. + """ + + type = 'external_discussion' + # Translators: 'Discussion' refers to the tab in the courseware that leads to the discussion forums + title = _('Discussion') + priority = None + + @classmethod + def validate(cls, tab_dict, raise_error=True): + """ Validate that the tab_dict for this course tab has the necessary information to render. """ + return (super(ExternalDiscussionCourseTab, cls).validate(tab_dict, raise_error) and + key_checker(['link'])(tab_dict, raise_error)) + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + if not super(ExternalDiscussionCourseTab, cls).is_enabled(course, user=user): + return False + return course.discussion_link + + +class ExternalLinkCourseTab(LinkTab): + """ + A course tab containing an external link. + """ + type = 'external_link' + priority = None + is_default = False # An external link tab is not added to a course by default + allow_multiple = True + + @classmethod + def validate(cls, tab_dict, raise_error=True): + """ Validate that the tab_dict for this course tab has the necessary information to render. """ + return (super(ExternalLinkCourseTab, cls).validate(tab_dict, raise_error) and + key_checker(['link', 'name'])(tab_dict, raise_error)) + class SingleTextbookTab(CourseTab): """ @@ -308,7 +267,11 @@ class SingleTextbookTab(CourseTab): """ Constructs a link for textbooks from a view name, a course, and an index. """ return reverse_func(view_name, args=[unicode(course.id), index]) - super(SingleTextbookTab, self).__init__(name, tab_id, link_func) + tab_dict = dict() + tab_dict['name'] = name + tab_dict['tab_id'] = tab_id + tab_dict['link_func'] = link_func + super(SingleTextbookTab, self).__init__(tab_dict) def to_json(self): raise NotImplementedError('SingleTextbookTab should not be serialized.') @@ -347,9 +310,9 @@ def _get_dynamic_tabs(course, user): instead added dynamically based upon the user's role. """ dynamic_tabs = list() - for tab_type in CourseViewTypeManager.get_course_view_types(): + for tab_type in CourseTabPluginManager.get_tab_types(): if getattr(tab_type, "is_dynamic", False): - tab = tab_type.create_tab(dict()) + tab = tab_type(dict()) if tab.is_enabled(course, user=user): dynamic_tabs.append(tab) dynamic_tabs.sort(key=lambda dynamic_tab: dynamic_tab.name) diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index 27900d26360545bf0bb0b4d71495b6ba104d739c..8eeb3bfef56d34a7322e786a3e38ad52c1d788ca 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -11,8 +11,8 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.courses import get_course_by_id from courseware.tabs import ( - get_course_tab_list, CoursewareViewType, CourseInfoViewType, ProgressCourseViewType, - StaticCourseViewType, ExternalDiscussionCourseViewType, ExternalLinkCourseViewType + get_course_tab_list, CoursewareTab, CourseInfoTab, ProgressTab, + ExternalDiscussionCourseTab, ExternalLinkCourseTab ) from courseware.tests.helpers import get_request_for_user, LoginEnrollmentTestCase from courseware.tests.factories import InstructorFactory, StaffFactory @@ -85,7 +85,7 @@ class TabTestCase(ModuleStoreTestCase): Can be 'None' if the given tab class does not have any keys to validate. """ # create tab - tab = tab_class.create_tab(tab_dict=dict_tab) + tab = tab_class(tab_dict=dict_tab) # name is as expected self.assertEqual(tab.name, expected_name) @@ -475,17 +475,17 @@ class TabListTestCase(TabTestCase): # invalid tabs self.invalid_tabs = [ # less than 2 tabs - [{'type': CoursewareViewType.name}], + [{'type': CoursewareTab.type}], # missing course_info - [{'type': CoursewareViewType.name}, {'type': 'discussion', 'name': 'fake_name'}], + [{'type': CoursewareTab.type}, {'type': 'discussion', 'name': 'fake_name'}], # incorrect order - [{'type': CourseInfoViewType.name, 'name': 'fake_name'}, {'type': CoursewareViewType.name}], + [{'type': CourseInfoTab.type, 'name': 'fake_name'}, {'type': CoursewareTab.type}], ] # tab types that should appear only once unique_tab_types = [ - CoursewareViewType.name, - CourseInfoViewType.name, + CoursewareTab.type, + CourseInfoTab.type, 'textbooks', 'pdf_textbooks', 'html_textbooks', @@ -493,8 +493,8 @@ class TabListTestCase(TabTestCase): for unique_tab_type in unique_tab_types: self.invalid_tabs.append([ - {'type': CoursewareViewType.name}, - {'type': CourseInfoViewType.name, 'name': 'fake_name'}, + {'type': CoursewareTab.type}, + {'type': CourseInfoTab.type, 'name': 'fake_name'}, # add the unique tab multiple times {'type': unique_tab_type}, {'type': unique_tab_type}, @@ -502,26 +502,27 @@ class TabListTestCase(TabTestCase): # valid tabs self.valid_tabs = [ - # empty list + # any empty list is valid because a default list of tabs will be + # generated to replace the empty list. [], # all valid tabs [ - {'type': CoursewareViewType.name}, - {'type': CourseInfoViewType.name, 'name': 'fake_name'}, + {'type': CoursewareTab.type}, + {'type': CourseInfoTab.type, 'name': 'fake_name'}, {'type': 'discussion', 'name': 'fake_name'}, - {'type': ExternalLinkCourseViewType.name, 'name': 'fake_name', 'link': 'fake_link'}, + {'type': ExternalLinkCourseTab.type, 'name': 'fake_name', 'link': 'fake_link'}, {'type': 'textbooks'}, {'type': 'pdf_textbooks'}, {'type': 'html_textbooks'}, - {'type': ProgressCourseViewType.name, 'name': 'fake_name'}, - {'type': StaticCourseViewType.name, 'name': 'fake_name', 'url_slug': 'schlug'}, + {'type': ProgressTab.type, 'name': 'fake_name'}, + {'type': xmodule_tabs.StaticTab.type, 'name': 'fake_name', 'url_slug': 'schlug'}, {'type': 'syllabus'}, ], # with external discussion [ - {'type': CoursewareViewType.name}, - {'type': CourseInfoViewType.name, 'name': 'fake_name'}, - {'type': ExternalDiscussionCourseViewType.name, 'name': 'fake_name', 'link': 'fake_link'} + {'type': CoursewareTab.type}, + {'type': CourseInfoTab.type, 'name': 'fake_name'}, + {'type': ExternalDiscussionCourseTab.type, 'name': 'fake_name', 'link': 'fake_link'} ], ] @@ -550,8 +551,8 @@ class ValidateTabsTestCase(TabListTestCase): tab_list = xmodule_tabs.CourseTabList() self.assertEquals( len(tab_list.from_json([ - {'type': CoursewareViewType.name}, - {'type': CourseInfoViewType.name, 'name': 'fake_name'}, + {'type': CoursewareTab.type}, + {'type': CourseInfoTab.type, 'name': 'fake_name'}, {'type': 'no_such_type'} ])), 2 @@ -660,10 +661,10 @@ class ProgressTestCase(TabTestCase): def check_progress_tab(self): """Helper function for verifying the progress tab.""" return self.check_tab( - tab_class=ProgressCourseViewType, - dict_tab={'type': ProgressCourseViewType.name, 'name': 'same'}, + tab_class=ProgressTab, + dict_tab={'type': ProgressTab.type, 'name': 'same'}, expected_link=self.reverse('progress', args=[self.course.id.to_deprecated_string()]), - expected_tab_id=ProgressCourseViewType.name, + expected_tab_id=ProgressTab.type, invalid_dict_tab=None, ) @@ -692,8 +693,8 @@ class StaticTabTestCase(TabTestCase): url_slug = 'schmug' tab = self.check_tab( - tab_class=StaticCourseViewType, - dict_tab={'type': StaticCourseViewType.name, 'name': 'same', 'url_slug': url_slug}, + tab_class=xmodule_tabs.StaticTab, + dict_tab={'type': xmodule_tabs.StaticTab.type, 'name': 'same', 'url_slug': url_slug}, expected_link=self.reverse('static_tab', args=[self.course.id.to_deprecated_string(), url_slug]), expected_tab_id='static_tab_schmug', invalid_dict_tab=self.fake_dict_tab, diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index d478c3de00a438ec60224745c1014069680643c2..8fa1d642397cdce2b5b82dd52852abb6f87d440f 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -1135,9 +1135,9 @@ def notification_image_for_tab(course_tab, user, course): """ tab_notification_handlers = { - StaffGradingTab.name: open_ended_notifications.staff_grading_notifications, - PeerGradingTab.name: open_ended_notifications.peer_grading_notifications, - OpenEndedGradingTab.name: open_ended_notifications.combined_notifications + StaffGradingTab.type: open_ended_notifications.staff_grading_notifications, + PeerGradingTab.type: open_ended_notifications.peer_grading_notifications, + OpenEndedGradingTab.type: open_ended_notifications.combined_notifications } if course_tab.name in tab_notification_handlers: diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 44e3e4e1c371bcc99fca02345a68ad895a63f23e..bd99930a6017c834a663e1eff8d4d638b7903d8c 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -25,7 +25,7 @@ from openedx.core.djangoapps.course_groups.cohorts import ( get_course_cohorts, is_commentable_cohorted ) -from courseware.tabs import EnrolledCourseViewType +from courseware.tabs import EnrolledTab from courseware.access import has_access from xmodule.modulestore.django import modulestore from ccx.overrides import get_current_ccx @@ -49,19 +49,19 @@ PAGES_NEARBY_DELTA = 2 log = logging.getLogger("edx.discussions") -class DiscussionCourseViewType(EnrolledCourseViewType): +class DiscussionTab(EnrolledTab): """ A tab for the cs_comments_service forums. """ - name = 'discussion' + type = 'discussion' title = _('Discussion') priority = None view_name = 'django_comment_client.forum.views.forum_form_discussion' @classmethod def is_enabled(cls, course, user=None): - if not super(DiscussionCourseViewType, cls).is_enabled(course, user): + if not super(DiscussionTab, cls).is_enabled(course, user): return False if settings.FEATURES.get('CUSTOM_COURSES_EDX', False): diff --git a/lms/djangoapps/edxnotes/plugins.py b/lms/djangoapps/edxnotes/plugins.py index b99c5df5ee3d95cd35eda1485ff97b6078aa6ac4..f427a3e9a258078d4af9bf4ed524bedd67dfafbc 100644 --- a/lms/djangoapps/edxnotes/plugins.py +++ b/lms/djangoapps/edxnotes/plugins.py @@ -4,15 +4,15 @@ Registers the "edX Notes" feature for the edX platform. from django.utils.translation import ugettext as _ -from courseware.tabs import EnrolledCourseViewType +from courseware.tabs import EnrolledTab -class EdxNotesCourseViewType(EnrolledCourseViewType): +class EdxNotesTab(EnrolledTab): """ - The representation of the edX Notes course view type. + The representation of the edX Notes course tab type. """ - name = "edxnotes" + type = "edxnotes" title = _("Notes") view_name = "edxnotes" @@ -25,6 +25,6 @@ class EdxNotesCourseViewType(EnrolledCourseViewType): settings (dict): a dict of configuration settings user (User): the user interacting with the course """ - if not super(EdxNotesCourseViewType, cls).is_enabled(course, user=user): + if not super(EdxNotesTab, cls).is_enabled(course, user=user): return False return course.edxnotes diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index be24f588c56d7a7c9a52a944e89e8a2f96524535..a7abd41bdd8affd7710e562d03a67c78dcd6878f 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -26,6 +26,7 @@ from lms.djangoapps.lms_xblock.runtime import quote_slashes from openedx.core.lib.xblock_utils import wrap_xblock from xmodule.html_module import HtmlDescriptor from xmodule.modulestore.django import modulestore +from xmodule.tabs import CourseTab from xblock.field_data import DictFieldData from xblock.fields import ScopeIds from courseware.access import has_access @@ -38,7 +39,6 @@ from course_modes.models import CourseMode, CourseModesArchive from student.roles import CourseFinanceAdminRole, CourseSalesAdminRole from certificates.models import CertificateGenerationConfiguration from certificates import api as certs_api -from openedx.core.djangoapps.course_views.course_views import CourseViewType from class_dashboard.dashboard_data import get_section_display_name, get_array_section_has_problem from .tools import get_units_with_due_date, title_or_url, bulk_email_is_enabled_for_course @@ -47,12 +47,12 @@ from opaque_keys.edx.locations import SlashSeparatedCourseKey log = logging.getLogger(__name__) -class InstructorDashboardViewType(CourseViewType): +class InstructorDashboardTab(CourseTab): """ Defines the Instructor Dashboard view type that is shown as a course tab. """ - name = "instructor" + type = "instructor" title = _('Instructor') view_name = "instructor_dashboard" is_dynamic = True # The "Instructor" tab is instead dynamically added when it is enabled diff --git a/lms/djangoapps/notes/views.py b/lms/djangoapps/notes/views.py index f512ddc13b331ca2c9987218b446db01a917cb51..eedce977aa98c2785a8c76d6faee55ff47ff869e 100644 --- a/lms/djangoapps/notes/views.py +++ b/lms/djangoapps/notes/views.py @@ -9,7 +9,7 @@ from django.http import Http404 from edxmako.shortcuts import render_to_response from opaque_keys.edx.locations import SlashSeparatedCourseKey from courseware.courses import get_course_with_access -from courseware.tabs import EnrolledCourseViewType +from courseware.tabs import EnrolledTab from notes.models import Note from notes.utils import notes_enabled_for_course from xmodule.annotator_token import retrieve_token @@ -40,16 +40,16 @@ def notes(request, course_id): return render_to_response('notes.html', context) -class NotesCourseViewType(EnrolledCourseViewType): +class NotesTab(EnrolledTab): """ A tab for the course notes. """ - name = 'notes' + type = 'notes' title = _("My Notes") view_name = "notes" @classmethod def is_enabled(cls, course, user=None): - if not super(NotesCourseViewType, cls).is_enabled(course, user): + if not super(NotesTab, cls).is_enabled(course, user): return False return settings.FEATURES.get('ENABLE_STUDENT_NOTES') and "notes" in course.advanced_modules diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 6e1285b5011cfa91c778ca41c57f0da5a339ebea..002023685b80092996dc898e3770659b207e3eb9 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -4,11 +4,10 @@ from django.views.decorators.cache import cache_control from edxmako.shortcuts import render_to_response from django.core.urlresolvers import reverse -from openedx.core.djangoapps.course_views.course_views import CourseViewType from courseware.courses import get_course_with_access from courseware.access import has_access -from courseware.tabs import EnrolledCourseViewType +from courseware.tabs import EnrolledTab from xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError import json @@ -66,11 +65,11 @@ ALERT_DICT = { } -class StaffGradingTab(CourseViewType): +class StaffGradingTab(EnrolledTab): """ A tab for staff grading. """ - name = 'staff_grading' + type = 'staff_grading' title = _("Staff grading") view_name = "staff_grading" @@ -81,11 +80,11 @@ class StaffGradingTab(CourseViewType): return "combinedopenended" in course.advanced_modules -class PeerGradingTab(EnrolledCourseViewType): +class PeerGradingTab(EnrolledTab): """ A tab for peer grading. """ - name = 'peer_grading' + type = 'peer_grading' # Translators: "Peer grading" appears on a tab that allows # students to view open-ended problems that require grading title = _("Peer grading") @@ -98,11 +97,11 @@ class PeerGradingTab(EnrolledCourseViewType): return "combinedopenended" in course.advanced_modules -class OpenEndedGradingTab(EnrolledCourseViewType): +class OpenEndedGradingTab(EnrolledTab): """ A tab for open ended grading. """ - name = 'open_ended' + type = 'open_ended' # Translators: "Open Ended Panel" appears on a tab that, when clicked, opens up a panel that # displays information about open-ended problems that a user has submitted or needs to grade title = _("Open Ended Panel") diff --git a/lms/djangoapps/teams/plugins.py b/lms/djangoapps/teams/plugins.py index f010e97ecb59c1e09df6205cd5c12dad2bc88020..9ff162288ec03bd723104a42ed8e39cdc6be2c7c 100644 --- a/lms/djangoapps/teams/plugins.py +++ b/lms/djangoapps/teams/plugins.py @@ -3,16 +3,16 @@ Definition of the course team feature. """ from django.utils.translation import ugettext as _ -from courseware.tabs import EnrolledCourseViewType +from courseware.tabs import EnrolledTab from .views import is_feature_enabled -class TeamsCourseViewType(EnrolledCourseViewType): +class TeamsTab(EnrolledTab): """ The representation of the course teams view type. """ - name = "teams" + type = "teams" title = _("Teams") view_name = "teams_dashboard" @@ -24,7 +24,7 @@ class TeamsCourseViewType(EnrolledCourseViewType): course (CourseDescriptor): the course using the feature user (User): the user interacting with the course """ - if not super(TeamsCourseViewType, cls).is_enabled(course, user=user): + if not super(TeamsTab, cls).is_enabled(course, user=user): return False return is_feature_enabled(course) diff --git a/openedx/core/djangoapps/course_views/__init__.py b/openedx/core/djangoapps/course_views/__init__.py deleted file mode 100644 index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000 diff --git a/openedx/core/djangoapps/course_views/course_views.py b/openedx/core/djangoapps/course_views/course_views.py deleted file mode 100644 index 976546b462ddd8ee7bff653704af5ed7b39aff1d..0000000000000000000000000000000000000000 --- a/openedx/core/djangoapps/course_views/course_views.py +++ /dev/null @@ -1,201 +0,0 @@ -""" -Tabs for courseware. -""" -from openedx.core.lib.api.plugins import PluginManager -from xmodule.tabs import CourseTab - -_ = lambda text: text - - -# Stevedore extension point namespaces -COURSE_VIEW_TYPE_NAMESPACE = 'openedx.course_view_type' - - -def link_reverse_func(reverse_name): - """ - Returns a function that takes in a course and reverse_url_func, - and calls the reverse_url_func with the given reverse_name and course' ID. - """ - return lambda course, reverse_url_func: reverse_url_func(reverse_name, args=[course.id.to_deprecated_string()]) - - -class CourseViewType(object): - """ - Base class of all course view type plugins. - - These are responsible for defining tabs that can be displayed in the courseware. In order to create - and register a new CourseViewType. Create a class (either in edx-platform or in a pip installable library) - that inherits from CourseViewType and create a new entry in setup.py. - - For example: - - entry_points={ - "openedx.course_view_type": [ - "new_view = my_feature.NewCourseViewType", - ], - } - - """ - name = None # The name of the view type, which is used for persistence and view type lookup - title = None # The title of the view, which should be internationalized - priority = None # The relative priority of this view that affects the ordering (lower numbers shown first) - view_name = None # The name of the Django view to show this view - tab_id = None # The id to be used to show a tab for this view - is_movable = True # True if this course view can be moved - is_dynamic = False # True if this course view is dynamically added to the list of tabs - is_default = True # True if this course view is a default for the course (when enabled) - is_hideable = False # True if this course view's visibility can be toggled by the author - allow_multiple = False # True if this tab can be included more than once for a course. - - @classmethod - def is_enabled(cls, course, user=None): # pylint: disable=unused-argument - """Returns true if this course view is enabled in the course. - - Args: - course (CourseDescriptor): the course using the feature - user (User): an optional user interacting with the course (defaults to None) - """ - raise NotImplementedError() - - @classmethod - def validate(cls, tab_dict, raise_error=True): # pylint: disable=unused-argument - """ - Validates the given dict-type `tab_dict` object to ensure it contains the expected keys. - This method should be overridden by subclasses that require certain keys to be persisted in the tab. - """ - return True - - @classmethod - def create_tab(cls, tab_dict): - """ - Returns the tab that will be shown to represent an instance of a view. - """ - return CourseViewTab(cls, tab_dict=tab_dict) - - -class CourseViewTypeManager(PluginManager): - """ - Manager for all of the course view types that have been made available. - - All course view types should implement `CourseViewType`. - """ - NAMESPACE = COURSE_VIEW_TYPE_NAMESPACE - - @classmethod - def get_course_view_types(cls): - """ - Returns the list of available course view types in their canonical order. - """ - def compare_course_view_types(first_type, second_type): - """Compares two course view types, for use in sorting.""" - first_priority = first_type.priority - second_priority = second_type.priority - if not first_priority == second_priority: - if not first_priority: - return 1 - elif not second_priority: - return -1 - else: - return first_priority - second_priority - first_name = first_type.name - second_name = second_type.name - if first_name < second_name: - return -1 - elif first_name == second_name: - return 0 - else: - return 1 - course_view_types = cls.get_available_plugins().values() - course_view_types.sort(cmp=compare_course_view_types) - return course_view_types - - -class CourseViewTab(CourseTab): - """ - A tab that renders a course view. - """ - - def __init__(self, course_view_type, tab_dict=None): - super(CourseViewTab, self).__init__( - name=tab_dict.get('name', course_view_type.title) if tab_dict else course_view_type.title, - tab_id=course_view_type.tab_id if course_view_type.tab_id else course_view_type.name, - link_func=link_reverse_func(course_view_type.view_name), - ) - self.type = course_view_type.name - self.course_view_type = course_view_type - self.is_hideable = course_view_type.is_hideable - self.is_hidden = tab_dict.get('is_hidden', False) if tab_dict else False - self.is_collection = course_view_type.is_collection if hasattr(course_view_type, 'is_collection') else False - self.is_movable = course_view_type.is_movable - - def is_enabled(self, course, user=None): - """ Returns True if the tab has been enabled for this course and this user, False otherwise. """ - if not super(CourseViewTab, self).is_enabled(course, user=user): - return False - return self.course_view_type.is_enabled(course, user=user) - - def __getitem__(self, key): - if key == 'is_hidden': - return self.is_hidden - else: - return super(CourseViewTab, self).__getitem__(key) - - def __setitem__(self, key, value): - if key == 'is_hidden': - self.is_hidden = value - else: - super(CourseViewTab, self).__setitem__(key, value) - - def to_json(self): - """ Return a dictionary representation of this tab. """ - to_json_val = super(CourseViewTab, self).to_json() - if self.is_hidden: - to_json_val.update({'is_hidden': True}) - return to_json_val - - def items(self, course): - """ If this tab is a collection, this will fetch the items in the collection. """ - for item in self.course_view_type.items(course): - yield item - - -class StaticTab(CourseTab): - """ - A custom tab. - """ - type = 'static_tab' - - def __init__(self, tab_dict=None, name=None, url_slug=None): - def link_func(course, reverse_func): - """ Returns a url for a given course and reverse function. """ - return reverse_func(self.type, args=[course.id.to_deprecated_string(), self.url_slug]) - - self.url_slug = tab_dict['url_slug'] if tab_dict else url_slug - super(StaticTab, self).__init__( - name=tab_dict['name'] if tab_dict else name, - tab_id='static_tab_{0}'.format(self.url_slug), - link_func=link_func, - ) - - def __getitem__(self, key): - if key == 'url_slug': - return self.url_slug - else: - return super(StaticTab, self).__getitem__(key) - - def __setitem__(self, key, value): - if key == 'url_slug': - self.url_slug = value - else: - super(StaticTab, self).__setitem__(key, value) - - def to_json(self): - """ Return a dictionary representation of this tab. """ - to_json_val = super(StaticTab, self).to_json() - to_json_val.update({'url_slug': self.url_slug}) - return to_json_val - - def __eq__(self, other): - if not super(StaticTab, self).__eq__(other): - return False - return self.url_slug == other.get('url_slug') diff --git a/openedx/core/djangoapps/course_views/tests/__init__.py b/openedx/core/djangoapps/course_views/tests/__init__.py deleted file mode 100644 index e69de29bb2d1d6434b8b29ae775ad8c2e48c5391..0000000000000000000000000000000000000000 diff --git a/openedx/core/lib/course_tabs.py b/openedx/core/lib/course_tabs.py new file mode 100644 index 0000000000000000000000000000000000000000..c3a2ea5a06a742d6831ff0d4a8bd13895f4e6866 --- /dev/null +++ b/openedx/core/lib/course_tabs.py @@ -0,0 +1,47 @@ +""" +Tabs for courseware. +""" +from openedx.core.lib.api.plugins import PluginManager + +_ = lambda text: text + + +# Stevedore extension point namespaces +COURSE_TAB_NAMESPACE = 'openedx.course_tab' + + +class CourseTabPluginManager(PluginManager): + """ + Manager for all of the course tabs that have been made available. + + All course tabs should implement `CourseTab`. + """ + NAMESPACE = COURSE_TAB_NAMESPACE + + @classmethod + def get_tab_types(cls): + """ + Returns the list of available course tabs in their canonical order. + """ + def compare_tabs(first_type, second_type): + """Compares two course tabs, for use in sorting.""" + first_priority = first_type.priority + second_priority = second_type.priority + if first_priority != second_priority: + if first_priority is None: + return 1 + elif second_priority is None: + return -1 + else: + return first_priority - second_priority + first_type = first_type.type + second_type = second_type.type + if first_type < second_type: + return -1 + elif first_type == second_type: + return 0 + else: + return 1 + tab_types = cls.get_available_plugins().values() + tab_types.sort(cmp=compare_tabs) + return tab_types diff --git a/openedx/core/djangoapps/course_views/tests/test_api.py b/openedx/core/lib/tests/test_course_tab_api.py similarity index 55% rename from openedx/core/djangoapps/course_views/tests/test_api.py rename to openedx/core/lib/tests/test_course_tab_api.py index 4f558a10021501c0d55110af17facc48a660d767..e6cc09f1f090c763ea9391f81000191128a74254 100644 --- a/openedx/core/djangoapps/course_views/tests/test_api.py +++ b/openedx/core/lib/tests/test_course_tab_api.py @@ -5,7 +5,7 @@ Tests for the plugin API from django.test import TestCase from openedx.core.lib.api.plugins import PluginError -from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager +from openedx.core.lib.course_tabs import CourseTabPluginManager class TestPluginApi(TestCase): @@ -17,8 +17,8 @@ class TestPluginApi(TestCase): """ Verify that get_plugin works as expected. """ - course_view_type = CourseViewTypeManager.get_plugin("instructor") - self.assertEqual(course_view_type.title, "Instructor") + tab_type = CourseTabPluginManager.get_plugin("instructor") + self.assertEqual(tab_type.title, "Instructor") with self.assertRaises(PluginError): - CourseViewTypeManager.get_plugin("no_such_type") + CourseTabPluginManager.get_plugin("no_such_type") diff --git a/openedx/core/djangoapps/course_views/tests/test_course_views.py b/openedx/core/lib/tests/test_course_tabs.py similarity index 67% rename from openedx/core/djangoapps/course_views/tests/test_course_views.py rename to openedx/core/lib/tests/test_course_tabs.py index 04f5bf0b457567ed25d8b92558993912bd1e6963..6bb56e1c89eea9ac8a371860ade01d7342dc29b9 100644 --- a/openedx/core/djangoapps/course_views/tests/test_course_views.py +++ b/openedx/core/lib/tests/test_course_tabs.py @@ -5,34 +5,34 @@ from unittest import TestCase import xmodule.tabs as xmodule_tabs -from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager +from openedx.core.lib.course_tabs import CourseTabPluginManager -class CourseViewTypeManagerTestCase(TestCase): - """Test cases for CourseViewTypeManager class""" +class CourseTabPluginManagerTestCase(TestCase): + """Test cases for CourseTabPluginManager class""" - @patch('openedx.core.djangoapps.course_views.course_views.CourseViewTypeManager.get_available_plugins') - def test_get_course_view_types(self, get_available_plugins): + @patch('openedx.core.lib.course_tabs.CourseTabPluginManager.get_available_plugins') + def test_get_tab_types(self, get_available_plugins): """ Verify that get_course_view_types sorts appropriately """ - def create_mock_plugin(name, priority): + def create_mock_plugin(tab_type, priority): """ Create a mock plugin with the specified name and priority. """ mock_plugin = Mock() - mock_plugin.name = name + mock_plugin.type = tab_type mock_plugin.priority = priority return mock_plugin mock_plugins = { - "Last": create_mock_plugin(name="Last", priority=None), - "Duplicate1": create_mock_plugin(name="Duplicate", priority=None), - "Duplicate2": create_mock_plugin(name="Duplicate", priority=None), - "First": create_mock_plugin(name="First", priority=1), - "Second": create_mock_plugin(name="Second", priority=1), - "Third": create_mock_plugin(name="Third", priority=3), + "Last": create_mock_plugin(tab_type="Last", priority=None), + "Duplicate1": create_mock_plugin(tab_type="Duplicate", priority=None), + "Duplicate2": create_mock_plugin(tab_type="Duplicate", priority=None), + "First": create_mock_plugin(tab_type="First", priority=1), + "Second": create_mock_plugin(tab_type="Second", priority=1), + "Third": create_mock_plugin(tab_type="Third", priority=3), } get_available_plugins.return_value = mock_plugins self.assertEqual( - [plugin.name for plugin in CourseViewTypeManager.get_course_view_types()], + [plugin.type for plugin in CourseTabPluginManager.get_tab_types()], ["First", "Second", "Third", "Duplicate", "Duplicate", "Last"] ) diff --git a/setup.py b/setup.py index 9a075ec92a6a4959a01b7907a5026da6a6f24481..6dbbdd90aa467b25203038b0d4af0178591e90af 100644 --- a/setup.py +++ b/setup.py @@ -6,7 +6,7 @@ from setuptools import setup setup( name="Open edX", - version="0.3", + version="0.4", install_requires=["distribute"], requires=[], # NOTE: These are not the names we should be installing. This tree should @@ -18,24 +18,24 @@ setup( "cms", ], entry_points={ - "openedx.course_view_type": [ - "ccx = lms.djangoapps.ccx.plugins:CcxCourseViewType", - "courseware = lms.djangoapps.courseware.tabs:CoursewareViewType", - "course_info = lms.djangoapps.courseware.tabs:CourseInfoViewType", - "discussion = lms.djangoapps.django_comment_client.forum.views:DiscussionCourseViewType", - "edxnotes = lms.djangoapps.edxnotes.plugins:EdxNotesCourseViewType", - "external_discussion = lms.djangoapps.courseware.tabs:ExternalDiscussionCourseViewType", - "external_link = lms.djangoapps.courseware.tabs:ExternalLinkCourseViewType", - "html_textbooks = lms.djangoapps.courseware.tabs:HtmlTextbookCourseViews", - "instructor = lms.djangoapps.instructor.views.instructor_dashboard:InstructorDashboardViewType", - "notes = lms.djangoapps.notes.views:NotesCourseViewType", - "pdf_textbooks = lms.djangoapps.courseware.tabs:PDFTextbookCourseViews", - "progress = lms.djangoapps.courseware.tabs:ProgressCourseViewType", - "static_tab = lms.djangoapps.courseware.tabs:StaticCourseViewType", - "syllabus = lms.djangoapps.courseware.tabs:SyllabusCourseViewType", - "teams = lms.djangoapps.teams.plugins:TeamsCourseViewType", - "textbooks = lms.djangoapps.courseware.tabs:TextbookCourseViews", - "wiki = lms.djangoapps.course_wiki.tab:WikiCourseViewType", + "openedx.course_tab": [ + "ccx = lms.djangoapps.ccx.plugins:CcxCourseTab", + "courseware = lms.djangoapps.courseware.tabs:CoursewareTab", + "course_info = lms.djangoapps.courseware.tabs:CourseInfoTab", + "discussion = lms.djangoapps.django_comment_client.forum.views:DiscussionTab", + "edxnotes = lms.djangoapps.edxnotes.plugins:EdxNotesTab", + "external_discussion = lms.djangoapps.courseware.tabs:ExternalDiscussionCourseTab", + "external_link = lms.djangoapps.courseware.tabs:ExternalLinkCourseTab", + "html_textbooks = lms.djangoapps.courseware.tabs:HtmlTextbookTabs", + "instructor = lms.djangoapps.instructor.views.instructor_dashboard:InstructorDashboardTab", + "notes = lms.djangoapps.notes.views:NotesTab", + "pdf_textbooks = lms.djangoapps.courseware.tabs:PDFTextbookTabs", + "progress = lms.djangoapps.courseware.tabs:ProgressTab", + "static_tab = xmodule.tabs:StaticTab", + "syllabus = lms.djangoapps.courseware.tabs:SyllabusTab", + "teams = lms.djangoapps.teams.plugins:TeamsTab", + "textbooks = lms.djangoapps.courseware.tabs:TextbookTabs", + "wiki = lms.djangoapps.course_wiki.tab:WikiTab", # ORA 1 tabs (deprecated) "peer_grading = lms.djangoapps.open_ended_grading.views:PeerGradingTab",