diff --git a/cms/djangoapps/contentstore/management/commands/edit_course_tabs.py b/cms/djangoapps/contentstore/management/commands/edit_course_tabs.py index 9aa9a7cc72aa..65d45bf0b206 100644 --- a/cms/djangoapps/contentstore/management/commands/edit_course_tabs.py +++ b/cms/djangoapps/contentstore/management/commands/edit_course_tabs.py @@ -12,7 +12,7 @@ from courseware.courses import get_course_by_id -from xmodule.tabs import primitive_insert, primitive_delete +from contentstore.views import tabs from opaque_keys import InvalidKeyError from opaque_keys.edx.locations import SlashSeparatedCourseKey from opaque_keys.edx.keys import CourseKey @@ -25,6 +25,7 @@ def print_course(course): for index, item in enumerate(course.tabs): print index + 1, '"' + item.get('type') + '"', '"' + item.get('name', '') + '"' + # course.tabs looks like this # [{u'type': u'courseware'}, {u'type': u'course_info', u'name': u'Course Info'}, {u'type': u'textbooks'}, # {u'type': u'discussion', u'name': u'Discussion'}, {u'type': u'wiki', u'name': u'Wiki'}, @@ -83,7 +84,7 @@ def handle(self, *args, **options): raise CommandError(Command.delete_option.help) num = int(args[0]) if query_yes_no('Deleting tab {0} Confirm?'.format(num), default='no'): - primitive_delete(course, num - 1) # -1 for 0-based indexing + tabs.primitive_delete(course, num - 1) # -1 for 0-based indexing elif options['insert']: if len(args) != 3: raise CommandError(Command.insert_option.help) @@ -91,7 +92,7 @@ def handle(self, *args, **options): tab_type = args[1] name = args[2] if query_yes_no('Inserting tab {0} "{1}" "{2}" Confirm?'.format(num, tab_type, name), default='no'): - primitive_insert(course, num - 1, tab_type, name) # -1 as above + tabs.primitive_insert(course, num - 1, tab_type, name) # -1 as above except ValueError as e: # Cute: translate to CommandError so the CLI error prints nicely. raise CommandError(e) diff --git a/cms/djangoapps/contentstore/views/tabs.py b/cms/djangoapps/contentstore/views/tabs.py index 1bc54381d648..f79d4d07211b 100644 --- a/cms/djangoapps/contentstore/views/tabs.py +++ b/cms/djangoapps/contentstore/views/tabs.py @@ -12,6 +12,7 @@ from edxmako.shortcuts import render_to_response from xmodule.modulestore.django import modulestore +from xmodule.modulestore import ModuleStoreEnum from xmodule.tabs import CourseTabList, CourseTab, InvalidTabsException, StaticTab from opaque_keys.edx.keys import CourseKey, UsageKey @@ -167,3 +168,36 @@ def get_tab_by_locator(tab_list, usage_key_string): url_slug=item.location.name, ) return CourseTabList.get_tab_by_id(tab_list, static_tab.tab_id) + + +# "primitive" tab edit functions driven by the command line. +# These should be replaced/deleted by a more capable GUI someday. +# Note that the command line UI identifies the tabs with 1-based +# indexing, but this implementation code is standard 0-based. + +def validate_args(num, tab_type): + "Throws for the disallowed cases." + if num <= 1: + raise ValueError('Tabs 1 and 2 cannot be edited') + if tab_type == 'static_tab': + raise ValueError('Tabs of type static_tab cannot be edited here (use Studio)') + + +def primitive_delete(course, num): + "Deletes the given tab number (0 based)." + tabs = course.tabs + validate_args(num, tabs[num].get('type', '')) + del tabs[num] + # Note for future implementations: if you delete a static_tab, then Chris Dodge + # points out that there's other stuff to delete beyond this element. + # This code happens to not delete static_tab so it doesn't come up. + modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command) + + +def primitive_insert(course, num, tab_type, name): + "Inserts a new tab at the given number (0 based)." + validate_args(num, tab_type) + new_tab = CourseTab.from_json({u'type': unicode(tab_type), u'name': unicode(name)}) + tabs = course.tabs + tabs.insert(num, new_tab) + modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command) diff --git a/cms/djangoapps/contentstore/views/tests/test_tabs.py b/cms/djangoapps/contentstore/views/tests/test_tabs.py index 8fa631f8c44a..2ef15a481d41 100644 --- a/cms/djangoapps/contentstore/views/tests/test_tabs.py +++ b/cms/djangoapps/contentstore/views/tests/test_tabs.py @@ -1,7 +1,7 @@ """ Tests for tab functions (just primitive). """ import json -from xmodule.tabs import primitive_insert, primitive_delete +from contentstore.views import tabs from contentstore.tests.utils import CourseTestCase from contentstore.utils import reverse_course_url from xmodule.x_module import STUDENT_VIEW @@ -198,12 +198,12 @@ def test_delete(self): """Test primitive tab deletion.""" course = CourseFactory.create() with self.assertRaises(ValueError): - primitive_delete(course, 0) + tabs.primitive_delete(course, 0) with self.assertRaises(ValueError): - primitive_delete(course, 1) + tabs.primitive_delete(course, 1) with self.assertRaises(IndexError): - primitive_delete(course, 6) - primitive_delete(course, 2) + tabs.primitive_delete(course, 6) + tabs.primitive_delete(course, 2) self.assertFalse({u'type': u'textbooks'} in course.tabs) # Check that discussion has shifted up self.assertEquals(course.tabs[2], {'type': 'discussion', 'name': 'Discussion'}) @@ -211,16 +211,16 @@ def test_delete(self): def test_insert(self): """Test primitive tab insertion.""" course = CourseFactory.create() - primitive_insert(course, 2, 'notes', 'aname') + tabs.primitive_insert(course, 2, 'notes', 'aname') self.assertEquals(course.tabs[2], {'type': 'notes', 'name': 'aname'}) with self.assertRaises(ValueError): - primitive_insert(course, 0, 'notes', 'aname') + tabs.primitive_insert(course, 0, 'notes', 'aname') with self.assertRaises(ValueError): - primitive_insert(course, 3, 'static_tab', 'aname') + tabs.primitive_insert(course, 3, 'static_tab', 'aname') def test_save(self): """Test course saving.""" course = CourseFactory.create() - primitive_insert(course, 3, 'notes', 'aname') + tabs.primitive_insert(course, 3, 'notes', 'aname') course2 = modulestore().get_course(course.id) self.assertEquals(course2.tabs[3], {'type': 'notes', 'name': 'aname'}) diff --git a/common/lib/xmodule/xmodule/tabs.py b/common/lib/xmodule/xmodule/tabs.py index 6cc792b249af..08548d281721 100644 --- a/common/lib/xmodule/xmodule/tabs.py +++ b/common/lib/xmodule/xmodule/tabs.py @@ -6,8 +6,6 @@ import logging from xblock.fields import List -from xmodule.modulestore.django import modulestore -from xmodule.modulestore import ModuleStoreEnum 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 @@ -531,32 +529,3 @@ class UnequalTabsException(Exception): A complaint about tab lists being unequal """ pass - - -# Tab functions -def validate_args(num, tab_type): - "Throws for the disallowed cases." - if num <= 1: - raise ValueError('Tabs 1 and 2 cannot be edited') - if tab_type == 'static_tab': - raise ValueError('Tabs of type static_tab cannot be edited here (use Studio)') - - -def primitive_delete(course, num): - "Deletes the given tab number (0 based)." - tabs = course.tabs - validate_args(num, tabs[num].get('type', '')) - del tabs[num] - # Note for future implementations: if you delete a static_tab, then Chris Dodge - # points out that there's other stuff to delete beyond this element. - # This code happens to not delete static_tab so it doesn't come up. - modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command) - - -def primitive_insert(course, num, tab_type, name): - "Inserts a new tab at the given number (0 based)." - validate_args(num, tab_type) - new_tab = CourseTab.from_json({u'type': unicode(tab_type), u'name': unicode(name)}) - tabs = course.tabs - tabs.insert(num, new_tab) - modulestore().update_item(course, ModuleStoreEnum.UserID.primitive_command) diff --git a/lms/djangoapps/dashboard/sysadmin.py b/lms/djangoapps/dashboard/sysadmin.py index 855b13c60509..090c9584e350 100644 --- a/lms/djangoapps/dashboard/sysadmin.py +++ b/lms/djangoapps/dashboard/sysadmin.py @@ -38,7 +38,6 @@ import dashboard.git_import as git_import from django_comment_client.management_utils import rename_user as rename_user_util from dashboard.git_import import GitImportError -from dashboard.sysadmin_extensions import sysadmin_course_tabs from dashboard.models import CourseImportLog from external_auth.models import ExternalAuthMap from external_auth.views import generate_password @@ -382,48 +381,6 @@ def post(self, request): return render_to_response(self.template_name, context) -class CourseTabs(SysadminDashboardView): - """ - Handles rendering the view and processing requests for Edit Course Tabs - """ - - def get(self, request): - """ - Displays the form for Edit Course Tabs - """ - - if not request.user.is_superuser: - raise Http404 - - context = { - 'msg': self.msg, - 'djangopid': os.getpid(), - 'modeflag': {'course_tabs': 'active-section'}, - 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), - } - return render_to_response(self.template_name, context) - - def post(self, request): - """Handle requests to Edit Course Tabs""" - - if not request.user.is_superuser: - raise Http404 - - action = request.POST.get('action', '') - track.views.server_track(request, action, {}, page='course_tabs_sysdashboard') - - if action in ['get_current_tabs', 'delete_tab', 'insert_tab']: - self.msg = sysadmin_course_tabs.process_request(action, request) - - context = { - 'msg': self.msg, - 'djangopid': os.getpid(), - 'modeflag': {'course_tabs': 'active-section'}, - 'edx_platform_version': getattr(settings, 'EDX_PLATFORM_VERSION_STRING', ''), - } - return render_to_response(self.template_name, context) - - class Courses(SysadminDashboardView): """ This manages adding/updating courses from git, deleting courses, and @@ -637,7 +594,6 @@ def post(self, request): page='courses_sysdashboard') courses = {course.id: course for course in self.get_courses()} - if action == 'add_course': gitloc = request.POST.get('repo_location', '').strip().replace(' ', '').replace(';', '') branch = request.POST.get('repo_branch', '').strip().replace(' ', '').replace(';', '') diff --git a/lms/djangoapps/dashboard/sysadmin_extensions/__init__.py b/lms/djangoapps/dashboard/sysadmin_extensions/__init__.py deleted file mode 100644 index e69de29bb2d1..000000000000 diff --git a/lms/djangoapps/dashboard/sysadmin_extensions/sysadmin_course_tabs.py b/lms/djangoapps/dashboard/sysadmin_extensions/sysadmin_course_tabs.py deleted file mode 100644 index 7ccb11ed11df..000000000000 --- a/lms/djangoapps/dashboard/sysadmin_extensions/sysadmin_course_tabs.py +++ /dev/null @@ -1,110 +0,0 @@ -"""Helper module that processes edit_course_tabs requests""" -from django.utils.translation import ugettext as _ - -from courseware.courses import get_course_by_id -from opaque_keys.edx import locator -from opaque_keys import InvalidKeyError -from xmodule.tabs import primitive_insert, primitive_delete -from xmodule.tabs import InvalidTabsException - - -def process_request(action, request): - """Routes requests to the appropriate helper function""" - - course_id = request.POST.get('course_id', '').strip() - if action == 'get_current_tabs': - title = _('Current Tabs:') - message = get_current_tabs(course_id) - elif action == 'delete_tab': - title = _('Delete Tab Status:') - delete_tab_args = request.POST.get('tab_delete', '').strip() - message = delete_tab(course_id, delete_tab_args) - elif action == 'insert_tab': - title = _('Insert Tab Status:') - insert_tab_args = request.POST.get('tab_insert', '').strip() - message = insert_tab(course_id, insert_tab_args) - return u"

