From dae137feaa6978aebec18a551884d784ba7c2371 Mon Sep 17 00:00:00 2001 From: Diana Huang Date: Tue, 19 May 2015 17:16:16 -0400 Subject: [PATCH] Convert all tabs to the new plugin framework. --- .../contentstore/tests/test_course_listing.py | 11 - .../tests/test_course_settings.py | 59 +- cms/djangoapps/contentstore/utils.py | 5 - cms/djangoapps/contentstore/views/course.py | 95 +-- cms/djangoapps/contentstore/views/helpers.py | 2 +- cms/djangoapps/contentstore/views/tabs.py | 6 +- .../contentstore/views/tests/test_tabs.py | 11 +- .../models/settings/course_metadata.py | 5 +- cms/templates/edit-tabs.html | 2 +- .../student/tests/test_course_listing.py | 14 - .../xmodule/modulestore/tests/django_utils.py | 10 - .../xmodule/modulestore/tests/factories.py | 7 +- .../xmodule/modulestore/tests/test_mongo.py | 19 - .../tests/test_split_modulestore.py | 18 +- .../xmodule/modulestore/tests/test_xml.py | 3 +- common/lib/xmodule/xmodule/tabs.py | 716 ++--------------- common/lib/xmodule/xmodule/tests/test_tabs.py | 731 ------------------ lms/djangoapps/ccx/plugins.py | 7 +- lms/djangoapps/ccx/tests/test_views.py | 33 +- lms/djangoapps/course_wiki/tab.py | 31 + lms/djangoapps/course_wiki/tests/test_tab.py | 65 ++ lms/djangoapps/courseware/tabs.py | 325 +++++++- lms/djangoapps/courseware/tests/test_tabs.py | 570 +++++++++++++- lms/djangoapps/courseware/views.py | 15 +- lms/djangoapps/discussion_api/api.py | 3 +- .../discussion_api/tests/test_api.py | 3 +- .../django_comment_client/forum/views.py | 25 + .../django_comment_client/tests/test_utils.py | 42 +- lms/djangoapps/edxnotes/plugins.py | 12 +- lms/djangoapps/edxnotes/tests.py | 13 +- .../instructor/views/instructor_dashboard.py | 6 +- lms/djangoapps/notes/tests.py | 64 +- lms/djangoapps/notes/utils.py | 2 +- lms/djangoapps/notes/views.py | 23 + lms/djangoapps/open_ended_grading/tests.py | 3 +- lms/djangoapps/open_ended_grading/views.py | 53 ++ lms/templates/help_modal.html | 2 +- .../course_groups/tests/test_cohorts.py | 5 +- .../course_views}/__init__.py | 0 .../djangoapps/course_views/course_views.py | 201 +++++ .../course_views}/tests/__init__.py | 0 .../course_views}/tests/test_api.py | 3 +- .../course_views/tests/test_course_views.py | 74 ++ .../lib/{plugins/api.py => api/plugins.py} | 44 -- setup.py | 18 + 45 files changed, 1678 insertions(+), 1678 deletions(-) delete mode 100644 common/lib/xmodule/xmodule/tests/test_tabs.py create mode 100644 lms/djangoapps/course_wiki/tab.py create mode 100644 lms/djangoapps/course_wiki/tests/test_tab.py rename openedx/core/{lib/plugins => djangoapps/course_views}/__init__.py (100%) create mode 100644 openedx/core/djangoapps/course_views/course_views.py rename openedx/core/{lib/plugins => djangoapps/course_views}/tests/__init__.py (100%) rename openedx/core/{lib/plugins => djangoapps/course_views}/tests/test_api.py (78%) create mode 100644 openedx/core/djangoapps/course_views/tests/test_course_views.py rename openedx/core/lib/{plugins/api.py => api/plugins.py} (51%) diff --git a/cms/djangoapps/contentstore/tests/test_course_listing.py b/cms/djangoapps/contentstore/tests/test_course_listing.py index 8066a8e26a..3970a8c1e8 100644 --- a/cms/djangoapps/contentstore/tests/test_course_listing.py +++ b/cms/djangoapps/contentstore/tests/test_course_listing.py @@ -225,17 +225,6 @@ class TestCourseListing(ModuleStoreTestCase): self._create_course_with_access_groups(course_location, self.user) store.delete_course(course_location, self.user.id) - course_location = self.store.make_course_key('testOrg', 'erroredCourse', 'RunBabyRun') - course = self._create_course_with_access_groups(course_location, self.user) - course_db_record = store._find_one(course.location) - course_db_record.setdefault('metadata', {}).get('tabs', []).append({"type": "wiko", "name": "Wiki"}) - store.collection.update( - {'_id': course.location.to_deprecated_son()}, - {'$set': { - 'metadata.tabs': course_db_record['metadata']['tabs'], - }}, - ) - courses_list, __ = _accessible_courses_list_from_groups(self.request) self.assertEqual(len(courses_list), 1, courses_list) diff --git a/cms/djangoapps/contentstore/tests/test_course_settings.py b/cms/djangoapps/contentstore/tests/test_course_settings.py index a68cdd593e..1450f37df4 100644 --- a/cms/djangoapps/contentstore/tests/test_course_settings.py +++ b/cms/djangoapps/contentstore/tests/test_course_settings.py @@ -14,7 +14,7 @@ from django.conf import settings from models.settings.course_details import (CourseDetails, CourseSettingsEncoder) from models.settings.course_grading import CourseGradingModel -from contentstore.utils import EXTRA_TAB_PANELS, reverse_course_url, reverse_usage_url +from contentstore.utils import reverse_course_url, reverse_usage_url from xmodule.modulestore.tests.factories import CourseFactory from models.settings.course_metadata import CourseMetadata @@ -662,7 +662,7 @@ class CourseMetadataEditingTest(CourseTestCase): If feature flag is off, then giturl must be filtered. """ # pylint: disable=unused-variable - is_valid, errors, test_model = CourseMetadata.validate_from_json( + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { "giturl": {"value": "http://example.com"}, @@ -677,7 +677,7 @@ class CourseMetadataEditingTest(CourseTestCase): If feature flag is on, then giturl must not be filtered. """ # pylint: disable=unused-variable - is_valid, errors, test_model = CourseMetadata.validate_from_json( + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { "giturl": {"value": "http://example.com"}, @@ -736,7 +736,7 @@ class CourseMetadataEditingTest(CourseTestCase): If feature flag is off, then edxnotes must be filtered. """ # pylint: disable=unused-variable - is_valid, errors, test_model = CourseMetadata.validate_from_json( + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { "edxnotes": {"value": "true"}, @@ -751,7 +751,7 @@ class CourseMetadataEditingTest(CourseTestCase): If feature flag is on, then edxnotes must not be filtered. """ # pylint: disable=unused-variable - is_valid, errors, test_model = CourseMetadata.validate_from_json( + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { "edxnotes": {"value": "true"}, @@ -789,7 +789,7 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertNotIn('edxnotes', test_model) def test_validate_from_json_correct_inputs(self): - is_valid, errors, test_model = CourseMetadata.validate_from_json( + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { "advertised_start": {"value": "start A"}, @@ -808,7 +808,7 @@ class CourseMetadataEditingTest(CourseTestCase): def test_validate_from_json_wrong_inputs(self): # input incorrectly formatted data - is_valid, errors, test_model = CourseMetadata.validate_from_json( + is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { "advertised_start": {"value": 1, "display_name": "Course Advertised Start Date", }, @@ -819,7 +819,7 @@ class CourseMetadataEditingTest(CourseTestCase): user=self.user ) - # Check valid results from validate_from_json + # Check valid results from validate_and_update_from_json self.assertFalse(is_valid) self.assertEqual(len(errors), 3) self.assertFalse(test_model) @@ -928,19 +928,50 @@ class CourseMetadataEditingTest(CourseTestCase): """ Test that adding and removing specific advanced components adds and removes tabs. """ - self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), self.course.tabs) - self.assertNotIn(EXTRA_TAB_PANELS.get("notes"), self.course.tabs) + open_ended_tab = {"type": "open_ended", "name": "Open Ended Panel"} + peer_grading_tab = {"type": "peer_grading", "name": "Peer grading"} + notes_tab = {"type": "notes", "name": "My Notes"} + + # First ensure that none of the tabs are visible + self.assertNotIn(open_ended_tab, self.course.tabs) + self.assertNotIn(peer_grading_tab, self.course.tabs) + self.assertNotIn(notes_tab, self.course.tabs) + + # Now add the "combinedopenended" component and verify that the tab has been added self.client.ajax_post(self.course_setting_url, { ADVANCED_COMPONENT_POLICY_KEY: {"value": ["combinedopenended"]} }) course = modulestore().get_course(self.course.id) - self.assertIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs) - self.assertNotIn(EXTRA_TAB_PANELS.get("notes"), course.tabs) + self.assertIn(open_ended_tab, course.tabs) + self.assertIn(peer_grading_tab, course.tabs) + self.assertNotIn(notes_tab, course.tabs) + + # Now enable student notes and verify that the "My Notes" tab has also been added self.client.ajax_post(self.course_setting_url, { - ADVANCED_COMPONENT_POLICY_KEY: {"value": []} + ADVANCED_COMPONENT_POLICY_KEY: {"value": ["combinedopenended", "notes"]} }) course = modulestore().get_course(self.course.id) - self.assertNotIn(EXTRA_TAB_PANELS.get("open_ended"), course.tabs) + self.assertIn(open_ended_tab, course.tabs) + self.assertIn(peer_grading_tab, course.tabs) + self.assertIn(notes_tab, course.tabs) + + # Now remove the "combinedopenended" component and verify that the tab is gone + self.client.ajax_post(self.course_setting_url, { + ADVANCED_COMPONENT_POLICY_KEY: {"value": ["notes"]} + }) + course = modulestore().get_course(self.course.id) + self.assertNotIn(open_ended_tab, course.tabs) + self.assertNotIn(peer_grading_tab, course.tabs) + self.assertIn(notes_tab, course.tabs) + + # Finally disable student notes and verify that the "My Notes" tab is gone + self.client.ajax_post(self.course_setting_url, { + ADVANCED_COMPONENT_POLICY_KEY: {"value": [""]} + }) + course = modulestore().get_course(self.course.id) + self.assertNotIn(open_ended_tab, course.tabs) + self.assertNotIn(peer_grading_tab, course.tabs) + self.assertNotIn(notes_tab, course.tabs) class CourseGraderUpdatesTest(CourseTestCase): diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index 5e3812542e..5828d4e761 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -26,11 +26,6 @@ from student import auth log = logging.getLogger(__name__) -# In order to instantiate an open ended tab automatically, need to have this data -OPEN_ENDED_PANEL = {"name": _("Open Ended Panel"), "type": "open_ended"} -NOTES_PANEL = {"name": _("My Notes"), "type": "notes"} -EXTRA_TAB_PANELS = {p['type']: p for p in [OPEN_ENDED_PANEL, NOTES_PANEL]} - def add_instructor(course_key, requesting_user, new_instructor): """ diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index e708818a6e..9e5cc1ce16 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -16,20 +16,19 @@ from django.core.urlresolvers import reverse from django.http import HttpResponseBadRequest, HttpResponseNotFound, HttpResponse, Http404 from util.json_request import JsonResponse, JsonResponseBadRequest from util.date_utils import get_default_time_display -from util.db import generate_int_id, MYSQL_MAX_INT from edxmako.shortcuts import render_to_response from xmodule.course_module import DEFAULT_START_DATE from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore from xmodule.contentstore.content import StaticContent -from xmodule.tabs import PDFTextbookTabs, CourseTab, CourseTabManager +from xmodule.tabs import CourseTab +from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager from xmodule.modulestore import EdxJSONEncoder from xmodule.modulestore.exceptions import ItemNotFoundError, DuplicateCourseError from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import Location from opaque_keys.edx.keys import CourseKey -from openedx.core.lib.plugins.api import CourseViewType from django_future.csrf import ensure_csrf_cookie from contentstore.course_info_model import get_course_updates, update_course_updates, delete_course_update @@ -46,10 +45,8 @@ from contentstore.utils import ( get_lms_link_for_item, reverse_course_url, reverse_library_url, - reverse_usage_url, reverse_url, remove_all_instructors, - EXTRA_TAB_PANELS, ) from models.settings.course_details import CourseDetails, CourseSettingsEncoder from models.settings.course_grading import CourseGradingModel @@ -58,9 +55,6 @@ from util.json_request import expect_json from util.string_utils import _has_non_ascii_characters from student.auth import has_studio_write_access, has_studio_read_access from .component import ( - OPEN_ENDED_COMPONENT_TYPES, - NOTE_COMPONENT_TYPES, - ADVANCED_COMPONENT_POLICY_KEY, SPLIT_TEST_COMPONENT_TYPE, ADVANCED_COMPONENT_TYPES, ) @@ -84,7 +78,6 @@ from course_action_state.models import CourseRerunState, CourseRerunUIStateManag from course_action_state.managers import CourseActionStateItemNotFoundError from microsite_configuration import microsite from xmodule.course_module import CourseFields -from xmodule.split_test_module import get_split_user_partitions from student.auth import has_course_author_access from util.milestones_helpers import ( @@ -994,88 +987,38 @@ def grading_handler(request, course_key_string, grader_index=None): return JsonResponse() -def is_advanced_component_present(request, advanced_components): - """ - Return True when one of `advanced_components` is present in the request. - - raises TypeError - when request.ADVANCED_COMPONENT_POLICY_KEY is malformed (not iterable) - """ - if ADVANCED_COMPONENT_POLICY_KEY not in request.json: - return False - - new_advanced_component_list = request.json[ADVANCED_COMPONENT_POLICY_KEY]['value'] - for ac_type in advanced_components: - if ac_type in new_advanced_component_list and ac_type in ADVANCED_COMPONENT_TYPES: - return True - - -def is_field_value_true(request, field_list): - """ - Return True when one of field values is set to True by request - """ - return any([request.json.get(field, {}).get('value') for field in field_list]) - - def _refresh_course_tabs(request, course_module): """ Automatically adds/removes tabs if changes to the course require them. """ - tab_component_map = { - # 'tab_type': (check_function, list_of_checked_components_or_values), - - # open ended tab by combinedopendended or peergrading module - 'open_ended': (is_advanced_component_present, OPEN_ENDED_COMPONENT_TYPES), - # notes tab - 'notes': (is_advanced_component_present, NOTE_COMPONENT_TYPES), - } def update_tab(tabs, tab_type, tab_enabled): """ Adds or removes a course tab based upon whether it is enabled. """ - tab_panel = _get_tab_panel_for_type(tab_type) - if tab_enabled: + tab_panel = { + "type": tab_type.name, + "name": tab_type.title, + } + has_tab = tab_panel in tabs + if tab_enabled and not has_tab: tabs.append(CourseTab.from_json(tab_panel)) - elif tab_panel in tabs: + elif not tab_enabled and has_tab: tabs.remove(tab_panel) course_tabs = copy.copy(course_module.tabs) - for tab_type in tab_component_map.keys(): - check, component_types = tab_component_map[tab_type] - try: - tab_enabled = check(request, component_types) - except TypeError: - # user has failed to put iterable value into advanced component list. - # return immediately and let validation handle. - return - update_tab(course_tabs, tab_type, tab_enabled) - - # Additionally update any persistent tabs provided by course views - for tab_type in CourseTabManager.get_tab_types().values(): - if issubclass(tab_type, CourseViewType) and tab_type.is_persistent: - tab_enabled = tab_type.is_enabled(course_module, settings, user=request.user) + # Additionally update any tabs that are provided by non-dynamic course views + for tab_type in CourseViewTypeManager.get_course_view_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) # Save the tabs into the course if they have been changed - if not course_tabs == course_module.tabs: + if course_tabs != course_module.tabs: course_module.tabs = course_tabs -def _get_tab_panel_for_type(tab_type): - """ - Returns a tab panel representation for the specified tab type. - """ - tab_panel = EXTRA_TAB_PANELS.get(tab_type) - if tab_panel: - return tab_panel - return { - "name": tab_type.title, - "type": tab_type.name - } - - @login_required @ensure_csrf_cookie @require_http_methods(("GET", "POST", "PUT")) @@ -1107,7 +1050,7 @@ def advanced_settings_handler(request, course_key_string): try: # validate data formats and update the course module. # Note: don't update mongo yet, but wait until after any tabs are changed - is_valid, errors, updated_data = CourseMetadata.validate_from_json( + is_valid, errors, updated_data = CourseMetadata.validate_and_update_from_json( course_module, request.json, user=request.user, @@ -1238,8 +1181,8 @@ def textbooks_list_handler(request, course_key_string): textbook["id"] = tid tids.add(tid) - if not any(tab['type'] == PDFTextbookTabs.type for tab in course.tabs): - course.tabs.append(PDFTextbookTabs()) + if not any(tab['type'] == 'pdf_textbooks' for tab in course.tabs): + course.tabs.append(CourseTab.load('pdf_textbooks')) course.pdf_textbooks = textbooks store.update_item(course, request.user.id) return JsonResponse(course.pdf_textbooks) @@ -1255,8 +1198,8 @@ def textbooks_list_handler(request, course_key_string): existing = course.pdf_textbooks existing.append(textbook) course.pdf_textbooks = existing - if not any(tab['type'] == PDFTextbookTabs.type for tab in course.tabs): - course.tabs.append(PDFTextbookTabs()) + if not any(tab['type'] == 'pdf_textbooks' for tab in course.tabs): + course.tabs.append(CourseTab.load('pdf_textbooks')) store.update_item(course, request.user.id) resp = JsonResponse(textbook, status=201) resp["Location"] = reverse_course_url( diff --git a/cms/djangoapps/contentstore/views/helpers.py b/cms/djangoapps/contentstore/views/helpers.py index 72df1bc44d..36def116e4 100644 --- a/cms/djangoapps/contentstore/views/helpers.py +++ b/cms/djangoapps/contentstore/views/helpers.py @@ -12,12 +12,12 @@ 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.tabs import StaticTab from xmodule.x_module import DEPRECATION_VSCOMPAT_EVENT from contentstore.utils import reverse_course_url, reverse_library_url, reverse_usage_url diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 4764363a7f..eecb80672b 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -10,11 +10,13 @@ from django.contrib.auth.decorators import login_required from django.core.exceptions import PermissionDenied from django_future.csrf import ensure_csrf_cookie 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, StaticTab, CourseTab, InvalidTabsException +from xmodule.tabs import CourseTabList, CourseTab, InvalidTabsException 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 @@ -61,7 +63,7 @@ def tabs_handler(request, course_key_string): # present in the same order they are displayed in LMS tabs_to_render = [] - for tab in CourseTabList.iterate_displayable(course_item, settings, inline_collections=False): + for tab in CourseTabList.iterate_displayable(course_item, inline_collections=False): if isinstance(tab, StaticTab): # static tab needs its locator information to render itself as an xmodule static_tab_loc = course_key.make_usage_key('static_tab', tab.url_slug) diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index eb2b78188c..fde68931a3 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -8,7 +8,7 @@ from contentstore.utils import reverse_course_url from xmodule.x_module import STUDENT_VIEW from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.tabs import CourseTabList, WikiTab +from xmodule.tabs import CourseTabList from xmodule.modulestore.django import modulestore @@ -52,7 +52,7 @@ class TabsPageTests(CourseTestCase): self.client.ajax_post( self.url, data=json.dumps({ - 'tab_id_locator': {'tab_id': WikiTab.type}, + 'tab_id_locator': {'tab_id': 'courseware'}, 'unsupported_request': None, }), ) @@ -158,10 +158,9 @@ class TabsPageTests(CourseTestCase): self.assertEqual(new_tab.is_hidden, new_is_hidden_setting) def test_toggle_tab_visibility(self): - """Test toggling of tab visiblity""" - - self.check_toggle_tab_visiblity(WikiTab.type, True) - self.check_toggle_tab_visiblity(WikiTab.type, False) + """Test toggling of tab visibility""" + self.check_toggle_tab_visiblity('wiki', True) + self.check_toggle_tab_visiblity('wiki', False) def test_toggle_invalid_tab_visibility(self): """Test toggling visibility of an invalid tab""" diff --git a/cms/djangoapps/models/settings/course_metadata.py b/cms/djangoapps/models/settings/course_metadata.py index a62902e0e9..3516ae6176 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -150,11 +150,12 @@ class CourseMetadata(object): return cls.update_from_dict(key_values, descriptor, user) @classmethod - def validate_from_json(cls, descriptor, jsondict, user, filter_tabs=True): + def validate_and_update_from_json(cls, descriptor, jsondict, user, filter_tabs=True): """ Validate the values in the json dict (validated by xblock fields from_json method) - If all fields validate, go ahead and update those values on the object and return it. + If all fields validate, go ahead and update those values on the object and return it without + persisting it to the DB. If not, return the error objects list. Returns: diff --git a/cms/templates/edit-tabs.html b/cms/templates/edit-tabs.html index 6ad7d6f4d4..b1bec3663c 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 xmodule.tabs import StaticTab + from openedx.core.djangoapps.course_views.course_views import StaticTab from django.template.defaultfilters import escapejs %> <%block name="title">${_("Pages")} diff --git a/common/djangoapps/student/tests/test_course_listing.py b/common/djangoapps/student/tests/test_course_listing.py index 87160c6307..0e6bdd7299 100644 --- a/common/djangoapps/student/tests/test_course_listing.py +++ b/common/djangoapps/student/tests/test_course_listing.py @@ -112,20 +112,6 @@ class TestCourseListing(ModuleStoreTestCase): self._create_course_with_access_groups(course_location, default_store=ModuleStoreEnum.Type.mongo) mongo_store.delete_course(course_location, ModuleStoreEnum.UserID.test) - course_location = mongo_store.make_course_key('testOrg', 'erroredCourse', 'RunBabyRun') - course = self._create_course_with_access_groups(course_location, default_store=ModuleStoreEnum.Type.mongo) - course_db_record = mongo_store._find_one(course.location) - course_db_record.setdefault('metadata', {}).get('tabs', []).append({ - "type": "wiko", - "name": "Wiki", - }) - mongo_store.collection.update( - {'_id': course.location.to_deprecated_son()}, - {'$set': { - 'metadata.tabs': course_db_record['metadata']['tabs'], - }}, - ) - courses_list = list(get_course_enrollment_pairs(self.student, None, [])) self.assertEqual(len(courses_list), 1, courses_list) self.assertEqual(courses_list[0][0].id, good_location) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py index 81fb0baede..1998ae2978 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/django_utils.py @@ -22,7 +22,6 @@ from xmodule.modulestore.django import modulestore, clear_existing_modulestores from xmodule.modulestore.tests.mongo_connection import MONGO_PORT_NUM, MONGO_HOST from xmodule.modulestore.tests.sample_courses import default_block_info_tree, TOY_BLOCK_INFO_TREE from xmodule.modulestore.tests.factories import XMODULE_FACTORY_LOCK -from xmodule.tabs import CoursewareTab, CourseInfoTab, StaticTab, DiscussionTab, ProgressTab, WikiTab class StoreConstructors(object): @@ -379,15 +378,6 @@ class ModuleStoreTestCase(TestCase): "wiki_slug": "toy", "display_name": "Toy Course", "graded": True, - "tabs": [ - CoursewareTab(), - CourseInfoTab(), - StaticTab(name="Syllabus", url_slug="syllabus"), - StaticTab(name="Resources", url_slug="resources"), - DiscussionTab(), - WikiTab(), - ProgressTab(), - ], "discussion_topics": {"General": {"id": "i4x-edX-toy-course-2012_Fall"}}, "graceperiod": datetime.timedelta(days=2, seconds=21599), "start": datetime.datetime(2015, 07, 17, 12, tzinfo=pytz.utc), diff --git a/common/lib/xmodule/xmodule/modulestore/tests/factories.py b/common/lib/xmodule/xmodule/modulestore/tests/factories.py index a665a16516..9e1f1ae5c7 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/factories.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/factories.py @@ -13,8 +13,8 @@ import dogstats_wrapper as dog_stats_api from opaque_keys.edx.locations import Location from opaque_keys.edx.keys import UsageKey from xblock.core import XBlock -from xmodule.tabs import StaticTab from xmodule.modulestore import prefer_xmodules, ModuleStoreEnum +from xmodule.tabs import CourseTab from xmodule.x_module import DEPRECATION_VSCOMPAT_EVENT @@ -277,10 +277,7 @@ class ItemFactory(XModuleFactory): course = store.get_course(location.course_key) course.tabs.append( - StaticTab( - name=display_name, - url_slug=location.name, - ) + CourseTab.load('static_tab', name='Static Tab', url_slug=location.name) ) store.update_item(course, user_id) diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py index c0bac5094e..4e29d65803 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_mongo.py @@ -373,25 +373,6 @@ class TestMongoModuleStore(TestMongoModuleStoreBase): '{0} is a template course'.format(course) ) - def test_static_tab_names(self): - - def get_tab_name(index): - """ - Helper function for pulling out the name of a given static tab. - - Assumes the information is desired for courses[4] ('toy' course). - """ - course = self.draft_store.get_course(SlashSeparatedCourseKey('edX', 'toy', '2012_Fall')) - return course.tabs[index]['name'] - - # There was a bug where model.save was not getting called after the static tab name - # was set set for tabs that have a URL slug. 'Syllabus' and 'Resources' fall into that - # category, but for completeness, I'm also testing 'Course Info' and 'Discussion' (no url slug). - assert_equals('Course Info', get_tab_name(1)) - assert_equals('Syllabus', get_tab_name(2)) - assert_equals('Resources', get_tab_name(3)) - assert_equals('Discussion', get_tab_name(4)) - def test_contentstore_attrs(self): """ Test getting, setting, and defaulting the locked attr and arbitrary attrs. 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 c04c056c01..5d2df978dd 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_split_modulestore.py @@ -1,6 +1,7 @@ """ Test split modulestore w/o using any django stuff. """ +from mock import patch import datetime from importlib import import_module from path import path @@ -36,6 +37,14 @@ 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): ''' @@ -596,7 +605,8 @@ class SplitModuleCourseTests(SplitModuleTest): Course CRUD operation tests ''' - def test_get_courses(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_get_courses(self, _from_json): courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT) # should have gotten 3 draft courses self.assertEqual(len(courses), 3, "Wrong number of courses") @@ -635,7 +645,8 @@ class SplitModuleCourseTests(SplitModuleTest): courses = modulestore().get_courses(branch=BRANCH_NAME_DRAFT) self.assertEqual(len(courses), 3) - def test_branch_requests(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_branch_requests(self, _from_json): # query w/ branch qualifier (both draft and published) def _verify_published_course(courses_published): """ Helper function for verifying published course. """ @@ -665,7 +676,8 @@ class SplitModuleCourseTests(SplitModuleTest): locator_key_fields=['org', 'course', 'run'] ) - def test_get_course(self): + @patch('xmodule.tabs.CourseTab.from_json', side_effect=mock_tab_from_json) + def test_get_course(self, _from_json): ''' Test the various calling forms for get_course ''' diff --git a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py index 3bac20f363..f029de4283 100644 --- a/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py +++ b/common/lib/xmodule/xmodule/modulestore/tests/test_xml.py @@ -5,7 +5,7 @@ well-formed and not-well-formed XML. import os.path import unittest from glob import glob -from mock import patch +from mock import patch, Mock from xmodule.modulestore.xml import XMLModuleStore from xmodule.modulestore import ModuleStoreEnum @@ -35,6 +35,7 @@ class TestXMLModuleStore(unittest.TestCase): store = XMLModuleStore(DATA_DIR, source_dirs=[]) self.assertEqual(store.get_modulestore_type(), ModuleStoreEnum.Type.xml) + @patch('xmodule.tabs.CourseTabList.initialize_default', Mock()) def test_unicode_chars_in_xml_content(self): # edX/full/6.002_Spring_2012 has non-ASCII chars, and during # uniquification of names, would raise a UnicodeError. It no longer does. diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 424d789ce0..973c24bdb0 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -1,14 +1,19 @@ """ Implement CourseTab """ -from abc import ABCMeta, abstractmethod + +from abc import ABCMeta +import logging from xblock.fields import List +from openedx.core.lib.api.plugins import PluginError # We should only scrape strings for i18n in this file, since the target language is known only when # they are rendered in the template. So ugettext gets called in the template. _ = lambda text: text +log = logging.getLogger("edx.courseware") + class CourseTab(object): """ @@ -26,6 +31,9 @@ class CourseTab(object): # 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 + # Class property that specifies whether the tab can be moved within a course's list of tabs is_movable = True @@ -52,7 +60,7 @@ class CourseTab(object): self.link_func = link_func - def is_enabled(self, course, settings, user=None): # pylint: disable=unused-argument + 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 @@ -61,13 +69,6 @@ class CourseTab(object): Args: course: An xModule CourseDescriptor - settings: The configuration settings, including values for: - WIKI_ENABLED - FEATURES['ENABLE_DISCUSSION_SERVICE'] - FEATURES['ENABLE_EDXNOTES'] - FEATURES['ENABLE_STUDENT_NOTES'] - FEATURES['ENABLE_TEXTBOOK'] - user: An optional user for whom the tab will be displayed. If none, then the code should assume a staff user or an author. @@ -145,6 +146,22 @@ class CourseTab(object): """ return key_checker(['type'])(tab_dict, raise_error) + @classmethod + def load(cls, type_name, **kwargs): + """ + Constructs a tab of the given type_name. + + Args: + type_name (str) - the type of tab that should be constructed + **kwargs - any other keyword arguments needed for constructing this tab + + Returns: + an instance of the CourseTab subclass that matches the type_name + """ + json_dict = kwargs.copy() + json_dict['type'] = type_name + return cls.from_json(json_dict) + def to_json(self): """ Serializes the necessary members of the CourseTab object to a json-serializable representation. @@ -163,597 +180,33 @@ class CourseTab(object): The subclass that is instantiated is determined by the value of the 'type' key in the given dict-type tab. The given dict-type tab is validated before instantiating the CourseTab object. + If the tab_type is not recognized, then an exception is logged and None is returned. + The intention is that the user should still be able to use the course even if a + particular tab is not found for some reason. + Args: tab: a dictionary with keys for the properties of the tab. Raises: InvalidTabsException if the given tab doesn't have the right keys. """ - available_tab_types = CourseTabManager.get_tab_types() - tab_type_name = tab_dict.get('type') - if tab_type_name not in available_tab_types: - raise InvalidTabsException( - 'Unknown tab type {0}. Known types: {1}'.format(tab_type_name, available_tab_types) - ) - tab_type = available_tab_types[tab_dict['type']] - tab_type.validate(tab_dict) # TODO: don't import openedx capabilities from common - from openedx.core.lib.plugins.api import CourseViewType - if issubclass(tab_type, CourseViewType): - return CourseViewTab(tab_type, tab_dict=tab_dict) - else: - return tab_type(tab_dict=tab_dict) - - -class CourseTabManager(object): - """ - A manager that handles the set of available course tabs. - """ - - @classmethod - def get_tab_types(cls): - """ - Returns the list of available tab types. - """ - if not hasattr(cls, "_tab_types"): - tab_types = { - 'courseware': CoursewareTab, - 'course_info': CourseInfoTab, - 'wiki': WikiTab, - 'discussion': DiscussionTab, - 'external_discussion': ExternalDiscussionTab, - 'external_link': ExternalLinkTab, - 'textbooks': TextbookTabs, - 'pdf_textbooks': PDFTextbookTabs, - 'html_textbooks': HtmlTextbookTabs, - 'progress': ProgressTab, - 'static_tab': StaticTab, - 'peer_grading': PeerGradingTab, - 'staff_grading': StaffGradingTab, - 'open_ended': OpenEndedGradingTab, - 'notes': NotesTab, - 'syllabus': SyllabusTab, - } - - # Add any registered course views - # TODO: don't import openedx capabilities from common - from openedx.core.lib.plugins.api import CourseViewTypeManager - for course_view_type in CourseViewTypeManager.get_available_plugins().values(): - tab_types[course_view_type.name] = course_view_type - - cls._tab_types = tab_types - return cls._tab_types - - -class AuthenticatedCourseTab(CourseTab): - """ - Abstract class for tabs that can be accessed by only authenticated users. - """ - def is_enabled(self, course, settings, user=None): - return not user or user.is_authenticated() - - -class StaffTab(AuthenticatedCourseTab): - """ - Abstract class for tabs that can be accessed by only users with staff access. - """ - def is_enabled(self, course, settings, user=None): # pylint: disable=unused-argument - return not user or is_user_staff(course, user) - - -def is_user_staff(course, user): - """ - Returns true if the user is staff in the specified course, or globally. - """ - from courseware.access import has_access # pylint: disable=import-error - return has_access(user, 'staff', course, course.id) - - -def is_user_enrolled_or_staff(course, user): - """ - Returns true if the user is enrolled in the specified course, - or if the user is staff. - """ - from student.models import CourseEnrollment # pylint: disable=import-error - return is_user_staff(course, user) or CourseEnrollment.is_enrolled(user, course.id) - - -class EnrolledOrStaffTab(AuthenticatedCourseTab): - """ - Abstract class for tabs that can be accessed by only users with staff access - or users enrolled in the course. - """ - def is_enabled(self, course, settings, user=None): # pylint: disable=unused-argument - if not user: - return True - return is_user_enrolled_or_staff(course, user) - - -class HideableTab(CourseTab): - """ - Abstract class for tabs that are hideable - """ - is_hideable = True - - def __init__(self, name, tab_id, link_func, tab_dict): - super(HideableTab, self).__init__( - name=name, - tab_id=tab_id, - link_func=link_func, - ) - self.is_hidden = tab_dict.get('is_hidden', False) if tab_dict else False - - def __getitem__(self, key): - if key == 'is_hidden': - return self.is_hidden - else: - return super(HideableTab, self).__getitem__(key) - - def __setitem__(self, key, value): - if key == 'is_hidden': - self.is_hidden = value - else: - super(HideableTab, self).__setitem__(key, value) - - def to_json(self): - to_json_val = super(HideableTab, self).to_json() - if self.is_hidden: - to_json_val.update({'is_hidden': True}) - return to_json_val - - def __eq__(self, other): - if not super(HideableTab, self).__eq__(other): - return False - return self.is_hidden == other.get('is_hidden', False) - - -class CoursewareTab(EnrolledOrStaffTab): - """ - A tab containing the course content. - """ - - type = 'courseware' - is_movable = False - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(CoursewareTab, self).__init__( - # Translators: 'Courseware' refers to the tab in the courseware that leads to the content of a course - name=_('Courseware'), # support fixed name for the courseware tab - tab_id=self.type, - link_func=link_reverse_func(self.type), - ) - - -class CourseInfoTab(CourseTab): - """ - A tab containing information about the course. - """ - - type = 'course_info' - is_movable = False - - def __init__(self, tab_dict=None): - super(CourseInfoTab, self).__init__( - # Translators: "Course Info" is the name of the course's information and updates page - name=tab_dict['name'] if tab_dict else _('Course Info'), - tab_id='info', - link_func=link_reverse_func('info'), - ) - - @classmethod - def validate(cls, tab_dict, raise_error=True): - return super(CourseInfoTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) - - -class ProgressTab(EnrolledOrStaffTab): - """ - A tab containing information about the authenticated user's progress. - """ - - type = 'progress' - - def __init__(self, tab_dict=None): - super(ProgressTab, self).__init__( - # Translators: "Progress" is the name of the student's course progress page - name=tab_dict['name'] if tab_dict else _('Progress'), - tab_id=self.type, - link_func=link_reverse_func(self.type), - ) - - def is_enabled(self, course, settings, user=None): - super_can_display = super(ProgressTab, self).is_enabled(course, settings, user=user) - return super_can_display and not course.hide_progress_tab - - @classmethod - def validate(cls, tab_dict, raise_error=True): - return super(ProgressTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) - - -class WikiTab(HideableTab): - """ - A tab_dict containing the course wiki. - """ - - type = 'wiki' - - def __init__(self, tab_dict=None): - super(WikiTab, self).__init__( - # Translators: "Wiki" is the name of the course's wiki page - name=tab_dict['name'] if tab_dict else _('Wiki'), - tab_id=self.type, - link_func=link_reverse_func('course_wiki'), - tab_dict=tab_dict, - ) - - def is_enabled(self, course, settings, user=None): - if not settings.WIKI_ENABLED: - return False - if not user or course.allow_public_wiki_access: - return True - return is_user_enrolled_or_staff(course, user) - - @classmethod - def validate(cls, tab_dict, raise_error=True): - return super(WikiTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) - - -class DiscussionTab(EnrolledOrStaffTab): - """ - A tab only for the new Berkeley discussion forums. - """ - - type = 'discussion' - - def __init__(self, tab_dict=None): - super(DiscussionTab, self).__init__( - # Translators: "Discussion" is the title of the course forum page - name=tab_dict['name'] if tab_dict else _('Discussion'), - tab_id=self.type, - link_func=link_reverse_func('django_comment_client.forum.views.forum_form_discussion'), - ) - - def is_enabled(self, course, settings, user=None): - if settings.FEATURES.get('CUSTOM_COURSES_EDX', False): - from ccx.overrides import get_current_ccx # pylint: disable=import-error - if get_current_ccx(): - return False - super_can_display = super(DiscussionTab, self).is_enabled(course, settings, user=user) - return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE') and super_can_display - - @classmethod - def validate(cls, tab_dict, raise_error=True): - return super(DiscussionTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) - - -class LinkTab(CourseTab): - """ - Abstract class for tabs that contain external links. - """ - link_value = '' - - def __init__(self, name, tab_id, link_value): - self.link_value = link_value - super(LinkTab, self).__init__( - name=name, - tab_id=tab_id, - link_func=link_value_func(self.link_value), - ) - - def __getitem__(self, key): - if key == 'link': - return self.link_value - else: - return super(LinkTab, self).__getitem__(key) - - def __setitem__(self, key, value): - if key == 'link': - self.link_value = value - else: - super(LinkTab, self).__setitem__(key, value) - - def to_json(self): - to_json_val = super(LinkTab, self).to_json() - to_json_val.update({'link': self.link_value}) - return to_json_val - - def __eq__(self, other): - if not super(LinkTab, self).__eq__(other): - return False - return self.link_value == other.get('link') - - @classmethod - def validate(cls, tab_dict, raise_error=True): - return super(LinkTab, cls).validate(tab_dict, raise_error) and key_checker(['link'])(tab_dict, raise_error) - - -class ExternalDiscussionTab(LinkTab): - """ - A tab that links to an external discussion service. - """ - - type = 'external_discussion' - - def __init__(self, tab_dict=None, link_value=None): - super(ExternalDiscussionTab, self).__init__( - # Translators: 'Discussion' refers to the tab in the courseware that leads to the discussion forums - name=_('Discussion'), - tab_id='discussion', - link_value=tab_dict['link'] if tab_dict else link_value, - ) - - -class ExternalLinkTab(LinkTab): - """ - A tab containing an external link. - """ - type = 'external_link' - - def __init__(self, tab_dict): - super(ExternalLinkTab, self).__init__( - name=tab_dict['name'], - tab_id=None, # External links are never active. - link_value=tab_dict['link'], - ) - - -class StaticTab(CourseTab): - """ - A custom tab. - """ - type = 'static_tab' - - @classmethod - def validate(cls, tab_dict, raise_error=True): - return super(StaticTab, cls).validate(tab_dict, raise_error) and key_checker(['name', 'url_slug'])(tab_dict, raise_error) - - def __init__(self, tab_dict=None, name=None, url_slug=None): - 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=lambda course, reverse_func: reverse_func(self.type, args=[course.id.to_deprecated_string(), self.url_slug]), - ) - - 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): - 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 SingleTextbookTab(CourseTab): - """ - A tab representing a single textbook. It is created temporarily when enumerating all textbooks within a - Textbook collection tab. It should not be serialized or persisted. - """ - type = 'single_textbook' - is_movable = False - is_collection_item = True - - def to_json(self): - raise NotImplementedError('SingleTextbookTab should not be serialized.') - - -class TextbookTabsBase(AuthenticatedCourseTab): - """ - Abstract class for textbook collection tabs classes. - """ - is_collection = True - - def __init__(self, tab_id): - # Translators: 'Textbooks' refers to the tab in the course that leads to the course' textbooks - super(TextbookTabsBase, self).__init__( - name=_("Textbooks"), - tab_id=tab_id, - link_func=None, - ) - - @abstractmethod - def items(self, course): - """ - A generator for iterating through all the SingleTextbookTab book objects associated with this - collection of textbooks. - """ - pass - - -class TextbookTabs(TextbookTabsBase): - """ - A tab representing the collection of all textbook tabs. - """ - type = 'textbooks' - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(TextbookTabs, self).__init__( - tab_id=self.type, - ) - - def is_enabled(self, course, settings, user=None): - return settings.FEATURES.get('ENABLE_TEXTBOOK') - - def items(self, course): - for index, textbook in enumerate(course.textbooks): - yield SingleTextbookTab( - name=textbook.title, - tab_id='textbook/{0}'.format(index), - link_func=lambda course, reverse_func, index=index: reverse_func( - 'book', args=[course.id.to_deprecated_string(), index] - ), + from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager + 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) + except PluginError: + log.exception( + "Unknown tab type %r Known types: %r.", + tab_type_name, + CourseViewTypeManager.get_course_view_types() ) - - -class PDFTextbookTabs(TextbookTabsBase): - """ - A tab representing the collection of all PDF textbook tabs. - """ - type = 'pdf_textbooks' - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(PDFTextbookTabs, self).__init__( - tab_id=self.type, - ) - - def items(self, course): - for index, textbook in enumerate(course.pdf_textbooks): - yield SingleTextbookTab( - name=textbook['tab_title'], - tab_id='pdftextbook/{0}'.format(index), - link_func=lambda course, reverse_func, index=index: reverse_func( - 'pdf_book', args=[course.id.to_deprecated_string(), index] - ), - ) - - -class HtmlTextbookTabs(TextbookTabsBase): - """ - A tab representing the collection of all Html textbook tabs. - """ - type = 'html_textbooks' - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(HtmlTextbookTabs, self).__init__( - tab_id=self.type, - ) - - def items(self, course): - for index, textbook in enumerate(course.html_textbooks): - yield SingleTextbookTab( - name=textbook['tab_title'], - tab_id='htmltextbook/{0}'.format(index), - link_func=lambda course, reverse_func, index=index: reverse_func( - 'html_book', args=[course.id.to_deprecated_string(), index] - ), - ) - - -class GradingTab(object): - """ - Abstract class for tabs that involve Grading. - """ - pass - - -class StaffGradingTab(StaffTab, GradingTab): - """ - A tab for staff grading. - """ - type = 'staff_grading' - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(StaffGradingTab, self).__init__( - # Translators: "Staff grading" appears on a tab that allows - # staff to view open-ended problems that require staff grading - name=_("Staff grading"), - tab_id=self.type, - link_func=link_reverse_func(self.type), - ) - - -class PeerGradingTab(AuthenticatedCourseTab, GradingTab): - """ - A tab for peer grading. - """ - type = 'peer_grading' - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(PeerGradingTab, self).__init__( - # Translators: "Peer grading" appears on a tab that allows - # students to view open-ended problems that require grading - name=_("Peer grading"), - tab_id=self.type, - link_func=link_reverse_func(self.type), - ) - - -class OpenEndedGradingTab(AuthenticatedCourseTab, GradingTab): - """ - A tab for open ended grading. - """ - type = 'open_ended' - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(OpenEndedGradingTab, self).__init__( - # 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 - name=_("Open Ended Panel"), - tab_id=self.type, - link_func=link_reverse_func('open_ended_notifications'), - ) - - -class SyllabusTab(CourseTab): - """ - A tab for the course syllabus. - """ - type = 'syllabus' - - def is_enabled(self, course, settings, user=None): - return hasattr(course, 'syllabus_present') and course.syllabus_present - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(SyllabusTab, self).__init__( - # Translators: "Syllabus" appears on a tab that, when clicked, opens the syllabus of the course. - name=_('Syllabus'), - tab_id=self.type, - link_func=link_reverse_func(self.type), - ) - - -class NotesTab(AuthenticatedCourseTab): - """ - A tab for the course notes. - """ - type = 'notes' - - def is_enabled(self, course, settings, user=None): - return settings.FEATURES.get('ENABLE_STUDENT_NOTES') - - def __init__(self, tab_dict=None): - super(NotesTab, self).__init__( - name=tab_dict['name'], - tab_id=self.type, - link_func=link_reverse_func(self.type), - ) - - @classmethod - def validate(cls, tab_dict, raise_error=True): - return super(NotesTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) - - -class CourseViewTab(AuthenticatedCourseTab): - """ - A tab that renders a course view. - """ - - def __init__(self, course_view_type, tab_dict=None): - super(CourseViewTab, self).__init__( - name=tab_dict['name'] if tab_dict else course_view_type.title, - tab_id=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 - - def is_enabled(self, course, settings, user=None): - if not super(CourseViewTab, self).is_enabled(course, settings, user=user): - return False - return self.course_view_type.is_enabled(course, settings, user=user) + return None + tab_type.validate(tab_dict) + return tab_type.create_tab(tab_dict=tab_dict) class CourseTabList(List): @@ -762,6 +215,9 @@ class CourseTabList(List): It is automatically created and can be retrieved through a CourseDescriptor object: course.tabs """ + # TODO: Ideally, we'd like for this list of tabs to be dynamically + # generated by the tabs plugin code. For now, we're leaving it like this to + # preserve backwards compatibility. @staticmethod def initialize_default(course): """ @@ -771,43 +227,47 @@ class CourseTabList(List): """ course.tabs.extend([ - CoursewareTab(), - CourseInfoTab(), + CourseTab.load('courseware'), + CourseTab.load('course_info') ]) # Presence of syllabus tab is indicated by a course attribute if hasattr(course, 'syllabus_present') and course.syllabus_present: - course.tabs.append(SyllabusTab()) + course.tabs.append(CourseTab.load('syllabus')) # If the course has a discussion link specified, use that even if we feature # flag discussions off. Disabling that is mostly a server safety feature # at this point, and we don't need to worry about external sites. if course.discussion_link: - discussion_tab = ExternalDiscussionTab(link_value=course.discussion_link) + discussion_tab = CourseTab.load( + 'external_discussion', name=_('External Discussion'), link=course.discussion_link + ) else: - discussion_tab = DiscussionTab() + discussion_tab = CourseTab.load('discussion') course.tabs.extend([ - TextbookTabs(), + CourseTab.load('textbooks'), discussion_tab, - WikiTab(), - ProgressTab(), + CourseTab.load('wiki'), + CourseTab.load('progress'), ]) @staticmethod def get_discussion(course): """ - Returns the discussion tab for the given course. It can be either of type DiscussionTab - or ExternalDiscussionTab. The returned tab object is self-aware of the 'link' that it corresponds to. + Returns the discussion tab for the given course. It can be either of type 'discussion' + or 'external_discussion'. The returned tab object is self-aware of the 'link' that it corresponds to. """ # the discussion_link setting overrides everything else, even if there is a discussion tab in the course tabs if course.discussion_link: - return ExternalDiscussionTab(link_value=course.discussion_link) + return CourseTab.load( + 'external_discussion', name=_('External Discussion'), link=course.discussion_link + ) # find one of the discussion tab types in the course tabs for tab in course.tabs: - if isinstance(tab, DiscussionTab) or isinstance(tab, ExternalDiscussionTab): + if tab.type == 'discussion' or tab.type == 'external_discussion': return tab return None @@ -833,13 +293,13 @@ class CourseTabList(List): return next((tab for tab in tab_list if tab.tab_id == tab_id), None) @staticmethod - def iterate_displayable(course, settings, user=None, inline_collections=True): + def iterate_displayable(course, user=None, inline_collections=True): """ Generator method for iterating through all tabs that can be displayed for the given course and the given user with the provided access settings. """ for tab in course.tabs: - if tab.is_enabled(course, settings, user=user) and (not user or not tab.is_hideable or not tab.is_hidden): + if tab.is_enabled(course, user=user) and not (user and tab.is_hidden): if tab.is_collection: # If rendering inline that add each item in the collection, # else just show the tab itself as long as it is not empty. @@ -868,25 +328,20 @@ class CourseTabList(List): if len(tabs) < 2: raise InvalidTabsException("Expected at least two tabs. tabs: '{0}'".format(tabs)) - if tabs[0].get('type') != CoursewareTab.type: + if tabs[0].get('type') != 'courseware': raise InvalidTabsException( "Expected first tab to have type 'courseware'. tabs: '{0}'".format(tabs)) - if tabs[1].get('type') != CourseInfoTab.type: + if tabs[1].get('type') != 'course_info': raise InvalidTabsException( "Expected second tab to have type 'course_info'. tabs: '{0}'".format(tabs)) # the following tabs should appear only once - for tab_type in [ - CoursewareTab.type, - CourseInfoTab.type, - NotesTab.type, - TextbookTabs.type, - PDFTextbookTabs.type, - HtmlTextbookTabs.type, - CourseViewTab.type - ]: - cls._validate_num_tabs_of_type(tabs, tab_type, 1) + # 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) @staticmethod def _validate_num_tabs_of_type(tabs, tab_type, max_num): @@ -923,26 +378,15 @@ class CourseTabList(List): Overrides the from_json method to de-serialize the CourseTab objects from a json-like representation. """ self.validate_tabs(values) - return [CourseTab.from_json(tab_dict) for tab_dict in values] + tabs = [] + for tab_dict in values: + tab = CourseTab.from_json(tab_dict) + if tab: + tabs.append(tab) + return tabs -#### Link Functions -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()]) - - -def link_value_func(value): - """ - Returns a function takes in a course and reverse_url_func, and returns the given value. - """ - return lambda course, reverse_url_func: value - - -#### Validators +# Validators # A validator takes a dict and raises InvalidTabsException if required fields are missing or otherwise wrong. # (e.g. "is there a 'name' field?). Validators can assume that the type field is valid. def key_checker(expected_keys): diff --git a/common/lib/xmodule/xmodule/tests/test_tabs.py b/common/lib/xmodule/xmodule/tests/test_tabs.py deleted file mode 100644 index f15fbe2a5f..0000000000 --- a/common/lib/xmodule/xmodule/tests/test_tabs.py +++ /dev/null @@ -1,731 +0,0 @@ -"""Tests for Tab classes""" -from mock import MagicMock, patch -import xmodule.tabs as tabs -import unittest -from opaque_keys.edx.locations import SlashSeparatedCourseKey - - -class TabTestCase(unittest.TestCase): - """Base class for Tab-related test cases.""" - def setUp(self): - super(TabTestCase, self).setUp() - - self.course = MagicMock() - self.course.id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') - self.fake_dict_tab = {'fake_key': 'fake_value'} - self.settings = MagicMock() - self.settings.FEATURES = {} - self.reverse = lambda name, args: "name/{0}/args/{1}".format(name, ",".join(str(a) for a in args)) - self.books = None - - def create_mock_user(self, is_authenticated=True, is_staff=True, is_enrolled=True): - """ - Creates a mock user with the specified properties. - """ - user = MagicMock() - user.name = 'mock_user' - user.is_staff = is_staff - user.is_enrolled = is_enrolled - user.is_authenticated = lambda: is_authenticated - return user - - @patch('xmodule.tabs.is_user_enrolled_or_staff') - @patch('xmodule.tabs.is_user_staff') - def is_tab_enabled(self, tab, course, settings, user, is_staff_mock=None, is_enrolled_or_staff_mock=None): - """ - Returns true if the specified tab is enabled. - """ - is_staff_mock.return_value = user.is_staff - is_enrolled_or_staff_mock.return_value = user.is_enrolled or user.is_staff - - return tab.is_enabled(course, settings, user=user) - - def set_up_books(self, num_books): - """Initializes the textbooks in the course and adds the given number of books to each textbook""" - self.books = [MagicMock() for _ in range(num_books)] - for book_index, book in enumerate(self.books): - book.title = 'Book{0}'.format(book_index) - self.course.textbooks = self.books - self.course.pdf_textbooks = self.books - self.course.html_textbooks = self.books - - def check_tab( - self, - tab_class, - dict_tab, - expected_link, - expected_tab_id, - expected_name='same', - invalid_dict_tab=None, - ): - """ - Helper method to verify a tab class. - - 'tab_class' is the class of the tab that is being tested - 'dict_tab' is the raw dictionary value of the tab - 'expected_link' is the expected value for the hyperlink of the tab - 'expected_tab_id' is the expected value for the unique id of the tab - 'expected_name' is the expected value for the name of the tab - 'invalid_dict_tab' is an invalid dictionary value for the tab. - Can be 'None' if the given tab class does not have any keys to validate. - """ - # create tab - tab = tab_class(dict_tab) - - # name is as expected - self.assertEqual(tab.name, expected_name) - - # link is as expected - self.assertEqual(tab.link_func(self.course, self.reverse), expected_link) - - # verify active page name - self.assertEqual(tab.tab_id, expected_tab_id) - - # validate tab - self.assertTrue(tab.validate(dict_tab)) - if invalid_dict_tab: - with self.assertRaises(tabs.InvalidTabsException): - tab.validate(invalid_dict_tab) - - # check get and set methods - self.check_get_and_set_methods(tab) - - # check to_json and from_json methods - self.check_tab_json_methods(tab) - - # check equality methods - self.check_tab_equality(tab, dict_tab) - - # return tab for any additional tests - return tab - - def check_tab_equality(self, tab, dict_tab): - """Tests the equality methods on the given tab""" - self.assertEquals(tab, dict_tab) # test __eq__ - ne_dict_tab = dict_tab - ne_dict_tab['type'] = 'fake_type' - self.assertNotEquals(tab, ne_dict_tab) # test __ne__: incorrect type - self.assertNotEquals(tab, {'fake_key': 'fake_value'}) # test __ne__: missing type - - def check_tab_json_methods(self, tab): - """Tests the json from and to methods on the given tab""" - serialized_tab = tab.to_json() - deserialized_tab = tab.from_json(serialized_tab) - self.assertEquals(serialized_tab, deserialized_tab) - - def check_can_display_results( - self, - tab, - expected_value=True, - for_authenticated_users_only=False, - for_staff_only=False, - for_enrolled_users_only=False - ): - """Checks can display results for various users""" - if for_staff_only: - user = self.create_mock_user(is_authenticated=True, is_staff=True, is_enrolled=True) - self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, self.settings, user)) - if for_authenticated_users_only: - user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=False) - self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, self.settings, user)) - if not for_staff_only and not for_authenticated_users_only and not for_enrolled_users_only: - user = self.create_mock_user(is_authenticated=False, is_staff=False, is_enrolled=False) - self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, self.settings, user)) - if for_enrolled_users_only: - user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=True) - self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, self.settings, user)) - - def check_get_and_set_methods(self, tab): - """Test __getitem__ and __setitem__ calls""" - self.assertEquals(tab['type'], tab.type) - self.assertEquals(tab['tab_id'], tab.tab_id) - with self.assertRaises(KeyError): - _ = tab['invalid_key'] - - self.check_get_and_set_method_for_key(tab, 'name') - self.check_get_and_set_method_for_key(tab, 'tab_id') - with self.assertRaises(KeyError): - tab['invalid_key'] = 'New Value' - - def check_get_and_set_method_for_key(self, tab, key): - """Test __getitem__ and __setitem__ for the given key""" - old_value = tab[key] - new_value = 'New Value' - tab[key] = new_value - self.assertEquals(tab[key], new_value) - tab[key] = old_value - self.assertEquals(tab[key], old_value) - - -class ProgressTestCase(TabTestCase): - """Test cases for Progress Tab.""" - - def check_progress_tab(self): - """Helper function for verifying the progress tab.""" - return self.check_tab( - tab_class=tabs.ProgressTab, - dict_tab={'type': tabs.ProgressTab.type, 'name': 'same'}, - expected_link=self.reverse('progress', args=[self.course.id.to_deprecated_string()]), - expected_tab_id=tabs.ProgressTab.type, - invalid_dict_tab=None, - ) - - def test_progress(self): - - self.course.hide_progress_tab = False - tab = self.check_progress_tab() - self.check_can_display_results( - tab, for_staff_only=True, for_enrolled_users_only=True - ) - - self.course.hide_progress_tab = True - self.check_progress_tab() - self.check_can_display_results( - tab, for_staff_only=True, for_enrolled_users_only=True, expected_value=False - ) - - -class WikiTestCase(TabTestCase): - """Test cases for Wiki Tab.""" - - def check_wiki_tab(self): - """Helper function for verifying the wiki tab.""" - return self.check_tab( - tab_class=tabs.WikiTab, - dict_tab={'type': tabs.WikiTab.type, 'name': 'same'}, - expected_link=self.reverse('course_wiki', args=[self.course.id.to_deprecated_string()]), - expected_tab_id=tabs.WikiTab.type, - invalid_dict_tab=self.fake_dict_tab, - ) - - def test_wiki_enabled_and_public(self): - """ - Test wiki tab when Enabled setting is True and the wiki is open to - the public. - """ - self.settings.WIKI_ENABLED = True - self.course.allow_public_wiki_access = True - tab = self.check_wiki_tab() - self.check_can_display_results(tab) - - def test_wiki_enabled_and_not_public(self): - """ - Test wiki when it is enabled but not open to the public - """ - self.settings.WIKI_ENABLED = True - self.course.allow_public_wiki_access = False - tab = self.check_wiki_tab() - self.check_can_display_results(tab, for_enrolled_users_only=True, for_staff_only=True) - - def test_wiki_enabled_false(self): - """Test wiki tab when Enabled setting is False""" - - self.settings.WIKI_ENABLED = False - tab = self.check_wiki_tab() - self.check_can_display_results(tab, expected_value=False) - - def test_wiki_visibility(self): - """Test toggling of visibility of wiki tab""" - - wiki_tab = tabs.WikiTab() - self.assertTrue(wiki_tab.is_hideable) - wiki_tab.is_hidden = True - self.assertTrue(wiki_tab['is_hidden']) - self.check_tab_json_methods(wiki_tab) - self.check_tab_equality(wiki_tab, wiki_tab.to_json()) - wiki_tab['is_hidden'] = False - self.assertFalse(wiki_tab.is_hidden) - - -class ExternalLinkTestCase(TabTestCase): - """Test cases for External Link Tab.""" - - def test_external_link(self): - - link_value = 'link_value' - tab = self.check_tab( - tab_class=tabs.ExternalLinkTab, - dict_tab={'type': tabs.ExternalLinkTab.type, 'name': 'same', 'link': link_value}, - expected_link=link_value, - expected_tab_id=None, - invalid_dict_tab=self.fake_dict_tab, - ) - self.check_can_display_results(tab) - self.check_get_and_set_method_for_key(tab, 'link') - - -class StaticTabTestCase(TabTestCase): - """Test cases for Static Tab.""" - - def test_static_tab(self): - - url_slug = 'schmug' - - tab = self.check_tab( - tab_class=tabs.StaticTab, - dict_tab={'type': 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, - ) - self.check_can_display_results(tab) - self.check_get_and_set_method_for_key(tab, 'url_slug') - - -class TextbooksTestCase(TabTestCase): - """Test cases for Textbook Tab.""" - - def setUp(self): - super(TextbooksTestCase, self).setUp() - - self.set_up_books(2) - - self.dict_tab = MagicMock() - self.course.tabs = [ - tabs.CoursewareTab(), - tabs.CourseInfoTab(), - tabs.TextbookTabs(), - tabs.PDFTextbookTabs(), - tabs.HtmlTextbookTabs(), - ] - self.num_textbook_tabs = sum(1 for tab in self.course.tabs if isinstance(tab, tabs.TextbookTabsBase)) - self.num_textbooks = self.num_textbook_tabs * len(self.books) - - @patch('xmodule.tabs.is_user_enrolled_or_staff') - def test_textbooks_enabled(self, is_enrolled_or_staff_mock): - is_enrolled_or_staff_mock.return_value = True - - type_to_reverse_name = {'textbook': 'book', 'pdftextbook': 'pdf_book', 'htmltextbook': 'html_book'} - - self.settings.FEATURES['ENABLE_TEXTBOOK'] = True - num_textbooks_found = 0 - user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=True) - for tab in tabs.CourseTabList.iterate_displayable(self.course, self.settings, user=user): - # verify all textbook type tabs - if isinstance(tab, tabs.SingleTextbookTab): - book_type, book_index = tab.tab_id.split("/", 1) - expected_link = self.reverse( - type_to_reverse_name[book_type], - args=[self.course.id.to_deprecated_string(), book_index] - ) - self.assertEqual(tab.link_func(self.course, self.reverse), expected_link) - self.assertTrue(tab.name.startswith('Book{0}'.format(book_index))) - num_textbooks_found = num_textbooks_found + 1 - self.assertEquals(num_textbooks_found, self.num_textbooks) - - def test_textbooks_disabled(self): - - self.settings.FEATURES['ENABLE_TEXTBOOK'] = False - tab = tabs.TextbookTabs(self.dict_tab) - self.check_can_display_results(tab, for_authenticated_users_only=True, expected_value=False) - - -class GradingTestCase(TabTestCase): - """Test cases for Grading related Tabs.""" - - def check_grading_tab(self, tab_class, name, link_value): - """Helper function for verifying the grading tab.""" - return self.check_tab( - tab_class=tab_class, - dict_tab={'type': tab_class.type, 'name': name}, - expected_name=name, - expected_link=self.reverse(link_value, args=[self.course.id.to_deprecated_string()]), - expected_tab_id=tab_class.type, - invalid_dict_tab=None, - ) - - def test_grading_tabs(self): - - peer_grading_tab = self.check_grading_tab( - tabs.PeerGradingTab, - 'Peer grading', - 'peer_grading' - ) - self.check_can_display_results(peer_grading_tab, for_authenticated_users_only=True) - open_ended_grading_tab = self.check_grading_tab( - tabs.OpenEndedGradingTab, - 'Open Ended Panel', - 'open_ended_notifications' - ) - self.check_can_display_results(open_ended_grading_tab, for_authenticated_users_only=True) - staff_grading_tab = self.check_grading_tab( - tabs.StaffGradingTab, - 'Staff grading', - 'staff_grading' - ) - self.check_can_display_results(staff_grading_tab, for_staff_only=True) - - -class NotesTestCase(TabTestCase): - """Test cases for Notes Tab.""" - - def check_notes_tab(self): - """Helper function for verifying the notes tab.""" - return self.check_tab( - tab_class=tabs.NotesTab, - dict_tab={'type': tabs.NotesTab.type, 'name': 'same'}, - expected_link=self.reverse('notes', args=[self.course.id.to_deprecated_string()]), - expected_tab_id=tabs.NotesTab.type, - invalid_dict_tab=self.fake_dict_tab, - ) - - def test_notes_tabs_enabled(self): - self.settings.FEATURES['ENABLE_STUDENT_NOTES'] = True - tab = self.check_notes_tab() - self.check_can_display_results(tab, for_authenticated_users_only=True) - - def test_notes_tabs_disabled(self): - self.settings.FEATURES['ENABLE_STUDENT_NOTES'] = False - tab = self.check_notes_tab() - self.check_can_display_results(tab, expected_value=False) - - -class SyllabusTestCase(TabTestCase): - """Test cases for Syllabus Tab.""" - - def check_syllabus_tab(self, expected_can_display_value): - """Helper function for verifying the syllabus tab.""" - - name = 'Syllabus' - tab = self.check_tab( - tab_class=tabs.SyllabusTab, - dict_tab={'type': tabs.SyllabusTab.type, 'name': name}, - expected_name=name, - expected_link=self.reverse('syllabus', args=[self.course.id.to_deprecated_string()]), - expected_tab_id=tabs.SyllabusTab.type, - invalid_dict_tab=None, - ) - self.check_can_display_results(tab, expected_value=expected_can_display_value) - - def test_syllabus_tab_enabled(self): - self.course.syllabus_present = True - self.check_syllabus_tab(True) - - def test_syllabus_tab_disabled(self): - self.course.syllabus_present = False - self.check_syllabus_tab(False) - - -class KeyCheckerTestCase(unittest.TestCase): - """Test cases for KeyChecker class""" - - def setUp(self): - super(KeyCheckerTestCase, self).setUp() - - self.valid_keys = ['a', 'b'] - self.invalid_keys = ['a', 'v', 'g'] - self.dict_value = {'a': 1, 'b': 2, 'c': 3} - - def test_key_checker(self): - - self.assertTrue(tabs.key_checker(self.valid_keys)(self.dict_value, raise_error=False)) - self.assertFalse(tabs.key_checker(self.invalid_keys)(self.dict_value, raise_error=False)) - with self.assertRaises(tabs.InvalidTabsException): - tabs.key_checker(self.invalid_keys)(self.dict_value) - - -class NeedNameTestCase(unittest.TestCase): - """Test cases for NeedName validator""" - - def setUp(self): - super(NeedNameTestCase, self).setUp() - - self.valid_dict1 = {'a': 1, 'name': 2} - self.valid_dict2 = {'name': 1} - self.valid_dict3 = {'a': 1, 'name': 2, 'b': 3} - self.invalid_dict = {'a': 1, 'b': 2} - - def test_need_name(self): - self.assertTrue(tabs.need_name(self.valid_dict1)) - self.assertTrue(tabs.need_name(self.valid_dict2)) - self.assertTrue(tabs.need_name(self.valid_dict3)) - with self.assertRaises(tabs.InvalidTabsException): - tabs.need_name(self.invalid_dict) - - -class TabListTestCase(TabTestCase): - """Base class for Test cases involving tab lists.""" - - def setUp(self): - super(TabListTestCase, self).setUp() - - # invalid tabs - self.invalid_tabs = [ - # less than 2 tabs - [{'type': tabs.CoursewareTab.type}], - # missing course_info - [{'type': tabs.CoursewareTab.type}, {'type': tabs.DiscussionTab.type, 'name': 'fake_name'}], - # incorrect order - [{'type': tabs.CourseInfoTab.type, 'name': 'fake_name'}, {'type': tabs.CoursewareTab.type}], - # invalid type - [{'type': tabs.CoursewareTab.type}, {'type': tabs.CourseInfoTab.type, 'name': 'fake_name'}, {'type': 'fake_type'}], - ] - - # tab types that should appear only once - unique_tab_types = [ - tabs.CourseInfoTab.type, - tabs.CoursewareTab.type, - tabs.NotesTab.type, - tabs.TextbookTabs.type, - tabs.PDFTextbookTabs.type, - tabs.HtmlTextbookTabs.type, - ] - - for unique_tab_type in unique_tab_types: - self.invalid_tabs.append([ - {'type': tabs.CoursewareTab.type}, - {'type': tabs.CourseInfoTab.type, 'name': 'fake_name'}, - # add the unique tab multiple times - {'type': unique_tab_type}, - {'type': unique_tab_type}, - ]) - - # valid tabs - self.valid_tabs = [ - # empty list - [], - # all valid tabs - [ - {'type': tabs.CoursewareTab.type}, - {'type': tabs.CourseInfoTab.type, 'name': 'fake_name'}, - {'type': tabs.WikiTab.type, 'name': 'fake_name'}, - {'type': tabs.DiscussionTab.type, 'name': 'fake_name'}, - {'type': tabs.ExternalLinkTab.type, 'name': 'fake_name', 'link': 'fake_link'}, - {'type': tabs.TextbookTabs.type}, - {'type': tabs.PDFTextbookTabs.type}, - {'type': tabs.HtmlTextbookTabs.type}, - {'type': tabs.ProgressTab.type, 'name': 'fake_name'}, - {'type': tabs.StaticTab.type, 'name': 'fake_name', 'url_slug': 'schlug'}, - {'type': tabs.PeerGradingTab.type}, - {'type': tabs.StaffGradingTab.type}, - {'type': tabs.OpenEndedGradingTab.type}, - {'type': tabs.NotesTab.type, 'name': 'fake_name'}, - {'type': tabs.SyllabusTab.type}, - ], - # with external discussion - [ - {'type': tabs.CoursewareTab.type}, - {'type': tabs.CourseInfoTab.type, 'name': 'fake_name'}, - {'type': tabs.ExternalDiscussionTab.type, 'name': 'fake_name', 'link': 'fake_link'} - ], - ] - - self.all_valid_tab_list = tabs.CourseTabList().from_json(self.valid_tabs[1]) - - -class ValidateTabsTestCase(TabListTestCase): - """Test cases for validating tabs.""" - - def test_validate_tabs(self): - tab_list = tabs.CourseTabList() - for invalid_tab_list in self.invalid_tabs: - with self.assertRaises(tabs.InvalidTabsException): - tab_list.from_json(invalid_tab_list) - - for valid_tab_list in self.valid_tabs: - from_json_result = tab_list.from_json(valid_tab_list) - self.assertEquals(len(from_json_result), len(valid_tab_list)) - - -class CourseTabListTestCase(TabListTestCase): - """Testing the generator method for iterating through displayable tabs""" - - def test_initialize_default_without_syllabus(self): - self.course.tabs = [] - self.course.syllabus_present = False - tabs.CourseTabList.initialize_default(self.course) - self.assertTrue(tabs.SyllabusTab() not in self.course.tabs) - - def test_initialize_default_with_syllabus(self): - self.course.tabs = [] - self.course.syllabus_present = True - tabs.CourseTabList.initialize_default(self.course) - self.assertTrue(tabs.SyllabusTab() in self.course.tabs) - - def test_initialize_default_with_external_link(self): - self.course.tabs = [] - self.course.discussion_link = "other_discussion_link" - tabs.CourseTabList.initialize_default(self.course) - self.assertTrue(tabs.ExternalDiscussionTab(link_value="other_discussion_link") in self.course.tabs) - self.assertTrue(tabs.DiscussionTab() not in self.course.tabs) - - def test_initialize_default_without_external_link(self): - self.course.tabs = [] - self.course.discussion_link = "" - tabs.CourseTabList.initialize_default(self.course) - self.assertTrue(tabs.ExternalDiscussionTab() not in self.course.tabs) - self.assertTrue(tabs.DiscussionTab() in self.course.tabs) - - @patch('xmodule.tabs.is_user_enrolled_or_staff') - @patch('xmodule.tabs.is_user_staff') - def test_iterate_displayable(self, is_staff_mock, is_enrolled_or_staff_mock): - is_staff_mock.return_value = True - is_enrolled_or_staff_mock.return_value = True - # enable all tab types - self.settings.FEATURES['ENABLE_TEXTBOOK'] = True - self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = True - self.settings.FEATURES['ENABLE_STUDENT_NOTES'] = True - self.settings.FEATURES['ENABLE_EDXNOTES'] = True - self.course.hide_progress_tab = False - - # create 1 book per textbook type - self.set_up_books(1) - - # initialize the course tabs to a list of all valid tabs - self.course.tabs = self.all_valid_tab_list - - # enumerate the tabs with no user - for i, tab in enumerate(tabs.CourseTabList.iterate_displayable( - self.course, - self.settings, - inline_collections=False - )): - self.assertEquals(tab.type, self.course.tabs[i].type) - - # enumerate the tabs with a staff user - user = self.create_mock_user(is_authenticated=True, is_staff=True, is_enrolled=True) - for i, tab in enumerate(tabs.CourseTabList.iterate_displayable( - self.course, - self.settings, - user=user - )): - if getattr(tab, 'is_collection_item', False): - # a collection item was found as a result of a collection tab - self.assertTrue(getattr(self.course.tabs[i], 'is_collection', False)) - else: - # all other tabs must match the expected type - self.assertEquals(tab.type, self.course.tabs[i].type) - - # test including non-empty collections - self.assertIn( - tabs.HtmlTextbookTabs(), - list(tabs.CourseTabList.iterate_displayable(self.course, self.settings, inline_collections=False)), - ) - - # test not including empty collections - self.course.html_textbooks = [] - self.assertNotIn( - tabs.HtmlTextbookTabs(), - list(tabs.CourseTabList.iterate_displayable(self.course, self.settings, inline_collections=False)), - ) - - def test_get_tab_by_methods(self): - """Tests the get_tab methods in CourseTabList""" - self.course.tabs = self.all_valid_tab_list - for tab in self.course.tabs: - - # get tab by type - self.assertEquals(tabs.CourseTabList.get_tab_by_type(self.course.tabs, tab.type), tab) - - # get tab by id - self.assertEquals(tabs.CourseTabList.get_tab_by_id(self.course.tabs, tab.tab_id), tab) - - -class DiscussionLinkTestCase(TabTestCase): - """Test cases for discussion link tab.""" - - def setUp(self): - super(DiscussionLinkTestCase, self).setUp() - - self.tabs_with_discussion = [ - tabs.CoursewareTab(), - tabs.CourseInfoTab(), - tabs.DiscussionTab(), - tabs.TextbookTabs(), - ] - self.tabs_without_discussion = [ - tabs.CoursewareTab(), - tabs.CourseInfoTab(), - tabs.TextbookTabs(), - ] - - @staticmethod - def _reverse(course): - """Custom reverse function""" - def reverse_discussion_link(viewname, args): - """reverse lookup for discussion link""" - if viewname == "django_comment_client.forum.views.forum_form_discussion" and args == [course.id.to_deprecated_string()]: - return "default_discussion_link" - return reverse_discussion_link - - def check_discussion( - self, tab_list, - expected_discussion_link, - expected_can_display_value, - discussion_link_in_course="", - is_staff=True, - is_enrolled=True, - ): - """Helper function to verify whether the discussion tab exists and can be displayed""" - self.course.tabs = tab_list - self.course.discussion_link = discussion_link_in_course - discussion_tab = tabs.CourseTabList.get_discussion(self.course) - user = self.create_mock_user(is_authenticated=True, is_staff=is_staff, is_enrolled=is_enrolled) - self.assertEquals( - ( - discussion_tab is not None and - self.is_tab_enabled(discussion_tab, self.course, self.settings, user) and - (discussion_tab.link_func(self.course, self._reverse(self.course)) == expected_discussion_link) - ), - expected_can_display_value - ) - - def test_explicit_discussion_link(self): - """Test that setting discussion_link overrides everything else""" - self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = False - self.check_discussion( - tab_list=self.tabs_with_discussion, - discussion_link_in_course="other_discussion_link", - expected_discussion_link="other_discussion_link", - expected_can_display_value=True, - ) - - def test_discussions_disabled(self): - """Test that other cases return None with discussions disabled""" - self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = False - for tab_list in [[], self.tabs_with_discussion, self.tabs_without_discussion]: - self.check_discussion( - tab_list=tab_list, - expected_discussion_link=not None, - expected_can_display_value=False, - ) - - def test_tabs_with_discussion(self): - """Test a course with a discussion tab configured""" - self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = True - self.check_discussion( - tab_list=self.tabs_with_discussion, - expected_discussion_link="default_discussion_link", - expected_can_display_value=True, - ) - - def test_tabs_without_discussion(self): - """Test a course with tabs configured but without a discussion tab""" - self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = True - self.check_discussion( - tab_list=self.tabs_without_discussion, - expected_discussion_link=not None, - expected_can_display_value=False, - ) - - def test_tabs_enrolled_or_staff(self): - self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = True - for is_enrolled, is_staff in [(True, False), (False, True)]: - self.check_discussion( - tab_list=self.tabs_with_discussion, - expected_discussion_link="default_discussion_link", - expected_can_display_value=True, - is_enrolled=is_enrolled, - is_staff=is_staff - ) - - def test_tabs_not_enrolled_or_staff(self): - self.settings.FEATURES['ENABLE_DISCUSSION_SERVICE'] = True - is_enrolled = is_staff = False - self.check_discussion( - tab_list=self.tabs_with_discussion, - expected_discussion_link="default_discussion_link", - expected_can_display_value=False, - is_enrolled=is_enrolled, - is_staff=is_staff - ) diff --git a/lms/djangoapps/ccx/plugins.py b/lms/djangoapps/ccx/plugins.py index b1611d93c3..3479fa9a56 100644 --- a/lms/djangoapps/ccx/plugins.py +++ b/lms/djangoapps/ccx/plugins.py @@ -2,9 +2,10 @@ Registers the CCX feature for the edX platform. """ +from django.conf import settings from django.utils.translation import ugettext as _ -from openedx.core.lib.plugins.api import CourseViewType +from openedx.core.djangoapps.course_views.course_views import CourseViewType from student.roles import CourseCcxCoachRole @@ -16,10 +17,10 @@ class CcxCourseViewType(CourseViewType): name = "ccx_coach" title = _("CCX Coach") view_name = "ccx_coach_dashboard" - is_persistent = False + is_dynamic = True # The CCX view is dynamically added to the set of tabs when it is enabled @classmethod - def is_enabled(cls, course, settings, user=None): + def is_enabled(cls, course, user=None): """ Returns true if CCX has been enabled and the specified user is a coach """ diff --git a/lms/djangoapps/ccx/tests/test_views.py b/lms/djangoapps/ccx/tests/test_views.py index 28679a8a04..a217e791cc 100644 --- a/lms/djangoapps/ccx/tests/test_views.py +++ b/lms/djangoapps/ccx/tests/test_views.py @@ -14,6 +14,7 @@ from capa.tests.response_xml_factory import StringResponseXMLFactory from courseware.field_overrides import OverrideFieldData # pylint: disable=import-error from courseware.tests.factories import StudentModuleFactory # pylint: disable=import-error from courseware.tests.helpers import LoginEnrollmentTestCase # pylint: disable=import-error +from courseware.tabs import get_course_tab_list from django.core.urlresolvers import reverse from django.test.utils import override_settings from django.test import RequestFactory @@ -773,21 +774,24 @@ class TestSwitchActiveCCX(ModuleStoreTestCase, LoginEnrollmentTestCase): @ddt.ddt -class CCXCoachTabTestCase(unittest.TestCase): +class CCXCoachTabTestCase(ModuleStoreTestCase): """ Test case for CCX coach tab. """ def setUp(self): super(CCXCoachTabTestCase, self).setUp() - self.course = MagicMock() - self.course.id = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') - self.settings = MagicMock() - self.settings.FEATURES = {} + self.course = CourseFactory.create() + self.user = UserFactory.create() + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) + role = CourseCcxCoachRole(self.course.id) + role.add_users(self.user) def check_ccx_tab(self): """Helper function for verifying the ccx tab.""" - tab = tabs.CcxCoachTab({'type': tabs.CcxCoachTab.type, 'name': 'CCX Coach'}) - return tab + request = RequestFactory().request() + request.user = self.user + all_tabs = get_course_tab_list(request, self.course) + return any(tab.type == 'ccx_coach' for tab in all_tabs) @ddt.data( (True, True, True), @@ -796,22 +800,17 @@ class CCXCoachTabTestCase(unittest.TestCase): (False, False, False), (True, None, False) ) - @patch('ccx.overrides.get_current_request', ccx_dummy_request) @ddt.unpack def test_coach_tab_for_ccx_advance_settings(self, ccx_feature_flag, enable_ccx, expected_result): """ Test ccx coach tab state (visible or hidden) depending on the value of enable_ccx flag, ccx feature flag. """ - tab = self.check_ccx_tab() - self.settings.FEATURES = {'CUSTOM_COURSES_EDX': ccx_feature_flag} - - self.course.enable_ccx = enable_ccx - self.assertEquals( - expected_result, - tab.can_display( - self.course, self.settings, is_user_authenticated=True, is_user_staff=False, is_user_enrolled=True + with self.settings(FEATURES={'CUSTOM_COURSES_EDX': ccx_feature_flag}): + self.course.enable_ccx = enable_ccx + self.assertEquals( + expected_result, + self.check_ccx_tab() ) - ) def flatten(seq): diff --git a/lms/djangoapps/course_wiki/tab.py b/lms/djangoapps/course_wiki/tab.py new file mode 100644 index 0000000000..e5fbdee949 --- /dev/null +++ b/lms/djangoapps/course_wiki/tab.py @@ -0,0 +1,31 @@ +""" +These callables are used by django-wiki to check various permissions +a user has on an article. +""" + +from django.conf import settings +from django.utils.translation import ugettext as _ + +from courseware.tabs import EnrolledCourseViewType + + +class WikiCourseViewType(EnrolledCourseViewType): + """ + Defines the Wiki view type that is shown as a course tab. + """ + + name = "wiki" + title = _('Wiki') + view_name = "course_wiki" + is_hideable = True + + @classmethod + def is_enabled(cls, course, user=None): + """ + Returns true if the wiki is enabled and the specified user is enrolled or has staff access. + """ + if not settings.WIKI_ENABLED: + return False + if course.allow_public_wiki_access: + return True + return super(WikiCourseViewType, cls).is_enabled(course, user=user) diff --git a/lms/djangoapps/course_wiki/tests/test_tab.py b/lms/djangoapps/course_wiki/tests/test_tab.py new file mode 100644 index 0000000000..87b322f280 --- /dev/null +++ b/lms/djangoapps/course_wiki/tests/test_tab.py @@ -0,0 +1,65 @@ +""" +Tests for wiki views. +""" + +from django.conf import settings +from django.test.client import RequestFactory + +from courseware.tabs import get_course_tab_list +from student.tests.factories import AdminFactory, UserFactory +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory + + +class WikiTabTestCase(ModuleStoreTestCase): + """Test cases for Wiki Tab.""" + + def setUp(self): + super(WikiTabTestCase, self).setUp() + self.course = CourseFactory.create() + self.instructor = AdminFactory.create() + self.user = UserFactory() + + def get_wiki_tab(self, user, course): + """Returns true if the "Wiki" tab is shown.""" + request = RequestFactory().request() + request.user = user + all_tabs = get_course_tab_list(request, course) + wiki_tabs = [tab for tab in all_tabs if tab.name == 'Wiki'] + return wiki_tabs[0] if len(wiki_tabs) == 1 else None + + def test_wiki_enabled_and_public(self): + """ + Test wiki tab when Enabled setting is True and the wiki is open to + the public. + """ + settings.WIKI_ENABLED = True + self.course.allow_public_wiki_access = True + self.assertIsNotNone(self.get_wiki_tab(self.user, self.course)) + + def test_wiki_enabled_and_not_public(self): + """ + Test wiki when it is enabled but not open to the public + """ + settings.WIKI_ENABLED = True + self.course.allow_public_wiki_access = False + self.assertIsNone(self.get_wiki_tab(self.user, self.course)) + self.assertIsNotNone(self.get_wiki_tab(self.instructor, self.course)) + + def test_wiki_enabled_false(self): + """Test wiki tab when Enabled setting is False""" + settings.WIKI_ENABLED = False + self.assertIsNone(self.get_wiki_tab(self.user, self.course)) + self.assertIsNone(self.get_wiki_tab(self.instructor, self.course)) + + def test_wiki_visibility(self): + """Test toggling of visibility of wiki tab""" + settings.WIKI_ENABLED = True + self.course.allow_public_wiki_access = True + wiki_tab = self.get_wiki_tab(self.user, self.course) + self.assertIsNotNone(wiki_tab) + self.assertTrue(wiki_tab.is_hideable) + wiki_tab.is_hidden = True + self.assertTrue(wiki_tab['is_hidden']) + wiki_tab['is_hidden'] = False + self.assertFalse(wiki_tab.is_hidden) diff --git a/lms/djangoapps/courseware/tabs.py b/lms/djangoapps/courseware/tabs.py index ddcdbebcc6..7bb9f15ea5 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -5,8 +5,313 @@ perform some LMS-specific tab display gymnastics for the Entrance Exams feature from django.conf import settings from django.utils.translation import ugettext as _ +from courseware.access import has_access from courseware.entrance_exams import user_must_complete_entrance_exam -from xmodule.tabs import CourseTabList, CourseViewTab, CourseTabManager +from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager, CourseViewType, StaticTab +from student.models import CourseEnrollment +from xmodule.tabs import CourseTab, CourseTabList, key_checker + + +class EnrolledCourseViewType(CourseViewType): + """ + A base class for any view types that require a user to be enrolled. + """ + @classmethod + def is_enabled(cls, course, user=None): + if user is None: + return True + return CourseEnrollment.is_enrolled(user, course.id) or has_access(user, 'staff', course, course.id) + + +class CoursewareViewType(EnrolledCourseViewType): + """ + The main courseware view. + """ + name = 'courseware' + title = _('Courseware') + priority = 10 + view_name = 'courseware' + is_movable = False + + +class CourseInfoViewType(CourseViewType): + """ + The course info view. + """ + name = 'course_info' + title = _('Course Info') + priority = 20 + view_name = 'info' + tab_id = 'info' + is_movable = False + + @classmethod + def is_enabled(cls, course, user=None): + return True + + +class SyllabusCourseViewType(EnrolledCourseViewType): + """ + A tab for the course syllabus. + """ + name = 'syllabus' + title = _('Syllabus') + priority = 30 + view_name = 'syllabus' + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + if not super(SyllabusCourseViewType, cls).is_enabled(course, user=user): + return False + return getattr(course, 'syllabus_present', False) + + +class ProgressCourseViewType(EnrolledCourseViewType): + """ + The course progress view. + """ + name = 'progress' + title = _('Progress') + priority = 40 + view_name = 'progress' + is_hideable = True + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + if not super(ProgressCourseViewType, cls).is_enabled(course, user=user): + return False + return not course.hide_progress_tab + + +class TextbookCourseViewsBase(CourseViewType): + """ + Abstract class for textbook collection tabs classes. + """ + # Translators: 'Textbooks' refers to the tab in the course that leads to the course' textbooks + title = _("Textbooks") + is_collection = True + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + return user is None or user.is_authenticated() + + @classmethod + def items(cls, course): + """ + A generator for iterating through all the SingleTextbookTab book objects associated with this + collection of textbooks. + """ + raise NotImplementedError() + + +class TextbookCourseViews(TextbookCourseViewsBase): + """ + A tab representing the collection of all textbook tabs. + """ + name = '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) + return settings.FEATURES.get('ENABLE_TEXTBOOK') and parent_is_enabled + + @classmethod + def items(cls, course): + for index, textbook in enumerate(course.textbooks): + yield SingleTextbookTab( + name=textbook.title, + tab_id='textbook/{0}'.format(index), + view_name=cls.view_name, + index=index + ) + + +class PDFTextbookCourseViews(TextbookCourseViewsBase): + """ + A tab representing the collection of all PDF textbook tabs. + """ + name = 'pdf_textbooks' + priority = None + view_name = 'pdf_book' + + @classmethod + def items(cls, course): + for index, textbook in enumerate(course.pdf_textbooks): + yield SingleTextbookTab( + name=textbook['tab_title'], + tab_id='pdftextbook/{0}'.format(index), + view_name=cls.view_name, + index=index + ) + + +class HtmlTextbookCourseViews(TextbookCourseViewsBase): + """ + A tab representing the collection of all Html textbook tabs. + """ + name = 'html_textbooks' + priority = None + view_name = 'html_book' + + @classmethod + def items(cls, course): + for index, textbook in enumerate(course.html_textbooks): + yield SingleTextbookTab( + name=textbook['tab_title'], + tab_id='htmltextbook/{0}'.format(index), + view_name=cls.view_name, + index=index + ) + + +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. + """ + link_value = '' + + def __init__(self, tab_dict=None, name=None, link=None): + self.link_value = tab_dict['link'] if tab_dict else link + + def link_value_func(_course, _reverse_func): + """ Returns the link_value as the link. """ + return self.link_value + + 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, + ) + + def __getitem__(self, key): + if key == 'link': + return self.link_value + else: + return super(LinkTab, self).__getitem__(key) + + def __setitem__(self, key, value): + if key == 'link': + self.link_value = value + else: + super(LinkTab, self).__setitem__(key, value) + + def to_json(self): + to_json_val = super(LinkTab, self).to_json() + to_json_val.update({'link': self.link_value}) + return to_json_val + + def __eq__(self, other): + if not super(LinkTab, self).__eq__(other): + return False + return self.link_value == other.get('link') + + +class SingleTextbookTab(CourseTab): + """ + A tab representing a single textbook. It is created temporarily when enumerating all textbooks within a + Textbook collection tab. It should not be serialized or persisted. + """ + type = 'single_textbook' + is_movable = False + is_collection_item = True + priority = None + + def __init__(self, name, tab_id, view_name, index): + def link_func(course, reverse_func, index=index): + """ 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) + + def to_json(self): + raise NotImplementedError('SingleTextbookTab should not be serialized.') def get_course_tab_list(request, course): @@ -14,12 +319,11 @@ def get_course_tab_list(request, course): Retrieves the course tab list from xmodule.tabs and manipulates the set as necessary """ user = request.user - xmodule_tab_list = CourseTabList.iterate_displayable(course, settings, user=user) + xmodule_tab_list = CourseTabList.iterate_displayable(course, user=user) - # Now that we've loaded the tabs for this course, perform the Entrance Exam work - # If the user has to take an entrance exam, we'll need to hide away all of the tabs - # except for the Courseware and Instructor tabs (latter is only viewed if applicable) - # We don't have access to the true request object in this context, but we can use a mock + # Now that we've loaded the tabs for this course, perform the Entrance Exam work. + # If the user has to take an entrance exam, we'll need to hide away all but the + # "Courseware" tab. The tab is then renamed as "Entrance Exam". course_tab_list = [] for tab in xmodule_tab_list: if user_must_complete_entrance_exam(request, user, course): @@ -32,7 +336,6 @@ def get_course_tab_list(request, course): # Add in any dynamic tabs, i.e. those that are not persisted course_tab_list += _get_dynamic_tabs(course, user) - return course_tab_list @@ -44,10 +347,10 @@ def _get_dynamic_tabs(course, user): instead added dynamically based upon the user's role. """ dynamic_tabs = list() - for tab_type in CourseTabManager.get_tab_types().values(): - if not getattr(tab_type, "is_persistent", True): - tab = CourseViewTab(tab_type) - if tab.is_enabled(course, settings, user=user): + for tab_type in CourseViewTypeManager.get_course_view_types(): + if getattr(tab_type, "is_dynamic", False): + tab = tab_type.create_tab(dict()) + if tab.is_enabled(course, user=user): dynamic_tabs.append(tab) dynamic_tabs.sort(key=lambda dynamic_tab: dynamic_tab.name) return dynamic_tabs diff --git a/lms/djangoapps/courseware/tests/test_tabs.py b/lms/djangoapps/courseware/tests/test_tabs.py index b7b40cab31..27900d2636 100644 --- a/lms/djangoapps/courseware/tests/test_tabs.py +++ b/lms/djangoapps/courseware/tests/test_tabs.py @@ -1,7 +1,7 @@ """ Test cases for tabs. -Note: Tests covering workflows in the actual tabs.py file begin after line 100 """ + from django.conf import settings from django.core.urlresolvers import reverse from django.http import Http404 @@ -10,21 +10,208 @@ from nose.plugins.attrib import attr 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 +) from courseware.tests.helpers import get_request_for_user, LoginEnrollmentTestCase from courseware.tests.factories import InstructorFactory, StaffFactory -from xmodule import tabs +from courseware.views import get_static_tab_contents, static_tab +from student.models import CourseEnrollment +from student.tests.factories import UserFactory +from util import milestones_helpers +from xmodule import tabs as xmodule_tabs from xmodule.modulestore.tests.django_utils import ( TEST_DATA_MIXED_TOY_MODULESTORE, TEST_DATA_MIXED_CLOSED_MODULESTORE ) - -from courseware.tabs import get_course_tab_list -from courseware.views import get_static_tab_contents, static_tab -from student.tests.factories import UserFactory -from util import milestones_helpers from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory +class TabTestCase(ModuleStoreTestCase): + """Base class for Tab-related test cases.""" + def setUp(self): + super(TabTestCase, self).setUp() + + self.course = CourseFactory.create(org='edX', course='toy', run='2012_Fall') + self.fake_dict_tab = {'fake_key': 'fake_value'} + self.reverse = lambda name, args: "name/{0}/args/{1}".format(name, ",".join(str(a) for a in args)) + self.books = None + + def create_mock_user(self, is_authenticated=True, is_staff=True, is_enrolled=True): + """ + Creates a mock user with the specified properties. + """ + user = UserFactory() + user.name = 'mock_user' + user.is_staff = is_staff + user.is_enrolled = is_enrolled + user.is_authenticated = lambda: is_authenticated + return user + + def is_tab_enabled(self, tab, course, user): + """ + Returns true if the specified tab is enabled. + """ + return tab.is_enabled(course, user=user) + + def set_up_books(self, num_books): + """Initializes the textbooks in the course and adds the given number of books to each textbook""" + self.books = [MagicMock() for _ in range(num_books)] + for book_index, book in enumerate(self.books): + book.title = 'Book{0}'.format(book_index) + self.course.textbooks = self.books + self.course.pdf_textbooks = self.books + self.course.html_textbooks = self.books + + def check_tab( + self, + tab_class, + dict_tab, + expected_link, + expected_tab_id, + expected_name='same', + invalid_dict_tab=None, + ): + """ + Helper method to verify a tab class. + + 'tab_class' is the class of the tab that is being tested + 'dict_tab' is the raw dictionary value of the tab + 'expected_link' is the expected value for the hyperlink of the tab + 'expected_tab_id' is the expected value for the unique id of the tab + 'expected_name' is the expected value for the name of the tab + 'invalid_dict_tab' is an invalid dictionary value for the tab. + 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) + + # name is as expected + self.assertEqual(tab.name, expected_name) + + # link is as expected + self.assertEqual(tab.link_func(self.course, self.reverse), expected_link) + + # verify active page name + self.assertEqual(tab.tab_id, expected_tab_id) + + # validate tab + self.assertTrue(tab.validate(dict_tab)) + if invalid_dict_tab: + with self.assertRaises(xmodule_tabs.InvalidTabsException): + tab.validate(invalid_dict_tab) + + # check get and set methods + self.check_get_and_set_methods(tab) + + # check to_json and from_json methods + self.check_tab_json_methods(tab) + + # check equality methods + self.check_tab_equality(tab, dict_tab) + + # return tab for any additional tests + return tab + + def check_tab_equality(self, tab, dict_tab): + """Tests the equality methods on the given tab""" + self.assertEquals(tab, dict_tab) # test __eq__ + ne_dict_tab = dict_tab + ne_dict_tab['type'] = 'fake_type' + self.assertNotEquals(tab, ne_dict_tab) # test __ne__: incorrect type + self.assertNotEquals(tab, {'fake_key': 'fake_value'}) # test __ne__: missing type + + def check_tab_json_methods(self, tab): + """Tests the json from and to methods on the given tab""" + serialized_tab = tab.to_json() + deserialized_tab = tab.from_json(serialized_tab) + self.assertEquals(serialized_tab, deserialized_tab) + + def check_can_display_results( + self, + tab, + expected_value=True, + for_authenticated_users_only=False, + for_staff_only=False, + for_enrolled_users_only=False + ): + """Checks can display results for various users""" + if for_staff_only: + user = self.create_mock_user(is_authenticated=True, is_staff=True, is_enrolled=True) + self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) + if for_authenticated_users_only: + user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=False) + self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) + if not for_staff_only and not for_authenticated_users_only and not for_enrolled_users_only: + user = self.create_mock_user(is_authenticated=False, is_staff=False, is_enrolled=False) + self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) + if for_enrolled_users_only: + user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=True) + self.assertEquals(expected_value, self.is_tab_enabled(tab, self.course, user)) + + def check_get_and_set_methods(self, tab): + """Test __getitem__ and __setitem__ calls""" + self.assertEquals(tab['type'], tab.type) + self.assertEquals(tab['tab_id'], tab.tab_id) + with self.assertRaises(KeyError): + _ = tab['invalid_key'] + + self.check_get_and_set_method_for_key(tab, 'name') + self.check_get_and_set_method_for_key(tab, 'tab_id') + with self.assertRaises(KeyError): + tab['invalid_key'] = 'New Value' + + def check_get_and_set_method_for_key(self, tab, key): + """Test __getitem__ and __setitem__ for the given key""" + old_value = tab[key] + new_value = 'New Value' + tab[key] = new_value + self.assertEquals(tab[key], new_value) + tab[key] = old_value + self.assertEquals(tab[key], old_value) + + +class TextbooksTestCase(TabTestCase): + """Test cases for Textbook Tab.""" + + def setUp(self): + super(TextbooksTestCase, self).setUp() + + self.set_up_books(2) + + self.dict_tab = MagicMock() + self.course.tabs = [ + xmodule_tabs.CourseTab.load('textbooks'), + xmodule_tabs.CourseTab.load('pdf_textbooks'), + xmodule_tabs.CourseTab.load('html_textbooks'), + ] + self.num_textbook_tabs = sum(1 for tab in self.course.tabs if tab.type in [ + 'textbooks', 'pdf_textbooks', 'html_textbooks' + ]) + self.num_textbooks = self.num_textbook_tabs * len(self.books) + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_TEXTBOOK": True}) + def test_textbooks_enabled(self): + + type_to_reverse_name = {'textbook': 'book', 'pdftextbook': 'pdf_book', 'htmltextbook': 'html_book'} + + num_textbooks_found = 0 + user = self.create_mock_user(is_authenticated=True, is_staff=False, is_enrolled=True) + for tab in xmodule_tabs.CourseTabList.iterate_displayable(self.course, user=user): + # verify all textbook type tabs + if tab.type == 'single_textbook': + book_type, book_index = tab.tab_id.split("/", 1) + expected_link = self.reverse( + type_to_reverse_name[book_type], + args=[self.course.id.to_deprecated_string(), book_index] + ) + self.assertEqual(tab.link_func(self.course, self.reverse), expected_link) + self.assertTrue(tab.name.startswith('Book{0}'.format(book_index))) + num_textbooks_found = num_textbooks_found + 1 + self.assertEquals(num_textbooks_found, self.num_textbooks) + + @attr('shard_1') class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): """Test cases for Static Tab Dates.""" @@ -38,6 +225,8 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): category="static_tab", parent_location=self.course.location, data="OOGIE BLOOGIE", display_name="new_tab" ) + self.course.tabs.append(xmodule_tabs.CourseTab.load('static_tab', name='New Tab', url_slug='new_tab')) + self.course.save() self.toy_course_key = SlashSeparatedCourseKey('edX', 'toy', '2012_Fall') def test_logged_in(self): @@ -63,7 +252,7 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.setup_user() course = get_course_by_id(self.toy_course_key) request = get_request_for_user(self.user) - tab = tabs.CourseTabList.get_tab_by_slug(course.tabs, 'resources') + tab = xmodule_tabs.CourseTabList.get_tab_by_slug(course.tabs, 'resources') # Test render works okay tab_content = get_static_tab_contents(request, course, tab) @@ -223,27 +412,22 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): @attr('shard_1') -class TextBookTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): +class TextBookCourseViewsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): """ Validate tab behavior when dealing with textbooks. """ MODULESTORE = TEST_DATA_MIXED_TOY_MODULESTORE def setUp(self): - super(TextBookTabsTestCase, self).setUp() + super(TextBookCourseViewsTestCase, self).setUp() self.course = CourseFactory.create() self.set_up_books(2) - self.course.tabs = [ - tabs.CoursewareTab(), - tabs.CourseInfoTab(), - tabs.TextbookTabs(), - tabs.PDFTextbookTabs(), - tabs.HtmlTextbookTabs(), - ] self.setup_user() self.enroll(self.course) - self.num_textbook_tabs = sum(1 for tab in self.course.tabs if isinstance(tab, tabs.TextbookTabsBase)) + self.num_textbook_tabs = sum(1 for tab in self.course.tabs if tab.type in [ + 'textbooks', 'pdf_textbooks', 'html_textbooks' + ]) self.num_textbooks = self.num_textbook_tabs * len(self.books) def set_up_books(self, num_books): @@ -265,7 +449,7 @@ class TextBookTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): num_of_textbooks_found = 0 for tab in course_tab_list: # Verify links of all textbook type tabs. - if isinstance(tab, tabs.SingleTextbookTab): + if tab.type == 'single_textbook': book_type, book_index = tab.tab_id.split("/", 1) expected_link = reverse( type_to_reverse_name[book_type], @@ -275,3 +459,351 @@ class TextBookTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.assertEqual(tab_link, expected_link) num_of_textbooks_found += 1 self.assertEqual(num_of_textbooks_found, self.num_textbooks) + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_TEXTBOOK": False}) + def test_textbooks_disabled(self): + tab = xmodule_tabs.CourseTab.load('textbooks') + self.assertFalse(tab.is_enabled(self.course, self.user)) + + +class TabListTestCase(TabTestCase): + """Base class for Test cases involving tab lists.""" + + def setUp(self): + super(TabListTestCase, self).setUp() + + # invalid tabs + self.invalid_tabs = [ + # less than 2 tabs + [{'type': CoursewareViewType.name}], + # missing course_info + [{'type': CoursewareViewType.name}, {'type': 'discussion', 'name': 'fake_name'}], + # incorrect order + [{'type': CourseInfoViewType.name, 'name': 'fake_name'}, {'type': CoursewareViewType.name}], + ] + + # tab types that should appear only once + unique_tab_types = [ + CoursewareViewType.name, + CourseInfoViewType.name, + 'textbooks', + 'pdf_textbooks', + 'html_textbooks', + ] + + for unique_tab_type in unique_tab_types: + self.invalid_tabs.append([ + {'type': CoursewareViewType.name}, + {'type': CourseInfoViewType.name, 'name': 'fake_name'}, + # add the unique tab multiple times + {'type': unique_tab_type}, + {'type': unique_tab_type}, + ]) + + # valid tabs + self.valid_tabs = [ + # empty list + [], + # all valid tabs + [ + {'type': CoursewareViewType.name}, + {'type': CourseInfoViewType.name, 'name': 'fake_name'}, + {'type': 'discussion', 'name': 'fake_name'}, + {'type': ExternalLinkCourseViewType.name, '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': 'syllabus'}, + ], + # with external discussion + [ + {'type': CoursewareViewType.name}, + {'type': CourseInfoViewType.name, 'name': 'fake_name'}, + {'type': ExternalDiscussionCourseViewType.name, 'name': 'fake_name', 'link': 'fake_link'} + ], + ] + + self.all_valid_tab_list = xmodule_tabs.CourseTabList().from_json(self.valid_tabs[1]) + + +@attr('shard_1') +class ValidateTabsTestCase(TabListTestCase): + """Test cases for validating tabs.""" + + def test_validate_tabs(self): + tab_list = xmodule_tabs.CourseTabList() + for invalid_tab_list in self.invalid_tabs: + with self.assertRaises(xmodule_tabs.InvalidTabsException): + tab_list.from_json(invalid_tab_list) + + for valid_tab_list in self.valid_tabs: + from_json_result = tab_list.from_json(valid_tab_list) + self.assertEquals(len(from_json_result), len(valid_tab_list)) + + def test_invalid_tab_type(self): + """ + Verifies that having an unrecognized tab type does not cause + the tabs to be undisplayable. + """ + tab_list = xmodule_tabs.CourseTabList() + self.assertEquals( + len(tab_list.from_json([ + {'type': CoursewareViewType.name}, + {'type': CourseInfoViewType.name, 'name': 'fake_name'}, + {'type': 'no_such_type'} + ])), + 2 + ) + + +@attr('shard_1') +class CourseTabListTestCase(TabListTestCase): + """Testing the generator method for iterating through displayable tabs""" + + def has_tab(self, tab_list, tab_type): + """ Searches the given lab_list for a given tab_type. """ + for tab in tab_list: + if tab.type == tab_type: + return True + return False + + def test_initialize_default_without_syllabus(self): + self.course.tabs = [] + self.course.syllabus_present = False + xmodule_tabs.CourseTabList.initialize_default(self.course) + self.assertFalse(self.has_tab(self.course.tabs, 'syllabus')) + + def test_initialize_default_with_syllabus(self): + self.course.tabs = [] + self.course.syllabus_present = True + xmodule_tabs.CourseTabList.initialize_default(self.course) + self.assertTrue(self.has_tab(self.course.tabs, 'syllabus')) + + def test_initialize_default_with_external_link(self): + self.course.tabs = [] + self.course.discussion_link = "other_discussion_link" + xmodule_tabs.CourseTabList.initialize_default(self.course) + self.assertTrue(self.has_tab(self.course.tabs, 'external_discussion')) + self.assertFalse(self.has_tab(self.course.tabs, 'discussion')) + + def test_initialize_default_without_external_link(self): + self.course.tabs = [] + self.course.discussion_link = "" + xmodule_tabs.CourseTabList.initialize_default(self.course) + self.assertFalse(self.has_tab(self.course.tabs, 'external_discussion')) + self.assertTrue(self.has_tab(self.course.tabs, 'discussion')) + + @patch.dict("django.conf.settings.FEATURES", { + "ENABLE_TEXTBOOK": True, + "ENABLE_DISCUSSION_SERVICE": True, + "ENABLE_STUDENT_NOTES": True, + "ENABLE_EDXNOTES": True, + }) + def test_iterate_displayable(self): + self.course.hide_progress_tab = False + + # create 1 book per textbook type + self.set_up_books(1) + + # initialize the course tabs to a list of all valid tabs + self.course.tabs = self.all_valid_tab_list + + # enumerate the tabs with no user + for i, tab in enumerate(xmodule_tabs.CourseTabList.iterate_displayable( + self.course, + inline_collections=False + )): + self.assertEquals(tab.type, self.course.tabs[i].type) + + # enumerate the tabs with a staff user + user = UserFactory(is_staff=True) + CourseEnrollment.enroll(user, self.course.id) + for i, tab in enumerate(xmodule_tabs.CourseTabList.iterate_displayable(self.course, user=user)): + if getattr(tab, 'is_collection_item', False): + # a collection item was found as a result of a collection tab + self.assertTrue(getattr(self.course.tabs[i], 'is_collection', False)) + else: + # all other tabs must match the expected type + self.assertEquals(tab.type, self.course.tabs[i].type) + + # test including non-empty collections + self.assertIn( + {'type': 'html_textbooks'}, + list(xmodule_tabs.CourseTabList.iterate_displayable(self.course, inline_collections=False)), + ) + + # test not including empty collections + self.course.html_textbooks = [] + self.assertNotIn( + {'type': 'html_textbooks'}, + list(xmodule_tabs.CourseTabList.iterate_displayable(self.course, inline_collections=False)), + ) + + def test_get_tab_by_methods(self): + """Tests the get_tab methods in CourseTabList""" + self.course.tabs = self.all_valid_tab_list + for tab in self.course.tabs: + + # get tab by type + self.assertEquals(xmodule_tabs.CourseTabList.get_tab_by_type(self.course.tabs, tab.type), tab) + + # get tab by id + self.assertEquals(xmodule_tabs.CourseTabList.get_tab_by_id(self.course.tabs, tab.tab_id), tab) + + +@attr('shard_1') +class ProgressTestCase(TabTestCase): + """Test cases for Progress Tab.""" + + 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'}, + expected_link=self.reverse('progress', args=[self.course.id.to_deprecated_string()]), + expected_tab_id=ProgressCourseViewType.name, + invalid_dict_tab=None, + ) + + @patch('student.models.CourseEnrollment.is_enrolled') + def test_progress(self, is_enrolled): + is_enrolled.return_value = True + self.course.hide_progress_tab = False + tab = self.check_progress_tab() + self.check_can_display_results( + tab, for_staff_only=True, for_enrolled_users_only=True + ) + + self.course.hide_progress_tab = True + self.check_progress_tab() + self.check_can_display_results( + tab, for_staff_only=True, for_enrolled_users_only=True, expected_value=False + ) + + +@attr('shard_1') +class StaticTabTestCase(TabTestCase): + """Test cases for Static Tab.""" + + def test_static_tab(self): + + url_slug = 'schmug' + + tab = self.check_tab( + tab_class=StaticCourseViewType, + dict_tab={'type': StaticCourseViewType.name, '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, + ) + self.check_can_display_results(tab) + self.check_get_and_set_method_for_key(tab, 'url_slug') + + +@attr('shard_1') +class DiscussionLinkTestCase(TabTestCase): + """Test cases for discussion link tab.""" + + def setUp(self): + super(DiscussionLinkTestCase, self).setUp() + + self.tabs_with_discussion = [ + xmodule_tabs.CourseTab.load('discussion'), + ] + self.tabs_without_discussion = [ + ] + + @staticmethod + def _reverse(course): + """Custom reverse function""" + def reverse_discussion_link(viewname, args): + """reverse lookup for discussion link""" + if viewname == "django_comment_client.forum.views.forum_form_discussion" and args == [unicode(course.id)]: + return "default_discussion_link" + return reverse_discussion_link + + def check_discussion( + self, tab_list, + expected_discussion_link, + expected_can_display_value, + discussion_link_in_course="", + is_staff=True, + is_enrolled=True, + ): + """Helper function to verify whether the discussion tab exists and can be displayed""" + self.course.tabs = tab_list + self.course.discussion_link = discussion_link_in_course + discussion_tab = xmodule_tabs.CourseTabList.get_discussion(self.course) + user = self.create_mock_user(is_authenticated=True, is_staff=is_staff, is_enrolled=is_enrolled) + with patch('student.models.CourseEnrollment.is_enrolled') as check_is_enrolled: + check_is_enrolled.return_value = is_enrolled + self.assertEquals( + ( + discussion_tab is not None and + self.is_tab_enabled(discussion_tab, self.course, user) and + (discussion_tab.link_func(self.course, self._reverse(self.course)) == expected_discussion_link) + ), + expected_can_display_value + ) + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": False}) + def test_explicit_discussion_link(self): + """Test that setting discussion_link overrides everything else""" + self.check_discussion( + tab_list=self.tabs_with_discussion, + discussion_link_in_course="other_discussion_link", + expected_discussion_link="other_discussion_link", + expected_can_display_value=True, + ) + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": False}) + def test_discussions_disabled(self): + """Test that other cases return None with discussions disabled""" + for tab_list in [[], self.tabs_with_discussion, self.tabs_without_discussion]: + self.check_discussion( + tab_list=tab_list, + expected_discussion_link=not None, + expected_can_display_value=False, + ) + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def test_tabs_with_discussion(self): + """Test a course with a discussion tab configured""" + self.check_discussion( + tab_list=self.tabs_with_discussion, + expected_discussion_link="default_discussion_link", + expected_can_display_value=True, + ) + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def test_tabs_without_discussion(self): + """Test a course with tabs configured but without a discussion tab""" + self.check_discussion( + tab_list=self.tabs_without_discussion, + expected_discussion_link=not None, + expected_can_display_value=False, + ) + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def test_tabs_enrolled_or_staff(self): + for is_enrolled, is_staff in [(True, False), (False, True)]: + self.check_discussion( + tab_list=self.tabs_with_discussion, + expected_discussion_link="default_discussion_link", + expected_can_display_value=True, + is_enrolled=is_enrolled, + is_staff=is_staff + ) + + @patch.dict("django.conf.settings.FEATURES", {"ENABLE_DISCUSSION_SERVICE": True}) + def test_tabs_not_enrolled_or_staff(self): + is_enrolled = is_staff = False + self.check_discussion( + tab_list=self.tabs_with_discussion, + expected_discussion_link="default_discussion_link", + expected_can_display_value=False, + is_enrolled=is_enrolled, + is_staff=is_staff + ) diff --git a/lms/djangoapps/courseware/views.py b/lms/djangoapps/courseware/views.py index 2e2fab6f4c..4eaf6c413c 100644 --- a/lms/djangoapps/courseware/views.py +++ b/lms/djangoapps/courseware/views.py @@ -51,13 +51,14 @@ from courseware.models import StudentModule, StudentModuleHistory from course_modes.models import CourseMode from open_ended_grading import open_ended_notifications +from open_ended_grading.views import StaffGradingTab, PeerGradingTab, OpenEndedGradingTab from student.models import UserTestGroup, CourseEnrollment from student.views import is_course_blocked from util.cache import cache, cache_if_anonymous from xblock.fragment import Fragment from xmodule.modulestore.django import modulestore from xmodule.modulestore.exceptions import ItemNotFoundError, NoPathToItem -from xmodule.tabs import CourseTabList, StaffGradingTab, PeerGradingTab, OpenEndedGradingTab +from xmodule.tabs import CourseTabList from xmodule.x_module import STUDENT_VIEW import shoppingcart from shoppingcart.models import CourseRegistrationCode @@ -741,8 +742,6 @@ def static_tab(request, course_id, tab_slug): 'tab_contents': contents, }) -# TODO arjun: remove when custom tabs in place, see courseware/syllabus.py - @ensure_csrf_cookie @ensure_valid_course_key @@ -1136,13 +1135,13 @@ def notification_image_for_tab(course_tab, user, course): """ tab_notification_handlers = { - StaffGradingTab.type: open_ended_notifications.staff_grading_notifications, - PeerGradingTab.type: open_ended_notifications.peer_grading_notifications, - OpenEndedGradingTab.type: open_ended_notifications.combined_notifications + StaffGradingTab.name: open_ended_notifications.staff_grading_notifications, + PeerGradingTab.name: open_ended_notifications.peer_grading_notifications, + OpenEndedGradingTab.name: open_ended_notifications.combined_notifications } - if course_tab.type in tab_notification_handlers: - notifications = tab_notification_handlers[course_tab.type](course, user) + if course_tab.name in tab_notification_handlers: + notifications = tab_notification_handlers[course_tab.name](course, user) if notifications and notifications['pending_grading']: return notifications['img_path'] diff --git a/lms/djangoapps/discussion_api/api.py b/lms/djangoapps/discussion_api/api.py index 6c072176f8..f4df4a7974 100644 --- a/lms/djangoapps/discussion_api/api.py +++ b/lms/djangoapps/discussion_api/api.py @@ -24,7 +24,6 @@ from django_comment_client.utils import get_accessible_discussion_modules from lms.lib.comment_client.thread import Thread from lms.lib.comment_client.utils import CommentClientRequestError from openedx.core.djangoapps.course_groups.cohorts import get_cohort_id -from xmodule.tabs import DiscussionTab def _get_course_or_404(course_key, user): @@ -34,7 +33,7 @@ def _get_course_or_404(course_key, user): disabled for the course. """ course = get_course_with_access(user, 'load_forum', course_key) - if not any([isinstance(tab, DiscussionTab) for tab in course.tabs]): + if not any([tab.type == 'discussion' for tab in course.tabs]): raise Http404 return course diff --git a/lms/djangoapps/discussion_api/tests/test_api.py b/lms/djangoapps/discussion_api/tests/test_api.py index 601b6a8cea..296070cf3a 100644 --- a/lms/djangoapps/discussion_api/tests/test_api.py +++ b/lms/djangoapps/discussion_api/tests/test_api.py @@ -44,7 +44,6 @@ from xmodule.modulestore.django import modulestore from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.partitions.partitions import Group, UserPartition -from xmodule.tabs import DiscussionTab def _remove_discussion_tab(course, user_id): @@ -53,7 +52,7 @@ def _remove_discussion_tab(course, user_id): user_id is passed to the modulestore as the editor of the module. """ - course.tabs = [tab for tab in course.tabs if not isinstance(tab, DiscussionTab)] + course.tabs = [tab for tab in course.tabs if not tab.type == 'discussion'] modulestore().update_item(course, user_id) diff --git a/lms/djangoapps/django_comment_client/forum/views.py b/lms/djangoapps/django_comment_client/forum/views.py index 1e66a97c9f..44e3e4e1c3 100644 --- a/lms/djangoapps/django_comment_client/forum/views.py +++ b/lms/djangoapps/django_comment_client/forum/views.py @@ -8,10 +8,12 @@ import logging import xml.sax.saxutils as saxutils from django.contrib.auth.decorators import login_required +from django.conf import settings from django.core.context_processors import csrf from django.core.urlresolvers import reverse from django.contrib.auth.models import User from django.http import Http404, HttpResponseBadRequest +from django.utils.translation import ugettext as _ from django.views.decorators.http import require_GET import newrelic.agent @@ -23,8 +25,10 @@ from openedx.core.djangoapps.course_groups.cohorts import ( get_course_cohorts, is_commentable_cohorted ) +from courseware.tabs import EnrolledCourseViewType from courseware.access import has_access from xmodule.modulestore.django import modulestore +from ccx.overrides import get_current_ccx from django_comment_client.permissions import cached_has_permission from django_comment_client.utils import ( @@ -45,6 +49,27 @@ PAGES_NEARBY_DELTA = 2 log = logging.getLogger("edx.discussions") +class DiscussionCourseViewType(EnrolledCourseViewType): + """ + A tab for the cs_comments_service forums. + """ + + name = '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): + return False + + if settings.FEATURES.get('CUSTOM_COURSES_EDX', False): + if get_current_ccx(): + return False + return settings.FEATURES.get('ENABLE_DISCUSSION_SERVICE') + + def _attr_safe_json(obj): """ return a JSON string for obj which is safe to embed as the value of an attribute in a DOM node diff --git a/lms/djangoapps/django_comment_client/tests/test_utils.py b/lms/djangoapps/django_comment_client/tests/test_utils.py index 39ab33876b..ca69ceb0be 100644 --- a/lms/djangoapps/django_comment_client/tests/test_utils.py +++ b/lms/djangoapps/django_comment_client/tests/test_utils.py @@ -6,11 +6,8 @@ from nose.plugins.attrib import attr from pytz import UTC from django.utils.timezone import UTC as django_utc -from django_comment_client.tests.factories import RoleFactory -from django_comment_client.tests.unicode import UnicodeTestMixin -import django_comment_client.utils as utils from django.core.urlresolvers import reverse -from django.test import TestCase +from django.test import TestCase, RequestFactory from edxmako import add_lookup from django_comment_client.tests.factories import RoleFactory @@ -18,8 +15,9 @@ from django_comment_client.tests.unicode import UnicodeTestMixin import django_comment_client.utils as utils from courseware.tests.factories import InstructorFactory +from courseware.tabs import get_course_tab_list from openedx.core.djangoapps.course_groups.cohorts import set_course_cohort_settings -from student.tests.factories import UserFactory, CourseEnrollmentFactory +from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentFactory from openedx.core.djangoapps.util.testing import ContentGroupTestCase from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase @@ -951,3 +949,37 @@ class RenderMustacheTests(TestCase): """ add_lookup('main', '', package=__name__) self.assertEqual(utils.render_mustache('test.mustache', {}), 'Testing 1 2 3.\n') + + +class DiscussionTabTestCase(ModuleStoreTestCase): + """ Test visibility of the discussion tab. """ + + def setUp(self): + super(DiscussionTabTestCase, self).setUp() + self.course = CourseFactory.create() + self.enrolled_user = UserFactory.create() + self.staff_user = AdminFactory.create() + CourseEnrollmentFactory.create(user=self.enrolled_user, course_id=self.course.id) + self.unenrolled_user = UserFactory.create() + + def discussion_tab_present(self, user): + """ Returns true if the user has access to the discussion tab. """ + request = RequestFactory().request() + request.user = user + all_tabs = get_course_tab_list(request, self.course) + return any(tab.type == 'discussion' for tab in all_tabs) + + def test_tab_access(self): + with self.settings(FEATURES={'ENABLE_DISCUSSION_SERVICE': True}): + self.assertTrue(self.discussion_tab_present(self.staff_user)) + self.assertTrue(self.discussion_tab_present(self.enrolled_user)) + self.assertFalse(self.discussion_tab_present(self.unenrolled_user)) + + @mock.patch('ccx.overrides.get_current_ccx') + def test_tab_settings(self, mock_get_ccx): + mock_get_ccx.return_value = True + with self.settings(FEATURES={'ENABLE_DISCUSSION_SERVICE': False}): + self.assertFalse(self.discussion_tab_present(self.enrolled_user)) + + with self.settings(FEATURES={'CUSTOM_COURSES_EDX': True}): + self.assertFalse(self.discussion_tab_present(self.enrolled_user)) diff --git a/lms/djangoapps/edxnotes/plugins.py b/lms/djangoapps/edxnotes/plugins.py index 62b1e10e9b..b99c5df5ee 100644 --- a/lms/djangoapps/edxnotes/plugins.py +++ b/lms/djangoapps/edxnotes/plugins.py @@ -4,10 +4,10 @@ Registers the "edX Notes" feature for the edX platform. from django.utils.translation import ugettext as _ -from openedx.core.lib.plugins.api import CourseViewType +from courseware.tabs import EnrolledCourseViewType -class EdxNotesCourseViewType(CourseViewType): +class EdxNotesCourseViewType(EnrolledCourseViewType): """ The representation of the edX Notes course view type. """ @@ -15,13 +15,9 @@ class EdxNotesCourseViewType(CourseViewType): name = "edxnotes" title = _("Notes") view_name = "edxnotes" - is_persistent = True - - # The course field that indicates that this feature is enabled - feature_flag_field_name = "edxnotes" @classmethod - def is_enabled(cls, course, settings, user=None): # pylint: disable=unused-argument + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument """Returns true if the edX Notes feature is enabled in the course. Args: @@ -29,4 +25,6 @@ class EdxNotesCourseViewType(CourseViewType): 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): + return False return course.edxnotes diff --git a/lms/djangoapps/edxnotes/tests.py b/lms/djangoapps/edxnotes/tests.py index 398a64fda7..5d5be0fd9d 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -25,14 +25,14 @@ from xmodule.tabs import CourseTab from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor from courseware.tabs import get_course_tab_list -from student.tests.factories import UserFactory +from student.tests.factories import UserFactory, CourseEnrollmentFactory def enable_edxnotes_for_the_course(course, user_id): """ Enable EdxNotes for the course. """ - course.tabs.append(CourseTab.from_json({"type": "edxnotes", "name": "Notes"})) + course.tabs.append(CourseTab.load("edxnotes")) modulestore().update_item(course, user_id) @@ -798,6 +798,7 @@ class EdxNotesViewsTest(ModuleStoreTestCase): super(EdxNotesViewsTest, self).setUp() self.course = CourseFactory.create(edxnotes=True) self.user = UserFactory.create(username="Bob", email="bob@example.com", password="edx") + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) self.client.login(username=self.user.username, password="edx") self.notes_page_url = reverse("edxnotes", args=[unicode(self.course.id)]) self.search_url = reverse("search_notes", args=[unicode(self.course.id)]) @@ -820,10 +821,16 @@ class EdxNotesViewsTest(ModuleStoreTestCase): request = RequestFactory().request() request.user = user tabs = get_course_tab_list(request, course) - return len([tab for tab in tabs if tab.name == 'Notes']) == 1 + return len([tab for tab in tabs if tab.type == 'edxnotes']) == 1 self.assertFalse(has_notes_tab(self.user, self.course)) enable_edxnotes_for_the_course(self.course, self.user.id) + # disable course.edxnotes + self.course.edxnotes = False + self.assertFalse(has_notes_tab(self.user, self.course)) + + # reenable course.edxnotes + self.course.edxnotes = True self.assertTrue(has_notes_tab(self.user, self.course)) # pylint: disable=unused-argument diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 869ce50534..b0227f8ae3 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -38,7 +38,7 @@ 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.lib.plugins.api import CourseViewType +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 @@ -55,10 +55,10 @@ class InstructorDashboardViewType(CourseViewType): name = "instructor" title = _('Instructor') view_name = "instructor_dashboard" - is_persistent = False + is_dynamic = True # The "Instructor" tab is instead dynamically added when it is enabled @classmethod - def is_enabled(cls, course, settings, user=None): # pylint: disable=unused-argument,redefined-outer-name + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument,redefined-outer-name """ Returns true if the specified user has staff access. """ diff --git a/lms/djangoapps/notes/tests.py b/lms/djangoapps/notes/tests.py index 7aab06e1c8..75f0d2bdd3 100644 --- a/lms/djangoapps/notes/tests.py +++ b/lms/djangoapps/notes/tests.py @@ -5,27 +5,30 @@ Unit tests for the notes app. from mock import patch, Mock from opaque_keys.edx.locations import SlashSeparatedCourseKey -from django.test import TestCase +from django.test import TestCase, RequestFactory from django.test.client import Client from django.core.urlresolvers import reverse from django.contrib.auth.models import User from django.core.exceptions import ValidationError -import collections import json +from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase +from xmodule.modulestore.tests.factories import CourseFactory +from courseware.tabs import get_course_tab_list, CourseTab +from student.tests.factories import UserFactory, CourseEnrollmentFactory from notes import utils, api, models -class UtilsTest(TestCase): +class UtilsTest(ModuleStoreTestCase): + """ Tests for the notes utils. """ def setUp(self): ''' Setup a dummy course-like object with a tabs field that can be accessed via attribute lookup. ''' super(UtilsTest, self).setUp() - self.course = collections.namedtuple('DummyCourse', ['tabs']) - self.course.tabs = [] + self.course = CourseFactory.create() def test_notes_not_enabled(self): ''' @@ -39,11 +42,54 @@ class UtilsTest(TestCase): Tests that notes are enabled when the course tab configuration contains a tab with type "notes." ''' - self.course.tabs = [{'type': 'foo'}, - {'name': 'My Notes', 'type': 'notes'}, - {'type': 'bar'}] + with self.settings(FEATURES={'ENABLE_STUDENT_NOTES': True}): + self.course.advanced_modules = ["notes"] + self.assertTrue(utils.notes_enabled_for_course(self.course)) - self.assertTrue(utils.notes_enabled_for_course(self.course)) + +class CourseTabTest(ModuleStoreTestCase): + """ + Test that the course tab shows up the way we expect. + """ + def setUp(self): + ''' + Setup a dummy course-like object with a tabs field that can be + accessed via attribute lookup. + ''' + super(CourseTabTest, self).setUp() + self.course = CourseFactory.create() + self.user = UserFactory() + CourseEnrollmentFactory.create(user=self.user, course_id=self.course.id) + + def enable_notes(self): + """Enable notes and add the tab to the course.""" + self.course.tabs.append(CourseTab.load("notes")) + self.course.advanced_modules = ["notes"] + + def has_notes_tab(self, course, user): + """ Returns true if the current course and user have a notes tab, false otherwise. """ + request = RequestFactory().request() + request.user = user + all_tabs = get_course_tab_list(request, course) + return any([tab.name == u'My Notes' for tab in all_tabs]) + + def test_course_tab_not_visible(self): + # module not enabled in the course + self.assertFalse(self.has_notes_tab(self.course, self.user)) + + with self.settings(FEATURES={'ENABLE_STUDENT_NOTES': False}): + # setting not enabled and the module is not enabled + self.assertFalse(self.has_notes_tab(self.course, self.user)) + + # module is enabled and the setting is not enabled + self.course.advanced_modules = ["notes"] + self.assertFalse(self.has_notes_tab(self.course, self.user)) + + def test_course_tab_visible(self): + self.enable_notes() + self.assertTrue(self.has_notes_tab(self.course, self.user)) + self.course.advanced_modules = [] + self.assertFalse(self.has_notes_tab(self.course, self.user)) class ApiTest(TestCase): diff --git a/lms/djangoapps/notes/utils.py b/lms/djangoapps/notes/utils.py index c340206945..b0add40ff5 100644 --- a/lms/djangoapps/notes/utils.py +++ b/lms/djangoapps/notes/utils.py @@ -11,7 +11,7 @@ def notes_enabled_for_course(course): 2) present in the course tab configuration. ''' - tab_found = next((True for t in course.tabs if t['type'] == 'notes'), False) + tab_found = "notes" in course.advanced_modules feature_enabled = settings.FEATURES.get('ENABLE_STUDENT_NOTES') return feature_enabled and tab_found diff --git a/lms/djangoapps/notes/views.py b/lms/djangoapps/notes/views.py index f121e2473a..f512ddc13b 100644 --- a/lms/djangoapps/notes/views.py +++ b/lms/djangoapps/notes/views.py @@ -1,11 +1,19 @@ +""" +Views to support the edX Notes feature. +""" + +from django.conf import settings from django.contrib.auth.decorators import login_required 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 notes.models import Note from notes.utils import notes_enabled_for_course from xmodule.annotator_token import retrieve_token +from django.utils.translation import ugettext as _ @login_required @@ -30,3 +38,18 @@ def notes(request, course_id): } return render_to_response('notes.html', context) + + +class NotesCourseViewType(EnrolledCourseViewType): + """ + A tab for the course notes. + """ + name = 'notes' + title = _("My Notes") + view_name = "notes" + + @classmethod + def is_enabled(cls, course, user=None): + if not super(NotesCourseViewType, 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/tests.py b/lms/djangoapps/open_ended_grading/tests.py index 4614e8bf47..ead4b101ff 100644 --- a/lms/djangoapps/open_ended_grading/tests.py +++ b/lms/djangoapps/open_ended_grading/tests.py @@ -26,8 +26,7 @@ from student.models import unique_id_for_user from xmodule import peer_grading_module from xmodule.error_module import ErrorDescriptor from xmodule.modulestore.django import modulestore -from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE +from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE, ModuleStoreTestCase from xmodule.modulestore.xml_importer import import_course_from_xml from xmodule.open_ended_grading_classes import peer_grading_service, controller_query_service from xmodule.tests import test_util_open_ended diff --git a/lms/djangoapps/open_ended_grading/views.py b/lms/djangoapps/open_ended_grading/views.py index 03fe8ec592..6e1285b501 100644 --- a/lms/djangoapps/open_ended_grading/views.py +++ b/lms/djangoapps/open_ended_grading/views.py @@ -4,7 +4,11 @@ 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 xmodule.open_ended_grading_classes.grading_service_module import GradingServiceError import json @@ -62,6 +66,55 @@ ALERT_DICT = { } +class StaffGradingTab(CourseViewType): + """ + A tab for staff grading. + """ + name = 'staff_grading' + title = _("Staff grading") + view_name = "staff_grading" + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + if user and not has_access(user, 'staff', course, course.id): + return False + return "combinedopenended" in course.advanced_modules + + +class PeerGradingTab(EnrolledCourseViewType): + """ + A tab for peer grading. + """ + name = 'peer_grading' + # Translators: "Peer grading" appears on a tab that allows + # students to view open-ended problems that require grading + title = _("Peer grading") + view_name = "peer_grading" + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + if not super(PeerGradingTab, cls).is_enabled(course, user=user): + return False + return "combinedopenended" in course.advanced_modules + + +class OpenEndedGradingTab(EnrolledCourseViewType): + """ + A tab for open ended grading. + """ + name = '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") + view_name = "open_ended_notifications" + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + if not super(OpenEndedGradingTab, cls).is_enabled(course, user=user): + return False + return "combinedopenended" in course.advanced_modules + + @cache_control(no_cache=True, no_store=True, must_revalidate=True) def staff_grading(request, course_id): """ diff --git a/lms/templates/help_modal.html b/lms/templates/help_modal.html index 62cc29221f..233d3a623f 100644 --- a/lms/templates/help_modal.html +++ b/lms/templates/help_modal.html @@ -37,7 +37,7 @@ <% discussion_tab = CourseTabList.get_discussion(course) if course else None - discussion_link = discussion_tab.link_func(course, reverse) if (discussion_tab and discussion_tab.is_enabled(course, settings, user=user)) else None + discussion_link = discussion_tab.link_func(course, reverse) if (discussion_tab and discussion_tab.is_enabled(course, user=user)) else None %> % if discussion_link: diff --git a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py index ede2581223..37ad200f39 100644 --- a/openedx/core/djangoapps/course_groups/tests/test_cohorts.py +++ b/openedx/core/djangoapps/course_groups/tests/test_cohorts.py @@ -9,13 +9,12 @@ from django.contrib.auth.models import User from django.db import IntegrityError from django.http import Http404 from django.test import TestCase -from django.test.utils import override_settings from opaque_keys.edx.locations import SlashSeparatedCourseKey from student.models import CourseEnrollment from student.tests.factories import UserFactory -from xmodule.modulestore.django import modulestore, clear_existing_modulestores -from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE, mixed_store_config, ModuleStoreTestCase +from xmodule.modulestore.django import modulestore +from xmodule.modulestore.tests.django_utils import TEST_DATA_MIXED_TOY_MODULESTORE, ModuleStoreTestCase from ..models import CourseUserGroup, CourseCohort, CourseUserGroupPartitionGroup from .. import cohorts diff --git a/openedx/core/lib/plugins/__init__.py b/openedx/core/djangoapps/course_views/__init__.py similarity index 100% rename from openedx/core/lib/plugins/__init__.py rename to openedx/core/djangoapps/course_views/__init__.py diff --git a/openedx/core/djangoapps/course_views/course_views.py b/openedx/core/djangoapps/course_views/course_views.py new file mode 100644 index 0000000000..976546b462 --- /dev/null +++ b/openedx/core/djangoapps/course_views/course_views.py @@ -0,0 +1,201 @@ +""" +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/lib/plugins/tests/__init__.py b/openedx/core/djangoapps/course_views/tests/__init__.py similarity index 100% rename from openedx/core/lib/plugins/tests/__init__.py rename to openedx/core/djangoapps/course_views/tests/__init__.py diff --git a/openedx/core/lib/plugins/tests/test_api.py b/openedx/core/djangoapps/course_views/tests/test_api.py similarity index 78% rename from openedx/core/lib/plugins/tests/test_api.py rename to openedx/core/djangoapps/course_views/tests/test_api.py index e48f17ab93..4f558a1002 100644 --- a/openedx/core/lib/plugins/tests/test_api.py +++ b/openedx/core/djangoapps/course_views/tests/test_api.py @@ -4,7 +4,8 @@ Tests for the plugin API from django.test import TestCase -from ..api import CourseViewTypeManager, PluginError +from openedx.core.lib.api.plugins import PluginError +from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager class TestPluginApi(TestCase): diff --git a/openedx/core/djangoapps/course_views/tests/test_course_views.py b/openedx/core/djangoapps/course_views/tests/test_course_views.py new file mode 100644 index 0000000000..04f5bf0b45 --- /dev/null +++ b/openedx/core/djangoapps/course_views/tests/test_course_views.py @@ -0,0 +1,74 @@ +""" Tests of specific tabs. """ + +from mock import patch, Mock +from unittest import TestCase + +import xmodule.tabs as xmodule_tabs + +from openedx.core.djangoapps.course_views.course_views import CourseViewTypeManager + + +class CourseViewTypeManagerTestCase(TestCase): + """Test cases for CourseViewTypeManager class""" + + @patch('openedx.core.djangoapps.course_views.course_views.CourseViewTypeManager.get_available_plugins') + def test_get_course_view_types(self, get_available_plugins): + """ + Verify that get_course_view_types sorts appropriately + """ + def create_mock_plugin(name, priority): + """ Create a mock plugin with the specified name and priority. """ + mock_plugin = Mock() + mock_plugin.name = name + 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), + } + get_available_plugins.return_value = mock_plugins + self.assertEqual( + [plugin.name for plugin in CourseViewTypeManager.get_course_view_types()], + ["First", "Second", "Third", "Duplicate", "Duplicate", "Last"] + ) + + +class KeyCheckerTestCase(TestCase): + """Test cases for KeyChecker class""" + + def setUp(self): + super(KeyCheckerTestCase, self).setUp() + + self.valid_keys = ['a', 'b'] + self.invalid_keys = ['a', 'v', 'g'] + self.dict_value = {'a': 1, 'b': 2, 'c': 3} + + def test_key_checker(self): + + self.assertTrue(xmodule_tabs.key_checker(self.valid_keys)(self.dict_value, raise_error=False)) + self.assertFalse(xmodule_tabs.key_checker(self.invalid_keys)(self.dict_value, raise_error=False)) + with self.assertRaises(xmodule_tabs.InvalidTabsException): + xmodule_tabs.key_checker(self.invalid_keys)(self.dict_value) + + +class NeedNameTestCase(TestCase): + """Test cases for NeedName validator""" + + def setUp(self): + super(NeedNameTestCase, self).setUp() + + self.valid_dict1 = {'a': 1, 'name': 2} + self.valid_dict2 = {'name': 1} + self.valid_dict3 = {'a': 1, 'name': 2, 'b': 3} + self.invalid_dict = {'a': 1, 'b': 2} + + def test_need_name(self): + self.assertTrue(xmodule_tabs.need_name(self.valid_dict1)) + self.assertTrue(xmodule_tabs.need_name(self.valid_dict2)) + self.assertTrue(xmodule_tabs.need_name(self.valid_dict3)) + with self.assertRaises(xmodule_tabs.InvalidTabsException): + xmodule_tabs.need_name(self.invalid_dict) diff --git a/openedx/core/lib/plugins/api.py b/openedx/core/lib/api/plugins.py similarity index 51% rename from openedx/core/lib/plugins/api.py rename to openedx/core/lib/api/plugins.py index 23df185ff1..576dc9cec5 100644 --- a/openedx/core/lib/plugins/api.py +++ b/openedx/core/lib/api/plugins.py @@ -4,9 +4,6 @@ Adds support for first class features that can be added to the edX platform. from stevedore.extension import ExtensionManager -# Stevedore extension point namespaces -COURSE_VIEW_TYPE_NAMESPACE = 'openedx.course_view_type' - class PluginError(Exception): """ @@ -46,44 +43,3 @@ class PluginManager(object): namespace=cls.NAMESPACE # pylint: disable=no-member )) return plugins[name] - - -class CourseViewType(object): - """ - Base class of all course view type plugins. - """ - name = None - title = None - view_name = None - is_persistent = False - - # The course field that indicates that this feature is enabled - feature_flag_field_name = None - - @classmethod - def is_enabled(cls, course, settings, 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 - settings (dict): a dict of configuration settings - user (User): the user interacting with the course - """ - 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 - - -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 diff --git a/setup.py b/setup.py index c8dd55388f..1a4c167f6f 100644 --- a/setup.py +++ b/setup.py @@ -20,8 +20,26 @@ setup( 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", + "textbooks = lms.djangoapps.courseware.tabs:TextbookCourseViews", + "wiki = lms.djangoapps.course_wiki.tab:WikiCourseViewType", + + # ORA 1 tabs (deprecated) + "peer_grading = lms.djangoapps.open_ended_grading.views:PeerGradingTab", + "staff_grading = lms.djangoapps.open_ended_grading.views:StaffGradingTab", + "open_ended = lms.djangoapps.open_ended_grading.views:OpenEndedGradingTab", ], "openedx.user_partition_scheme": [ "random = openedx.core.djangoapps.user_api.partition_schemes:RandomUserPartitionScheme",