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 a060f21eac..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 @@ -788,7 +788,7 @@ class CourseMetadataEditingTest(CourseTestCase): ) self.assertNotIn('edxnotes', test_model) - def test_validate_and_update_from_json_correct_inputs(self): + def test_validate_from_json_correct_inputs(self): is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, { @@ -802,16 +802,11 @@ class CourseMetadataEditingTest(CourseTestCase): self.assertTrue(len(errors) == 0) self.update_check(test_model) - # fresh fetch to ensure persistence - fresh = modulestore().get_course(self.course.id) - test_model = CourseMetadata.fetch(fresh) - self.update_check(test_model) - # Tab gets tested in test_advanced_settings_munge_tabs self.assertIn('advanced_modules', test_model, 'Missing advanced_modules') self.assertEqual(test_model['advanced_modules']['value'], ['combinedopenended'], 'advanced_module is not updated') - def test_validate_and_update_from_json_wrong_inputs(self): + def test_validate_from_json_wrong_inputs(self): # input incorrectly formatted data is_valid, errors, test_model = CourseMetadata.validate_and_update_from_json( self.course, @@ -933,36 +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.client.ajax_post(self.course_setting_url, { - ADVANCED_COMPONENT_POLICY_KEY: {"value": []} - }) - 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.assertNotIn(notes_tab, course.tabs) - @patch.dict(settings.FEATURES, {'ENABLE_EDXNOTES': True}) - def test_course_settings_munge_tabs(self): - """ - Test that adding and removing specific course settings adds and removes tabs. - """ - self.assertNotIn(EXTRA_TAB_PANELS.get("edxnotes"), self.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, { - "edxnotes": {"value": True} + ADVANCED_COMPONENT_POLICY_KEY: {"value": ["combinedopenended", "notes"]} }) course = modulestore().get_course(self.course.id) - self.assertIn(EXTRA_TAB_PANELS.get("edxnotes"), 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, { - "edxnotes": {"value": False} + ADVANCED_COMPONENT_POLICY_KEY: {"value": ["notes"]} }) course = modulestore().get_course(self.course.id) - self.assertNotIn(EXTRA_TAB_PANELS.get("edxnotes"), course.tabs) + 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/tests/test_utils.py b/cms/djangoapps/contentstore/tests/test_utils.py index a9a0c42474..c29231b2ea 100644 --- a/cms/djangoapps/contentstore/tests/test_utils.py +++ b/cms/djangoapps/contentstore/tests/test_utils.py @@ -108,61 +108,6 @@ class ExtraPanelTabTestCase(TestCase): course.tabs = tabs return course - def test_add_extra_panel_tab(self): - """ Tests if a tab can be added to a course tab list. """ - for tab_type in utils.EXTRA_TAB_PANELS.keys(): - tab = utils.EXTRA_TAB_PANELS.get(tab_type) - - # test adding with changed = True - for tab_setup in ['', 'x', 'x,y,z']: - course = self.get_course_with_tabs(tab_setup) - expected_tabs = copy.copy(course.tabs) - expected_tabs.append(tab) - changed, actual_tabs = utils.add_extra_panel_tab(tab_type, course) - self.assertTrue(changed) - self.assertEqual(actual_tabs, expected_tabs) - - # test adding with changed = False - tab_test_setup = [ - [tab], - [tab, self.get_tab_type_dicts('x,y,z')], - [self.get_tab_type_dicts('x,y'), tab, self.get_tab_type_dicts('z')], - [self.get_tab_type_dicts('x,y,z'), tab]] - - for tab_setup in tab_test_setup: - course = self.get_course_with_tabs(tab_setup) - expected_tabs = copy.copy(course.tabs) - changed, actual_tabs = utils.add_extra_panel_tab(tab_type, course) - self.assertFalse(changed) - self.assertEqual(actual_tabs, expected_tabs) - - def test_remove_extra_panel_tab(self): - """ Tests if a tab can be removed from a course tab list. """ - for tab_type in utils.EXTRA_TAB_PANELS.keys(): - tab = utils.EXTRA_TAB_PANELS.get(tab_type) - - # test removing with changed = True - tab_test_setup = [ - [tab], - [tab, self.get_tab_type_dicts('x,y,z')], - [self.get_tab_type_dicts('x,y'), tab, self.get_tab_type_dicts('z')], - [self.get_tab_type_dicts('x,y,z'), tab]] - - for tab_setup in tab_test_setup: - course = self.get_course_with_tabs(tab_setup) - expected_tabs = [t for t in course.tabs if t != utils.EXTRA_TAB_PANELS.get(tab_type)] - changed, actual_tabs = utils.remove_extra_panel_tab(tab_type, course) - self.assertTrue(changed) - self.assertEqual(actual_tabs, expected_tabs) - - # test removing with changed = False - for tab_setup in ['', 'x', 'x,y,z']: - course = self.get_course_with_tabs(tab_setup) - expected_tabs = copy.copy(course.tabs) - changed, actual_tabs = utils.remove_extra_panel_tab(tab_type, course) - self.assertFalse(changed) - self.assertEqual(actual_tabs, expected_tabs) - class CourseImageTestCase(ModuleStoreTestCase): """Tests for course image URLs.""" diff --git a/cms/djangoapps/contentstore/tests/utils.py b/cms/djangoapps/contentstore/tests/utils.py index 3eaf65b592..551f5ef17a 100644 --- a/cms/djangoapps/contentstore/tests/utils.py +++ b/cms/djangoapps/contentstore/tests/utils.py @@ -95,7 +95,7 @@ class CourseTestCase(ModuleStoreTestCase): client = AjaxEnabledTestClient() if authenticate: client.login(username=nonstaff.username, password=password) - nonstaff.is_authenticated = True + nonstaff.is_authenticated = lambda: authenticate return client, nonstaff def populate_course(self, branching=2): diff --git a/cms/djangoapps/contentstore/utils.py b/cms/djangoapps/contentstore/utils.py index c8fe7ce15d..5828d4e761 100644 --- a/cms/djangoapps/contentstore/utils.py +++ b/cms/djangoapps/contentstore/utils.py @@ -3,7 +3,6 @@ Common utility functions useful throughout the contentstore """ # pylint: disable=no-member -import copy import logging import re from datetime import datetime @@ -27,12 +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"} -EDXNOTES_PANEL = {"name": _("Notes"), "type": "edxnotes"} -EXTRA_TAB_PANELS = dict([(p['type'], p) for p in [OPEN_ENDED_PANEL, NOTES_PANEL, EDXNOTES_PANEL]]) - def add_instructor(course_key, requesting_user, new_instructor): """ @@ -287,46 +280,6 @@ def ancestor_has_staff_lock(xblock, parent_xblock=None): return parent_xblock.visible_to_staff_only -def add_extra_panel_tab(tab_type, course): - """ - Used to add the panel tab to a course if it does not exist. - @param tab_type: A string representing the tab type. - @param course: A course object from the modulestore. - @return: Boolean indicating whether or not a tab was added and a list of tabs for the course. - """ - # Copy course tabs - course_tabs = copy.copy(course.tabs) - changed = False - # Check to see if open ended panel is defined in the course - - tab_panel = EXTRA_TAB_PANELS.get(tab_type) - if tab_panel not in course_tabs: - # Add panel to the tabs if it is not defined - course_tabs.append(tab_panel) - changed = True - return changed, course_tabs - - -def remove_extra_panel_tab(tab_type, course): - """ - Used to remove the panel tab from a course if it exists. - @param tab_type: A string representing the tab type. - @param course: A course object from the modulestore. - @return: Boolean indicating whether or not a tab was added and a list of tabs for the course. - """ - # Copy course tabs - course_tabs = copy.copy(course.tabs) - changed = False - # Check to see if open ended panel is defined in the course - - tab_panel = EXTRA_TAB_PANELS.get(tab_type) - if tab_panel in course_tabs: - # Add panel to the tabs if it is not defined - course_tabs = [ct for ct in course_tabs if ct != tab_panel] - changed = True - return changed, course_tabs - - def reverse_url(handler_name, key_name=None, key_value=None, kwargs=None): """ Creates the URL for the given handler. diff --git a/cms/djangoapps/contentstore/views/course.py b/cms/djangoapps/contentstore/views/course.py index 4fe48d432c..9e5cc1ce16 100644 --- a/cms/djangoapps/contentstore/views/course.py +++ b/cms/djangoapps/contentstore/views/course.py @@ -1,6 +1,7 @@ """ Views related to operations on course objects """ +import copy from django.shortcuts import redirect import json import random @@ -15,14 +16,14 @@ 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 +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 @@ -42,11 +43,8 @@ from contentstore.utils import ( add_instructor, initialize_permissions, get_lms_link_for_item, - add_extra_panel_tab, - remove_extra_panel_tab, reverse_course_url, reverse_library_url, - reverse_usage_url, reverse_url, remove_all_instructors, ) @@ -57,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, ) @@ -83,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 ( @@ -993,101 +987,36 @@ def grading_handler(request, course_key_string, grader_index=None): return JsonResponse() -# pylint: disable=invalid-name -def _add_tab(request, tab_type, course_module): +def _refresh_course_tabs(request, course_module): """ - Adds tab to the course. + Automatically adds/removes tabs if changes to the course require them. """ - # Add tab to the course if needed - changed, new_tabs = add_extra_panel_tab(tab_type, course_module) - # If a tab has been added to the course, then send the - # metadata along to CourseMetadata.update_from_json - if changed: - course_module.tabs = new_tabs - request.json.update({'tabs': {'value': new_tabs}}) - # Indicate that tabs should not be filtered out of - # the metadata - return True - return False + def update_tab(tabs, tab_type, tab_enabled): + """ + Adds or removes a course tab based upon whether it is 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 not tab_enabled and has_tab: + tabs.remove(tab_panel) -# pylint: disable=invalid-name -def _remove_tab(request, tab_type, course_module): - """ - Removes the tab from the course. - """ - changed, new_tabs = remove_extra_panel_tab(tab_type, course_module) - if changed: - course_module.tabs = new_tabs - request.json.update({'tabs': {'value': new_tabs}}) - return True - return False + course_tabs = copy.copy(course_module.tabs) + # Additionally update any tabs that are provided by non-dynamic course views + for tab_type in CourseViewTypeManager.get_course_view_types(): + 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) -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]) - - -# pylint: disable=invalid-name -def _modify_tabs_to_components(request, course_module): - """ - Automatically adds/removes tabs if user indicated that they want - respective modules enabled in the course - - Return True when tab configuration has been modified. - """ - 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), - # student notes tab - 'edxnotes': (is_field_value_true, ['edxnotes']) - } - - tabs_changed = False - 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 - - if tab_enabled: - # check passed, some of this component_types are present, adding tab - if _add_tab(request, tab_type, course_module): - # tab indeed was added, the change needs to propagate - tabs_changed = True - else: - # the tab should not be present (anymore) - if _remove_tab(request, tab_type, course_module): - # tab indeed was removed, the change needs to propagate - tabs_changed = True - - return tabs_changed + # Save the tabs into the course if they have been changed + if course_tabs != course_module.tabs: + course_module.tabs = course_tabs @login_required @@ -1119,18 +1048,21 @@ def advanced_settings_handler(request, course_key_string): return JsonResponse(CourseMetadata.fetch(course_module)) else: try: - # do not process tabs unless they were modified according to course metadata - filter_tabs = not _modify_tabs_to_components(request, course_module) - - # validate data formats and update + # 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_and_update_from_json( course_module, request.json, - filter_tabs=filter_tabs, user=request.user, ) if is_valid: + # update the course tabs if required by any setting changes + _refresh_course_tabs(request, course_module) + + # now update mongo + modulestore().update_item(course_module, request.user.id) + return JsonResponse(updated_data) else: return JsonResponseBadRequest(errors) @@ -1249,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) @@ -1266,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 131be1d946..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,10 +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_cms( - course_item, - settings, - ): + 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_import_export.py b/cms/djangoapps/contentstore/views/tests/test_import_export.py index 17c30f51fa..3375a30d09 100644 --- a/cms/djangoapps/contentstore/views/tests/test_import_export.py +++ b/cms/djangoapps/contentstore/views/tests/test_import_export.py @@ -116,7 +116,7 @@ class ImportTestCase(CourseTestCase): Check that course is imported successfully in existing course and users have their access roles """ # Create a non_staff user and add it to course staff only - __, nonstaff_user = self.create_non_staff_authed_user_client(authenticate=False) + __, nonstaff_user = self.create_non_staff_authed_user_client() auth.add_users(self.user, CourseStaffRole(self.course.id), nonstaff_user) course = self.store.get_course(self.course.id) 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 0eb74623da..3516ae6176 100644 --- a/cms/djangoapps/models/settings/course_metadata.py +++ b/cms/djangoapps/models/settings/course_metadata.py @@ -154,7 +154,8 @@ class CourseMetadata(object): """ Validate the values in the json dict (validated by xblock fields from_json method) - If all fields validate, go ahead and update those values in the database. + 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: @@ -183,19 +184,19 @@ class CourseMetadata(object): # If did validate, go ahead and update the metadata if did_validate: - updated_data = cls.update_from_dict(key_values, descriptor, user) + updated_data = cls.update_from_dict(key_values, descriptor, user, save=False) return did_validate, errors, updated_data @classmethod - def update_from_dict(cls, key_values, descriptor, user): + def update_from_dict(cls, key_values, descriptor, user, save=True): """ - Update metadata descriptor in modulestore from key_values. + Update metadata descriptor from key_values. Saves to modulestore if save is true. """ for key, value in key_values.iteritems(): setattr(descriptor, key, value) - if len(key_values): + if save and len(key_values): modulestore().update_item(descriptor, user.id) return cls.fetch(descriptor) 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/models.py b/common/djangoapps/student/models.py index 944a074ace..507a94865e 100644 --- a/common/djangoapps/student/models.py +++ b/common/djangoapps/student/models.py @@ -1191,6 +1191,8 @@ class CourseEnrollment(models.Model): `course_id` is our usual course_id string (e.g. "edX/Test101/2013_Fall) """ + if not user.is_authenticated(): + return False try: record = CourseEnrollment.objects.get(user=user, course_id=course_key) return record.is_active 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 1dd6b72eba..973c24bdb0 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -1,13 +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): """ @@ -25,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 @@ -51,33 +60,20 @@ class CourseTab(object): self.link_func = link_func - def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): # pylint: disable=unused-argument + def is_enabled(self, course, user=None): # pylint: disable=unused-argument """ - Determines whether the tab should be displayed in the UI for the given course and a particular user. - This method is to be overridden by subclasses when applicable. The base class implementation - always returns True. + Determines whether the tab is enabled for the given course and a particular user. + This method is to be overridden by subclasses when applicable. The base class + implementation always returns True. 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'] - - is_user_authenticated: Indicates whether the user is authenticated. If the tab is of - type AuthenticatedCourseTab and this value is False, then can_display will return False. - - is_user_staff: Indicates whether the user has staff access to the course. If the tab is of - type StaffTab and this value is False, then can_display will return False. - - is_user_enrolled: Indicates whether the user is enrolled in the course + user: An optional user for whom the tab will be displayed. If none, + then the code should assume a staff user or an author. Returns: - A boolean value to indicate whether this instance of the tab should be displayed to a - given user for the given course. + A boolean value to indicate whether this instance of the tab is enabled. """ return True @@ -150,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. @@ -168,610 +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. """ - sub_class_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, - 'edxnotes': EdxNotesTab, - 'syllabus': SyllabusTab, - 'instructor': InstructorTab, # not persisted - 'ccx_coach': CcxCoachTab, # not persisted - } - - tab_type = tab_dict.get('type') - if tab_type not in sub_class_types: - raise InvalidTabsException( - 'Unknown tab type {0}. Known types: {1}'.format(tab_type, sub_class_types) + # TODO: don't import openedx capabilities from common + 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() ) - - tab_class = sub_class_types[tab_dict['type']] - tab_class.validate(tab_dict) - return tab_class(tab_dict=tab_dict) - - -class AuthenticatedCourseTab(CourseTab): - """ - Abstract class for tabs that can be accessed by only authenticated users. - """ - def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - return is_user_authenticated - - -class StaffTab(AuthenticatedCourseTab): - """ - Abstract class for tabs that can be accessed by only users with staff access. - """ - def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): # pylint: disable=unused-argument - return is_user_staff - - -class EnrolledOrStaffTab(CourseTab): - """ - Abstract class for tabs that can be accessed by only users with staff access - or users enrolled in the course. - """ - def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): # pylint: disable=unused-argument - return is_user_authenticated and (is_user_staff or is_user_enrolled) - - -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 can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - super_can_display = super(ProgressTab, self).can_display( - course, settings, is_user_authenticated, is_user_staff, is_user_enrolled - ) - 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 can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - return settings.WIKI_ENABLED and ( - course.allow_public_wiki_access or is_user_enrolled or is_user_staff - ) - - @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 can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - 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).can_display( - course, settings, is_user_authenticated, is_user_staff, is_user_enrolled - ) - 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 can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - 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] - ), - ) - - -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 can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - 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 can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - 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 EdxNotesTab(AuthenticatedCourseTab): - """ - A tab for the course student notes. - """ - type = 'edxnotes' - - def can_display(self, course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - return settings.FEATURES.get('ENABLE_EDXNOTES') - - def __init__(self, tab_dict=None): - super(EdxNotesTab, self).__init__( - name=tab_dict['name'] if tab_dict else _('Notes'), - tab_id=self.type, - link_func=link_reverse_func(self.type), - ) - - @classmethod - def validate(cls, tab_dict, raise_error=True): - return super(EdxNotesTab, cls).validate(tab_dict, raise_error) and need_name(tab_dict, raise_error) - - -class InstructorTab(StaffTab): - """ - A tab for the course instructors. - """ - type = 'instructor' - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(InstructorTab, self).__init__( - # Translators: 'Instructor' appears on the tab that leads to the instructor dashboard, which is - # a portal where an instructor can get data and perform various actions on their course - name=_('Instructor'), - tab_id=self.type, - link_func=link_reverse_func('instructor_dashboard'), - ) - - -class CcxCoachTab(CourseTab): - """ - A tab for the custom course coaches. - """ - type = 'ccx_coach' - - def __init__(self, tab_dict=None): # pylint: disable=unused-argument - super(CcxCoachTab, self).__init__( - name=_('CCX Coach'), - tab_id=self.type, - link_func=link_reverse_func('ccx_coach_dashboard'), - ) - - def can_display(self, course, settings, *args, **kw): - """ - Since we don't get the user here, we use a thread local defined in the ccx - overrides to get it, then use the course to get the coach role and find out if - the user is one. - """ - user_is_coach = False - if settings.FEATURES.get('CUSTOM_COURSES_EDX', False) and course.enable_ccx: - from opaque_keys.edx.locations import SlashSeparatedCourseKey - from student.roles import CourseCcxCoachRole # pylint: disable=import-error - from ccx.overrides import get_current_request # pylint: disable=import-error - course_id = course.id.to_deprecated_string() - course_key = SlashSeparatedCourseKey.from_deprecated_string(course_id) - role = CourseCcxCoachRole(course_key) - request = get_current_request() - if request is not None: - user_is_coach = role.has_user(request.user) - super_can_display = super(CcxCoachTab, self).can_display( - course, settings, *args, **kw - ) - return user_is_coach and super_can_display + return None + tab_type.validate(tab_dict) + return tab_type.create_tab(tab_dict=tab_dict) class CourseTabList(List): @@ -780,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): """ @@ -789,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 @@ -851,48 +293,23 @@ class CourseTabList(List): return next((tab for tab in tab_list if tab.tab_id == tab_id), None) @staticmethod - def iterate_displayable( - course, - settings, - is_user_authenticated=True, - is_user_staff=True, - is_user_enrolled=False - ): + 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.can_display( - course, settings, is_user_authenticated, is_user_staff, is_user_enrolled - ) and (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: - for item in tab.items(course): - yield item + # If rendering inline that add each item in the collection, + # else just show the tab itself as long as it is not empty. + if inline_collections: + for item in tab.items(course): + yield item + elif len(list(tab.items(course))) > 0: + yield tab else: yield tab - instructor_tab = InstructorTab() - if instructor_tab.can_display(course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - yield instructor_tab - ccx_coach_tab = CcxCoachTab() - if ccx_coach_tab.can_display(course, settings, is_user_authenticated, is_user_staff, is_user_enrolled): - yield ccx_coach_tab - - @staticmethod - def iterate_displayable_cms( - course, - settings - ): - """ - Generator method for iterating through all tabs that can be displayed for the given course - with the provided settings. - """ - for tab in course.tabs: - if tab.can_display(course, settings, is_user_authenticated=True, is_user_staff=True, is_user_enrolled=True): - if tab.is_collection and not len(list(tab.items(course))): - # do not yield collections that have no items - continue - yield tab @classmethod def validate_tabs(cls, tabs): @@ -911,24 +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, - EdxNotesTab.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): @@ -965,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 3c9b54bb43..0000000000 --- a/common/lib/xmodule/xmodule/tests/test_tabs.py +++ /dev/null @@ -1,769 +0,0 @@ -"""Tests for Tab classes""" -from mock import MagicMock -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 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: - self.assertEquals( - expected_value, - tab.can_display( - self.course, self.settings, is_user_authenticated=True, is_user_staff=True, is_user_enrolled=True - ) - ) - if for_authenticated_users_only: - self.assertEquals( - expected_value, - tab.can_display( - self.course, self.settings, is_user_authenticated=True, is_user_staff=False, is_user_enrolled=False - ) - ) - if not for_staff_only and not for_authenticated_users_only and not for_enrolled_users_only: - self.assertEquals( - expected_value, - tab.can_display( - self.course, self.settings, is_user_authenticated=False, is_user_staff=False, is_user_enrolled=False - ) - ) - if for_enrolled_users_only: - self.assertEquals( - expected_value, - tab.can_display( - self.course, self.settings, is_user_authenticated=True, is_user_staff=False, is_user_enrolled=True - ) - ) - - 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) - - def test_textbooks_enabled(self): - - type_to_reverse_name = {'textbook': 'book', 'pdftextbook': 'pdf_book', 'htmltextbook': 'html_book'} - - self.settings.FEATURES['ENABLE_TEXTBOOK'] = True - num_textbooks_found = 0 - for tab in tabs.CourseTabList.iterate_displayable(self.course, self.settings): - # 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 InstructorTestCase(TabTestCase): - """Test cases for Instructor Tab.""" - - def test_instructor_tab(self): - name = 'Instructor' - tab = self.check_tab( - tab_class=tabs.InstructorTab, - dict_tab={'type': tabs.InstructorTab.type, 'name': name}, - expected_name=name, - expected_link=self.reverse('instructor_dashboard', args=[self.course.id.to_deprecated_string()]), - expected_tab_id=tabs.InstructorTab.type, - invalid_dict_tab=None, - ) - self.check_can_display_results(tab, for_staff_only=True) - - -class EdxNotesTestCase(TabTestCase): - """ - Test cases for Notes Tab. - """ - - def check_edxnotes_tab(self): - """ - Helper function for verifying the edxnotes tab. - """ - return self.check_tab( - tab_class=tabs.EdxNotesTab, - dict_tab={'type': tabs.EdxNotesTab.type, 'name': 'same'}, - expected_link=self.reverse('edxnotes', args=[self.course.id.to_deprecated_string()]), - expected_tab_id=tabs.EdxNotesTab.type, - invalid_dict_tab=self.fake_dict_tab, - ) - - def test_edxnotes_tabs_enabled(self): - """ - Tests that edxnotes tab is shown when feature is enabled. - """ - self.settings.FEATURES['ENABLE_EDXNOTES'] = True - tab = self.check_edxnotes_tab() - self.check_can_display_results(tab, for_authenticated_users_only=True) - - def test_edxnotes_tabs_disabled(self): - """ - Tests that edxnotes tab is not shown when feature is disabled. - """ - self.settings.FEATURES['ENABLE_EDXNOTES'] = False - tab = self.check_edxnotes_tab() - self.check_can_display_results(tab, expected_value=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, - tabs.EdxNotesTab.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}, - {'type': tabs.EdxNotesTab.type, 'name': 'fake_name'}, - ], - # 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) - - def test_iterate_displayable(self): - # 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 using the CMS call - for i, tab in enumerate(tabs.CourseTabList.iterate_displayable_cms( - self.course, - self.settings, - )): - self.assertEquals(tab.type, self.course.tabs[i].type) - - # enumerate the tabs and verify textbooks and the instructor tab - for i, tab in enumerate(tabs.CourseTabList.iterate_displayable( - self.course, - self.settings, - )): - 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)) - elif i == len(self.course.tabs): - # the last tab must be the Instructor tab - self.assertEquals(tab.type, tabs.InstructorTab.type) - 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_cms(self.course, self.settings)), - ) - - # test not including empty collections - self.course.html_textbooks = [] - self.assertNotIn( - tabs.HtmlTextbookTabs(), - list(tabs.CourseTabList.iterate_displayable_cms(self.course, self.settings)), - ) - - 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 = tabs.CourseTabList.get_discussion(self.course) - self.assertEquals( - ( - discussion is not None and - discussion.can_display(self.course, self.settings, True, is_staff, is_enrolled) and - (discussion.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/overrides.py b/lms/djangoapps/ccx/overrides.py index 752de4bca6..b9573a027f 100644 --- a/lms/djangoapps/ccx/overrides.py +++ b/lms/djangoapps/ccx/overrides.py @@ -64,15 +64,6 @@ def get_current_ccx(): return _CCX_CONTEXT.ccx -def get_current_request(): - """ - Return the active request, so that we can get context information in places - where it is limited, like in the tabs. - """ - request = _CCX_CONTEXT.request - return request - - def get_override_for_ccx(ccx, block, name, default=None): """ Gets the value of the overridden field for the `ccx`. `block` and `name` diff --git a/lms/djangoapps/ccx/plugins.py b/lms/djangoapps/ccx/plugins.py new file mode 100644 index 0000000000..3479fa9a56 --- /dev/null +++ b/lms/djangoapps/ccx/plugins.py @@ -0,0 +1,32 @@ +""" +Registers the CCX feature for the edX platform. +""" + +from django.conf import settings +from django.utils.translation import ugettext as _ + +from openedx.core.djangoapps.course_views.course_views import CourseViewType +from student.roles import CourseCcxCoachRole + + +class CcxCourseViewType(CourseViewType): + """ + The representation of the CCX course view type. + """ + + name = "ccx_coach" + title = _("CCX Coach") + view_name = "ccx_coach_dashboard" + is_dynamic = True # The CCX view is dynamically added to the set of tabs when it is enabled + + @classmethod + def is_enabled(cls, course, user=None): + """ + Returns true if CCX has been enabled and the specified user is a coach + """ + if not user: + return True + if not settings.FEATURES.get('CUSTOM_COURSES_EDX', False) or not course.enable_ccx: + return False + role = CourseCcxCoachRole(course.id) + return role.has_user(user) 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 cca02a73bb..7bb9f15ea5 100644 --- a/lms/djangoapps/courseware/tabs.py +++ b/lms/djangoapps/courseware/tabs.py @@ -3,44 +3,354 @@ This module is essentially a broker to xmodule/tabs.py -- it was originally intr perform some LMS-specific tab display gymnastics for the Entrance Exams feature """ from django.conf import settings -from django.test.client import RequestFactory from django.utils.translation import ugettext as _ from courseware.access import has_access from courseware.entrance_exams import user_must_complete_entrance_exam -from student.models import CourseEnrollment, EntranceExamConfiguration -from xmodule.tabs import CourseTabList - -from util import milestones_helpers +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 -def get_course_tab_list(course, user): +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): """ Retrieves the course tab list from xmodule.tabs and manipulates the set as necessary """ - user_is_enrolled = user.is_authenticated() and CourseEnrollment.is_enrolled(user, course.id) - xmodule_tab_list = CourseTabList.iterate_displayable( - course, - settings, - user.is_authenticated(), - has_access(user, 'staff', course, course.id), - user_is_enrolled - ) + user = request.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 - request = RequestFactory().request() - request.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 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): - # Hide all of the tabs except for 'Courseware' and 'Instructor' + # Hide all of the tabs except for 'Courseware' # Rename 'Courseware' tab to 'Entrance Exam' - if tab.type not in ['courseware', 'instructor']: + if tab.type is not 'courseware': continue - if tab.type == 'courseware': - tab.name = _("Entrance Exam") + tab.name = _("Entrance Exam") course_tab_list.append(tab) + + # Add in any dynamic tabs, i.e. those that are not persisted + course_tab_list += _get_dynamic_tabs(course, user) return course_tab_list + + +def _get_dynamic_tabs(course, user): + """ + Returns the dynamic tab types for the current user. + + Note: dynamic tabs are those that are not persisted in the course, but are + instead added dynamically based upon the user's role. + """ + dynamic_tabs = list() + 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 704977f934..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): @@ -54,14 +243,16 @@ class StaticTabDateTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.assertIn("OOGIE BLOOGIE", resp.content) def test_invalid_course_key(self): - request = get_request_for_user(UserFactory.create()) + self.setup_user() + request = get_request_for_user(self.user) with self.assertRaises(Http404): static_tab(request, course_id='edX/toy', tab_slug='new_tab') def test_get_static_tab_contents(self): + self.setup_user() course = get_course_by_id(self.toy_course_key) - request = get_request_for_user(UserFactory.create()) - tab = tabs.CourseTabList.get_tab_by_slug(course.tabs, 'resources') + request = get_request_for_user(self.user) + tab = xmodule_tabs.CourseTabList.get_tab_by_slug(course.tabs, 'resources') # Test render works okay tab_content = get_static_tab_contents(request, course, tab) @@ -162,6 +353,7 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): 'description': 'Testing Courseware Tabs' } self.user.is_staff = False + request = get_request_for_user(self.user) self.course.entrance_exam_enabled = True self.course.entrance_exam_id = unicode(entrance_exam.location) milestone = milestones_helpers.add_milestone(milestone) @@ -176,7 +368,7 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.relationship_types['FULFILLS'], milestone ) - course_tab_list = get_course_tab_list(self.course, self.user) + course_tab_list = get_course_tab_list(request, self.course) self.assertEqual(len(course_tab_list), 1) self.assertEqual(course_tab_list[0]['tab_id'], 'courseware') self.assertEqual(course_tab_list[0]['name'], 'Entrance Exam') @@ -201,7 +393,8 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): # log in again as student self.client.logout() self.login(self.email, self.password) - course_tab_list = get_course_tab_list(self.course, self.user) + request = get_request_for_user(self.user) + course_tab_list = get_course_tab_list(request, self.course) self.assertEqual(len(course_tab_list), 5) def test_course_tabs_list_for_staff_members(self): @@ -213,33 +406,28 @@ class EntranceExamsTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): self.client.logout() staff_user = StaffFactory(course_key=self.course.id) self.client.login(username=staff_user.username, password='test') - - course_tab_list = get_course_tab_list(self.course, staff_user) + request = get_request_for_user(staff_user) + course_tab_list = get_course_tab_list(request, self.course) self.assertEqual(len(course_tab_list), 5) @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): @@ -256,12 +444,12 @@ class TextBookTabsTestCase(LoginEnrollmentTestCase, ModuleStoreTestCase): Test that all textbooks tab links generating correctly. """ type_to_reverse_name = {'textbook': 'book', 'pdftextbook': 'pdf_book', 'htmltextbook': 'html_book'} - - course_tab_list = get_course_tab_list(self.course, self.user) + request = get_request_for_user(self.user) + course_tab_list = get_course_tab_list(request, self.course) 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], @@ -271,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 new file mode 100644 index 0000000000..b99c5df5ee --- /dev/null +++ b/lms/djangoapps/edxnotes/plugins.py @@ -0,0 +1,30 @@ +""" +Registers the "edX Notes" feature for the edX platform. +""" + +from django.utils.translation import ugettext as _ + +from courseware.tabs import EnrolledCourseViewType + + +class EdxNotesCourseViewType(EnrolledCourseViewType): + """ + The representation of the edX Notes course view type. + """ + + name = "edxnotes" + title = _("Notes") + view_name = "edxnotes" + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument + """Returns true if the edX Notes feature 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 + """ + 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 cce3aeb9f7..5d5be0fd9d 100644 --- a/lms/djangoapps/edxnotes/tests.py +++ b/lms/djangoapps/edxnotes/tests.py @@ -6,6 +6,7 @@ import jwt from mock import patch, MagicMock from unittest import skipUnless from datetime import datetime + from edxmako.shortcuts import render_to_string from edxnotes import helpers from edxnotes.decorators import edxnotes @@ -13,23 +14,25 @@ from edxnotes.exceptions import EdxNotesParseError, EdxNotesServiceUnavailable from django.conf import settings from django.core.urlresolvers import reverse from django.core.exceptions import ImproperlyConfigured +from django.test.client import RequestFactory from oauth2_provider.tests.factories import ClientFactory from provider.oauth2.models import Client -from xmodule.tabs import EdxNotesTab from xmodule.modulestore.tests.factories import CourseFactory, ItemFactory from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore import ModuleStoreEnum from xmodule.modulestore.django import modulestore +from xmodule.tabs import CourseTab from courseware.model_data import FieldDataCache from courseware.module_render import get_module_for_descriptor -from student.tests.factories import UserFactory +from courseware.tabs import get_course_tab_list +from student.tests.factories import UserFactory, CourseEnrollmentFactory def enable_edxnotes_for_the_course(course, user_id): """ Enable EdxNotes for the course. """ - course.tabs.append(EdxNotesTab()) + course.tabs.append(CourseTab.load("edxnotes")) modulestore().update_item(course, user_id) @@ -795,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)]) @@ -808,6 +812,27 @@ class EdxNotesViewsTest(ModuleStoreTestCase): field_data_cache = FieldDataCache([self.course], self.course.id, self.user) return get_module_for_descriptor(self.user, MagicMock(), self.course, field_data_cache, self.course.id) + def test_edxnotes_tab(self): + """ + Tests that edxnotes tab is shown only when the feature is enabled. + """ + def has_notes_tab(user, course): + """Returns true if the "Notes" tab is shown.""" + request = RequestFactory().request() + request.user = user + tabs = get_course_tab_list(request, course) + 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 @patch.dict("django.conf.settings.FEATURES", {"ENABLE_EDXNOTES": True}) @patch("edxnotes.views.get_notes", return_value=[]) diff --git a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py index e53e41535b..0439c25e13 100644 --- a/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py +++ b/lms/djangoapps/instructor/tests/views/test_instructor_dashboard.py @@ -6,7 +6,11 @@ from mock import patch from django.conf import settings from django.core.urlresolvers import reverse +from django.test.client import RequestFactory from django.test.utils import override_settings + +from courseware.tabs import get_course_tab_list +from courseware.tests.factories import UserFactory from courseware.tests.helpers import LoginEnrollmentTestCase from student.tests.factories import AdminFactory, UserFactory @@ -56,6 +60,21 @@ class TestInstructorDashboard(ModuleStoreTestCase, LoginEnrollmentTestCase): return 'Demographic data is now available in Example.'.format(unicode(self.course.id)) + def test_instructor_tab(self): + """ + Verify that the instructor tab appears for staff only. + """ + def has_instructor_tab(user, course): + """Returns true if the "Instructor" tab is shown.""" + request = RequestFactory().request() + request.user = user + tabs = get_course_tab_list(request, course) + return len([tab for tab in tabs if tab.name == 'Instructor']) == 1 + + self.assertTrue(has_instructor_tab(self.instructor, self.course)) + student = UserFactory.create() + self.assertFalse(has_instructor_tab(student, self.course)) + def test_default_currency_in_the_html_response(self): """ Test that checks the default currency_symbol ($) in the response diff --git a/lms/djangoapps/instructor/views/instructor_dashboard.py b/lms/djangoapps/instructor/views/instructor_dashboard.py index 2c44060ca0..56903b2768 100644 --- a/lms/djangoapps/instructor/views/instructor_dashboard.py +++ b/lms/djangoapps/instructor/views/instructor_dashboard.py @@ -38,15 +38,33 @@ from course_modes.models import CourseMode, CourseModesArchive from student.roles import CourseFinanceAdminRole, CourseSalesAdminRole from certificates.models import CertificateGenerationConfiguration from certificates import api as certs_api +from openedx.core.djangoapps.course_views.course_views import CourseViewType from class_dashboard.dashboard_data import get_section_display_name, get_array_section_has_problem from .tools import get_units_with_due_date, title_or_url, bulk_email_is_enabled_for_course from opaque_keys.edx.locations import SlashSeparatedCourseKey - log = logging.getLogger(__name__) +class InstructorDashboardViewType(CourseViewType): + """ + Defines the Instructor Dashboard view type that is shown as a course tab. + """ + + name = "instructor" + title = _('Instructor') + view_name = "instructor_dashboard" + is_dynamic = True # The "Instructor" tab is instead dynamically added when it is enabled + + @classmethod + def is_enabled(cls, course, user=None): # pylint: disable=unused-argument,redefined-outer-name + """ + Returns true if the specified user has staff access. + """ + return user and has_access(user, 'staff', course, course.id) + + @ensure_csrf_cookie @cache_control(no_cache=True, no_store=True, must_revalidate=True) def instructor_dashboard_2(request, course_id): 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/courseware/course_navigation.html b/lms/templates/courseware/course_navigation.html index 801279c7d6..6c78313fa3 100644 --- a/lms/templates/courseware/course_navigation.html +++ b/lms/templates/courseware/course_navigation.html @@ -54,7 +54,7 @@ def url_class(is_active):