{title}

{message}

".format( - title=title, - message=message, - ) - - -def get_current_tabs(course_id): - """Displays a list of tabs for the given course""" - - try: - course_key = locator.CourseLocator.from_string(course_id) - course = get_course_by_id(course_key) - except InvalidKeyError: - message = _('Error - Invalid Course ID') - else: - # Translators: number, type, and name refer to tab number, tab type, and tab name - message = _('number, type, name') - for index, item in enumerate(course.tabs): - message += "
{tab_number}, {tab_type}, {tab_name}".format( - tab_number=index + 1, - tab_type=item['type'], - tab_name=item['name'], - ) - return message - - -def delete_tab(course_id, args): - """Deletes the specified tab from the course""" - - try: - tab_number = int(args) - course_key = locator.CourseLocator.from_string(course_id) - course = get_course_by_id(course_key) - except InvalidKeyError: - message = _('Error - Invalid Course ID') - return message - except ValueError: - message = _('Error - Invalid arguments. Expecting one argument [tab-number]') - return message - try: - primitive_delete(course, tab_number - 1) # -1 for 0-based indexing - message = _("Tab {tab_number} for course {course_key} successfully deleted".format( - tab_number=tab_number, - course_key=course_key, - )) - except ValueError as error: - message = _("Command Failed - {msg}".format( - msg=error, - )) - return message - - -def insert_tab(course_id, args): - """Inserts the specified tab into the list of tabs for this course""" - - try: - course_key = locator.CourseLocator.from_string(course_id) - course = get_course_by_id(course_key) - except InvalidKeyError: - message = _('Error - Invalid Course ID') - return message - args = [arg.strip() for arg in args.split(",")] - if len(args) != 3: - message = _('Error - Invalid number of arguments. Expecting [tab-number], [tab-type], [tab-name]') - return message - try: - tab_number = int(args[0]) - tab_type = args[1] - tab_name = args[2] - except ValueError: - message = _('Error - Invalid arguments. Expecting [tab-number], [tab-type], [tab-name]') - return message - try: - primitive_insert(course, tab_number - 1, tab_type, tab_name) # -1 as above - message = _("Tab {tab_number}, {tab_type}, {tab_name} for course {course_key} successfully created".format( - tab_number=tab_number, - tab_type=tab_type, - tab_name=tab_name, - course_key=course_key, - )) - except (ValueError, InvalidTabsException) as error: - message = _("Command Failed - {msg}".format( - msg=error, - )) - return message diff --git a/lms/djangoapps/dashboard/sysadmin_urls.py b/lms/djangoapps/dashboard/sysadmin_urls.py index 06752eb0cb9a..1a68d6a66430 100644 --- a/lms/djangoapps/dashboard/sysadmin_urls.py +++ b/lms/djangoapps/dashboard/sysadmin_urls.py @@ -10,7 +10,6 @@ '', url(r'^$', sysadmin.Users.as_view(), name="sysadmin"), url(r'^courses/?$', sysadmin.Courses.as_view(), name="sysadmin_courses"), - url(r'^course_tabs/?$', sysadmin.CourseTabs.as_view(), name="sysadmin_course_tabs"), url(r'^staffing/?$', sysadmin.Staffing.as_view(), name="sysadmin_staffing"), url(r'^gitlogs/?$', sysadmin.GitLogs.as_view(), name="gitlogs"), url(r'^gitlogs/(?P.+)$', sysadmin.GitLogs.as_view(), diff --git a/lms/djangoapps/dashboard/tests/test_sysadmin_course_tabs.py b/lms/djangoapps/dashboard/tests/test_sysadmin_course_tabs.py deleted file mode 100644 index 18cca3036a8d..000000000000 --- a/lms/djangoapps/dashboard/tests/test_sysadmin_course_tabs.py +++ /dev/null @@ -1,120 +0,0 @@ -"""Tests for the Edit Course Tabs Feature on the Sysadmin Page""" -import unittest -from mock import patch - -from django.conf import settings -from django.core.urlresolvers import reverse - -from dashboard.tests.test_sysadmin import SysadminBaseTestCase -from xmodule.modulestore.tests.factories import CourseFactory - - -@unittest.skipUnless( - settings.FEATURES.get('ENABLE_SYSADMIN_DASHBOARD'), - 'ENABLE_SYSADMIN_DASHBOARD not set', -) -class TestSysadminCourseTabs(SysadminBaseTestCase): - """Tests all code paths in sysadmin_course_tabs""" - - def setUp(self): - super(TestSysadminCourseTabs, self).setUp() - self.course = CourseFactory.create() - self._setsuperuser_login() - - def test_get_course_tabs_valid_course_id(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'get_current_tabs', - 'course_id': unicode(self.course.id), - }) - self.assertIn('number, type, name', response.content.decode('utf-8')) - - def test_get_course_tabs_invalid_course_id(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'get_current_tabs', - 'course_id': u'InvalidCourseID', - }) - self.assertIn('Error - Invalid Course ID', response.content.decode('utf-8')) - - def test_delete_tab_valid_course_id(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'delete_tab', - 'course_id': unicode(self.course.id), - 'tab_delete': u'3', - }) - self.assertIn("Tab 3 for course {course_key} successfully deleted".format( - course_key=unicode(self.course.id), - ), response.content.decode('utf-8')) - - def test_delete_tab_invalid_course_id(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'delete_tab', - 'course_id': u'InvalidCourseID', - 'tab_delete': u'3', - }) - self.assertIn('Error - Invalid Course ID', response.content.decode('utf-8')) - - def test_delete_tab_invalid_args(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'delete_tab', - 'course_id': unicode(self.course.id), - 'tab_delete': u'', - }) - self.assertIn('Error - Invalid arguments. Expecting one argument [tab-number]', response.content.decode('utf-8')) - - def test_delete_tab_throws_exception(self): - with patch('dashboard.sysadmin_extensions.sysadmin_course_tabs.primitive_delete') as mock_primitive_delete: - mock_primitive_delete.side_effect = ValueError() - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'delete_tab', - 'course_id': unicode(self.course.id), - 'tab_delete': u'3', - }) - self.assertIn("Command Failed - ", response.content.decode('utf-8')) - - def test_insert_tab_valid_course_id(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'insert_tab', - 'course_id': unicode(self.course.id), - 'tab_insert': u'3, progress, Course Progress', - }) - self.assertIn("Tab 3, progress, Course Progress for course {course_key} successfully created".format( - course_key=unicode(self.course.id), - ), response.content.decode('utf-8')) - - def test_insert_tab_invalid_course_id(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'insert_tab', - 'course_id': u'InvalidCourseID', - 'tab_insert': u'3, progress, Course Progress', - }) - self.assertIn("Error - Invalid Course ID", response.content.decode('utf-8')) - - def test_insert_tab_invalid_number_args(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'insert_tab', - 'course_id': unicode(self.course.id), - 'tab_insert': u'3, progress, Course Progress, extra arg', - }) - self.assertIn("Error - Invalid number of arguments. Expecting [tab-number], [tab-type], [tab-name]".format( - course_key=unicode(self.course.id), - ), response.content.decode('utf-8')) - - def test_insert_tab_invalid_args(self): - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'insert_tab', - 'course_id': unicode(self.course.id), - 'tab_insert': u'three, progress, Course Progress', - }) - self.assertIn("Error - Invalid arguments. Expecting [tab-number], [tab-type], [tab-name]".format( - course_key=unicode(self.course.id), - ), response.content.decode('utf-8')) - - def test_insert_tab_throws_exception(self): - with patch('dashboard.sysadmin_extensions.sysadmin_course_tabs.primitive_insert') as mock_primitive_insert: - mock_primitive_insert.side_effect = ValueError() - response = self.client.post(reverse('sysadmin_course_tabs'), { - 'action': 'insert_tab', - 'course_id': unicode(self.course.id), - 'tab_insert': u'3, progress, Course Progress', - }) - self.assertIn('Command Failed - ', response.content.decode('utf-8')) diff --git a/lms/static/js/sysadmin/course_tabs.js b/lms/static/js/sysadmin/course_tabs.js deleted file mode 100644 index cf1b0ae2c571..000000000000 --- a/lms/static/js/sysadmin/course_tabs.js +++ /dev/null @@ -1,27 +0,0 @@ -(function () { - function confirmAndSubmit(e) { - var $target = $(e.target); - var args = $target.siblings('input').val(); - var fmt = gettext('Are you sure you want to execute the command: %(command)s with arguments: %(args)s ?'); - var confirmMessage = interpolate(fmt, { - command: $target.val(), - args: args - }, true); - if (confirm(confirmMessage)) { - var $form = $('.sysadmin-dashboard-wrapper .sysadmin-course-tabs-form'); - var $hiddenInput = $(document.createElement('input')); - $hiddenInput.attr({ - type: 'hidden', - name: 'action' - }); - $hiddenInput.val($target.val()); - $hiddenInput.appendTo($form); - $form.submit(); - } - } - - $(function () { - $('.edit-course-tabs .insert-tab-button').click(confirmAndSubmit); - $('.edit-course-tabs .delete-tab-button').click(confirmAndSubmit); - }); -})(); diff --git a/lms/static/sass/multicourse/_sysadmin.scss b/lms/static/sass/multicourse/_sysadmin.scss index 81272ef909e9..1c6c20f5cd10 100644 --- a/lms/static/sass/multicourse/_sysadmin.scss +++ b/lms/static/sass/multicourse/_sysadmin.scss @@ -49,11 +49,3 @@ .mgmt-commands-body .checkbox-label { display: inline; } - -.edit-course-tabs .long-input { - width: 60%; -} - -.edit-course-tabs .unique-row { - margin-top: 20px; -} diff --git a/lms/templates/sysadmin_dashboard.html b/lms/templates/sysadmin_dashboard.html index 85656de80f5e..2714d167c3ff 100644 --- a/lms/templates/sysadmin_dashboard.html +++ b/lms/templates/sysadmin_dashboard.html @@ -7,15 +7,11 @@ <%block name="headextra"> <%static:css group='style-course'/> - %if modeflag.get('mgmt_commands'): %endif - %if modeflag.get('course_tabs'): - - %endif